crypto: skip DH checks for recognized groups

Explicit DH parameters are recognized as named groups during OpenSSL
provider import, but DHPointer::check() only skips primality checks for
groups constructed by name. This makes createDiffieHellman() block for
seconds on standard MODP groups.

Query the imported key's group name to restore the legacy DH_check()
shortcut without changing key generation. Add a deterministic regression
test with randomness disabled and cover explicit and padded parameters
through the JavaScript API.

Fixes: #67067
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
PR-URL: #67071
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Author: panva

deps/ncrypto/ncrypto.cc

@@ -2503,6 +2503,15 @@ DHPointer::CheckResult DHPointer::check() {
   // through the provider instead of preserving the historical verifyError.
   if (group_name_ != nullptr) return CheckResult::NONE;
 
+  // Like DH_check(), skip primality checks for groups OpenSSL recognizes from
+  // explicitly supplied parameters.
+  size_t group_name_len = 0;
+  if (dh_ &&
+      EVP_PKEY_get_group_name(dh_.get(), nullptr, 0, &group_name_len) == 1 &&
+      group_name_len > 0) {
+    return CheckResult::NONE;
+  }
+
   DeleteFnPtr<BIGNUM, BN_free> p;
   DeleteFnPtr<BIGNUM, BN_free> g;
   DeleteFnPtr<BIGNUM, BN_free> q;

test/cctest/test_node_crypto.cc

@@ -22,6 +22,7 @@
 #include <openssl/core_names.h>
 #include <openssl/params.h>
 #include <openssl/provider.h>
+#include <openssl/rand.h>
 #endif
 
 using ncrypto::Ec;
@@ -120,6 +121,55 @@ TEST(NodeCrypto, KeyAlgorithmNames) {
 }
 
 #if NCRYPTO_USE_OPENSSL_PROVIDER
+TEST(NodeCrypto, DhNamedGroupCheckDoesNotRequireRandomness) {
+  ncrypto::ClearErrorOnReturn clear_errors;
+  ncrypto::DeleteFnPtr<OSSL_LIB_CTX, OSSL_LIB_CTX_free> libctx(
+      OSSL_LIB_CTX_new());
+  ASSERT_TRUE(libctx);
+  ASSERT_EQ(
+      RAND_set_DRBG_type(
+          libctx.get(), "node-test-unavailable-rng", nullptr, nullptr, nullptr),
+      1);
+  OSSL_LIB_CTX* previous_libctx = OSSL_LIB_CTX_set0_default(libctx.get());
+  auto restore_libctx = node::OnScopeLeave(
+      [previous_libctx] { OSSL_LIB_CTX_set0_default(previous_libctx); });
+
+  // Primality checks need randomness. Known groups must skip those checks.
+  unsigned char random_byte;
+  ASSERT_NE(RAND_bytes_ex(libctx.get(), &random_byte, 1, 0), 1);
+  ERR_clear_error();
+
+  using DH = ncrypto::DHPointer;
+  for (const char* name :
+       {"modp5", "modp14", "modp15", "modp16", "modp17", "modp18"}) {
+    auto prime = DH::FindGroup(name);
+    auto generator = DH::GetStandardGenerator();
+    ASSERT_TRUE(prime);
+    ASSERT_TRUE(generator);
+    auto dh = DH::New(std::move(prime), std::move(generator));
+    ASSERT_TRUE(dh);
+    EXPECT_EQ(dh.check(), DH::CheckResult::NONE) << name;
+  }
+
+  auto prime = DH::FindGroup("modp14");
+  auto generator = DH::GetStandardGenerator();
+  ASSERT_TRUE(prime);
+  ASSERT_TRUE(generator);
+  ASSERT_EQ(BN_add_word(prime.get(), 1), 1);
+  auto composite = DH::New(std::move(prime), std::move(generator));
+  ASSERT_TRUE(composite);
+  EXPECT_EQ(composite.check(), DH::CheckResult::P_NOT_PRIME);
+
+  prime = DH::FindGroup("modp14");
+  generator = DH::GetStandardGenerator();
+  ASSERT_TRUE(prime);
+  ASSERT_TRUE(generator);
+  ASSERT_EQ(BN_add_word(generator.get(), 1), 1);
+  auto other_generator = DH::New(std::move(prime), std::move(generator));
+  ASSERT_TRUE(other_generator);
+  EXPECT_EQ(other_generator.check(), DH::CheckResult::CHECK_FAILED);
+}
+
 TEST(NodeCrypto, ProviderPkcs1PublicKeyImport) {
   ncrypto::ClearErrorOnReturn clear_errors;
   auto ctx = EVPKeyCtxPointer::NewFromAlgorithm(KeyAlgorithm::RSA);

test/parallel/test-crypto-dh-known-groups.js

@@ -0,0 +1,61 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('node:assert');
+const crypto = require('node:crypto');
+const { hasFIPS, isBoringSSL } = require('../common/crypto');
+
+if (isBoringSSL)
+  common.skip('OpenSSL named-group validation');
+
+const groups = ['modp14', 'modp15', 'modp16', 'modp17', 'modp18'];
+if (!hasFIPS(3))
+  groups.unshift('modp5');
+
+const privateKey = Buffer.from('01020304', 'hex');
+const generator = Buffer.from([2]);
+
+for (const name of groups) {
+  const peer = crypto.getDiffieHellman(name);
+  const prime = peer.getPrime();
+  const peerPublicKey = peer.generateKeys();
+
+  // SSH encodes positive integers with a leading zero when necessary.
+  for (const [p, g] of [
+    [prime, 2],
+    [Buffer.concat([Buffer.from([0]), prime]), Buffer.from([0, 2])],
+  ]) {
+    const dh = crypto.createDiffieHellman(p, g);
+    assert.strictEqual(dh.verifyError, 0);
+    assert.deepStrictEqual(dh.getPrime(), prime);
+    assert.deepStrictEqual(dh.getGenerator(), generator);
+
+    dh.setPrivateKey(privateKey);
+    const publicKey = dh.generateKeys();
+    assert.deepStrictEqual(dh.generateKeys(), publicKey);
+    assert.deepStrictEqual(dh.getPrivateKey(), privateKey);
+    assert.deepStrictEqual(dh.computeSecret(peerPublicKey),
+                           peer.computeSecret(publicKey));
+  }
+}
+
+// A known modulus alone must not bypass validation of different parameters.
+{
+  const prime = crypto.getDiffieHellman('modp14').getPrime();
+  const notPrime = Buffer.from(prime);
+  notPrime[notPrime.length - 1] &= 0xfe;
+  assert.strictEqual(
+    crypto.createDiffieHellman(notPrime, 2).verifyError &
+      crypto.constants.DH_CHECK_P_NOT_PRIME,
+    crypto.constants.DH_CHECK_P_NOT_PRIME);
+
+  const notGenerator = Buffer.from(prime);
+  notGenerator[notGenerator.length - 1]--;
+  assert.strictEqual(
+    crypto.createDiffieHellman(prime, notGenerator).verifyError &
+      crypto.constants.DH_NOT_SUITABLE_GENERATOR,
+    crypto.constants.DH_NOT_SUITABLE_GENERATOR);
+}