Skip to content

Commit

Permalink
src: sync access for report and openssl options
Browse files Browse the repository at this point in the history
Audited usage of per-process OpenSSL and Report options, adding two
required mutexes.

Also documented existence and typical use of the per-process cli option
mutex.

PR-URL: #32618
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
  • Loading branch information
sam-github authored and targos committed Apr 11, 2020
1 parent 756a049 commit 1763649
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 5 deletions.
2 changes: 2 additions & 0 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -993,6 +993,8 @@ static X509_STORE* NewRootCertStore() {
if (*system_cert_path != '\0') {
X509_STORE_load_locations(store, system_cert_path, nullptr);
}

Mutex::ScopedLock cli_lock(node::per_process::cli_options_mutex);
if (per_process::cli_options->ssl_openssl_cert_store) {
X509_STORE_set_default_paths(store);
} else {
Expand Down
8 changes: 7 additions & 1 deletion src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,13 @@ void OnFatalError(const char* location, const char* message) {

Isolate* isolate = Isolate::GetCurrent();
Environment* env = Environment::GetCurrent(isolate);
if (per_process::cli_options->report_on_fatalerror) {
bool report_on_fatalerror;
{
Mutex::ScopedLock lock(node::per_process::cli_options_mutex);
report_on_fatalerror = per_process::cli_options->report_on_fatalerror;
}

if (report_on_fatalerror) {
report::TriggerNodeReport(
isolate, env, message, "FatalError", "", Local<String>());
}
Expand Down
21 changes: 17 additions & 4 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,14 @@ class PerIsolateOptions : public Options {

class PerProcessOptions : public Options {
public:
// Options shouldn't be here unless they affect the entire process scope, and
// that should avoided when possible.
//
// When an option is used during process initialization, it does not need
// protection, but any use after that will likely require synchronization
// using the node::per_process::cli_options_mutex, typically:
//
// Mutex::ScopedLock lock(node::per_process::cli_options_mutex);
std::shared_ptr<PerIsolateOptions> per_isolate { new PerIsolateOptions() };

std::string title;
Expand All @@ -213,7 +221,8 @@ class PerProcessOptions : public Options {
std::string icu_data_dir;
#endif

// TODO(addaleax): Some of these could probably be per-Environment.
// Per-process because they affect singleton OpenSSL shared library state,
// or are used once during process intialization.
#if HAVE_OPENSSL
std::string openssl_config;
std::string tls_cipher_list = DEFAULT_CIPHER_LIST_CORE;
Expand All @@ -229,14 +238,18 @@ class PerProcessOptions : public Options {
bool force_fips_crypto = false;
#endif
#endif
std::string use_largepages = "off";
bool trace_sigint = false;
std::vector<std::string> cmdline;

// Per-process because reports can be triggered outside a known V8 context.
bool report_on_fatalerror = false;
bool report_compact = false;
std::string report_directory;
std::string report_filename;

// TODO(addaleax): Some of these could probably be per-Environment.
std::string use_largepages = "off";
bool trace_sigint = false;
std::vector<std::string> cmdline;

inline PerIsolateOptions* get_per_isolate_options();
void CheckOptions(std::vector<std::string>* errors) override;
};
Expand Down

0 comments on commit 1763649

Please sign in to comment.