From 81d2e8ce6030eef33f7a16f46bc9876235fde740 Mon Sep 17 00:00:00 2001 From: larabr <7375870+larabr@users.noreply.github.com> Date: Thu, 1 Feb 2024 14:43:38 +0100 Subject: [PATCH] Ensure primary key meets strength and algo requirements when encrypting/verifying/signing using subkeys Breaking change: the requirements of `config.minRSABits`, `rejectPublicKeyAlgorithms` and `rejectCurves` are now applied to the primary key, aside from the selected subkey. The motivation is that the subkeys are certified by the primary key, but if the latter is weak, arbitrary subkeys could potentially be added. Note that the change does not affect decryption, to allow decrypting older messages. --- src/key/key.js | 14 ++++++++++++-- test/general/config.js | 21 ++++++++++++++++++--- test/general/key.js | 8 +++----- test/general/openpgp.js | 2 +- test/general/signature.js | 3 ++- 5 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/key/key.js b/src/key/key.js index 43154a88..1662ca5a 100644 --- a/src/key/key.js +++ b/src/key/key.js @@ -257,8 +257,13 @@ class Key { * @async */ async getSigningKey(keyID = null, date = new Date(), userID = {}, config = defaultConfig) { - await this.verifyPrimaryKey(date, userID, config); const primaryKey = this.keyPacket; + await this.verifyPrimaryKey(date, userID, config); + try { + helper.checkKeyRequirements(primaryKey, config); + } catch (err) { + throw util.wrapError('Could not verify primary key', err); + } const subkeys = this.subkeys.slice().sort((a, b) => b.keyPacket.created - a.keyPacket.created); let exception; for (const subkey of subkeys) { @@ -311,8 +316,13 @@ class Key { * @async */ async getEncryptionKey(keyID, date = new Date(), userID = {}, config = defaultConfig) { - await this.verifyPrimaryKey(date, userID, config); const primaryKey = this.keyPacket; + await this.verifyPrimaryKey(date, userID, config); + try { + helper.checkKeyRequirements(primaryKey, config); + } catch (err) { + throw util.wrapError('Could not verify primary key', err); + } // V4: by convention subkeys are preferred for encryption service const subkeys = this.subkeys.slice().sort((a, b) => b.keyPacket.created - a.keyPacket.created); let exception; diff --git a/test/general/config.js b/test/general/config.js index 34e8a1fa..c9080ea0 100644 --- a/test/general/config.js +++ b/test/general/config.js @@ -300,10 +300,25 @@ n9/quqtmyOtYOA6gXNCw0Fal3iANKBmsPmYI message, encryptionKeys: [key], config: { rejectCurves: new Set([openpgp.enums.curve.curve25519Legacy]) } })).to.be.eventually.rejectedWith(/Support for ecdh keys using curve curve25519Legacy is disabled/); - const echdEncrypted = await openpgp.encrypt({ + await expect(openpgp.encrypt({ message, encryptionKeys: [key], config: { rejectCurves: new Set([openpgp.enums.curve.ed25519Legacy]) } - }); - expect(echdEncrypted).to.match(/---BEGIN PGP MESSAGE---/); + })).to.be.eventually.rejectedWith(/Could not verify primary key: Support for eddsaLegacy keys using curve ed25519Legacy is disabled/); + + // RSA 512 bits primary key, ECC subkey + const weakPrimaryKey = await openpgp.readKey({ armoredKey: `-----BEGIN PGP PUBLIC KEY BLOCK----- + +xk0EZbuUkQECAOVRTj4yGjMTk94lHaJGJZpwAnPJzSLr0lsqRzbsaeL+JeUr +HtKQyv8wEnqN0o7j39DXdFBI8f2/T0DkC4gkbQsAEQEAAc0OPHRlc3RAdGVz +dC5pdD7CiwQQAQgAPwWCZbuUkQMLCQcJkL/AJzZtSewrBRUICgwOBBYAAgEC +GQECmwMCHgEWIQSUuxkscrvIncEIj8K/wCc2bUnsKwAABpYCAMsq3UDscj6W +IVz8+VubCuJma95dgMXjqDGd2XGLUthYzKQ+k0USut3nwrt5aJOiQGse7W9O +Mjr/KnRCNGrJdm7OOARlu5SREgorBgEEAZdVAQUBAQdAkDQHPjXorB969PXZ +p09HqVCOqcOAzKi4KLL7I3QosmsDAQgHwnYEGAEIACoFgmW7lJEJkL/AJzZt +SewrApsMFiEElLsZLHK7yJ3BCI/Cv8AnNm1J7CsAAJ6VAf9uBYUWIM2LFx1L +c1HGHD56KA0Mu4eQksKNEugotEyBuWiZCVO+LBrDUFztC1IwXaNPL3bCjYaD +5f5A+c8qOY1f +-----END PGP PUBLIC KEY BLOCK-----` }); + await expect(openpgp.encrypt({ message, encryptionKeys: weakPrimaryKey, config: { minRSABits: 2048 } })).to.be.rejectedWith(/Could not verify primary key: RSA keys shorter than 2048 bits are considered too weak./); } finally { openpgp.config.aeadProtect = aeadProtectVal; openpgp.config.preferredCompressionAlgorithm = preferredCompressionAlgorithmVal; diff --git a/test/general/key.js b/test/general/key.js index caeec0f9..a49f0b20 100644 --- a/test/general/key.js +++ b/test/general/key.js @@ -4284,11 +4284,9 @@ VYGdb3eNlV8CfoEC it('Reject encryption with key revoked with appended revocation cert', async function() { const key = await openpgp.readKey({ armoredKey: pub_revoked_with_cert }); - return openpgp.encrypt({ encryptionKeys: [key], message: await openpgp.createMessage({ text: 'random data' }) }).then(() => { - throw new Error('encryptSessionKey should not encrypt with revoked public key'); - }).catch(function(error) { - expect(error.message).to.equal('Error encrypting message: Primary key is revoked'); - }); + await expect( + openpgp.encrypt({ encryptionKeys: [key], message: await openpgp.createMessage({ text: 'random data' }) }) + ).to.be.rejectedWith(/Primary key is revoked/); }); it('Merge key with another key with non-ID user attributes', async function() { diff --git a/test/general/openpgp.js b/test/general/openpgp.js index 1c4eb283..d605c2fb 100644 --- a/test/general/openpgp.js +++ b/test/general/openpgp.js @@ -3990,7 +3990,7 @@ XfA3pqV4mTzF }).then(function() { throw new Error('Should not encrypt with revoked key'); }).catch(function(error) { - expect(error.message).to.match(/Error encrypting message: Primary key is revoked/); + expect(error.message).to.match(/Primary key is revoked/); }); }); }); diff --git a/test/general/signature.js b/test/general/signature.js index 01a3a0c0..a66b71b5 100644 --- a/test/general/signature.js +++ b/test/general/signature.js @@ -895,7 +895,8 @@ DQmhI0SZoTKy4EGhS0bNJ+g2+dJ8Y22fKzLWXwo= await expect(openpgp.sign({ signingKeys: key, date: new Date(key.keyPacket.created - 3600), - message: await openpgp.createMessage({ text: 'Hello World' }) + message: await openpgp.createMessage({ text: 'Hello World' }), + config: { minRSABits: 1024 } })).to.be.rejectedWith(/Signature creation time is in the future/); });