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.
2026-03-05 00:28:47 +00:00 · 2021-03-25 15:08:49 +01:00
parent 3e808c1578
commit 8a57246ec4
17 changed files with 759 additions and 518 deletions
--- a/src/cleartext.js
+++ b/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) {
--- a/src/config/config.js
+++ b/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])
 };
--- a/src/key/factory.js
+++ b/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);

--- a/src/key/helper.js
+++ b/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,7 +400,7 @@ 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;
  }

@@ -404,3 +408,14 @@ export function isValidDecryptionKeyPacket(signature, config) {
    (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.`);
+  }
+}
--- a/src/key/key.js
+++ b/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;
--- a/src/message.js
+++ b/src/message.js
@@ -523,7 +523,9 @@ export class Message {
   * @param {Date} [date] - Verify the signature against the given date, i.e. check signature creation time < date < expiration time
   * @param {Boolean} [streaming] - Whether to process data as a stream
   * @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 signatures.
   * @async
   */
  async verify(keys, date = new Date(), streaming, config = defaultConfig) {
@@ -576,7 +578,9 @@ export class Message {
   * @param {Signature} signature
   * @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
   */
  verifyDetached(signature, keys, date = new Date(), streaming, config = defaultConfig) {
@@ -733,28 +737,39 @@ export async function createSignaturePackets(literalDataPacket, privateKeys, sig
 *                    i.e. check signature creation time < date < expiration time
 * @param {Boolean} [detached] - Whether to verify detached signature packets
 * @param {Object} [config] - Full configuration, defaults to openpgp.config
- * @returns {Promise<Array<{keyid: module:type/keyid~Keyid,
- *                          valid: Boolean|null}>>} list of signer's keyid and validity of signature
+ * @returns {{keyid: module:type/keyid~Keyid,
+ *            signature: Promise<Signature>,
+ *            verified: Promise<Boolean>}} signer's keyid and validity of signature
 * @async
 * @private
 */
 async function createVerificationObject(signature, literalDataList, keys, date = new Date(), detached = false, streaming = false, config = defaultConfig) {
-  let primaryKey = null;
-  let signingKey = null;
-  await Promise.all(keys.map(async function(key) {
-    // Look for the unique key that matches issuerKeyId of signature
-    try {
-      signingKey = await key.getSigningKey(signature.issuerKeyId, null, undefined, config);
-      primaryKey = key;
-    } catch (e) {}
-  }));
+  let primaryKey;
+  let signingKey;
+  let keyError;

+  for (const key of keys) {
+    const issuerKeys = key.getKeys(signature.issuerKeyId);
+    if (issuerKeys.length > 0) {
+      primaryKey = key;
+      break;
+    }
+  }
+  if (!primaryKey) {
+    keyError = new Error(`Could not find signing key with key ID ${signature.issuerKeyId.toHex()}`);
+  } else {
+    try {
+      signingKey = await primaryKey.getSigningKey(signature.issuerKeyId, null, undefined, config);
+    } catch (e) {
+      keyError = e;
+    }
+  }
  const signaturePacket = signature.correspondingSig || signature;
  const verifiedSig = {
    keyid: signature.issuerKeyId,
    verified: (async () => {
-      if (!signingKey) {
-        return null;
+      if (keyError) {
+        throw keyError;
      }
      await signature.verify(signingKey.keyPacket, signature.signatureType, literalDataList[0], detached, streaming, config);
      const sig = await signaturePacket;
@@ -796,8 +811,9 @@ async function createVerificationObject(signature, literalDataList, keys, date =
 *                    i.e. check signature creation time < date < expiration time
 * @param {Boolean} [detached] - Whether to verify detached signature packets
 * @param {Object} [config] - Full configuration, defaults to openpgp.config
- * @returns {Promise<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 signatures
 * @async
 * @private
 */
--- a/src/packet/public_key.js
+++ b/src/packet/public_key.js
@@ -234,7 +234,7 @@ class PublicKeyPacket {
    // RSA, DSA or ElGamal public modulo
    const modulo = this.publicParams.n || this.publicParams.p;
    if (modulo) {
-      result.bits = modulo.length * 8;
+      result.bits = util.uint8ArrayBitLength(modulo);
    } else {
      result.curve = this.publicParams.oid.getName();
    }
--- a/src/packet/signature.js
+++ b/src/packet/signature.js
@@ -652,7 +652,7 @@ class SignaturePacket {
  /**
   * verifies the signature packet. Note: not all signature types are implemented
   * @param {PublicSubkeyPacket|PublicKeyPacket|
-   *         SecretSubkeyPacket|SecretKeyPacket} key the public key to verify the signature
+   *         SecretSubkeyPacket|SecretKeyPacket} key - the public key to verify the signature
   * @param {module:enums.signature} signatureType - Expected signature type
   * @param {String|Object} data - Data which on the signature applies
   * @param {Boolean} [detached] - Whether to verify a detached signature
--- a/src/util.js
+++ b/src/util.js
@@ -192,15 +192,28 @@ const util = {
   * @returns {Uint8Array} MPI-formatted Uint8Array.
   */
  uint8ArrayToMpi: function (bin) {
+    const bitSize = util.uint8ArrayBitLength(bin);
+    if (bitSize === 0) {
+      throw new Error('Zero MPI');
+    }
+    const stripped = bin.subarray(bin.length - Math.ceil(bitSize / 8));
+    const prefix = Uint8Array.from([(bitSize & 0xFF00) >> 8, bitSize & 0xFF]);
+    return util.concatUint8Array([prefix, stripped]);
+  },
+
+  /**
+   * Return bit length of the input data
+   * @param {Uint8Array} bin input data (big endian)
+   * @returns bit length
+   */
+  uint8ArrayBitLength: function (bin) {
    let i; // index of leading non-zero byte
    for (i = 0; i < bin.length; i++) if (bin[i] !== 0) break;
    if (i === bin.length) {
-      throw new Error('Zero MPI');
+      return 0;
    }
    const stripped = bin.subarray(i);
-    const size = (stripped.length - 1) * 8 + util.nbits(stripped[0]);
-    const prefix = Uint8Array.from([(size & 0xFF00) >> 8, size & 0xFF]);
-    return util.concatUint8Array([prefix, stripped]);
+    return (stripped.length - 1) * 8 + util.nbits(stripped[0]);
  },

  /**