Add config.rejectCurves and prevent generating keys using blacklisted algorithms (#1395)

Breaking changes:
- throw error on key generation if the requested public key algorithm is
included in `config.rejectPublicKeyAlgorithms`;
- add `config.rejectCurves` to blacklist a set of ECC curves, to prevent keys
using those curves from being generated, or being used to
encrypt/decrypt/sign/verify messages.
By default, `config.rejectCurves` includes the brainpool curves
(`brainpoolP256r1`, `brainpoolP384r1`, `brainpoolP512r1`) and the Bitcoin curve
(`secp256k1`). This is because it's unclear whether these curves will be
standardised[1], and we prefer to blacklist them already, rather than introduce
a breaking change after release.

[1] https://gitlab.com/openpgp-wg/rfc4880bis/-/merge_requests/47#note_634199141

openpgp.d.ts

@@ -337,6 +337,7 @@ interface Config {
   rejectHashAlgorithms: Set<enums.hash>;
   rejectMessageHashAlgorithms: Set<enums.hash>;
   rejectPublicKeyAlgorithms: Set<enums.publicKey>;
+  rejectCurves: Set<enums.curve>;
 }
 export var config: Config;
 
@@ -814,6 +815,18 @@ export namespace enums {
     aedsa = 24,
   }
 
+  enum curve {
+    p256 = 'p256',
+    p384 = 'p384',
+    p521 = 'p521',
+    ed25519 = 'ed25519',
+    curve25519 = 'curve25519',
+    secp256k1 = 'secp256k1',
+    brainpoolP256r1 = 'brainpoolP256r1',
+    brainpoolP384r1 = 'brainpoolP384r1',
+    brainpoolP512r1 = 'brainpoolP512r1'
+  }
+
   export type symmetricNames = 'plaintext' | 'idea' | 'tripledes' | 'cast5' | 'blowfish' | 'aes128' | 'aes192' | 'aes256' | 'twofish';
   enum symmetric {
     plaintext = 0,

src/config/config.js

@@ -181,8 +181,11 @@ export default {
    */
   knownNotations: ['preferred-email-encoding@pgp.com', 'pka-address@gnupg.org'],
   /**
+   * Whether to use the indutny/elliptic library for curves (other than Curve25519) that are not supported by the available native crypto API.
+   * When false, certain standard curves will not be supported (depending on the platform).
+   * Note: the indutny/elliptic curve library is not designed to be constant time.
    * @memberof module:config
-   * @property {Boolean} useIndutnyElliptic Whether to use the indutny/elliptic library. When false, certain curves will not be supported.
+   * @property {Boolean} useIndutnyElliptic
    */
   useIndutnyElliptic: true,
   /**
@@ -198,9 +201,15 @@ export default {
    */
   rejectMessageHashAlgorithms: new Set([enums.hash.md5, enums.hash.ripemd, enums.hash.sha1]),
   /**
-   * Reject insecure public key algorithms for message encryption, signing or verification
+   * Reject insecure public key algorithms for key generation and message encryption, signing or verification
    * @memberof module:config
    * @property {Set<Integer>} rejectPublicKeyAlgorithms {@link module:enums.publicKey}
    */
-  rejectPublicKeyAlgorithms: new Set([enums.publicKey.elgamal, enums.publicKey.dsa])
+  rejectPublicKeyAlgorithms: new Set([enums.publicKey.elgamal, enums.publicKey.dsa]),
+  /**
+   * Reject non-standard curves for key generation, message encryption, signing or verification
+   * @memberof module:config
+   * @property {Set<String>} rejectCurves {@link module:enums.curve}
+   */
+  rejectCurves: new Set([enums.curve.brainpoolP256r1, enums.curve.brainpoolP384r1, enums.curve.brainpoolP512r1, enums.curve.secp256k1])
 };

src/key/helper.js

@@ -391,13 +391,35 @@ export function isValidDecryptionKeyPacket(signature, config) {
     (signature.keyFlags[0] & enums.keyFlags.encryptStorage) !== 0;
 }
 
-export function checkKeyStrength(keyPacket, config) {
+/**
+ * Check key against blacklisted algorithms and minimum strength requirements.
+ * @param {SecretKeyPacket|PublicKeyPacket|
+ *        SecretSubkeyPacket|PublicSubkeyPacket} keyPacket
+ * @param {Config} config
+ * @throws {Error} if the key packet does not meet the requirements
+ */
+export function checkKeyRequirements(keyPacket, config) {
   const keyAlgo = enums.write(enums.publicKey, keyPacket.algorithm);
   if (config.rejectPublicKeyAlgorithms.has(keyAlgo)) {
     throw new Error(`${keyPacket.algorithm} keys are considered too weak.`);
   }
-  const rsaAlgos = new Set([enums.publicKey.rsaEncryptSign, enums.publicKey.rsaSign, enums.publicKey.rsaEncrypt]);
-  if (rsaAlgos.has(keyAlgo) && util.uint8ArrayBitLength(keyPacket.publicParams.n) < config.minRSABits) {
-    throw new Error(`RSA keys shorter than ${config.minRSABits} bits are considered too weak.`);
+  const algoInfo = keyPacket.getAlgorithmInfo();
+  switch (keyAlgo) {
+    case enums.publicKey.rsaEncryptSign:
+    case enums.publicKey.rsaSign:
+    case enums.publicKey.rsaEncrypt:
+      if (algoInfo.bits < config.minRSABits) {
+        throw new Error(`RSA keys shorter than ${config.minRSABits} bits are considered too weak.`);
+      }
+      break;
+    case enums.publicKey.ecdsa:
+    case enums.publicKey.eddsa:
+    case enums.publicKey.ecdh:
+      if (config.rejectCurves.has(algoInfo.curve)) {
+        throw new Error(`Support for ${keyPacket.algorithm} keys using curve ${algoInfo.curve} is disabled.`);
+      }
+      break;
+    default:
+      break;
   }
 }

src/key/key.js

@@ -254,7 +254,7 @@ class Key {
           await helper.getLatestValidSignature(
             [bindingSignature.embeddedSignature], subkey.keyPacket, enums.signature.keyBinding, dataToVerify, date, config
           );
-          helper.checkKeyStrength(subkey.keyPacket, config);
+          helper.checkKeyRequirements(subkey.keyPacket, config);
           return subkey;
         } catch (e) {
           exception = e;
@@ -266,7 +266,7 @@ class Key {
       const primaryUser = await this.getPrimaryUser(date, userID, config);
       if ((!keyID || primaryKey.getKeyID().equals(keyID)) &&
           helper.isValidSigningKeyPacket(primaryKey, primaryUser.selfCertification, config)) {
-        helper.checkKeyStrength(primaryKey, config);
+        helper.checkKeyRequirements(primaryKey, config);
         return this;
       }
     } catch (e) {
@@ -298,7 +298,7 @@ class Key {
           const dataToVerify = { key: primaryKey, bind: subkey.keyPacket };
           const bindingSignature = await helper.getLatestValidSignature(subkey.bindingSignatures, primaryKey, enums.signature.subkeyBinding, dataToVerify, date, config);
           if (helper.isValidEncryptionKeyPacket(subkey.keyPacket, bindingSignature)) {
-            helper.checkKeyStrength(subkey.keyPacket, config);
+            helper.checkKeyRequirements(subkey.keyPacket, config);
             return subkey;
           }
         } catch (e) {
@@ -312,7 +312,7 @@ class Key {
       const primaryUser = await this.getPrimaryUser(date, userID, config);
       if ((!keyID || primaryKey.getKeyID().equals(keyID)) &&
           helper.isValidEncryptionKeyPacket(primaryKey, primaryUser.selfCertification)) {
-        helper.checkKeyStrength(primaryKey, config);
+        helper.checkKeyRequirements(primaryKey, config);
         return this;
       }
     } catch (e) {

src/openpgp.js

@@ -21,6 +21,7 @@ import { CleartextMessage } from './cleartext';
 import { generate, reformat, getPreferredAlgo } from './key';
 import defaultConfig from './config';
 import util from './util';
+import { checkKeyRequirements } from './key/helper';
 
 
 //////////////////////
@@ -63,10 +64,12 @@ export async function generateKey({ userIDs = [], passphrase = '', type = 'ecc',
   if (type === 'rsa' && rsaBits < config.minRSABits) {
     throw new Error(`rsaBits should be at least ${config.minRSABits}, got: ${rsaBits}`);
   }
+
   const options = { userIDs, passphrase, type, rsaBits, curve, keyExpirationTime, date, subkeys };
 
   try {
     const { key, revocationCertificate } = await generate(options, config);
+    key.getKeys().forEach(({ keyPacket }) => checkKeyRequirements(keyPacket, config));
 
     return {
       privateKey: formatObject(key, format, config),

test/crypto/validate.js

@@ -81,7 +81,8 @@ async function cloneKeyPacket(key) {
 }
 
 async function generatePrivateKeyObject(options) {
-  const { privateKey } = await openpgp.generateKey({ ...options, userIDs: [{ name: 'Test', email: 'test@test.com' }], format: 'object' });
+  const config = { rejectCurves: new Set() };
+  const { privateKey } = await openpgp.generateKey({ ...options, userIDs: [{ name: 'Test', email: 'test@test.com' }], format: 'object', config });
   return privateKey;
 }
 

test/general/brainpool.js

@@ -10,12 +10,23 @@ const input = require('./testInputs.js');
 const expect = chai.expect;
 
 module.exports = () => (openpgp.config.ci ? describe.skip : describe)('Brainpool Cryptography @lightweight', function () {
-  //only x25519 crypto is fully functional in lightbuild
-  if (!openpgp.config.useIndutnyElliptic && !util.getNodeCrypto()) {
-    before(function() {
+  let rejectCurvesVal;
+  before(function() {
+    //only x25519 crypto is fully functional in lightbuild
+    if (!openpgp.config.useIndutnyElliptic && !util.getNodeCrypto()) {
       this.skip(); // eslint-disable-line no-invalid-this
-    });
-  }
+    }
+  });
+
+  beforeEach(function () {
+    rejectCurvesVal = openpgp.config.rejectCurves;
+    openpgp.config.rejectCurves = new Set();
+  });
+
+  afterEach(function () {
+    openpgp.config.rejectCurves = rejectCurvesVal;
+  });
+
   const data = {
     romeo: {
       id: 'fa3d64c9bcf338bc',
@@ -282,39 +293,46 @@ EJ4QcD/oQ6x1M/8X/iKQCtxZP8RnlrbH7ExkNON5s5g=
 
 function omnibus() {
   it('Omnibus BrainpoolP256r1 Test', async function() {
-    const testData = input.createSomeMessage();
-    const testData2 = input.createSomeMessage();
+    const { rejectCurves } = openpgp.config;
+    openpgp.config.rejectCurves = new Set();
 
-    const { privateKey: hi, publicKey: pubHi } = await openpgp.generateKey({ userIDs: { name: 'Hi', email: 'hi@hel.lo' }, curve: 'brainpoolP256r1', format: 'object' });
-    const { privateKey: bye, publicKey: pubBye } = await openpgp.generateKey({ userIDs: { name: 'Bye', email: 'bye@good.bye' }, curve: 'brainpoolP256r1', format: 'object' });
+    try {
+      const testData = input.createSomeMessage();
+      const testData2 = input.createSomeMessage();
 
-    const cleartextMessage = await openpgp.sign({ message: await openpgp.createCleartextMessage({ text: testData }), signingKeys: hi });
-    await openpgp.verify({
-      message: await openpgp.readCleartextMessage({ cleartextMessage }),
-      verificationKeys: pubHi
-    }).then(output => expect(output.signatures[0].verified).to.eventually.be.true);
-    // Verifying detached signature
-    await openpgp.verify({
-      message: await openpgp.createMessage({ text: util.removeTrailingSpaces(testData) }),
-      verificationKeys: pubHi,
-      signature: (await openpgp.readCleartextMessage({ cleartextMessage })).signature
-    }).then(output => expect(output.signatures[0].verified).to.eventually.be.true);
+      const { privateKey: hi, publicKey: pubHi } = await openpgp.generateKey({ userIDs: { name: 'Hi', email: 'hi@hel.lo' }, curve: 'brainpoolP256r1', format: 'object' });
+      const { privateKey: bye, publicKey: pubBye } = await openpgp.generateKey({ userIDs: { name: 'Bye', email: 'bye@good.bye' }, curve: 'brainpoolP256r1', format: 'object' });
 
-    // Encrypting and signing
-    const encrypted = await openpgp.encrypt({
-      message: await openpgp.createMessage({ text: testData2 }),
-      encryptionKeys: [pubBye],
-      signingKeys: [hi]
-    });
-    // Decrypting and verifying
-    return openpgp.decrypt({
-      message: await openpgp.readMessage({ armoredMessage: encrypted }),
-      decryptionKeys: bye,
-      verificationKeys: [pubHi]
-    }).then(async output => {
-      expect(output.data).to.equal(testData2);
-      await expect(output.signatures[0].verified).to.eventually.be.true;
-    });
+      const cleartextMessage = await openpgp.sign({ message: await openpgp.createCleartextMessage({ text: testData }), signingKeys: hi });
+      await openpgp.verify({
+        message: await openpgp.readCleartextMessage({ cleartextMessage }),
+        verificationKeys: pubHi
+      }).then(output => expect(output.signatures[0].verified).to.eventually.be.true);
+      // Verifying detached signature
+      await openpgp.verify({
+        message: await openpgp.createMessage({ text: util.removeTrailingSpaces(testData) }),
+        verificationKeys: pubHi,
+        signature: (await openpgp.readCleartextMessage({ cleartextMessage })).signature
+      }).then(output => expect(output.signatures[0].verified).to.eventually.be.true);
+
+      // Encrypting and signing
+      const encrypted = await openpgp.encrypt({
+        message: await openpgp.createMessage({ text: testData2 }),
+        encryptionKeys: [pubBye],
+        signingKeys: [hi]
+      });
+      // Decrypting and verifying
+      return openpgp.decrypt({
+        message: await openpgp.readMessage({ armoredMessage: encrypted }),
+        decryptionKeys: bye,
+        verificationKeys: [pubHi]
+      }).then(async output => {
+        expect(output.data).to.equal(testData2);
+        await expect(output.signatures[0].verified).to.eventually.be.true;
+      });
+    } finally {
+      openpgp.config.rejectCurves = rejectCurves;
+    }
   });
 }
 

test/general/config.js

@@ -230,6 +230,15 @@ vAFM3jjrAQDgJPXsv8PqCrLGDuMa/2r6SgzYd03aw/xt1WM6hgUvhQD+J54Z
       await expect(openpgp.encrypt({
         message, encryptionKeys: [key], config: { rejectPublicKeyAlgorithms: new Set([openpgp.enums.publicKey.ecdh]) }
       })).to.be.eventually.rejectedWith(/ecdh keys are considered too weak/);
+
+      await expect(openpgp.encrypt({
+        message, encryptionKeys: [key], config: { rejectCurves: new Set([openpgp.enums.curve.curve25519]) }
+      })).to.be.eventually.rejectedWith(/Support for ecdh keys using curve curve25519 is disabled/);
+
+      const echdEncrypted = await openpgp.encrypt({
+        message, encryptionKeys: [key], config: { rejectCurves: new Set([openpgp.enums.curve.ed25519]) }
+      });
+      expect(echdEncrypted).to.match(/---BEGIN PGP MESSAGE---/);
     } finally {
       openpgp.config.aeadProtect = aeadProtectVal;
       openpgp.config.preferredCompressionAlgorithm = preferredCompressionAlgorithmVal;
@@ -295,6 +304,9 @@ vAFM3jjrAQDgJPXsv8PqCrLGDuMa/2r6SgzYd03aw/xt1WM6hgUvhQD+J54Z
     await expect(openpgp.sign({
       message, signingKeys: [key], config: { rejectPublicKeyAlgorithms: new Set([openpgp.enums.publicKey.eddsa]) }
     })).to.be.eventually.rejectedWith(/eddsa keys are considered too weak/);
+    await expect(openpgp.sign({
+      message, signingKeys: [key], config: { rejectCurves: new Set([openpgp.enums.curve.ed25519]) }
+    })).to.be.eventually.rejectedWith(/Support for eddsa keys using curve ed25519 is disabled/);
   });
 
   it('openpgp.verify', async function() {
@@ -339,6 +351,14 @@ vAFM3jjrAQDgJPXsv8PqCrLGDuMa/2r6SgzYd03aw/xt1WM6hgUvhQD+J54Z
     };
     const { signatures: [sig4] } = await openpgp.verify(opt4);
     await expect(sig4.verified).to.be.rejectedWith(/eddsa keys are considered too weak/);
+
+    const opt5 = {
+      message: await openpgp.readMessage({ armoredMessage: signed }),
+      verificationKeys: [key],
+      config: { rejectCurves: new Set([openpgp.enums.curve.ed25519]) }
+    };
+    const { signatures: [sig5] } = await openpgp.verify(opt5);
+    await expect(sig5.verified).to.be.eventually.rejectedWith(/Support for eddsa keys using curve ed25519 is disabled/);
   });
 
   describe('detects unknown config property', async function() {

test/general/ecc_secp256k1.js

@@ -12,6 +12,17 @@ module.exports = () => describe('Elliptic Curve Cryptography for secp256k1 curve
       this.skip(); // eslint-disable-line no-invalid-this
     });
   }
+
+  let rejectCurvesVal;
+  beforeEach(() => {
+    rejectCurvesVal = openpgp.config.rejectCurves;
+    openpgp.config.rejectCurves = new Set();
+  });
+
+  afterEach(() => {
+    openpgp.config.rejectCurves = rejectCurvesVal;
+  });
+
   const data = {
     romeo: {
       id: 'c2b12389b401a43d',

test/general/openpgp.js

@@ -3724,10 +3724,11 @@ amnR6g==
       const curves = ['secp256k1' , 'p256', 'p384', 'p521', 'curve25519', 'brainpoolP256r1', 'brainpoolP384r1', 'brainpoolP512r1'];
       curves.forEach(curve => {
         it(`sign/verify with ${curve}`, async function() {
+          const config = { rejectCurves: new Set() };
           const plaintext = 'short message';
-          const { privateKey: key } = await openpgp.generateKey({ curve, userIDs: { name: 'Alice', email: 'info@alice.com' }, format: 'object' });
-          const signed = await openpgp.sign({ signingKeys:[key], message: await openpgp.createCleartextMessage({ text: plaintext }) });
-          const verified = await openpgp.verify({ verificationKeys:[key], message: await openpgp.readCleartextMessage({ cleartextMessage: signed }) });
+          const { privateKey: key } = await openpgp.generateKey({ curve, userIDs: { name: 'Alice', email: 'info@alice.com' }, format: 'object', config });
+          const signed = await openpgp.sign({ signingKeys:[key], message: await openpgp.createCleartextMessage({ text: plaintext }), config });
+          const verified = await openpgp.verify({ verificationKeys:[key], message: await openpgp.readCleartextMessage({ cleartextMessage: signed }), config });
           expect(await verified.signatures[0].verified).to.be.true;
         });
       });

test/general/streaming.js

@@ -383,11 +383,13 @@ function tests() {
     });
 
     try {
+      const config = { rejectCurves: new Set() };
       const encrypted = await openpgp.encrypt({
         message: await openpgp.createMessage({ binary: data }),
         encryptionKeys: pub,
         signingKeys: priv,
-        format: 'binary'
+        format: 'binary',
+        config
       });
       expect(stream.isStream(encrypted)).to.equal(expectedType);
 
@@ -396,7 +398,8 @@ function tests() {
         verificationKeys: pub,
         decryptionKeys: priv,
         message,
-        format: 'binary'
+        format: 'binary',
+        config
       });
       expect(stream.isStream(decrypted.data)).to.equal(expectedType);
       const reader = stream.getReader(decrypted.data);
@@ -706,11 +709,13 @@ function tests() {
       privateKey: await openpgp.readKey({ armoredKey: brainpoolPriv }),
       passphrase: brainpoolPass
     });
+    const config = { rejectCurves: new Set() };
 
     const signed = await openpgp.sign({
       message: await openpgp.createMessage({ binary: data }),
       signingKeys: priv,
-      detached: true
+      detached: true,
+      config
     });
     expect(stream.isStream(signed)).to.equal(expectedType);
     const armoredSignature = await stream.readToEnd(signed);
@@ -718,7 +723,8 @@ function tests() {
     const verified = await openpgp.verify({
       signature,
       verificationKeys: pub,
-      message: await openpgp.createMessage({ text: 'hello world' })
+      message: await openpgp.createMessage({ text: 'hello world' }),
+      config
     });
     expect(verified.data).to.equal('hello world');
     expect(verified.signatures).to.exist.and.have.length(1);