Ignore Trust and Marker packets on parsing and always throw on unexpected packets (#1340)

- When parsing, throw on unexpected packets even if `config.tolerant = true`
(e.g. if a Public Key packet is found when reading a signature).
- Always ignore Trust and Marker packets on parsing.
- Fix #1145: correctly verify signatures that include Marker packets when
`config.tolerant = false`.
This commit is contained in:
larabr 2021-06-23 12:17:29 +02:00 committed by GitHub
parent a9252c6649
commit b76236755a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 144 additions and 56 deletions

View File

@ -85,21 +85,15 @@ export class Message {
* @returns {Array<module:type/keyid~KeyID>} 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);
}
/**

View File

@ -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)]);

View File

@ -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);

View File

@ -57,12 +57,7 @@ export class Signature {
* @returns {Array<KeyID>} 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);
}
}

View File

@ -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/);
});

View File

@ -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/);
});
});

View File

@ -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({