crypto: make --use-system-ca per-env rather than per-process

PR-URL: #60678
Backport-PR-URL: #64675
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Author: Aditi-1400

src/crypto/crypto_common.cc

@@ -61,7 +61,7 @@ MaybeLocal<Value> 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, "

src/crypto/crypto_context.cc

@@ -103,12 +103,39 @@ static thread_local X509_STORE* root_cert_store = nullptr;
 // copy generated by NewRootCertStore() will then contain the certificates
 // from this set.
 static thread_local std::unique_ptr<X509Set> root_certs_from_users;
+static thread_local bool has_cleanup_hook = false;
 
-X509_STORE* GetOrCreateRootCertStore() {
+static void CleanupRootCertStore(void*) {
+  if (root_cert_store != nullptr) {
+    X509_STORE_free(root_cert_store);
+    root_cert_store = nullptr;
+  }
+
+  if (root_certs_from_users != nullptr) {
+    for (X509* cert : *root_certs_from_users) {
+      X509_free(cert);
+    }
+    root_certs_from_users.reset();
+  }
+
+  has_cleanup_hook = false;
+}
+
+static void EnsureRootCertStoreCleanupHook(Environment* env) {
+  if (env == nullptr || has_cleanup_hook) {
+    return;
+  }
+
+  env->AddCleanupHook(CleanupRootCertStore, nullptr);
+  has_cleanup_hook = true;
+}
+
+X509_STORE* GetOrCreateRootCertStore(Environment* env) {
+  EnsureRootCertStoreCleanupHook(env);
   if (root_cert_store != nullptr) {
     return root_cert_store;
   }
-  root_cert_store = NewRootCertStore();
+  root_cert_store = NewRootCertStore(env);
   return root_cert_store;
 }
 
@@ -932,23 +959,22 @@ static void LoadCACertificates(void* data) {
                        "Started loading extra root certificates off-thread\n");
     GetExtraCACertificates();
   }
+}
 
-  {
-    Mutex::ScopedLock cli_lock(node::per_process::cli_options_mutex);
-    if (!per_process::cli_options->use_system_ca) {
-      return;
-    }
-  }
-
+static void LoadSystemCACertificates(void* data) {
   per_process::Debug(DebugCategory::CRYPTO,
                      "Started loading system root certificates off-thread\n");
   GetSystemStoreCACertificates();
 }
 
 static std::atomic<bool> tried_cert_loading_off_thread = false;
 static std::atomic<bool> cert_loading_thread_started = false;
+static std::atomic<bool> tried_system_cert_loading_off_thread = false;
+static std::atomic<bool> system_cert_loading_thread_started = false;
 static Mutex start_cert_loading_thread_mutex;
+static Mutex start_system_cert_loading_thread_mutex;
 static uv_thread_t cert_loading_thread;
+static uv_thread_t system_cert_loading_thread;
 
 void StartLoadingCertificatesOffThread(
     const FunctionCallbackInfo<Value>& args) {
@@ -968,23 +994,46 @@ void StartLoadingCertificatesOffThread(
     }
   }
 
+  Environment* env = Environment::GetCurrent(args);
+  const bool use_system_ca = env != nullptr && env->options()->use_system_ca;
+  per_process::Debug(
+      DebugCategory::CRYPTO, "StartLoadingCertificatesOffThread env=%p\n", env);
   // Only try to start the thread once. If it ever fails, we won't try again.
-  if (tried_cert_loading_off_thread.load()) {
-    return;
-  }
-  {
+  // Quick check, if it's already tried, no need to lock.
+  if (!tried_cert_loading_off_thread.load()) {
     Mutex::ScopedLock lock(start_cert_loading_thread_mutex);
-    // Re-check under the lock.
-    if (tried_cert_loading_off_thread.load()) {
-      return;
+    // Check again under the lock.
+    if (!tried_cert_loading_off_thread.load()) {
+      tried_cert_loading_off_thread.store(true);
+      int r =
+          uv_thread_create(&cert_loading_thread, LoadCACertificates, nullptr);
+      cert_loading_thread_started.store(r == 0);
+      if (r != 0) {
+        FPrintF(stderr,
+                "Warning: Failed to load CA certificates off thread: %s\n",
+                uv_strerror(r));
+      }
     }
-    tried_cert_loading_off_thread.store(true);
-    int r = uv_thread_create(&cert_loading_thread, LoadCACertificates, nullptr);
-    cert_loading_thread_started.store(r == 0);
-    if (r != 0) {
-      FPrintF(stderr,
-              "Warning: Failed to load CA certificates off thread: %s\n",
-              uv_strerror(r));
+  }
+
+  // If the system CA list hasn't been loaded off-thread yet, allow a worker
+  // enabling --use-system-ca to trigger its off-thread loading.
+  // Quick check, if it's already tried, no need to lock.
+  if (use_system_ca && !has_cached_system_root_certs.load() &&
+      !tried_system_cert_loading_off_thread.load()) {
+    Mutex::ScopedLock lock(start_system_cert_loading_thread_mutex);
+    if (!has_cached_system_root_certs.load() &&
+        !tried_system_cert_loading_off_thread.load()) {
+      tried_system_cert_loading_off_thread.store(true);
+      int r = uv_thread_create(
+          &system_cert_loading_thread, LoadSystemCACertificates, nullptr);
+      system_cert_loading_thread_started.store(r == 0);
+      if (r != 0) {
+        FPrintF(
+            stderr,
+            "Warning: Failed to load system CA certificates off thread: %s\n",
+            uv_strerror(r));
+      }
     }
   }
 }
@@ -1009,13 +1058,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);
 
@@ -1037,14 +1086,26 @@ X509_STORE* NewRootCertStore() {
   }
 #endif
 
-  Mutex::ScopedLock cli_lock(node::per_process::cli_options_mutex);
-  if (per_process::cli_options->ssl_openssl_cert_store) {
+  bool use_system_ca = false;
+  bool ssl_openssl_cert_store = false;
+  {
+    Mutex::ScopedLock cli_lock(node::per_process::cli_options_mutex);
+    ssl_openssl_cert_store = per_process::cli_options->ssl_openssl_cert_store;
+    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 (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));
       }
@@ -1061,6 +1122,22 @@ X509_STORE* NewRootCertStore() {
 }
 
 void CleanupCachedRootCertificates() {
+  // Serialize with starters to avoid the race window.
+  {
+    Mutex::ScopedLock lock(start_cert_loading_thread_mutex);
+    if (tried_cert_loading_off_thread.load() &&
+        cert_loading_thread_started.load()) {
+      uv_thread_join(&cert_loading_thread);
+    }
+  }
+  {
+    Mutex::ScopedLock lock(start_system_cert_loading_thread_mutex);
+    if (tried_system_cert_loading_off_thread.load() &&
+        system_cert_loading_thread_started.load()) {
+      uv_thread_join(&system_cert_loading_thread);
+    }
+  }
+
   if (has_cached_bundled_root_certs.load()) {
     for (X509* cert : GetBundledRootCertificates()) {
       X509_free(cert);
@@ -1077,13 +1154,6 @@ void CleanupCachedRootCertificates() {
       X509_free(cert);
     }
   }
-
-  // Serialize with starter to avoid the race window.
-  Mutex::ScopedLock lock(start_cert_loading_thread_mutex);
-  if (tried_cert_loading_off_thread.load() &&
-      cert_loading_thread_started.load()) {
-    uv_thread_join(&cert_loading_thread);
-  }
 }
 
 void GetBundledRootCertificates(const FunctionCallbackInfo<Value>& args) {
@@ -1195,6 +1265,8 @@ void ResetRootCertStore(const FunctionCallbackInfo<Value>& args) {
   Local<Context> context = args.GetIsolate()->GetCurrentContext();
   CHECK(args[0]->IsArray());
   Local<Array> cert_array = args[0].As<Array>();
+  Environment* env = Environment::GetCurrent(context);
+  EnsureRootCertStoreCleanupHook(env);
 
   if (cert_array->Length() == 0) {
     // If the array is empty, just clear the user certs and reset the store.
@@ -1249,9 +1321,7 @@ void ResetRootCertStore(const FunctionCallbackInfo<Value>& args) {
     X509_STORE_free(root_cert_store);
   }
 
-  // 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<Value>& args) {
@@ -1778,11 +1848,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);
   }
 
@@ -1855,7 +1926,8 @@ void SecureContext::AddCRL(const FunctionCallbackInfo<Value>& 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);

src/crypto/crypto_context.h

@@ -23,9 +23,9 @@ constexpr int kMaxSupportedVersion = TLS1_3_VERSION;
 void GetRootCertificates(
     const v8::FunctionCallbackInfo<v8::Value>& args);
 
-X509_STORE* NewRootCertStore();
+X509_STORE* NewRootCertStore(Environment* env);
 
-X509_STORE* GetOrCreateRootCertStore();
+X509_STORE* GetOrCreateRootCertStore(Environment* env);
 
 ncrypto::BIOPointer LoadBIO(Environment* env, v8::Local<v8::Value> v);
 

src/node.cc

@@ -867,15 +867,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;

src/node_options.cc

@@ -199,6 +199,13 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors,
                       "used, not both");
   }
 
+#if HAVE_OPENSSL
+  if (use_system_ca && per_process::cli_options->use_openssl_ca) {
+    errors->push_back("either --use-openssl-ca or --use-system-ca can be "
+                      "used, not both");
+  }
+#endif  // HAVE_OPENSSL
+
   if (heap_snapshot_near_heap_limit < 0) {
     errors->push_back("--heapsnapshot-near-heap-limit must not be negative");
   }
@@ -1052,6 +1059,13 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
       &EnvironmentOptions::trace_env_native_stack,
       kAllowedInEnvvar);
 
+#if HAVE_OPENSSL
+  AddOption("--use-system-ca",
+            "use system's CA store",
+            &EnvironmentOptions::use_system_ca,
+            kAllowedInEnvvar);
+#endif  // HAVE_OPENSSL
+
   AddOption(
       "--trace-require-module",
       "Print access to require(esm). Options are 'all' (print all usage) and "
@@ -1394,10 +1408,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)
@@ -2160,6 +2170,10 @@ void HandleEnvOptions(std::shared_ptr<EnvironmentOptions> 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");
 }

src/node_options.h

@@ -229,6 +229,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;
@@ -364,7 +365,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;

src/quic/endpoint.cc

@@ -932,7 +932,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());
@@ -974,7 +974,7 @@ BaseObjectPtr<Session> 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",