From e9ae6069a8d91c1e9202f1d0b4dc91b810e5a1e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Ondruch?= Date: Tue, 25 Aug 2020 14:04:54 +0200 Subject: [PATCH] Make FIPS related options and functionality always awailable. There is no reason to hide FIPS functionality behind build flags. OpenSSL always provide the information about FIPS availability via `FIPS_mode()` function. This makes the user experience more consistent, because the OpenSSL library is always queried and the `crypto.getFips()` always returns OpenSSL settings. Fixes #34903 --- node.gypi | 3 --- src/crypto/crypto_cipher.cc | 4 ---- src/crypto/crypto_sig.cc | 2 -- src/crypto/crypto_util.cc | 17 +++++++++++------ src/crypto/crypto_util.h | 7 +++++-- src/node.cc | 6 +++--- src/node_config.cc | 2 -- src/node_options.cc | 2 -- src/node_options.h | 2 -- 9 files changed, 19 insertions(+), 26 deletions(-) diff --git a/node.gypi b/node.gypi index 43dbda7bbf5302..ad088b133be471 100644 --- a/node.gypi +++ b/node.gypi @@ -319,9 +319,6 @@ [ 'node_use_openssl=="true"', { 'defines': [ 'HAVE_OPENSSL=1' ], 'conditions': [ - ['openssl_fips != "" or openssl_is_fips=="true"', { - 'defines': [ 'NODE_FIPS_MODE' ], - }], [ 'node_shared_openssl=="false"', { 'dependencies': [ './deps/openssl/openssl.gyp:openssl', diff --git a/src/crypto/crypto_cipher.cc b/src/crypto/crypto_cipher.cc index ddbf7114b673cd..47764b5bc7f153 100644 --- a/src/crypto/crypto_cipher.cc +++ b/src/crypto/crypto_cipher.cc @@ -343,12 +343,10 @@ void CipherBase::Init(const char* cipher_type, HandleScope scope(env()->isolate()); MarkPopErrorOnReturn mark_pop_error_on_return; -#ifdef NODE_FIPS_MODE if (FIPS_mode()) { return THROW_ERR_CRYPTO_UNSUPPORTED_OPERATION(env(), "crypto.createCipher() is not supported in FIPS mode."); } -#endif // NODE_FIPS_MODE const EVP_CIPHER* const cipher = EVP_get_cipherbyname(cipher_type); if (cipher == nullptr) @@ -528,14 +526,12 @@ bool CipherBase::InitAuthenticated( return false; } -#ifdef NODE_FIPS_MODE // TODO(tniessen) Support CCM decryption in FIPS mode if (mode == EVP_CIPH_CCM_MODE && kind_ == kDecipher && FIPS_mode()) { THROW_ERR_CRYPTO_UNSUPPORTED_OPERATION(env(), "CCM encryption not supported in FIPS mode"); return false; } -#endif // Tell OpenSSL about the desired length. if (!EVP_CIPHER_CTX_ctrl(ctx_.get(), EVP_CTRL_AEAD_SET_TAG, auth_tag_len, diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc index b0d97ade6c8bd1..e48f94dfa3c3c4 100644 --- a/src/crypto/crypto_sig.cc +++ b/src/crypto/crypto_sig.cc @@ -27,7 +27,6 @@ using v8::Value; namespace crypto { namespace { bool ValidateDSAParameters(EVP_PKEY* key) { -#ifdef NODE_FIPS_MODE /* Validate DSA2 parameters from FIPS 186-4 */ if (FIPS_mode() && EVP_PKEY_DSA == EVP_PKEY_base_id(key)) { DSA* dsa = EVP_PKEY_get0_DSA(key); @@ -43,7 +42,6 @@ bool ValidateDSAParameters(EVP_PKEY* key) { (L == 2048 && N == 256) || (L == 3072 && N == 256); } -#endif // NODE_FIPS_MODE return true; } diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index 0a40b1f7bb4c2e..34058d2dcae90d 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -133,7 +133,6 @@ void InitCryptoOnce() { } #endif -#ifdef NODE_FIPS_MODE /* Override FIPS settings in cnf file, if needed. */ unsigned long err = 0; // NOLINT(runtime/int) if (per_process::cli_options->enable_fips_crypto || @@ -148,7 +147,6 @@ void InitCryptoOnce() { ERR_error_string(err, nullptr)); UNREACHABLE(); } -#endif // NODE_FIPS_MODE // Turn off compression. Saves memory and protects against CRIME attacks. // No-op with OPENSSL_NO_COMP builds of OpenSSL. @@ -162,7 +160,6 @@ void InitCryptoOnce() { NodeBIO::GetMethod(); } -#ifdef NODE_FIPS_MODE void GetFipsCrypto(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(FIPS_mode() ? 1 : 0); } @@ -180,7 +177,16 @@ void SetFipsCrypto(const FunctionCallbackInfo& args) { return ThrowCryptoError(env, err); } } -#endif /* NODE_FIPS_MODE */ + +void TestFipsCrypto(const v8::FunctionCallbackInfo& args) { +#ifdef OPENSSL_FIPS + const auto enabled = FIPS_selftest() ? 1 : 0; +#else // OPENSSL_FIPS + const auto enabled = 0; +#endif // OPENSSL_FIPS + + args.GetReturnValue().Set(enabled); +} void CryptoErrorVector::Capture() { clear(); @@ -652,10 +658,9 @@ void Initialize(Environment* env, Local target) { env->SetMethod(target, "setEngine", SetEngine); #endif // !OPENSSL_NO_ENGINE -#ifdef NODE_FIPS_MODE env->SetMethodNoSideEffect(target, "getFipsCrypto", GetFipsCrypto); env->SetMethod(target, "setFipsCrypto", SetFipsCrypto); -#endif + env->SetMethodNoSideEffect(target, "testFipsCrypto", TestFipsCrypto); NODE_DEFINE_CONSTANT(target, kCryptoJobAsync); NODE_DEFINE_CONSTANT(target, kCryptoJobSync); diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h index 7e6e4c02b4992c..c9f8c62a8d034f 100644 --- a/src/crypto/crypto_util.h +++ b/src/crypto/crypto_util.h @@ -22,6 +22,9 @@ #ifndef OPENSSL_NO_ENGINE # include #endif // !OPENSSL_NO_ENGINE +#ifdef OPENSSL_FIPS +# include +#endif // OPENSSL_FIPS #include #include @@ -510,11 +513,11 @@ bool SetEngine( void SetEngine(const v8::FunctionCallbackInfo& args); #endif // !OPENSSL_NO_ENGINE -#ifdef NODE_FIPS_MODE void GetFipsCrypto(const v8::FunctionCallbackInfo& args); void SetFipsCrypto(const v8::FunctionCallbackInfo& args); -#endif /* NODE_FIPS_MODE */ + +void TestFipsCrypto(const v8::FunctionCallbackInfo& args); class CipherPushContext { public: diff --git a/src/node.cc b/src/node.cc index 09c6fcd7bed812..f3e9ef3511fd00 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1012,11 +1012,11 @@ InitializationResult InitializeOncePerProcess(int argc, char** argv) { if (credentials::SafeGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs)) crypto::UseExtraCaCerts(extra_ca_certs); } -#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 + if (FIPS_mode()) { + OPENSSL_init(); + } // 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_config.cc b/src/node_config.cc index 2d8ad25bbe9c02..176daa88b0fab1 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -42,9 +42,7 @@ static void Initialize(Local target, READONLY_FALSE_PROPERTY(target, "hasOpenSSL"); #endif // HAVE_OPENSSL -#ifdef NODE_FIPS_MODE READONLY_TRUE_PROPERTY(target, "fipsMode"); -#endif #ifdef NODE_HAVE_I18N_SUPPORT diff --git a/src/node_options.cc b/src/node_options.cc index d292231218f1fc..76beb2225ce4c4 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -766,7 +766,6 @@ PerProcessOptionsParser::PerProcessOptionsParser( &PerProcessOptions::ssl_openssl_cert_store); Implies("--use-openssl-ca", "[ssl_openssl_cert_store]"); ImpliesNot("--use-bundled-ca", "[ssl_openssl_cert_store]"); -#if NODE_FIPS_MODE AddOption("--enable-fips", "enable FIPS crypto at startup", &PerProcessOptions::enable_fips_crypto, @@ -775,7 +774,6 @@ PerProcessOptionsParser::PerProcessOptionsParser( "force FIPS crypto (cannot be disabled)", &PerProcessOptions::force_fips_crypto, kAllowedInEnvironment); -#endif AddOption("--secure-heap", "total size of the OpenSSL secure heap", &PerProcessOptions::secure_heap, diff --git a/src/node_options.h b/src/node_options.h index 555adb246a4b1c..b45a93520dfdad 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -245,10 +245,8 @@ class PerProcessOptions : public Options { #endif bool use_openssl_ca = false; bool use_bundled_ca = false; -#if NODE_FIPS_MODE bool enable_fips_crypto = false; bool force_fips_crypto = false; -#endif #endif // Per-process because reports can be triggered outside a known V8 context.