diff --git a/src/message.js b/src/message.js index 6bfe63de..68dda0a3 100644 --- a/src/message.js +++ b/src/message.js @@ -36,7 +36,7 @@ import { OnePassSignaturePacket, SignaturePacket } from './packet'; -import { getMessageGrammarValidator } from './packet/grammar'; +import { MessageGrammarValidator } from './packet/grammar'; // A Message can contain the following packets const allowedMessagePackets = /*#__PURE__*/ util.constructAllowedPackets([ @@ -879,7 +879,7 @@ export async function readMessage({ armoredMessage, binaryMessage, config, ...re } input = data; } - const packetlist = await PacketList.fromBinary(input, allowedMessagePackets, config, getMessageGrammarValidator()); + const packetlist = await PacketList.fromBinary(input, allowedMessagePackets, config, new MessageGrammarValidator()); const message = new Message(packetlist); message.fromStream = streamType; return message; diff --git a/src/packet/aead_encrypted_data.js b/src/packet/aead_encrypted_data.js index 52e2f993..d184c64c 100644 --- a/src/packet/aead_encrypted_data.js +++ b/src/packet/aead_encrypted_data.js @@ -28,7 +28,7 @@ import CompressedDataPacket from './compressed_data'; import OnePassSignaturePacket from './one_pass_signature'; import SignaturePacket from './signature'; import PacketList from './packetlist'; -import { getMessageGrammarValidator } from './grammar'; +import { MessageGrammarValidator } from './grammar'; // An AEAD-encrypted Data packet can contain the following packet types const allowedPackets = /*#__PURE__*/ util.constructAllowedPackets([ @@ -106,7 +106,7 @@ class AEADEncryptedDataPacket { await runAEAD(this, 'decrypt', key, streamClone(this.encrypted)), allowedPackets, config, - getMessageGrammarValidator() + new MessageGrammarValidator() ); } diff --git a/src/packet/compressed_data.js b/src/packet/compressed_data.js index 54b1c56f..cdeddea0 100644 --- a/src/packet/compressed_data.js +++ b/src/packet/compressed_data.js @@ -25,7 +25,7 @@ import LiteralDataPacket from './literal_data'; import OnePassSignaturePacket from './one_pass_signature'; import SignaturePacket from './signature'; import PacketList from './packetlist'; -import { getMessageGrammarValidator } from './grammar'; +import { MessageGrammarValidator } from './grammar'; // A Compressed Data packet can contain the following packet types const allowedPackets = /*#__PURE__*/ util.constructAllowedPackets([ @@ -114,7 +114,7 @@ class CompressedDataPacket { } // Decompressing a Compressed Data packet MUST also yield a valid OpenPGP Message - this.packets = await PacketList.fromBinary(await decompressionFn(this.compressed), allowedPackets, config, getMessageGrammarValidator()); + this.packets = await PacketList.fromBinary(await decompressionFn(this.compressed), allowedPackets, config, new MessageGrammarValidator()); } /** diff --git a/src/packet/grammar.ts b/src/packet/grammar.ts index ae1e75f1..6e0e7038 100644 --- a/src/packet/grammar.ts +++ b/src/packet/grammar.ts @@ -1,6 +1,4 @@ -import { type Config } from '../config'; import enums from '../enums'; -import util from '../util'; export class GrammarError extends Error { constructor(...params: any[]) { @@ -14,74 +12,60 @@ 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 +]); -const isValidLiteralMessage = (tagList: enums.packet[]) => tagList.length === 1 && tagList[0] === enums.packet.literalData; -const isValidCompressedMessage = (tagList: enums.packet[]) => tagList.length === 1 && tagList[0] === enums.packet.compressedData; -const isValidEncryptedMessage = (tagList: enums.packet[]) => { - // Encrypted Message: Encrypted Data | ESK Sequence, Encrypted Data. - const isValidESKSequence = (tagList: enums.packet[]) => ( - tagList.every(packetTag => new Set([enums.packet.publicKeyEncryptedSessionKey, enums.packet.symEncryptedSessionKey]).has(packetTag)) - ); - const encryptedDataPacketIndex = tagList.findIndex(tag => new Set([enums.packet.aeadEncryptedData, enums.packet.symmetricallyEncryptedData, enums.packet.symEncryptedIntegrityProtectedData]).has(tag)); - if (encryptedDataPacketIndex < 0) { - return isValidESKSequence(tagList); - } +export class MessageGrammarValidator { + sawDataPacket: boolean = false; + sawESKs: number = 0; + sawOPSs: number = 0; + sawTrailingSigs: number = 0; - return (encryptedDataPacketIndex === tagList.length - 1) && - isValidESKSequence(tagList.slice(0, encryptedDataPacketIndex)); -}; - -const isValidSignedMessage = (tagList: enums.packet[], acceptPartial: boolean) => { - // Signature Packet, OpenPGP Message | One-Pass Signed Message. - if (tagList.findIndex(tag => tag === enums.packet.signature) === 0) { - return isValidOpenPGPMessage(tagList.slice(1), acceptPartial); - } - - // One-Pass Signed Message: - // One-Pass Signature Packet, OpenPGP Message, Corresponding Signature Packet. - if (tagList.findIndex(tag => tag === enums.packet.onePassSignature) === 0) { - const correspondingSigPacketIndex = util.findLastIndex(tagList, tag => tag === enums.packet.signature); - if (correspondingSigPacketIndex !== tagList.length - 1 && !acceptPartial) { - return false; - } - return isValidOpenPGPMessage(tagList.slice(1, correspondingSigPacketIndex < 0 ? undefined : correspondingSigPacketIndex), acceptPartial); - } - - return false; -}; - -/** - * Implements grammar checks based on https://www.rfc-editor.org/rfc/rfc9580.html#section-10.3 . - * @param packetList - list of packet tags to validate; marker/padding/unknown packet tags are expected to have been already filtered out. - * @param acceptPartial - whether the list of tags corresponds to a partially-parsed message - * @returns whether the list of tags is valid - */ -const isValidOpenPGPMessage = ( - packetList: enums.packet[], - acceptPartial: boolean -): boolean => { - return isValidLiteralMessage(packetList) || - isValidCompressedMessage(packetList) || - isValidEncryptedMessage(packetList) || - isValidSignedMessage(packetList, acceptPartial); -}; - -export const getMessageGrammarValidator = () => { - let logged = false; - - /** - * @throws on grammar error, provided `config.enforceGrammar` is enabled. - */ - return (list: number[], isPartial: boolean, config: Config): undefined => { - if (!isValidOpenPGPMessage(list, isPartial)) { - const error = new GrammarError(`Data does not respect OpenPGP grammar [${list}]`); - if (!logged) { - util.printDebugError(error); - logged = true; + 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'); } - if (config.enforceGrammar) { - throw error; + 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; + } else { + throw new GrammarError(`Unexpected packet: ${packet}`); } - }; -}; + } + + recordEnd() { + if (this.sawOPSs !== this.sawTrailingSigs) { + throw new GrammarError('Mismatched one-pass signature and signature packets'); + } + } +} diff --git a/src/packet/packetlist.js b/src/packet/packetlist.js index a7ab8eb0..1c253b22 100644 --- a/src/packet/packetlist.js +++ b/src/packet/packetlist.js @@ -73,7 +73,6 @@ class PacketList extends Array { if (config.additionalAllowedPackets.length) { allowedPackets = { ...allowedPackets, ...util.constructAllowedPackets(config.additionalAllowedPackets) }; } - const tagsRead = []; this.stream = streamTransformPair(bytes, async (readable, writable) => { const reader = streamGetReader(readable); const writer = streamGetWriter(writable); @@ -95,7 +94,15 @@ class PacketList extends Array { const packet = newPacketFromTag(parsed.tag, allowedPackets); // Unknown packets throw in the call above, we ignore them // in the grammar checker. - tagsRead.push(parsed.tag); + try { + grammarValidator?.recordPacket(parsed.tag); + } catch (e) { + if (config.enforceGrammar) { + throw e; + } else { + util.printDebugError(e); + } + } packet.packets = new PacketList(); packet.fromStream = util.isStream(parsed.packet); wasStream = packet.fromStream; @@ -179,7 +186,15 @@ class PacketList extends Array { if (done) { // Here we are past the MDC check for SEIPDv1 data, hence // the data is always authenticated at this point. - grammarValidator?.(tagsRead, false, config); + try { + grammarValidator?.recordEnd(); + } catch (e) { + if (config.enforceGrammar) { + throw e; + } else { + util.printDebugError(e); + } + } await writer.ready; await writer.close(); return; @@ -198,12 +213,8 @@ class PacketList extends Array { this.push(value); } else { this.stream = null; - break; } - if (supportsStreaming(value.constructor.tag)) { - if (!delayErrors) { - grammarValidator?.(tagsRead, true, config); - } + if (done || supportsStreaming(value.constructor.tag)) { break; } } diff --git a/src/packet/sym_encrypted_integrity_protected_data.js b/src/packet/sym_encrypted_integrity_protected_data.js index de04511e..9fbfdb49 100644 --- a/src/packet/sym_encrypted_integrity_protected_data.js +++ b/src/packet/sym_encrypted_integrity_protected_data.js @@ -28,7 +28,7 @@ import OnePassSignaturePacket from './one_pass_signature'; import SignaturePacket from './signature'; import PacketList from './packetlist'; import { UnsupportedError } from './packet'; -import { getMessageGrammarValidator } from './grammar'; +import { MessageGrammarValidator } from './grammar'; // A SEIP packet can contain the following packet types const allowedPackets = /*#__PURE__*/ util.constructAllowedPackets([ @@ -224,7 +224,7 @@ class SymEncryptedIntegrityProtectedDataPacket { // MUST yield a valid OpenPGP Message. // - Decrypting a version 2 Symmetrically Encrypted and Integrity Protected Data packet // MUST yield a valid Optionally Padded Message. - this.packets = await PacketList.fromBinary(packetbytes, allowedPackets, config, getMessageGrammarValidator(), delayErrors); + this.packets = await PacketList.fromBinary(packetbytes, allowedPackets, config, new MessageGrammarValidator(), delayErrors); return true; } } diff --git a/test/general/config.js b/test/general/config.js index 98e22af5..29573866 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(/Data does not respect OpenPGP grammar/); + ).to.be.rejectedWith(/Non-encrypted data packet following ESK packet/); }); it('openpgp.readSignature', async function() { diff --git a/test/general/packet.js b/test/general/packet.js index 743f5e46..07c0d73e 100644 --- a/test/general/packet.js +++ b/test/general/packet.js @@ -13,7 +13,7 @@ import * as random from '../../src/crypto/random'; import * as input from './testInputs.js'; import { mockCryptoRandomGenerator, restoreCryptoRandomGenerator } from '../mockRandom.ts'; -import { getMessageGrammarValidator } from '../../src/packet/grammar.js'; +import { MessageGrammarValidator } from '../../src/packet/grammar.js'; function stringify(array) { if (stream.isStream(array)) { @@ -1379,7 +1379,7 @@ kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu 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, getMessageGrammarValidator())).to.be.rejectedWith(/Data does not respect OpenPGP grammar/); + await expect(openpgp.PacketList.fromBinary(packets.write(), allAllowedPackets, openpgp.config, new MessageGrammarValidator())).to.be.rejectedWith(/Multiple data packets in message/); }); it('reject duplicate literal packet inside encrypted data', async () => { @@ -1397,7 +1397,7 @@ kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu binaryMessage: packets.write() }), sessionKeys: [{ algorithm: 'aes128', data: new Uint8Array(16) }] - })).to.be.rejectedWith(/Data does not respect OpenPGP grammar/); + })).to.be.rejectedWith(/Multiple data packets in message/); }); it('reject duplicate literal packet inside encrypted data (streaming)', async () => { @@ -1424,7 +1424,7 @@ kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu allowUnauthenticatedStream: true } }); - await expect(stream.readToEnd(decrypted.data)).to.be.rejectedWith(/Data does not respect OpenPGP grammar/); + await expect(stream.readToEnd(decrypted.data)).to.be.rejectedWith(/Multiple data packets in message/); }); it('reject duplicate literal packet inside encrypted data (MDC error gets precedence)', async () => { @@ -1529,13 +1529,13 @@ kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu packets.push(padding); packets.push(new openpgp.MarkerPacket()); packets.push(new openpgp.LiteralDataPacket()); - const parsed = await openpgp.PacketList.fromBinary(packets.write(), allAllowedPackets, openpgp.config, getMessageGrammarValidator()); + const parsed = await openpgp.PacketList.fromBinary(packets.write(), allAllowedPackets, openpgp.config, new MessageGrammarValidator()); expect(parsed.length).to.equal(1); // marker and padding packets are always dropped on parsing }); it('accepts unknown packets', async () => { const unknownPacketTag63 = util.hexToUint8Array('ff0a750064bf943d6e756c6c'); // non-critical tag - const parsed = await openpgp.PacketList.fromBinary(unknownPacketTag63, allAllowedPackets, openpgp.config, getMessageGrammarValidator()); + const parsed = await openpgp.PacketList.fromBinary(unknownPacketTag63, allAllowedPackets, openpgp.config, new MessageGrammarValidator()); expect(parsed.length).to.equal(1); }); }); diff --git a/test/general/signature.js b/test/general/signature.js index 05360536..e5ad23e9 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('Data does not respect OpenPGP grammar'); + await expect(stream.readToEnd(data)).to.be.rejectedWith('Mismatched one-pass signature and signature packets'); expect(signatures).to.have.length(1); - await expect(signatures[0].verified).to.be.rejectedWith('Data does not respect OpenPGP grammar'); - await expect(signatures[0].signature).to.be.rejectedWith('Data does not respect OpenPGP grammar'); + 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 openpgp.verify({ verificationKeys: [pubKey], message: messageWithoutGrammar, config: { minRSABits: 1024 } }).then(async ({ data, signatures }) => { expect(await stream.readToEnd(data)).to.equal(plaintext);