7.x backport: crypto: support OPENSSL_CONF again (and its dependencies)

doc/api/cli.md

@@ -363,6 +363,18 @@ misformatted, but any errors are otherwise ignored.
 Note that neither the well known nor extra certificates are used when the `ca`
 options property is explicitly specified for a TLS or HTTPS client or server.
 
+### `OPENSSL_CONF=file`
+<!-- YAML
+added: REPLACEME
+-->
+
+Load an OpenSSL configuration file on startup. Among other uses, this can be
+used to enable FIPS-compliant crypto if Node.js is built with `./configure
+\-\-openssl\-fips`.
+
+If the [`--openssl-config`][] command line option is used, the environment
+variable is ignored.
+
 ### `SSL_CERT_DIR=dir`
 
 If `--use-openssl-ca` is enabled, this overrides and sets OpenSSL's directory
@@ -386,3 +398,4 @@ OpenSSL, it may cause them to trust the same CAs as node.
 [debugger]: debugger.html
 [REPL]: repl.html
 [SlowBuffer]: buffer.html#buffer_class_slowbuffer
+[`--openssl-config`]: #cli_openssl_config_file

doc/node.1

@@ -243,6 +243,16 @@ asynchronous when outputting to a TTY on platforms which support async stdio.
 Setting this will void any guarantee that stdio will not be interleaved or
 dropped at program exit. \fBAvoid use.\fR
 
+.TP
+.BR OPENSSL_CONF = \fIfile\fR
+Load an OpenSSL configuration file on startup. Among other uses, this can be
+used to enable FIPS-compliant crypto if Node.js is built with
+\fB./configure \-\-openssl\-fips\fR.
+
+If the
+\fB\-\-openssl\-config\fR
+command line option is used, the environment variable is ignored.
+
 .TP
 .BR SSL_CERT_DIR = \fIdir\fR
 If \fB\-\-use\-openssl\-ca\fR is enabled, this overrides and sets OpenSSL's directory

lib/internal/process.js

@@ -122,7 +122,7 @@ function setupConfig(_source) {
     const oldV8BreakIterator = Intl.v8BreakIterator;
     const des = Object.getOwnPropertyDescriptor(Intl, 'v8BreakIterator');
     des.value = require('internal/util').deprecate(function v8BreakIterator() {
-      if (processConfig.hasSmallICU && !process.icu_data_dir) {
+      if (processConfig.hasSmallICU && !processConfig.icuDataDir) {
         // Intl.v8BreakIterator() would crash w/ fatal error, so throw instead.
         throw new Error('v8BreakIterator: full ICU data not installed. ' +
                         'See https://github.com/nodejs/node/wiki/Intl');
@@ -131,8 +131,6 @@ function setupConfig(_source) {
     }, 'Intl.v8BreakIterator is deprecated and will be removed soon.');
     Object.defineProperty(Intl, 'v8BreakIterator', des);
   }
-  // Don’t let icu_data_dir leak through.
-  delete process.icu_data_dir;
 }
 
 

src/node.cc

@@ -154,7 +154,7 @@ static node_module* modlist_addon;
 
 #if defined(NODE_HAVE_I18N_SUPPORT)
 // Path to ICU data (for i18n / Intl)
-static const char* icu_data_dir = nullptr;
+std::string icu_data_dir;  // NOLINT(runtime/string)
 #endif
 
 // used by C++ modules as well
@@ -174,7 +174,7 @@ bool ssl_openssl_cert_store =
 bool enable_fips_crypto = false;
 bool force_fips_crypto = false;
 # endif  // NODE_FIPS_MODE
-const char* openssl_config = nullptr;
+std::string openssl_config;  // NOLINT(runtime/string)
 #endif  // HAVE_OPENSSL
 
 // true if process warnings should be suppressed
@@ -901,12 +901,21 @@ Local<Value> UVException(Isolate* isolate,
 
 
 // Look up environment variable unless running as setuid root.
-inline const char* secure_getenv(const char* key) {
+bool SafeGetenv(const char* key, std::string* text) {
 #ifndef _WIN32
-  if (getuid() != geteuid() || getgid() != getegid())
-    return nullptr;
+  // TODO(bnoordhuis) Should perhaps also check whether getauxval(AT_SECURE)
+  // is non-zero on Linux.
+  if (getuid() != geteuid() || getgid() != getegid()) {
+    text->clear();
+    return false;
+  }
 #endif
-  return getenv(key);
+  if (const char* value = getenv(key)) {
+    *text = value;
+    return true;
+  }
+  text->clear();
+  return false;
 }
 
 
@@ -3060,17 +3069,6 @@ void SetupProcessObject(Environment* env,
                     "ares",
                     FIXED_ONE_BYTE_STRING(env->isolate(), ARES_VERSION_STR));
 
-#if defined(NODE_HAVE_I18N_SUPPORT) && defined(U_ICU_VERSION)
-  // ICU-related versions are now handled on the js side, see bootstrap_node.js
-
-  if (icu_data_dir != nullptr) {
-    // Did the user attempt (via env var or parameter) to set an ICU path?
-    READONLY_PROPERTY(process,
-                      "icu_data_dir",
-                      OneByteString(env->isolate(), icu_data_dir));
-  }
-#endif
-
   const char node_modules_version[] = NODE_STRINGIFY(NODE_MODULE_VERSION);
   READONLY_PROPERTY(
       versions,
@@ -3521,8 +3519,9 @@ static void PrintHelp() {
          "  --enable-fips              enable FIPS crypto at startup\n"
          "  --force-fips               force FIPS crypto (cannot be disabled)\n"
 #endif  /* NODE_FIPS_MODE */
-         "  --openssl-config=path      load OpenSSL configuration file from\n"
-         "                             the specified path\n"
+         "  --openssl-config=file      load OpenSSL configuration from the\n"
+         "                             specified file (overrides\n"
+         "                             OPENSSL_CONF)\n"
 #endif /* HAVE_OPENSSL */
 #if defined(NODE_HAVE_I18N_SUPPORT)
          "  --icu-data-dir=dir         set ICU data load path to dir\n"
@@ -3555,6 +3554,8 @@ static void PrintHelp() {
          "                             prefixed to the module search path\n"
          "NODE_REPL_HISTORY            path to the persistent REPL history\n"
          "                             file\n"
+         "OPENSSL_CONF                 load OpenSSL configuration from file\n"
+         "\n"
          "Documentation can be found at https://nodejs.org/\n");
 }
 
@@ -3692,11 +3693,11 @@ static void ParseArgs(int* argc,
       force_fips_crypto = true;
 #endif /* NODE_FIPS_MODE */
     } else if (strncmp(arg, "--openssl-config=", 17) == 0) {
-      openssl_config = arg + 17;
+      openssl_config.assign(arg + 17);
 #endif /* HAVE_OPENSSL */
 #if defined(NODE_HAVE_I18N_SUPPORT)
     } else if (strncmp(arg, "--icu-data-dir=", 15) == 0) {
-      icu_data_dir = arg + 15;
+      icu_data_dir.assign(arg + 15);
 #endif
     } else if (strcmp(arg, "--expose-internals") == 0 ||
                strcmp(arg, "--expose_internals") == 0) {
@@ -4183,10 +4184,15 @@ void Init(int* argc,
 #endif
 
   // Allow for environment set preserving symlinks.
-  if (auto preserve_symlinks = secure_getenv("NODE_PRESERVE_SYMLINKS")) {
-    config_preserve_symlinks = (*preserve_symlinks == '1');
+  {
+    std::string text;
+    config_preserve_symlinks =
+        SafeGetenv("NODE_PRESERVE_SYMLINKS", &text) && text[0] == '1';
   }
 
+  if (openssl_config.empty())
+    SafeGetenv("OPENSSL_CONF", &openssl_config);
+
   // Parse a few arguments which are specific to Node.
   int v8_argc;
   const char** v8_argv;
@@ -4213,12 +4219,11 @@ void Init(int* argc,
 #endif
 
 #if defined(NODE_HAVE_I18N_SUPPORT)
-  if (icu_data_dir == nullptr) {
-    // if the parameter isn't given, use the env variable.
-    icu_data_dir = secure_getenv("NODE_ICU_DATA");
-  }
+  // If the parameter isn't given, use the env variable.
+  if (icu_data_dir.empty())
+    SafeGetenv("NODE_ICU_DATA", &icu_data_dir);
   // Initialize ICU.
-  // If icu_data_dir is nullptr here, it will load the 'minimal' data.
+  // If icu_data_dir is empty here, it will load the 'minimal' data.
   if (!i18n::InitializeICUDirectory(icu_data_dir)) {
     FatalError(nullptr, "Could not initialize ICU "
                      "(check NODE_ICU_DATA or --icu-data-dir parameters)");
@@ -4483,8 +4488,11 @@ int Start(int argc, char** argv) {
   Init(&argc, const_cast<const char**>(argv), &exec_argc, &exec_argv);
 
 #if HAVE_OPENSSL
-  if (const char* extra = secure_getenv("NODE_EXTRA_CA_CERTS"))
-    crypto::UseExtraCaCerts(extra);
+  {
+    std::string extra_ca_certs;
+    if (SafeGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs))
+      crypto::UseExtraCaCerts(extra_ca_certs);
+  }
 #ifdef NODE_FIPS_MODE
   // In the case of FIPS builds we should make sure
   // the random source is properly initialized first.
@@ -4493,7 +4501,7 @@ int Start(int argc, char** argv) {
   // V8 on Windows doesn't have a good source of entropy. Seed it from
   // OpenSSL's pool.
   V8::SetEntropySource(crypto::EntropySource);
-#endif
+#endif  // HAVE_OPENSSL
 
   v8_platform.Initialize(v8_thread_pool_size);
   V8::Initialize();

src/node_config.cc

@@ -38,8 +38,11 @@ void InitConfig(Local<Object> target,
   READONLY_BOOLEAN_PROPERTY("hasSmallICU");
 #endif  // NODE_HAVE_SMALL_ICU
 
-  if (flag_icu_data_dir)
-    READONLY_BOOLEAN_PROPERTY("usingICUDataDir");
+  target->DefineOwnProperty(env->context(),
+                            OneByteString(env->isolate(), "icuDataDir"),
+                            OneByteString(env->isolate(), icu_data_dir.data()))
+      .FromJust();
+
 #endif  // NODE_HAVE_I18N_SUPPORT
 
   if (config_preserve_symlinks)

src/node_crypto.cc

@@ -5881,14 +5881,14 @@ void InitCryptoOnce() {
   OPENSSL_no_config();
 
   // --openssl-config=...
-  if (openssl_config != nullptr) {
+  if (!openssl_config.empty()) {
     OPENSSL_load_builtin_modules();
 #ifndef OPENSSL_NO_ENGINE
     ENGINE_load_builtin_engines();
 #endif
     ERR_clear_error();
     CONF_modules_load_file(
-        openssl_config,
+        openssl_config.c_str(),
         nullptr,
         CONF_MFLAGS_DEFAULT_SECTION);
     int err = ERR_get_error();

src/node_i18n.cc

@@ -70,8 +70,6 @@ using v8::Object;
 using v8::String;
 using v8::Value;
 
-bool flag_icu_data_dir = false;
-
 namespace i18n {
 
 const size_t kStorageSize = 1024;
@@ -404,12 +402,8 @@ static void GetVersion(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
-bool InitializeICUDirectory(const char* icu_data_path) {
-  if (icu_data_path != nullptr) {
-    flag_icu_data_dir = true;
-    u_setDataDirectory(icu_data_path);
-    return true;  // no error
-  } else {
+bool InitializeICUDirectory(const std::string& path) {
+  if (path.empty()) {
     UErrorCode status = U_ZERO_ERROR;
 #ifdef NODE_HAVE_SMALL_ICU
     // install the 'small' data.
@@ -418,6 +412,9 @@ bool InitializeICUDirectory(const char* icu_data_path) {
     // no small data, so nothing to do.
 #endif  // !NODE_HAVE_SMALL_ICU
     return (status == U_ZERO_ERROR);
+  } else {
+    u_setDataDirectory(path.c_str());
+    return true;  // No error.
   }
 }
 

src/node_i18n.h

@@ -4,16 +4,17 @@
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
 #include "node.h"
+#include <string>
 
 #if defined(NODE_HAVE_I18N_SUPPORT)
 
 namespace node {
 
-extern bool flag_icu_data_dir;
+extern std::string icu_data_dir;  // NOLINT(runtime/string)
 
 namespace i18n {
 
-bool InitializeICUDirectory(const char* icu_data_path);
+bool InitializeICUDirectory(const std::string& path);
 
 int32_t ToASCII(MaybeStackBuffer<char>* buf,
                 const char* input,

src/node_internals.h

@@ -12,6 +12,8 @@
 #include <stdint.h>
 #include <stdlib.h>
 
+#include <string>
+
 struct sockaddr;
 
 // Variation on NODE_DEFINE_CONSTANT that sets a String value.
@@ -34,7 +36,7 @@ namespace node {
 
 // Set in node.cc by ParseArgs with the value of --openssl-config.
 // Used in node_crypto.cc when initializing OpenSSL.
-extern const char* openssl_config;
+extern std::string openssl_config;
 
 // Set in node.cc by ParseArgs when --preserve-symlinks is used.
 // Used in node_config.cc to set a constant on process.binding('config')
@@ -106,6 +108,8 @@ void RegisterSignalHandler(int signal,
                            bool reset_handler = false);
 #endif
 
+bool SafeGetenv(const char* key, std::string* text);
+
 template <typename T, size_t N>
 constexpr size_t arraysize(const T(&)[N]) { return N; }
 

test/parallel/test-crypto-fips.js

@@ -37,8 +37,9 @@ function testHelper(stream, args, expectedOutput, cmd, env) {
     env: env
   });
 
-  console.error('Spawned child [pid:' + child.pid + '] with cmd ' +
-      cmd + ' and args \'' + args + '\'');
+  console.error('Spawned child [pid:' + child.pid + '] with cmd \'' +
+      cmd + '\' expect %j with args \'' + args + '\'' +
+      ' OPENSSL_CONF=%j', expectedOutput, env.OPENSSL_CONF);
 
   function childOk(child) {
     console.error('Child #' + ++num_children_ok +
@@ -92,10 +93,26 @@ testHelper(
   compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED,
   'require("crypto").fips',
   process.env);
-// OPENSSL_CONF should _not_ be able to turn on FIPS mode
+
+// OPENSSL_CONF should be able to turn on FIPS mode
 testHelper(
   'stdout',
   [],
+  compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED,
+  'require("crypto").fips',
+  addToEnv('OPENSSL_CONF', CNF_FIPS_ON));
+
+// --openssl-config option should override OPENSSL_CONF
+testHelper(
+  'stdout',
+  [`--openssl-config=${CNF_FIPS_ON}`],
+  compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED,
+  'require("crypto").fips',
+  addToEnv('OPENSSL_CONF', CNF_FIPS_OFF));
+
+testHelper(
+  'stdout',
+  [`--openssl-config=${CNF_FIPS_OFF}`],
   FIPS_DISABLED,
   'require("crypto").fips',
   addToEnv('OPENSSL_CONF', CNF_FIPS_ON));
@@ -107,6 +124,7 @@ testHelper(
   compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
   'require("crypto").fips',
   process.env);
+
 // OPENSSL_CONF should _not_ make a difference to --enable-fips
 testHelper(
   compiledWithFips() ? 'stdout' : 'stderr',
@@ -122,6 +140,7 @@ testHelper(
   compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
   'require("crypto").fips',
   process.env);
+
 // Using OPENSSL_CONF should not make a difference to --force-fips
 testHelper(
   compiledWithFips() ? 'stdout' : 'stderr',

test/parallel/test-intl-no-icu-data.js

@@ -0,0 +1,9 @@
+// Flags: --icu-data-dir=test/fixtures/empty/
+'use strict';
+require('../common');
+const assert = require('assert');
+const config = process.binding('config');
+
+// No-op when ICU case mappings are unavailable.
+assert.strictEqual('ç'.toLocaleUpperCase('el'), 'ç');
+assert.strictEqual(config.icuDataDir, 'test/fixtures/empty/');