From 88cd1810a303dccd9432ef1813e1afdeef66e8b3 Mon Sep 17 00:00:00 2001 From: larabr <7375870+larabr@users.noreply.github.com> Date: Fri, 9 May 2025 18:26:23 +0200 Subject: [PATCH] Implement OpenPGP message grammar validation (add `config.enforceGrammar`) It enforces a message structure as defined in https://www.rfc-editor.org/rfc/rfc9580.html#section-10.3 (but slightly more permissive with Padding packets allowed in all cases). Since we are unclear on whether this change might impact handling of some messages in the wild, generated by odd use-cases or non-conformant implementations, we also add the option to disable the grammar check via `config.enforceGrammar`. GrammarErrors are only sensitive in the context of unauthenticated decrypted streams. --- .eslintrc.cjs | 8 +- src/config/config.d.ts | 2 + src/config/config.js | 7 + src/message.js | 3 +- src/openpgp.js | 21 +-- src/packet/aead_encrypted_data.js | 4 +- src/packet/compressed_data.js | 4 +- src/packet/grammar.ts | 122 ++++++++++++++++++ src/packet/index.js | 1 + src/packet/packetlist.js | 23 +++- .../sym_encrypted_integrity_protected_data.js | 18 ++- src/packet/symmetrically_encrypted_data.js | 4 + src/util.js | 15 +++ test/general/config.js | 15 +++ test/general/openpgp.js | 2 +- test/general/packet.js | 46 +++++++ test/general/signature.js | 42 +++--- 17 files changed, 301 insertions(+), 36 deletions(-) create mode 100644 src/packet/grammar.ts diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 1be7c193..70dd627d 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -108,8 +108,12 @@ module.exports = { 'indent': 'off', '@typescript-eslint/indent': ['error', 2, { 'SwitchCase': 1 }], 'no-unused-vars': 'off', - '@typescript-eslint/no-unused-vars': 'error', - + "@typescript-eslint/no-unused-vars": [ + "error", + { + "argsIgnorePattern": "^_", + } + ], // eslint-plugin-import rules: 'import/named': 'error', 'import/extensions': 'off', // temporary: we use them in tests (ESM compliant), but not in the lib (to limit diff) diff --git a/src/config/config.d.ts b/src/config/config.d.ts index 82d2bc5e..240f99ba 100644 --- a/src/config/config.d.ts +++ b/src/config/config.d.ts @@ -17,6 +17,8 @@ export interface Config { passwordCollisionCheck: boolean; ignoreUnsupportedPackets: boolean; ignoreMalformedPackets: boolean; + enforceGrammar: boolean; + additionalAllowedPackets: Array<{ new(): any }>; versionString: string; commentString: string; allowInsecureDecryptionWithSigningKeys: boolean; diff --git a/src/config/config.js b/src/config/config.js index c24d47c7..dc53a12f 100644 --- a/src/config/config.js +++ b/src/config/config.js @@ -218,10 +218,17 @@ export default { * @property {Boolean} ignoreMalformedPackets Ignore malformed packets on parsing instead of throwing an error */ ignoreMalformedPackets: false, + /** + * @memberof module:config + * @property {Boolean} enforceGrammar whether parsed OpenPGP messages must comform to the OpenPGP grammar + * defined in https://www.rfc-editor.org/rfc/rfc9580.html#name-openpgp-messages . + */ + enforceGrammar: true, /** * Parsing of packets is normally restricted to a predefined set of packets. For example a Sym. Encrypted Integrity Protected Data Packet can only * contain a certain set of packets including LiteralDataPacket. With this setting we can allow additional packets, which is probably not advisable * as a global config setting, but can be used for specific function calls (e.g. decrypt method of Message). + * NB: `config.enforceGrammar` may need to be disabled as well. * @memberof module:config * @property {Array} additionalAllowedPackets Allow additional packets on parsing. Defined as array of packet classes, e.g. [PublicKeyPacket] */ diff --git a/src/message.js b/src/message.js index 3518b8e6..f21ac597 100644 --- a/src/message.js +++ b/src/message.js @@ -36,6 +36,7 @@ import { OnePassSignaturePacket, SignaturePacket } from './packet'; +import { getMessageGrammarValidator } from './packet/grammar'; // A Message can contain the following packets const allowedMessagePackets = /*#__PURE__*/ util.constructAllowedPackets([ @@ -878,7 +879,7 @@ export async function readMessage({ armoredMessage, binaryMessage, config, ...re } input = data; } - const packetlist = await PacketList.fromBinary(input, allowedMessagePackets, config); + const packetlist = await PacketList.fromBinary(input, allowedMessagePackets, config, getMessageGrammarValidator({ delayReporting: false })); const message = new Message(packetlist); message.fromStream = streamType; return message; diff --git a/src/openpgp.js b/src/openpgp.js index 1b8cfa64..3e922db4 100644 --- a/src/openpgp.js +++ b/src/openpgp.js @@ -357,7 +357,7 @@ export async function decrypt({ message, decryptionKeys, passwords, sessionKeys, result.signatures = signature ? await decrypted.verifyDetached(signature, verificationKeys, date, config) : await decrypted.verify(verificationKeys, date, config); result.data = format === 'binary' ? decrypted.getLiteralData() : decrypted.getText(); result.filename = decrypted.getFilename(); - linkStreams(result, message); + linkStreams(result, message, ...new Set([decrypted, decrypted.unwrapCompressed()])); if (expectSigned) { if (verificationKeys.length === 0) { throw new Error('Verification keys are required to verify message signatures'); @@ -492,7 +492,9 @@ export async function verify({ message, verificationKeys, expectSigned = false, result.signatures = await message.verify(verificationKeys, date, config); } result.data = format === 'binary' ? message.getLiteralData() : message.getText(); - if (message.fromStream && !signature) linkStreams(result, message); + if (message.fromStream && !signature) { + linkStreams(result, ...new Set([message, message.unwrapCompressed()])); + } if (expectSigned) { if (result.signatures.length === 0) { throw new Error('Message is not signed'); @@ -689,22 +691,25 @@ async function convertStream(data) { } /** - * Link result.data to the message stream for cancellation. - * Also, forward errors in the message to result.data. + * Link result.data to the input message stream for cancellation. + * Also, forward errors in the input message and intermediate messages to result.data. * @param {Object} result - the data to convert - * @param {Message} message - message object + * @param {Message} message - message object provided by the user + * @param {Message} intermediateMessages - intermediate message object with packet streams to link * @returns {Object} * @private */ -function linkStreams(result, message) { - result.data = streamTransformPair(message.packets.stream, async (readable, writable) => { +function linkStreams(result, inputMessage, ...intermediateMessages) { + result.data = streamTransformPair(inputMessage.packets.stream, async (readable, writable) => { await streamPipe(result.data, writable, { preventClose: true }); const writer = streamGetWriter(writable); try { - // Forward errors in the message stream to result.data. + // Forward errors in the message streams to result.data. await streamReadToEnd(readable, _ => _); + await Promise.all(intermediateMessages.map(intermediate => streamReadToEnd(intermediate.packets.stream, _ => _))); + // if result.data throws, the writable will be in errored state, and `close()` fails, but its ok. await writer.close(); } catch (e) { await writer.abort(e); diff --git a/src/packet/aead_encrypted_data.js b/src/packet/aead_encrypted_data.js index a7542425..e2129fdb 100644 --- a/src/packet/aead_encrypted_data.js +++ b/src/packet/aead_encrypted_data.js @@ -28,6 +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'; // An AEAD-encrypted Data packet can contain the following packet types const allowedPackets = /*#__PURE__*/ util.constructAllowedPackets([ @@ -104,7 +105,8 @@ class AEADEncryptedDataPacket { this.packets = await PacketList.fromBinary( await runAEAD(this, 'decrypt', key, streamClone(this.encrypted)), allowedPackets, - config + config, + getMessageGrammarValidator({ enforceDelay: false }) ); } diff --git a/src/packet/compressed_data.js b/src/packet/compressed_data.js index d0014cf4..e3dfa237 100644 --- a/src/packet/compressed_data.js +++ b/src/packet/compressed_data.js @@ -25,6 +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'; // A Compressed Data packet can contain the following packet types const allowedPackets = /*#__PURE__*/ util.constructAllowedPackets([ @@ -112,7 +113,8 @@ class CompressedDataPacket { throw new Error(`${compressionName} decompression not supported`); } - this.packets = await PacketList.fromBinary(await decompressionFn(this.compressed), allowedPackets, config); + // Decompressing a Compressed Data packet MUST also yield a valid OpenPGP Message + this.packets = await PacketList.fromBinary(await decompressionFn(this.compressed), allowedPackets, config, getMessageGrammarValidator({ enforceDelay: false })); } /** diff --git a/src/packet/grammar.ts b/src/packet/grammar.ts new file mode 100644 index 00000000..bc578343 --- /dev/null +++ b/src/packet/grammar.ts @@ -0,0 +1,122 @@ +import { type Config } from '../config'; +import enums from '../enums'; +import util from '../util'; + +export class GrammarError extends Error { + constructor(...params: any[]) { + super(...params); + + if (Error.captureStackTrace) { + Error.captureStackTrace(this, GrammarError); + } + + this.name = 'GrammarError'; + } +} + + +const isValidLiteralMessage = (tagList: enums.packet[], _acceptPartial: boolean) => tagList.length === 1 && tagList[0] === enums.packet.literalData; +const isValidCompressedMessage = (tagList: enums.packet[], _acceptPartial: boolean) => tagList.length === 1 && tagList[0] === enums.packet.compressedData; +const isValidEncryptedMessage = (tagList: enums.packet[], acceptPartial: boolean) => { + // Encrypted Message: Encrypted Data | ESK Sequence, Encrypted Data. + const isValidESKSequence = (tagList: enums.packet[], _acceptPartial: boolean) => ( + 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, acceptPartial); + } + + return (encryptedDataPacketIndex === tagList.length - 1) && + isValidESKSequence(tagList.slice(0, encryptedDataPacketIndex), acceptPartial); +}; + +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; +}; + +const isUnknownPacketTag = (tag: number): tag is enums.packet => { + try { + enums.read(enums.packet, tag); + return false; + } catch (e) { + return true; + } +}; + +/** + * Implements grammar checks based on https://www.rfc-editor.org/rfc/rfc9580.html#section-10.3 . + * @param notNormalizedList - list of packet tags to validate + * @param acceptPartial - whether the list of tags corresponds to a partially-parsed message + * @returns whether the list of tags is valid + */ +const isValidOpenPGPMessage = ( + notNormalizedList: number[] /** might have unknown tags */, + acceptPartial: boolean +): boolean => { + // Take care of packet tags that can appear anywhere in the sequence: + // 1. A Marker packet (Section 5.8) can appear anywhere in the sequence. + // 2. An implementation MUST be able to process Padding packets anywhere else in an OpenPGP stream so that future revisions of this document may specify further locations for padding. + // 3. An unknown non-critical packet MUST be ignored (criticality is enforced on parsing). + const normalizedList: enums.packet[] = notNormalizedList.filter(tag => ( + tag !== enums.packet.marker && + tag !== enums.packet.padding && + !isUnknownPacketTag(tag) + )); + + return isValidLiteralMessage(normalizedList, acceptPartial) || + isValidCompressedMessage(normalizedList, acceptPartial) || + isValidEncryptedMessage(normalizedList, acceptPartial) || + isValidSignedMessage(normalizedList, acceptPartial); +}; + +/** + * If `delayReporting === false`, the grammar validator throws as soon as an invalid packet sequence is detected during parsing. + * This setting MUST NOT be used when parsing unauthenticated decrypted data, to avoid instantiating decryption oracles. + * Passing `delayReporting === true` allows checking the grammar validity in an async manner, by + * only reporting the validity status after parsing is done (i.e. and authentication is expected to + * have been enstablished) + */ +export const getMessageGrammarValidator = ({ delayReporting }: { delayReporting: boolean }) => { + let logged = false; + + + /** + * @returns `true` on successful grammar validation; if `delayReporting` is set, `null` is returned + * if validation is still pending (partial parsing, waiting for authentication to be confirmed). + * @throws on grammar error, provided `config.enforceGrammar` is enabled. + */ + return (list: number[], isPartial: boolean, config: Config): true | null => { + if (delayReporting && isPartial) return null; // delay until the full message has been parsed (i.e. authenticated) + + if (!isValidOpenPGPMessage(list, isPartial)) { + const error = new GrammarError(`Data does not respect OpenPGP grammar [${list}]`); + if (!logged) { + util.printDebugError(error); + logged = true; + } + if (config.enforceGrammar) { + throw error; + } else { + return true; + } + } + + return true; + }; +}; diff --git a/src/packet/index.js b/src/packet/index.js index 562b3205..04e7fe0a 100644 --- a/src/packet/index.js +++ b/src/packet/index.js @@ -1,3 +1,4 @@ export * from './all_packets'; export { default as PacketList } from './packetlist'; export { UnparseablePacket } from './packet'; +export { GrammarError } from './grammar'; diff --git a/src/packet/packetlist.js b/src/packet/packetlist.js index ebaaa55b..cf034861 100644 --- a/src/packet/packetlist.js +++ b/src/packet/packetlist.js @@ -10,6 +10,7 @@ import { import util from '../util'; import enums from '../enums'; import defaultConfig from '../config'; +import { GrammarError } from './grammar'; /** * Instantiate a new packet given its tag @@ -50,9 +51,9 @@ class PacketList extends Array { * @throws on parsing errors * @async */ - static async fromBinary(bytes, allowedPackets, config = defaultConfig) { + static async fromBinary(bytes, allowedPackets, config = defaultConfig, grammarValidator = null) { const packets = new PacketList(); - await packets.read(bytes, allowedPackets, config); + await packets.read(bytes, allowedPackets, config, grammarValidator); return packets; } @@ -61,15 +62,17 @@ class PacketList extends Array { * @param {Uint8Array | ReadableStream} bytes - binary data to parse * @param {Object} allowedPackets - mapping where keys are allowed packet tags, pointing to their Packet class * @param {Object} [config] - full configuration, defaults to openpgp.config + * @param {function(enums.packet[], boolean, Object): void} [grammarValidator] * @throws on parsing errors * @async */ - async read(bytes, allowedPackets, config = defaultConfig) { + async read(bytes, allowedPackets, config = defaultConfig, grammarValidator = null) { if (config.additionalAllowedPackets.length) { allowedPackets = { ...allowedPackets, ...util.constructAllowedPackets(config.additionalAllowedPackets) }; } this.stream = streamTransformPair(bytes, async (readable, writable) => { const writer = streamGetWriter(writable); + const writtenTags = []; try { while (true) { await writer.ready; @@ -87,6 +90,12 @@ class PacketList extends Array { packet.fromStream = util.isStream(parsed.packet); await packet.read(parsed.packet, config); await writer.write(packet); + writtenTags.push(parsed.tag); + // The `writtenTags` are only sensitive if we are parsing an _unauthenticated_ decrypted stream, + // since they can enable an decryption oracle. + // It's responsibility of the caller to pass a `grammarValidator` that takes care of + // postponing error reporting until the data has been authenticated. + grammarValidator?.(writtenTags, true, config); } catch (e) { // If an implementation encounters a critical packet where the packet type is unknown in a packet sequence, // it MUST reject the whole packet sequence. On the other hand, an unknown non-critical packet MUST be ignored. @@ -101,7 +110,8 @@ class PacketList extends Array { const throwUnsupportedError = !config.ignoreUnsupportedPackets && e instanceof UnsupportedError; const throwMalformedError = !config.ignoreMalformedPackets && !(e instanceof UnsupportedError); - if (throwUnsupportedError || throwMalformedError || supportsStreaming(parsed.tag)) { + const throwGrammarError = e instanceof GrammarError; + if (throwUnsupportedError || throwMalformedError || throwGrammarError || supportsStreaming(parsed.tag)) { // The packets that support streaming are the ones that contain message data. // Those are also the ones we want to be more strict about and throw on parse errors // (since we likely cannot process the message without these packets anyway). @@ -109,11 +119,16 @@ class PacketList extends Array { } else { const unparsedPacket = new UnparseablePacket(parsed.tag, parsed.packet); await writer.write(unparsedPacket); + writtenTags.push(parsed.tag); + grammarValidator?.(writtenTags, true, config); } util.printDebugError(e); } }); if (done) { + // Here we are past the MDC check for SEIPDv1 data, hence + // the data is always authenticated at this point. + grammarValidator?.(writtenTags, false, config); await writer.ready; await writer.close(); return; diff --git a/src/packet/sym_encrypted_integrity_protected_data.js b/src/packet/sym_encrypted_integrity_protected_data.js index 9e08002c..7941c7c5 100644 --- a/src/packet/sym_encrypted_integrity_protected_data.js +++ b/src/packet/sym_encrypted_integrity_protected_data.js @@ -28,6 +28,7 @@ import OnePassSignaturePacket from './one_pass_signature'; import SignaturePacket from './signature'; import PacketList from './packetlist'; import { UnsupportedError } from './packet'; +import { getMessageGrammarValidator } from './grammar'; // A SEIP packet can contain the following packet types const allowedPackets = /*#__PURE__*/ util.constructAllowedPackets([ @@ -183,16 +184,23 @@ class SymEncryptedIntegrityProtectedDataPacket { if (isArrayStream(encrypted)) encrypted = await streamReadToEnd(encrypted); let packetbytes; + let grammarValidator; if (this.version === 2) { if (this.cipherAlgorithm !== sessionKeyAlgorithm) { // sanity check throw new Error('Unexpected session key algorithm'); } packetbytes = await runAEAD(this, 'decrypt', key, encrypted); + grammarValidator = getMessageGrammarValidator({ delayReporting: false }); } else { const { blockSize } = getCipherParams(sessionKeyAlgorithm); const decrypted = await cipherMode.cfb.decrypt(sessionKeyAlgorithm, key, encrypted, new Uint8Array(blockSize)); + // Grammar validation cannot be run before message integrity has been enstablished, + // to avoid leaking info about the unauthenticated message structure. + const releaseUnauthenticatedStream = util.isStream(encrypted) && config.allowUnauthenticatedStream; + grammarValidator = getMessageGrammarValidator({ delayReporting: releaseUnauthenticatedStream }); + // there must be a modification detection code packet as the // last packet and everything gets hashed except the hash itself const realHash = streamSlice(streamPassiveClone(decrypted), -20); @@ -204,17 +212,23 @@ class SymEncryptedIntegrityProtectedDataPacket { if (!util.equalsUint8Array(hash, mdc)) { throw new Error('Modification detected.'); } + // this last chunk comes at the end of the stream passed to Packetlist.read's streamTransformPair, + // which can thus be 'done' only after the MDC has been checked. return new Uint8Array(); }); const bytes = streamSlice(tohash, blockSize + 2); // Remove random prefix packetbytes = streamSlice(bytes, 0, -2); // Remove MDC packet packetbytes = streamConcat([packetbytes, streamFromAsync(() => verifyHash)]); - if (!util.isStream(encrypted) || !config.allowUnauthenticatedStream) { + if (!releaseUnauthenticatedStream) { packetbytes = await streamReadToEnd(packetbytes); } } - this.packets = await PacketList.fromBinary(packetbytes, allowedPackets, config); + // - Decrypting a version 1 Symmetrically Encrypted and Integrity Protected Data packet + // 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, grammarValidator); return true; } } diff --git a/src/packet/symmetrically_encrypted_data.js b/src/packet/symmetrically_encrypted_data.js index 24bf8f8e..a4c008f7 100644 --- a/src/packet/symmetrically_encrypted_data.js +++ b/src/packet/symmetrically_encrypted_data.js @@ -93,6 +93,10 @@ class SymmetricallyEncryptedDataPacket { encrypted.subarray(2, blockSize + 2) ); + // Decrypting a Symmetrically Encrypted Data packet MUST yield a valid OpenPGP Message. + // But due to the lack of authentication over the decrypted data, + // we do not run any grammarValidator, to avoid enabling a decryption oracle + // (plus, there is probably a higher chance that these messages have an expected structure). this.packets = await PacketList.fromBinary(decrypted, allowedPackets, config); } diff --git a/src/util.js b/src/util.js index db4d4c5c..c57ea431 100644 --- a/src/util.js +++ b/src/util.js @@ -314,6 +314,21 @@ const util = { return true; }, + /** + * Same as Array.findLastIndex, which is not supported on Safari 14 . + * @param {Array} arr + * @param {function(element, index, arr): boolean} findFn + * @return index of last element matching `findFn`, -1 if not found + */ + findLastIndex: function(arr, findFn) { + for (let i = arr.length; i >= 0; i--) { + if (findFn(arr[i], i, arr)) { + return i; + } + } + return -1; + }, + /** * Calculates a 16bit sum of a Uint8Array by adding each character * codes modulus 65535 diff --git a/test/general/config.js b/test/general/config.js index c9080ea0..98e22af5 100644 --- a/test/general/config.js +++ b/test/general/config.js @@ -21,6 +21,21 @@ export default () => describe('Custom configuration', function() { await expect( openpgp.readMessage({ armoredMessage: parsedMessage.armor(), config }) ).to.be.rejectedWith(/Version 1 of the SKESK packet is unsupported/); + + const skeskPlusLiteralData = `-----BEGIN PGP MESSAGE----- + +wy4ECQMIjvrInhvTxJwAbkqXp+KWFdBcjoPn03jCdyspVi9qXBDbyGaP1lrM +habAyxd1AGKaNp1wbGFpbnRleHQgbWVzc2FnZQ== +=XoUx +-----END PGP MESSAGE----- +`; + + const parsedInvalidMessage = await openpgp.readMessage({ armoredMessage: skeskPlusLiteralData, config: { enforceGrammar: false } }); + expect(parsedInvalidMessage.packets[0]).to.be.instanceOf(openpgp.SymEncryptedSessionKeyPacket); + + await expect( + openpgp.readMessage({ armoredMessage: skeskPlusLiteralData, config: { enforceGrammar: true } }) + ).to.be.rejectedWith(/Data does not respect OpenPGP grammar/); }); it('openpgp.readSignature', async function() { diff --git a/test/general/openpgp.js b/test/general/openpgp.js index 3eb42d70..4558cb5c 100644 --- a/test/general/openpgp.js +++ b/test/general/openpgp.js @@ -4405,7 +4405,7 @@ habAyxd1AGKaNp1wbGFpbnRleHQgbWVzc2FnZQ== -----END PGP MESSAGE----- `; - const message = await openpgp.readMessage({ armoredMessage: skeskPlusLiteralData }); + const message = await openpgp.readMessage({ armoredMessage: skeskPlusLiteralData, config: { enforceGrammar: false } }); await expect(openpgp.decrypt({ message, passwords: 'password' })).to.be.rejectedWith(/No encrypted data found/); }); diff --git a/test/general/packet.js b/test/general/packet.js index acda172c..76982462 100644 --- a/test/general/packet.js +++ b/test/general/packet.js @@ -13,6 +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'; function stringify(array) { if (stream.isStream(array)) { @@ -1372,6 +1373,51 @@ kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu expect(otherPackets.length).to.equal(1); expect(otherPackets[0].constructor.tag).to.equal(openpgp.enums.packet.userID); }); + + describe('Grammar validation', async function () { + 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, getMessageGrammarValidator({ delayReporting: false }))).to.be.rejectedWith(/Data does not respect OpenPGP grammar/); + }); + + it('accepts padding and marker packets', async () => { + const packets = new openpgp.PacketList(); + const padding = new openpgp.PaddingPacket(); + await padding.createPadding(14); + 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({ delayReporting: false })); + expect(parsed.length).to.equal(1); // marker and padding packets are always dropped on parsing + + const messageGrammarValidatorWithLatentReporting = getMessageGrammarValidator({ delayReporting: true }); + const parsed2 = await openpgp.PacketList.fromBinary(packets.write(), allAllowedPackets, openpgp.config, messageGrammarValidatorWithLatentReporting); + expect(parsed2.length).to.equal(1); + const sampleGrammarValidatorReturnValue = isPartial => messageGrammarValidatorWithLatentReporting([] /* valid */, isPartial, openpgp.config); + expect(sampleGrammarValidatorReturnValue(true)).to.be.null; + expect(sampleGrammarValidatorReturnValue(false)).to.be.true; + }); + + it('accepts unknown packets', async () => { + const unknownPacketTag63 = util.hexToUint8Array('ff0a750064bf943d6e756c6c'); // non-critical tag + const parsed = await openpgp.PacketList.fromBinary(unknownPacketTag63, allAllowedPackets, openpgp.config, getMessageGrammarValidator({ delayReporting: false })); + expect(parsed.length).to.equal(0); + }); + + it('delay reporting', () => { + const messageGrammarValidatorWithLatentReporting = getMessageGrammarValidator({ delayReporting: true }); + + const sampleGrammarValidatorReturnValueValid = isPartial => messageGrammarValidatorWithLatentReporting([] /* valid */, isPartial, openpgp.config); + expect(sampleGrammarValidatorReturnValueValid(true)).to.be.null; + expect(sampleGrammarValidatorReturnValueValid(false)).to.be.true; + + const sampleGrammarValidatorReturnValueInvalid = isPartial => messageGrammarValidatorWithLatentReporting([openpgp.enums.packet.literalData, openpgp.enums.packet.literalData] /* invalid */, isPartial, openpgp.config); + expect(sampleGrammarValidatorReturnValueInvalid(true)).to.be.null; + expect(() => sampleGrammarValidatorReturnValueInvalid(false)).to.throw(/Data does not respect OpenPGP grammar/); + }); + }); }); describe('UserID', () => { diff --git a/test/general/signature.js b/test/general/signature.js index 648dd890..05360536 100644 --- a/test/general/signature.js +++ b/test/general/signature.js @@ -1836,7 +1836,7 @@ hkJiXopCSWKSlQInL1devkJJUWJmTmZeugJYlpdLAagQJM0JpsCqIQZwKgAA -----END PGP MESSAGE-----`; const plaintext = 'space: \nspace and tab: \t\nno trailing space\n \ntab:\t\ntab and space:\t '; - const message = await openpgp.readMessage({ armoredMessage }); + const message = await openpgp.readMessage({ armoredMessage, config: { enforceGrammar: false } }); const pubKey = await openpgp.readKey({ armoredKey: pub_key_arm2 }); const keyIDs = message.getSigningKeyIDs(); @@ -1849,8 +1849,10 @@ hkJiXopCSWKSlQInL1devkJJUWJmTmZeugJYlpdLAagQJM0JpsCqIQZwKgAA }); it('Streaming verify signed message with missing signature packet', async function() { - const armoredMessage = - `-----BEGIN PGP MESSAGE----- + const plaintext = 'space: \nspace and tab: \t\nno trailing space\n \ntab:\t\ntab and space:\t '; + await loadStreamsPolyfill(); + const getStreamedMessage = config => { + const armoredMessage = `-----BEGIN PGP MESSAGE----- Version: OpenPGP.js v3.1.3 Comment: https://openpgpjs.org @@ -1859,24 +1861,32 @@ hkJiXopCSWKSlQInL1devkJJUWJmTmZeugJYlpdLAagQJM0JpsCqIQZwKgAA =D6TZ -----END PGP MESSAGE-----`.split(''); - - const plaintext = 'space: \nspace and tab: \t\nno trailing space\n \ntab:\t\ntab and space:\t '; - await loadStreamsPolyfill(); - const message = await openpgp.readMessage({ - armoredMessage: new ReadableStream({ - async pull(controller) { - await new Promise(setTimeout); - controller.enqueue(armoredMessage.shift()); - if (!armoredMessage.length) controller.close(); - } - }) - }); + return openpgp.readMessage({ + armoredMessage: new ReadableStream({ + async pull(controller) { + await new Promise(setTimeout); + controller.enqueue(armoredMessage.shift()); + if (!armoredMessage.length) controller.close(); + } + }), + config + }); + }; const pubKey = await openpgp.readKey({ armoredKey: pub_key_arm2 }); + const message = await getStreamedMessage(); + const messageWithoutGrammar = await getStreamedMessage({ enforceGrammar: false }); + const keyIDs = message.getSigningKeyIDs(); expect(pubKey.getKeys(keyIDs[0])).to.not.be.empty; - return openpgp.verify({ verificationKeys: [pubKey], message, config: { minRSABits: 1024 } }).then(async ({ data, signatures }) => { + 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'); + 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 openpgp.verify({ verificationKeys: [pubKey], message: messageWithoutGrammar, config: { minRSABits: 1024 } }).then(async ({ data, signatures }) => { expect(await stream.readToEnd(data)).to.equal(plaintext); expect(signatures).to.have.length(1); await expect(signatures[0].verified).to.be.rejectedWith('Corresponding signature packet missing');