Add config.parseAEADEncryptedV4KeysAsLegacy to support AEAD-encrypted v4 keys from OpenPGP.js v5 or older (#1672)

The config option must be set when reading v4 private keys (e.g. those
generated in OpenPGP.js by default, without setting `config.v5Keys = true`)
which were encrypted by OpenPGP.js v5 (or older) using `config.aeadProtect = true`.
Otherwise, key parsing and/or key decryption will fail.

Additional context: OpenPGP.js up to v5 used to support encrypting v4 keys
using AEAD as specified by draft RFC4880bis
(https://www.ietf.org/archive/id/draft-ietf-openpgp-rfc4880bis-10.html#section-5.5.3-3.5).
Said AEAD mechanism was not standardized as-is, and it's been replaced in the
crypto-refresh with a new version that guarantees full key integrity on decryption.
The legacy AEAD format is incompatible, but fundamentally indistinguishable,
from that of the crypto-refresh for v4 keys. Thus, we rely on the caller to
instruct us to process the key as legacy, via the new config flag.

Co-authored-by: Daniel Huigens <d.huigens@protonmail.com>

src/config/config.js

@@ -55,6 +55,14 @@ export default {
    * @property {Boolean} aeadProtect
    */
   aeadProtect: false,
+  /**
+   * When reading OpenPGP v4 private keys (e.g. those generated in OpenPGP.js when not setting `config.v5Keys = true`)
+   * which were encrypted by OpenPGP.js v5 (or older) using `config.aeadProtect = true`,
+   * this option must be set, otherwise key parsing and/or key decryption will fail.
+   * Note: only set this flag if you know that the keys are of the legacy type, as non-legacy keys
+   * will be processed incorrectly.
+   */
+  parseAEADEncryptedV4KeysAsLegacy: false,
   /**
    * Default Authenticated Encryption with Additional Data (AEAD) encryption mode
    * Only has an effect when aeadProtect is set to true.

src/packet/secret_key.js

@@ -69,6 +69,15 @@ class SecretKeyPacket extends PublicKeyPacket {
      * @type {enums.aead}
      */
     this.aead = null;
+    /**
+     * Whether the key is encrypted using the legacy AEAD format proposal from RFC4880bis
+     * (i.e. it was encrypted with the flag `config.aeadProtect` in OpenPGP.js v5 or older).
+     * This value is only relevant to know how to decrypt the key:
+     * if AEAD is enabled, a v4 key is always re-encrypted using the standard AEAD mechanism.
+     * @type {Boolean}
+     * @private
+     */
+    this.isLegacyAEAD = null;
     /**
      * Decrypted private parameters, referenced by name
      * @type {Object}
@@ -84,7 +93,7 @@ class SecretKeyPacket extends PublicKeyPacket {
    * @param {Uint8Array} bytes - Input string to read the packet from
    * @async
    */
-  async read(bytes) {
+  async read(bytes, config = defaultConfig) {
     // - A Public-Key or Public-Subkey packet, as described above.
     let i = await this.readPublicKey(bytes);
     const startOfSecretKeyData = i;
@@ -143,12 +152,19 @@ class SecretKeyPacket extends PublicKeyPacket {
 
 
       if (this.s2kUsage) {
+        // OpenPGP.js up to v5 used to support encrypting v4 keys using AEAD as specified by draft RFC4880bis (https://www.ietf.org/archive/id/draft-ietf-openpgp-rfc4880bis-10.html#section-5.5.3-3.5).
+        // This legacy format is incompatible, but fundamentally indistinguishable, from that of the crypto-refresh for v4 keys (v5 keys are always in legacy format).
+        // While parsing the key may succeed (if IV and AES block sizes match), key decryption will always
+        // fail if the key was parsed according to the wrong format, since the keys are processed differently.
+        // Thus, for v4 keys, we rely on the caller to instruct us to process the key as legacy, via config flag.
+        this.isLegacyAEAD = this.s2kUsage === 253 && (
+          this.version === 5 || (this.version === 4 && config.parseAEADEncryptedV4KeysAsLegacy));
         // - crypto-refresh: If string-to-key usage octet was 255, 254 [..], an initialization vector (IV)
         // of the same length as the cipher's block size.
         // - RFC4880bis (v5 keys, regardless of AEAD): an Initial Vector (IV) of the same length as the
         //   cipher's block size. If string-to-key usage octet was 253 the IV is used as the nonce for the AEAD algorithm.
         // If the AEAD algorithm requires a shorter nonce, the high-order bits of the IV are used and the remaining bits MUST be zero
-        if (this.s2kUsage !== 253 || this.version === 5) {
+        if (this.s2kUsage !== 253 || this.isLegacyAEAD) {
           this.iv = bytes.subarray(
             i,
             i + crypto.getCipher(this.symmetric).blockSize
@@ -330,6 +346,7 @@ class SecretKeyPacket extends PublicKeyPacket {
     this.s2k.type = 'gnu-dummy';
     this.s2kUsage = 254;
     this.symmetric = enums.symmetric.aes256;
+    this.isLegacyAEAD = null;
   }
 
   /**
@@ -366,13 +383,14 @@ class SecretKeyPacket extends PublicKeyPacket {
       this.s2kUsage = 253;
       this.aead = enums.aead.eax;
       const mode = crypto.getAEADMode(this.aead);
+      this.isLegacyAEAD = this.version === 5; // v4 is always re-encrypted with standard format instead.
 
       const serializedPacketTag = writeTag(this.constructor.tag);
       const key = await produceEncryptionKey(this.version, this.s2k, passphrase, this.symmetric, this.aead, serializedPacketTag);
 
       const modeInstance = await mode(this.symmetric, key);
-      this.iv = (this.version === 5) ? crypto.random.getRandomBytes(blockSize) : crypto.random.getRandomBytes(mode.ivLength);
+      this.iv = this.isLegacyAEAD ? crypto.random.getRandomBytes(blockSize) : crypto.random.getRandomBytes(mode.ivLength);
-      const associateData = this.version === 5 ?
+      const associateData = this.isLegacyAEAD ?
         new Uint8Array() :
         util.concatUint8Array([serializedPacketTag, this.writePublicKey()]);
 
@@ -414,7 +432,7 @@ class SecretKeyPacket extends PublicKeyPacket {
     const serializedPacketTag = writeTag(this.constructor.tag); // relevant for AEAD only
     if (this.s2kUsage === 254 || this.s2kUsage === 253) {
       key = await produceEncryptionKey(
-        this.version, this.s2k, passphrase, this.symmetric, this.aead, serializedPacketTag);
+        this.version, this.s2k, passphrase, this.symmetric, this.aead, serializedPacketTag, this.isLegacyAEAD);
     } else if (this.s2kUsage === 255) {
       throw new Error('Encrypted private key is authenticated using an insecure two-byte hash');
     } else {
@@ -426,7 +444,7 @@ class SecretKeyPacket extends PublicKeyPacket {
       const mode = crypto.getAEADMode(this.aead);
       const modeInstance = await mode(this.symmetric, key);
       try {
-        const associateData = this.version === 5 ?
+        const associateData = this.isLegacyAEAD ?
           new Uint8Array() :
           util.concatUint8Array([serializedPacketTag, this.writePublicKey()]);
         cleartext = await modeInstance.decrypt(this.keyMaterial, this.iv.subarray(0, mode.ivLength), associateData);
@@ -458,6 +476,7 @@ class SecretKeyPacket extends PublicKeyPacket {
     this.s2kUsage = 0;
     this.aead = null;
     this.symmetric = null;
+    this.isLegacyAEAD = null;
   }
 
   /**
@@ -526,13 +545,14 @@ class SecretKeyPacket extends PublicKeyPacket {
  * @param {String} passphrase
  * @param {module:enums.symmetric} cipherAlgo
  * @param {module:enums.aead} [aeadMode] - for AEAD-encrypted keys only (excluding v5)
- * @param {Uint8Array} serializedPacketTag - for AEAD-encrypted keys only (excluding v5)
+ * @param {Uint8Array} [serializedPacketTag] - for AEAD-encrypted keys only (excluding v5)
+ * @param {Boolean} [isLegacyAEAD] - for AEAD-encrypted keys from RFC4880bis (v4 and v5 only)
  * @returns encryption key
  */
-async function produceEncryptionKey(keyVersion, s2k, passphrase, cipherAlgo, aeadMode, serializedPacketTag) {
+async function produceEncryptionKey(keyVersion, s2k, passphrase, cipherAlgo, aeadMode, serializedPacketTag, isLegacyAEAD) {
   const { keySize } = crypto.getCipher(cipherAlgo);
   const derivedKey = await s2k.produceKey(passphrase, keySize);
-  if (!aeadMode || keyVersion === 5) {
+  if (!aeadMode || keyVersion === 5 || isLegacyAEAD) {
     return derivedKey;
   }
   const info = util.concatUint8Array([

test/general/key.js

@@ -10,7 +10,6 @@ import util from '../../src/util.js';
 import { getPreferredCipherSuite } from '../../src/key';
 import KeyID from '../../src/type/keyid.js';
 
-
 const priv_key_arm2 =
   ['-----BEGIN PGP PRIVATE KEY BLOCK-----',
     'Version: GnuPG v2.0.19 (GNU/Linux)',
@@ -3084,6 +3083,49 @@ T/efFOC6BDkAAHcjAPwIPNHnR9bKmkVop6cE05dCIpZ/W8zXDGnjKYrrC4Hb
     expect(redecryptedKey.write()).to.deep.equal(decryptedKey.write());
   });
 
+  it('Parsing, decrypting, encrypting and serializing V4 key (AEAD-encrypted, deprecated/legacy format from RFC4880bis)', async function() {
+    // v4 key from OpenPGP.js v5, generated with config.aeadProtect flag (https://www.ietf.org/archive/id/draft-ietf-openpgp-rfc4880bis-10.html#section-5.5.3-3.5)
+    const armoredKey = `-----BEGIN PGP PRIVATE KEY BLOCK-----
+
+xYMEZPXfehYJKwYBBAHaRw8BAQdAw/62MUaSzRSY5gR18DOlfeo/m8eKUkbr
+ZSRqS4wtss39CQEDCGHd70SYYYPkAAALGg1YptTjEuIk4wiBreDE9U/urf3J
+6Z8fP3oy+plzBRKs+8k+kzXY/643Ayesfy3qXA4zM6fqNrrlS6AqT8wDys0A
+wpAEEBYKAEIFgmT133oECwkHCAmQB4Z/qOo0isgDFQgKBBYAAgECGQECmwMC
+HgMWIQQm7YhFQvi0bx7ixrQHhn+o6jSKyAMiAQIAADQZAP9kECruRBva4izE
+9ZET1iQ6i1HiisUKrO6miHfjsxDycgD9EXvtbpi1AORIrYO/pPpGUHpUt25n
+JM+rgWhJwOHw1g3HiARk9d96EgorBgEEAZdVAQUBAQdAiVNiLZRC9wZG9/ef
+V9eu8VKEoHqAFjci3Lu2N+8hQQoDAQgH/QkBAwh+kYDhbTGARwBZRY0lR39D
+EriFZ1N5PKW1TAdxTMNecP3sOyUWRutHQgRrxuF0512fCnelgr2a3of5bQHC
+0XWFfbac2d91VEP2mqrCeAQYFggAKgWCZPXfegmQB4Z/qOo0isgCmwwWIQQm
+7YhFQvi0bx7ixrQHhn+o6jSKyAAAkN4A/31Hm3vy7FHFGJh+VYdqmeESo7mr
+18jzxSbxd71FGTw7AQDqfERTB7zZzk1EqNSAqfrg3hbI7+4XXgHz6qnA3vFm
+Cg==
+=mTGB
+-----END PGP PRIVATE KEY BLOCK-----`;
+    const binaryKey = (await openpgp.unarmor(armoredKey)).data;
+    const passphrase = 'passphrase';
+    const encryptedKey = await openpgp.readKey({ armoredKey, config: { parseAEADEncryptedV4KeysAsLegacy: true } });
+    expect(encryptedKey.keyPacket.isLegacyAEAD).to.be.true;
+    expect(encryptedKey.write()).to.deep.equal(binaryKey);
+
+    const decryptedKey = await openpgp.decryptKey({
+      privateKey: encryptedKey,
+      passphrase
+    });
+    const reecryptedKey = await openpgp.encryptKey({
+      privateKey: decryptedKey,
+      passphrase,
+      config: { aeadProtect: true }
+    });
+    expect(reecryptedKey.keyPacket.s2kUsage).to.equal(253);
+    expect(reecryptedKey.keyPacket.isLegacyAEAD).to.be.false;
+    const redecryptedKey = await openpgp.decryptKey({
+      privateKey: reecryptedKey,
+      passphrase
+    });
+    expect(redecryptedKey.write()).to.deep.equal(decryptedKey.write());
+  });
+
   it('Parsing, decrypting, encrypting and serializing V4 key (AEAD-encrypted)', async function() {
     // key from gopenpgp
     const armoredKey = `-----BEGIN PGP PRIVATE KEY BLOCK-----