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

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

@@ -101,11 +101,11 @@ static thread_local X509_STORE* root_cert_store = nullptr;
 // from this set.
 static thread_local std::unique_ptr<X509Set> 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;
 }
 
@@ -870,23 +870,21 @@ 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 uv_thread_t cert_loading_thread;
+static uv_thread_t system_cert_loading_thread;
 
 void StartLoadingCertificatesOffThread(
     const FunctionCallbackInfo<Value>& args) {
@@ -906,16 +904,13 @@ void StartLoadingCertificatesOffThread(
     }
   }
 
+  Environment* env = Environment::GetCurrent(args);
+  const bool use_system_ca = env != nullptr && env->options()->use_system_ca;
+
+  Mutex::ScopedLock lock(start_cert_loading_thread_mutex);
+
   // Only try to start the thread once. If it ever fails, we won't try again.
-  if (tried_cert_loading_off_thread.load()) {
-    return;
-  }
-  {
-    Mutex::ScopedLock lock(start_cert_loading_thread_mutex);
-    // Re-check under the lock.
-    if (tried_cert_loading_off_thread.load()) {
-      return;
-    }
+  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);
@@ -925,6 +920,21 @@ void StartLoadingCertificatesOffThread(
               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.
+  if (use_system_ca && !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));
+    }
+  }
 }
 
 // Due to historical reasons the various options of CA certificates
@@ -947,13 +957,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 +985,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));
       }
@@ -1022,6 +1042,10 @@ void CleanupCachedRootCertificates() {
       cert_loading_thread_started.load()) {
     uv_thread_join(&cert_loading_thread);
   }
+  if (tried_system_cert_loading_off_thread.load() &&
+      system_cert_loading_thread_started.load()) {
+    uv_thread_join(&system_cert_loading_thread);
+  }
 }
 
 void GetBundledRootCertificates(const FunctionCallbackInfo<Value>& args) {
@@ -1187,9 +1211,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) {
@@ -1700,11 +1722,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 +1800,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

@@ -19,9 +19,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

@@ -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;

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<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

@@ -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;

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<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",

src/quic/tlscontext.cc

@@ -293,16 +293,18 @@ bool OSSLContext::ConfigureClient() const {
 
 // ============================================================================
 
-std::shared_ptr<TLSContext> TLSContext::CreateClient(const Options& options) {
-  return std::make_shared<TLSContext>(Side::CLIENT, options);
+std::shared_ptr<TLSContext> TLSContext::CreateClient(Environment* env,
+                                                     const Options& options) {
+  return std::make_shared<TLSContext>(env, Side::CLIENT, options);
 }
 
-std::shared_ptr<TLSContext> TLSContext::CreateServer(const Options& options) {
-  return std::make_shared<TLSContext>(Side::SERVER, options);
+std::shared_ptr<TLSContext> TLSContext::CreateServer(Environment* env,
+                                                     const Options& options) {
+  return std::make_shared<TLSContext>(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);
       }
 

src/quic/tlscontext.h

@@ -229,10 +229,12 @@ class TLSContext final : public MemoryRetainer,
     std::string ToString() const;
   };
 
-  static std::shared_ptr<TLSContext> CreateClient(const Options& options);
-  static std::shared_ptr<TLSContext> CreateServer(const Options& options);
+  static std::shared_ptr<TLSContext> CreateClient(Environment* env,
+                                                  const Options& options);
+  static std::shared_ptr<TLSContext> 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_;

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";