From 43a9f70d19b9576ae922768dd521915367529009 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Wed, 10 Apr 2024 16:30:35 +0200 Subject: [PATCH] feat: support `NODE_EXTRA_CA_CERTS` (#41689) * feat: support NODE_EXTRA_CA_CERTS * chore: allow disabling NODE_EXTRA_CA_CERTS * chore: call base::Environment::UnSetVar * docs: link to fuses from env vars * chore: update patch to match upstream * docs: note enabled by default * Update environment-variables.md Co-authored-by: John Kleinschmidt --------- Co-authored-by: John Kleinschmidt --- docs/api/environment-variables.md | 14 +++++++++ docs/tutorial/fuses.md | 2 +- ...ling_v8_enablewebassemblytraphandler.patch | 2 +- ...ingssl_and_openssl_incompatibilities.patch | 30 +++++++++++++++++++ script/node-disabled-tests.json | 4 --- shell/app/node_main.cc | 1 + shell/common/node_bindings.cc | 13 ++++++-- 7 files changed, 57 insertions(+), 9 deletions(-) diff --git a/docs/api/environment-variables.md b/docs/api/environment-variables.md index c30d192c85..dd20cae334 100644 --- a/docs/api/environment-variables.md +++ b/docs/api/environment-variables.md @@ -51,6 +51,18 @@ Unsupported options are: --http-parser ``` +If the [`nodeOptions` fuse](../tutorial/fuses.md#L27) is disabled, `NODE_OPTIONS` will be ignored. + +### `NODE_EXTRA_CA_CERTS` + +See [Node.js cli documentation](https://github.com/nodejs/node/blob/main/doc/api/cli.md#node_extra_ca_certsfile) for details. + +```sh +export NODE_EXTRA_CA_CERTS=/path/to/cert.pem +``` + +If the [`nodeOptions` fuse](../tutorial/fuses.md#L27) is disabled, `NODE_EXTRA_CA_CERTS` will be ignored. + ### `GOOGLE_API_KEY` Geolocation support in Electron requires the use of Google Cloud Platform's @@ -92,6 +104,8 @@ you would when running the normal Node.js executable, with the exception of the These flags are disabled owing to the fact that Electron uses BoringSSL instead of OpenSSL when building Node.js' `crypto` module, and so will not work as designed. +If the [`runAsNode` fuse](../tutorial/fuses.md#L13) is disabled, `ELECTRON_RUN_AS_NODE` will be ignored. + ### `ELECTRON_NO_ATTACH_CONSOLE` _Windows_ Don't attach to the current console session. diff --git a/docs/tutorial/fuses.md b/docs/tutorial/fuses.md index de027fe366..920de081f4 100644 --- a/docs/tutorial/fuses.md +++ b/docs/tutorial/fuses.md @@ -29,7 +29,7 @@ The cookieEncryption fuse toggles whether the cookie store on disk is encrypted **Default:** Enabled **@electron/fuses:** `FuseV1Options.EnableNodeOptionsEnvironmentVariable` -The nodeOptions fuse toggles whether the [`NODE_OPTIONS`](https://nodejs.org/api/cli.html#node_optionsoptions) environment variable is respected or not. This environment variable can be used to pass all kinds of custom options to the Node.js runtime and isn't typically used by apps in production. Most apps can safely disable this fuse. +The nodeOptions fuse toggles whether the [`NODE_OPTIONS`](https://nodejs.org/api/cli.html#node_optionsoptions) and [`NODE_EXTRA_CA_CERTS`](https://github.com/nodejs/node/blob/main/doc/api/cli.md#node_extra_ca_certsfile) environment variables are respected. The `NODE_OPTIONS` environment variable can be used to pass all kinds of custom options to the Node.js runtime and isn't typically used by apps in production. Most apps can safely disable this fuse. ### `nodeCliInspect` diff --git a/patches/node/feat_optionally_prevent_calling_v8_enablewebassemblytraphandler.patch b/patches/node/feat_optionally_prevent_calling_v8_enablewebassemblytraphandler.patch index ac21297966..456f87f586 100644 --- a/patches/node/feat_optionally_prevent_calling_v8_enablewebassemblytraphandler.patch +++ b/patches/node/feat_optionally_prevent_calling_v8_enablewebassemblytraphandler.patch @@ -10,7 +10,7 @@ already been called. This should be upstreamed. diff --git a/src/node.cc b/src/node.cc -index 524f80ee69ee5248e045a2b61faf5610c9ba4285..971668792eabe5be299849b5a3fd8a2790a2210a 100644 +index 1d77a8b31cb0bfbeeeac594b6e1ac7dd303c902d..dadddf33527beebfcde12214da4084badbd27af1 100644 --- a/src/node.cc +++ b/src/node.cc @@ -605,6 +605,7 @@ static void PlatformInit(ProcessInitializationFlags::Flags flags) { diff --git a/patches/node/fix_handle_boringssl_and_openssl_incompatibilities.patch b/patches/node/fix_handle_boringssl_and_openssl_incompatibilities.patch index 18c3b4822b..759d19414e 100644 --- a/patches/node/fix_handle_boringssl_and_openssl_incompatibilities.patch +++ b/patches/node/fix_handle_boringssl_and_openssl_incompatibilities.patch @@ -373,6 +373,36 @@ index 5734d8fdc5505e1586f571c19b840bd56e9c9f1f..3034b114e081e2b32dd5b71653927a41 } } // namespace +diff --git a/src/node.cc b/src/node.cc +index 524f80ee69ee5248e045a2b61faf5610c9ba4285..1d77a8b31cb0bfbeeeac594b6e1ac7dd303c902d 100644 +--- a/src/node.cc ++++ b/src/node.cc +@@ -1027,7 +1027,8 @@ InitializeOncePerProcessInternal(const std::vector& args, + } + + if (!(flags & ProcessInitializationFlags::kNoInitOpenSSL)) { +-#if HAVE_OPENSSL && !defined(OPENSSL_IS_BORINGSSL) ++#if HAVE_OPENSSL ++#if !defined(OPENSSL_IS_BORINGSSL) + auto GetOpenSSLErrorString = []() -> std::string { + std::string ret; + ERR_print_errors_cb( +@@ -1127,13 +1128,13 @@ InitializeOncePerProcessInternal(const std::vector& args, + CHECK(crypto::CSPRNG(buffer, length).is_ok()); + return true; + }); +- ++#endif // !defined(OPENSSL_IS_BORINGSSL) + { + std::string extra_ca_certs; + if (credentials::SafeGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs)) + crypto::UseExtraCaCerts(extra_ca_certs); + } +-#endif // HAVE_OPENSSL && !defined(OPENSSL_IS_BORINGSSL) ++#endif // HAVE_OPENSSL + } + + if (!(flags & ProcessInitializationFlags::kNoInitializeNodeV8Platform)) { diff --git a/src/node_metadata.cc b/src/node_metadata.cc index b88cfb98e75aca426224e19376b3ff4c23b92e53..b66f4e2b5cbd8f36af42f82a8921207302360e39 100644 --- a/src/node_metadata.cc diff --git a/script/node-disabled-tests.json b/script/node-disabled-tests.json index c1ed8eeaf3..b05bff18e0 100644 --- a/script/node-disabled-tests.json +++ b/script/node-disabled-tests.json @@ -62,8 +62,6 @@ "parallel/test-snapshot-worker", "parallel/test-strace-openat-openssl", "parallel/test-tls-alpn-server-client", - "parallel/test-tls-cert-chains-concat", - "parallel/test-tls-cert-chains-in-ca", "parallel/test-tls-cli-max-version-1.2", "parallel/test-tls-cli-max-version-1.3", "parallel/test-tls-cli-min-version-1.1", @@ -77,8 +75,6 @@ "parallel/test-tls-cnnic-whitelist", "parallel/test-tls-disable-renegotiation", "parallel/test-tls-empty-sni-context", - "parallel/test-tls-env-bad-extra-ca", - "parallel/test-tls-env-extra-ca", "parallel/test-tls-finished", "parallel/test-tls-generic-stream", "parallel/test-tls-getcipher", diff --git a/shell/app/node_main.cc b/shell/app/node_main.cc index 1c96b02c35..61a3ad4d8c 100644 --- a/shell/app/node_main.cc +++ b/shell/app/node_main.cc @@ -127,6 +127,7 @@ int NodeMain(int argc, char* argv[]) { bool node_options_enabled = electron::fuses::IsNodeOptionsEnabled(); if (!node_options_enabled) { os_env->UnSetVar("NODE_OPTIONS"); + os_env->UnSetVar("NODE_EXTRA_CA_CERTS"); } #if BUILDFLAG(IS_MAC) diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc index cad0ebf057..2ee8c120d6 100644 --- a/shell/common/node_bindings.cc +++ b/shell/common/node_bindings.cc @@ -338,7 +338,7 @@ bool IsAllowedOption(const std::string_view option) { // Initialize NODE_OPTIONS to pass to Node.js // See https://nodejs.org/api/cli.html#cli_node_options_options void SetNodeOptions(base::Environment* env) { - // Options that are unilaterally disallowed + // Options that are expressly disallowed static constexpr auto disallowed = base::MakeFixedFlatSet({ "--enable-fips", "--experimental-policy", @@ -353,6 +353,13 @@ void SetNodeOptions(base::Environment* env) { "--max-http-header-size", }); + if (env->HasVar("NODE_EXTRA_CA_CERTS")) { + if (!electron::fuses::IsNodeOptionsEnabled()) { + LOG(WARNING) << "NODE_OPTIONS ignored due to disabled nodeOptions fuse."; + env->UnSetVar("NODE_EXTRA_CA_CERTS"); + } + } + if (env->HasVar("NODE_OPTIONS")) { if (electron::fuses::IsNodeOptionsEnabled()) { std::string options; @@ -383,8 +390,8 @@ void SetNodeOptions(base::Environment* env) { // overwrite new NODE_OPTIONS without unsupported variables env->SetVar("NODE_OPTIONS", options); } else { - LOG(ERROR) << "NODE_OPTIONS have been disabled in this app"; - env->SetVar("NODE_OPTIONS", ""); + LOG(WARNING) << "NODE_OPTIONS ignored due to disabled nodeOptions fuse."; + env->UnSetVar("NODE_OPTIONS"); } } }