Flag not-yet-authenticated streams on the stream object

Instead of signalling that grammar checking should be delayed for
not-yet-authenticated streams on the grammar validator, signal that
the stream is not yet authenticated on the stream object.
This commit is contained in:
Daniel Huigens 2025-05-20 01:10:56 +02:00
parent e010101c42
commit dcf456049e
No known key found for this signature in database
GPG Key ID: CB064A128FA90686
7 changed files with 15 additions and 56 deletions

View File

@ -879,7 +879,7 @@ export async function readMessage({ armoredMessage, binaryMessage, config, ...re
} }
input = data; input = data;
} }
const packetlist = await PacketList.fromBinary(input, allowedMessagePackets, config, getMessageGrammarValidator({ delayReporting: false })); const packetlist = await PacketList.fromBinary(input, allowedMessagePackets, config, getMessageGrammarValidator());
const message = new Message(packetlist); const message = new Message(packetlist);
message.fromStream = streamType; message.fromStream = streamType;
return message; return message;

View File

@ -106,7 +106,7 @@ class AEADEncryptedDataPacket {
await runAEAD(this, 'decrypt', key, streamClone(this.encrypted)), await runAEAD(this, 'decrypt', key, streamClone(this.encrypted)),
allowedPackets, allowedPackets,
config, config,
getMessageGrammarValidator({ enforceDelay: false }) getMessageGrammarValidator()
); );
} }

View File

@ -114,7 +114,7 @@ class CompressedDataPacket {
} }
// Decompressing a Compressed Data packet MUST also yield a valid OpenPGP Message // 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 })); this.packets = await PacketList.fromBinary(await decompressionFn(this.compressed), allowedPackets, config, getMessageGrammarValidator());
} }
/** /**

View File

@ -66,24 +66,13 @@ const isValidOpenPGPMessage = (
isValidSignedMessage(packetList, acceptPartial); isValidSignedMessage(packetList, acceptPartial);
}; };
/** export const getMessageGrammarValidator = () => {
* 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; 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. * @throws on grammar error, provided `config.enforceGrammar` is enabled.
*/ */
return (list: number[], isPartial: boolean, config: Config): true | null => { return (list: number[], isPartial: boolean, config: Config): undefined => {
if (delayReporting && isPartial) return null; // delay until the full message has been parsed (i.e. authenticated)
if (!isValidOpenPGPMessage(list, isPartial)) { if (!isValidOpenPGPMessage(list, isPartial)) {
const error = new GrammarError(`Data does not respect OpenPGP grammar [${list}]`); const error = new GrammarError(`Data does not respect OpenPGP grammar [${list}]`);
if (!logged) { if (!logged) {
@ -92,11 +81,7 @@ export const getMessageGrammarValidator = ({ delayReporting }: { delayReporting:
} }
if (config.enforceGrammar) { if (config.enforceGrammar) {
throw error; throw error;
} else {
return true;
} }
} }
return true;
}; };
}; };

View File

@ -167,11 +167,9 @@ class PacketList extends Array {
break; break;
} }
if (supportsStreaming(value.constructor.tag)) { if (supportsStreaming(value.constructor.tag)) {
// The `tagsRead` are only sensitive if we are parsing an _unauthenticated_ decrypted stream, if (!bytes.unauthenticated) {
// 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?.(tagsRead, true, config); grammarValidator?.(tagsRead, true, config);
}
break; break;
} }
} }

View File

@ -184,23 +184,16 @@ class SymEncryptedIntegrityProtectedDataPacket {
if (isArrayStream(encrypted)) encrypted = await streamReadToEnd(encrypted); if (isArrayStream(encrypted)) encrypted = await streamReadToEnd(encrypted);
let packetbytes; let packetbytes;
let grammarValidator;
if (this.version === 2) { if (this.version === 2) {
if (this.cipherAlgorithm !== sessionKeyAlgorithm) { if (this.cipherAlgorithm !== sessionKeyAlgorithm) {
// sanity check // sanity check
throw new Error('Unexpected session key algorithm'); throw new Error('Unexpected session key algorithm');
} }
packetbytes = await runAEAD(this, 'decrypt', key, encrypted); packetbytes = await runAEAD(this, 'decrypt', key, encrypted);
grammarValidator = getMessageGrammarValidator({ delayReporting: false });
} else { } else {
const { blockSize } = getCipherParams(sessionKeyAlgorithm); const { blockSize } = getCipherParams(sessionKeyAlgorithm);
const decrypted = await cipherMode.cfb.decrypt(sessionKeyAlgorithm, key, encrypted, new Uint8Array(blockSize)); 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 // there must be a modification detection code packet as the
// last packet and everything gets hashed except the hash itself // last packet and everything gets hashed except the hash itself
const realHash = streamSlice(streamPassiveClone(decrypted), -20); const realHash = streamSlice(streamPassiveClone(decrypted), -20);
@ -219,7 +212,9 @@ class SymEncryptedIntegrityProtectedDataPacket {
const bytes = streamSlice(tohash, blockSize + 2); // Remove random prefix const bytes = streamSlice(tohash, blockSize + 2); // Remove random prefix
packetbytes = streamSlice(bytes, 0, -2); // Remove MDC packet packetbytes = streamSlice(bytes, 0, -2); // Remove MDC packet
packetbytes = streamConcat([packetbytes, streamFromAsync(() => verifyHash)]); packetbytes = streamConcat([packetbytes, streamFromAsync(() => verifyHash)]);
if (!releaseUnauthenticatedStream) { if (util.isStream(encrypted) && config.allowUnauthenticatedStream) {
packetbytes.unauthenticated = true;
} else {
packetbytes = await streamReadToEnd(packetbytes); packetbytes = await streamReadToEnd(packetbytes);
} }
} }
@ -228,7 +223,7 @@ class SymEncryptedIntegrityProtectedDataPacket {
// MUST yield a valid OpenPGP Message. // MUST yield a valid OpenPGP Message.
// - Decrypting a version 2 Symmetrically Encrypted and Integrity Protected Data packet // - Decrypting a version 2 Symmetrically Encrypted and Integrity Protected Data packet
// MUST yield a valid Optionally Padded Message. // MUST yield a valid Optionally Padded Message.
this.packets = await PacketList.fromBinary(packetbytes, allowedPackets, config, grammarValidator); this.packets = await PacketList.fromBinary(packetbytes, allowedPackets, config, getMessageGrammarValidator());
return true; return true;
} }
} }

View File

@ -1379,7 +1379,7 @@ kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu
const packets = new openpgp.PacketList(); const packets = new openpgp.PacketList();
packets.push(new openpgp.LiteralDataPacket()); packets.push(new openpgp.LiteralDataPacket());
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/); await expect(openpgp.PacketList.fromBinary(packets.write(), allAllowedPackets, openpgp.config, getMessageGrammarValidator())).to.be.rejectedWith(/Data does not respect OpenPGP grammar/);
}); });
it('accepts padding and marker packets', async () => { it('accepts padding and marker packets', async () => {
@ -1389,34 +1389,15 @@ kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu
packets.push(padding); packets.push(padding);
packets.push(new openpgp.MarkerPacket()); packets.push(new openpgp.MarkerPacket());
packets.push(new openpgp.LiteralDataPacket()); packets.push(new openpgp.LiteralDataPacket());
const parsed = await openpgp.PacketList.fromBinary(packets.write(), allAllowedPackets, openpgp.config, getMessageGrammarValidator({ delayReporting: false })); const parsed = await openpgp.PacketList.fromBinary(packets.write(), allAllowedPackets, openpgp.config, getMessageGrammarValidator());
expect(parsed.length).to.equal(1); // marker and padding packets are always dropped on parsing 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 () => { it('accepts unknown packets', async () => {
const unknownPacketTag63 = util.hexToUint8Array('ff0a750064bf943d6e756c6c'); // non-critical tag const unknownPacketTag63 = util.hexToUint8Array('ff0a750064bf943d6e756c6c'); // non-critical tag
const parsed = await openpgp.PacketList.fromBinary(unknownPacketTag63, allAllowedPackets, openpgp.config, getMessageGrammarValidator({ delayReporting: false })); const parsed = await openpgp.PacketList.fromBinary(unknownPacketTag63, allAllowedPackets, openpgp.config, getMessageGrammarValidator());
expect(parsed.length).to.equal(1); expect(parsed.length).to.equal(1);
}); });
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/);
});
}); });
}); });