Skip to content

Commit 08b2a4a

Browse files
danbevtargos
authored andcommitted
src,test: raise error for --enable-fips when no FIPS
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 <midawson@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Richard Lau <rlau@redhat.com>
1 parent 1799ea3 commit 08b2a4a

File tree

4 files changed

+53
-26
lines changed

4 files changed

+53
-26
lines changed

src/crypto/crypto_util.cc

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,9 @@
1414

1515
#include "math.h"
1616

17-
#ifdef OPENSSL_FIPS
1817
#if OPENSSL_VERSION_MAJOR >= 3
1918
#include "openssl/provider.h"
2019
#endif
21-
#endif
2220

2321
#include <openssl/rand.h>
2422

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

108+
bool ProcessFipsOptions() {
109+
/* Override FIPS settings in configuration file, if needed. */
110+
if (per_process::cli_options->enable_fips_crypto ||
111+
per_process::cli_options->force_fips_crypto) {
112+
#if OPENSSL_VERSION_MAJOR >= 3
113+
OSSL_PROVIDER* fips_provider = OSSL_PROVIDER_load(nullptr, "fips");
114+
if (fips_provider == nullptr)
115+
return false;
116+
OSSL_PROVIDER_unload(fips_provider);
117+
118+
return EVP_default_properties_enable_fips(nullptr, 1) &&
119+
EVP_default_properties_is_fips_enabled(nullptr);
120+
#else
121+
return FIPS_mode() == 0 && FIPS_mode_set(1);
122+
#endif
123+
}
124+
return true;
125+
}
126+
110127
void InitCryptoOnce() {
111128
#ifndef OPENSSL_IS_BORINGSSL
112129
OPENSSL_INIT_SETTINGS* settings = OPENSSL_INIT_new();
@@ -143,25 +160,6 @@ void InitCryptoOnce() {
143160
}
144161
#endif
145162

146-
/* Override FIPS settings in cnf file, if needed. */
147-
unsigned long err = 0; // NOLINT(runtime/int)
148-
if (per_process::cli_options->enable_fips_crypto ||
149-
per_process::cli_options->force_fips_crypto) {
150-
#if OPENSSL_VERSION_MAJOR >= 3
151-
if (0 == EVP_default_properties_is_fips_enabled(nullptr) &&
152-
!EVP_default_properties_enable_fips(nullptr, 1)) {
153-
#else
154-
if (0 == FIPS_mode() && !FIPS_mode_set(1)) {
155-
#endif
156-
err = ERR_get_error();
157-
}
158-
}
159-
if (0 != err) {
160-
auto* isolate = Isolate::GetCurrent();
161-
auto* env = Environment::GetCurrent(isolate);
162-
return ThrowCryptoError(env, err);
163-
}
164-
165163
// Turn off compression. Saves memory and protects against CRIME attacks.
166164
// No-op with OPENSSL_NO_COMP builds of OpenSSL.
167165
sk_SSL_COMP_zero(SSL_COMP_get_compression_methods());

src/crypto/crypto_util.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ using DsaSigPointer = DeleteFnPtr<DSA_SIG, DSA_SIG_free>;
8686
// callback has been made.
8787
extern int VerifyCallback(int preverify_ok, X509_STORE_CTX* ctx);
8888

89+
bool ProcessFipsOptions();
90+
8991
void InitCryptoOnce();
9092

9193
void InitCrypto(v8::Local<v8::Object> target);

src/node.cc

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,9 +1080,17 @@ InitializationResult InitializeOncePerProcess(
10801080
OPENSSL_init();
10811081
}
10821082
#endif
1083-
// V8 on Windows doesn't have a good source of entropy. Seed it from
1084-
// OpenSSL's pool.
1085-
V8::SetEntropySource(crypto::EntropySource);
1083+
if (!crypto::ProcessFipsOptions()) {
1084+
result.exit_code = ERR_GET_REASON(ERR_peek_error());
1085+
result.early_return = true;
1086+
fprintf(stderr, "OpenSSL error when trying to enable FIPS:\n");
1087+
ERR_print_errors_fp(stderr);
1088+
return result;
1089+
}
1090+
1091+
// V8 on Windows doesn't have a good source of entropy. Seed it from
1092+
// OpenSSL's pool.
1093+
V8::SetEntropySource(crypto::EntropySource);
10861094
#endif // HAVE_OPENSSL
10871095
}
10881096
per_process::v8_platform.Initialize(

test/parallel/test-crypto-fips.js

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ const FIPS_ERROR_STRING2 =
1717
'Error [ERR_CRYPTO_FIPS_FORCED]: Cannot set FIPS mode, it was forced with ' +
1818
'--force-fips at startup.';
1919
const FIPS_UNSUPPORTED_ERROR_STRING = 'fips mode not supported';
20+
const FIPS_ENABLE_ERROR_STRING = 'OpenSSL error when trying to enable FIPS:';
2021

2122
const CNF_FIPS_ON = fixtures.path('openssl_fips_enabled.cnf');
2223
const CNF_FIPS_OFF = fixtures.path('openssl_fips_disabled.cnf');
@@ -49,15 +50,33 @@ function testHelper(stream, args, expectedOutput, cmd, env) {
4950
// In the case of expected errors just look for a substring.
5051
assert.ok(response.includes(expectedOutput));
5152
} else {
52-
// Normal path where we expect either FIPS enabled or disabled.
53-
assert.strictEqual(Number(response), expectedOutput);
53+
const getFipsValue = Number(response);
54+
if (!Number.isNaN(getFipsValue))
55+
// Normal path where we expect either FIPS enabled or disabled.
56+
assert.strictEqual(getFipsValue, expectedOutput);
5457
}
5558
childOk(child);
5659
}
5760

5861
responseHandler(child[stream], expectedOutput);
5962
}
6063

64+
// --enable-fips should raise an error if OpenSSL is not FIPS enabled.
65+
testHelper(
66+
testFipsCrypto() ? 'stdout' : 'stderr',
67+
['--enable-fips'],
68+
testFipsCrypto() ? FIPS_ENABLED : FIPS_ENABLE_ERROR_STRING,
69+
'process.versions',
70+
process.env);
71+
72+
// --force-fips should raise an error if OpenSSL is not FIPS enabled.
73+
testHelper(
74+
testFipsCrypto() ? 'stdout' : 'stderr',
75+
['--force-fips'],
76+
testFipsCrypto() ? FIPS_ENABLED : FIPS_ENABLE_ERROR_STRING,
77+
'process.versions',
78+
process.env);
79+
6180
// By default FIPS should be off in both FIPS and non-FIPS builds.
6281
testHelper(
6382
'stdout',

0 commit comments

Comments
 (0)