From a1653ac71507d2db54632c09183428bb57ad9f52 Mon Sep 17 00:00:00 2001 From: Sergey Petushkov Date: Fri, 8 Jul 2022 12:50:24 +0200 Subject: [PATCH] crypto: do not allow to call setFips from the worker thread PR-URL: https://github.com/nodejs/node/pull/43624 Reviewed-By: Anna Henningsen Reviewed-By: Filip Skokan Reviewed-By: Antoine du Hamel Reviewed-By: Richard Lau --- lib/crypto.js | 10 ++++++++++ src/crypto/crypto_util.cc | 3 +-- test/parallel/test-crypto-fips.js | 8 ++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/crypto.js b/lib/crypto.js index 77debb9471f03c..35d54b5efb3af7 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -37,6 +37,7 @@ assertCrypto(); const { ERR_CRYPTO_FIPS_FORCED, + ERR_WORKER_UNSUPPORTED_OPERATION, } = require('internal/errors').codes; const constants = internalBinding('constants').crypto; const { getOptionValue } = require('internal/options'); @@ -127,6 +128,12 @@ function lazyWebCrypto() { return webcrypto; } +let ownsProcessState; +function lazyOwnsProcessState() { + ownsProcessState ??= require('internal/worker').ownsProcessState; + return ownsProcessState; +} + // These helper functions are needed because the constructors can // use new, in which case V8 cannot inline the recursive constructor call function createHash(algorithm, options) { @@ -250,6 +257,9 @@ function setFips(val) { if (val) return; throw new ERR_CRYPTO_FIPS_FORCED(); } else { + if (!lazyOwnsProcessState()) { + throw new ERR_WORKER_UNSUPPORTED_OPERATION('Calling crypto.setFips()'); + } setFipsCrypto(val); } } diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index 5d8f0bbe8e3cb4..7da959b45d4683 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -218,8 +218,7 @@ void SetFipsCrypto(const FunctionCallbackInfo& args) { CHECK(!per_process::cli_options->force_fips_crypto); Environment* env = Environment::GetCurrent(args); - // TODO(addaleax): This should not be possible to set from worker threads. - // CHECK(env->owns_process_state()); + CHECK(env->owns_process_state()); bool enable = args[0]->BooleanValue(env->isolate()); #if OPENSSL_VERSION_MAJOR >= 3 diff --git a/test/parallel/test-crypto-fips.js b/test/parallel/test-crypto-fips.js index cc3e4eed79d80f..964d5dbb4054f4 100644 --- a/test/parallel/test-crypto-fips.js +++ b/test/parallel/test-crypto-fips.js @@ -85,6 +85,14 @@ testHelper( 'require("crypto").getFips()', { ...process.env, 'OPENSSL_CONF': ' ' }); +// Toggling fips with setFips should not be allowed from a worker thread +testHelper( + 'stderr', + [], + 'Calling crypto.setFips() is not supported in workers', + 'new worker_threads.Worker(\'require("crypto").setFips(true);\', { eval: true })', + process.env); + // This should succeed for both FIPS and non-FIPS builds in combination with // OpenSSL 1.1.1 or OpenSSL 3.0 const test_result = testFipsCrypto();