From bb173f931ae85b464e901806f15dcc219dec6b73 Mon Sep 17 00:00:00 2001
From: Rod Vagg <rod@vagg.org>
Date: Tue, 18 Oct 2016 16:35:22 +1100
Subject: [PATCH] crypto: fix openssl.cnf FIPS handling & testing

* Add documentation for `--openssl-conf=file`.
* Fix openssl.cnf loading and OpenSSL init ordering
* Fix FIPS tests so `OPENSSL_CONF` is not longer usable but
  `--openssl-conf` is

PR-URL: https://github.com/nodejs/node-private/pull/82
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
---
 doc/api/cli.md                    | 10 +++++++++
 doc/node.1                        |  6 ++++++
 src/node.h                        |  7 ++-----
 src/node_crypto.cc                |  9 ++++++--
 src/node_internals.h              |  4 ++++
 test/parallel/test-crypto-fips.js | 35 ++++++++++++++++++++++++-------
 6 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 221d0c631a26e9..13d4df49d8b62a 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -234,6 +234,16 @@ Force FIPS-compliant crypto on startup. (Cannot be disabled from script code.)
 (Same requirements as `--enable-fips`)
 
 
+### `--openssl-config=file`
+<!-- YAML
+added: v6.9.0
+-->
+
+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`.
+
+
 ### `--icu-data-dir=file`
 <!-- YAML
 added: v0.11.15
diff --git a/doc/node.1 b/doc/node.1
index 43ec7c12b63e58..ec41a44c7598c3 100644
--- a/doc/node.1
+++ b/doc/node.1
@@ -165,6 +165,12 @@ Enable FIPS-compliant crypto at startup. (Requires Node.js to be built with
 Force FIPS-compliant crypto on startup. (Cannot be disabled from script code.)
 (Same requirements as \fB\-\-enable\-fips\fR)
 
+.TP
+.BR \-\-openssl\-config =\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.
+
 .TP
 .BR \-\-icu\-data\-dir =\fIfile\fR
 Specify ICU data load path. (overrides \fBNODE_ICU_DATA\fR)
diff --git a/src/node.h b/src/node.h
index 3f6b9639358092..873551fa338a97 100644
--- a/src/node.h
+++ b/src/node.h
@@ -179,13 +179,10 @@ typedef intptr_t ssize_t;
 namespace node {
 
 NODE_EXTERN extern bool no_deprecation;
-#if HAVE_OPENSSL
-# if NODE_FIPS_MODE
+#if HAVE_OPENSSL && NODE_FIPS_MODE
 NODE_EXTERN extern bool enable_fips_crypto;
 NODE_EXTERN extern bool force_fips_crypto;
-# endif  // NODE_FIPS_MODE
-NODE_EXTERN extern const char* openssl_config;
-#endif  // HAVE_OPENSSL
+#endif
 
 NODE_EXTERN int Start(int argc, char *argv[]);
 NODE_EXTERN void Init(int* argc,
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 722e7cea39793c..25cb003d39e90a 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -5767,14 +5767,20 @@ void TimingSafeEqual(const FunctionCallbackInfo<Value>& args) {
 }
 
 void InitCryptoOnce() {
+  SSL_load_error_strings();
   OPENSSL_no_config();
 
   // --openssl-config=...
   if (openssl_config != nullptr) {
+    OPENSSL_load_builtin_modules();
+#ifndef OPENSSL_NO_ENGINE
+    ENGINE_load_builtin_engines();
+#endif
+    ERR_clear_error();
     CONF_modules_load_file(
         openssl_config,
         nullptr,
-        CONF_MFLAGS_DEFAULT_SECTION | CONF_MFLAGS_IGNORE_MISSING_FILE);
+        CONF_MFLAGS_DEFAULT_SECTION);
     int err = ERR_get_error();
     if (0 != err) {
       fprintf(stderr,
@@ -5786,7 +5792,6 @@ void InitCryptoOnce() {
 
   SSL_library_init();
   OpenSSL_add_all_algorithms();
-  SSL_load_error_strings();
 
   crypto_lock_init();
   CRYPTO_set_locking_callback(crypto_lock_cb);
diff --git a/src/node_internals.h b/src/node_internals.h
index 72888ef36d519e..99b0cec2df2178 100644
--- a/src/node_internals.h
+++ b/src/node_internals.h
@@ -32,6 +32,10 @@ struct sockaddr;
 
 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;
+
 // Set in node.cc by ParseArgs when --preserve-symlinks is used.
 // Used in node_config.cc to set a constant on process.binding('config')
 // that is used by lib/module.js
diff --git a/test/parallel/test-crypto-fips.js b/test/parallel/test-crypto-fips.js
index 0fe30214696d7c..55b542ca9ead83 100644
--- a/test/parallel/test-crypto-fips.js
+++ b/test/parallel/test-crypto-fips.js
@@ -88,12 +88,26 @@ testHelper(
 // OpenSSL config file should be able to turn on FIPS mode
 testHelper(
   'stdout',
-  [],
+  [`--openssl-config=${CNF_FIPS_ON}`],
   compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED,
   'require("crypto").fips',
+  process.env);
+// OPENSSL_CONF should _not_ be able to turn on FIPS mode
+testHelper(
+  'stdout',
+  [],
+  FIPS_DISABLED,
+  'require("crypto").fips',
   addToEnv('OPENSSL_CONF', CNF_FIPS_ON));
 
 // --enable-fips should take precedence over OpenSSL config file
+testHelper(
+  compiledWithFips() ? 'stdout' : 'stderr',
+  ['--enable-fips', `--openssl-config=${CNF_FIPS_OFF}`],
+  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',
   ['--enable-fips'],
@@ -102,6 +116,13 @@ testHelper(
   addToEnv('OPENSSL_CONF', CNF_FIPS_OFF));
 
 // --force-fips should take precedence over OpenSSL config file
+testHelper(
+  compiledWithFips() ? 'stdout' : 'stderr',
+  ['--force-fips', `--openssl-config=${CNF_FIPS_OFF}`],
+  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',
   ['--force-fips'],
@@ -116,7 +137,7 @@ testHelper(
   compiledWithFips() ? FIPS_ENABLED : FIPS_ERROR_STRING,
   '(require("crypto").fips = true,' +
   'require("crypto").fips)',
-  addToEnv('OPENSSL_CONF', ''));
+  process.env);
 
 // setFipsCrypto should be able to turn FIPS mode on and off
 testHelper(
@@ -126,25 +147,25 @@ testHelper(
   '(require("crypto").fips = true,' +
   'require("crypto").fips = false,' +
   'require("crypto").fips)',
-  addToEnv('OPENSSL_CONF', ''));
+  process.env);
 
 // setFipsCrypto takes precedence over OpenSSL config file, FIPS on
 testHelper(
   compiledWithFips() ? 'stdout' : 'stderr',
-  [],
+  [`--openssl-config=${CNF_FIPS_OFF}`],
   compiledWithFips() ? FIPS_ENABLED : FIPS_ERROR_STRING,
   '(require("crypto").fips = true,' +
   'require("crypto").fips)',
-  addToEnv('OPENSSL_CONF', CNF_FIPS_OFF));
+  process.env);
 
 // setFipsCrypto takes precedence over OpenSSL config file, FIPS off
 testHelper(
   compiledWithFips() ? 'stdout' : 'stderr',
-  [],
+  [`--openssl-config=${CNF_FIPS_ON}`],
   compiledWithFips() ? FIPS_DISABLED : FIPS_ERROR_STRING,
   '(require("crypto").fips = false,' +
   'require("crypto").fips)',
-  addToEnv('OPENSSL_CONF', CNF_FIPS_ON));
+  process.env);
 
 // --enable-fips does not prevent use of setFipsCrypto API
 testHelper(