From 315b6433842d3c8de22211292ed2578e379fc534 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 14 Jan 2025 19:18:10 +0000 Subject: [PATCH 1/4] crypto: add api to get openssl security level Distros may compile with a different openssl security level than the default. In addition there has been some discussion with respect to shipping with a different default security security level in different Node.js versions in order to main stabilty. Exposing the default openssl security level with let us have tests that work in these situations as well as allow applications to better cope with the avialable crypto algorithms. - add API to get openssl security level - modify one test to use security level instead of openssl version as an example Signed-off-by: Michael Dawson --- lib/internal/crypto/util.js | 2 ++ src/crypto/crypto_util.cc | 27 ++++++++++++++++++++++++++ test/parallel/test-crypto-sec-level.js | 11 +++++++++++ test/parallel/test-tls-dhe.js | 10 +++++++--- 4 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-crypto-sec-level.js diff --git a/lib/internal/crypto/util.js b/lib/internal/crypto/util.js index 3cf6599a186c3f..b2fa04c6691539 100644 --- a/lib/internal/crypto/util.js +++ b/lib/internal/crypto/util.js @@ -32,6 +32,7 @@ const { setEngine: _setEngine, secureHeapUsed: _secureHeapUsed, getCachedAliases, + getOpenSSLSecLevelCrypto: getOpenSSLSecLevel, } = internalBinding('crypto'); const { getOptionValue } = require('internal/options'); @@ -631,4 +632,5 @@ module.exports = { secureHeapUsed, getCachedHashId, getHashCache, + getOpenSSLSecLevel, }; diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index 61f8cbf6703b50..55f23f024d2b52 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -31,6 +31,8 @@ using ncrypto::BIOPointer; using ncrypto::CryptoErrorList; using ncrypto::EnginePointer; using ncrypto::EVPKeyCtxPointer; +using ncrypto::SSLCtxPointer; +using ncrypto::SSLPointer; using v8::ArrayBuffer; using v8::BackingStore; using v8::BigInt; @@ -201,6 +203,27 @@ void TestFipsCrypto(const v8::FunctionCallbackInfo& args) { args.GetReturnValue().Set(ncrypto::testFipsEnabled() ? 1 : 0); } +void GetOpenSSLSecLevelCrypto(const FunctionCallbackInfo& args) { + // for BoringSSL assume the same as the default + int sec_level = 1; +#ifndef OPENSSL_IS_BORINGSSL + Environment* env = Environment::GetCurrent(args); + + auto ctx = SSLCtxPointer::New(); + if (!ctx) { + return ThrowCryptoError(env, ERR_get_error(), "SSL_CTX_new"); + } + + auto ssl = SSLPointer::New(ctx); + if (!ssl) { + return ThrowCryptoError(env, ERR_get_error(), "SSL_new"); + } + + sec_level = SSL_get_security_level(ssl); +#endif // OPENSSL_IS_BORINGSSL + args.GetReturnValue().Set(sec_level); +} + void CryptoErrorStore::Capture() { errors_.clear(); while (const uint32_t err = ERR_get_error()) { @@ -699,6 +722,9 @@ void Initialize(Environment* env, Local target) { SetMethod(context, target, "secureBuffer", SecureBuffer); SetMethod(context, target, "secureHeapUsed", SecureHeapUsed); + + SetMethodNoSideEffect( + context, target, "getOpenSSLSecLevelCrypto", GetOpenSSLSecLevelCrypto); } void RegisterExternalReferences(ExternalReferenceRegistry* registry) { #ifndef OPENSSL_NO_ENGINE @@ -710,6 +736,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(TestFipsCrypto); registry->Register(SecureBuffer); registry->Register(SecureHeapUsed); + registry->Register(GetOpenSSLSecLevelCrypto); } } // namespace Util diff --git a/test/parallel/test-crypto-sec-level.js b/test/parallel/test-crypto-sec-level.js new file mode 100644 index 00000000000000..b48c308ba3d17f --- /dev/null +++ b/test/parallel/test-crypto-sec-level.js @@ -0,0 +1,11 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); + +const secLevel = require('internal/crypto/util').getOpenSSLSecLevel(); +assert.ok(secLevel >= 0 && secLevel <= 5); diff --git a/test/parallel/test-tls-dhe.js b/test/parallel/test-tls-dhe.js index 25b58191e1d413..f7296eceafd3d2 100644 --- a/test/parallel/test-tls-dhe.js +++ b/test/parallel/test-tls-dhe.js @@ -1,4 +1,4 @@ -// Flags: --no-warnings +// Flags: --no-warnings --expose-internals // Copyright Joyent, Inc. and other Node contributors. // // Permission is hereby granted, free of charge, to any person obtaining a @@ -31,6 +31,8 @@ const { opensslCli, } = require('../common/crypto'); +const secLevel = require('internal/crypto/util').getOpenSSLSecLevel(); + if (!opensslCli) { common.skip('missing openssl-cli'); } @@ -50,7 +52,7 @@ const dheCipher = 'DHE-RSA-AES128-SHA256'; const ecdheCipher = 'ECDHE-RSA-AES128-SHA256'; const ciphers = `${dheCipher}:${ecdheCipher}`; -if (!hasOpenSSL(3, 2)) { +if (secLevel < 2) { // Test will emit a warning because the DH parameter size is < 2048 bits // when the test is run on versions lower than OpenSSL32 common.expectWarning('SecurityWarning', @@ -114,7 +116,9 @@ function testCustomParam(keylen, expectedCipher) { }, /DH parameter is less than 1024 bits/); // Custom DHE parameters are supported (but discouraged). - if (!hasOpenSSL(3, 2)) { + // 1024 is disallowed at security level 2 and above so use 3072 instead + // for higher security levels + if (secLevel < 2) { await testCustomParam(1024, dheCipher); } else { await testCustomParam(3072, dheCipher); From fb76bd0bb97c090ada6e98611f84852e4c1ac482 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 21 Jan 2025 10:25:37 -0500 Subject: [PATCH 2/4] Update src/crypto/crypto_util.cc --- src/crypto/crypto_util.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index 55f23f024d2b52..deedd6ce55d21e 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -205,7 +205,7 @@ void TestFipsCrypto(const v8::FunctionCallbackInfo& args) { void GetOpenSSLSecLevelCrypto(const FunctionCallbackInfo& args) { // for BoringSSL assume the same as the default - int sec_level = 1; + int sec_level = OPENSSL_TLS_SECURITY_LEVEL; #ifndef OPENSSL_IS_BORINGSSL Environment* env = Environment::GetCurrent(args); From 0b49c32a3b3288794ba2ea7f86afaa46861ba0a4 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Wed, 22 Jan 2025 20:13:35 +0000 Subject: [PATCH 3/4] squash: address comments Signed-off-by: Michael Dawson --- test/parallel/test-crypto-sec-level.js | 7 +++++++ test/parallel/test-tls-dhe.js | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/test/parallel/test-crypto-sec-level.js b/test/parallel/test-crypto-sec-level.js index b48c308ba3d17f..017db3e361e8bf 100644 --- a/test/parallel/test-crypto-sec-level.js +++ b/test/parallel/test-crypto-sec-level.js @@ -7,5 +7,12 @@ if (!common.hasCrypto) const assert = require('assert'); +// OpenSSL has a set of security levels which affect what algorithms +// are available by default. Different OpenSSL veresions have different +// default security levels and we use this value to adjust what a test +// expects based on the security level. You can read more in +// https://docs.openssl.org/1.1.1/man3/SSL_CTX_set_security_level/#default-callback-behaviour +// 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); diff --git a/test/parallel/test-tls-dhe.js b/test/parallel/test-tls-dhe.js index f7296eceafd3d2..da46c75ef30ce3 100644 --- a/test/parallel/test-tls-dhe.js +++ b/test/parallel/test-tls-dhe.js @@ -31,6 +31,11 @@ const { opensslCli, } = require('../common/crypto'); +// OpenSSL has a set of security levels which affect what algorithms +// are available by default. Different OpenSSL veresions have different +// default security levels and we use this value to adjust what a test +// expects based on the security level. You can read more in +// https://docs.openssl.org/1.1.1/man3/SSL_CTX_set_security_level/#default-callback-behaviour const secLevel = require('internal/crypto/util').getOpenSSLSecLevel(); if (!opensslCli) { From c68d14db380cf57aebdb34b7eeaca73267929100 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Mon, 27 Jan 2025 22:28:24 +0000 Subject: [PATCH 4/4] squash: address linter Signed-off-by: Michael Dawson --- test/parallel/test-tls-dhe.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/parallel/test-tls-dhe.js b/test/parallel/test-tls-dhe.js index da46c75ef30ce3..b4f0337c43fa57 100644 --- a/test/parallel/test-tls-dhe.js +++ b/test/parallel/test-tls-dhe.js @@ -27,7 +27,6 @@ if (!common.hasCrypto) { } const { - hasOpenSSL, opensslCli, } = require('../common/crypto');