Support parsing encrypted key with unknown s2k types or cipher algos (#1658)

Such keys are still capable of encryption and signature verification.
This change is relevant for forward compatibility of v4 keys encrypted using e.g. argon2.

openpgp.d.ts

@@ -395,6 +395,7 @@ declare abstract class BaseSecretKeyPacket extends BasePublicKeyPacket {
   public decrypt(passphrase: string): Promise<void>; // throws on error
   public validate(): Promise<void>; // throws on error
   public isDummy(): boolean;
+  public isMissingSecretKeyMaterial(): boolean;
   public makeDummy(config?: Config): void;
 }
 

src/packet/secret_key.js

@@ -86,6 +86,7 @@ class SecretKeyPacket extends PublicKeyPacket {
   async read(bytes) {
     // - A Public-Key or Public-Subkey packet, as described above.
     let i = await this.readPublicKey(bytes);
+    const startOfSecretKeyData = i;
 
     // - One octet indicating string-to-key usage conventions.  Zero
     //   indicates that the secret-key data is not encrypted.  255 or 254
@@ -99,40 +100,47 @@ class SecretKeyPacket extends PublicKeyPacket {
       i++;
     }
 
-    // - [Optional] If string-to-key usage octet was 255, 254, or 253, a
+    try {
-    //   one-octet symmetric encryption algorithm.
-    if (this.s2kUsage === 255 || this.s2kUsage === 254 || this.s2kUsage === 253) {
-      this.symmetric = bytes[i++];
-
-      // - [Optional] If string-to-key usage octet was 253, a one-octet
-      //   AEAD algorithm.
-      if (this.s2kUsage === 253) {
-        this.aead = bytes[i++];
-      }
-
       // - [Optional] If string-to-key usage octet was 255, 254, or 253, a
-      //   string-to-key specifier.  The length of the string-to-key
+      //   one-octet symmetric encryption algorithm.
-      //   specifier is implied by its type, as described above.
+      if (this.s2kUsage === 255 || this.s2kUsage === 254 || this.s2kUsage === 253) {
-      this.s2k = new S2K();
+        this.symmetric = bytes[i++];
-      i += this.s2k.read(bytes.subarray(i, bytes.length));
 
-      if (this.s2k.type === 'gnu-dummy') {
+        // - [Optional] If string-to-key usage octet was 253, a one-octet
-        return;
+        //   AEAD algorithm.
+        if (this.s2kUsage === 253) {
+          this.aead = bytes[i++];
+        }
+
+        // - [Optional] If string-to-key usage octet was 255, 254, or 253, a
+        //   string-to-key specifier.  The length of the string-to-key
+        //   specifier is implied by its type, as described above.
+        this.s2k = new S2K();
+        i += this.s2k.read(bytes.subarray(i, bytes.length));
+
+        if (this.s2k.type === 'gnu-dummy') {
+          return;
+        }
+      } else if (this.s2kUsage) {
+        this.symmetric = this.s2kUsage;
       }
-    } else if (this.s2kUsage) {
-      this.symmetric = this.s2kUsage;
-    }
 
-    // - [Optional] If secret data is encrypted (string-to-key usage octet
+      // - [Optional] If secret data is encrypted (string-to-key usage octet
-    //   not zero), an Initial Vector (IV) of the same length as the
+      //   not zero), an Initial Vector (IV) of the same length as the
-    //   cipher's block size.
+      //   cipher's block size.
-    if (this.s2kUsage) {
+      if (this.s2kUsage) {
-      this.iv = bytes.subarray(
+        this.iv = bytes.subarray(
-        i,
+          i,
-        i + crypto.getCipher(this.symmetric).blockSize
+          i + crypto.getCipher(this.symmetric).blockSize
-      );
+        );
 
-      i += this.iv.length;
+        i += this.iv.length;
+      }
+    } catch (e) {
+      // if the s2k is unsupported, we still want to support encrypting and verifying with the given key
+      if (!this.s2kUsage) throw e; // always throw for decrypted keys
+      this.unparseableKeyMaterial = bytes.subarray(startOfSecretKeyData);
+      this.isEncrypted = true;
     }
 
     // - Only for a version 5 packet, a four-octet scalar octet count for
@@ -168,8 +176,15 @@ class SecretKeyPacket extends PublicKeyPacket {
    * @returns {Uint8Array} A string of bytes containing the secret key OpenPGP packet.
    */
   write() {
-    const arr = [this.writePublicKey()];
+    const serializedPublicKey = this.writePublicKey();
+    if (this.unparseableKeyMaterial) {
+      return util.concatUint8Array([
+        serializedPublicKey,
+        this.unparseableKeyMaterial
+      ]);
+    }
 
+    const arr = [serializedPublicKey];
     arr.push(new Uint8Array([this.s2kUsage]));
 
     const optionalFieldsArr = [];
@@ -229,6 +244,18 @@ class SecretKeyPacket extends PublicKeyPacket {
     return this.isEncrypted === false;
   }
 
+  /**
+   * Check whether the key includes secret key material.
+   * Some secret keys do not include it, and can thus only be used
+   * for public-key operations (encryption and verification).
+   * Such keys are:
+   * - GNU-dummy keys, where the secret material has been stripped away
+   * - encrypted keys with unsupported S2K or cipher
+   */
+  isMissingSecretKeyMaterial() {
+    return this.unparseableKeyMaterial !== undefined || this.isDummy();
+  }
+
   /**
    * Check whether this is a gnu-dummy key
    * @returns {Boolean}
@@ -249,6 +276,7 @@ class SecretKeyPacket extends PublicKeyPacket {
     if (this.isDecrypted()) {
       this.clearPrivateParams();
     }
+    delete this.unparseableKeyMaterial;
     this.isEncrypted = null;
     this.keyMaterial = null;
     this.s2k = new S2K(config);
@@ -320,6 +348,10 @@ class SecretKeyPacket extends PublicKeyPacket {
       return false;
     }
 
+    if (this.unparseableKeyMaterial) {
+      throw new Error('Key packet cannot be decrypted: unsupported S2K or cipher algo');
+    }
+
     if (this.isDecrypted()) {
       throw new Error('Key packet is already decrypted.');
     }
@@ -404,7 +436,7 @@ class SecretKeyPacket extends PublicKeyPacket {
    * Clear private key parameters
    */
   clearPrivateParams() {
-    if (this.isDummy()) {
+    if (this.isMissingSecretKeyMaterial()) {
       return;
     }
 

test/general/key.js

@@ -2217,6 +2217,25 @@ UGHMDD0RTiyoiQjvVdCRq3YDQtu38TdIKUurvfjeDjLBfuF1RmED9lCRREqRGwKU
 =kUWS
 -----END PGP PUBLIC KEY BLOCK-----`;
 
+// key encrypted with invalid s2kType = 23, to test that it can still be used for encryption/verification
+const encryptedKeyUnknownS2K = `-----BEGIN PGP PRIVATE KEY BLOCK-----
+
+xYYEZJ2H3RYJKwYBBAHaRw8BAQdA3V39Xv0+436Rpn/2UlcnOC1BGprmAlWY
+RBKjAq0hAtD+CRcIdHzwqoLa54cAbBOEIgBh7Xa1Qh5wCGAmEVWnAldaqvk+
+NcvUL2bR6AQsGIT6YEihOS3xLKobMOd2XlO5ItQoWnONzkWgzjFvctgnlhmq
+I80AwowEEBYKAD4FgmSdh90ECwkHCAmQaBT7gxSTsXwDFQgKBBYAAgECGQEC
+mwMCHgEWIQSvRnJTQT6TtdZFk0NoFPuDFJOxfAAAT7kBALmmUEJt5HMAOWiW
+7/8y4wllm8zNQ9vbl5Q0cWbeWj/8AP9HDa2rRxHY/37g5zXdmL9f/qNWr9Fk
+EBRhLLwusumuDMeLBGSdh90SCisGAQQBl1UBBQEBB0Am2yjjialeIVXHJJ2P
+b7KiapCC0mD95F0EFz6zz0l4DgMBCAf+CRcISMdt0OUFCNUABB/OD0UW7MPK
+Y3t8RrUTYoiCuhuPRDLOJ5NnMNagVQLt3jQsI8JRjzmYbiTrA/V3iJIEDu5C
+NWbnvCM7Hs7+OqPzJPJ2w8J4BBgWCAAqBYJknYfdCZBoFPuDFJOxfAKbDBYh
+BK9GclNBPpO11kWTQ2gU+4MUk7F8AADwfwD8CsOVw/3zm1UwUbGUi+fuf6Pr
+VFBLG8uc9IiaKann/DYBAJcZNZHRSfpDoV2pUA5EAEi2MdjxkRysFQnYPRAu
+0pYO
+=rWL8
+-----END PGP PRIVATE KEY BLOCK-----`;
+
 function versionSpecificTests() {
   it('Preferences of generated key', function() {
     const testPref = function(key) {
@@ -2928,6 +2947,17 @@ module.exports = () => describe('Key', function() {
     expect(primaryUser).to.exist;
   });
 
+  it('Parsing and serialization of encrypted key with unknown S2K type (unparseableKeyMaterial)', async function() {
+    const key = await openpgp.readKey({ armoredKey: encryptedKeyUnknownS2K });
+    expect(key.isPrivate()).to.equal(true);
+    expect(key.isDecrypted()).to.equal(false);
+    expect(key.getSubkeys()).to.have.length(1);
+    expect(key.keyPacket.isMissingSecretKeyMaterial()).to.equal(true);
+
+    const expectedSerializedKey = await openpgp.unarmor(encryptedKeyUnknownS2K);
+    expect(key.write()).to.deep.equal(expectedSerializedKey.data);
+  });
+
   it('Parsing V5 public key packet', async function() {
     // Manually modified from https://gitlab.com/openpgp-wg/rfc4880bis/blob/00b2092/back.mkd#sample-eddsa-key
     const packetBytes = util.hexToUint8Array(`
@@ -3218,6 +3248,14 @@ module.exports = () => describe('Key', function() {
     await openpgp.readKey({ armoredKey: decryptedKey.armor() });
   });
 
+  it('makeDummy() - should work for encrypted keys with unknown s2k (unparseableKeyMaterial)', async function() {
+    const key = await openpgp.readKey({ armoredKey: encryptedKeyUnknownS2K });
+    expect(key.keyPacket.isDummy()).to.be.false;
+    expect(key.keyPacket.makeDummy()).to.not.throw;
+    expect(key.keyPacket.isDummy()).to.be.true;
+    expect(key.keyPacket.unparseableKeyMaterial).to.not.exist;
+  });
+
   it('clearPrivateParams() - check that private key can no longer be used', async function() {
     const key = await openpgp.decryptKey({
       privateKey: await openpgp.readKey({ armoredKey: priv_key_rsa }),

test/general/openpgp.js

@@ -1154,6 +1154,31 @@ module.exports = () => describe('OpenPGP.js public api tests', function() {
         expect(privateKeyMismatchingParams).to.deep.equal(originalKey);
       });
     });
+
+    it('should fail for encrypted key with unknown s2k (unparseableKeyMaterial)', async function() {
+      // key encrypted with invalid s2kType = 23, to test that it can still be used for encryption/verification
+      const encryptedKeyUnknownS2K = await openpgp.readKey({ armoredKey: `-----BEGIN PGP PRIVATE KEY BLOCK-----
+
+xYYEZJ2H3RYJKwYBBAHaRw8BAQdA3V39Xv0+436Rpn/2UlcnOC1BGprmAlWY
+RBKjAq0hAtD+CRcIdHzwqoLa54cAbBOEIgBh7Xa1Qh5wCGAmEVWnAldaqvk+
+NcvUL2bR6AQsGIT6YEihOS3xLKobMOd2XlO5ItQoWnONzkWgzjFvctgnlhmq
+I80AwowEEBYKAD4FgmSdh90ECwkHCAmQaBT7gxSTsXwDFQgKBBYAAgECGQEC
+mwMCHgEWIQSvRnJTQT6TtdZFk0NoFPuDFJOxfAAAT7kBALmmUEJt5HMAOWiW
+7/8y4wllm8zNQ9vbl5Q0cWbeWj/8AP9HDa2rRxHY/37g5zXdmL9f/qNWr9Fk
+EBRhLLwusumuDMeLBGSdh90SCisGAQQBl1UBBQEBB0Am2yjjialeIVXHJJ2P
+b7KiapCC0mD95F0EFz6zz0l4DgMBCAf+CRcISMdt0OUFCNUABB/OD0UW7MPK
+Y3t8RrUTYoiCuhuPRDLOJ5NnMNagVQLt3jQsI8JRjzmYbiTrA/V3iJIEDu5C
+NWbnvCM7Hs7+OqPzJPJ2w8J4BBgWCAAqBYJknYfdCZBoFPuDFJOxfAKbDBYh
+BK9GclNBPpO11kWTQ2gU+4MUk7F8AADwfwD8CsOVw/3zm1UwUbGUi+fuf6Pr
+VFBLG8uc9IiaKann/DYBAJcZNZHRSfpDoV2pUA5EAEi2MdjxkRysFQnYPRAu
+0pYO
+=rWL8
+-----END PGP PRIVATE KEY BLOCK-----` });
+      await expect(openpgp.decryptKey({
+        privateKey: encryptedKeyUnknownS2K,
+        passphrase: 'test'
+      })).to.be.rejectedWith(/Key packet cannot be decrypted: unsupported S2K or cipher algo/);
+    });
   });
 
   describe('encryptKey - unit tests', function() {
@@ -1992,6 +2017,53 @@ aOU=
       const { data: streamedData } = await openpgp.decrypt({ message: objectMessage, passwords, verificationKeys: privateKey, expectSigned: true, config });
       expect(await stream.readToEnd(streamedData)).to.equal(text);
     });
+
+    it('should support encrypting with encrypted key with unknown s2k (unparseableKeyMaterial)', async function() {
+      const originalDecryptedKey = await openpgp.readKey({ armoredKey: `-----BEGIN PGP PRIVATE KEY BLOCK-----
+
+xVgEZJ2H3RYJKwYBBAHaRw8BAQdA3V39Xv0+436Rpn/2UlcnOC1BGprmAlWY
+RBKjAq0hAtAAAQCykslk/kEP7+O9sOsuvgX2Xfz4peQuNo2vD/w4dMZpchKj
+zQDCjAQQFgoAPgWCZJ2H3QQLCQcICZBoFPuDFJOxfAMVCAoEFgACAQIZAQKb
+AwIeARYhBK9GclNBPpO11kWTQ2gU+4MUk7F8AABPuQEAuaZQQm3kcwA5aJbv
+/zLjCWWbzM1D29uXlDRxZt5aP/wA/0cNratHEdj/fuDnNd2Yv1/+o1av0WQQ
+FGEsvC6y6a4Mx10EZJ2H3RIKKwYBBAGXVQEFAQEHQCbbKOOJqV4hVccknY9v
+sqJqkILSYP3kXQQXPrPPSXgOAwEIBwAA/34s+u8hyLdzdLxjrEEN9zNb+C8d
+EyBNxMpyZ/NJsUxoEIPCeAQYFggAKgWCZJ2H3QmQaBT7gxSTsXwCmwwWIQSv
+RnJTQT6TtdZFk0NoFPuDFJOxfAAA8H8A/ArDlcP985tVMFGxlIvn7n+j61RQ
+SxvLnPSImimp5/w2AQCXGTWR0Un6Q6FdqVAORABItjHY8ZEcrBUJ2D0QLtKW
+Dg==
+=wiwz
+-----END PGP PRIVATE KEY BLOCK-----` });
+      // `originalDecryptedKey` encrypted with invalid s2kType = 23, to test that it can still be used for encryption/verification
+      const encryptedKeyUnknownS2K = await openpgp.readKey({ armoredKey: `-----BEGIN PGP PRIVATE KEY BLOCK-----
+
+xYYEZJ2H3RYJKwYBBAHaRw8BAQdA3V39Xv0+436Rpn/2UlcnOC1BGprmAlWY
+RBKjAq0hAtD+CRcIdHzwqoLa54cAbBOEIgBh7Xa1Qh5wCGAmEVWnAldaqvk+
+NcvUL2bR6AQsGIT6YEihOS3xLKobMOd2XlO5ItQoWnONzkWgzjFvctgnlhmq
+I80AwowEEBYKAD4FgmSdh90ECwkHCAmQaBT7gxSTsXwDFQgKBBYAAgECGQEC
+mwMCHgEWIQSvRnJTQT6TtdZFk0NoFPuDFJOxfAAAT7kBALmmUEJt5HMAOWiW
+7/8y4wllm8zNQ9vbl5Q0cWbeWj/8AP9HDa2rRxHY/37g5zXdmL9f/qNWr9Fk
+EBRhLLwusumuDMeLBGSdh90SCisGAQQBl1UBBQEBB0Am2yjjialeIVXHJJ2P
+b7KiapCC0mD95F0EFz6zz0l4DgMBCAf+CRcISMdt0OUFCNUABB/OD0UW7MPK
+Y3t8RrUTYoiCuhuPRDLOJ5NnMNagVQLt3jQsI8JRjzmYbiTrA/V3iJIEDu5C
+NWbnvCM7Hs7+OqPzJPJ2w8J4BBgWCAAqBYJknYfdCZBoFPuDFJOxfAKbDBYh
+BK9GclNBPpO11kWTQ2gU+4MUk7F8AADwfwD8CsOVw/3zm1UwUbGUi+fuf6Pr
+VFBLG8uc9IiaKann/DYBAJcZNZHRSfpDoV2pUA5EAEi2MdjxkRysFQnYPRAu
+0pYO
+=rWL8
+-----END PGP PRIVATE KEY BLOCK-----` });
+      const encrypted = await openpgp.encrypt({
+        message: await openpgp.createMessage({ text: 'test' }),
+        encryptionKeys: encryptedKeyUnknownS2K
+      });
+
+      // decrypt with original key
+      const decrypted = await openpgp.decrypt({
+        message: await openpgp.readMessage({ armoredMessage: encrypted }),
+        decryptionKeys: originalDecryptedKey
+      });
+      expect(decrypted.data).to.equal('test');
+    });
   });
 
   describe('encryptSessionKey - unit tests', function() {