Add config.rejectPublicKeyAlgorithms (#1264)

- Add `config.rejectPublicKeyAlgorithms` to disallow using the given algorithms
  to verify, sign or encrypt new messages or third-party certifications.

- Consider `config.minRsaBits` when signing, verifying and encrypting messages
  and third-party certifications, not just on key generation.

- When verifying a message, if the verification key is not found (i.e. not
  provided or too weak), the corresponding `signature` will have
  `signature.valid=false` (used to be `signature.valid=null`).
  `signature.error` will detail whether the key is missing/too weak/etc.

Generating and verifying key certification signatures is still permitted in all cases.

.eslintrc.js

@@ -215,7 +215,7 @@ module.exports = {
     "no-restricted-imports": "error",
     "no-restricted-modules": "error",
     "no-restricted-properties": "error",
-    "no-restricted-syntax": "error",
+    "no-restricted-syntax": ["error", "ForInStatement", "LabeledStatement", "WithStatement"],
     "no-return-assign": "error",
     "no-return-await": "error",
     "no-script-url": "error",
@@ -344,7 +344,7 @@ module.exports = {
     "no-use-before-define": [ 2, { "functions": false, "classes": true, "variables": false }],
     "no-constant-condition": [ 2, { "checkLoops": false } ],
     "new-cap": [ 2, { "properties": false, "capIsNewExceptionPattern": "EAX|OCB|GCM|CMAC|CBC|OMAC|CTR", "newIsCapExceptionPattern": "type|hash*"}],
-    "max-lines": [ 2, { "max": 600, "skipBlankLines": true, "skipComments": true } ],
+    "max-lines": [ 2, { "max": 620, "skipBlankLines": true, "skipComments": true } ],
     "no-unused-expressions": 0,
     "chai-friendly/no-unused-expressions": [ 2, { "allowShortCircuit": true } ],
 

src/cleartext.js

@@ -77,7 +77,9 @@ export class CleartextMessage {
    * @param {Array<Key>} keys - Array of keys to verify signatures
    * @param {Date} [date] - Verify the signature against the given date, i.e. check signature creation time < date < expiration time
    * @param {Object} [config] - Full configuration, defaults to openpgp.config
-   * @returns {Array<{keyid: module:type/keyid~Keyid, valid: Boolean}>} List of signer's keyid and validity of signature.
+   * @returns {Array<{keyid: module:type/keyid~Keyid,
+   *                  signature: Promise<Signature>,
+   *                  verified: Promise<Boolean>}>} List of signer's keyid and validity of signature.
    * @async
    */
   verify(keys, date = new Date(), config = defaultConfig) {

src/config/config.js

@@ -105,7 +105,7 @@ export default {
   checksumRequired: false,
   /**
    * @memberof module:config
-   * @property {Number} minRsaBits Minimum RSA key size allowed for key generation
+   * @property {Number} minRsaBits Minimum RSA key size allowed for key generation and message signing, verification and encryption
    */
   minRsaBits: 2048,
   /**
@@ -180,13 +180,21 @@ export default {
    */
   useIndutnyElliptic: true,
   /**
+   * Reject insecure hash algorithms
    * @memberof module:config
-   * @property {Set<Integer>} reject_hash_algorithms Reject insecure hash algorithms {@link module:enums.hash}
+   * @property {Set<Integer>} rejectHashAlgorithms {@link module:enums.hash}
    */
-  rejectHashAlgorithms: new globalThis.Set([enums.hash.md5, enums.hash.ripemd]),
+  rejectHashAlgorithms: new Set([enums.hash.md5, enums.hash.ripemd]),
   /**
+   * Reject insecure message hash algorithms
    * @memberof module:config
-   * @property {Set<Integer>} reject_message_hash_algorithms Reject insecure message hash algorithms {@link module:enums.hash}
+   * @property {Set<Integer>} rejectMessageHashAlgorithms {@link module:enums.hash}
    */
-  rejectMessageHashAlgorithms: new globalThis.Set([enums.hash.md5, enums.hash.ripemd, enums.hash.sha1])
+  rejectMessageHashAlgorithms: new Set([enums.hash.md5, enums.hash.ripemd, enums.hash.sha1]),
+  /**
+   * Reject insecure public key algorithms for 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])
 };

src/key/factory.js

@@ -67,42 +67,42 @@ export async function generate(options, config) {
  */
 export async function reformat(options, config) {
   options = sanitize(options);
+  const { privateKey } = options;
 
-  if (options.privateKey.primaryKey.isDummy()) {
+  if (privateKey.isPublic()) {
+    throw new Error('Cannot reformat a public key');
+  }
+
+  if (privateKey.primaryKey.isDummy()) {
     throw new Error('Cannot reformat a gnu-dummy primary key');
   }
 
-  const isDecrypted = options.privateKey.getKeys().every(({ keyPacket }) => keyPacket.isDecrypted());
+  const isDecrypted = privateKey.getKeys().every(({ keyPacket }) => keyPacket.isDecrypted());
   if (!isDecrypted) {
     throw new Error('Key is not decrypted');
   }
 
-  const packetlist = options.privateKey.toPacketlist();
-  let secretKeyPacket;
-  const secretSubkeyPackets = [];
-  for (let i = 0; i < packetlist.length; i++) {
-    if (packetlist[i].tag === enums.packet.secretKey) {
-      secretKeyPacket = packetlist[i];
-    } else if (packetlist[i].tag === enums.packet.secretSubkey) {
-      secretSubkeyPackets.push(packetlist[i]);
-    }
-  }
-  if (!secretKeyPacket) {
-    throw new Error('Key does not contain a secret key packet');
-  }
+  const secretKeyPacket = privateKey.keyPacket;
 
   if (!options.subkeys) {
-    options.subkeys = await Promise.all(secretSubkeyPackets.map(async secretSubkeyPacket => ({
-      sign: await options.privateKey.getSigningKey(secretSubkeyPacket.getKeyId(), null, undefined, config).catch(() => {}) &&
-          !await options.privateKey.getEncryptionKey(secretSubkeyPacket.getKeyId(), null, undefined, config).catch(() => {})
-    })));
+    options.subkeys = await Promise.all(privateKey.subKeys.map(async subkey => {
+      const secretSubkeyPacket = subkey.keyPacket;
+      const dataToVerify = { key: secretKeyPacket, bind: secretSubkeyPacket };
+      const bindingSignature = await (
+        helper.getLatestValidSignature(subkey.bindingSignatures, secretKeyPacket, enums.signature.subkeyBinding, dataToVerify, null, config)
+      ).catch(() => ({}));
+      return {
+        sign: bindingSignature.keyFlags && (bindingSignature.keyFlags[0] & enums.keyFlags.signData)
+      };
+    }));
   }
 
+  const secretSubkeyPackets = privateKey.subKeys.map(subkey => subkey.keyPacket);
   if (options.subkeys.length !== secretSubkeyPackets.length) {
     throw new Error('Number of subkey options does not match number of subkeys');
   }
 
-  options.subkeys = options.subkeys.map(function(subkey, index) { return sanitize(options.subkeys[index], options); });
+  options.subkeys = options.subkeys.map(subkeyOptions => sanitize(subkeyOptions, options));
 
   return wrapKeyObject(secretKeyPacket, secretSubkeyPackets, options, config);
 

src/key/helper.js

@@ -370,9 +370,11 @@ export function isValidSigningKeyPacket(keyPacket, signature) {
   if (!signature.verified || signature.revoked !== false) { // Sanity check
     throw new Error('Signature not verified');
   }
-  return keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.rsaEncrypt) &&
-    keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.elgamal) &&
-    keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.ecdh) &&
+
+  const keyAlgo = enums.write(enums.publicKey, keyPacket.algorithm);
+  return keyAlgo !== enums.publicKey.rsaEncrypt &&
+    keyAlgo !== enums.publicKey.elgamal &&
+    keyAlgo !== enums.publicKey.ecdh &&
     (!signature.keyFlags ||
       (signature.keyFlags[0] & enums.keyFlags.signData) !== 0);
 }
@@ -381,10 +383,12 @@ export function isValidEncryptionKeyPacket(keyPacket, signature) {
   if (!signature.verified || signature.revoked !== false) { // Sanity check
     throw new Error('Signature not verified');
   }
-  return keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.dsa) &&
-    keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.rsaSign) &&
-    keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.ecdsa) &&
-    keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.eddsa) &&
+
+  const keyAlgo = enums.write(enums.publicKey, keyPacket.algorithm);
+  return keyAlgo !== enums.publicKey.dsa &&
+    keyAlgo !== enums.publicKey.rsaSign &&
+    keyAlgo !== enums.publicKey.ecdsa &&
+    keyAlgo !== enums.publicKey.eddsa &&
     (!signature.keyFlags ||
       (signature.keyFlags[0] & enums.keyFlags.encryptCommunication) !== 0 ||
       (signature.keyFlags[0] & enums.keyFlags.encryptStorage) !== 0);
@@ -396,11 +400,22 @@ export function isValidDecryptionKeyPacket(signature, config) {
   }
 
   if (config.allowInsecureDecryptionWithSigningKeys) {
-    // This is only relevant for RSA keys, all other signing ciphers cannot decrypt
+    // This is only relevant for RSA keys, all other signing algorithms cannot decrypt
     return true;
   }
 
   return !signature.keyFlags ||
     (signature.keyFlags[0] & enums.keyFlags.encryptCommunication) !== 0 ||
     (signature.keyFlags[0] & enums.keyFlags.encryptStorage) !== 0;
 }
+
+export function checkKeyStrength(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.`);
+  }
+}

src/key/key.js

@@ -291,29 +291,41 @@ class Key {
     const primaryKey = this.keyPacket;
     const subKeys = this.subKeys.slice().sort((a, b) => b.keyPacket.created - a.keyPacket.created);
     let exception;
-    for (let i = 0; i < subKeys.length; i++) {
-      if (!keyId || subKeys[i].getKeyId().equals(keyId)) {
+    for (const subKey of subKeys) {
+      if (!keyId || subKey.getKeyId().equals(keyId)) {
         try {
-          await subKeys[i].verify(primaryKey, date, config);
-          const dataToVerify = { key: primaryKey, bind: subKeys[i].keyPacket };
-          const bindingSignature = await helper.getLatestValidSignature(subKeys[i].bindingSignatures, primaryKey, enums.signature.subkeyBinding, dataToVerify, date, config);
-          if (
-            bindingSignature &&
-            bindingSignature.embeddedSignature &&
-            helper.isValidSigningKeyPacket(subKeys[i].keyPacket, bindingSignature) &&
-            await helper.getLatestValidSignature([bindingSignature.embeddedSignature], subKeys[i].keyPacket, enums.signature.keyBinding, dataToVerify, date, config)
-          ) {
-            return subKeys[i];
+          await subKey.verify(primaryKey, date, config);
+          const dataToVerify = { key: primaryKey, bind: subKey.keyPacket };
+          const bindingSignature = await helper.getLatestValidSignature(
+            subKey.bindingSignatures, primaryKey, enums.signature.subkeyBinding, dataToVerify, date, config
+          );
+          if (!helper.isValidSigningKeyPacket(subKey.keyPacket, bindingSignature)) {
+            continue;
           }
+          if (!bindingSignature.embeddedSignature) {
+            throw new Error('Missing embedded signature');
+          }
+          // verify embedded signature
+          await helper.getLatestValidSignature(
+            [bindingSignature.embeddedSignature], subKey.keyPacket, enums.signature.keyBinding, dataToVerify, date, config
+          );
+          helper.checkKeyStrength(subKey.keyPacket, config);
+          return subKey;
         } catch (e) {
           exception = e;
         }
       }
     }
-    const primaryUser = await this.getPrimaryUser(date, userId, config);
-    if ((!keyId || primaryKey.getKeyId().equals(keyId)) &&
-        helper.isValidSigningKeyPacket(primaryKey, primaryUser.selfCertification)) {
-      return this;
+
+    try {
+      const primaryUser = await this.getPrimaryUser(date, userId, config);
+      if ((!keyId || primaryKey.getKeyId().equals(keyId)) &&
+          helper.isValidSigningKeyPacket(primaryKey, primaryUser.selfCertification, config)) {
+        helper.checkKeyStrength(primaryKey, config);
+        return this;
+      }
+    } catch (e) {
+      exception = e;
     }
     throw util.wrapError('Could not find valid signing key packet in key ' + this.getKeyId().toHex(), exception);
   }
@@ -333,25 +345,32 @@ class Key {
     // 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;
-    for (let i = 0; i < subKeys.length; i++) {
-      if (!keyId || subKeys[i].getKeyId().equals(keyId)) {
+    for (const subKey of subKeys) {
+      if (!keyId || subKey.getKeyId().equals(keyId)) {
         try {
-          await subKeys[i].verify(primaryKey, date, config);
-          const dataToVerify = { key: primaryKey, bind: subKeys[i].keyPacket };
-          const bindingSignature = await helper.getLatestValidSignature(subKeys[i].bindingSignatures, primaryKey, enums.signature.subkeyBinding, dataToVerify, date, config);
-          if (bindingSignature && helper.isValidEncryptionKeyPacket(subKeys[i].keyPacket, bindingSignature)) {
-            return subKeys[i];
+          await subKey.verify(primaryKey, date, config);
+          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);
+            return subKey;
           }
         } catch (e) {
           exception = e;
         }
       }
     }
-    // if no valid subkey for encryption, evaluate primary key
-    const primaryUser = await this.getPrimaryUser(date, userId, config);
-    if ((!keyId || primaryKey.getKeyId().equals(keyId)) &&
-        helper.isValidEncryptionKeyPacket(primaryKey, primaryUser.selfCertification)) {
-      return this;
+
+    try {
+      // if no valid subkey for encryption, evaluate primary key
+      const primaryUser = await this.getPrimaryUser(date, userId, config);
+      if ((!keyId || primaryKey.getKeyId().equals(keyId)) &&
+          helper.isValidEncryptionKeyPacket(primaryKey, primaryUser.selfCertification)) {
+        helper.checkKeyStrength(primaryKey, config);
+        return this;
+      }
+    } catch (e) {
+      exception = e;
     }
     throw util.wrapError('Could not find valid encryption key packet in key ' + this.getKeyId().toHex(), exception);
   }
@@ -374,7 +393,7 @@ class Key {
         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 (bindingSignature && helper.isValidDecryptionKeyPacket(bindingSignature, config)) {
+          if (helper.isValidDecryptionKeyPacket(bindingSignature, config)) {
             keys.push(this.subKeys[i]);
           }
         } catch (e) {}
@@ -486,7 +505,7 @@ class Key {
        * It is enough to validate any signing keys
        * since its binding signatures are also checked
        */
-      const signingKey = await this.getSigningKey(null, null, undefined, config);
+      const signingKey = await this.getSigningKey(null, null, undefined, { ...config, rejectPublicKeyAlgorithms: new Set(), minRsaBits: 0 });
       // This could again be a dummy key
       if (signingKey && !signingKey.keyPacket.isDummy()) {
         signingKeyPacket = signingKey.keyPacket;
@@ -581,16 +600,16 @@ class Key {
     let expiry = keyExpiry < sigExpiry ? keyExpiry : sigExpiry;
     if (capabilities === 'encrypt' || capabilities === 'encrypt_sign') {
       const encryptKey =
-        await this.getEncryptionKey(keyId, expiry, userId, config).catch(() => {}) ||
-        await this.getEncryptionKey(keyId, null, userId, config).catch(() => {});
+        await this.getEncryptionKey(keyId, expiry, userId, { ...config, rejectPublicKeyAlgorithms: new Set(), minRsaBits: 0 }).catch(() => {}) ||
+        await this.getEncryptionKey(keyId, null, userId, { ...config, rejectPublicKeyAlgorithms: new Set(), minRsaBits: 0 }).catch(() => {});
       if (!encryptKey) return null;
       const encryptExpiry = await encryptKey.getExpirationTime(this.keyPacket, undefined, config);
       if (encryptExpiry < expiry) expiry = encryptExpiry;
     }
     if (capabilities === 'sign' || capabilities === 'encrypt_sign') {
       const signKey =
-        await this.getSigningKey(keyId, expiry, userId, config).catch(() => {}) ||
-        await this.getSigningKey(keyId, null, userId, config).catch(() => {});
+        await this.getSigningKey(keyId, expiry, userId, { ...config, rejectPublicKeyAlgorithms: new Set(), minRsaBits: 0 }).catch(() => {}) ||
+        await this.getSigningKey(keyId, null, userId, { ...config, rejectPublicKeyAlgorithms: new Set(), minRsaBits: 0 }).catch(() => {});
       if (!signKey) return null;
       const signExpiry = await signKey.getExpirationTime(this.keyPacket, undefined, config);
       if (signExpiry < expiry) expiry = signExpiry;