diff --git a/src/packet/secret_key.js b/src/packet/secret_key.js index 59d2a0af..a80feba7 100644 --- a/src/packet/secret_key.js +++ b/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 diff --git a/test/general/key.js b/test/general/key.js index 3dd6320e..caeec0f9 100644 --- a/test/general/key.js +++ b/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' });