Skip to content

Commit

Permalink
crypto: do not allow to call setFips from the worker thread
Browse files Browse the repository at this point in the history
PR-URL: #43624
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
  • Loading branch information
gribnoysup authored Jul 8, 2022
1 parent 1523a18 commit a1653ac
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 2 deletions.
10 changes: 10 additions & 0 deletions lib/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/crypto/crypto_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,7 @@ void SetFipsCrypto(const FunctionCallbackInfo<Value>& 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
Expand Down
8 changes: 8 additions & 0 deletions test/parallel/test-crypto-fips.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit a1653ac

Please sign in to comment.