tls: only do off-thread certificate loading on loading tls

lib/tls.js

@@ -39,29 +39,45 @@ const {
   ERR_INVALID_ARG_VALUE,
   ERR_INVALID_ARG_TYPE,
 } = require('internal/errors').codes;
-const internalUtil = require('internal/util');
-internalUtil.assertCrypto();
-const {
-  isArrayBufferView,
-  isUint8Array,
-} = require('internal/util/types');
 
-const net = require('net');
-const { getOptionValue } = require('internal/options');
 const {
   getBundledRootCertificates,
   getExtraCACertificates,
   getSystemCACertificates,
   resetRootCertStore,
   getUserRootCertificates,
   getSSLCiphers,
+  startLoadingCertificatesOffThread,
 } = internalBinding('crypto');
+
+// Start loading root certificates in a separate thread as early as possible
+// once the tls module is loaded, so that by the time an actual TLS connection is
+// made, the loading is done.
+startLoadingCertificatesOffThread();
+
+const internalUtil = require('internal/util');
+internalUtil.assertCrypto();
+const {
+  isArrayBufferView,
+  isUint8Array,
+} = require('internal/util/types');
+
+const net = require('net');
+const { getOptionValue } = require('internal/options');
 const { Buffer } = require('buffer');
 const { canonicalizeIP } = internalBinding('cares_wrap');
 const tlsCommon = require('internal/tls/common');
 const tlsWrap = require('internal/tls/wrap');
 const { validateString } = require('internal/validators');
 
+const {
+  namespace: {
+    addDeserializeCallback,
+    addSerializeCallback,
+    isBuildingSnapshot,
+  },
+} = require('internal/v8/startup_snapshot');
+
 // Allow {CLIENT_RENEG_LIMIT} client-initiated session renegotiations
 // every {CLIENT_RENEG_WINDOW} seconds. An error event is emitted if more
 // renegotiations are seen. The settings are applied to all remote client
@@ -203,6 +219,28 @@ function setDefaultCACertificates(certs) {
 
 exports.setDefaultCACertificates = setDefaultCACertificates;
 
+if (isBuildingSnapshot()) {
+  addSerializeCallback(() => {
+    // Clear the cached certs so that they are reloaded at runtime.
+    // Bundled certificates are immutable so they are spared.
+    extraCACertificates = undefined;
+    systemCACertificates = undefined;
+    if (hasResetDefaultCACertificates) {
+      defaultCACertificates = undefined;
+    }
+  });
+  addDeserializeCallback(() => {
+    // If the tls module is loaded during snapshotting, load the certificates from
+    // various sources again at runtime so that by the time an actual TLS connection is
+    // made, the loading is done. If the default CA certificates have been overridden, then
+    // the serialized overriding certificates are likely to be used and pre-loading
+    // from the sources would probably not yield any benefit, so skip it.
+    if (!hasResetDefaultCACertificates) {
+      startLoadingCertificatesOffThread();
+    }
+  });
+}
+
 // Convert protocols array into valid OpenSSL protocols list
 // ("\x06spdy/2\x08http/1.1\x08http/1.0")
 function convertProtocols(protocols) {

src/crypto/crypto_context.cc

@@ -814,23 +814,6 @@ static std::vector<X509*>& GetSystemStoreCACertificates() {
   return system_store_certs;
 }
 
-static void LoadSystemCACertificates(void* data) {
-  GetSystemStoreCACertificates();
-}
-
-static uv_thread_t system_ca_thread;
-static bool system_ca_thread_started = false;
-int LoadSystemCACertificatesOffThread() {
-  // This is only run once during the initialization of the process, so
-  // it is safe to use a static thread here.
-  int r =
-      uv_thread_create(&system_ca_thread, LoadSystemCACertificates, nullptr);
-  if (r == 0) {
-    system_ca_thread_started = true;
-  }
-  return r;
-}
-
 static std::vector<X509*> InitializeExtraCACertificates() {
   std::vector<X509*> extra_certs;
   unsigned long err = LoadCertsFromFile(  // NOLINT(runtime/int)
@@ -854,6 +837,73 @@ static std::vector<X509*>& GetExtraCACertificates() {
   return extra_certs;
 }
 
+static void LoadCACertificates(void* data) {
+  per_process::Debug(DebugCategory::CRYPTO,
+                     "Started loading bundled root certificates off-thread\n");
+  GetBundledRootCertificates();
+
+  if (!extra_root_certs_file.empty()) {
+    per_process::Debug(DebugCategory::CRYPTO,
+                       "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;
+    }
+  }
+
+  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 Mutex start_cert_loading_thread_mutex;
+static uv_thread_t cert_loading_thread;
+
+void StartLoadingCertificatesOffThread(
+    const FunctionCallbackInfo<Value>& args) {
+  // Load the CA certificates eagerly off the main thread to avoid
+  // blocking the main thread when the first TLS connection is made. We
+  // don't need to wait for the thread to finish with code here, as
+  // Get*CACertificates() functions has a function-local static and any
+  // actual user of it will wait for that to complete initialization.
+
+  // --use-openssl-ca is mutually exclusive with --use-bundled-ca and
+  // --use-system-ca. If it's set, no need to optimize with off-thread
+  // loading.
+  {
+    Mutex::ScopedLock cli_lock(node::per_process::cli_options_mutex);
+    if (!per_process::cli_options->ssl_openssl_cert_store) {
+      return;
+    }
+  }
+
+  // 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;
+    }
+    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));
+    }
+  }
+}
+
 // Due to historical reasons the various options of CA certificates
 // may invalid one another. The current rule is:
 // 1. If the configure-time option --openssl-use-def-ca-store is NOT used
@@ -942,9 +992,12 @@ void CleanupCachedRootCertificates() {
       X509_free(cert);
     }
   }
-  if (system_ca_thread_started) {
-    uv_thread_join(&system_ca_thread);
-    system_ca_thread_started = false;
+
+  // 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);
   }
 }
 
@@ -1233,6 +1286,10 @@ void SecureContext::Initialize(Environment* env, Local<Object> target) {
   SetMethod(context, target, "resetRootCertStore", ResetRootCertStore);
   SetMethodNoSideEffect(
       context, target, "getUserRootCertificates", GetUserRootCertificates);
+  SetMethod(context,
+            target,
+            "startLoadingCertificatesOffThread",
+            StartLoadingCertificatesOffThread);
 }
 
 void SecureContext::RegisterExternalReferences(
@@ -1277,6 +1334,7 @@ void SecureContext::RegisterExternalReferences(
   registry->Register(GetExtraCACertificates);
   registry->Register(ResetRootCertStore);
   registry->Register(GetUserRootCertificates);
+  registry->Register(StartLoadingCertificatesOffThread);
 }
 
 SecureContext* SecureContext::Create(Environment* env) {

src/crypto/crypto_util.h

@@ -45,7 +45,6 @@ void InitCryptoOnce();
 void InitCrypto(v8::Local<v8::Object> target);
 
 extern void UseExtraCaCerts(std::string_view file);
-extern int LoadSystemCACertificatesOffThread();
 void CleanupCachedRootCertificates();
 
 int PasswordCallback(char* buf, int size, int rwflag, void* u);

src/debug_utils.h

@@ -42,6 +42,7 @@ void NODE_EXTERN_PRIVATE FWrite(FILE* file, const std::string& str);
 // from a provider type to a debug category.
 #define DEBUG_CATEGORY_NAMES(V)                                                \
   NODE_ASYNC_PROVIDER_TYPES(V)                                                 \
+  V(CRYPTO)                                                                    \
   V(COMPILE_CACHE)                                                             \
   V(DIAGNOSTICS)                                                               \
   V(HUGEPAGES)                                                                 \

src/node.cc

@@ -1210,20 +1210,6 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,
       return result;
     }
 
-    if (per_process::cli_options->use_system_ca) {
-      // Load the system CA certificates eagerly off the main thread to avoid
-      // blocking the main thread when the first TLS connection is made. We
-      // don't need to wait for the thread to finish with code here, as
-      // GetSystemStoreCACertificates() has a function-local static and any
-      // actual user of it will wait for that to complete initialization.
-      int r = crypto::LoadSystemCACertificatesOffThread();
-      if (r != 0) {
-        FPrintF(
-            stderr,
-            "Warning: Failed to load system CA certificates off thread: %s\n",
-            uv_strerror(r));
-      }
-    }
     // Ensure CSPRNG is properly seeded.
     CHECK(ncrypto::CSPRNG(nullptr, 0));