src: honor NODE_EXTRA_CA_CERTS set in an env file

doc/api/cli.md

@@ -4132,6 +4132,11 @@ Disable the [module compile cache][] for the Node.js instance. See the documenta
 
 <!-- YAML
 added: v7.3.0
+changes:
+  - version: REPLACEME
+    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`.
 -->
 
 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

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

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
   }

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,16 +72,33 @@ 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()) ||
       getuid() != geteuid() || getgid() != getegid())
 #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<std::string> 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.

src/node_dotenv.cc

@@ -86,6 +86,21 @@ Maybe<void> Dotenv::SetEnvironment(node::Environment* env) {
   return JustVoid();
 }
 
+std::optional<std::string> 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<std::string> Dotenv::GetKeys() const {
   std::vector<std::string> keys;
   keys.reserve(store_.size());

src/node_dotenv.h

@@ -7,6 +7,7 @@
 #include "v8.h"
 
 #include <map>
+#include <optional>
 
 namespace node {
 
@@ -32,6 +33,8 @@ class Dotenv {
   v8::MaybeLocal<v8::Object> ToObject(Environment* env) const;
   // The names of the variables parsed from the env files.
   std::vector<std::string> GetKeys() const;
+  // The value of a variable parsed from the env files, if it is defined there.
+  std::optional<std::string> Get(const std::string& key) const;
 
   static std::vector<env_file_data> GetDataFromArgs(
       const std::vector<std::string>& args);

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);

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);
+  }));
+}));