Skip to content

Commit

Permalink
src,test: raise error for --enable-fips when no FIPS
Browse files Browse the repository at this point in the history
This commit moves the check for FIPS from the crypto module
initialization to process startup.

The motivation for this is that when OpenSSL is not FIPS enabled and the
command line options --enable-fips, or --force-fips are used, there will
only be an error raised if the crypto module is used. This can be
surprising and we have gotten feedback that users assumed that there
would be an error if these options were specified and FIPS is not
available.

PR-URL: #38859
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
  • Loading branch information
danbev committed Jun 8, 2021
1 parent 911ff34 commit 1997aa3
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 26 deletions.
40 changes: 19 additions & 21 deletions src/crypto/crypto_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,9 @@

#include "math.h"

#ifdef OPENSSL_FIPS
#if OPENSSL_VERSION_MAJOR >= 3
#include "openssl/provider.h"
#endif
#endif

#include <openssl/rand.h>

Expand Down Expand Up @@ -107,6 +105,25 @@ int NoPasswordCallback(char* buf, int size, int rwflag, void* u) {
return 0;
}

bool ProcessFipsOptions() {
/* Override FIPS settings in configuration file, if needed. */
if (per_process::cli_options->enable_fips_crypto ||
per_process::cli_options->force_fips_crypto) {
#if OPENSSL_VERSION_MAJOR >= 3
OSSL_PROVIDER* fips_provider = OSSL_PROVIDER_load(nullptr, "fips");
if (fips_provider == nullptr)
return false;
OSSL_PROVIDER_unload(fips_provider);

return EVP_default_properties_enable_fips(nullptr, 1) &&
EVP_default_properties_is_fips_enabled(nullptr);
#else
return FIPS_mode() == 0 && FIPS_mode_set(1);
#endif
}
return true;
}

void InitCryptoOnce() {
#ifndef OPENSSL_IS_BORINGSSL
OPENSSL_INIT_SETTINGS* settings = OPENSSL_INIT_new();
Expand Down Expand Up @@ -143,25 +160,6 @@ void InitCryptoOnce() {
}
#endif

/* Override FIPS settings in cnf file, if needed. */
unsigned long err = 0; // NOLINT(runtime/int)
if (per_process::cli_options->enable_fips_crypto ||
per_process::cli_options->force_fips_crypto) {
#if OPENSSL_VERSION_MAJOR >= 3
if (0 == EVP_default_properties_is_fips_enabled(nullptr) &&
!EVP_default_properties_enable_fips(nullptr, 1)) {
#else
if (0 == FIPS_mode() && !FIPS_mode_set(1)) {
#endif
err = ERR_get_error();
}
}
if (0 != err) {
auto* isolate = Isolate::GetCurrent();
auto* env = Environment::GetCurrent(isolate);
return ThrowCryptoError(env, err);
}

// Turn off compression. Saves memory and protects against CRIME attacks.
// No-op with OPENSSL_NO_COMP builds of OpenSSL.
sk_SSL_COMP_zero(SSL_COMP_get_compression_methods());
Expand Down
2 changes: 2 additions & 0 deletions src/crypto/crypto_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ using DsaSigPointer = DeleteFnPtr<DSA_SIG, DSA_SIG_free>;
// callback has been made.
extern int VerifyCallback(int preverify_ok, X509_STORE_CTX* ctx);

bool ProcessFipsOptions();

void InitCryptoOnce();

void InitCrypto(v8::Local<v8::Object> target);
Expand Down
14 changes: 11 additions & 3 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1080,9 +1080,17 @@ InitializationResult InitializeOncePerProcess(
OPENSSL_init();
}
#endif
// V8 on Windows doesn't have a good source of entropy. Seed it from
// OpenSSL's pool.
V8::SetEntropySource(crypto::EntropySource);
if (!crypto::ProcessFipsOptions()) {
result.exit_code = ERR_GET_REASON(ERR_peek_error());
result.early_return = true;
fprintf(stderr, "OpenSSL error when trying to enable FIPS:\n");
ERR_print_errors_fp(stderr);
return result;
}

// V8 on Windows doesn't have a good source of entropy. Seed it from
// OpenSSL's pool.
V8::SetEntropySource(crypto::EntropySource);
#endif // HAVE_OPENSSL
}
per_process::v8_platform.Initialize(
Expand Down
23 changes: 21 additions & 2 deletions test/parallel/test-crypto-fips.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const FIPS_ERROR_STRING2 =
'Error [ERR_CRYPTO_FIPS_FORCED]: Cannot set FIPS mode, it was forced with ' +
'--force-fips at startup.';
const FIPS_UNSUPPORTED_ERROR_STRING = 'fips mode not supported';
const FIPS_ENABLE_ERROR_STRING = 'OpenSSL error when trying to enable FIPS:';

const CNF_FIPS_ON = fixtures.path('openssl_fips_enabled.cnf');
const CNF_FIPS_OFF = fixtures.path('openssl_fips_disabled.cnf');
Expand Down Expand Up @@ -49,15 +50,33 @@ function testHelper(stream, args, expectedOutput, cmd, env) {
// In the case of expected errors just look for a substring.
assert.ok(response.includes(expectedOutput));
} else {
// Normal path where we expect either FIPS enabled or disabled.
assert.strictEqual(Number(response), expectedOutput);
const getFipsValue = Number(response);
if (!Number.isNaN(getFipsValue))
// Normal path where we expect either FIPS enabled or disabled.
assert.strictEqual(getFipsValue, expectedOutput);
}
childOk(child);
}

responseHandler(child[stream], expectedOutput);
}

// --enable-fips should raise an error if OpenSSL is not FIPS enabled.
testHelper(
testFipsCrypto() ? 'stdout' : 'stderr',
['--enable-fips'],
testFipsCrypto() ? FIPS_ENABLED : FIPS_ENABLE_ERROR_STRING,
'process.versions',
process.env);

// --force-fips should raise an error if OpenSSL is not FIPS enabled.
testHelper(
testFipsCrypto() ? 'stdout' : 'stderr',
['--force-fips'],
testFipsCrypto() ? FIPS_ENABLED : FIPS_ENABLE_ERROR_STRING,
'process.versions',
process.env);

// By default FIPS should be off in both FIPS and non-FIPS builds.
testHelper(
'stdout',
Expand Down

0 comments on commit 1997aa3

Please sign in to comment.