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 1/7] 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. From 7aefc267fcb43cd91777b8f49303733a2128b237 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Mon, 14 Dec 2020 14:35:24 +0100 Subject: [PATCH 2/7] Throw JavaScript exception if FIPS mode cannot be enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jan Staněk --- src/crypto/crypto_util.cc | 7 +++---- src/node_crypto.cc | 10 ++++++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index 34058d2dcae90d..0d44d0bb4e16f7 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -142,10 +142,9 @@ void InitCryptoOnce() { } } if (0 != err) { - fprintf(stderr, - "openssl fips failed: %s\n", - ERR_error_string(err, nullptr)); - UNREACHABLE(); + auto* isolate = Isolate::GetCurrent(); + auto* env = Environment::GetCurrent(isolate); + return ThrowCryptoError(env, err); } // Turn off compression. Saves memory and protects against CRIME attacks. diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 861125111bec48..b37e47d35b9a92 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -31,6 +31,7 @@ namespace node { using v8::Context; using v8::Local; using v8::Object; +using v8::TryCatch; using v8::Value; namespace crypto { @@ -39,10 +40,15 @@ void Initialize(Local target, Local unused, Local context, void* priv) { + Environment* env = Environment::GetCurrent(context); + static uv_once_t init_once = UV_ONCE_INIT; + TryCatch try_catch{env->isolate()}; uv_once(&init_once, InitCryptoOnce); - - Environment* env = Environment::GetCurrent(context); + if (try_catch.HasCaught() && !try_catch.HasTerminated()) { + try_catch.ReThrow(); + return; + } AES::Initialize(env, target); CipherBase::Initialize(env, target); From eb4ff0b438fa522ed98ea233b4ed97709b4b986c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Stan=C4=9Bk?= Date: Mon, 30 Nov 2020 13:08:01 +0100 Subject: [PATCH 3/7] Adjust tests for always-available FIPS options - The fipsMode constant (defined at compile time) was replaced by the new `TestFipsCrypto()`/`testFipsCrypto()` functions, which rely on the OpenSSL function `FIPS_selftest()`. This results in the FIPS mode being always checked on runtime and being informed purely by the OpenSSL implementation in use. --- doc/api/cli.md | 8 +-- lib/crypto.js | 22 ++---- test/parallel/test-crypto-fips.js | 71 +++++++++---------- ...rocess-env-allowed-flags-are-documented.js | 9 --- 4 files changed, 40 insertions(+), 70 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 70cb69cd54a451..2eba5bb7a715ff 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -186,8 +186,8 @@ code from strings throw an exception instead. This does not affect the Node.js added: v6.0.0 --> -Enable FIPS-compliant crypto at startup. (Requires Node.js to be built with -`./configure --openssl-fips`.) +Enable FIPS-compliant crypto at startup. (Requires Node.js to be built +against FIPS-compatible OpenSSL.) ### `--enable-source-maps` Load an OpenSSL configuration file on startup. Among other uses, this can be -used to enable FIPS-compliant crypto if Node.js is built with -`./configure --openssl-fips`. +used to enable FIPS-compliant crypto if Node.js is built +with against FIPS-enabled OpenSSL. ### `--pending-deprecation`