crypto: support OPENSSL_CONF again

A side-effect of https://github.com/nodejs/node-private/pull/82
was to remove support for OPENSSL_CONF, as well as removing the default
read of a configuration file on startup.

Partly revert this, allowing OPENSSL_CONF to be used to specify a
configuration file to read on startup, but do not read a file by
default.

If the --openssl-config command line option is provided, its value is
used, not the OPENSSL_CONF environment variable.

Fix: #10938
PR-URL: #11006
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

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

src/node.cc

@@ -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
@@ -3517,8 +3517,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"
@@ -3551,6 +3552,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");
 }
 
@@ -3688,7 +3691,7 @@ 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) {
@@ -4185,6 +4188,9 @@ void Init(int* argc,
         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;

src/node_crypto.cc

@@ -5880,14 +5880,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_internals.h

@@ -36,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')

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',