GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — crypto: make --use-system-ca per-env rather than per-process · nodejs/node@ff485a9

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
  • src
    • crypto
      • crypto_common.cc
      • crypto_context.cc
      • crypto_context.h
    • node.cc
    • node_options.cc
    • node_options.h
    • quic
      • endpoint.cc
      • tlscontext.cc
      • tlscontext.h
  • test
    • cctest
      • test_node_crypto.cc
    • fixtures
      • system-ca
        • fetch-worker.mjs
      • tls-get-ca-certificates-worker.js
    • parallel
      • test-cli-node-options.js
      • test-openssl-ca-options.js
      • test-tls-get-ca-certificates-worker-no-use-system-ca.js
      • test-tls-get-ca-certificates-worker-use-system-ca.js
    • system-ca
      • test-use-system-ca-worker-disable.mjs
      • test-use-system-ca-worker-enable.mjs
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/crypto/crypto_common.cc‎

Copy file name to clipboardExpand all lines: src/crypto/crypto_common.cc
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ MaybeLocal<Value> GetValidationErrorReason(Environment* env, int err) {
6161
(err == X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE) ||
6262
(err == X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT) ||
6363
((err == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT) &&
64-
!per_process::cli_options->use_system_ca);
64+
!env->options()->use_system_ca);
6565

6666
if (suggest_system_ca) {
6767
reason.append("; if the root CA is installed locally, "
Collapse file

‎src/crypto/crypto_context.cc‎

Copy file name to clipboardExpand all lines: src/crypto/crypto_context.cc
+113-41Lines changed: 113 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -103,12 +103,39 @@ static thread_local X509_STORE* root_cert_store = nullptr;
103103
// copy generated by NewRootCertStore() will then contain the certificates
104104
// from this set.
105105
static thread_local std::unique_ptr<X509Set> root_certs_from_users;
106+
static thread_local bool has_cleanup_hook = false;
106107

107-
X509_STORE* GetOrCreateRootCertStore() {
108+
static void CleanupRootCertStore(void*) {
109+
if (root_cert_store != nullptr) {
110+
X509_STORE_free(root_cert_store);
111+
root_cert_store = nullptr;
112+
}
113+
114+
if (root_certs_from_users != nullptr) {
115+
for (X509* cert : *root_certs_from_users) {
116+
X509_free(cert);
117+
}
118+
root_certs_from_users.reset();
119+
}
120+
121+
has_cleanup_hook = false;
122+
}
123+
124+
static void EnsureRootCertStoreCleanupHook(Environment* env) {
125+
if (env == nullptr || has_cleanup_hook) {
126+
return;
127+
}
128+
129+
env->AddCleanupHook(CleanupRootCertStore, nullptr);
130+
has_cleanup_hook = true;
131+
}
132+
133+
X509_STORE* GetOrCreateRootCertStore(Environment* env) {
134+
EnsureRootCertStoreCleanupHook(env);
108135
if (root_cert_store != nullptr) {
109136
return root_cert_store;
110137
}
111-
root_cert_store = NewRootCertStore();
138+
root_cert_store = NewRootCertStore(env);
112139
return root_cert_store;
113140
}
114141

@@ -932,23 +959,22 @@ static void LoadCACertificates(void* data) {
932959
"Started loading extra root certificates off-thread\n");
933960
GetExtraCACertificates();
934961
}
962+
}
935963

936-
{
937-
Mutex::ScopedLock cli_lock(node::per_process::cli_options_mutex);
938-
if (!per_process::cli_options->use_system_ca) {
939-
return;
940-
}
941-
}
942-
964+
static void LoadSystemCACertificates(void* data) {
943965
per_process::Debug(DebugCategory::CRYPTO,
944966
"Started loading system root certificates off-thread\n");
945967
GetSystemStoreCACertificates();
946968
}
947969

948970
static std::atomic<bool> tried_cert_loading_off_thread = false;
949971
static std::atomic<bool> cert_loading_thread_started = false;
972+
static std::atomic<bool> tried_system_cert_loading_off_thread = false;
973+
static std::atomic<bool> system_cert_loading_thread_started = false;
950974
static Mutex start_cert_loading_thread_mutex;
975+
static Mutex start_system_cert_loading_thread_mutex;
951976
static uv_thread_t cert_loading_thread;
977+
static uv_thread_t system_cert_loading_thread;
952978

953979
void StartLoadingCertificatesOffThread(
954980
const FunctionCallbackInfo<Value>& args) {
@@ -968,23 +994,46 @@ void StartLoadingCertificatesOffThread(
968994
}
969995
}
970996

997+
Environment* env = Environment::GetCurrent(args);
998+
const bool use_system_ca = env != nullptr && env->options()->use_system_ca;
999+
per_process::Debug(
1000+
DebugCategory::CRYPTO, "StartLoadingCertificatesOffThread env=%p\n", env);
9711001
// Only try to start the thread once. If it ever fails, we won't try again.
972-
if (tried_cert_loading_off_thread.load()) {
973-
return;
974-
}
975-
{
1002+
// Quick check, if it's already tried, no need to lock.
1003+
if (!tried_cert_loading_off_thread.load()) {
9761004
Mutex::ScopedLock lock(start_cert_loading_thread_mutex);
977-
// Re-check under the lock.
978-
if (tried_cert_loading_off_thread.load()) {
979-
return;
1005+
// Check again under the lock.
1006+
if (!tried_cert_loading_off_thread.load()) {
1007+
tried_cert_loading_off_thread.store(true);
1008+
int r =
1009+
uv_thread_create(&cert_loading_thread, LoadCACertificates, nullptr);
1010+
cert_loading_thread_started.store(r == 0);
1011+
if (r != 0) {
1012+
FPrintF(stderr,
1013+
"Warning: Failed to load CA certificates off thread: %s\n",
1014+
uv_strerror(r));
1015+
}
9801016
}
981-
tried_cert_loading_off_thread.store(true);
982-
int r = uv_thread_create(&cert_loading_thread, LoadCACertificates, nullptr);
983-
cert_loading_thread_started.store(r == 0);
984-
if (r != 0) {
985-
FPrintF(stderr,
986-
"Warning: Failed to load CA certificates off thread: %s\n",
987-
uv_strerror(r));
1017+
}
1018+
1019+
// If the system CA list hasn't been loaded off-thread yet, allow a worker
1020+
// enabling --use-system-ca to trigger its off-thread loading.
1021+
// Quick check, if it's already tried, no need to lock.
1022+
if (use_system_ca && !has_cached_system_root_certs.load() &&
1023+
!tried_system_cert_loading_off_thread.load()) {
1024+
Mutex::ScopedLock lock(start_system_cert_loading_thread_mutex);
1025+
if (!has_cached_system_root_certs.load() &&
1026+
!tried_system_cert_loading_off_thread.load()) {
1027+
tried_system_cert_loading_off_thread.store(true);
1028+
int r = uv_thread_create(
1029+
&system_cert_loading_thread, LoadSystemCACertificates, nullptr);
1030+
system_cert_loading_thread_started.store(r == 0);
1031+
if (r != 0) {
1032+
FPrintF(
1033+
stderr,
1034+
"Warning: Failed to load system CA certificates off thread: %s\n",
1035+
uv_strerror(r));
1036+
}
9881037
}
9891038
}
9901039
}
@@ -1009,13 +1058,13 @@ void StartLoadingCertificatesOffThread(
10091058
// with all the other flags.
10101059
// 7. Certificates from --use-bundled-ca, --use-system-ca and
10111060
// NODE_EXTRA_CA_CERTS are cached after first load. Certificates
1012-
// from --use-system-ca are not cached and always reloaded from
1061+
// from --use-openssl-ca are not cached and always reloaded from
10131062
// disk.
10141063
// 8. If users have reset the root cert store by calling
10151064
// tls.setDefaultCACertificates(), the store will be populated with
10161065
// the certificates provided by users.
10171066
// TODO(joyeecheung): maybe these rules need a bit of consolidation?
1018-
X509_STORE* NewRootCertStore() {
1067+
X509_STORE* NewRootCertStore(Environment* env) {
10191068
X509_STORE* store = X509_STORE_new();
10201069
CHECK_NOT_NULL(store);
10211070

@@ -1037,14 +1086,26 @@ X509_STORE* NewRootCertStore() {
10371086
}
10381087
#endif
10391088

1040-
Mutex::ScopedLock cli_lock(node::per_process::cli_options_mutex);
1041-
if (per_process::cli_options->ssl_openssl_cert_store) {
1089+
bool use_system_ca = false;
1090+
bool ssl_openssl_cert_store = false;
1091+
{
1092+
Mutex::ScopedLock cli_lock(node::per_process::cli_options_mutex);
1093+
ssl_openssl_cert_store = per_process::cli_options->ssl_openssl_cert_store;
1094+
if (env != nullptr) {
1095+
use_system_ca = env->options()->use_system_ca;
1096+
} else if (per_process::cli_options->per_isolate != nullptr &&
1097+
per_process::cli_options->per_isolate->per_env != nullptr) {
1098+
use_system_ca =
1099+
per_process::cli_options->per_isolate->per_env->use_system_ca;
1100+
}
1101+
}
1102+
if (ssl_openssl_cert_store) {
10421103
CHECK_EQ(1, X509_STORE_set_default_paths(store));
10431104
} else {
10441105
for (X509* cert : GetBundledRootCertificates()) {
10451106
CHECK_EQ(1, X509_STORE_add_cert(store, cert));
10461107
}
1047-
if (per_process::cli_options->use_system_ca) {
1108+
if (use_system_ca) {
10481109
for (X509* cert : GetSystemStoreCACertificates()) {
10491110
CHECK_EQ(1, X509_STORE_add_cert(store, cert));
10501111
}
@@ -1061,6 +1122,22 @@ X509_STORE* NewRootCertStore() {
10611122
}
10621123

10631124
void CleanupCachedRootCertificates() {
1125+
// Serialize with starters to avoid the race window.
1126+
{
1127+
Mutex::ScopedLock lock(start_cert_loading_thread_mutex);
1128+
if (tried_cert_loading_off_thread.load() &&
1129+
cert_loading_thread_started.load()) {
1130+
uv_thread_join(&cert_loading_thread);
1131+
}
1132+
}
1133+
{
1134+
Mutex::ScopedLock lock(start_system_cert_loading_thread_mutex);
1135+
if (tried_system_cert_loading_off_thread.load() &&
1136+
system_cert_loading_thread_started.load()) {
1137+
uv_thread_join(&system_cert_loading_thread);
1138+
}
1139+
}
1140+
10641141
if (has_cached_bundled_root_certs.load()) {
10651142
for (X509* cert : GetBundledRootCertificates()) {
10661143
X509_free(cert);
@@ -1077,13 +1154,6 @@ void CleanupCachedRootCertificates() {
10771154
X509_free(cert);
10781155
}
10791156
}
1080-
1081-
// Serialize with starter to avoid the race window.
1082-
Mutex::ScopedLock lock(start_cert_loading_thread_mutex);
1083-
if (tried_cert_loading_off_thread.load() &&
1084-
cert_loading_thread_started.load()) {
1085-
uv_thread_join(&cert_loading_thread);
1086-
}
10871157
}
10881158

10891159
void GetBundledRootCertificates(const FunctionCallbackInfo<Value>& args) {
@@ -1195,6 +1265,8 @@ void ResetRootCertStore(const FunctionCallbackInfo<Value>& args) {
11951265
Local<Context> context = args.GetIsolate()->GetCurrentContext();
11961266
CHECK(args[0]->IsArray());
11971267
Local<Array> cert_array = args[0].As<Array>();
1268+
Environment* env = Environment::GetCurrent(context);
1269+
EnsureRootCertStoreCleanupHook(env);
11981270

11991271
if (cert_array->Length() == 0) {
12001272
// If the array is empty, just clear the user certs and reset the store.
@@ -1249,9 +1321,7 @@ void ResetRootCertStore(const FunctionCallbackInfo<Value>& args) {
12491321
X509_STORE_free(root_cert_store);
12501322
}
12511323

1252-
// TODO(joyeecheung): we can probably just reset it to nullptr
1253-
// and let the next call to NewRootCertStore() create a new one.
1254-
root_cert_store = NewRootCertStore();
1324+
root_cert_store = nullptr;
12551325
}
12561326

12571327
void GetSystemCACertificates(const FunctionCallbackInfo<Value>& args) {
@@ -1778,11 +1848,12 @@ void SecureContext::SetX509StoreFlag(unsigned long flags) {
17781848
}
17791849

17801850
X509_STORE* SecureContext::GetCertStoreOwnedByThisSecureContext() {
1851+
Environment* env = this->env();
17811852
if (own_cert_store_cache_ != nullptr) return own_cert_store_cache_;
17821853

17831854
X509_STORE* cert_store = SSL_CTX_get_cert_store(ctx_.get());
1784-
if (cert_store == GetOrCreateRootCertStore()) {
1785-
cert_store = NewRootCertStore();
1855+
if (cert_store == GetOrCreateRootCertStore(env)) {
1856+
cert_store = NewRootCertStore(env);
17861857
SSL_CTX_set_cert_store(ctx_.get(), cert_store);
17871858
}
17881859

@@ -1855,7 +1926,8 @@ void SecureContext::AddCRL(const FunctionCallbackInfo<Value>& args) {
18551926

18561927
void SecureContext::SetRootCerts() {
18571928
ClearErrorOnReturn clear_error_on_return;
1858-
auto store = GetOrCreateRootCertStore();
1929+
Environment* env = this->env();
1930+
auto store = GetOrCreateRootCertStore(env);
18591931

18601932
// Increment reference count so global store is not deleted along with CTX.
18611933
X509_STORE_up_ref(store);
Collapse file

‎src/crypto/crypto_context.h‎

Copy file name to clipboardExpand all lines: src/crypto/crypto_context.h
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ constexpr int kMaxSupportedVersion = TLS1_3_VERSION;
2323
void GetRootCertificates(
2424
const v8::FunctionCallbackInfo<v8::Value>& args);
2525

26-
X509_STORE* NewRootCertStore();
26+
X509_STORE* NewRootCertStore(Environment* env);
2727

28-
X509_STORE* GetOrCreateRootCertStore();
28+
X509_STORE* GetOrCreateRootCertStore(Environment* env);
2929

3030
ncrypto::BIOPointer LoadBIO(Environment* env, v8::Local<v8::Value> v);
3131

Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc
-9Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -867,15 +867,6 @@ static ExitCode InitializeNodeWithArgsInternal(
867867
// default value.
868868
V8::SetFlagsFromString("--rehash-snapshot");
869869

870-
#if HAVE_OPENSSL
871-
// TODO(joyeecheung): make this a per-env option and move the normalization
872-
// into HandleEnvOptions.
873-
std::string use_system_ca;
874-
if (credentials::SafeGetenv("NODE_USE_SYSTEM_CA", &use_system_ca) &&
875-
use_system_ca == "1") {
876-
per_process::cli_options->use_system_ca = true;
877-
}
878-
#endif // HAVE_OPENSSL
879870
HandleEnvOptions(per_process::cli_options->per_isolate->per_env);
880871

881872
std::string node_options;
Collapse file

‎src/node_options.cc‎

Copy file name to clipboardExpand all lines: src/node_options.cc
+18-4Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,13 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors,
199199
"used, not both");
200200
}
201201

202+
#if HAVE_OPENSSL
203+
if (use_system_ca && per_process::cli_options->use_openssl_ca) {
204+
errors->push_back("either --use-openssl-ca or --use-system-ca can be "
205+
"used, not both");
206+
}
207+
#endif // HAVE_OPENSSL
208+
202209
if (heap_snapshot_near_heap_limit < 0) {
203210
errors->push_back("--heapsnapshot-near-heap-limit must not be negative");
204211
}
@@ -1052,6 +1059,13 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
10521059
&EnvironmentOptions::trace_env_native_stack,
10531060
kAllowedInEnvvar);
10541061

1062+
#if HAVE_OPENSSL
1063+
AddOption("--use-system-ca",
1064+
"use system's CA store",
1065+
&EnvironmentOptions::use_system_ca,
1066+
kAllowedInEnvvar);
1067+
#endif // HAVE_OPENSSL
1068+
10551069
AddOption(
10561070
"--trace-require-module",
10571071
"Print access to require(esm). Options are 'all' (print all usage) and "
@@ -1394,10 +1408,6 @@ PerProcessOptionsParser::PerProcessOptionsParser(
13941408
,
13951409
&PerProcessOptions::use_openssl_ca,
13961410
kAllowedInEnvvar);
1397-
AddOption("--use-system-ca",
1398-
"use system's CA store",
1399-
&PerProcessOptions::use_system_ca,
1400-
kAllowedInEnvvar);
14011411
AddOption("--use-bundled-ca",
14021412
"use bundled CA store"
14031413
#if !defined(NODE_OPENSSL_CERT_STORE)
@@ -2160,6 +2170,10 @@ void HandleEnvOptions(std::shared_ptr<EnvironmentOptions> env_options,
21602170

21612171
env_options->use_env_proxy = opt_getter("NODE_USE_ENV_PROXY") == "1";
21622172

2173+
#if HAVE_OPENSSL
2174+
env_options->use_system_ca = opt_getter("NODE_USE_SYSTEM_CA") == "1";
2175+
#endif // HAVE_OPENSSL
2176+
21632177
if (env_options->redirect_warnings.empty())
21642178
env_options->redirect_warnings = opt_getter("NODE_REDIRECT_WARNINGS");
21652179
}
Collapse file

‎src/node_options.h‎

Copy file name to clipboardExpand all lines: src/node_options.h
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ class EnvironmentOptions : public Options {
229229
bool trace_env = false;
230230
bool trace_env_js_stack = false;
231231
bool trace_env_native_stack = false;
232+
bool use_system_ca = false;
232233
std::string trace_require_module;
233234
bool extra_info_on_fatal_exception = true;
234235
std::string unhandled_rejections;
@@ -364,7 +365,6 @@ class PerProcessOptions : public Options {
364365
bool ssl_openssl_cert_store = false;
365366
#endif
366367
bool use_openssl_ca = false;
367-
bool use_system_ca = false;
368368
bool use_bundled_ca = false;
369369
bool enable_fips_crypto = false;
370370
bool force_fips_crypto = false;
Collapse file

‎src/quic/endpoint.cc‎

Copy file name to clipboardExpand all lines: src/quic/endpoint.cc
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -932,7 +932,7 @@ void Endpoint::Listen(const Session::Options& options) {
932932
"not what you want.");
933933
}
934934

935-
auto context = TLSContext::CreateServer(options.tls_options);
935+
auto context = TLSContext::CreateServer(env(), options.tls_options);
936936
if (!*context) {
937937
THROW_ERR_INVALID_STATE(
938938
env(), "Failed to create TLS context: %s", context->validation_error());
@@ -974,7 +974,7 @@ BaseObjectPtr<Session> Endpoint::Connect(
974974
config,
975975
session_ticket.has_value() ? "yes" : "no");
976976

977-
auto tls_context = TLSContext::CreateClient(options.tls_options);
977+
auto tls_context = TLSContext::CreateClient(env(), options.tls_options);
978978
if (!*tls_context) {
979979
THROW_ERR_INVALID_STATE(env(),
980980
"Failed to create TLS context: %s",

0 commit comments

Comments
0 (0)
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.