Skip to content

Commit

Permalink
feat: support NODE_EXTRA_CA_CERTS (#41689)
Browse files Browse the repository at this point in the history
* 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 <jkleinsc@electronjs.org>

---------

Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
  • Loading branch information
codebytere and jkleinsc committed Apr 10, 2024
1 parent 0a7df0e commit 43a9f70
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 9 deletions.
14 changes: 14 additions & 0 deletions docs/api/environment-variables.md
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion docs/tutorial/fuses.md
Expand Up @@ -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`

Expand Down
Expand Up @@ -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) {
Expand Down
Expand Up @@ -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<std::string>& 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<std::string>& 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
Expand Down
4 changes: 0 additions & 4 deletions script/node-disabled-tests.json
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
1 change: 1 addition & 0 deletions shell/app/node_main.cc
Expand Up @@ -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)
Expand Down
13 changes: 10 additions & 3 deletions shell/common/node_bindings.cc
Expand Up @@ -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<std::string_view>({
"--enable-fips",
"--experimental-policy",
Expand All @@ -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;
Expand Down Expand Up @@ -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");
}
}
}
Expand Down

0 comments on commit 43a9f70

Please sign in to comment.