diff --git a/src/crypto/crypto_common.cc b/src/crypto/crypto_common.cc index d005bf0ffb9344..4f117f160a9673 100644 --- a/src/crypto/crypto_common.cc +++ b/src/crypto/crypto_common.cc @@ -61,7 +61,7 @@ MaybeLocal GetValidationErrorReason(Environment* env, int err) { (err == X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE) || (err == X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT) || ((err == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT) && - !per_process::cli_options->use_system_ca); + !env->options()->use_system_ca); if (suggest_system_ca) { reason.append("; if the root CA is installed locally, " diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc index 0f97f8884c06fe..f3c9e90d295a80 100644 --- a/src/crypto/crypto_context.cc +++ b/src/crypto/crypto_context.cc @@ -84,6 +84,7 @@ static std::string extra_root_certs_file; // NOLINT(runtime/string) static std::atomic has_cached_bundled_root_certs{false}; static std::atomic has_cached_system_root_certs{false}; static std::atomic has_cached_extra_root_certs{false}; +static std::atomic has_use_system_ca{false}; // Used for sets of X509. struct X509Less { @@ -101,11 +102,11 @@ static thread_local X509_STORE* root_cert_store = nullptr; // from this set. static thread_local std::unique_ptr root_certs_from_users; -X509_STORE* GetOrCreateRootCertStore() { +X509_STORE* GetOrCreateRootCertStore(Environment* env) { if (root_cert_store != nullptr) { return root_cert_store; } - root_cert_store = NewRootCertStore(); + root_cert_store = NewRootCertStore(env); return root_cert_store; } @@ -873,7 +874,7 @@ static void LoadCACertificates(void* data) { { Mutex::ScopedLock cli_lock(node::per_process::cli_options_mutex); - if (!per_process::cli_options->use_system_ca) { + if (!has_use_system_ca.load()) { return; } } @@ -917,6 +918,8 @@ void StartLoadingCertificatesOffThread( return; } tried_cert_loading_off_thread.store(true); + Environment* env = Environment::GetCurrent(args); + has_use_system_ca.store(env != nullptr && env->options()->use_system_ca); int r = uv_thread_create(&cert_loading_thread, LoadCACertificates, nullptr); cert_loading_thread_started.store(r == 0); if (r != 0) { @@ -947,13 +950,13 @@ void StartLoadingCertificatesOffThread( // with all the other flags. // 7. Certificates from --use-bundled-ca, --use-system-ca and // NODE_EXTRA_CA_CERTS are cached after first load. Certificates -// from --use-system-ca are not cached and always reloaded from +// from --use-openssl-ca are not cached and always reloaded from // disk. // 8. If users have reset the root cert store by calling // tls.setDefaultCACertificates(), the store will be populated with // the certificates provided by users. // TODO(joyeecheung): maybe these rules need a bit of consolidation? -X509_STORE* NewRootCertStore() { +X509_STORE* NewRootCertStore(Environment* env) { X509_STORE* store = X509_STORE_new(); CHECK_NOT_NULL(store); @@ -975,14 +978,24 @@ X509_STORE* NewRootCertStore() { } #endif - Mutex::ScopedLock cli_lock(node::per_process::cli_options_mutex); + bool use_system_ca = false; + { + Mutex::ScopedLock cli_lock(node::per_process::cli_options_mutex); + if (env != nullptr) { + use_system_ca = env->options()->use_system_ca; + } else if (per_process::cli_options->per_isolate != nullptr && + per_process::cli_options->per_isolate->per_env != nullptr) { + use_system_ca = + per_process::cli_options->per_isolate->per_env->use_system_ca; + } + } if (per_process::cli_options->ssl_openssl_cert_store) { CHECK_EQ(1, X509_STORE_set_default_paths(store)); } else { for (X509* cert : GetBundledRootCertificates()) { CHECK_EQ(1, X509_STORE_add_cert(store, cert)); } - if (per_process::cli_options->use_system_ca) { + if (use_system_ca) { for (X509* cert : GetSystemStoreCACertificates()) { CHECK_EQ(1, X509_STORE_add_cert(store, cert)); } @@ -1189,7 +1202,7 @@ void ResetRootCertStore(const FunctionCallbackInfo& args) { // TODO(joyeecheung): we can probably just reset it to nullptr // and let the next call to NewRootCertStore() create a new one. - root_cert_store = NewRootCertStore(); + root_cert_store = nullptr; } void GetSystemCACertificates(const FunctionCallbackInfo& args) { @@ -1700,11 +1713,12 @@ void SecureContext::SetX509StoreFlag(unsigned long flags) { } X509_STORE* SecureContext::GetCertStoreOwnedByThisSecureContext() { + Environment* env = this->env(); if (own_cert_store_cache_ != nullptr) return own_cert_store_cache_; X509_STORE* cert_store = SSL_CTX_get_cert_store(ctx_.get()); - if (cert_store == GetOrCreateRootCertStore()) { - cert_store = NewRootCertStore(); + if (cert_store == GetOrCreateRootCertStore(env)) { + cert_store = NewRootCertStore(env); SSL_CTX_set_cert_store(ctx_.get(), cert_store); } @@ -1777,7 +1791,8 @@ void SecureContext::AddCRL(const FunctionCallbackInfo& args) { void SecureContext::SetRootCerts() { ClearErrorOnReturn clear_error_on_return; - auto store = GetOrCreateRootCertStore(); + Environment* env = this->env(); + auto store = GetOrCreateRootCertStore(env); // Increment reference count so global store is not deleted along with CTX. X509_STORE_up_ref(store); diff --git a/src/crypto/crypto_context.h b/src/crypto/crypto_context.h index b6801fc0b40708..a9cf0113a8bc29 100644 --- a/src/crypto/crypto_context.h +++ b/src/crypto/crypto_context.h @@ -19,9 +19,9 @@ constexpr int kMaxSupportedVersion = TLS1_3_VERSION; void GetRootCertificates( const v8::FunctionCallbackInfo& args); -X509_STORE* NewRootCertStore(); +X509_STORE* NewRootCertStore(Environment* env); -X509_STORE* GetOrCreateRootCertStore(); +X509_STORE* GetOrCreateRootCertStore(Environment* env); ncrypto::BIOPointer LoadBIO(Environment* env, v8::Local v); diff --git a/src/node.cc b/src/node.cc index a18660d388be9d..5b6402a3ae9cd0 100644 --- a/src/node.cc +++ b/src/node.cc @@ -871,15 +871,6 @@ static ExitCode InitializeNodeWithArgsInternal( // default value. V8::SetFlagsFromString("--rehash-snapshot"); -#if HAVE_OPENSSL - // TODO(joyeecheung): make this a per-env option and move the normalization - // into HandleEnvOptions. - std::string use_system_ca; - if (credentials::SafeGetenv("NODE_USE_SYSTEM_CA", &use_system_ca) && - use_system_ca == "1") { - per_process::cli_options->use_system_ca = true; - } -#endif // HAVE_OPENSSL HandleEnvOptions(per_process::cli_options->per_isolate->per_env); std::string node_options; diff --git a/src/node_options.cc b/src/node_options.cc index 5ae511a52ad3d2..5ed0e9756c5006 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -1016,6 +1016,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { &EnvironmentOptions::trace_env_native_stack, kAllowedInEnvvar); + AddOption("--use-system-ca", + "use system's CA store", + &EnvironmentOptions::use_system_ca, + kAllowedInEnvvar); + AddOption( "--trace-require-module", "Print access to require(esm). Options are 'all' (print all usage) and " @@ -1356,10 +1361,6 @@ PerProcessOptionsParser::PerProcessOptionsParser( , &PerProcessOptions::use_openssl_ca, kAllowedInEnvvar); - AddOption("--use-system-ca", - "use system's CA store", - &PerProcessOptions::use_system_ca, - kAllowedInEnvvar); AddOption("--use-bundled-ca", "use bundled CA store" #if !defined(NODE_OPENSSL_CERT_STORE) @@ -2098,6 +2099,10 @@ void HandleEnvOptions(std::shared_ptr env_options, env_options->use_env_proxy = opt_getter("NODE_USE_ENV_PROXY") == "1"; +#if HAVE_OPENSSL + env_options->use_system_ca = opt_getter("NODE_USE_SYSTEM_CA") == "1"; +#endif // HAVE_OPENSSL + if (env_options->redirect_warnings.empty()) env_options->redirect_warnings = opt_getter("NODE_REDIRECT_WARNINGS"); } diff --git a/src/node_options.h b/src/node_options.h index 2d30c115dcb27e..67d8ef45e34ac5 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -221,6 +221,7 @@ class EnvironmentOptions : public Options { bool trace_env = false; bool trace_env_js_stack = false; bool trace_env_native_stack = false; + bool use_system_ca = false; std::string trace_require_module; bool extra_info_on_fatal_exception = true; std::string unhandled_rejections; @@ -357,7 +358,6 @@ class PerProcessOptions : public Options { bool ssl_openssl_cert_store = false; #endif bool use_openssl_ca = false; - bool use_system_ca = false; bool use_bundled_ca = false; bool enable_fips_crypto = false; bool force_fips_crypto = false; diff --git a/src/quic/endpoint.cc b/src/quic/endpoint.cc index 1edbfe8f32cc37..09ddd43a6fc977 100644 --- a/src/quic/endpoint.cc +++ b/src/quic/endpoint.cc @@ -895,7 +895,7 @@ void Endpoint::Listen(const Session::Options& options) { "not what you want."); } - auto context = TLSContext::CreateServer(options.tls_options); + auto context = TLSContext::CreateServer(env(), options.tls_options); if (!*context) { THROW_ERR_INVALID_STATE( env(), "Failed to create TLS context: %s", context->validation_error()); @@ -928,7 +928,7 @@ BaseObjectPtr Endpoint::Connect( config, session_ticket.has_value() ? "yes" : "no"); - auto tls_context = TLSContext::CreateClient(options.tls_options); + auto tls_context = TLSContext::CreateClient(env(), options.tls_options); if (!*tls_context) { THROW_ERR_INVALID_STATE(env(), "Failed to create TLS context: %s", diff --git a/src/quic/tlscontext.cc b/src/quic/tlscontext.cc index 7e7c286b135893..63b841378a500e 100644 --- a/src/quic/tlscontext.cc +++ b/src/quic/tlscontext.cc @@ -293,16 +293,18 @@ bool OSSLContext::ConfigureClient() const { // ============================================================================ -std::shared_ptr TLSContext::CreateClient(const Options& options) { - return std::make_shared(Side::CLIENT, options); +std::shared_ptr TLSContext::CreateClient(Environment* env, + const Options& options) { + return std::make_shared(env, Side::CLIENT, options); } -std::shared_ptr TLSContext::CreateServer(const Options& options) { - return std::make_shared(Side::SERVER, options); +std::shared_ptr TLSContext::CreateServer(Environment* env, + const Options& options) { + return std::make_shared(env, Side::SERVER, options); } -TLSContext::TLSContext(Side side, const Options& options) - : side_(side), options_(options), ctx_(Initialize()) {} +TLSContext::TLSContext(Environment* env, Side side, const Options& options) + : side_(side), options_(options), env_(env), ctx_(Initialize()) {} TLSContext::operator SSL_CTX*() const { DCHECK(ctx_); @@ -460,14 +462,14 @@ SSLCtxPointer TLSContext::Initialize() { { ClearErrorOnReturn clear_error_on_return; if (options_.ca.empty()) { - auto store = crypto::GetOrCreateRootCertStore(); + auto store = crypto::GetOrCreateRootCertStore(env_); X509_STORE_up_ref(store); SSL_CTX_set_cert_store(ctx.get(), store); } else { for (const auto& ca : options_.ca) { uv_buf_t buf = ca; if (buf.len == 0) { - auto store = crypto::GetOrCreateRootCertStore(); + auto store = crypto::GetOrCreateRootCertStore(env_); X509_STORE_up_ref(store); SSL_CTX_set_cert_store(ctx.get(), store); } else { @@ -477,8 +479,8 @@ SSLCtxPointer TLSContext::Initialize() { while ( auto x509 = X509Pointer(PEM_read_bio_X509_AUX( bio.get(), nullptr, crypto::NoPasswordCallback, nullptr))) { - if (cert_store == crypto::GetOrCreateRootCertStore()) { - cert_store = crypto::NewRootCertStore(); + if (cert_store == crypto::GetOrCreateRootCertStore(env_)) { + cert_store = crypto::NewRootCertStore(env_); SSL_CTX_set_cert_store(ctx.get(), cert_store); } CHECK_EQ(1, X509_STORE_add_cert(cert_store, x509.get())); @@ -535,8 +537,8 @@ SSLCtxPointer TLSContext::Initialize() { } X509_STORE* cert_store = SSL_CTX_get_cert_store(ctx.get()); - if (cert_store == crypto::GetOrCreateRootCertStore()) { - cert_store = crypto::NewRootCertStore(); + if (cert_store == crypto::GetOrCreateRootCertStore(env_)) { + cert_store = crypto::NewRootCertStore(env_); SSL_CTX_set_cert_store(ctx.get(), cert_store); } diff --git a/src/quic/tlscontext.h b/src/quic/tlscontext.h index 528085b00497a7..22b4949aaaf4d9 100644 --- a/src/quic/tlscontext.h +++ b/src/quic/tlscontext.h @@ -229,10 +229,12 @@ class TLSContext final : public MemoryRetainer, std::string ToString() const; }; - static std::shared_ptr CreateClient(const Options& options); - static std::shared_ptr CreateServer(const Options& options); + static std::shared_ptr CreateClient(Environment* env, + const Options& options); + static std::shared_ptr CreateServer(Environment* env, + const Options& options); - TLSContext(Side side, const Options& options); + TLSContext(Environment* env, Side side, const Options& options); DISALLOW_COPY_AND_MOVE(TLSContext) // Each QUIC Session has exactly one TLSSession. Each TLSSession maintains @@ -242,6 +244,7 @@ class TLSContext final : public MemoryRetainer, inline Side side() const { return side_; } inline const Options& options() const { return options_; } + inline Environment* env() const { return env_; } inline operator bool() const { return ctx_ != nullptr; } inline operator const ncrypto::SSLCtxPointer&() const { return ctx_; } @@ -269,6 +272,7 @@ class TLSContext final : public MemoryRetainer, Side side_; Options options_; + Environment* env_; ncrypto::X509Pointer cert_; ncrypto::X509Pointer issuer_; ncrypto::SSLCtxPointer ctx_; diff --git a/test/cctest/test_node_crypto.cc b/test/cctest/test_node_crypto.cc index 9d6405a40d90c7..bb841b8690de68 100644 --- a/test/cctest/test_node_crypto.cc +++ b/test/cctest/test_node_crypto.cc @@ -15,7 +15,7 @@ */ TEST(NodeCrypto, NewRootCertStore) { node::per_process::cli_options->ssl_openssl_cert_store = true; - X509_STORE* store = node::crypto::NewRootCertStore(); + X509_STORE* store = node::crypto::NewRootCertStore(nullptr); ASSERT_TRUE(store); ASSERT_EQ(ERR_peek_error(), 0UL) << "NewRootCertStore should not have left " "any errors on the OpenSSL error stack\n"; diff --git a/test/parallel/test-cli-node-options.js b/test/parallel/test-cli-node-options.js index 3eee6023025f85..f7ff78c9dddfdc 100644 --- a/test/parallel/test-cli-node-options.js +++ b/test/parallel/test-cli-node-options.js @@ -68,7 +68,7 @@ if (common.hasCrypto) { if (!hasOpenSSL3) expectNoWorker('--openssl-config=_ossl_cfg', 'B\n'); if (common.isMacOS) { - expectNoWorker('--use-system-ca', 'B\n'); + expect('--use-system-ca', 'B\n'); } } diff --git a/test/system-ca/test-use-system-ca-worker-disable.mjs b/test/system-ca/test-use-system-ca-worker-disable.mjs new file mode 100644 index 00000000000000..6e83a59ace0dcd --- /dev/null +++ b/test/system-ca/test-use-system-ca-worker-disable.mjs @@ -0,0 +1,103 @@ +// Flags: --use-system-ca + +// Verify that a worker can disable the system CA store while the parent uses it. + +import * as common from '../common/index.mjs'; +import assert from 'node:assert/strict'; +import https from 'node:https'; +import fixtures from '../common/fixtures.js'; +import { it, beforeEach, afterEach, describe } from 'node:test'; +import { once } from 'events'; +import { + Worker, + isMainThread, + parentPort, + workerData, +} from 'node:worker_threads'; + +if (!common.hasCrypto) { + common.skip('requires crypto'); +} + +// To run this test, the system needs to be configured to trust +// the CA certificate first (which needs an interactive GUI approval, e.g. TouchID): +// see the README.md in this folder for instructions on how to do this. +const handleRequest = (req, res) => { + const path = req.url; + switch (path) { + case '/hello-world': + res.writeHead(200); + res.end('hello world\n'); + break; + default: + common.mustNotCall(`Unexpected path: ${path}`)(); + } +}; + +describe('use-system-ca', function() { + async function setupServer(key, cert) { + const theServer = https.createServer( + { + key: fixtures.readKey(key), + cert: fixtures.readKey(cert), + }, + handleRequest, + ); + theServer.listen(0); + await once(theServer, 'listening'); + + return theServer; + } + + let server; + + beforeEach(async function() { + if (isMainThread) { + server = await setupServer('agent8-key.pem', 'agent8-cert.pem'); + } else { + server = undefined; + } + }); + + it('trusts a valid root certificate', async function() { + if (isMainThread) { + const worker = new Worker(new URL(import.meta.url), { + execArgv: ['--no-use-system-ca'], + workerData: { + url: `https://localhost:${server.address().port}/hello-world`, + }, + }); + await fetch(`https://localhost:${server.address().port}/hello-world`); + + const [message] = await once(worker, 'message'); + assert.strictEqual(message.ok, false); + assert( + message.code === 'UNABLE_TO_VERIFY_LEAF_SIGNATURE' || + message.code === 'SELF_SIGNED_CERT_IN_CHAIN', + `Expected certificate verification error but got: ${JSON.stringify( + message, + )}`, + ); + + const [exitCode] = await once(worker, 'exit'); + assert.strictEqual(exitCode, 0); + } else { + const { url } = workerData; + try { + const res = await fetch(url); + const text = await res.text(); + parentPort.postMessage({ ok: true, status: res.status, text }); + } catch (err) { + parentPort.postMessage({ + ok: false, + code: err?.cause?.code, + message: err.message, + }); + } + } + }); + + afterEach(async function() { + server?.close(); + }); +}); diff --git a/test/system-ca/test-use-system-ca-worker-enable.mjs b/test/system-ca/test-use-system-ca-worker-enable.mjs new file mode 100644 index 00000000000000..f0f82e4cbb84ee --- /dev/null +++ b/test/system-ca/test-use-system-ca-worker-enable.mjs @@ -0,0 +1,113 @@ +// Flags: --no-use-system-ca + +// Verify that a worker can enable the system CA store while the parent disables it. + +import * as common from '../common/index.mjs'; +import assert from 'node:assert/strict'; +import https from 'node:https'; +import fixtures from '../common/fixtures.js'; +import { it, beforeEach, afterEach, describe } from 'node:test'; +import { once } from 'events'; +import { + Worker, + isMainThread, + parentPort, + workerData, +} from 'node:worker_threads'; + +if (!common.hasCrypto) { + common.skip('requires crypto'); +} + +// To run this test, the system needs to be configured to trust +// the CA certificate first (which needs an interactive GUI approval, e.g. TouchID): +// see the README.md in this folder for instructions on how to do this. +const handleRequest = (req, res) => { + const path = req.url; + switch (path) { + case '/hello-world': + res.writeHead(200); + res.end('hello world\n'); + break; + default: + common.mustNotCall(`Unexpected path: ${path}`)(); + } +}; + +describe('use-system-ca', function() { + async function setupServer(key, cert) { + const theServer = https.createServer( + { + key: fixtures.readKey(key), + cert: fixtures.readKey(cert), + }, + handleRequest, + ); + theServer.listen(0); + await once(theServer, 'listening'); + + return theServer; + } + + let server; + + beforeEach(async function() { + if (isMainThread) { + server = await setupServer('agent8-key.pem', 'agent8-cert.pem'); + } else { + server = undefined; + } + }); + + it('trusts a valid root certificate', async function() { + if (isMainThread) { + const worker = new Worker(new URL(import.meta.url), { + execArgv: ['--use-system-ca'], + workerData: { + url: `https://localhost:${server.address().port}/hello-world`, + }, + }); + await assert.rejects( + fetch(`https://localhost:${server.address().port}/hello-world`), + (err) => { + assert.strictEqual(err.cause.code, 'UNABLE_TO_VERIFY_LEAF_SIGNATURE'); + return true; + }, + ); + + const [message] = await once(worker, 'message'); + await assert.rejects( + fetch(`https://localhost:${server.address().port}/hello-world`), + (err) => { + assert.strictEqual(err.cause.code, 'UNABLE_TO_VERIFY_LEAF_SIGNATURE'); + return true; + }, + ); + assert.strictEqual(message.ok, true); + assert.strictEqual( + message.status, + 200, + `Expected status 200, got ${message.status}`, + ); + const [exitCode] = await once(worker, 'exit'); + assert.strictEqual(exitCode, 0); + } else { + const { url } = workerData; + try { + const res = await fetch(url); + const text = await res.text(); + parentPort.postMessage({ ok: true, status: res.status, text }); + } catch (err) { + parentPort.postMessage({ + ok: false, + code: err?.cause?.code, + message: err.message, + }); + } + } + }); + + afterEach(async function() { + server?.close(); + }); +});