From 22c2682574b6b806662d9847fde438cfc198a5e6 Mon Sep 17 00:00:00 2001 From: larabr Date: Fri, 2 Feb 2024 14:50:32 +0100 Subject: [PATCH] Ensure primary key meets strength and algo requirements when encrypting/verifying/signing using subkeys (#1719) 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 | 10 ++++++++++ test/general/config.js | 21 ++++++++++++++++++--- test/general/key.js | 8 +++----- test/general/openpgp.js | 2 +- test/general/signature.js | 3 ++- 5 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/key/key.js b/src/key/key.js index 43154a88..3c7475ad 100644 --- a/src/key/key.js +++ b/src/key/key.js @@ -259,6 +259,11 @@ class Key { async getSigningKey(keyID = null, date = new Date(), userID = {}, config = defaultConfig) { await this.verifyPrimaryKey(date, userID, config); const primaryKey = this.keyPacket; + 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) { @@ -313,6 +318,11 @@ class Key { async getEncryptionKey(keyID, date = new Date(), userID = {}, config = defaultConfig) { await this.verifyPrimaryKey(date, userID, config); const primaryKey = this.keyPacket; + 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 6a9668da..5e20a31e 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 90b70fe5..6c1e7903 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/); });