Adjust tests for always-available FIPS options

- The fipsMode constant (defined at compile time) was replaced by the new `TestFipsCrypto()`/`testFipsCrypto()` functions, which rely on the OpenSSL function `FIPS_selftest()`. This results in the FIPS mode being always checked on runtime and being informed purely by the OpenSSL implementation in use.
nodejs · Dec 15, 2020 · 6725b14 · 6725b14
1 parent cc5dca5
commit 6725b14
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 70 deletions.
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -186,8 +186,8 @@ code from strings throw an exception instead. This does not affect the Node.js
 added: v6.0.0
 -->
 
-Enable FIPS-compliant crypto at startup. (Requires Node.js to be built with
-`./configure --openssl-fips`.)
+Enable FIPS-compliant crypto at startup. (Requires Node.js to be built
+against FIPS-compatible OpenSSL.)
 
 ### `--enable-source-maps`
 <!-- YAML
@@ -606,8 +606,8 @@ 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`.
+used to enable FIPS-compliant crypto if Node.js is built
+with against FIPS-enabled OpenSSL.
 
 ### `--pending-deprecation`
 <!-- YAML

diff --git a/lib/crypto.js b/lib/crypto.js
@@ -37,12 +37,10 @@ assertCrypto();
 
 const {
   ERR_CRYPTO_FIPS_FORCED,
-  ERR_CRYPTO_FIPS_UNAVAILABLE
 } = require('internal/errors').codes;
 const constants = internalBinding('constants').crypto;
 const { getOptionValue } = require('internal/options');
 const pendingDeprecation = getOptionValue('--pending-deprecation');
-const { fipsMode } = internalBinding('config');
 const fipsForced = getOptionValue('--force-fips');
 const {
   getFipsCrypto,
@@ -204,10 +202,8 @@ module.exports = {
   sign: signOneShot,
   setEngine,
   timingSafeEqual,
-  getFips: !fipsMode ? getFipsDisabled :
-    fipsForced ? getFipsForced : getFipsCrypto,
-  setFips: !fipsMode ? setFipsDisabled :
-    fipsForced ? setFipsForced : setFipsCrypto,
+  getFips: fipsForced ? getFipsForced : getFipsCrypto,
+  setFips: fipsForced ? setFipsForced : setFipsCrypto,
   verify: verifyOneShot,
 
   // Classes
@@ -226,19 +222,11 @@ module.exports = {
   Verify
 };
 
-function setFipsDisabled() {
-  throw new ERR_CRYPTO_FIPS_UNAVAILABLE();
-}
-
 function setFipsForced(val) {
   if (val) return;
   throw new ERR_CRYPTO_FIPS_FORCED();
 }
 
-function getFipsDisabled() {
-  return 0;
-}
-
 function getFipsForced() {
   return 1;
 }
@@ -260,10 +248,8 @@ ObjectDefineProperties(module.exports, {
   },
   // crypto.fips is deprecated. DEP0093. Use crypto.getFips()/crypto.setFips()
   fips: {
-    get: !fipsMode ? getFipsDisabled :
-      fipsForced ? getFipsForced : getFipsCrypto,
-    set: !fipsMode ? setFipsDisabled :
-      fipsForced ? setFipsForced : setFipsCrypto
+    get: fipsForced ? getFipsForced : getFipsCrypto,
+    set: fipsForced ? setFipsForced : setFipsCrypto
   },
   DEFAULT_ENCODING: {
     enumerable: false,

diff --git a/test/parallel/test-crypto-fips.js b/test/parallel/test-crypto-fips.js
@@ -9,27 +9,20 @@ const spawnSync = require('child_process').spawnSync;
 const path = require('path');
 const fixtures = require('../common/fixtures');
 const { internalBinding } = require('internal/test/binding');
-const { fipsMode } = internalBinding('config');
+const { testFipsCrypto } = internalBinding('crypto');
 
 const FIPS_ENABLED = 1;
 const FIPS_DISABLED = 0;
-const FIPS_ERROR_STRING =
-  'Error [ERR_CRYPTO_FIPS_UNAVAILABLE]: Cannot set FIPS mode in a ' +
-  'non-FIPS build.';
 const FIPS_ERROR_STRING2 =
   'Error [ERR_CRYPTO_FIPS_FORCED]: Cannot set FIPS mode, it was forced with ' +
   '--force-fips at startup.';
-const OPTION_ERROR_STRING = 'bad option';
+const FIPS_UNSUPPORTED_ERROR_STRING = 'fips mode not supported';
 
 const CNF_FIPS_ON = fixtures.path('openssl_fips_enabled.cnf');
 const CNF_FIPS_OFF = fixtures.path('openssl_fips_disabled.cnf');
 
 let num_children_ok = 0;
 
-function compiledWithFips() {
-  return fipsMode ? true : false;
-}
-
 function sharedOpenSSL() {
   return process.config.variables.node_shared_openssl;
 }
@@ -75,17 +68,17 @@ testHelper(
 
 // --enable-fips should turn FIPS mode on
 testHelper(
-  compiledWithFips() ? 'stdout' : 'stderr',
+  testFipsCrypto() ? 'stdout' : 'stderr',
   ['--enable-fips'],
-  compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
+  testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
   'require("crypto").getFips()',
   process.env);
 
 // --force-fips should turn FIPS mode on
 testHelper(
-  compiledWithFips() ? 'stdout' : 'stderr',
+  testFipsCrypto() ? 'stdout' : 'stderr',
   ['--force-fips'],
-  compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
+  testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
   'require("crypto").getFips()',
   process.env);
 
@@ -106,23 +99,23 @@ if (!sharedOpenSSL()) {
   testHelper(
     'stdout',
     [`--openssl-config=${CNF_FIPS_ON}`],
-    compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED,
+    testFipsCrypto() ? FIPS_ENABLED : FIPS_DISABLED,
     'require("crypto").getFips()',
     process.env);
 
   // OPENSSL_CONF should be able to turn on FIPS mode
   testHelper(
     'stdout',
     [],
-    compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED,
+    testFipsCrypto() ? FIPS_ENABLED : FIPS_DISABLED,
     'require("crypto").getFips()',
     Object.assign({}, process.env, { 'OPENSSL_CONF': CNF_FIPS_ON }));
 
   // --openssl-config option should override OPENSSL_CONF
   testHelper(
     'stdout',
     [`--openssl-config=${CNF_FIPS_ON}`],
-    compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED,
+    testFipsCrypto() ? FIPS_ENABLED : FIPS_DISABLED,
     'require("crypto").getFips()',
     Object.assign({}, process.env, { 'OPENSSL_CONF': CNF_FIPS_OFF }));
 }
@@ -136,78 +129,78 @@ testHelper(
 
 // --enable-fips should take precedence over OpenSSL config file
 testHelper(
-  compiledWithFips() ? 'stdout' : 'stderr',
+  testFipsCrypto() ? 'stdout' : 'stderr',
   ['--enable-fips', `--openssl-config=${CNF_FIPS_OFF}`],
-  compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
+  testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
   'require("crypto").getFips()',
   process.env);
 
 // OPENSSL_CONF should _not_ make a difference to --enable-fips
 testHelper(
-  compiledWithFips() ? 'stdout' : 'stderr',
+  testFipsCrypto() ? 'stdout' : 'stderr',
   ['--enable-fips'],
-  compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
+  testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
   'require("crypto").getFips()',
   Object.assign({}, process.env, { 'OPENSSL_CONF': CNF_FIPS_OFF }));
 
 // --force-fips should take precedence over OpenSSL config file
 testHelper(
-  compiledWithFips() ? 'stdout' : 'stderr',
+  testFipsCrypto() ? 'stdout' : 'stderr',
   ['--force-fips', `--openssl-config=${CNF_FIPS_OFF}`],
-  compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
+  testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
   'require("crypto").getFips()',
   process.env);
 
 // Using OPENSSL_CONF should not make a difference to --force-fips
 testHelper(
-  compiledWithFips() ? 'stdout' : 'stderr',
+  testFipsCrypto() ? 'stdout' : 'stderr',
   ['--force-fips'],
-  compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
+  testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
   'require("crypto").getFips()',
   Object.assign({}, process.env, { 'OPENSSL_CONF': CNF_FIPS_OFF }));
 
 // setFipsCrypto should be able to turn FIPS mode on
 testHelper(
-  compiledWithFips() ? 'stdout' : 'stderr',
+  testFipsCrypto() ? 'stdout' : 'stderr',
   [],
-  compiledWithFips() ? FIPS_ENABLED : FIPS_ERROR_STRING,
+  testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
   '(require("crypto").setFips(true),' +
   'require("crypto").getFips())',
   process.env);
 
 // setFipsCrypto should be able to turn FIPS mode on and off
 testHelper(
-  compiledWithFips() ? 'stdout' : 'stderr',
+  testFipsCrypto() ? 'stdout' : 'stderr',
   [],
-  compiledWithFips() ? FIPS_DISABLED : FIPS_ERROR_STRING,
+  testFipsCrypto() ? FIPS_DISABLED : FIPS_UNSUPPORTED_ERROR_STRING,
   '(require("crypto").setFips(true),' +
   'require("crypto").setFips(false),' +
   'require("crypto").getFips())',
   process.env);
 
 // setFipsCrypto takes precedence over OpenSSL config file, FIPS on
 testHelper(
-  compiledWithFips() ? 'stdout' : 'stderr',
+  testFipsCrypto() ? 'stdout' : 'stderr',
   [`--openssl-config=${CNF_FIPS_OFF}`],
-  compiledWithFips() ? FIPS_ENABLED : FIPS_ERROR_STRING,
+  testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
   '(require("crypto").setFips(true),' +
   'require("crypto").getFips())',
   process.env);
 
 // setFipsCrypto takes precedence over OpenSSL config file, FIPS off
 testHelper(
-  compiledWithFips() ? 'stdout' : 'stderr',
+  'stdout',
   [`--openssl-config=${CNF_FIPS_ON}`],
-  compiledWithFips() ? FIPS_DISABLED : FIPS_ERROR_STRING,
+  FIPS_DISABLED,
   '(require("crypto").setFips(false),' +
   'require("crypto").getFips())',
   process.env);
 
 // --enable-fips does not prevent use of setFipsCrypto API
 testHelper(
-  compiledWithFips() ? 'stdout' : 'stderr',
+  testFipsCrypto() ? 'stdout' : 'stderr',
   ['--enable-fips'],
-  compiledWithFips() ? FIPS_DISABLED : OPTION_ERROR_STRING,
+  testFipsCrypto() ? FIPS_DISABLED : FIPS_UNSUPPORTED_ERROR_STRING,
   '(require("crypto").setFips(false),' +
   'require("crypto").getFips())',
   process.env);
@@ -216,15 +209,15 @@ testHelper(
 testHelper(
   'stderr',
   ['--force-fips'],
-  compiledWithFips() ? FIPS_ERROR_STRING2 : OPTION_ERROR_STRING,
+  testFipsCrypto() ? FIPS_ERROR_STRING2 : FIPS_UNSUPPORTED_ERROR_STRING,
   'require("crypto").setFips(false)',
   process.env);
 
 // --force-fips makes setFipsCrypto enable a no-op (FIPS stays on)
 testHelper(
-  compiledWithFips() ? 'stdout' : 'stderr',
+  testFipsCrypto() ? 'stdout' : 'stderr',
   ['--force-fips'],
-  compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
+  testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
   '(require("crypto").setFips(true),' +
   'require("crypto").getFips())',
   process.env);
@@ -233,14 +226,14 @@ testHelper(
 testHelper(
   'stderr',
   ['--force-fips', '--enable-fips'],
-  compiledWithFips() ? FIPS_ERROR_STRING2 : OPTION_ERROR_STRING,
+  testFipsCrypto() ? FIPS_ERROR_STRING2 : FIPS_UNSUPPORTED_ERROR_STRING,
   'require("crypto").setFips(false)',
   process.env);
 
 // --enable-fips and --force-fips order does not matter
 testHelper(
   'stderr',
   ['--enable-fips', '--force-fips'],
-  compiledWithFips() ? FIPS_ERROR_STRING2 : OPTION_ERROR_STRING,
+  testFipsCrypto() ? FIPS_ERROR_STRING2 : FIPS_UNSUPPORTED_ERROR_STRING,
   'require("crypto").setFips(false)',
   process.env);
diff --git a/test/parallel/test-process-env-allowed-flags-are-documented.js b/test/parallel/test-process-env-allowed-flags-are-documented.js
@@ -46,15 +46,6 @@ const conditionalOpts = [
       return ['--openssl-config', '--tls-cipher-list', '--use-bundled-ca',
               '--use-openssl-ca' ].includes(opt);
     } },
-  {
-    // We are using openssl_is_fips from the configuration because it could be
-    // the case that OpenSSL is FIPS compatible but fips has not been enabled
-    // (starting node with --enable-fips). If we use common.hasFipsCrypto
-    // that would only tells us if fips has been enabled, but in this case we
-    // want to check options which will be available regardless of whether fips
-    // is enabled at runtime or not.
-    include: process.config.variables.openssl_is_fips,
-    filter: (opt) => opt.includes('-fips') },
   { include: common.hasIntl,
     filter: (opt) => opt === '--icu-data-dir' },
   { include: process.features.inspector,