crypto: migrate setFipsCrypto to internal/errors

With the exception of ThrowCryptoError, use internal/errors to report fips unavailable or forced PR-URL: #16428 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
nodejs · Oct 27, 2017 · ee76f31 · ee76f31
1 parent a0f7ae6
commit ee76f31
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 19 deletions.
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -643,6 +643,17 @@ Used when an invalid value for the `format` argument has been passed to the
 Used when an invalid crypto engine identifier is passed to
 [`require('crypto').setEngine()`][].
 
+<a id="ERR_CRYPTO_FIPS_FORCED"></a>
+### ERR_CRYPTO_FIPS_FORCED
+
+Used when trying to enable or disable FIPS mode in the crypto module and
+the [`--force-fips`][] command-line argument is used.
+
+<a id="ERR_CRYPTO_FIPS_UNAVAILABLE"></a>
+### ERR_CRYPTO_FIPS_UNAVAILABLE
+
+Used when trying to enable or disable FIPS mode when FIPS is not available.
+
 <a id="ERR_CRYPTO_HASH_DIGEST_NO_UTF16"></a>
 ### ERR_CRYPTO_HASH_DIGEST_NO_UTF16
 
@@ -1440,6 +1451,7 @@ Used when a given value is out of the accepted range.
 Used when an attempt is made to use a `zlib` object after it has already been
 closed.
 
+[`--force-fips`]: cli.html#cli_force_fips
 [`crypto.timingSafeEqual()`]: crypto.html#crypto_crypto_timingsafeequal_a_b
 [`dgram.createSocket()`]: dgram.html#dgram_dgram_createsocket_options_callback
 [`ERR_INVALID_ARG_TYPE`]: #ERR_INVALID_ARG_TYPE

diff --git a/lib/crypto.js b/lib/crypto.js
@@ -30,7 +30,12 @@ const {
 } = require('internal/util');
 assertCrypto();
 
+const errors = require('internal/errors');
 const constants = process.binding('constants').crypto;
+const {
+  fipsMode,
+  fipsForced
+} = process.binding('config');
 const {
   getFipsCrypto,
   setFipsCrypto,
@@ -173,10 +178,29 @@ module.exports = exports = {
   Verify
 };
 
+function setFipsDisabled() {
+  throw new errors.Error('ERR_CRYPTO_FIPS_UNAVAILABLE');
+}
+
+function setFipsForced(val) {
+  if (val) return;
+  throw new errors.Error('ERR_CRYPTO_FIPS_FORCED');
+}
+
+function getFipsDisabled() {
+  return 0;
+}
+
+function getFipsForced() {
+  return 1;
+}
+
 Object.defineProperties(exports, {
   fips: {
-    get: getFipsCrypto,
-    set: setFipsCrypto
+    get: !fipsMode ? getFipsDisabled :
+      fipsForced ? getFipsForced : getFipsCrypto,
+    set: !fipsMode ? setFipsDisabled :
+      fipsForced ? setFipsForced : setFipsCrypto
   },
   DEFAULT_ENCODING: {
     enumerable: true,

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -156,6 +156,9 @@ E('ERR_CONSOLE_WRITABLE_STREAM',
 E('ERR_CPU_USAGE', 'Unable to obtain cpu usage %s');
 E('ERR_CRYPTO_ECDH_INVALID_FORMAT', 'Invalid ECDH format: %s');
 E('ERR_CRYPTO_ENGINE_UNKNOWN', 'Engine "%s" was not found');
+E('ERR_CRYPTO_FIPS_FORCED',
+  'Cannot set FIPS mode, it was forced with --force-fips at startup.');
+E('ERR_CRYPTO_FIPS_UNAVAILABLE', 'Cannot set FIPS mode in a non-FIPS build.');
 E('ERR_CRYPTO_HASH_DIGEST_NO_UTF16', 'hash.digest() does not support UTF-16');
 E('ERR_CRYPTO_HASH_FINALIZED', 'Digest already called');
 E('ERR_CRYPTO_HASH_UPDATE_FAILED', 'Hash update failed');

diff --git a/src/node_config.cc b/src/node_config.cc
@@ -44,6 +44,12 @@ static void InitConfig(Local<Object> target,
   Environment* env = Environment::GetCurrent(context);
   Isolate* isolate = env->isolate();
 
+#ifdef NODE_FIPS_MODE
+  READONLY_BOOLEAN_PROPERTY("fipsMode");
+  if (force_fips_crypto)
+    READONLY_BOOLEAN_PROPERTY("fipsForced");
+#endif
+
 #ifdef NODE_HAVE_I18N_SUPPORT
 
   READONLY_BOOLEAN_PROPERTY("hasIntl");

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -5946,32 +5946,24 @@ void SetEngine(const FunctionCallbackInfo<Value>& args) {
 }
 #endif  // !OPENSSL_NO_ENGINE
 
+#ifdef NODE_FIPS_MODE
 void GetFipsCrypto(const FunctionCallbackInfo<Value>& args) {
-  if (FIPS_mode()) {
-    args.GetReturnValue().Set(1);
-  } else {
-    args.GetReturnValue().Set(0);
-  }
+  args.GetReturnValue().Set(FIPS_mode() ? 1 : 0);
 }
 
 void SetFipsCrypto(const FunctionCallbackInfo<Value>& args) {
+  CHECK(!force_fips_crypto);
   Environment* env = Environment::GetCurrent(args);
-#ifdef NODE_FIPS_MODE
   const bool enabled = FIPS_mode();
   const bool enable = args[0]->BooleanValue();
   if (enable == enabled)
     return;  // No action needed.
-  if (force_fips_crypto) {
-    return env->ThrowError(
-        "Cannot set FIPS mode, it was forced with --force-fips at startup.");
-  } else if (!FIPS_mode_set(enable)) {
+  if (!FIPS_mode_set(enable)) {
     unsigned long err = ERR_get_error();  // NOLINT(runtime/int)
     return ThrowCryptoError(env, err);
   }
-#else
-  return env->ThrowError("Cannot set FIPS mode in a non-FIPS build.");
-#endif /* NODE_FIPS_MODE */
 }
+#endif /* NODE_FIPS_MODE */
 
 void InitCrypto(Local<Object> target,
                 Local<Value> unused,
@@ -5997,8 +5989,12 @@ void InitCrypto(Local<Object> target,
 #ifndef OPENSSL_NO_ENGINE
   env->SetMethod(target, "setEngine", SetEngine);
 #endif  // !OPENSSL_NO_ENGINE
+
+#ifdef NODE_FIPS_MODE
   env->SetMethod(target, "getFipsCrypto", GetFipsCrypto);
   env->SetMethod(target, "setFipsCrypto", SetFipsCrypto);
+#endif
+
   env->SetMethod(target, "PBKDF2", PBKDF2);
   env->SetMethod(target, "randomBytes", RandomBytes);
   env->SetMethod(target, "randomFill", RandomBytesBuffer);

diff --git a/test/parallel/test-crypto-fips.js b/test/parallel/test-crypto-fips.js
@@ -10,7 +10,12 @@ const fixtures = require('../common/fixtures');
 
 const FIPS_ENABLED = 1;
 const FIPS_DISABLED = 0;
-const FIPS_ERROR_STRING = 'Error: Cannot set FIPS mode';
+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 CNF_FIPS_ON = fixtures.path('openssl_fips_enabled.cnf');
@@ -208,7 +213,7 @@ testHelper(
 testHelper(
   'stderr',
   ['--force-fips'],
-  compiledWithFips() ? FIPS_ERROR_STRING : OPTION_ERROR_STRING,
+  compiledWithFips() ? FIPS_ERROR_STRING2 : OPTION_ERROR_STRING,
   'require("crypto").fips = false',
   process.env);
 
@@ -225,14 +230,14 @@ testHelper(
 testHelper(
   'stderr',
   ['--force-fips', '--enable-fips'],
-  compiledWithFips() ? FIPS_ERROR_STRING : OPTION_ERROR_STRING,
+  compiledWithFips() ? FIPS_ERROR_STRING2 : OPTION_ERROR_STRING,
   'require("crypto").fips = false',
   process.env);
 
 //--enable-fips and --force-fips order does not matter
 testHelper(
   'stderr',
   ['--enable-fips', '--force-fips'],
-  compiledWithFips() ? FIPS_ERROR_STRING : OPTION_ERROR_STRING,
+  compiledWithFips() ? FIPS_ERROR_STRING2 : OPTION_ERROR_STRING,
   'require("crypto").fips = false',
   process.env);