diff --git a/src/packet/packetlist.js b/src/packet/packetlist.js index 67205661..fe0c2ec9 100644 --- a/src/packet/packetlist.js +++ b/src/packet/packetlist.js @@ -1,4 +1,4 @@ -import { transformPair as streamTransformPair, transform as streamTransform, getWriter as streamGetWriter, getReader as streamGetReader, clone as streamClone } from '@openpgp/web-stream-tools'; +import { transformPair as streamTransformPair, transform as streamTransform, getWriter as streamGetWriter, getReader as streamGetReader, clone as streamClone, readToEnd as streamReadToEnd } from '@openpgp/web-stream-tools'; import { readPackets, supportsStreaming, writeTag, writeHeader, @@ -75,6 +75,7 @@ class PacketList extends Array { try { while (true) { await writer.ready; + let error; const done = await readPackets(readable, async parsed => { try { if (parsed.tag === enums.packet.marker || parsed.tag === enums.packet.trust || parsed.tag === enums.packet.padding) { @@ -134,7 +135,11 @@ class PacketList extends Array { throwDataPacketError || throwOtherError ) { - await writer.abort(e); + if (bytes.unauthenticated) { + error = e; + } else { + await writer.abort(e); + } } else { const unparsedPacket = new UnparseablePacket(parsed.tag, parsed.packet); await writer.write(unparsedPacket); @@ -142,6 +147,14 @@ class PacketList extends Array { util.printDebugError(e); } }); + + // If there was a parse error, read the entire input first + // in case there's an MDC error, which should take precedence. + if (error) { + await streamReadToEnd(readable); + // eslint-disable-next-line @typescript-eslint/no-throw-literal + throw error; + } if (done) { // Here we are past the MDC check for SEIPDv1 data, hence // the data is always authenticated at this point. diff --git a/test/general/packet.js b/test/general/packet.js index fab13aa8..743f5e46 100644 --- a/test/general/packet.js +++ b/test/general/packet.js @@ -1382,6 +1382,146 @@ kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu await expect(openpgp.PacketList.fromBinary(packets.write(), allAllowedPackets, openpgp.config, getMessageGrammarValidator())).to.be.rejectedWith(/Data does not respect OpenPGP grammar/); }); + it('reject duplicate literal packet inside encrypted data', async () => { + const literalPackets = new openpgp.PacketList(); + literalPackets.push(new openpgp.LiteralDataPacket()); + literalPackets.push(new openpgp.LiteralDataPacket()); + const encrypted = new openpgp.SymEncryptedIntegrityProtectedDataPacket(); + encrypted.version = 1; + encrypted.packets = literalPackets; + const packets = new openpgp.PacketList(); + packets.push(encrypted); + await encrypted.encrypt(openpgp.enums.symmetric.aes128, new Uint8Array(16)); + await expect(openpgp.decrypt({ + message: await openpgp.readMessage({ + binaryMessage: packets.write() + }), + sessionKeys: [{ algorithm: 'aes128', data: new Uint8Array(16) }] + })).to.be.rejectedWith(/Data does not respect OpenPGP grammar/); + }); + + it('reject duplicate literal packet inside encrypted data (streaming)', async () => { + const literalPackets = new openpgp.PacketList(); + literalPackets.push(new openpgp.LiteralDataPacket()); + literalPackets.push(new openpgp.LiteralDataPacket()); + const encrypted = new openpgp.SymEncryptedIntegrityProtectedDataPacket(); + encrypted.version = 1; + encrypted.packets = literalPackets; + const packets = new openpgp.PacketList(); + packets.push(encrypted); + await encrypted.encrypt(openpgp.enums.symmetric.aes128, new Uint8Array(16)); + const decrypted = await openpgp.decrypt({ + message: await openpgp.readMessage({ + binaryMessage: new ReadableStream({ + start(controller) { + controller.enqueue(packets.write()); + controller.close(); + } + }) + }), + sessionKeys: [{ algorithm: 'aes128', data: new Uint8Array(16) }], + config: { + allowUnauthenticatedStream: true + } + }); + await expect(stream.readToEnd(decrypted.data)).to.be.rejectedWith(/Data does not respect OpenPGP grammar/); + }); + + it('reject duplicate literal packet inside encrypted data (MDC error gets precedence)', async () => { + const literalPackets = new openpgp.PacketList(); + literalPackets.push(new openpgp.LiteralDataPacket()); + const literal = new openpgp.LiteralDataPacket(); + literal.data = new Uint8Array(1000); + literalPackets.push(literal); + const encrypted = new openpgp.SymEncryptedIntegrityProtectedDataPacket(); + encrypted.version = 1; + encrypted.packets = literalPackets; + const packets = new openpgp.PacketList(); + packets.push(encrypted); + await encrypted.encrypt(openpgp.enums.symmetric.aes128, new Uint8Array(16)); + const encryptedData = packets.write(); + encryptedData[encryptedData.length - 5] ^= 1; + const decrypted = await openpgp.decrypt({ + message: await openpgp.readMessage({ + binaryMessage: new ReadableStream({ + start(controller) { + controller.enqueue(encryptedData.subarray(0, 500)); + controller.enqueue(encryptedData.subarray(500)); + controller.close(); + } + }) + }), + sessionKeys: [{ algorithm: 'aes128', data: new Uint8Array(16) }], + config: { + allowUnauthenticatedStream: true + } + }); + await expect(stream.readToEnd(decrypted.data)).to.be.rejectedWith(/Modification detected/); + }); + + it('reject malformed packet inside encrypted data', async () => { + const literalPackets = new openpgp.PacketList(); + const signature = new openpgp.SignaturePacket(); + signature.signatureData = signature.signedHashValue = new Uint8Array([4, 4]); + signature.params = {}; + literalPackets.push(signature); + literalPackets.push(new openpgp.LiteralDataPacket()); + const encrypted = new openpgp.SymEncryptedIntegrityProtectedDataPacket(); + encrypted.version = 1; + encrypted.packets = literalPackets; + const packets = new openpgp.PacketList(); + packets.push(encrypted); + await encrypted.encrypt(openpgp.enums.symmetric.aes128, new Uint8Array(16)); + const encryptedData = packets.write(); + await expect(openpgp.decrypt({ + message: await openpgp.readMessage({ + binaryMessage: new ReadableStream({ + start(controller) { + controller.enqueue(encryptedData.subarray(0, 500)); + controller.enqueue(encryptedData.subarray(500)); + controller.close(); + } + }) + }), + sessionKeys: [{ algorithm: 'aes128', data: new Uint8Array(16) }], + config: { + allowUnauthenticatedStream: true + } + })).to.be.rejectedWith(/Missing signature creation time subpacket/); + }); + + it('reject malformed packet inside encrypted data (MDC error gets precedence)', async () => { + const literalPackets = new openpgp.PacketList(); + const signature = new openpgp.SignaturePacket(); + signature.signatureData = signature.signedHashValue = new Uint8Array([4, 4]); + signature.params = {}; + literalPackets.push(signature); + literalPackets.push(new openpgp.LiteralDataPacket()); + const encrypted = new openpgp.SymEncryptedIntegrityProtectedDataPacket(); + encrypted.version = 1; + encrypted.packets = literalPackets; + const packets = new openpgp.PacketList(); + packets.push(encrypted); + await encrypted.encrypt(openpgp.enums.symmetric.aes128, new Uint8Array(16)); + const encryptedData = packets.write(); + encryptedData[encryptedData.length - 5] ^= 1; + await expect(openpgp.decrypt({ + message: await openpgp.readMessage({ + binaryMessage: new ReadableStream({ + start(controller) { + controller.enqueue(encryptedData.subarray(0, 500)); + controller.enqueue(encryptedData.subarray(500)); + controller.close(); + } + }) + }), + sessionKeys: [{ algorithm: 'aes128', data: new Uint8Array(16) }], + config: { + allowUnauthenticatedStream: true + } + })).to.be.rejectedWith(/Modification detected/); + }); + it('accepts padding and marker packets', async () => { const packets = new openpgp.PacketList(); const padding = new openpgp.PaddingPacket();