From 08b2a4a13809d00e7736fe07c2eeda9d777cde69 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Mon, 31 May 2021 06:08:01 +0200 Subject: [PATCH] src,test: raise error for --enable-fips when no FIPS This commit moves the check for FIPS from the crypto module initialization to process startup. The motivation for this is that when OpenSSL is not FIPS enabled and the command line options --enable-fips, or --force-fips are used, there will only be an error raised if the crypto module is used. This can be surprising and we have gotten feedback that users assumed that there would be an error if these options were specified and FIPS is not available. PR-URL: https://github.com/nodejs/node/pull/38859 Reviewed-By: Michael Dawson Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Richard Lau --- src/crypto/crypto_util.cc | 40 +++++++++++++++---------------- src/crypto/crypto_util.h | 2 ++ src/node.cc | 14 ++++++++--- test/parallel/test-crypto-fips.js | 23 ++++++++++++++++-- 4 files changed, 53 insertions(+), 26 deletions(-) diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index bc4efe5f597263..13c40dcb757661 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -14,11 +14,9 @@ #include "math.h" -#ifdef OPENSSL_FIPS #if OPENSSL_VERSION_MAJOR >= 3 #include "openssl/provider.h" #endif -#endif #include @@ -107,6 +105,25 @@ int NoPasswordCallback(char* buf, int size, int rwflag, void* u) { return 0; } +bool ProcessFipsOptions() { + /* Override FIPS settings in configuration file, if needed. */ + if (per_process::cli_options->enable_fips_crypto || + per_process::cli_options->force_fips_crypto) { +#if OPENSSL_VERSION_MAJOR >= 3 + OSSL_PROVIDER* fips_provider = OSSL_PROVIDER_load(nullptr, "fips"); + if (fips_provider == nullptr) + return false; + OSSL_PROVIDER_unload(fips_provider); + + return EVP_default_properties_enable_fips(nullptr, 1) && + EVP_default_properties_is_fips_enabled(nullptr); +#else + return FIPS_mode() == 0 && FIPS_mode_set(1); +#endif + } + return true; +} + void InitCryptoOnce() { #ifndef OPENSSL_IS_BORINGSSL OPENSSL_INIT_SETTINGS* settings = OPENSSL_INIT_new(); @@ -143,25 +160,6 @@ void InitCryptoOnce() { } #endif - /* Override FIPS settings in cnf file, if needed. */ - unsigned long err = 0; // NOLINT(runtime/int) - if (per_process::cli_options->enable_fips_crypto || - per_process::cli_options->force_fips_crypto) { -#if OPENSSL_VERSION_MAJOR >= 3 - if (0 == EVP_default_properties_is_fips_enabled(nullptr) && - !EVP_default_properties_enable_fips(nullptr, 1)) { -#else - if (0 == FIPS_mode() && !FIPS_mode_set(1)) { -#endif - err = ERR_get_error(); - } - } - if (0 != err) { - auto* isolate = Isolate::GetCurrent(); - auto* env = Environment::GetCurrent(isolate); - return ThrowCryptoError(env, err); - } - // Turn off compression. Saves memory and protects against CRIME attacks. // No-op with OPENSSL_NO_COMP builds of OpenSSL. sk_SSL_COMP_zero(SSL_COMP_get_compression_methods()); diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h index 94bcb100cca0e2..ac95612a0b1a85 100644 --- a/src/crypto/crypto_util.h +++ b/src/crypto/crypto_util.h @@ -86,6 +86,8 @@ using DsaSigPointer = DeleteFnPtr; // callback has been made. extern int VerifyCallback(int preverify_ok, X509_STORE_CTX* ctx); +bool ProcessFipsOptions(); + void InitCryptoOnce(); void InitCrypto(v8::Local target); diff --git a/src/node.cc b/src/node.cc index a9afbd2682f785..3ca2a05d8b8b96 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1080,9 +1080,17 @@ InitializationResult InitializeOncePerProcess( OPENSSL_init(); } #endif - // V8 on Windows doesn't have a good source of entropy. Seed it from - // OpenSSL's pool. - V8::SetEntropySource(crypto::EntropySource); + if (!crypto::ProcessFipsOptions()) { + result.exit_code = ERR_GET_REASON(ERR_peek_error()); + result.early_return = true; + fprintf(stderr, "OpenSSL error when trying to enable FIPS:\n"); + ERR_print_errors_fp(stderr); + return result; + } + + // V8 on Windows doesn't have a good source of entropy. Seed it from + // OpenSSL's pool. + V8::SetEntropySource(crypto::EntropySource); #endif // HAVE_OPENSSL } per_process::v8_platform.Initialize( diff --git a/test/parallel/test-crypto-fips.js b/test/parallel/test-crypto-fips.js index b6e70b62be68b9..ba8a1ba653ec55 100644 --- a/test/parallel/test-crypto-fips.js +++ b/test/parallel/test-crypto-fips.js @@ -17,6 +17,7 @@ const FIPS_ERROR_STRING2 = 'Error [ERR_CRYPTO_FIPS_FORCED]: Cannot set FIPS mode, it was forced with ' + '--force-fips at startup.'; const FIPS_UNSUPPORTED_ERROR_STRING = 'fips mode not supported'; +const FIPS_ENABLE_ERROR_STRING = 'OpenSSL error when trying to enable FIPS:'; const CNF_FIPS_ON = fixtures.path('openssl_fips_enabled.cnf'); const CNF_FIPS_OFF = fixtures.path('openssl_fips_disabled.cnf'); @@ -49,8 +50,10 @@ function testHelper(stream, args, expectedOutput, cmd, env) { // In the case of expected errors just look for a substring. assert.ok(response.includes(expectedOutput)); } else { - // Normal path where we expect either FIPS enabled or disabled. - assert.strictEqual(Number(response), expectedOutput); + const getFipsValue = Number(response); + if (!Number.isNaN(getFipsValue)) + // Normal path where we expect either FIPS enabled or disabled. + assert.strictEqual(getFipsValue, expectedOutput); } childOk(child); } @@ -58,6 +61,22 @@ function testHelper(stream, args, expectedOutput, cmd, env) { responseHandler(child[stream], expectedOutput); } +// --enable-fips should raise an error if OpenSSL is not FIPS enabled. +testHelper( + testFipsCrypto() ? 'stdout' : 'stderr', + ['--enable-fips'], + testFipsCrypto() ? FIPS_ENABLED : FIPS_ENABLE_ERROR_STRING, + 'process.versions', + process.env); + +// --force-fips should raise an error if OpenSSL is not FIPS enabled. +testHelper( + testFipsCrypto() ? 'stdout' : 'stderr', + ['--force-fips'], + testFipsCrypto() ? FIPS_ENABLED : FIPS_ENABLE_ERROR_STRING, + 'process.versions', + process.env); + // By default FIPS should be off in both FIPS and non-FIPS builds. testHelper( 'stdout',