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@095e7a8 · GitHub

/ node Public

Commit 095e7a8

Browse files
joyeecheung
authored and
aduh95
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 2870a73 commit 095e7a8

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,30 +39,46 @@ 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 { createSecurePair } = require('internal/tls/secure-pair');
6472
const { validateString } = require('internal/validators');
6573

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

205221
exports.setDefaultCACertificates = setDefaultCACertificates;
206222

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

‎src/crypto/crypto_context.cc‎

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

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

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

@@ -1231,6 +1264,10 @@ void SecureContext::Initialize(Environment* env, Local<Object> target) {
12311264
SetMethod(context, target, "resetRootCertStore", ResetRootCertStore);
12321265
SetMethodNoSideEffect(
12331266
context, target, "getUserRootCertificates", GetUserRootCertificates);
1267+
SetMethod(context,
1268+
target,
1269+
"startLoadingCertificatesOffThread",
1270+
StartLoadingCertificatesOffThread);
12341271
}
12351272

12361273
void SecureContext::RegisterExternalReferences(
@@ -1275,6 +1312,7 @@ void SecureContext::RegisterExternalReferences(
12751312
registry->Register(GetExtraCACertificates);
12761313
registry->Register(ResetRootCertStore);
12771314
registry->Register(GetUserRootCertificates);
1315+
registry->Register(StartLoadingCertificatesOffThread);
12781316
}
12791317

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

‎src/crypto/crypto_util.h‎

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

6464
extern void UseExtraCaCerts(const std::string& file);
65-
extern int LoadSystemCACertificatesOffThread();
6665
void CleanupCachedRootCertificates();
6766

6867
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
@@ -1255,20 +1255,6 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,
12551255
return result;
12561256
}
12571257

1258-
if (per_process::cli_options->use_system_ca) {
1259-
// Load the system CA certificates eagerly off the main thread to avoid
1260-
// blocking the main thread when the first TLS connection is made. We
1261-
// don't need to wait for the thread to finish with code here, as
1262-
// GetSystemStoreCACertificates() has a function-local static and any
1263-
// actual user of it will wait for that to complete initialization.
1264-
int r = crypto::LoadSystemCACertificatesOffThread();
1265-
if (r != 0) {
1266-
FPrintF(
1267-
stderr,
1268-
"Warning: Failed to load system CA certificates off thread: %s\n",
1269-
uv_strerror(r));
1270-
}
1271-
}
12721258
// Ensure CSPRNG is properly seeded.
12731259
CHECK(ncrypto::CSPRNG(nullptr, 0));
12741260

0 commit comments

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