From a19796db20b82d4a5ed5d1613f08f1792e94c057 Mon Sep 17 00:00:00 2001 From: marcopiraccini Date: Sat, 10 Oct 2026 10:11:35 +0200 Subject: [PATCH 1/2] src: honor NODE_EXTRA_CA_CERTS set in an env file NODE_EXTRA_CA_CERTS is read once at startup, before the files passed with --env-file are applied to the environment. A value set in an env file showed up in process.env but the certificates were never loaded. Look the variable up in the parsed env files when it is not in the environment. The environment keeps precedence, and the env files are ignored under the same conditions as the environment: when node runs setuid, setgid or with Linux file capabilities. Fixes: https://github.com/nodejs/node/issues/51426 Refs: https://github.com/nodejs/node/pull/51497 Signed-off-by: marcopiraccini --- doc/api/cli.md | 10 ++- doc/node.1 | 4 +- src/node.cc | 7 +- src/node_credentials.cc | 22 ++++- src/node_dotenv.cc | 15 ++++ src/node_dotenv.h | 3 + src/node_internals.h | 4 + .../test-tls-env-extra-ca-env-file.js | 84 +++++++++++++++++++ 8 files changed, 144 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-tls-env-extra-ca-env-file.js diff --git a/doc/api/cli.md b/doc/api/cli.md index 3822685d3d50..3fafaefc125e 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -4132,6 +4132,11 @@ Disable the [module compile cache][] for the Node.js instance. See the documenta When set, the well known "root" CAs (like VeriSign) will be extended with the @@ -4148,7 +4153,9 @@ has Linux file capabilities set. The `NODE_EXTRA_CA_CERTS` environment variable is only read when the Node.js process is first launched. Changing the value at runtime using -`process.env.NODE_EXTRA_CA_CERTS` has no effect on the current process. +`process.env.NODE_EXTRA_CA_CERTS` has no effect on the current process. This +includes [`process.loadEnvFile()`][]. The variable can be set in a file loaded +with [`--env-file`][] or [`--env-file-if-exists`][], which are read at startup. ### `NODE_ICU_DATA=file` @@ -4940,6 +4947,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12 [`node:stream/iter`]: stream_iter.md [`node:vfs`]: vfs.md [`permission.drop()`]: permissions.md#permissiondropscope-reference +[`process.loadEnvFile()`]: process.md#processloadenvfilepath [`process.setUncaughtExceptionCaptureCallback()`]: process.md#processsetuncaughtexceptioncapturecallbackfn [`session.connectToMainThread()`]: inspector.md#sessionconnecttomainthread [`tls.DEFAULT_MAX_VERSION`]: tls.md#tlsdefault_max_version diff --git a/doc/node.1 b/doc/node.1 index 863c468eed10..122b064c2fac 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -2115,7 +2115,9 @@ This environment variable is ignored when \fBnode\fR runs as setuid root or has Linux file capabilities set. The \fBNODE_EXTRA_CA_CERTS\fR environment variable is only read when the Node.js process is first launched. Changing the value at runtime using -\fBprocess.env.NODE_EXTRA_CA_CERTS\fR has no effect on the current process. +\fBprocess.env.NODE_EXTRA_CA_CERTS\fR has no effect on the current process. This +includes \fBprocess.loadEnvFile()\fR. The variable can be set in a file loaded +with \fB--env-file\fR or \fB--env-file-if-exists\fR, which are read at startup. . .It Ev NODE_ICU_DATA Ar file Data path for ICU (\fBIntl\fR object) data. Will extend linked-in data when compiled diff --git a/src/node.cc b/src/node.cc index d0b5310626c3..67dbd172c386 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1427,9 +1427,14 @@ InitializeOncePerProcessInternal( }); #endif // !defined(OPENSSL_IS_BORINGSSL) { + // The env files are not applied to the environment yet at this point. + // A variable from the real environment takes precedence over them. std::string extra_ca_certs; - if (credentials::SafeGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs)) + if (credentials::SafeGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs) || + credentials::SafeGetenvFromEnvFile("NODE_EXTRA_CA_CERTS", + &extra_ca_certs)) { crypto::UseExtraCaCerts(extra_ca_certs); + } } #endif // HAVE_OPENSSL } diff --git a/src/node_credentials.cc b/src/node_credentials.cc index d1f7c6e2a801..2ba5e2e09247 100644 --- a/src/node_credentials.cc +++ b/src/node_credentials.cc @@ -1,4 +1,5 @@ #include "env-inl.h" +#include "node_dotenv.h" #include "node_errors.h" #include "node_external_reference.h" #include "node_internals.h" @@ -71,7 +72,10 @@ static bool HasOnly(int capability) { // process only has the capability CAP_NET_BIND_SERVICE set. If the current // process does not have any capabilities set and the process is running as // setuid root then lookup will not be allowed. -bool SafeGetenv(const char* key, std::string* text, Environment* env) { +// Whether the process runs with privileges it was not started with: setuid, +// setgid or, on Linux, file capabilities. Configuration that comes from the +// environment is not trusted in that case. +static bool HasElevatedPrivileges() { #if !defined(__CloudABI__) && !defined(_WIN32) #if defined(__linux__) if ((!HasOnly(CAP_NET_BIND_SERVICE) && linux_at_secure()) || @@ -79,8 +83,22 @@ bool SafeGetenv(const char* key, std::string* text, Environment* env) { #else if (linux_at_secure() || getuid() != geteuid() || getgid() != getegid()) #endif - return false; + return true; #endif + return false; +} + +bool SafeGetenvFromEnvFile(const char* key, std::string* text) { + if (HasElevatedPrivileges()) return false; + + std::optional value = per_process::dotenv_file.Get(key); + if (!value.has_value()) return false; + *text = *value; + return true; +} + +bool SafeGetenv(const char* key, std::string* text, Environment* env) { + if (HasElevatedPrivileges()) return false; // Fallback to system environment which reads the real environment variable // through uv_os_getenv. diff --git a/src/node_dotenv.cc b/src/node_dotenv.cc index 0c6fa30a41d7..eddc64822e8a 100644 --- a/src/node_dotenv.cc +++ b/src/node_dotenv.cc @@ -86,6 +86,21 @@ Maybe Dotenv::SetEnvironment(node::Environment* env) { return JustVoid(); } +std::optional Dotenv::Get(const std::string& key) const { +#ifdef _WIN32 + // Environment variable names are case-insensitive on Windows, and so is + // the lookup in process.env once the env files are applied. + for (const auto& [name, value] : store_) { + if (StringEqualNoCase(name.c_str(), key.c_str())) return value; + } + return std::nullopt; +#else + auto match = store_.find(key); + if (match == store_.end()) return std::nullopt; + return match->second; +#endif +} + std::vector Dotenv::GetKeys() const { std::vector keys; keys.reserve(store_.size()); diff --git a/src/node_dotenv.h b/src/node_dotenv.h index e0069bb08ce8..4f8d3d0a8154 100644 --- a/src/node_dotenv.h +++ b/src/node_dotenv.h @@ -7,6 +7,7 @@ #include "v8.h" #include +#include namespace node { @@ -32,6 +33,8 @@ class Dotenv { v8::MaybeLocal ToObject(Environment* env) const; // The names of the variables parsed from the env files. std::vector GetKeys() const; + // The value of a variable parsed from the env files, if it is defined there. + std::optional Get(const std::string& key) const; static std::vector GetDataFromArgs( const std::vector& args); diff --git a/src/node_internals.h b/src/node_internals.h index bb45d73e9090..3ce3ee26d56a 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -333,6 +333,10 @@ class ThreadPoolWork { namespace credentials { bool SafeGetenv(const char* key, std::string* text, Environment* env = nullptr); +// Looks up a variable in the env files passed with --env-file, with the same +// privilege check as SafeGetenv(). For variables that are read before the env +// files are applied to the environment. +bool SafeGetenvFromEnvFile(const char* key, std::string* text); } // namespace credentials void TraceEnvVar(Environment* env, const char* message); diff --git a/test/parallel/test-tls-env-extra-ca-env-file.js b/test/parallel/test-tls-env-extra-ca-env-file.js new file mode 100644 index 000000000000..b03dfa124646 --- /dev/null +++ b/test/parallel/test-tls-env-extra-ca-env-file.js @@ -0,0 +1,84 @@ +'use strict'; + +// NODE_EXTRA_CA_CERTS is honored when it is set in an env file. +// Refs: https://github.com/nodejs/node/issues/51426 + +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const fs = require('fs'); +const tls = require('tls'); +const { fork, spawnSync } = require('child_process'); +const fixtures = require('../common/fixtures'); +const tmpdir = require('../common/tmpdir'); + +if (process.env.CHILD) { + const client = tls.connect({ + port: process.env.PORT, + checkServerIdentity: common.mustCall(), + }, common.mustCall(() => { + client.end('hi'); + })); + return; +} + +tmpdir.refresh(); +const envFile = tmpdir.resolve('extra-ca.env'); +fs.writeFileSync(envFile, + `NODE_EXTRA_CA_CERTS=${fixtures.path('keys', 'ca1-cert.pem')}\n`); + +const env = { ...process.env }; +delete env.NODE_EXTRA_CA_CERTS; + +function extraCertificates(args, extraEnv) { + const child = spawnSync(process.execPath, [ + ...args, + '-p', + 'JSON.stringify(require("tls").getCACertificates("extra"))', + ], { env: { ...env, ...extraEnv }, encoding: 'utf8' }); + assert.strictEqual(child.status, 0, child.stderr); + return JSON.parse(child.stdout); +} + +const ca1 = [fixtures.readKey('ca1-cert.pem', 'utf8').trim()]; +const ca2 = [fixtures.readKey('ca2-cert.pem', 'utf8').trim()]; +const trim = (certs) => certs.map((cert) => cert.trim()); + +assert.deepStrictEqual(extraCertificates([]), []); +assert.deepStrictEqual(trim(extraCertificates([`--env-file=${envFile}`])), ca1); +assert.deepStrictEqual( + trim(extraCertificates([`--env-file-if-exists=${envFile}`])), ca1); + +if (common.isWindows) { + // Environment variable names are case-insensitive on Windows. + const lowerCaseFile = tmpdir.resolve('extra-ca-lower-case.env'); + fs.writeFileSync(lowerCaseFile, + `node_extra_ca_certs=${fixtures.path('keys', 'ca1-cert.pem')}\n`); + assert.deepStrictEqual( + trim(extraCertificates([`--env-file=${lowerCaseFile}`])), ca1); +} + +// The environment takes precedence over the env file. +assert.deepStrictEqual( + trim(extraCertificates([`--env-file=${envFile}`], { + NODE_EXTRA_CA_CERTS: fixtures.path('keys', 'ca2-cert.pem'), + })), ca2); + +// The certificate is used for TLS peer validation. +const server = tls.createServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), +}, common.mustCall((socket) => { + socket.end('bye'); + server.close(); +})).listen(0, common.mustCall(() => { + fork(__filename, { + env: { ...env, CHILD: 'yes', PORT: server.address().port }, + execArgv: [`--env-file=${envFile}`], + }).on('exit', common.mustCall((status) => { + assert.strictEqual(status, 0); + })); +})); From 4f1d36eb98294ab3bed2c93d1e7be19b5dc39d54 Mon Sep 17 00:00:00 2001 From: marcopiraccini Date: Sat, 10 Oct 2026 10:53:40 +0200 Subject: [PATCH 2/2] doc: replace env-file extra CA PR URL placeholder Signed-off-by: marcopiraccini --- doc/api/cli.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 3fafaefc125e..68ba8778014a 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -4134,7 +4134,7 @@ Disable the [module compile cache][] for the Node.js instance. See the documenta added: v7.3.0 changes: - version: REPLACEME - pr-url: https://github.com/nodejs/node/pull/00000 + pr-url: https://github.com/nodejs/node/pull/66638 description: The variable is now honored when set in a file loaded with `--env-file` or `--env-file-if-exists`. -->