From b0873eb98dca0ee9e69035f05e9ef3cab29ecb74 Mon Sep 17 00:00:00 2001 From: larabr <7375870+larabr@users.noreply.github.com> Date: Tue, 20 Aug 2024 16:48:51 +0200 Subject: [PATCH] `PrivateKey.getDecryptionKeys`: throw if no decryption key is found To avoid returning dummy key packets, and improving error reporting. This new behavior is also better aligned with that of `Key.getSigningKey()`. This is a breaking change for apps that call `getDecryptionKeys()` directly. The related error messages returned by `openpgp.decrypt` have also changed, becoming more specific. This change is also made in preparation of supporting private keys with public key packets (to be released in the next minor version, hence we want to avoid breaking changes there). --- src/key/private_key.js | 25 +++++++++++++++++++++---- src/message.js | 11 +++++++++-- test/general/key.js | 7 ++++++- test/general/openpgp.js | 6 +++--- 4 files changed, 39 insertions(+), 10 deletions(-) diff --git a/src/key/private_key.js b/src/key/private_key.js index 904c09a0..37cd4588 100644 --- a/src/key/private_key.js +++ b/src/key/private_key.js @@ -77,28 +77,45 @@ class PrivateKey extends PublicKey { * @param {String} userID, optional * @param {Object} [config] - Full configuration, defaults to openpgp.config * @returns {Promise>} Array of decryption keys. + * @throws {Error} if no decryption key is found * @async */ async getDecryptionKeys(keyID, date = new Date(), userID = {}, config = defaultConfig) { const primaryKey = this.keyPacket; const keys = []; + let exception = null; for (let i = 0; i < this.subkeys.length; i++) { if (!keyID || this.subkeys[i].getKeyID().equals(keyID, true)) { + if (this.subkeys[i].keyPacket.isDummy()) { + exception = exception || new Error('Gnu-dummy key packets cannot be used for decryption'); + continue; + } + try { const dataToVerify = { key: primaryKey, bind: this.subkeys[i].keyPacket }; const bindingSignature = await helper.getLatestValidSignature(this.subkeys[i].bindingSignatures, primaryKey, enums.signature.subkeyBinding, dataToVerify, date, config); if (helper.validateDecryptionKeyPacket(this.subkeys[i].keyPacket, bindingSignature, config)) { keys.push(this.subkeys[i]); } - } catch (e) {} + } catch (e) { + exception = e; + } } } // evaluate primary key const selfCertification = await this.getPrimarySelfSignature(date, userID, config); - if ((!keyID || primaryKey.getKeyID().equals(keyID, true)) && - helper.validateDecryptionKeyPacket(primaryKey, selfCertification, config)) { - keys.push(this); + if ((!keyID || primaryKey.getKeyID().equals(keyID, true)) && helper.validateDecryptionKeyPacket(primaryKey, selfCertification, config)) { + if (primaryKey.isDummy()) { + exception = exception || new Error('Gnu-dummy key packets cannot be used for decryption'); + } else { + keys.push(this); + } + } + + if (keys.length === 0) { + // eslint-disable-next-line @typescript-eslint/no-throw-literal + throw exception || new Error('No decryption key packets found'); } return keys; diff --git a/src/message.js b/src/message.js index 113d5623..05ef3a60 100644 --- a/src/message.js +++ b/src/message.js @@ -199,6 +199,15 @@ export class Message { } await Promise.all(pkeskPackets.map(async function(pkeskPacket) { await Promise.all(decryptionKeys.map(async function(decryptionKey) { + let decryptionKeyPackets; + try { + // do not check key expiration to allow decryption of old messages + decryptionKeyPackets = (await decryptionKey.getDecryptionKeys(pkeskPacket.publicKeyID, null, undefined, config)).map(key => key.keyPacket); + } catch (err) { + exception = err; + return; + } + let algos = [ enums.symmetric.aes256, // Old OpenPGP.js default fallback enums.symmetric.aes128, // RFC4880bis fallback @@ -212,8 +221,6 @@ export class Message { } } catch (e) {} - // do not check key expiration to allow decryption of old messages - const decryptionKeyPackets = (await decryptionKey.getDecryptionKeys(pkeskPacket.publicKeyID, null, undefined, config)).map(key => key.keyPacket); await Promise.all(decryptionKeyPackets.map(async function(decryptionKeyPacket) { if (!decryptionKeyPacket || decryptionKeyPacket.isDummy()) { return; diff --git a/test/general/key.js b/test/general/key.js index 06ed5ab4..995c32b5 100644 --- a/test/general/key.js +++ b/test/general/key.js @@ -3541,7 +3541,7 @@ PzIEeL7UH3trraFmi+Gq8u4kAA== await expect(openpgp.decrypt({ message: await openpgp.readMessage({ armoredMessage: encryptedRsaSignOnly }), decryptionKeys: key - })).to.be.rejectedWith(/Session key decryption failed/); + })).to.be.rejectedWith(/No decryption key packets found/); await expect(openpgp.decrypt({ message: await openpgp.readMessage({ armoredMessage: encryptedRsaSignOnly }), @@ -3550,6 +3550,11 @@ PzIEeL7UH3trraFmi+Gq8u4kAA== })).to.be.fulfilled; }); + it('PrivateKey.getDecryptionKeys() - should throw for sign-only key', async function() { + const key = await openpgp.readKey({ armoredKey: rsaSignOnly }); + await expect(key.getDecryptionKeys()).to.be.rejectedWith(/No decryption key packets found/); + }); + it('Key.getExpirationTime()', async function() { const [, pubKey] = await openpgp.readKeys({ armoredKeys: twoKeys }); expect(pubKey).to.exist; diff --git a/test/general/openpgp.js b/test/general/openpgp.js index e6f5bbca..fe9ae8c2 100644 --- a/test/general/openpgp.js +++ b/test/general/openpgp.js @@ -1768,7 +1768,7 @@ aOU= message: await openpgp.readMessage({ armoredMessage: encrypted }), decryptionKeys: privateKeyRSA, config - })).to.be.rejectedWith(/Session key decryption failed/); + })).to.be.rejectedWith(/No decryption key packets found/); // decryption using ECC key should succeed (PKCS1 is not used, so constant time countermeasures are not applied) const { data } = await openpgp.decrypt({ message: await openpgp.readMessage({ armoredMessage: encrypted }), @@ -2642,7 +2642,7 @@ XfA3pqV4mTzF }).then(() => { throw new Error('Should not decrypt with invalid key'); }).catch(error => { - expect(error.message).to.match(/Error decrypting session keys: Session key decryption failed./); + expect(error.message).to.match(/Error decrypting session keys: Could not find valid subkey binding signature in key/); }); }); }); @@ -4174,7 +4174,7 @@ XfA3pqV4mTzF decryptionKeys: decryptedKeyDE }; // binding signature is invalid - await expect(openpgp.decrypt(decOpt)).to.be.rejectedWith(/Session key decryption failed/); + await expect(openpgp.decrypt(decOpt)).to.be.rejectedWith(/Could not find valid subkey binding signature in key/); }); it('RSA decryption with PKCS1 padding of wrong length should fail', async function() {