fixup! crypto: split OpenSSL 3, BoringSSL, and legacy backends

Author: panva

deps/ncrypto/ncrypto.cc

@@ -5693,6 +5693,7 @@ Rsa::Rsa(const EVP_PKEY* pkey) : Rsa() {
   GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_RSA_COEFFICIENT1, &qi_);
 
   if (type == EVP_PKEY_RSA_PSS) {
+    MarkPopErrorOnReturn pop_errors;
     PssParams params;
     if (ReadRsaPssParams(pkey, &params)) pss_params_ = params;
   }

src/crypto/crypto_dh.cc

@@ -206,6 +206,13 @@ void New(const FunctionCallbackInfo<Value>& args) {
     }
   }
 
+#ifndef OPENSSL_IS_BORINGSSL
+  if (BN_num_bits(bn_p.get()) >= 512 && BN_cmp(bn_g.get(), bn_p.get()) >= 0) {
+    PutDhError(DH_R_BAD_GENERATOR);
+    return ThrowCryptoError(env, ERR_get_error(), "Invalid generator");
+  }
+#endif
+
   auto dh = DHPointer::New(std::move(bn_p), std::move(bn_g));
   if (!dh) {
     return THROW_ERR_INVALID_ARG_VALUE(env, "Invalid DH parameters");

src/node_constants.cc

@@ -57,6 +57,22 @@
 #if !defined(RSA_PKCS1_PSS_PADDING)
 #define RSA_PKCS1_PSS_PADDING 6
 #endif
+#if !defined(OPENSSL_IS_BORINGSSL) && OPENSSL_VERSION_MAJOR >= 3
+// OpenSSL hides these deprecated DH check constants under
+// OPENSSL_NO_DEPRECATED, but the numeric verifyError values remain public API.
+#if !defined(DH_CHECK_P_NOT_PRIME)
+#define DH_CHECK_P_NOT_PRIME 0x01
+#endif
+#if !defined(DH_CHECK_P_NOT_SAFE_PRIME)
+#define DH_CHECK_P_NOT_SAFE_PRIME 0x02
+#endif
+#if !defined(DH_UNABLE_TO_CHECK_GENERATOR)
+#define DH_UNABLE_TO_CHECK_GENERATOR 0x04
+#endif
+#if !defined(DH_NOT_SUITABLE_GENERATOR)
+#define DH_NOT_SUITABLE_GENERATOR 0x08
+#endif
+#endif
 #ifndef OPENSSL_NO_ENGINE
 #if !defined(OPENSSL_IS_BORINGSSL) && OPENSSL_VERSION_MAJOR >= 3
 // Engine constants remain public API while engine implementation lives in the

test/parallel/test-crypto-dh-curves.js

@@ -6,6 +6,10 @@ if (!common.hasCrypto)
 const assert = require('assert');
 const crypto = require('crypto');
 const { hasOpenSSL } = require('../common/crypto');
+const {
+  DH_CHECK_P_NOT_PRIME,
+  DH_CHECK_P_NOT_SAFE_PRIME,
+} = crypto.constants;
 
 // Second OAKLEY group, see
 // https://github.com/nodejs/node-v0.x-archive/issues/2338 and
@@ -16,6 +20,47 @@ const p = 'FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74' +
           'EE386BFB5A899FA5AE9F24117C4B1FE649286651ECE65381FFFFFFFFFFFFFFFF';
 crypto.createDiffieHellman(p, 'hex');
 
+if (!process.features.openssl_is_boringssl) {
+  const notPrime = Buffer.from(p, 'hex');
+  notPrime[notPrime.length - 1] = 0xfd;
+  assert.strictEqual(
+    crypto.createDiffieHellman(notPrime, Buffer.from([2])).verifyError,
+    DH_CHECK_P_NOT_PRIME);
+
+  const notSafePrime = Buffer.from(
+    'd2d6d13e1c1e0bbb63c742199dee010411f089ac74f0f7213348388280700fd6' +
+    '0ef9c1e7b096a4257dcbce61c544a5d1d23db4c49c63ce302f63be5cf5804327',
+    'hex');
+  assert.strictEqual(
+    crypto.createDiffieHellman(notSafePrime, Buffer.from([2])).verifyError,
+    DH_CHECK_P_NOT_SAFE_PRIME);
+
+  const group = crypto.getDiffieHellman('modp14');
+  const alice = crypto.createDiffieHellman(
+    group.getPrime(), group.getGenerator());
+  alice.generateKeys();
+  const groupPrime = BigInt(`0x${group.getPrime('hex')}`);
+  assert.throws(
+    () => alice.computeSecret(Buffer.from([1])),
+    {
+      code: 'ERR_CRYPTO_INVALID_KEYLEN',
+      message: 'Supplied key is too small'
+    });
+  assert.throws(
+    () => alice.computeSecret(group.getPrime()),
+    {
+      code: 'ERR_CRYPTO_INVALID_KEYLEN',
+      message: 'Supplied key is too large'
+    });
+  assert.throws(
+    () => alice.computeSecret(
+      Buffer.from((groupPrime - 1n).toString(16), 'hex')),
+    {
+      code: 'ERR_CRYPTO_INVALID_KEYLEN',
+      message: 'Supplied key is too large'
+    });
+}
+
 // Confirm DH_check() results are exposed for optional examination.
 const bad_dh = process.features.openssl_is_boringssl ?
   crypto.createDiffieHellman('abcd', 'hex', 0) :
@@ -26,6 +71,11 @@ if (hasOpenSSL(3)) {
   const smallSafePrime = crypto.createDiffieHellman(
     Buffer.from([23]), Buffer.from([2]));
   assert.notStrictEqual(smallSafePrime.verifyError, 0);
+
+  assert.throws(
+    () => crypto.createDiffieHellman(Buffer.from(p, 'hex'),
+                                     Buffer.from(p, 'hex')),
+    { code: 'ERR_OSSL_DH_BAD_GENERATOR' });
 }
 
 const availableCurves = new Set(crypto.getCurves());

test/parallel/test-crypto-key-objects.js

@@ -909,6 +909,41 @@ if (!process.features.openssl_is_boringssl) {
         }
       }
     }
+
+    const der = publicKey.export({ format: 'der', type: 'spki' });
+    const saltLengthParam = Buffer.from([0xa2, 0x03, 0x02, 0x01, 0x10]);
+    const saltLengthOffset = der.indexOf(saltLengthParam);
+    assert.notStrictEqual(saltLengthOffset, -1);
+
+    const importMalformedPublicKey = (key) => {
+      const malformedKey = createPublicKey({
+        key,
+        format: 'der',
+        type: 'spki'
+      });
+      assert.strictEqual(malformedKey.asymmetricKeyType, 'rsa-pss');
+      assert.strictEqual(malformedKey.asymmetricKeyDetails.modulusLength, 2048);
+      assert.strictEqual(malformedKey.asymmetricKeyDetails.publicExponent,
+                         65537n);
+    };
+
+    {
+      const negativeSaltLength = Buffer.from(der);
+      negativeSaltLength[saltLengthOffset + saltLengthParam.length - 1] = 0x80;
+      importMalformedPublicKey(negativeSaltLength);
+    }
+
+    {
+      const oversizedSaltLength = Buffer.concat([
+        der.subarray(0, saltLengthOffset),
+        Buffer.from([0xa2, 0x0b, 0x02, 0x09, 1, 0, 0, 0, 0, 0, 0, 0, 0x10]),
+        der.subarray(saltLengthOffset + saltLengthParam.length),
+      ]);
+      oversizedSaltLength.writeUInt16BE(der.readUInt16BE(2) + 8, 2);
+      oversizedSaltLength[5] = der[5] + 8;
+      oversizedSaltLength[18] = der[18] + 8;
+      importMalformedPublicKey(oversizedSaltLength);
+    }
   }
 
   {