From 7c48cb560108bc2f4365bb0c290a9cd68dcc56e4 Mon Sep 17 00:00:00 2001 From: Stefan Budeanu Date: Fri, 22 Jan 2016 18:10:09 -0500 Subject: [PATCH] crypto: Improve control of FIPS mode Default to FIPS off even in FIPS builds. Add JS API to check and control FIPS mode. Add command line arguments to force FIPS on/off. Respect OPENSSL_CONF variable and read the config. Add testing for new features. Fixes: https://github.com/nodejs/node/issues/3819 PR-URL: https://github.com/nodejs/node/pull/5181 Reviewed-By: Fedor Indutny Reviewed-by: Michael Dawson --- doc/api/crypto.markdown | 5 + lib/crypto.js | 6 + src/node.cc | 25 +++- src/node.h | 4 + src/node_crypto.cc | 44 +++++- test/common.js | 2 +- test/fixtures/openssl_fips_disabled.cnf | 12 ++ test/fixtures/openssl_fips_enabled.cnf | 12 ++ test/parallel/test-crypto-fips.js | 180 ++++++++++++++++++++++++ test/pummel/test-crypto-dh.js | 4 +- 10 files changed, 283 insertions(+), 11 deletions(-) create mode 100644 test/fixtures/openssl_fips_disabled.cnf create mode 100644 test/fixtures/openssl_fips_enabled.cnf create mode 100644 test/parallel/test-crypto-fips.js diff --git a/doc/api/crypto.markdown b/doc/api/crypto.markdown index b14c179e017da5..0a1ea76f20e97b 100644 --- a/doc/api/crypto.markdown +++ b/doc/api/crypto.markdown @@ -815,6 +815,11 @@ with legacy programs that expect `'binary'` to be the default encoding. New applications should expect the default to be `'buffer'`. This property may become deprecated in a future Node.js release. +### crypto.fips + +Property for checking and controlling whether a FIPS compliant crypto provider is +currently in use. Setting to true requires a FIPS build of Node.js. + ### crypto.createCipher(algorithm, password) Creates and returns a `Cipher` object that uses the given `algorithm` and diff --git a/lib/crypto.js b/lib/crypto.js index 4b0539406e5ac6..cf92eff1eec0bc 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -11,6 +11,8 @@ try { var getCiphers = binding.getCiphers; var getHashes = binding.getHashes; var getCurves = binding.getCurves; + var getFipsCrypto = binding.getFipsCrypto; + var setFipsCrypto = binding.setFipsCrypto; } catch (e) { throw new Error('Node.js is not compiled with openssl crypto support'); } @@ -645,6 +647,10 @@ exports.getCurves = function() { return filterDuplicates(getCurves()); }; +Object.defineProperty(exports, 'fips', { + get: getFipsCrypto, + set: setFipsCrypto +}); function filterDuplicates(names) { // Drop all-caps names in favor of their lowercase aliases, diff --git a/src/node.cc b/src/node.cc index e11562af1acb96..c0e8dbcd44db21 100644 --- a/src/node.cc +++ b/src/node.cc @@ -161,6 +161,12 @@ static const char* icu_data_dir = nullptr; // used by C++ modules as well bool no_deprecation = false; +#if HAVE_OPENSSL && NODE_FIPS_MODE +// used by crypto module +bool enable_fips_crypto = false; +bool force_fips_crypto = false; +#endif + // process-relative uptime base, initialized at start-up static double prog_start_time; static bool debugger_running; @@ -3283,7 +3289,11 @@ static void PrintHelp() { " --v8-options print v8 command line options\n" #if HAVE_OPENSSL " --tls-cipher-list=val use an alternative default TLS cipher list\n" -#endif +#if NODE_FIPS_MODE + " --enable-fips enable FIPS crypto at startup\n" + " --force-fips force FIPS crypto (cannot be disabled)\n" +#endif /* NODE_FIPS_MODE */ +#endif /* HAVE_OPENSSL */ #if defined(NODE_HAVE_I18N_SUPPORT) " --icu-data-dir=dir set ICU data load path to dir\n" " (overrides NODE_ICU_DATA)\n" @@ -3425,7 +3435,13 @@ static void ParseArgs(int* argc, #if HAVE_OPENSSL } else if (strncmp(arg, "--tls-cipher-list=", 18) == 0) { default_cipher_list = arg + 18; -#endif +#if NODE_FIPS_MODE + } else if (strcmp(arg, "--enable-fips") == 0) { + enable_fips_crypto = true; + } else if (strcmp(arg, "--force-fips") == 0) { + force_fips_crypto = true; +#endif /* NODE_FIPS_MODE */ +#endif /* HAVE_OPENSSL */ #if defined(NODE_HAVE_I18N_SUPPORT) } else if (strncmp(arg, "--icu-data-dir=", 15) == 0) { icu_data_dir = arg + 15; @@ -4224,6 +4240,11 @@ int Start(int argc, char** argv) { Init(&argc, const_cast(argv), &exec_argc, &exec_argv); #if HAVE_OPENSSL +#ifdef NODE_FIPS_MODE + // In the case of FIPS builds we should make sure + // the random source is properly initialized first. + OPENSSL_init(); +#endif // NODE_FIPS_MODE // V8 on Windows doesn't have a good source of entropy. Seed it from // OpenSSL's pool. V8::SetEntropySource(crypto::EntropySource); diff --git a/src/node.h b/src/node.h index ef1f629d20aa0e..fcbb45085c5439 100644 --- a/src/node.h +++ b/src/node.h @@ -179,6 +179,10 @@ typedef intptr_t ssize_t; namespace node { NODE_EXTERN extern bool no_deprecation; +#if HAVE_OPENSSL && NODE_FIPS_MODE +NODE_EXTERN extern bool enable_fips_crypto; +NODE_EXTERN extern bool force_fips_crypto; +#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 5fe3632b109fa7..85fefe0bdf2ba8 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3126,8 +3126,10 @@ void CipherBase::Init(const char* cipher_type, HandleScope scope(env()->isolate()); #ifdef NODE_FIPS_MODE - return env()->ThrowError( - "crypto.createCipher() is not supported in FIPS mode."); + if (FIPS_mode()) { + return env()->ThrowError( + "crypto.createCipher() is not supported in FIPS mode."); + } #endif // NODE_FIPS_MODE CHECK_EQ(cipher_, nullptr); @@ -3858,7 +3860,7 @@ SignBase::Error Sign::SignFinal(const char* key_pem, #ifdef NODE_FIPS_MODE /* Validate DSA2 parameters from FIPS 186-4 */ - if (EVP_PKEY_DSA == pkey->type) { + if (FIPS_mode() && EVP_PKEY_DSA == pkey->type) { size_t L = BN_num_bits(pkey->pkey.dsa->p); size_t N = BN_num_bits(pkey->pkey.dsa->q); bool result = false; @@ -5665,14 +5667,21 @@ void InitCryptoOnce() { SSL_library_init(); OpenSSL_add_all_algorithms(); SSL_load_error_strings(); + OPENSSL_config(NULL); crypto_lock_init(); CRYPTO_set_locking_callback(crypto_lock_cb); CRYPTO_THREADID_set_callback(crypto_threadid_cb); #ifdef NODE_FIPS_MODE - if (!FIPS_mode_set(1)) { - int err = ERR_get_error(); + /* Override FIPS settings in cnf file, if needed. */ + unsigned long err = 0; + if (enable_fips_crypto || force_fips_crypto) { + if (0 == FIPS_mode() && !FIPS_mode_set(1)) { + err = ERR_get_error(); + } + } + if (0 != err) { fprintf(stderr, "openssl fips failed: %s\n", ERR_error_string(err, NULL)); UNREACHABLE(); } @@ -5739,6 +5748,29 @@ void SetEngine(const FunctionCallbackInfo& args) { } #endif // !OPENSSL_NO_ENGINE +void GetFipsCrypto(const FunctionCallbackInfo& args) { + if (FIPS_mode()) { + args.GetReturnValue().Set(1); + } else { + args.GetReturnValue().Set(0); + } +} + +void SetFipsCrypto(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); +#ifdef NODE_FIPS_MODE + bool mode = args[0]->BooleanValue(); + if (force_fips_crypto) { + return env->ThrowError( + "Cannot set FIPS mode, it was forced with --force-fips at startup."); + } else if (!FIPS_mode_set(mode)) { + unsigned long err = ERR_get_error(); + return ThrowCryptoError(env, err); + } +#else + return env->ThrowError("Cannot set FIPS mode in a non-FIPS build."); +#endif /* NODE_FIPS_MODE */ +} // FIXME(bnoordhuis) Handle global init correctly. void InitCrypto(Local target, @@ -5763,6 +5795,8 @@ void InitCrypto(Local target, #ifndef OPENSSL_NO_ENGINE env->SetMethod(target, "setEngine", SetEngine); #endif // !OPENSSL_NO_ENGINE + env->SetMethod(target, "getFipsCrypto", GetFipsCrypto); + env->SetMethod(target, "setFipsCrypto", SetFipsCrypto); env->SetMethod(target, "PBKDF2", PBKDF2); env->SetMethod(target, "randomBytes", RandomBytes); env->SetMethod(target, "getSSLCiphers", GetSSLCiphers); diff --git a/test/common.js b/test/common.js index b7c64d2852af13..cc54c65c7b2892 100644 --- a/test/common.js +++ b/test/common.js @@ -161,7 +161,7 @@ Object.defineProperty(exports, 'hasCrypto', { Object.defineProperty(exports, 'hasFipsCrypto', { get: function() { - return process.config.variables.openssl_fips ? true : false; + return exports.hasCrypto && require('crypto').fips; } }); diff --git a/test/fixtures/openssl_fips_disabled.cnf b/test/fixtures/openssl_fips_disabled.cnf new file mode 100644 index 00000000000000..8668370fac52f7 --- /dev/null +++ b/test/fixtures/openssl_fips_disabled.cnf @@ -0,0 +1,12 @@ +# Skeleton openssl.cnf for testing with FIPS + +openssl_conf = openssl_conf_section +authorityKeyIdentifier=keyid:always,issuer:always + +[openssl_conf_section] + # Configuration module list +alg_section = evp_sect + +[ evp_sect ] +# Set to "yes" to enter FIPS mode if supported +fips_mode = no diff --git a/test/fixtures/openssl_fips_enabled.cnf b/test/fixtures/openssl_fips_enabled.cnf new file mode 100644 index 00000000000000..9c1a90f5087727 --- /dev/null +++ b/test/fixtures/openssl_fips_enabled.cnf @@ -0,0 +1,12 @@ +# Skeleton openssl.cnf for testing with FIPS + +openssl_conf = openssl_conf_section +authorityKeyIdentifier=keyid:always,issuer:always + +[openssl_conf_section] + # Configuration module list +alg_section = evp_sect + +[ evp_sect ] +# Set to "yes" to enter FIPS mode if supported +fips_mode = yes diff --git a/test/parallel/test-crypto-fips.js b/test/parallel/test-crypto-fips.js new file mode 100644 index 00000000000000..abccb450ad8681 --- /dev/null +++ b/test/parallel/test-crypto-fips.js @@ -0,0 +1,180 @@ +'use strict'; +var common = require('../common'); +var assert = require('assert'); +var spawnSync = require('child_process').spawnSync; +var path = require('path'); + +if (!common.hasCrypto) { + console.log('1..0 # Skipped: missing crypto'); + return; +} + +const FIPS_ENABLED = 1; +const FIPS_DISABLED = 0; +const FIPS_ERROR_STRING = 'Error: Cannot set FIPS mode'; +const OPTION_ERROR_STRING = 'bad option'; +const CNF_FIPS_ON = path.join(common.fixturesDir, 'openssl_fips_enabled.cnf'); +const CNF_FIPS_OFF = path.join(common.fixturesDir, 'openssl_fips_disabled.cnf'); +var num_children_ok = 0; + +function compiledWithFips() { + return process.config.variables.openssl_fips ? true : false; +} + +function addToEnv(newVar, value) { + var envCopy = {}; + for (const e in process.env) { + envCopy[e] = process.env[e]; + } + envCopy[newVar] = value; + return envCopy; +} + +function testHelper(stream, args, expectedOutput, cmd, env) { + const fullArgs = args.concat(['-e', 'console.log(' + cmd + ')']); + const child = spawnSync(process.execPath, fullArgs, { + cwd: path.dirname(process.execPath), + env: env + }); + + console.error('Spawned child [pid:' + child.pid + '] with cmd ' + + cmd + ' and args \'' + args + '\''); + + function childOk(child) { + console.error('Child #' + ++num_children_ok + + ' [pid:' + child.pid + '] OK.'); + } + + function responseHandler(buffer, expectedOutput) { + const response = buffer.toString(); + assert.notEqual(0, response.length); + if (FIPS_ENABLED !== expectedOutput && FIPS_DISABLED !== expectedOutput) { + // In the case of expected errors just look for a substring. + assert.notEqual(-1, response.indexOf(expectedOutput)); + } else { + // Normal path where we expect either FIPS enabled or disabled. + assert.equal(expectedOutput, response); + } + childOk(child); + } + + responseHandler(child[stream], expectedOutput); +} + +// By default FIPS should be off in both FIPS and non-FIPS builds. +testHelper( + 'stdout', + [], + FIPS_DISABLED, + 'require("crypto").fips', + addToEnv('OPENSSL_CONF', '')); + +// --enable-fips should turn FIPS mode on +testHelper( + compiledWithFips() ? 'stdout' : 'stderr', + ['--enable-fips'], + compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING, + 'require("crypto").fips', + process.env); + +//--force-fips should turn FIPS mode on +testHelper( + compiledWithFips() ? 'stdout' : 'stderr', + ['--force-fips'], + compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING, + 'require("crypto").fips', + process.env); + +// OpenSSL config file should be able to turn on FIPS mode +testHelper( + 'stdout', + [], + compiledWithFips() ? FIPS_ENABLED : 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'], + compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING, + 'require("crypto").fips', + addToEnv('OPENSSL_CONF', CNF_FIPS_OFF)); + +// --force-fips should take precedence over OpenSSL config file +testHelper( + compiledWithFips() ? 'stdout' : 'stderr', + ['--force-fips'], + compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING, + 'require("crypto").fips', + addToEnv('OPENSSL_CONF', CNF_FIPS_OFF)); + +// setFipsCrypto should be able to turn FIPS mode on +testHelper( + compiledWithFips() ? 'stdout' : 'stderr', + [], + compiledWithFips() ? FIPS_ENABLED : FIPS_ERROR_STRING, + '(require("crypto").fips = true,' + + 'require("crypto").fips)', + addToEnv('OPENSSL_CONF', '')); + +// setFipsCrypto should be able to turn FIPS mode on and off +testHelper( + compiledWithFips() ? 'stdout' : 'stderr', + [], + compiledWithFips() ? FIPS_DISABLED : FIPS_ERROR_STRING, + '(require("crypto").fips = true,' + + 'require("crypto").fips = false,' + + 'require("crypto").fips)', + addToEnv('OPENSSL_CONF', '')); + +// setFipsCrypto takes precedence over OpenSSL config file, FIPS on +testHelper( + compiledWithFips() ? 'stdout' : 'stderr', + [], + compiledWithFips() ? FIPS_ENABLED : FIPS_ERROR_STRING, + '(require("crypto").fips = true,' + + 'require("crypto").fips)', + addToEnv('OPENSSL_CONF', CNF_FIPS_OFF)); + +// setFipsCrypto takes precedence over OpenSSL config file, FIPS off +testHelper( + compiledWithFips() ? 'stdout' : 'stderr', + [], + compiledWithFips() ? FIPS_DISABLED : FIPS_ERROR_STRING, + '(require("crypto").fips = false,' + + 'require("crypto").fips)', + addToEnv('OPENSSL_CONF', CNF_FIPS_ON)); + +// --enable-fips does not prevent use of setFipsCrypto API +testHelper( + compiledWithFips() ? 'stdout' : 'stderr', + ['--enable-fips'], + compiledWithFips() ? FIPS_DISABLED : OPTION_ERROR_STRING, + '(require("crypto").fips = false,' + + 'require("crypto").fips)', + process.env); + +// --force-fips prevents use of setFipsCrypto API +testHelper( + 'stderr', + ['--force-fips'], + compiledWithFips() ? FIPS_ERROR_STRING : OPTION_ERROR_STRING, + 'require("crypto").fips = false', + process.env); + +// --force-fips and --enable-fips order does not matter +testHelper( + 'stderr', + ['--force-fips', '--enable-fips'], + compiledWithFips() ? FIPS_ERROR_STRING : 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, + 'require("crypto").fips = false', + process.env); diff --git a/test/pummel/test-crypto-dh.js b/test/pummel/test-crypto-dh.js index 596d107a287a28..8d732ed92efd10 100644 --- a/test/pummel/test-crypto-dh.js +++ b/test/pummel/test-crypto-dh.js @@ -2,9 +2,7 @@ var common = require('../common'); var assert = require('assert'); -try { - var crypto = require('crypto'); -} catch (e) { +if (!common.hasCrypto) { console.log('1..0 # Skipped: node compiled without OpenSSL.'); return; }