Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix forward compatibility of keys, SKESKs, and detached/cleartext signatures and ECDH #1656

Merged
merged 4 commits into from Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cleartext.js
Expand Up @@ -89,7 +89,7 @@ export class CleartextMessage {
* @async
*/
verify(keys, date = new Date(), config = defaultConfig) {
const signatureList = this.signature.packets;
const signatureList = this.signature.packets.filterByTag(enums.packet.signature); // drop UnparsablePackets
const literalDataPacket = new LiteralDataPacket();
// we assume that cleartext signature is generated based on UTF8 cleartext
literalDataPacket.setText(this.text);
Expand Down
2 changes: 1 addition & 1 deletion src/message.js
Expand Up @@ -652,7 +652,7 @@ export class Message {
if (literalDataList.length !== 1) {
throw new Error('Can only verify message with one literal data packet.');
}
const signatureList = signature.packets;
const signatureList = signature.packets.filterByTag(enums.packet.signature); // drop UnparsablePackets
return createVerificationObjects(signatureList, literalDataList, verificationKeys, date, true, config);
}

Expand Down
4 changes: 3 additions & 1 deletion src/type/kdf_params.js
Expand Up @@ -15,6 +15,8 @@
// License along with this library; if not, write to the Free Software
// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA

import { UnsupportedError } from '../packet/packet';

/**
* Implementation of type KDF parameters
*
Expand Down Expand Up @@ -50,7 +52,7 @@ class KDFParams {
*/
read(input) {
if (input.length < 4 || input[0] !== 3 || input[1] !== 1) {
throw new Error('Cannot read KDFParams');
throw new UnsupportedError('Cannot read KDFParams');
}
this.hash = input[2];
this.cipher = input[3];
Expand Down
13 changes: 9 additions & 4 deletions src/type/s2k.js
Expand Up @@ -31,6 +31,7 @@
import defaultConfig from '../config';
import crypto from '../crypto';
import enums from '../enums';
import { UnsupportedError } from '../packet/packet';
import util from '../util';

class S2K {
Expand Down Expand Up @@ -70,7 +71,11 @@ class S2K {
*/
read(bytes) {
let i = 0;
this.type = enums.read(enums.s2k, bytes[i++]);
try {
this.type = enums.read(enums.s2k, bytes[i++]);
} catch (err) {
throw new UnsupportedError('Unknown S2K type.');
}
this.algorithm = bytes[i++];

switch (this.type) {
Expand Down Expand Up @@ -98,15 +103,15 @@ class S2K {
this.type = 'gnu-dummy';
// GnuPG extension mode 1001 -- don't write secret key at all
} else {
throw new Error('Unknown s2k gnu protection mode.');
throw new UnsupportedError('Unknown s2k gnu protection mode.');
}
} else {
throw new Error('Unknown s2k type.');
throw new UnsupportedError('Unknown s2k type.');
}
break;

default:
throw new Error('Unknown s2k type.');
throw new UnsupportedError('Unknown s2k type.'); // unreachable
}

return i;
Expand Down
40 changes: 40 additions & 0 deletions test/general/key.js
Expand Up @@ -2972,6 +2972,46 @@ module.exports = () => describe('Key', function() {
expect(key).to.exist;
});

it('Parsing ECDH key with unknown kdf param version', async function() {
// subkey with unknown kdfParam version 255. Parsing should not fail, the subkey should simply dropped
const key = await openpgp.readKey({ armoredKey: `-----BEGIN PGP PRIVATE KEY BLOCK-----

xVgEZAdtGBYJKwYBBAHaRw8BAQdAcNgHyRGEaqGmzEqEwCobfUkyrJnY8faBvsf9
R2c5ZzYAAP9bFL4nPBdo04ei0C2IAh5RXOpmuejGC3GAIn/UmL5cYQ+XzRtjaGFy
bGVzIDxjaGFybGVzQHByb3Rvbi5tZT7CigQTFggAPAUCZAdtGAmQFXJtmBzDhdcW
IQRl2gNflypl1XjRUV8Vcm2YHMOF1wIbAwIeAQIZAQILBwIVCAIWAAIiAQAAJKYA
/2qY16Ozyo5erNz51UrKViEoWbEpwY3XaFVNzrw+b54YAQC7zXkf/t5ieylvjmA/
LJz3/qgH5GxZRYAH9NTpWyW1AsdxBGQHbRgSCisGAQQBl1UBBQEBB0CxmxoJsHTW
TiETWh47ot+kwNA1hCk1IYB9WwKxkXYyIBf/CgmKXzV1ODP/mRmtiBYVV+VQk5MF
EAAA/1NW8D8nMc2ky140sPhQrwkeR7rVLKP2fe5n4BEtAnVQEB3CeAQYFggAKgUC
ZAdtGAmQFXJtmBzDhdcWIQRl2gNflypl1XjRUV8Vcm2YHMOF1wIbUAAAl/8A/iIS
zWBsBR8VnoOVfEE+VQk6YAi7cTSjcMjfsIez9FYtAQDKo9aCMhUohYyqvhZjn8aS
3t9mIZPc+zRJtCHzQYmhDg==
=lESj
-----END PGP PRIVATE KEY BLOCK-----` });

expect(key.subkeys).to.have.length(0);

await expect(openpgp.readKey({
armoredKey: `-----BEGIN PGP PRIVATE KEY BLOCK-----

xVgEZAdtGBYJKwYBBAHaRw8BAQdAcNgHyRGEaqGmzEqEwCobfUkyrJnY8faBvsf9
R2c5ZzYAAP9bFL4nPBdo04ei0C2IAh5RXOpmuejGC3GAIn/UmL5cYQ+XzRtjaGFy
bGVzIDxjaGFybGVzQHByb3Rvbi5tZT7CigQTFggAPAUCZAdtGAmQFXJtmBzDhdcW
IQRl2gNflypl1XjRUV8Vcm2YHMOF1wIbAwIeAQIZAQILBwIVCAIWAAIiAQAAJKYA
/2qY16Ozyo5erNz51UrKViEoWbEpwY3XaFVNzrw+b54YAQC7zXkf/t5ieylvjmA/
LJz3/qgH5GxZRYAH9NTpWyW1AsdxBGQHbRgSCisGAQQBl1UBBQEBB0CxmxoJsHTW
TiETWh47ot+kwNA1hCk1IYB9WwKxkXYyIBf/CgmKXzV1ODP/mRmtiBYVV+VQk5MF
EAAA/1NW8D8nMc2ky140sPhQrwkeR7rVLKP2fe5n4BEtAnVQEB3CeAQYFggAKgUC
ZAdtGAmQFXJtmBzDhdcWIQRl2gNflypl1XjRUV8Vcm2YHMOF1wIbUAAAl/8A/iIS
zWBsBR8VnoOVfEE+VQk6YAi7cTSjcMjfsIez9FYtAQDKo9aCMhUohYyqvhZjn8aS
3t9mIZPc+zRJtCHzQYmhDg==
=lESj
-----END PGP PRIVATE KEY BLOCK-----`,
config: { ignoreUnsupportedPackets: false }
})).to.be.rejectedWith(/Cannot read KDFParams/);
});

it('Testing key ID and fingerprint for V4 keys', async function() {
const pubKeysV4 = await openpgp.readKeys({ armoredKeys: twoKeys });
expect(pubKeysV4).to.exist;
Expand Down
12 changes: 12 additions & 0 deletions test/general/packet.js
Expand Up @@ -1032,6 +1032,18 @@ kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu
).to.be.rejectedWith(/Unknown public key encryption algorithm/);
});

it('Ignores unknown SKESK s2k only with `config.ignoreUnsupportedPackets` enabled', async function() {
const binaryMessage = util.hexToUint8Array('c1c0cc037c2faa4df93c37b2010c009bb74119f098efa43c7924b2effc7d32fc6d7bf7f6952d2cab1722d3192cfb9b90448592770dcbaed4ef377f73a110a7e208a87a74c18fc4088c60bb0f3abcba32551c8b0e69f3505a0717cfd998261f8ffd166a5e029c504ccd58c100abef7be78aef9650df36b9757ae864b20dda598feb9799128d959a525eee6e1dd7a609117cdc922ab98dce5ea89b498005d609e54e4ec8ff330c648c375a1f56618ebd34c15db928775b6d0ec50316796ea384ebc224737ba861e3b0254817d53d0c26b517eba9ba79f56a9cae4d75b34144f752bea81fd4fbbb17fd36c9c2c387ddb23356e928b5ba47ef7164b6e2ccfe80662321add5c23dc162dc09eda77f2f4c4b04c1d59061bf8625d8c9705fc377cc8b9f9e746e62b0b0d990dd20a3ff8478101efc3e4329e66ce2f3e915657bccf94a77a357055c22a68b23cc8f563e0baef17904c488ea885e8d0cff6b27fbf5e609c2334d1c26ea7445b58a3f9182cdbad8cfd540237b4b495a24fb9bf59a96600c547141c22b4e5adc8dfa292719efeca1c3500409170861616161616161614141414161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161d20101');

const parsed = await openpgp.PacketList.fromBinary(binaryMessage, allAllowedPackets, { ...openpgp.config, ignoreUnsupportedPackets: true });
expect(parsed.length).to.equal(3);
expect(parsed[1]).instanceOf(openpgp.UnparseablePacket);
expect(parsed[1].tag).to.equal(openpgp.enums.packet.symEncryptedSessionKey);

await expect(
openpgp.PacketList.fromBinary(binaryMessage, allAllowedPackets, { ...openpgp.config, ignoreUnsupportedPackets: false })
).to.be.rejectedWith(/Unknown S2K type/);
});

it('Throws on disallowed packet even with tolerant mode enabled', async function() {
const packets = new openpgp.PacketList();
Expand Down
98 changes: 98 additions & 0 deletions test/general/signature.js
Expand Up @@ -1642,6 +1642,104 @@ hkJiXopCSWKSlQInL1devkJJUWJmTmZeugJYlpdLAagQJM0JpsCqIQZwKgAA
});
});

it('Should verify detached signature with some unknown versions of Signature packets', async function () {
// Test from openpgp-interoperability-test-suite to ensure forward compatibility: https://tests.sequoia-pgp.org/?q=forward-compat
const plaintext = 'hello world';

// This signature includes two Signature packets: a v4 one (verifiable) and a 'dummy' v23 one.
const signatureUnknownTrailingPacketVersion = `-----BEGIN PGP SIGNATURE-----

wnUEARYKACcFgmSVpTQJkHEwNzxPuQajFiEE2KiARjeh+fU3dy+5cTA3PE+5
BqMAAKZNAP0fhECUqrE2Ts7Ho8/fuLFT+9jsGIGo0EviIEmW77vyhQEAtOBa
N77tTSawgDqnjIRH5RyI6YNC1LNz01VHCYWwegfCwTsXAAEKAG8FgmSVZN4J
EPv8yCoBXnMwRxQAAAAAAB4AIHNhbHRAbm90YXRpb25zLnNlcXVvaWEtcGdw
Lm9yZ8jF+epDaQ8yqg9h1mb0LcDLKC71kHyESC8fqFt9fNFsFiEE0aZuGiOx
gsmYD3iM+/zIKgFeczAAADLxDACKH0qwrZW+Eu3McHHfKojqlHoJ+Ofqotui
Gtcyx3HrE86xQHQl6346Joweomlzo2A6cjhT/nxL88sfy9yTQyUyKaON0wHz
4WI+Onu8rSaG99J/u34dDIPqFu5DzhwCrkv0IQwGYfDxG6Lrxg7gsxui2KAt
4rJqlbaeRGOTeNmew6aH74foUp86LWjdasanZ3RXxjk3yP+R/7nquQjkVGqE
jElkMwFh44TwTHlrXfI90Ki4gNrFQfbQCQm2v66rT0t3BSgVrL+FZIyXjjOh
dp83PCrkcvOcbBalvtbYPd5+23cGAylm5hkC9bxQUwUJrcJezdwSpxF5+Vgj
IkeanKfU2BhKry3Hpn3PL6vLfVkK/w0wUEbDMkFRbGAmW1sPCJWDSX6Zy75/
Li0CQ3u6tg3/m9VHUdwN5iNVk3g7AtV2eLinv4fKIuVUxUIyvacro+RBxGNc
EnZwTO2p2I0xifnoRizITFXclUc9J4vK+whpi9PHH5uoqRGcoer72rtjIIs=
=nReB
-----END PGP SIGNATURE-----`;

const publicKey = `-----BEGIN PGP PUBLIC KEY BLOCK-----

xjMEZJWk4RYJKwYBBAHaRw8BAQdA7p5RuL+Z05qld6xRz6tbJ+9pmDowaCYr
tMOW8MXHAx3NFW5hbWUgPGVtYWlsQHRlc3QuY29tPsKMBBAWCgA+BYJklaTh
BAsJBwgJkHEwNzxPuQajAxUICgQWAAIBAhkBApsDAh4BFiEE2KiARjeh+fU3
dy+5cTA3PE+5BqMAAB5pAQDUHdYs3HRK6yJZ6IrK8lfmLzeqSgW2j9wLG/zF
TXIARQEAj0PdOzSy3q75VIQraDSHWpBAue8QNEKV4Q8hlkJvmgPOOARklaTh
EgorBgEEAZdVAQUBAQdAR9bBkzKzh24TB6gJVHR49BWnhTmeF5+vA3PXtX/b
RHkDAQgHwngEGBYIACoFgmSVpOEJkHEwNzxPuQajApsMFiEE2KiARjeh+fU3
dy+5cTA3PE+5BqMAAFjVAQDKqKwFLKX+N7le3cDLHAYSqc4AWpksKS4eSBLa
uDvEBgD+LCEUOPejUTCMqPyd04ssdOq1AlMJOmUGUwLk7kFP7Aw=
=Q9Px
-----END PGP PUBLIC KEY BLOCK-----`;

const { signatures, data } = await openpgp.verify({
message: await openpgp.createMessage({ text: plaintext }),
signature: await openpgp.readSignature({ armoredSignature: signatureUnknownTrailingPacketVersion }),
verificationKeys: await openpgp.readKey({ armoredKey: publicKey })
});
expect(data).to.equal(plaintext);
expect(signatures).to.have.length(1);
expect(await signatures[0].verified).to.be.true;
expect((await signatures[0].signature).packets.length).to.equal(1);
});

it('Should verify cleartext signature with some unknown versions of Signature packets', async function () {
// Test to ensure forward compatibility:
// this signature includes two Signature packets: a v4 one (verifiable) and a 'dummy' v23 one.
const signatureUnknownTrailingPacketVersion = `-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

hello world
-----BEGIN PGP SIGNATURE-----

wnUEARYKACcFgmSVpTQJkHEwNzxPuQajFiEE2KiARjeh+fU3dy+5cTA3PE+5
BqMAAKZNAP0fhECUqrE2Ts7Ho8/fuLFT+9jsGIGo0EviIEmW77vyhQEAtOBa
N77tTSawgDqnjIRH5RyI6YNC1LNz01VHCYWwegfCwTsXAAEKAG8FgmSVZN4J
EPv8yCoBXnMwRxQAAAAAAB4AIHNhbHRAbm90YXRpb25zLnNlcXVvaWEtcGdw
Lm9yZ8jF+epDaQ8yqg9h1mb0LcDLKC71kHyESC8fqFt9fNFsFiEE0aZuGiOx
gsmYD3iM+/zIKgFeczAAADLxDACKH0qwrZW+Eu3McHHfKojqlHoJ+Ofqotui
Gtcyx3HrE86xQHQl6346Joweomlzo2A6cjhT/nxL88sfy9yTQyUyKaON0wHz
4WI+Onu8rSaG99J/u34dDIPqFu5DzhwCrkv0IQwGYfDxG6Lrxg7gsxui2KAt
4rJqlbaeRGOTeNmew6aH74foUp86LWjdasanZ3RXxjk3yP+R/7nquQjkVGqE
jElkMwFh44TwTHlrXfI90Ki4gNrFQfbQCQm2v66rT0t3BSgVrL+FZIyXjjOh
dp83PCrkcvOcbBalvtbYPd5+23cGAylm5hkC9bxQUwUJrcJezdwSpxF5+Vgj
IkeanKfU2BhKry3Hpn3PL6vLfVkK/w0wUEbDMkFRbGAmW1sPCJWDSX6Zy75/
Li0CQ3u6tg3/m9VHUdwN5iNVk3g7AtV2eLinv4fKIuVUxUIyvacro+RBxGNc
EnZwTO2p2I0xifnoRizITFXclUc9J4vK+whpi9PHH5uoqRGcoer72rtjIIs=
=nReB
-----END PGP SIGNATURE-----`;

const publicKey = `-----BEGIN PGP PUBLIC KEY BLOCK-----

xjMEZJWk4RYJKwYBBAHaRw8BAQdA7p5RuL+Z05qld6xRz6tbJ+9pmDowaCYr
tMOW8MXHAx3NFW5hbWUgPGVtYWlsQHRlc3QuY29tPsKMBBAWCgA+BYJklaTh
BAsJBwgJkHEwNzxPuQajAxUICgQWAAIBAhkBApsDAh4BFiEE2KiARjeh+fU3
dy+5cTA3PE+5BqMAAB5pAQDUHdYs3HRK6yJZ6IrK8lfmLzeqSgW2j9wLG/zF
TXIARQEAj0PdOzSy3q75VIQraDSHWpBAue8QNEKV4Q8hlkJvmgPOOARklaTh
EgorBgEEAZdVAQUBAQdAR9bBkzKzh24TB6gJVHR49BWnhTmeF5+vA3PXtX/b
RHkDAQgHwngEGBYIACoFgmSVpOEJkHEwNzxPuQajApsMFiEE2KiARjeh+fU3
dy+5cTA3PE+5BqMAAFjVAQDKqKwFLKX+N7le3cDLHAYSqc4AWpksKS4eSBLa
uDvEBgD+LCEUOPejUTCMqPyd04ssdOq1AlMJOmUGUwLk7kFP7Aw=
=Q9Px
-----END PGP PUBLIC KEY BLOCK-----`;

const { signatures } = await openpgp.verify({
message: await openpgp.readCleartextMessage({ cleartextMessage: signatureUnknownTrailingPacketVersion }),
verificationKeys: await openpgp.readKey({ armoredKey: publicKey })
});
expect(signatures).to.have.length(1);
expect(await signatures[0].verified).to.be.true;
expect((await signatures[0].signature).packets.length).to.equal(1);
});

it('Should verify cleartext message correctly when using a detached cleartext signature and binary literal data', async function () {
const plaintext = 'short message\nnext line \n한국어/조선말';
const pubKey = await openpgp.readKey({ armoredKey: pub_key_arm2 });
Expand Down