From 095c3e31e6b83a2d50ba96befe1e7ba81602514b Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 10 Mar 2022 11:56:03 +0100 Subject: [PATCH 1/2] src: add proper mutexes for accessing FIPS state The FIPS state handling and OpenSSL initialization code makes accesses to global OpenSSL state without any protection against parallel modifications from multiple threads. This commit adds such protections. --- src/crypto/crypto_util.cc | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index e93edd4b2fc952..13b871dc8cbf16 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -136,7 +136,13 @@ bool InitCryptoOnce(Isolate* isolate) { return true; } +// Protect accesses to FIPS state with a mutex. This should potentially +// be part of a larger mutex for global OpenSSL state. +static Mutex fips_mutex; + void InitCryptoOnce() { + Mutex::ScopedLock lock(per_process::cli_options_mutex); + Mutex::ScopedLock fips_lock(fips_mutex); #ifndef OPENSSL_IS_BORINGSSL OPENSSL_INIT_SETTINGS* settings = OPENSSL_INIT_new(); @@ -196,6 +202,9 @@ void InitCryptoOnce() { } void GetFipsCrypto(const FunctionCallbackInfo& args) { + Mutex::ScopedLock lock(per_process::cli_options_mutex); + Mutex::ScopedLock fips_lock(fips_mutex); + #if OPENSSL_VERSION_MAJOR >= 3 args.GetReturnValue().Set(EVP_default_properties_is_fips_enabled(nullptr) ? 1 : 0); @@ -205,8 +214,13 @@ void GetFipsCrypto(const FunctionCallbackInfo& args) { } void SetFipsCrypto(const FunctionCallbackInfo& args) { + Mutex::ScopedLock lock(per_process::cli_options_mutex); + Mutex::ScopedLock fips_lock(fips_mutex); + CHECK(!per_process::cli_options->force_fips_crypto); Environment* env = Environment::GetCurrent(args); + // TODO: This should not be possible to set from worker threads. + // CHECK(env->owns_process_state()); bool enable = args[0]->BooleanValue(env->isolate()); #if OPENSSL_VERSION_MAJOR >= 3 @@ -227,6 +241,9 @@ void SetFipsCrypto(const FunctionCallbackInfo& args) { } void TestFipsCrypto(const v8::FunctionCallbackInfo& args) { + Mutex::ScopedLock lock(per_process::cli_options_mutex); + Mutex::ScopedLock fips_lock(fips_mutex); + #ifdef OPENSSL_FIPS #if OPENSSL_VERSION_MAJOR >= 3 OSSL_PROVIDER* fips_provider = nullptr; From b5305589629ff35d5feca863eac012346beec3b5 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 10 Mar 2022 12:06:25 +0100 Subject: [PATCH 2/2] fixup! src: add proper mutexes for accessing FIPS state --- src/crypto/crypto_util.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index 13b871dc8cbf16..bbc86e6d88986f 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -219,7 +219,7 @@ void SetFipsCrypto(const FunctionCallbackInfo& args) { CHECK(!per_process::cli_options->force_fips_crypto); Environment* env = Environment::GetCurrent(args); - // TODO: This should not be possible to set from worker threads. + // TODO(addaleax): This should not be possible to set from worker threads. // CHECK(env->owns_process_state()); bool enable = args[0]->BooleanValue(env->isolate());