From ee76f3153b51c60c74e7e4b0882a99f3a3745294 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 23 Oct 2017 22:44:52 -0700 Subject: [PATCH] crypto: migrate setFipsCrypto to internal/errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the exception of ThrowCryptoError, use internal/errors to report fips unavailable or forced PR-URL: https://github.com/nodejs/node/pull/16428 Reviewed-By: Luigi Pinca Reviewed-By: Michaël Zasso Reviewed-By: Joyee Cheung Reviewed-By: Matteo Collina --- doc/api/errors.md | 12 ++++++++++++ lib/crypto.js | 28 ++++++++++++++++++++++++++-- lib/internal/errors.js | 3 +++ src/node_config.cc | 6 ++++++ src/node_crypto.cc | 22 +++++++++------------- test/parallel/test-crypto-fips.js | 13 +++++++++---- 6 files changed, 65 insertions(+), 19 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 753f84f7c56f4b..fcc39ab2ac81ee 100644 --- 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()`][]. + +### 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. + + +### ERR_CRYPTO_FIPS_UNAVAILABLE + +Used when trying to enable or disable FIPS mode when FIPS is not available. + ### 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 index 0082172c5c4a09..d7c59f553edd2a 100644 --- 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 index 5ac0fb9a5457f8..9bf0951eece120 100644 --- 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 index 38ce2a47bb0f3d..8af3472b0f42d0 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -44,6 +44,12 @@ static void InitConfig(Local 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 index 3d114d73d2c309..200d301dccdac6 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -5946,32 +5946,24 @@ void SetEngine(const FunctionCallbackInfo& args) { } #endif // !OPENSSL_NO_ENGINE +#ifdef NODE_FIPS_MODE void GetFipsCrypto(const FunctionCallbackInfo& args) { - if (FIPS_mode()) { - args.GetReturnValue().Set(1); - } else { - args.GetReturnValue().Set(0); - } + args.GetReturnValue().Set(FIPS_mode() ? 1 : 0); } void SetFipsCrypto(const FunctionCallbackInfo& 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 target, Local unused, @@ -5997,8 +5989,12 @@ void InitCrypto(Local 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 index 755c6e20c26b2d..faf5ab9588d21f 100644 --- 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,7 +230,7 @@ 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); @@ -233,6 +238,6 @@ testHelper( 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);