diff --git a/src/config/config.d.ts b/src/config/config.d.ts index 240f99ba..e063fb4d 100644 --- a/src/config/config.d.ts +++ b/src/config/config.d.ts @@ -18,7 +18,7 @@ export interface Config { ignoreUnsupportedPackets: boolean; ignoreMalformedPackets: boolean; enforceGrammar: boolean; - additionalAllowedPackets: Array<{ new(): any }>; + additionalAllowedPackets: Array<{ new(): any, tag: enums.packet }>; versionString: string; commentString: string; allowInsecureDecryptionWithSigningKeys: boolean; diff --git a/src/packet/grammar.ts b/src/packet/grammar.ts index 77e4dcf0..ec72b40c 100644 --- a/src/packet/grammar.ts +++ b/src/packet/grammar.ts @@ -1,4 +1,5 @@ import enums from '../enums'; +import type { Config } from '../config'; export class GrammarError extends Error { constructor(...params: any[]) { @@ -12,58 +13,132 @@ export class GrammarError extends Error { } } -const encryptedDataPackets = new Set([ - enums.packet.aeadEncryptedData, - enums.packet.symmetricallyEncryptedData, - enums.packet.symEncryptedIntegrityProtectedData -]); -const dataPackets = new Set([ - enums.packet.literalData, - enums.packet.compressedData, - ...encryptedDataPackets -]); +enum MessageType { + EmptyMessage, // incl. empty signed message + PlaintextOrEncryptedData, + EncryptedSessionKeys, + StandaloneAdditionalAllowedData +} +/** + * Implement OpenPGP message grammar based on: https://www.rfc-editor.org/rfc/rfc9580.html#section-10.3 . + * It is slightly more lenient as it also allows standalone ESK sequences, as well as empty (signed) messages. + * This latter case is needed to allow unknown packets. + * A new `MessageGrammarValidator` instance must be created for each packet sequence, as the instance is stateful: + * - `recordPacket` must be called for each packet in the sequence; the function will throw as soon as + * an invalid packet is detected. + * - `recordEnd` must be called at the end of the packet sequence to confirm its validity. + */ export class MessageGrammarValidator { - sawDataPacket: boolean = false; - sawESKs: number = 0; - sawOPSs: number = 0; - sawTrailingSigs: number = 0; + // PDA validator inspired by https://blog.jabberhead.tk/2022/10/26/implementing-packet-sequence-validation-using-pushdown-automata/ . + private state: MessageType = MessageType.EmptyMessage; + private leadingOnePassSignatureCounter: number = 0; - recordPacket(packet: enums.packet) { - if (packet === enums.packet.publicKeyEncryptedSessionKey || packet === enums.packet.symEncryptedSessionKey) { - if (this.sawDataPacket) { - throw new GrammarError('Encrypted session key packet following data packet'); - } - this.sawESKs++; - } else if (packet === enums.packet.onePassSignature) { - if (this.sawDataPacket) { - throw new GrammarError('One-pass signature packet following data packet'); - } - if (this.sawESKs) { - throw new GrammarError('One-pass signature packet following encrypted session key packet'); - } - this.sawOPSs++; - } else if (packet === enums.packet.signature) { - if (this.sawESKs) { - throw new GrammarError('Signature packet following encrypted session key packet'); - } - if (this.sawDataPacket) { - this.sawTrailingSigs++; - } - } else if (dataPackets.has(packet)) { - if (this.sawDataPacket) { - throw new GrammarError('Multiple data packets in message'); - } - if (this.sawESKs && !encryptedDataPackets.has(packet)) { - throw new GrammarError('Non-encrypted data packet following ESK packet'); - } - this.sawDataPacket = true; + /** + * Determine validity of the next packet in the sequence. + * NB: padding, marker and unknown packets are expected to already be filtered out on parsing, + * and are not accepted by `recordPacket`. + * @param packet - packet to validate + * @param config - needed to determine the `additionalAllowedPackets`: these are allowed anywhere in the sequence, except they cannot precede a OPS packet + * @throws {GrammarError} on invalid `packet` input + */ + recordPacket(packet: enums.packet, config: Config) { + const additionalAllowedPacketsTags = new Set(config.additionalAllowedPackets.map(c => c.tag)); + switch (this.state) { + case MessageType.EmptyMessage: + case MessageType.StandaloneAdditionalAllowedData: + switch (packet) { + case enums.packet.literalData: + case enums.packet.compressedData: + case enums.packet.aeadEncryptedData: + case enums.packet.symEncryptedIntegrityProtectedData: + case enums.packet.symmetricallyEncryptedData: + this.state = MessageType.PlaintextOrEncryptedData; + return; + case enums.packet.signature: + // Signature | and + // OPS | Signature | | Signature and + // OPS | | Signature are allowed + if (this.state === MessageType.StandaloneAdditionalAllowedData) { + if (--this.leadingOnePassSignatureCounter < 0) { + throw new GrammarError('Trailing signature packet without OPS'); + } + } + // this.state remains EmptyMessage or StandaloneAdditionalAllowedData + return; + case enums.packet.onePassSignature: + if (this.state === MessageType.StandaloneAdditionalAllowedData) { + // we do not allow this case, for simplicity + throw new GrammarError('OPS following StandaloneAdditionalAllowedData'); + } + this.leadingOnePassSignatureCounter++; + // this.state remains EmptyMessage + return; + case enums.packet.publicKeyEncryptedSessionKey: + case enums.packet.symEncryptedSessionKey: + this.state = MessageType.EncryptedSessionKeys; + return; + default: + if (!additionalAllowedPacketsTags.has(packet)) { + throw new GrammarError(`Unexpected packet ${packet} in state ${this.state}`); + } + this.state = MessageType.StandaloneAdditionalAllowedData; + return; + } + case MessageType.PlaintextOrEncryptedData: + switch (packet) { + case enums.packet.signature: + if (--this.leadingOnePassSignatureCounter < 0) { + throw new GrammarError('Trailing signature packet without OPS'); + } + this.state = MessageType.PlaintextOrEncryptedData; + return; + default: + if (!additionalAllowedPacketsTags.has(packet)) { + throw new GrammarError(`Unexpected packet ${packet} in state ${this.state}`); + } + this.state = MessageType.PlaintextOrEncryptedData; + return; + } + case MessageType.EncryptedSessionKeys: + switch (packet) { + case enums.packet.publicKeyEncryptedSessionKey: + case enums.packet.symEncryptedSessionKey: + this.state = MessageType.EncryptedSessionKeys; + return; + case enums.packet.symEncryptedIntegrityProtectedData: + case enums.packet.aeadEncryptedData: + case enums.packet.symmetricallyEncryptedData: + this.state = MessageType.PlaintextOrEncryptedData; + return; + case enums.packet.signature: + if (--this.leadingOnePassSignatureCounter < 0) { + throw new GrammarError('Trailing signature packet without OPS'); + } + this.state = MessageType.PlaintextOrEncryptedData; + return; + default: + if (!additionalAllowedPacketsTags.has(packet)) { + throw new GrammarError(`Unexpected packet ${packet} in state ${this.state}`); + } + this.state = MessageType.EncryptedSessionKeys; + } } } + /** + * Signal end of the packet sequence for final validity check + * @throws {GrammarError} on invalid sequence + */ recordEnd() { - if (this.sawOPSs !== this.sawTrailingSigs) { - throw new GrammarError('Mismatched one-pass signature and signature packets'); + switch (this.state) { + case MessageType.EmptyMessage: // needs to be allowed for PacketLists that only include unknown packets + case MessageType.PlaintextOrEncryptedData: + case MessageType.EncryptedSessionKeys: + case MessageType.StandaloneAdditionalAllowedData: + if (this.leadingOnePassSignatureCounter > 0) { + throw new GrammarError('Missing trailing signature packets'); + } } } } diff --git a/src/packet/packetlist.js b/src/packet/packetlist.js index 1c253b22..3c6bf9b5 100644 --- a/src/packet/packetlist.js +++ b/src/packet/packetlist.js @@ -95,7 +95,7 @@ class PacketList extends Array { // Unknown packets throw in the call above, we ignore them // in the grammar checker. try { - grammarValidator?.recordPacket(parsed.tag); + grammarValidator?.recordPacket(parsed.tag, config); } catch (e) { if (config.enforceGrammar) { throw e; diff --git a/test/general/config.js b/test/general/config.js index 29573866..89c6a969 100644 --- a/test/general/config.js +++ b/test/general/config.js @@ -35,7 +35,7 @@ habAyxd1AGKaNp1wbGFpbnRleHQgbWVzc2FnZQ== await expect( openpgp.readMessage({ armoredMessage: skeskPlusLiteralData, config: { enforceGrammar: true } }) - ).to.be.rejectedWith(/Non-encrypted data packet following ESK packet/); + ).to.be.rejectedWith(/Unexpected packet/); }); it('openpgp.readSignature', async function() { diff --git a/test/general/packet.js b/test/general/packet.js index 07c0d73e..2a9e0f7e 100644 --- a/test/general/packet.js +++ b/test/general/packet.js @@ -1375,11 +1375,97 @@ kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu }); describe('Grammar validation', async function () { + describe('MessageGrammarValidator - unit tests', () => { + it('valid nested signed messages should be valid', () => { + // Sig | OPS | Literal | Sig + const m1 = new MessageGrammarValidator(); + m1.recordPacket(openpgp.enums.packet.signature, { additionalAllowedPackets: [] }); + m1.recordPacket(openpgp.enums.packet.onePassSignature, { additionalAllowedPackets: [] }); + m1.recordPacket(openpgp.enums.packet.literalData, { additionalAllowedPackets: [] }); + m1.recordPacket(openpgp.enums.packet.signature, { additionalAllowedPackets: [] }); + expect(() => m1.recordEnd()).to.not.throw(); + + // OPS | Sig | Literal | Sig + const m2 = new MessageGrammarValidator(); + m2.recordPacket(openpgp.enums.packet.onePassSignature, { additionalAllowedPackets: [] }); + m2.recordPacket(openpgp.enums.packet.signature, { additionalAllowedPackets: [] }); + m2.recordPacket(openpgp.enums.packet.literalData, { additionalAllowedPackets: [] }); + m2.recordPacket(openpgp.enums.packet.signature, { additionalAllowedPackets: [] }); + expect(() => m2.recordEnd()).to.not.throw(); + + // OPS | Sig | Literal - should throw due to missing trailing signature + const m3 = new MessageGrammarValidator(); + m3.recordPacket(openpgp.enums.packet.onePassSignature, { additionalAllowedPackets: [] }); + m3.recordPacket(openpgp.enums.packet.signature, { additionalAllowedPackets: [] }); + m3.recordPacket(openpgp.enums.packet.literalData, { additionalAllowedPackets: [] }); + expect(() => m3.recordEnd()).to.throw(); + + // Sig - should throw due to standalone signature packet + const m4 = new MessageGrammarValidator(); + m4.recordPacket(openpgp.enums.packet.signature, { additionalAllowedPackets: [] }); + expect(() => m3.recordEnd()).to.throw(); + + // ESK | Sig | SEIPD - should throw + const m5 = new MessageGrammarValidator(); + m5.recordPacket(openpgp.enums.packet.publicKeyEncryptedSessionKey, { additionalAllowedPackets: [] }); + expect(() => m5.recordPacket(openpgp.enums.packet.signature, { additionalAllowedPackets: [] })).to.throw(); + }); + + it('standalone additional allowed packets should be valid', () => { + const additionalAllowedPackets = [openpgp.PublicKeyPacket]; + // Sig | OPS | PublicKeyPacket | Sig + const m1 = new MessageGrammarValidator(); + m1.recordPacket(openpgp.enums.packet.signature, { additionalAllowedPackets }); + m1.recordPacket(openpgp.enums.packet.onePassSignature, { additionalAllowedPackets }); + m1.recordPacket(openpgp.enums.packet.publicKey, { additionalAllowedPackets }); + m1.recordPacket(openpgp.enums.packet.signature, { additionalAllowedPackets }); + expect(() => m1.recordEnd()).to.not.throw(); + + // OPS | Sig | PublicKeyPacket | Sig + const m2 = new MessageGrammarValidator(); + m2.recordPacket(openpgp.enums.packet.onePassSignature, { additionalAllowedPackets }); + m2.recordPacket(openpgp.enums.packet.signature, { additionalAllowedPackets }); + m2.recordPacket(openpgp.enums.packet.publicKey, { additionalAllowedPackets }); + m2.recordPacket(openpgp.enums.packet.signature, { additionalAllowedPackets }); + expect(() => m2.recordEnd()).to.not.throw(); + + // standalone PublicKeyPacket + const m3 = new MessageGrammarValidator(); + m3.recordPacket(openpgp.enums.packet.publicKey, { additionalAllowedPackets }); + expect(() => m3.recordEnd()).to.not.throw(); + + // OPS | Sig | PublicKey - should throw due to missing trailing signature + const m4 = new MessageGrammarValidator(); + m4.recordPacket(openpgp.enums.packet.onePassSignature, { additionalAllowedPackets }); + m4.recordPacket(openpgp.enums.packet.signature, { additionalAllowedPackets }); + m4.recordPacket(openpgp.enums.packet.publicKey, { additionalAllowedPackets }); + expect(() => m4.recordEnd()).to.throw(); + + // Sig | PublicKey | Sig | PublicKey - should throw + const m5 = new MessageGrammarValidator(); + m5.recordPacket(openpgp.enums.packet.signature, { additionalAllowedPackets }); + m5.recordPacket(openpgp.enums.packet.publicKey, { additionalAllowedPackets }); + expect(() => m5.recordPacket(openpgp.enums.packet.signature, { additionalAllowedPackets })).to.throw(); + + // Sig | PublicKey | OPS | PublicKey | Sig - should throw + const m6 = new MessageGrammarValidator(); + m6.recordPacket(openpgp.enums.packet.signature, { additionalAllowedPackets }); + m6.recordPacket(openpgp.enums.packet.publicKey, { additionalAllowedPackets }); + expect(() => m6.recordPacket(openpgp.enums.packet.onePassSignature, { additionalAllowedPackets })).to.throw(); + }); + + it('standalone disallowed packets should not be valid', () => { + // standalone PublicKeyPacket + const m1 = new MessageGrammarValidator(); + expect(() => m1.recordPacket(openpgp.enums.packet.publicKey, { additionalAllowedPackets: [] })).to.throw(); + }); + }); + it('reject duplicate literal packet', async () => { const packets = new openpgp.PacketList(); packets.push(new openpgp.LiteralDataPacket()); packets.push(new openpgp.LiteralDataPacket()); - await expect(openpgp.PacketList.fromBinary(packets.write(), allAllowedPackets, openpgp.config, new MessageGrammarValidator())).to.be.rejectedWith(/Multiple data packets in message/); + await expect(openpgp.PacketList.fromBinary(packets.write(), allAllowedPackets, openpgp.config, new MessageGrammarValidator())).to.be.rejectedWith(/Unexpected packet/); }); it('reject duplicate literal packet inside encrypted data', async () => { @@ -1397,7 +1483,7 @@ kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu binaryMessage: packets.write() }), sessionKeys: [{ algorithm: 'aes128', data: new Uint8Array(16) }] - })).to.be.rejectedWith(/Multiple data packets in message/); + })).to.be.rejectedWith(/Unexpected packet/); }); it('reject duplicate literal packet inside encrypted data (streaming)', async () => { @@ -1424,7 +1510,7 @@ kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu allowUnauthenticatedStream: true } }); - await expect(stream.readToEnd(decrypted.data)).to.be.rejectedWith(/Multiple data packets in message/); + await expect(stream.readToEnd(decrypted.data)).to.be.rejectedWith(/Unexpected packet/); }); it('reject duplicate literal packet inside encrypted data (MDC error gets precedence)', async () => { diff --git a/test/general/signature.js b/test/general/signature.js index e5ad23e9..78c23f00 100644 --- a/test/general/signature.js +++ b/test/general/signature.js @@ -1881,10 +1881,10 @@ hkJiXopCSWKSlQInL1devkJJUWJmTmZeugJYlpdLAagQJM0JpsCqIQZwKgAA expect(pubKey.getKeys(keyIDs[0])).to.not.be.empty; await openpgp.verify({ verificationKeys: [pubKey], message, config: { minRSABits: 1024 } }).then(async ({ data, signatures }) => { - await expect(stream.readToEnd(data)).to.be.rejectedWith('Mismatched one-pass signature and signature packets'); + await expect(stream.readToEnd(data)).to.be.rejectedWith('Missing trailing signature packets'); expect(signatures).to.have.length(1); - await expect(signatures[0].verified).to.be.rejectedWith('Mismatched one-pass signature and signature packets'); - await expect(signatures[0].signature).to.be.rejectedWith('Mismatched one-pass signature and signature packets'); + await expect(signatures[0].verified).to.be.rejectedWith('Missing trailing signature packets'); + await expect(signatures[0].signature).to.be.rejectedWith('Missing trailing signature packets'); }); await openpgp.verify({ verificationKeys: [pubKey], message: messageWithoutGrammar, config: { minRSABits: 1024 } }).then(async ({ data, signatures }) => { expect(await stream.readToEnd(data)).to.equal(plaintext);