diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc index d731e823b39d..089465ad5b95 100644 --- a/deps/ncrypto/ncrypto.cc +++ b/deps/ncrypto/ncrypto.cc @@ -396,15 +396,7 @@ std::optional 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& 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 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(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(buf + 5); -#else - // Boringssl doesn't have a public API for this. - return {}; -#endif -} - std::optional SSLPointer::GetServerName( const SSL* ssl) { if (ssl == nullptr) return std::nullopt; @@ -4386,6 +4306,13 @@ std::optional 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 SSLPointer::getCipherVersion() const { } std::optional 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) {} diff --git a/deps/ncrypto/ncrypto.h b/deps/ncrypto/ncrypto.h index a11d67ae460a..26b359d1f231 100644 --- a/deps/ncrypto/ncrypto.h +++ b/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 getServerName() const; X509View getCertificate() const; EVPKeyPointer getPeerTempKey() const; diff --git a/src/crypto/crypto_dh.cc b/src/crypto/crypto_dh.cc index 92780cfeeebf..7fbaf4fff5d9 100644 --- a/src/crypto/crypto_dh.cc +++ b/src/crypto/crypto_dh.cc @@ -319,12 +319,10 @@ void ComputeSecret(const FunctionCallbackInfo& 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: diff --git a/src/crypto/crypto_rsa.cc b/src/crypto/crypto_rsa.cc index c89d6b698b2b..15479284933d 100644 --- a/src/crypto/crypto_rsa.cc +++ b/src/crypto/crypto_rsa.cc @@ -137,18 +137,6 @@ Maybe RsaKeyGenTraits::AdditionalConfig( params->params.modulus_bits = args[*offset + 1].As()->Value(); params->params.exponent = args[*offset + 2].As()->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(); - } -#endif - *offset += 3; if (params->params.variant == kKeyVariantRSA_PSS) { diff --git a/test/common/boringssl.js b/test/common/boringssl.js index e6e91387c304..ab7f0505b0ce 100644 --- a/test/common/boringssl.js +++ b/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, diff --git a/test/parallel/test-crypto-dh-curves.js b/test/parallel/test-crypto-dh-curves.js index c2449a292894..ee8849163ae8 100644 --- a/test/parallel/test-crypto-dh-curves.js +++ b/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) : diff --git a/test/parallel/test-crypto-dh.js b/test/parallel/test-crypto-dh.js index dc55c5226efb..f5d239f1d457 100644 --- a/test/parallel/test-crypto-dh.js +++ b/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' }); } } diff --git a/test/parallel/test-crypto-keygen.js b/test/parallel/test-crypto-keygen.js index 1a616ed5d11f..c525a54aa434 100644 --- a/test/parallel/test-crypto-keygen.js +++ b/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); + })); } } diff --git a/test/parallel/test-crypto-sec-level.js b/test/parallel/test-crypto-sec-level.js index d7d2252be6c3..f2c0e3900624 100644 --- a/test/parallel/test-crypto-sec-level.js +++ b/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); +} diff --git a/test/parallel/test-tls-client-getephemeralkeyinfo.js b/test/parallel/test-tls-client-getephemeralkeyinfo.js index db41fdf6a098..638df3ff0141 100644 --- a/test/parallel/test-tls-client-getephemeralkeyinfo.js +++ b/test/parallel/test-tls-client-getephemeralkeyinfo.js @@ -4,7 +4,7 @@ if (!common.hasCrypto) common.skip('missing crypto'); if (process.features.openssl_is_boringssl) { - require('../common/boringssl').testEphemeralKeyInfoUnsupported(); + require('../common/boringssl').testEphemeralKeyInfo(); return; } diff --git a/test/parallel/test-webcrypto-export-import-ml-kem.js b/test/parallel/test-webcrypto-export-import-ml-kem.js index a3b1b3fe7730..cd05224969c8 100644 --- a/test/parallel/test-webcrypto-export-import-ml-kem.js +++ b/test/parallel/test-webcrypto-export-import-ml-kem.js @@ -107,10 +107,8 @@ async function testImportPkcs8({ name, privateUsages }, extractable) { } catch (err) { if (process.features.openssl_is_boringssl) { assert.strictEqual(err.name, 'DataError'); - // It should really only be ERR_OSSL_EVP_PRIVATE_KEY_WAS_NOT_SEED - // but BoringSSL is inconsistent between handling ML-KEM and ML-DSA - // Fixed in https://github.com/google/boringssl/commit/94c4c7f9e0eeeff72ea1ac6abf1aed5bd2a82c0c - assert.match(err.cause.code, /ERR_OSSL_EVP_UNSUPPORTED_ALGORITHM|ERR_OSSL_EVP_PRIVATE_KEY_WAS_NOT_SEED/); + assert.strictEqual(err.cause.code, + 'ERR_OSSL_EVP_PRIVATE_KEY_WAS_NOT_SEED'); common.printSkipMessage('Skipping unsupported private key format test'); return; }