Skip to content

Commit 8c4531b

Browse files
panvaaduh95
panva
authored and
aduh95
committed
src: report why --enable-fips failed
The startup failure always appended the OpenSSL error queue, so when Node.js itself detected the missing fips provider it printed an error header followed by nothing. Report the reason instead. Signed-off-by: Filip Skokan <[email protected]> PR-URL: #64979 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
1 parent 28cad47 commit 8c4531b

4 files changed

Lines changed: 45 additions & 21 deletions

File tree

‎src/crypto/crypto_util.cc‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -99,20 +99,36 @@ int NoPasswordCallback(char* buf, int size, int rwflag, void* u) {
9999
return 0;
100100
}
101101

102-
bool ProcessFipsOptions() {
103-
/* Override FIPS settings in configuration file, if needed. */
104-
if (per_process::cli_options->enable_fips_crypto ||
105-
per_process::cli_options->force_fips_crypto) {
102+
std::optional<std::string> ProcessFipsOptions() {
103+
const bool enable_fips = per_process::cli_options->enable_fips_crypto;
104+
const bool force_fips = per_process::cli_options->force_fips_crypto;
105+
if (!enable_fips && !force_fips) return std::nullopt;
106+
106107
#if OPENSSL_VERSION_MAJOR >= 3
107-
if (!ncrypto::testFipsEnabled()) return false;
108-
return ncrypto::setFipsEnabled(true, nullptr);
109-
#else
110-
// TODO(@jasnell): Remove this ifdef branch when openssl 1.1.1 is
111-
// no longer supported.
112-
if (FIPS_mode() == 0) return FIPS_mode_set(1);
108+
// Whether FIPS-approved implementations are reachable is decided by the
109+
// OpenSSL configuration, not by Node.js. Refuse to start rather than
110+
// restrict the default property query to a provider that is not there,
111+
// which would leave every operation failing as unsupported.
112+
if (!ncrypto::testFipsEnabled()) {
113+
const std::string option = force_fips ? "--force-fips" : "--enable-fips";
114+
return option + " requires an active OpenSSL provider named \"fips\". "
115+
"FIPS mode is configured through OpenSSL; see "
116+
"https://nodejs.org/api/crypto.html#fips-mode";
117+
}
113118
#endif
119+
120+
CryptoErrorList errors{CryptoErrorList::Option::NONE};
121+
if (!ncrypto::setFipsEnabled(true, &errors)) {
122+
std::string error = "OpenSSL error when trying to enable FIPS";
123+
if (!errors.empty()) error += ':';
124+
for (const auto& openssl_error : errors) {
125+
error += '\n';
126+
error += openssl_error;
127+
}
128+
return error;
114129
}
115-
return true;
130+
131+
return std::nullopt;
116132
}
117133

118134
bool InitCryptoOnce(Isolate* isolate) {

‎src/crypto/crypto_util.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,10 @@ constexpr T NumBitsToBytes(T bits) {
6262
return (bits / CHAR_BIT) + ((CHAR_BIT - 1 + (bits % CHAR_BIT)) / CHAR_BIT);
6363
}
6464

65-
bool ProcessFipsOptions();
65+
// Applies the FIPS related command line options. Returns a description of
66+
// what went wrong, or std::nullopt when there was nothing to do or the
67+
// options were applied successfully.
68+
std::optional<std::string> ProcessFipsOptions();
6669

6770
bool InitCryptoOnce(v8::Isolate* isolate);
6871
void InitCryptoOnce();

‎src/node.cc‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,6 +1168,7 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,
11681168
if (!(flags & ProcessInitializationFlags::kNoInitOpenSSL)) {
11691169
#if HAVE_OPENSSL
11701170
#ifndef OPENSSL_IS_BORINGSSL
1171+
#if OPENSSL_VERSION_MAJOR >= 3
11711172
auto GetOpenSSLErrorString = []() -> std::string {
11721173
std::string ret;
11731174
ERR_print_errors_cb(
@@ -1183,7 +1184,6 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,
11831184

11841185
// In the case of FIPS builds we should make sure
11851186
// the random source is properly initialized first.
1186-
#if OPENSSL_VERSION_MAJOR >= 3
11871187
// Call OPENSSL_init_crypto to initialize OPENSSL_INIT_LOAD_CONFIG to
11881188
// avoid the default behavior where errors raised during the parsing of the
11891189
// OpenSSL configuration file are not propagated and cannot be detected.
@@ -1244,12 +1244,10 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,
12441244
OPENSSL_init();
12451245
}
12461246
#endif
1247-
if (!crypto::ProcessFipsOptions()) {
1247+
if (auto fips_error = crypto::ProcessFipsOptions()) {
12481248
result->exit_code_ = ExitCode::kGenericUserError;
12491249
result->early_return_ = true;
1250-
result->errors_.emplace_back(
1251-
"OpenSSL error when trying to enable FIPS:\n" +
1252-
GetOpenSSLErrorString());
1250+
result->errors_.emplace_back(std::move(*fips_error));
12531251
return result;
12541252
}
12551253

‎test/parallel/test-crypto-fips.js‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,14 @@ const FIPS_ERROR_STRING2 =
2121
'Error [ERR_CRYPTO_FIPS_FORCED]: Cannot set FIPS mode, it was forced with ' +
2222
'--force-fips at startup.';
2323
const FIPS_UNSUPPORTED_ERROR_STRING = 'fips mode not supported';
24-
const FIPS_ENABLE_ERROR_STRING = 'OpenSSL error when trying to enable FIPS:';
24+
const FIPS_ENABLE_ERROR_STRING =
25+
hasOpenSSL3 ?
26+
'--enable-fips requires an active OpenSSL provider named "fips"' :
27+
'OpenSSL error when trying to enable FIPS:';
28+
const FIPS_FORCE_ERROR_STRING =
29+
hasOpenSSL3 ?
30+
'--force-fips requires an active OpenSSL provider named "fips"' :
31+
'OpenSSL error when trying to enable FIPS:';
2532

2633
const CNF_FIPS_ON = fixtures.path('openssl_fips_enabled.cnf');
2734
const CNF_FIPS_OFF = fixtures.path('openssl_fips_disabled.cnf');
@@ -75,16 +82,16 @@ testHelper(
7582
['--enable-fips'],
7683
testFipsCrypto() ? kNoFailure : kGenericUserError,
7784
testFipsCrypto() ? FIPS_ENABLED : FIPS_ENABLE_ERROR_STRING,
78-
'process.versions',
85+
'require("crypto").getFips()',
7986
process.env);
8087

8188
// --force-fips should raise an error if OpenSSL is not FIPS enabled.
8289
testHelper(
8390
testFipsCrypto() ? 'stdout' : 'stderr',
8491
['--force-fips'],
8592
testFipsCrypto() ? kNoFailure : kGenericUserError,
86-
testFipsCrypto() ? FIPS_ENABLED : FIPS_ENABLE_ERROR_STRING,
87-
'process.versions',
93+
testFipsCrypto() ? FIPS_ENABLED : FIPS_FORCE_ERROR_STRING,
94+
'require("crypto").getFips()',
8895
process.env);
8996

9097
// By default FIPS should be off in both FIPS and non-FIPS builds

0 commit comments

Comments
 (0)