FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

tls: only do off-thread certificate loading on loading tls · nodejs/node@f5d3f91 · GitHub

/ node Public

Commit f5d3f91

Browse files
joyeecheung
authored and
targos
committed
tls: only do off-thread certificate loading on loading tls
This patch makes the off-thread loading lazy and only done when the `tls` builtin is actually loaded by the application. Thus if the application never uses tls, it would not get hit by the extra off-thread loading overhead. paving the way to enable --use-system-ca by default. PR-URL: #59856 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent fa40d3a commit f5d3f91

5 files changed

Lines changed: 105 additions & 43 deletions

File tree

‎lib/tls.js‎

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,29 +39,45 @@ const {
3939
ERR_INVALID_ARG_VALUE,
4040
ERR_INVALID_ARG_TYPE,
4141
} = require('internal/errors').codes;
42-
const internalUtil = require('internal/util');
43-
internalUtil.assertCrypto();
44-
const {
45-
isArrayBufferView,
46-
isUint8Array,
47-
} = require('internal/util/types');
4842

49-
const net = require('net');
50-
const { getOptionValue } = require('internal/options');
5143
const {
5244
getBundledRootCertificates,
5345
getExtraCACertificates,
5446
getSystemCACertificates,
5547
resetRootCertStore,
5648
getUserRootCertificates,
5749
getSSLCiphers,
50+
startLoadingCertificatesOffThread,
5851
} = internalBinding('crypto');
52+
53+
// Start loading root certificates in a separate thread as early as possible
54+
// once the tls module is loaded, so that by the time an actual TLS connection is
55+
// made, the loading is done.
56+
startLoadingCertificatesOffThread();
57+
58+
const internalUtil = require('internal/util');
59+
internalUtil.assertCrypto();
60+
const {
61+
isArrayBufferView,
62+
isUint8Array,
63+
} = require('internal/util/types');
64+
65+
const net = require('net');
66+
const { getOptionValue } = require('internal/options');
5967
const { Buffer } = require('buffer');
6068
const { canonicalizeIP } = internalBinding('cares_wrap');
6169
const _tls_common = require('_tls_common');
6270
const _tls_wrap = require('_tls_wrap');
6371
const { validateString } = require('internal/validators');
6472

73+
const {
74+
namespace: {
75+
addDeserializeCallback,
76+
addSerializeCallback,
77+
isBuildingSnapshot,
78+
},
79+
} = require('internal/v8/startup_snapshot');
80+
6581
// Allow {CLIENT_RENEG_LIMIT} client-initiated session renegotiations
6682
// every {CLIENT_RENEG_WINDOW} seconds. An error event is emitted if more
6783
// renegotiations are seen. The settings are applied to all remote client
@@ -203,6 +219,28 @@ function setDefaultCACertificates(certs) {
203219

204220
exports.setDefaultCACertificates = setDefaultCACertificates;
205221

222+
if (isBuildingSnapshot()) {
223+
addSerializeCallback(() => {
224+
// Clear the cached certs so that they are reloaded at runtime.
225+
// Bundled certificates are immutable so they are spared.
226+
extraCACertificates = undefined;
227+
systemCACertificates = undefined;
228+
if (hasResetDefaultCACertificates) {
229+
defaultCACertificates = undefined;
230+
}
231+
});
232+
addDeserializeCallback(() => {
233+
// If the tls module is loaded during snapshotting, load the certificates from
234+
// various sources again at runtime so that by the time an actual TLS connection is
235+
// made, the loading is done. If the default CA certificates have been overridden, then
236+
// the serialized overriding certificates are likely to be used and pre-loading
237+
// from the sources would probably not yield any benefit, so skip it.
238+
if (!hasResetDefaultCACertificates) {
239+
startLoadingCertificatesOffThread();
240+
}
241+
});
242+
}
243+
206244
// Convert protocols array into valid OpenSSL protocols list
207245
// ("\x06spdy/2\x08http/1.1\x08http/1.0")
208246
function convertProtocols(protocols) {

‎src/crypto/crypto_context.cc‎

Lines changed: 58 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -814,23 +814,6 @@ static std::vector<X509*>& GetSystemStoreCACertificates() {
814814
return system_store_certs;
815815
}
816816

817-
static void LoadSystemCACertificates(void* data) {
818-
GetSystemStoreCACertificates();
819-
}
820-
821-
static uv_thread_t system_ca_thread;
822-
static bool system_ca_thread_started = false;
823-
int LoadSystemCACertificatesOffThread() {
824-
// This is only run once during the initialization of the process, so
825-
// it is safe to use a static thread here.
826-
int r =
827-
uv_thread_create(&system_ca_thread, LoadSystemCACertificates, nullptr);
828-
if (r == 0) {
829-
system_ca_thread_started = true;
830-
}
831-
return r;
832-
}
833-
834817
static std::vector<X509*> InitializeExtraCACertificates() {
835818
std::vector<X509*> extra_certs;
836819
unsigned long err = LoadCertsFromFile( // NOLINT(runtime/int)
@@ -854,6 +837,53 @@ static std::vector<X509*>& GetExtraCACertificates() {
854837
return extra_certs;
855838
}
856839

840+
static void LoadCACertificates(void* data) {
841+
per_process::Debug(DebugCategory::CRYPTO,
842+
"Started loading system root certificates off-thread\n");
843+
GetSystemStoreCACertificates();
844+
}
845+
846+
static std::atomic<bool> tried_cert_loading_off_thread = false;
847+
static std::atomic<bool> cert_loading_thread_started = false;
848+
static Mutex start_cert_loading_thread_mutex;
849+
static uv_thread_t cert_loading_thread;
850+
851+
void StartLoadingCertificatesOffThread(
852+
const FunctionCallbackInfo<Value>& args) {
853+
// Load the CA certificates eagerly off the main thread to avoid
854+
// blocking the main thread when the first TLS connection is made. We
855+
// don't need to wait for the thread to finish with code here, as
856+
// Get*CACertificates() functions has a function-local static and any
857+
// actual user of it will wait for that to complete initialization.
858+
859+
{
860+
Mutex::ScopedLock cli_lock(node::per_process::cli_options_mutex);
861+
if (!per_process::cli_options->use_system_ca) {
862+
return;
863+
}
864+
}
865+
866+
// Only try to start the thread once. If it ever fails, we won't try again.
867+
if (tried_cert_loading_off_thread.load()) {
868+
return;
869+
}
870+
{
871+
Mutex::ScopedLock lock(start_cert_loading_thread_mutex);
872+
// Re-check under the lock.
873+
if (tried_cert_loading_off_thread.load()) {
874+
return;
875+
}
876+
tried_cert_loading_off_thread.store(true);
877+
int r = uv_thread_create(&cert_loading_thread, LoadCACertificates, nullptr);
878+
cert_loading_thread_started.store(r == 0);
879+
if (r != 0) {
880+
FPrintF(stderr,
881+
"Warning: Failed to load CA certificates off thread: %s\n",
882+
uv_strerror(r));
883+
}
884+
}
885+
}
886+
857887
// Due to historical reasons the various options of CA certificates
858888
// may invalid one another. The current rule is:
859889
// 1. If the configure-time option --openssl-use-def-ca-store is NOT used
@@ -942,9 +972,12 @@ void CleanupCachedRootCertificates() {
942972
X509_free(cert);
943973
}
944974
}
945-
if (system_ca_thread_started) {
946-
uv_thread_join(&system_ca_thread);
947-
system_ca_thread_started = false;
975+
976+
// Serialize with starter to avoid the race window.
977+
Mutex::ScopedLock lock(start_cert_loading_thread_mutex);
978+
if (tried_cert_loading_off_thread.load() &&
979+
cert_loading_thread_started.load()) {
980+
uv_thread_join(&cert_loading_thread);
948981
}
949982
}
950983

@@ -1233,6 +1266,10 @@ void SecureContext::Initialize(Environment* env, Local<Object> target) {
12331266
SetMethod(context, target, "resetRootCertStore", ResetRootCertStore);
12341267
SetMethodNoSideEffect(
12351268
context, target, "getUserRootCertificates", GetUserRootCertificates);
1269+
SetMethod(context,
1270+
target,
1271+
"startLoadingCertificatesOffThread",
1272+
StartLoadingCertificatesOffThread);
12361273
}
12371274

12381275
void SecureContext::RegisterExternalReferences(
@@ -1277,6 +1314,7 @@ void SecureContext::RegisterExternalReferences(
12771314
registry->Register(GetExtraCACertificates);
12781315
registry->Register(ResetRootCertStore);
12791316
registry->Register(GetUserRootCertificates);
1317+
registry->Register(StartLoadingCertificatesOffThread);
12801318
}
12811319

12821320
SecureContext* SecureContext::Create(Environment* env) {

‎src/crypto/crypto_util.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ void InitCryptoOnce();
4545
void InitCrypto(v8::Local<v8::Object> target);
4646

4747
extern void UseExtraCaCerts(std::string_view file);
48-
extern int LoadSystemCACertificatesOffThread();
4948
void CleanupCachedRootCertificates();
5049

5150
int PasswordCallback(char* buf, int size, int rwflag, void* u);

‎src/debug_utils.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ void NODE_EXTERN_PRIVATE FWrite(FILE* file, const std::string& str);
4242
// from a provider type to a debug category.
4343
#define DEBUG_CATEGORY_NAMES(V) \
4444
NODE_ASYNC_PROVIDER_TYPES(V) \
45+
V(CRYPTO) \
4546
V(COMPILE_CACHE) \
4647
V(DIAGNOSTICS) \
4748
V(HUGEPAGES) \

‎src/node.cc‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,20 +1218,6 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,
12181218
return result;
12191219
}
12201220

1221-
if (per_process::cli_options->use_system_ca) {
1222-
// Load the system CA certificates eagerly off the main thread to avoid
1223-
// blocking the main thread when the first TLS connection is made. We
1224-
// don't need to wait for the thread to finish with code here, as
1225-
// GetSystemStoreCACertificates() has a function-local static and any
1226-
// actual user of it will wait for that to complete initialization.
1227-
int r = crypto::LoadSystemCACertificatesOffThread();
1228-
if (r != 0) {
1229-
FPrintF(
1230-
stderr,
1231-
"Warning: Failed to load system CA certificates off thread: %s\n",
1232-
uv_strerror(r));
1233-
}
1234-
}
12351221
// Ensure CSPRNG is properly seeded.
12361222
CHECK(ncrypto::CSPRNG(nullptr, 0));
12371223

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL