Merge pull request #1656

Fix forward compatibility of keys, SKESKs, and detached/cleartext signatures and ECDH. Relevant for forward compatibility when: - verifying **detached** or cleartext signatures that include e.g. v4 and v6 packets. Non-detached signatures are not affected. - parsing messages with sym. encrypted session keys (SKESK) that rely on unsupported S2K types (e.g. argon2) - parsing keys that include ECDH subkeys with unknown KDF params version (e.g. end-to-end encrypted forwarding use-case)
2023-07-10 19:38:39 +02:00 · 2023-07-10 19:38:39 +02:00 · b6170aa40d
commit b6170aa40d
parent d72cece54a 32caf41929
7 changed files with 164 additions and 7 deletions
--- a/src/cleartext.js
+++ b/src/cleartext.js
@ -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);
--- a/src/message.js
+++ b/src/message.js
@ -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);
  }

--- a/src/type/kdf_params.js
+++ b/src/type/kdf_params.js
@ -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
 *
@ -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];
--- a/src/type/s2k.js
+++ b/src/type/s2k.js
@ -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 {
@ -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) {
@ -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;
--- a/test/general/key.js
+++ b/test/general/key.js
@ -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;
--- a/test/general/packet.js
+++ b/test/general/packet.js
@ -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();
--- a/test/general/signature.js
+++ b/test/general/signature.js
@ -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 });