crypto: remove obsolete BoringSSL shims and use available APIs

deps/ncrypto/ncrypto.cc

@@ -396,15 +396,7 @@ std::optional<std::string> CryptoErrorList::pop_front() {
 
 // ============================================================================
 DataPointer DataPointer::Alloc(size_t len) {
-#ifdef OPENSSL_IS_BORINGSSL
-  // Boringssl does not implement OPENSSL_zalloc
-  auto ptr = OPENSSL_malloc(len);
-  if (ptr == nullptr) return {};
-  memset(ptr, 0, len);
-  return DataPointer(ptr, len);
-#else
   return DataPointer(OPENSSL_zalloc(len), len);
-#endif
 }
 
 DataPointer DataPointer::SecureAlloc(size_t len) {
@@ -427,18 +419,11 @@ DataPointer DataPointer::SecureAlloc(size_t len) {
 }
 
 size_t DataPointer::GetSecureHeapUsed() {
-#ifndef OPENSSL_IS_BORINGSSL
   return CRYPTO_secure_malloc_initialized() ? CRYPTO_secure_used() : 0;
-#else
-  // BoringSSL does not have the secure heap and therefore
-  // will always return 0.
-  return 0;
-#endif
 }
 
 DataPointer::InitSecureHeapResult DataPointer::TryInitSecureHeap(size_t amount,
                                                                  size_t min) {
-#ifndef OPENSSL_IS_BORINGSSL
   switch (CRYPTO_secure_malloc_init(amount, min)) {
     case 0:
       return InitSecureHeapResult::FAILED;
@@ -449,10 +434,6 @@ DataPointer::InitSecureHeapResult DataPointer::TryInitSecureHeap(size_t amount,
     default:
       return InitSecureHeapResult::FAILED;
   }
-#else
-  // BoringSSL does not actually support the secure heap
-  return InitSecureHeapResult::FAILED;
-#endif
 }
 
 DataPointer DataPointer::Copy(const Buffer<const void>& buffer) {
@@ -580,12 +561,7 @@ BignumPointer BignumPointer::New() {
 }
 
 BignumPointer BignumPointer::NewSecure() {
-#ifdef OPENSSL_IS_BORINGSSL
-  // Boringssl does not implement BN_secure_new.
-  return New();
-#else
   return BignumPointer(BN_secure_new());
-#endif
 }
 
 BignumPointer& BignumPointer::operator=(BignumPointer&& other) noexcept {
@@ -2276,14 +2252,11 @@ DHPointer::CheckPublicKeyResult DHPointer::checkPublicKey(
   if (DH_check_pub_key(dh_.get(), pub_key.get(), &codes) != 1) {
     return DHPointer::CheckPublicKeyResult::CHECK_FAILED;
   }
-#ifndef OPENSSL_IS_BORINGSSL
-  // Boringssl does not define DH_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
   if (codes & DH_CHECK_PUBKEY_TOO_SMALL) {
     return DHPointer::CheckPublicKeyResult::TOO_SMALL;
   } else if (codes & DH_CHECK_PUBKEY_TOO_LARGE) {
     return DHPointer::CheckPublicKeyResult::TOO_LARGE;
   }
-#endif
   if (codes != 0) {
     return DHPointer::CheckPublicKeyResult::INVALID;
   }
@@ -4288,59 +4261,6 @@ std::optional<uint32_t> SSLPointer::verifyPeerCertificate() const {
   return std::nullopt;
 }
 
-const char* SSLPointer::getClientHelloAlpn() const {
-  if (ssl_ == nullptr) return {};
-#ifndef OPENSSL_IS_BORINGSSL
-  const unsigned char* buf;
-  size_t len;
-  size_t rem;
-
-  if (!SSL_client_hello_get0_ext(
-          get(),
-          TLSEXT_TYPE_application_layer_protocol_negotiation,
-          &buf,
-          &rem) ||
-      rem < 2) {
-    return {};
-  }
-
-  len = (buf[0] << 8) | buf[1];
-  if (len + 2 != rem) return {};
-  return reinterpret_cast<const char*>(buf + 3);
-#else
-  // Boringssl doesn't have a public API for this.
-  return {};
-#endif
-}
-
-const char* SSLPointer::getClientHelloServerName() const {
-  if (ssl_ == nullptr) return {};
-#ifndef OPENSSL_IS_BORINGSSL
-  const unsigned char* buf;
-  size_t len;
-  size_t rem;
-
-  if (!SSL_client_hello_get0_ext(get(), TLSEXT_TYPE_server_name, &buf, &rem) ||
-      rem <= 2) {
-    return {};
-  }
-
-  len = (*buf << 8) | *(buf + 1);
-  if (len + 2 != rem) return {};
-  rem = len;
-
-  if (rem == 0 || *(buf + 2) != TLSEXT_NAMETYPE_host_name) return {};
-  rem--;
-  if (rem <= 2) return {};
-  len = (*(buf + 3) << 8) | *(buf + 4);
-  if (len + 2 > rem) return {};
-  return reinterpret_cast<const char*>(buf + 5);
-#else
-  // Boringssl doesn't have a public API for this.
-  return {};
-#endif
-}
-
 std::optional<const std::string_view> SSLPointer::GetServerName(
     const SSL* ssl) {
   if (ssl == nullptr) return std::nullopt;
@@ -4386,6 +4306,13 @@ std::optional<std::string_view> SSLPointer::getNegotiatedGroup() const {
   const char* group = SSL_get0_group_name(get());
   if (group == nullptr) return std::nullopt;
   return group;
+#elif defined(OPENSSL_IS_BORINGSSL)
+  if (!ssl_) return std::nullopt;
+  const int nid = SSL_get_negotiated_group(get());
+  if (nid == NID_undef) return std::nullopt;
+  const char* group = OBJ_nid2sn(nid);
+  if (group == nullptr) return std::nullopt;
+  return group;
 #else
   return std::nullopt;
 #endif
@@ -4410,19 +4337,17 @@ std::optional<std::string_view> SSLPointer::getCipherVersion() const {
 }
 
 std::optional<int> SSLPointer::getSecurityLevel() {
-#ifndef OPENSSL_IS_BORINGSSL
   auto ctx = SSLCtxPointer::New();
   if (!ctx) return std::nullopt;
 
+#ifdef OPENSSL_IS_BORINGSSL
+  return SSL_CTX_get_security_level(ctx.get());
+#else
   auto ssl = SSLPointer::New(ctx);
   if (!ssl) return std::nullopt;
 
   return SSL_get_security_level(ssl);
-#else
-  // OPENSSL_TLS_SECURITY_LEVEL is not defined in BoringSSL
-  // so assume it is the default OPENSSL_TLS_SECURITY_LEVEL value.
-  return 1;
-#endif  // OPENSSL_IS_BORINGSSL
+#endif
 }
 
 SSLCtxPointer::SSLCtxPointer(SSL_CTX* ctx) : ctx_(ctx) {}

deps/ncrypto/ncrypto.h

@@ -1231,9 +1231,9 @@ class DHPointer final {
     UNABLE_TO_CHECK_GENERATOR = 0x04,
     NOT_SUITABLE_GENERATOR = 0x08,
     Q_NOT_PRIME = 0x10,
-#ifndef OPENSSL_IS_BORINGSSL
-    // Boringssl does not define the DH_CHECK_INVALID_[Q or J]_VALUE
     INVALID_Q = 0x20,
+#ifndef OPENSSL_IS_BORINGSSL
+    // BoringSSL does not define DH_CHECK_INVALID_J_VALUE.
     INVALID_J = 0x40,
     MODULUS_TOO_SMALL = 0x80,
     MODULUS_TOO_LARGE = 0x100,
@@ -1244,14 +1244,9 @@ class DHPointer final {
 
   enum class CheckPublicKeyResult {
     NONE,
-#ifndef OPENSSL_IS_BORINGSSL
-    // Boringssl does not define DH_R_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
-    TOO_SMALL = DH_R_CHECK_PUBKEY_TOO_SMALL,
-    TOO_LARGE = DH_R_CHECK_PUBKEY_TOO_LARGE,
-    INVALID = DH_R_CHECK_PUBKEY_INVALID,
-#else
-    INVALID = DH_R_INVALID_PUBKEY,
-#endif
+    TOO_SMALL,
+    TOO_LARGE,
+    INVALID,
     CHECK_FAILED = 512,
   };
   // Check to see if the given public key is suitable for this DH instance.
@@ -1346,9 +1341,6 @@ class SSLPointer final {
   bool setSession(const SSLSessionPointer& session);
   bool setSniContext(const SSLCtxPointer& ctx) const;
 
-  const char* getClientHelloAlpn() const;
-  const char* getClientHelloServerName() const;
-
   std::optional<const std::string_view> getServerName() const;
   X509View getCertificate() const;
   EVPKeyPointer getPeerTempKey() const;

src/crypto/crypto_dh.cc

@@ -319,12 +319,10 @@ void ComputeSecret(const FunctionCallbackInfo<Value>& args) {
     case DHPointer::CheckPublicKeyResult::CHECK_FAILED:
       return THROW_ERR_CRYPTO_INVALID_KEYTYPE(env,
                                               "Unspecified validation error");
-#ifndef OPENSSL_IS_BORINGSSL
     case DHPointer::CheckPublicKeyResult::TOO_SMALL:
       return THROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too small");
     case DHPointer::CheckPublicKeyResult::TOO_LARGE:
       return THROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too large");
-#endif
     case DHPointer::CheckPublicKeyResult::INVALID:
       return THROW_ERR_CRYPTO_INVALID_KEYTYPE(env, "Supplied key is invalid");
     case DHPointer::CheckPublicKeyResult::NONE:

src/crypto/crypto_rsa.cc

@@ -137,18 +137,6 @@ Maybe<void> RsaKeyGenTraits::AdditionalConfig(
   params->params.modulus_bits = args[*offset + 1].As<Uint32>()->Value();
   params->params.exponent = args[*offset + 2].As<Uint32>()->Value();
 
-#ifdef OPENSSL_IS_BORINGSSL
-  // BoringSSL hangs indefinitely generating an RSA key with e=1, and for
-  // other invalid exponents (e=0, even values) reports the misleading error
-  // RSA_R_TOO_MANY_ITERATIONS only after running the full keygen loop. Reject
-  // those up-front with a clear error. The constraint here (odd integer >= 3)
-  // matches BoringSSL's own rsa_check_public_key validation.
-  if (params->params.exponent < 3 || (params->params.exponent & 1) == 0) {
-    THROW_ERR_OUT_OF_RANGE(env, "publicExponent is invalid");
-    return Nothing<void>();
-  }
-#endif
-
   *offset += 3;
 
   if (params->params.variant == kKeyVariantRSA_PSS) {

test/common/boringssl.js

@@ -137,12 +137,9 @@ function testRenegotiationUnsupported() {
 }
 
 /**
- * OpenSSL exposes the negotiated ephemeral key type, name, and size for TLS
- * clients. With BoringSSL the same ECDHE TLS 1.2 handshake succeeds, but
- * getEphemeralKeyInfo() returns null on the server side and an object whose
- * fields are undefined on the client side.
+ * BoringSSL exposes the negotiated TLS group but not the ephemeral key size.
  */
-function testEphemeralKeyInfoUnsupported() {
+function testEphemeralKeyInfo() {
   const server = tls.createServer({
     key: fixtures.readKey('agent2-key.pem'),
     cert: fixtures.readKey('agent2-cert.pem'),
@@ -161,8 +158,8 @@ function testEphemeralKeyInfoUnsupported() {
       maxVersion: 'TLSv1.2',
     }, common.mustCall(() => {
       assert.deepStrictEqual(client.getEphemeralKeyInfo(), {
-        type: undefined,
-        name: undefined,
+        type: 'TLSGroup',
+        name: 'prime256v1',
         size: undefined,
       });
       server.close();
@@ -337,7 +334,7 @@ module.exports = {
   assertMultiKeyUnsupported,
   assertNoCipherMatch,
   assertOpenSSLSecurityLevelsUnsupported,
-  testEphemeralKeyInfoUnsupported,
+  testEphemeralKeyInfo,
   testLegacyProtocolUnsupported,
   testMultiPfxSelectionDifference,
   testPskTls13Unsupported,

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

@@ -35,33 +35,33 @@ if (!process.features.openssl_is_boringssl) {
   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'
-    });
 }
 
+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) :

test/parallel/test-crypto-dh.js

@@ -93,9 +93,7 @@ const {
   {
     assert.throws(() => {
       dh3.computeSecret('');
-    }, { message: process.features.openssl_is_boringssl ?
-      'Supplied key is invalid' :
-      'Supplied key is too small' });
+    }, { message: 'Supplied key is too small' });
   }
 }
 

test/parallel/test-crypto-keygen.js

@@ -376,25 +376,20 @@ const isBoringSSL = process.features.openssl_is_boringssl;
   }
 
   // Test invalid exponents. (caught by OpenSSL)
+  let invalidExponentError = /bad e value/;
+  if (isBoringSSL) {
+    invalidExponentError = /BAD_E_VALUE/;
+  } else if (hasOpenSSL3) {
+    invalidExponentError = /exponent/;
+  }
   for (const publicExponent of [1, 1 + 0x10001]) {
-    if (isBoringSSL) {
-      assert.throws(() => generateKeyPair('rsa', {
-        modulusLength: 4096,
-        publicExponent
-      }, common.mustNotCall()), {
-        name: 'RangeError',
-        code: 'ERR_OUT_OF_RANGE',
-        message: 'publicExponent is invalid',
-      });
-    } else {
-      generateKeyPair('rsa', {
-        modulusLength: 4096,
-        publicExponent
-      }, common.mustCall((err) => {
-        assert.strictEqual(err.name, 'Error');
-        assert.match(err.message, hasOpenSSL3 ? /exponent/ : /bad e value/);
-      }));
-    }
+    generateKeyPair('rsa', {
+      modulusLength: 4096,
+      publicExponent
+    }, common.mustCall((err) => {
+      assert.strictEqual(err.name, 'Error');
+      assert.match(err.message, invalidExponentError);
+    }));
   }
 }
 

test/parallel/test-crypto-sec-level.js

@@ -15,4 +15,8 @@ const assert = require('assert');
 // This test simply validates that we can get some value for the secLevel
 // when needed by tests.
 const secLevel = require('internal/crypto/util').getOpenSSLSecLevel();
-assert.ok(secLevel >= 0 && secLevel <= 5);
+if (process.features.openssl_is_boringssl) {
+  assert.strictEqual(secLevel, 0);
+} else {
+  assert.ok(secLevel >= 0 && secLevel <= 5);
+}