diff --git a/doc/api/cli.md b/doc/api/cli.md index 3822685d3d5..68ba8778014 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 863c468eed1..122b064c2fa 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 d0b5310626c..67dbd172c38 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 d1f7c6e2a80..2ba5e2e0924 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 0c6fa30a41d..eddc64822e8 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 e0069bb08ce..4f8d3d0a815 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 bb45d73e909..3ce3ee26d56 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 00000000000..b03dfa12464 --- /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); + })); +}));