Skip key validation for keys encrypted with non-legacy AEAD mechanism (#1713)

The public key material integrity is guaranteed by the new encryption mechanism,
hence `.validate()` does not need to run further checks.

src/packet/secret_key.js

@@ -83,6 +83,13 @@ class SecretKeyPacket extends PublicKeyPacket {
      * @type {Object}
      */
     this.privateParams = null;
+    /**
+     * `true` for keys whose integrity is already confirmed, based on
+     * the AEAD encryption mechanism
+     * @type {Boolean}
+     * @private
+     */
+    this.usedModernAEAD = null;
   }
 
   // 5.5.3.  Secret-Key Packet Formats
@@ -169,6 +176,7 @@ class SecretKeyPacket extends PublicKeyPacket {
             i,
             i + crypto.getCipher(this.symmetric).blockSize
           );
+          this.usedModernAEAD = false;
         } else {
           // crypto-refresh: If string-to-key usage octet was 253 (that is, the secret data is AEAD-encrypted),
           // an initialization vector (IV) of size specified by the AEAD algorithm (see Section 5.13.2), which
@@ -177,6 +185,8 @@ class SecretKeyPacket extends PublicKeyPacket {
             i,
             i + crypto.getAEADMode(this.aead).ivLength
           );
+          // the non-legacy AEAD encryption mechanism also authenticates public key params; no need for manual validation.
+          this.usedModernAEAD = true;
         }
 
         i += this.iv.length;
@@ -347,6 +357,7 @@ class SecretKeyPacket extends PublicKeyPacket {
     this.s2kUsage = 254;
     this.symmetric = enums.symmetric.aes256;
     this.isLegacyAEAD = null;
+    this.usedModernAEAD = null;
   }
 
   /**
@@ -384,6 +395,7 @@ class SecretKeyPacket extends PublicKeyPacket {
       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.
+      this.usedModernAEAD = !this.isLegacyAEAD; // legacy AEAD does not guarantee integrity of public key material
 
       const serializedPacketTag = writeTag(this.constructor.tag);
       const key = await produceEncryptionKey(this.version, this.s2k, passphrase, this.symmetric, this.aead, serializedPacketTag);
@@ -397,6 +409,7 @@ class SecretKeyPacket extends PublicKeyPacket {
       this.keyMaterial = await modeInstance.encrypt(cleartext, this.iv.subarray(0, mode.ivLength), associateData);
     } else {
       this.s2kUsage = 254;
+      this.usedModernAEAD = false;
       const key = await produceEncryptionKey(this.version, this.s2k, passphrase, this.symmetric);
       this.iv = crypto.random.getRandomBytes(blockSize);
       this.keyMaterial = await crypto.mode.cfb.encrypt(this.symmetric, key, util.concatUint8Array([
@@ -493,6 +506,11 @@ class SecretKeyPacket extends PublicKeyPacket {
       throw new Error('Key is not decrypted');
     }
 
+    if (this.usedModernAEAD) {
+      // key integrity confirmed by successful AEAD decryption
+      return;
+    }
+
     let validParams;
     try {
       // this can throw if some parameters are undefined

test/general/key.js

@@ -3106,6 +3106,7 @@ Cg==
     const passphrase = 'passphrase';
     const encryptedKey = await openpgp.readKey({ armoredKey, config: { parseAEADEncryptedV4KeysAsLegacy: true } });
     expect(encryptedKey.keyPacket.isLegacyAEAD).to.be.true;
+    expect(encryptedKey.keyPacket.usedModernAEAD).to.be.false; // legacy AEAD does not guarantee integrity of public key material
     expect(encryptedKey.write()).to.deep.equal(binaryKey);
 
     const decryptedKey = await openpgp.decryptKey({
@@ -3578,6 +3579,69 @@ PzIEeL7UH3trraFmi+Gq8u4kAA==
     await expect(key.validate()).to.be.rejectedWith('Key is invalid');
   });
 
+  it('validate() - should skip for AEAD-encrypted key (non-legacy)', async function() {
+    const v4KeyWithAEAD = await openpgp.readKey({ armoredKey: `-----BEGIN PGP PRIVATE KEY BLOCK-----
+
+xYMEZaAFFxYJKwYBBAHaRw8BAQdA/C3ybUC91HiWAGd/KtPtjmYwWk7VmB+X
+GXhQY9yyZkf9CQEDCBpqDPXSr+aPAGoAsXz/V2uY7EE0Xlutx3MVMEbqWz6m
+fRRUn3/mtGr+PCdj9bbl0rVK+fR62kTbwATUpNdL4rrt1cNgOTlDtq1ZCc0P
+PGFlYWRAdGVzdC5jb20+wpsEEBYKAE0FgmWgBRcDCwkHCZB2ZM+malVbdgUV
+CAoMDgQWAAIBAhkBApsDAh4JFiEE8e8z3IJRZ+bZze8ldmTPpmpVW3YNJwkD
+BwMJAQcBCQIHAgAAplYBALLKSm4Q0dPoX4mBgEuOMgtAEewfyUhp8MJdJvCa
+9KIMAP9f3Qxf4ykXQwgL/e1pn1nNQgiQ7x33LXQ2vHynPkOyDceIBGWgBRcS
+CisGAQQBl1UBBQEBB0Bfk/JMj2ONL/1hi31q3OePnDHLmo8IsafeG0RZY8wF
+OgMBCAf9CQEDCHS8bQidEDvkAP6LovPpHodzcF7F+zbKlJKTI3l6xK4t2Dj6
+BIgBQov9zJ4OK2xFbyraXSLqStQJOQV7XBfIYKHYdIBtxj6cfTYtBMJ4BBgW
+CgAqBYJloAUXCZB2ZM+malVbdgKbDBYhBPHvM9yCUWfm2c3vJXZkz6ZqVVt2
+AAA+gQD9EpMMjlBFvvyACsKwQRmIqUNTfCy4uHL1Ee1fJ4ur9ZQBAP2CiOSN
+CNa5yq6lyexhsn2Vs8DsX+SOSUyNJiy5FyIJ
+-----END PGP PRIVATE KEY BLOCK-----` });
+    const passphrase = 'passphrase';
+    // sanity checks about key encryption mechanism
+    expect(v4KeyWithAEAD.keyPacket.aead).to.not.be.null;
+    expect(v4KeyWithAEAD.keyPacket.isLegacyAEAD).to.be.false;
+    expect(v4KeyWithAEAD.keyPacket.usedModernAEAD).to.be.true;
+
+    const decryptedKey = await openpgp.decryptKey({ privateKey: v4KeyWithAEAD, passphrase });
+    decryptedKey.keyPacket.privateParams.seed = new Uint8Array(1); // corrupt key to confirm that the actual validation is skipped
+    await expect(decryptedKey.validate()).to.be.fulfilled;
+  });
+
+  it('validate() - should be run if AEAD-encrypted key gets re-encrypted without AEAD', async function() {
+    const v4KeyWithAEAD = await openpgp.readKey({ armoredKey: `-----BEGIN PGP PRIVATE KEY BLOCK-----
+
+xYMEZaAFFxYJKwYBBAHaRw8BAQdA/C3ybUC91HiWAGd/KtPtjmYwWk7VmB+X
+GXhQY9yyZkf9CQEDCBpqDPXSr+aPAGoAsXz/V2uY7EE0Xlutx3MVMEbqWz6m
+fRRUn3/mtGr+PCdj9bbl0rVK+fR62kTbwATUpNdL4rrt1cNgOTlDtq1ZCc0P
+PGFlYWRAdGVzdC5jb20+wpsEEBYKAE0FgmWgBRcDCwkHCZB2ZM+malVbdgUV
+CAoMDgQWAAIBAhkBApsDAh4JFiEE8e8z3IJRZ+bZze8ldmTPpmpVW3YNJwkD
+BwMJAQcBCQIHAgAAplYBALLKSm4Q0dPoX4mBgEuOMgtAEewfyUhp8MJdJvCa
+9KIMAP9f3Qxf4ykXQwgL/e1pn1nNQgiQ7x33LXQ2vHynPkOyDceIBGWgBRcS
+CisGAQQBl1UBBQEBB0Bfk/JMj2ONL/1hi31q3OePnDHLmo8IsafeG0RZY8wF
+OgMBCAf9CQEDCHS8bQidEDvkAP6LovPpHodzcF7F+zbKlJKTI3l6xK4t2Dj6
+BIgBQov9zJ4OK2xFbyraXSLqStQJOQV7XBfIYKHYdIBtxj6cfTYtBMJ4BBgW
+CgAqBYJloAUXCZB2ZM+malVbdgKbDBYhBPHvM9yCUWfm2c3vJXZkz6ZqVVt2
+AAA+gQD9EpMMjlBFvvyACsKwQRmIqUNTfCy4uHL1Ee1fJ4ur9ZQBAP2CiOSN
+CNa5yq6lyexhsn2Vs8DsX+SOSUyNJiy5FyIJ
+-----END PGP PRIVATE KEY BLOCK-----` });
+    const passphrase = 'passphrase';
+    // sanity checks about key encryption mechanism
+    expect(v4KeyWithAEAD.keyPacket.aead).to.not.be.null;
+    expect(v4KeyWithAEAD.keyPacket.isLegacyAEAD).to.be.false;
+    expect(v4KeyWithAEAD.keyPacket.usedModernAEAD).to.be.true;
+
+    const reEncryptedKey = await openpgp.encryptKey({
+      privateKey: await openpgp.decryptKey({ privateKey: v4KeyWithAEAD, passphrase }),
+      passphrase,
+      config: { aeadProtect: false }
+    });
+
+    expect(reEncryptedKey.keyPacket.usedModernAEAD).to.be.false;
+    const reDecryptedKey = await openpgp.decryptKey({ privateKey: reEncryptedKey, passphrase });
+    reDecryptedKey.keyPacket.privateParams.seed = new Uint8Array(1); // corrupt key to confirm that the actual validation is now run
+    await expect(reDecryptedKey.validate()).to.be.rejectedWith('Key is invalid');
+  });
+
   it('isDecrypted() - should reflect whether all (sub)keys are encrypted', async function() {
     const passphrase = '12345678';
     const { privateKey: key } = await openpgp.generateKey({ userIDs: {}, curve: 'ed25519Legacy', passphrase, format: 'object' });