diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc index 823d87e7f20..d314d15cc54 100644 --- a/src/crypto/crypto_context.cc +++ b/src/crypto/crypto_context.cc @@ -95,37 +95,27 @@ struct X509Less { }; using X509Set = std::set; -// Per-thread root cert store. See NewRootCertStore() on what it contains. -static thread_local DeleteFnPtr root_cert_store; -// If the user calls tls.setDefaultCACertificates() this will be used -// to hold the user-provided certificates, the root_cert_store and any new -// copy generated by NewRootCertStore() will then contain the certificates -// from this set. -static thread_local std::unique_ptr root_certs_from_users; -static thread_local bool has_cleanup_hook = false; +struct RootCertStore { + // See NewRootCertStore() on what it contains. + DeleteFnPtr store; + // Set by tls.setDefaultCACertificates(). Once set, NewRootCertStore() + // copies these certificates instead of loading the defaults. + std::unique_ptr certs_from_users; +}; -static void CleanupRootCertStore(void*) { - root_cert_store.reset(); - root_certs_from_users.reset(); - has_cleanup_hook = false; +void FreeRootCertStore(RootCertStore* root_certs) { + delete root_certs; } -static void EnsureRootCertStoreCleanupHook(Environment* env) { - if (env == nullptr || has_cleanup_hook) { - return; - } - - env->AddCleanupHook(CleanupRootCertStore, nullptr); - has_cleanup_hook = true; +static RootCertStore* GetRootCertStore(Environment* env) { + if (!env->root_cert_store) env->root_cert_store.reset(new RootCertStore()); + return env->root_cert_store.get(); } X509_STORE* GetOrCreateRootCertStore(Environment* env) { - EnsureRootCertStoreCleanupHook(env); - if (root_cert_store != nullptr) { - return root_cert_store.get(); - } - root_cert_store.reset(NewRootCertStore(env)); - return root_cert_store.get(); + RootCertStore* root_certs = GetRootCertStore(env); + if (!root_certs->store) root_certs->store.reset(NewRootCertStore(env)); + return root_certs->store.get(); } // Takes a string or buffer and loads it into a BIO. @@ -1062,8 +1052,10 @@ X509_STORE* NewRootCertStore(Environment* env) { // If the root cert store is already reset by users through // tls.setDefaultCACertificates(), just create a copy from the // user-provided certificates. - if (root_certs_from_users != nullptr) { - for (const auto& cert : *root_certs_from_users) { + const X509Set* certs_from_users = + env != nullptr ? GetRootCertStore(env)->certs_from_users.get() : nullptr; + if (certs_from_users != nullptr) { + for (const auto& cert : *certs_from_users) { CHECK_EQ(1, X509_STORE_add_cert(store, cert.get())); } return store; @@ -1230,12 +1222,13 @@ MaybeLocal X509sToArrayOfStrings(Environment* env, void GetUserRootCertificates(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - CHECK_NOT_NULL(root_certs_from_users); + const auto& certs_from_users = GetRootCertStore(env)->certs_from_users; + CHECK(certs_from_users); Local results; if (X509sToArrayOfStrings(env, - root_certs_from_users->begin(), - root_certs_from_users->end(), - root_certs_from_users->size()) + certs_from_users->begin(), + certs_from_users->end(), + certs_from_users->size()) .ToLocal(&results)) { args.GetReturnValue().Set(results); } @@ -1246,12 +1239,12 @@ void ResetRootCertStore(const FunctionCallbackInfo& args) { CHECK(args[0]->IsArray()); Local cert_array = args[0].As(); Environment* env = Environment::GetCurrent(context); - EnsureRootCertStoreCleanupHook(env); + RootCertStore* root_certs = GetRootCertStore(env); if (cert_array->Length() == 0) { // If the array is empty, just clear the user certs and reset the store. - root_cert_store.reset(); - root_certs_from_users = std::make_unique(); + root_certs->store.reset(); + root_certs->certs_from_users = std::make_unique(); return; } @@ -1263,7 +1256,6 @@ void ResetRootCertStore(const FunctionCallbackInfo& args) { } if (certs->empty()) { - Environment* env = Environment::GetCurrent(context); return THROW_ERR_CRYPTO_OPERATION_FAILED( env, "No valid certificates found in the provided array"); } @@ -1275,11 +1267,11 @@ void ResetRootCertStore(const FunctionCallbackInfo& args) { // is not consumed by insert (element already exists). } - root_certs_from_users = std::move(new_set); + root_certs->certs_from_users = std::move(new_set); - // Reset the global root cert store so it will be recreated with the - // new certificates. - root_cert_store.reset(); + // Reset the root cert store so it will be recreated with the new + // certificates. + root_certs->store.reset(); } void GetSystemCACertificates(const FunctionCallbackInfo& args) { diff --git a/src/env.h b/src/env.h index 63c52524695..0349add7544 100644 --- a/src/env.h +++ b/src/env.h @@ -76,6 +76,13 @@ class MacCache; namespace node { +#if HAVE_OPENSSL +namespace crypto { +struct RootCertStore; +void FreeRootCertStore(RootCertStore* root_certs); +} // namespace crypto +#endif // HAVE_OPENSSL + namespace shadow_realm { class ShadowRealm; } @@ -1219,6 +1226,7 @@ class Environment final : public MemoryRetainer { std::unique_ptr provider_mac_cache; std::vector supported_mac_algorithms; bool supported_mac_algorithms_initialized = false; + DeleteFnPtr root_cert_store; #endif // HAVE_OPENSSL v8::Global temporary_required_module_facade_original; diff --git a/test/cctest/test_environment_shared_isolate.cc b/test/cctest/test_environment_shared_isolate.cc index e8787d45c10..be9638211bf 100644 --- a/test/cctest/test_environment_shared_isolate.cc +++ b/test/cctest/test_environment_shared_isolate.cc @@ -6,8 +6,12 @@ #include "cppgc/allocation.h" #include "cppgc/garbage-collected.h" +#include "env-inl.h" #include "node_test_fixture.h" #include "v8-cppgc.h" +#if HAVE_OPENSSL +#include "crypto/crypto_context.h" +#endif #include #include @@ -446,6 +450,36 @@ TEST_P(SharedIsolateTest, FreeIsolateDataBeforeItsEnvironmentAsserts) { FreeInstance(std::move(instance)); } +#if HAVE_OPENSSL +TEST_P(SharedIsolateTest, RootCertStoreIsPerEnvironment) { + const HandleScope handle_scope(isolate_); + std::unique_ptr first = + CreateInstance(0, EnvironmentFlags::kNoCreateInspector); + std::unique_ptr second = + CreateInstance(1, EnvironmentFlags::kNoCreateInspector); + auto store_size = [](Instance* instance) { + return sk_X509_OBJECT_num(X509_STORE_get0_objects( + node::crypto::GetOrCreateRootCertStore(instance->env))); + }; + auto set_default_ca_count = [this](Instance* instance, int count) { + std::string source = + "const tls = process.getBuiltinModule('tls');" + "tls.setDefaultCACertificates(tls.rootCertificates.slice(0, " + + std::to_string(count) + "))"; + Evaluate(instance, source.c_str()); + }; + + set_default_ca_count(first.get(), 1); + set_default_ca_count(second.get(), 2); + EXPECT_EQ(store_size(first.get()), 1); + EXPECT_EQ(store_size(second.get()), 2); + + FreeInstance(std::move(first)); + EXPECT_EQ(store_size(second.get()), 2); + FreeInstance(std::move(second)); +} +#endif // HAVE_OPENSSL + INSTANTIATE_TEST_SUITE_P( EnvironmentTest, SharedIsolateTest,