diff --git a/src/message.js b/src/message.js index cc92cc95..111246de 100644 --- a/src/message.js +++ b/src/message.js @@ -85,21 +85,15 @@ export class Message { * @returns {Array} Array of keyID objects. */ getSigningKeyIDs() { - const keyIDs = []; const msg = this.unwrapCompressed(); // search for one pass signatures const onePassSigList = msg.packets.filterByTag(enums.packet.onePassSignature); - onePassSigList.forEach(function(packet) { - keyIDs.push(packet.issuerKeyID); - }); - // if nothing found look for signature packets - if (!keyIDs.length) { - const signatureList = msg.packets.filterByTag(enums.packet.signature); - signatureList.forEach(function(packet) { - keyIDs.push(packet.issuerKeyID); - }); + if (onePassSigList.length > 0) { + return onePassSigList.map(packet => packet.issuerKeyID); } - return keyIDs; + // if nothing found look for signature packets + const signatureList = msg.packets.filterByTag(enums.packet.signature); + return signatureList.map(packet => packet.issuerKeyID); } /** diff --git a/src/packet/one_pass_signature.js b/src/packet/one_pass_signature.js index 5572fce8..6ca206b7 100644 --- a/src/packet/one_pass_signature.js +++ b/src/packet/one_pass_signature.js @@ -109,7 +109,7 @@ class OnePassSignaturePacket { * @returns {Uint8Array} A Uint8Array representation of a one-pass signature packet. */ write() { - const start = new Uint8Array([3, enums.write(enums.signature, this.signatureType), + const start = new Uint8Array([VERSION, enums.write(enums.signature, this.signatureType), enums.write(enums.hash, this.hashAlgorithm), enums.write(enums.publicKey, this.publicKeyAlgorithm)]); diff --git a/src/packet/packetlist.js b/src/packet/packetlist.js index 7db6f417..aee167b4 100644 --- a/src/packet/packetlist.js +++ b/src/packet/packetlist.js @@ -15,6 +15,7 @@ import defaultConfig from '../config'; * @param {module:enums.packet} tag - Property value from {@link module:enums.packet} * @param {Object} allowedPackets - mapping where keys are allowed packet tags, pointing to their Packet class * @returns {Object} New packet object with type based on tag + * @throws {Error|UnsupportedError} for disallowed or unknown packets */ export function newPacketFromTag(tag, allowedPackets) { if (!allowedPackets[tag]) { @@ -25,7 +26,7 @@ export function newPacketFromTag(tag, allowedPackets) { } catch (e) { throw new UnsupportedError(`Unknown packet type with tag: ${tag}`); } - throw new UnsupportedError(`Packet not allowed in this context: ${packetType}`); + throw new Error(`Packet not allowed in this context: ${packetType}`); } return new allowedPackets[tag](); } @@ -69,6 +70,12 @@ class PacketList extends Array { await writer.ready; const done = await readPackets(readable, async parsed => { try { + if (parsed.tag === enums.packet.marker || parsed.tag === enums.packet.trust) { + // According to the spec, these packet types should be ignored and not cause parsing errors, even if not esplicitly allowed: + // - Marker packets MUST be ignored when received: https://github.com/openpgpjs/openpgpjs/issues/1145 + // - Trust packets SHOULD be ignored outside of keyrings (unsupported): https://datatracker.ietf.org/doc/html/rfc4880#section-5.10 + return; + } const packet = newPacketFromTag(parsed.tag, allowedPackets); packet.packets = new PacketList(); packet.fromStream = util.isStream(parsed.packet); diff --git a/src/signature.js b/src/signature.js index b50b6262..dd05363c 100644 --- a/src/signature.js +++ b/src/signature.js @@ -57,12 +57,7 @@ export class Signature { * @returns {Array} The Key IDs of the signing keys */ getSigningKeyIDs() { - const keyIDs = []; - const signatureList = this.packets.filterByTag(enums.packet.signature); - signatureList.forEach(function(packet) { - keyIDs.push(packet.issuerKeyID); - }); - return keyIDs; + return this.packets.map(packet => packet.issuerKeyID); } } diff --git a/test/general/config.js b/test/general/config.js index 2998a4c6..da2dc084 100644 --- a/test/general/config.js +++ b/test/general/config.js @@ -6,16 +6,16 @@ module.exports = () => describe('Custom configuration', function() { it('openpgp.readMessage', async function() { const armoredMessage = await openpgp.encrypt({ message: await openpgp.createMessage({ text:"hello world" }), passwords: 'password' }); const message = await openpgp.readMessage({ armoredMessage }); - message.packets.unshift(new openpgp.MarkerPacket()); // MarkerPacket is not allowed in the Message context + message.packets.findPacket(openpgp.SymEncryptedSessionKeyPacket.tag).version = 1; // unsupported SKESK version const config = { tolerant: true }; const parsedMessage = await openpgp.readMessage({ armoredMessage: message.armor(), config }); - expect(parsedMessage.packets.length).to.equal(2); + expect(parsedMessage.packets.length).to.equal(1); config.tolerant = false; await expect( openpgp.readMessage({ armoredMessage: message.armor(), config }) - ).to.be.rejectedWith(/Packet not allowed in this context/); + ).to.be.rejectedWith(/Version 1 of the SKESK packet is unsupported/); }); it('openpgp.readSignature', async function() { @@ -28,44 +28,26 @@ vAFM3jjrAQDgJPXsv8PqCrLGDuMa/2r6SgzYd03aw/xt1WM6hgUvhQD+J54Z -----END PGP SIGNATURE-----`; const signature = await openpgp.readSignature({ armoredSignature }); - signature.packets.unshift(new openpgp.MarkerPacket()); // MarkerPacket is not allowed in the Signature context + signature.packets[0].signatureData[0] = 1; // set unsupported signature version const config = { tolerant: true }; const parsedSignature = await openpgp.readSignature({ armoredSignature: signature.armor(), config }); - expect(parsedSignature.packets.length).to.equal(1); + expect(parsedSignature.packets.length).to.equal(0); config.tolerant = false; await expect( openpgp.readSignature({ armoredSignature: signature.armor(), config }) - ).to.be.rejectedWith(/Packet not allowed in this context/); + ).to.be.rejectedWith(/Version 1 of the signature packet is unsupported/); }); it('openpgp.readKey', async function() { - const armoredKey = `-----BEGIN PGP PUBLIC KEY BLOCK----- - -xjMEYI/KsBYJKwYBBAHaRw8BAQdAW2lu0r97hQztwP8+WbSF9N/QJ5hkevhm -CGJbM3HBvznNEHRlc3QgPHRlc3RAYS5pdD7CjAQQFgoAHQUCYI/KsAQLCQcI -AxUICgQWAAIBAhkBAhsDAh4BACEJEKgxHS8jVhd9FiEE8hy8OerCaNGuKPDw -qDEdLyNWF32XOQD/dq2/D394eW67VwUhvRpQl9gwToDf+SixATEFigok5JgA -/3ZeH9eXiZqo3rChfdQ+3VKTd7yoI2gM/pjbHupemYYAzjgEYI/KsBIKKwYB -BAGXVQEFAQEHQN/8mxAaro95FmvPQ4wlAfk3WKUHZtNvpaqzXo1K6WdMAwEI -B8J4BBgWCAAJBQJgj8qwAhsMACEJEKgxHS8jVhd9FiEE8hy8OerCaNGuKPDw -qDEdLyNWF30o6wD/fZYCV8aS4dAu2U3fpN5y5+PbuXFRYljA5gQ/1zrGN/UA -/3r62WsCVupzKdISZYOMPwEY5qN/4f9i6ZWxIynmVX0E -=6+P3 ------END PGP PUBLIC KEY BLOCK-----`; - - const keyPackets = (await openpgp.readKey({ armoredKey })).toPacketList(); - keyPackets.unshift(new openpgp.MarkerPacket()); // MarkerPacket is not allowed in the Signature context - - const config = { tolerant: true }; - const parsedKey = await openpgp.readKey({ binaryKey: keyPackets.write(), config }); - expect(parsedKey.toPacketList().length).to.equal(5); - - config.tolerant = false; + const { privateKeyArmored: armoredKey } = await openpgp.generateKey({ userIDs:[{ name:'test', email:'test@a.it' }] }); await expect( - openpgp.readKey({ binaryKey: keyPackets.write(), config }) - ).to.be.rejectedWith(/Packet not allowed in this context/); + openpgp.readKey({ armoredKey, config: { tolerant: false, maxUserIDLength: 2 } }) + ).to.be.rejectedWith(/User ID string is too long/); + await expect( + openpgp.readKey({ armoredKey, config: { tolerant: true, maxUserIDLength: 2 } }) + ).to.be.rejectedWith(/User ID string is too long/); }); diff --git a/test/general/packet.js b/test/general/packet.js index f5f356f4..dfc64e13 100644 --- a/test/general/packet.js +++ b/test/general/packet.js @@ -950,25 +950,68 @@ V+HOQJQxXJkVRYa3QrFUehiMzTeqqMdgC6ZqJy7+ }); describe('PacketList parsing', function () { - it('Ignores disallowed packet with tolerant mode enabled', async function() { + it('Ignores unknown packet version with tolerant mode enabled', async function() { + const armoredSignature = `-----BEGIN PGP SIGNATURE----- + +iQFKBAEBCgA0FiEEdOyNPagqedqiXfEMa6Ve2Dq64bsFAlszXwQWHHRlc3Qtd2tk +QG1ldGFjb2RlLmJpegAKCRBrpV7YOrrhuw1PB/9KhFRR/M3OR6NmIent6ri1ekWn +vlcnVqj6N4Xqi1ahRVw19/Jx36mGyijxNwqqGrziqRiPCdT0pKfCfv7nXQf2Up1Z +LoR1StqpBMSDQfuF6JAJmJuB9T+mPQO8wYeUp+O63vQnm5CgqyoRlIoqX8MN6GTY +xK5PdTRjw6IEIGr9uLgSoUwTd0ECY1F9ptyuLGD5ET5ZtyUenQSbX+cw5WCGLFzi +7TwKTY+kGQpkwDJKZJSGpoP7ob6xdDfZx6dHV6IfIJg8/F9gtAXFp8uE51L90cV2 +kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu +=wEIR +-----END PGP SIGNATURE-----`; + + const { packets: [signaturePacket] } = await openpgp.readSignature({ armoredSignature }); const packets = new openpgp.PacketList(); - packets.push(new openpgp.MarkerPacket()); + signaturePacket.signatureData[0] = 1; + packets.push(signaturePacket); const bytes = packets.write(); - const parsed = await openpgp.PacketList.fromBinary(bytes, {}, { ...openpgp.config, tolerant: true }); + const parsed = await openpgp.PacketList.fromBinary(bytes, allAllowedPackets, { ...openpgp.config, tolerant: true }); expect(parsed.length).to.equal(0); }); - it('Throws on disallowed packet with tolerant mode disabled', async function() { + it('Throws on unknown packet version with tolerant mode disabled', async function() { + const armoredSignature = `-----BEGIN PGP SIGNATURE----- + +iQFKBAEBCgA0FiEEdOyNPagqedqiXfEMa6Ve2Dq64bsFAlszXwQWHHRlc3Qtd2tk +QG1ldGFjb2RlLmJpegAKCRBrpV7YOrrhuw1PB/9KhFRR/M3OR6NmIent6ri1ekWn +vlcnVqj6N4Xqi1ahRVw19/Jx36mGyijxNwqqGrziqRiPCdT0pKfCfv7nXQf2Up1Z +LoR1StqpBMSDQfuF6JAJmJuB9T+mPQO8wYeUp+O63vQnm5CgqyoRlIoqX8MN6GTY +xK5PdTRjw6IEIGr9uLgSoUwTd0ECY1F9ptyuLGD5ET5ZtyUenQSbX+cw5WCGLFzi +7TwKTY+kGQpkwDJKZJSGpoP7ob6xdDfZx6dHV6IfIJg8/F9gtAXFp8uE51L90cV2 +kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu +=wEIR +-----END PGP SIGNATURE-----`; + + const { packets: [signaturePacket] } = await openpgp.readSignature({ armoredSignature }); const packets = new openpgp.PacketList(); - packets.push(new openpgp.MarkerPacket()); + signaturePacket.signatureData[0] = 1; + packets.push(signaturePacket); + const bytes = packets.write(); + await expect( + openpgp.PacketList.fromBinary(bytes, allAllowedPackets, { ...openpgp.config, tolerant: false }) + ).to.be.rejectedWith(/Version 1 of the signature packet is unsupported/); + }); + + it('Throws on disallowed packet even with tolerant mode enabled', async function() { + const packets = new openpgp.PacketList(); + packets.push(new openpgp.LiteralDataPacket()); const bytes = packets.write(); await expect(openpgp.PacketList.fromBinary(bytes, {}, { ...openpgp.config, tolerant: false })).to.be.rejectedWith(/Packet not allowed in this context/); + await expect(openpgp.PacketList.fromBinary(bytes, {}, { ...openpgp.config, tolerant: true })).to.be.rejectedWith(/Packet not allowed in this context/); }); it('Throws on parsing errors even with tolerant mode enabled', async function () { - const { privateKeyArmored: armoredKey } = await openpgp.generateKey({ userIDs:[{ name:'test', email:'test@a.it' }] }); + const packets = new openpgp.PacketList(); + packets.push(openpgp.UserIDPacket.fromObject({ name:'test', email:'test@a.it' })); + const bytes = packets.write(); await expect( - openpgp.readKey({ armoredKey, config: { tolerant: true, maxUserIDLength: 2 } }) + openpgp.PacketList.fromBinary(bytes, allAllowedPackets, { ...openpgp.config, maxUserIDLength: 2, tolerant: false }) + ).to.be.rejectedWith(/User ID string is too long/); + await expect( + openpgp.PacketList.fromBinary(bytes, allAllowedPackets, { ...openpgp.config, maxUserIDLength: 2, tolerant: true }) ).to.be.rejectedWith(/User ID string is too long/); }); }); diff --git a/test/general/signature.js b/test/general/signature.js index a265a73f..8a2cdf86 100644 --- a/test/general/signature.js +++ b/test/general/signature.js @@ -629,6 +629,49 @@ lRrCkyLzwq0M+UUHQAuYpAfobDlDdnxxOD2jm5GyTzak3GSVFfjW09QFVO6HlGp5 01/jtzkUiS6nwoHHkfnyn0beZuR8X6KlcrzLB0VFgQFLmkSM9cSOgYhD0PTu9aHb hW1Hj9AO8lzggBQ= =Nt+N +-----END PGP PUBLIC KEY BLOCK-----`; + + const sequoiaBobPublicKey = `-----BEGIN PGP PUBLIC KEY BLOCK----- +Comment: Bob's OpenPGP certificate + +mQGNBF2lnPIBDAC5cL9PQoQLTMuhjbYvb4Ncuuo0bfmgPRFywX53jPhoFf4Zg6mv +/seOXpgecTdOcVttfzC8ycIKrt3aQTiwOG/ctaR4Bk/t6ayNFfdUNxHWk4WCKzdz +/56fW2O0F23qIRd8UUJp5IIlN4RDdRCtdhVQIAuzvp2oVy/LaS2kxQoKvph/5pQ/ +5whqsyroEWDJoSV0yOb25B/iwk/pLUFoyhDG9bj0kIzDxrEqW+7Ba8nocQlecMF3 +X5KMN5kp2zraLv9dlBBpWW43XktjcCZgMy20SouraVma8Je/ECwUWYUiAZxLIlMv +9CurEOtxUw6N3RdOtLmYZS9uEnn5y1UkF88o8Nku890uk6BrewFzJyLAx5wRZ4F0 +qV/yq36UWQ0JB/AUGhHVPdFf6pl6eaxBwT5GXvbBUibtf8YI2og5RsgTWtXfU7eb +SGXrl5ZMpbA6mbfhd0R8aPxWfmDWiIOhBufhMCvUHh1sApMKVZnvIff9/0Dca3wb +vLIwa3T4CyshfT0AEQEAAbQhQm9iIEJhYmJhZ2UgPGJvYkBvcGVucGdwLmV4YW1w +bGU+iQHOBBMBCgA4AhsDBQsJCAcCBhUKCQgLAgQWAgMBAh4BAheAFiEE0aZuGiOx +gsmYD3iM+/zIKgFeczAFAl2lnvoACgkQ+/zIKgFeczBvbAv/VNk90a6hG8Od9xTz +XxH5YRFUSGfIA1yjPIVOnKqhMwps2U+sWE3urL+MvjyQRlyRV8oY9IOhQ5Esm6DO +ZYrTnE7qVETm1ajIAP2OFChEc55uH88x/anpPOXOJY7S8jbn3naC9qad75BrZ+3g +9EBUWiy5p8TykP05WSnSxNRt7vFKLfEB4nGkehpwHXOVF0CRNwYle42bg8lpmdXF +DcCZCi+qEbafmTQzkAqyzS3nCh3IAqq6Y0kBuaKLm2tSNUOlZbD+OHYQNZ5Jix7c +ZUzs6Xh4+I55NRWl5smrLq66yOQoFPy9jot/Qxikx/wP3MsAzeGaZSEPc0fHp5G1 +6rlGbxQ3vl8/usUV7W+TMEMljgwd5x8POR6HC8EaCDfVnUBCPi/Gv+egLjsIbPJZ +ZEroiE40e6/UoCiQtlpQB5exPJYSd1Q1txCwueih99PHepsDhmUQKiACszNU+RRo +zAYau2VdHqnRJ7QYdxHDiH49jPK4NTMyb/tJh2TiIwcmsIpGuQGNBF2lnPIBDADW +ML9cbGMrp12CtF9b2P6z9TTT74S8iyBOzaSvdGDQY/sUtZXRg21HWamXnn9sSXvI +DEINOQ6A9QxdxoqWdCHrOuW3ofneYXoG+zeKc4dC86wa1TR2q9vW+RMXSO4uImA+ +Uzula/6k1DogDf28qhCxMwG/i/m9g1c/0aApuDyKdQ1PXsHHNlgd/Dn6rrd5y2AO +baifV7wIhEJnvqgFXDN2RXGjLeCOHV4Q2WTYPg/S4k1nMXVDwZXrvIsA0YwIMgIT +86Rafp1qKlgPNbiIlC1g9RY/iFaGN2b4Ir6GDohBQSfZW2+LXoPZuVE/wGlQ01rh +827KVZW4lXvqsge+wtnWlszcselGATyzqOK9LdHPdZGzROZYI2e8c+paLNDdVPL6 +vdRBUnkCaEkOtl1mr2JpQi5nTU+gTX4IeInC7E+1a9UDF/Y85ybUz8XV8rUnR76U +qVC7KidNepdHbZjjXCt8/Zo+Tec9JNbYNQB/e9ExmDntmlHEsSEQzFwzj8sxH48A +EQEAAYkBtgQYAQoAIBYhBNGmbhojsYLJmA94jPv8yCoBXnMwBQJdpZzyAhsMAAoJ +EPv8yCoBXnMw6f8L/26C34dkjBffTzMj5Bdzm8MtF67OYneJ4TQMw7+41IL4rVcS +KhIhk/3Ud5knaRtP2ef1+5F66h9/RPQOJ5+tvBwhBAcUWSupKnUrdVaZQanYmtSx +cVV2PL9+QEiNN3tzluhaWO//rACxJ+K/ZXQlIzwQVTpNhfGzAaMVV9zpf3u0k14i +tcv6alKY8+rLZvO1wIIeRZLmU0tZDD5HtWDvUV7rIFI1WuoLb+KZgbYn3OWjCPHV +dTrdZ2CqnZbG3SXw6awH9bzRLV9EXkbhIMez0deCVdeo+wFFklh8/5VK2b0vk/+w +qMJxfpa1lHvJLobzOP9fvrswsr92MA2+k901WeISR7qEzcI0Fdg8AyFAExaEK6Vy +jP7SXGLwvfisw34OxuZr3qmx1Sufu4toH3XrB7QJN8XyqqbsGxUCBqWif9RSK4xj +zRTe56iPeiSJJOIciMP9i2ldI+KgLycyeDvGoBj0HCLO3gVaBe4ubVrj5KjhX2PV +NEJd3XZRzaXZE2aAMQ== +=NXei -----END PGP PUBLIC KEY BLOCK-----`; const signature_with_critical_notation = `-----BEGIN PGP MESSAGE----- @@ -692,6 +735,30 @@ kCNcH9WI6idSzFjuYegECf+ZA1xOCjS9oLTGbSeT7jNfC8dH5+E92qlBLq4Ctt7k await expect(openpgp.readSignature({ armoredSignature })).to.be.rejectedWith(/Missing signature creation time/); }); + it('Ignores marker packets when verifying signatures', async function () { + const signatureWithMarkerPacket = `-----BEGIN PGP SIGNATURE----- + +ygNQR1DCwPMEAAEKAAYFgl8831AAIQkQ+/zIKgFeczAWIQTRpm4aI7GCyZgPeIz7 +/MgqAV5zMLckDACWiDbasKMTX/+czxHXyVcFJ/+ZeYqKEjYq6LueHy11XjJ0NZAM +LG9TqsXpWOsHrwE6wUQ7RvKQYtfIAeMUZtD87/XomIj6B/rQC5dHuQTb0b8lrRJb +OuW1sz6AYwceqkSvN3T5+KKNMXkaFw/DzWGPfqQQJDOqfgKxf5uO7GPVzaIU6aXn +76iKHQ7wowT2qHoFhd+t4S11iGr6XJef6QqIW2kTetZMf2Dp/rr7228VJJ1S0RdD +xxKJEbNrmdMNgE8/U+pkWjMQyVOOxWyPKlG3kv2Cu/naj4Lg2io3RhOAuNW5xEJF +gAId3WUNl3/PCu/PcTS1yS/Nj0ptwjKHwG0Zg8Dk5Jey8lUVyVhjxrV5tb6NLoAG +RlyTajZ3Sjhsg4mXHopjSF2w30saN64L5VAfGF1afFu7yzNYC+Fn6GL5yTJfKs4j +PNo4zCwOCumsVP0jWp09yUNflE6MTd21miBgbmPxyLyuwP2YrvT4+rCl+meNZ98a +cJRRGJPL16wINuk= +=VNoM +-----END PGP SIGNATURE-----`; + + const key = await openpgp.readKey({ armoredKey: sequoiaBobPublicKey }); + const message = await openpgp.createMessage({ text: 'Marker + Detached signature' }); + const signature = await openpgp.readSignature({ armoredSignature: signatureWithMarkerPacket }); + const { signatures: [sigInfo] } = await openpgp.verify({ message, signature, verificationKeys: key }); + expect(sigInfo.valid).to.be.true; + }); + + it('Testing signature checking on CAST5-enciphered message', async function() { const publicKey = await openpgp.readKey({ armoredKey: pub_key_arm1 }); const privateKey = await openpgp.decryptKey({