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

panva · panva · commit ef00769a75c2 · 2026-07-02T09:32:45.000+02:00
diff --git a/deps/ncrypto/engine.cc b/deps/ncrypto/engine.cc
@@ -1,7 +1,8 @@
 #include "ncrypto.h"
 
 #if !defined(OPENSSL_NO_ENGINE) &&                                             \
-    (NCRYPTO_ENGINE_COMPAT || NCRYPTO_USE_LEGACY_OPENSSL)
+    ((defined(NCRYPTO_ENGINE_COMPAT) && NCRYPTO_ENGINE_COMPAT) ||              \
+     NCRYPTO_USE_LEGACY_OPENSSL)
 #include <openssl/engine.h>
 #endif
 
diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc
@@ -1837,6 +1837,8 @@ bool GenerateDhPrivateKey(BignumPointer* out,
   return true;
 }
 
+// Recompute DH public keys locally when a private key already exists. Provider
+// keygen creates a fresh keypair, which is both slower and changes semantics.
 bool GenerateDhPublicKey(BignumPointer* out,
                          const BIGNUM* p,
                          const BIGNUM* g,
@@ -1860,6 +1862,8 @@ std::optional<int> CheckDhParams(const BIGNUM* p,
                                  const BIGNUM* g,
                                  const BIGNUM* q,
                                  const BIGNUM* j) {
+  // TODO(panva): In a semver-major, consider tightening OpenSSL 3 validation
+  // to report generator and q failures as strictly as legacy DH_check().
   if (p == nullptr || g == nullptr) return std::nullopt;
 
   const int p_bits = BN_num_bits(p);
@@ -2119,6 +2123,8 @@ DHPointer::CheckResult DHPointer::check() {
   ClearErrorOnReturn clearErrorOnReturn;
   if (!*this) return DHPointer::CheckResult::NONE;
 #if NCRYPTO_USE_OPENSSL3_PROVIDER
+  // TODO(panva): In a semver-major, consider validating named DH groups
+  // through the provider instead of preserving the historical verifyError.
   if (group_name_ != nullptr) return CheckResult::NONE;
 
   DeleteFnPtr<BIGNUM, BN_free> p;
@@ -2564,12 +2570,8 @@ DataPointer DHPointer::stateless(const EVPKeyPointer& ourKey,
   if (!ctx || EVP_PKEY_derive_init(ctx.get()) <= 0) {
     return {};
   }
-#if NCRYPTO_USE_OPENSSL3_PROVIDER
-  if (ourKey.id() == EVP_PKEY_DH &&
-      EVP_PKEY_CTX_set_dh_pad(ctx.get(), 1) <= 0) {
-    return {};
-  }
-#endif
+  // TODO(panva): In a semver-major, consider padding OpenSSL 3 DH derivation
+  // results here to match DiffieHellman::computeSecret().
   if (EVP_PKEY_derive_set_peer(ctx.get(), theirKey.get()) <= 0 ||
       EVP_PKEY_derive(ctx.get(), nullptr, &out_size) <= 0) {
     return {};
@@ -5651,11 +5653,17 @@ bool ReadRsaPssParams(const EVP_PKEY* pkey, Rsa::PssParams* params) {
                 item, item_len, 0x02, &int_header, &int_len, &int_total)) {
           return false;
         }
-        int64_t salt_length = 0;
+        // TODO(panva): In a semver-major, reject malformed RSA-PSS parameters
+        // at key import instead of omitting asymmetricKeyDetails fields.
+        if (int_len == 0 || int_len > sizeof(uint64_t) ||
+            (item[int_header] & 0x80) != 0) {
+          return false;
+        }
+        uint64_t salt_length = 0;
         for (size_t n = 0; n < int_len; n++) {
           salt_length = (salt_length << 8) | item[int_header + n];
         }
-        params->salt_length = salt_length;
+        params->salt_length = static_cast<int64_t>(salt_length);
         break;
       }
     }
diff --git a/deps/ncrypto/ncrypto.h b/deps/ncrypto/ncrypto.h
@@ -19,7 +19,8 @@
 #include <optional>
 #include <string>
 #include <string_view>
-#if NCRYPTO_ENGINE_COMPAT && !defined(OPENSSL_NO_ENGINE)
+#if defined(NCRYPTO_ENGINE_COMPAT) && NCRYPTO_ENGINE_COMPAT &&                 \
+    !defined(OPENSSL_NO_ENGINE)
 #include <openssl/engine.h>
 #endif  // NCRYPTO_ENGINE_COMPAT && !OPENSSL_NO_ENGINE
 
diff --git a/deps/ncrypto/unofficial.gni b/deps/ncrypto/unofficial.gni
@@ -26,7 +26,11 @@ template("ncrypto_gn_build") {
   source_set(target_name) {
     forward_variables_from(invoker, "*")
     public_configs = [ ":ncrypto_config" ]
-    sources = gypi_values.ncrypto_sources
+    defines = [
+      "NCRYPTO_ENGINE_COMPAT=1",
+      "OPENSSL_SUPPRESS_DEPRECATED",
+    ]
+    sources = gypi_values.ncrypto_sources + gypi_values.ncrypto_engine_sources
     deps = [ "$node_openssl_path" ]
   }
 }
diff --git a/src/crypto/crypto_common.cc b/src/crypto/crypto_common.cc
@@ -232,11 +232,14 @@ MaybeLocal<Object> GetEphemeralKey(Environment* env, const SSLPointer& ssl) {
         const char* curve_name;
         if (kid == EVP_PKEY_EC) {
           ECKeyPointer ec(key);
+          if (!ec) break;
           int nid = EC_GROUP_get_curve_name(ec.getGroup());
+          if (nid == NID_undef) break;
           curve_name = OBJ_nid2sn(nid);
         } else {
           curve_name = OBJ_nid2sn(kid);
         }
+        if (curve_name == nullptr) break;
         values[0] = env->ecdh_string();
         values[1] = OneByteString(env->isolate(), curve_name);
         values[2] = Integer::New(env->isolate(), key.bits());
diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc
@@ -2023,14 +2023,17 @@ void SecureContext::SetDHParam(const FunctionCallbackInfo<Value>& args) {
       return;
 
 #if NCRYPTO_USE_OPENSSL3_PROVIDER
-    dh.reset(PEM_read_bio_Parameters(bio.get(), nullptr));
+    EVPKeyPointer params(PEM_read_bio_Parameters(bio.get(), nullptr));
+    if (params && params.id() == EVP_PKEY_DH) dh.reset(params.release());
 #else
     dh.reset(PEM_read_bio_DHparams(bio.get(), nullptr, nullptr, nullptr));
 #endif
   }
 
   // Invalid dhparam is silently discarded and DHE is no longer used.
   // TODO(tniessen): don't silently discard invalid dhparam.
+  // TODO(panva): In a semver-major, reject non-DH parameter PEMs instead of
+  // silently treating them as absent.
   if (!dh)
     return;
 
diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc
@@ -471,7 +471,10 @@ bool ExportJWKEcKey(Environment* env,
   CHECK_EQ(m_pkey.id(), EVP_PKEY_EC);
 
   ECKeyPointer ec(m_pkey);
-  CHECK(ec);
+  if (!ec) {
+    THROW_ERR_CRYPTO_INVALID_JWK(env, "Invalid JWK EC key");
+    return false;
+  }
 
   const auto pub = ec.getPublicKey();
   const auto group = ec.getGroup();
@@ -753,10 +756,11 @@ bool GetEcKeyDetail(Environment* env,
   CHECK_EQ(m_pkey.id(), EVP_PKEY_EC);
 
   ECKeyPointer ec(m_pkey);
-  CHECK(ec);
+  if (!ec) return true;
 
   const auto group = ec.getGroup();
   int nid = EC_GROUP_get_curve_name(group);
+  if (nid == NID_undef) return true;
 
   return target
       ->Set(env->context(),
@@ -772,10 +776,11 @@ bool GetEcKeyDetail(Environment* env,
 
 size_t GroupOrderSize(const EVPKeyPointer& key) {
   ECKeyPointer ec(key);
-  CHECK(ec);
+  if (!ec) return 0;
   auto order = BignumPointer::New();
-  CHECK(order);
-  CHECK(EC_GROUP_get_order(ec.getGroup(), order.get(), nullptr));
+  if (!order || !EC_GROUP_get_order(ec.getGroup(), order.get(), nullptr)) {
+    return 0;
+  }
   return order.byteLength();
 }
 }  // namespace crypto
diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc
@@ -386,7 +386,10 @@ bool KeyObjectData::ToEncodedPublicKey(
     const auto& pkey = GetAsymmetricKey();
     if (pkey.id() == EVP_PKEY_EC) {
       ECKeyPointer ec_key(pkey);
-      CHECK(ec_key);
+      if (!ec_key) {
+        THROW_ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS(env);
+        return false;
+      }
       auto form = static_cast<point_conversion_form_t>(config.ec_point_form);
       const auto group = ec_key.getGroup();
       const auto point = ec_key.getPublicKey();
@@ -432,13 +435,22 @@ bool KeyObjectData::ToEncodedPrivateKey(
     const auto& pkey = GetAsymmetricKey();
     if (pkey.id() == EVP_PKEY_EC) {
       ECKeyPointer ec_key(pkey);
-      CHECK(ec_key);
+      if (!ec_key) {
+        THROW_ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS(env);
+        return false;
+      }
       const BIGNUM* private_key = ec_key.getPrivateKey();
-      CHECK_NOT_NULL(private_key);
+      if (private_key == nullptr) {
+        THROW_ERR_CRYPTO_OPERATION_FAILED(env, "Failed to get EC private key");
+        return false;
+      }
       const auto group = ec_key.getGroup();
       auto order = BignumPointer::New();
-      CHECK(order);
-      CHECK(EC_GROUP_get_order(group, order.get(), nullptr));
+      if (!order || !EC_GROUP_get_order(group, order.get(), nullptr)) {
+        THROW_ERR_CRYPTO_OPERATION_FAILED(env,
+                                          "Failed to export EC private key");
+        return false;
+      }
       auto buf = BignumPointer::EncodePadded(private_key, order.byteLength());
       if (!buf) {
         THROW_ERR_CRYPTO_OPERATION_FAILED(env,
@@ -1463,7 +1475,9 @@ void KeyObjectHandle::ExportECPublicRaw(
   }
 
   ECKeyPointer ec_key(m_pkey);
-  CHECK(ec_key);
+  if (!ec_key) {
+    return THROW_ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS(env);
+  }
 
   CHECK(args[0]->IsInt32());
   auto form =
@@ -1494,15 +1508,22 @@ void KeyObjectHandle::ExportECPrivateRaw(
   }
 
   ECKeyPointer ec_key(m_pkey);
-  CHECK(ec_key);
+  if (!ec_key) {
+    return THROW_ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS(env);
+  }
 
   const BIGNUM* private_key = ec_key.getPrivateKey();
-  CHECK_NOT_NULL(private_key);
+  if (private_key == nullptr) {
+    return THROW_ERR_CRYPTO_OPERATION_FAILED(env,
+                                             "Failed to get EC private key");
+  }
 
   const auto group = ec_key.getGroup();
   auto order = BignumPointer::New();
-  CHECK(order);
-  CHECK(EC_GROUP_get_order(group, order.get(), nullptr));
+  if (!order || !EC_GROUP_get_order(group, order.get(), nullptr)) {
+    return THROW_ERR_CRYPTO_OPERATION_FAILED(env,
+                                             "Failed to export EC private key");
+  }
 
   auto buf = BignumPointer::EncodePadded(private_key, order.byteLength());
   if (!buf) {
diff --git a/test/parallel/test-crypto-dh-curves.js b/test/parallel/test-crypto-dh-curves.js
@@ -5,7 +5,7 @@ if (!common.hasCrypto)
 
 const assert = require('assert');
 const crypto = require('crypto');
-const { hasOpenSSL3 } = require('../common/crypto');
+const { hasOpenSSL } = require('../common/crypto');
 
 // Second OAKLEY group, see
 // https://github.com/nodejs/node-v0.x-archive/issues/2338 and
@@ -22,7 +22,7 @@ const bad_dh = process.features.openssl_is_boringssl ?
   crypto.createDiffieHellman('02', 'hex');
 assert.notStrictEqual(bad_dh.verifyError, 0);
 
-if (hasOpenSSL3) {
+if (hasOpenSSL(3)) {
   const smallSafePrime = crypto.createDiffieHellman(
     Buffer.from([23]), Buffer.from([2]));
   assert.notStrictEqual(smallSafePrime.verifyError, 0);
diff --git a/test/parallel/test-tls-connect-secure-context.js b/test/parallel/test-tls-connect-secure-context.js
@@ -23,6 +23,9 @@ connect({
   return cleanup();
 }));
 
+// Invalid dhparam input is silently discarded for compatibility.
+tls.createSecureContext({ dhparam: fixtures.readKey('ec-key.pem') });
+
 connect({
   client: {
     servername: 'agent1',

Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,11 @@ template("ncrypto_gn_build") {`
`26`	`26`	`source_set(target_name) {`
`27`	`27`	`forward_variables_from(invoker, "*")`
`28`	`28`	`public_configs = [ ":ncrypto_config" ]`
`29`		`- sources = gypi_values.ncrypto_sources`
	`29`	`+ defines = [`
	`30`	`+ "NCRYPTO_ENGINE_COMPAT=1",`
	`31`	`+ "OPENSSL_SUPPRESS_DEPRECATED",`
	`32`	`+ ]`
	`33`	`+ sources = gypi_values.ncrypto_sources + gypi_values.ncrypto_engine_sources`
`30`	`34`	`deps = [ "$node_openssl_path" ]`
`31`	`35`	`}`
`32`	`36`	`}`