OSSForks/node
1
0
Fork 0
mirror of https://github.com/zebrajr/node.git synced 2025-12-06 00:20:08 +01:00
Code Issues Actions 8 Packages Projects Releases Wiki Activity

src: fix FIPS init error handling

Browse Source 
If `--enable-fips` or `--force-fips` fails to be applied during
`ProcessFipsOptions()`, the node process might exit with
`ExitCode::kNoFailure` because `ERR_GET_REASON(ERR_peek_error())` can
return `0` since `ncrypto::testFipsEnabled()` does not populate the
OpenSSL error queue.

You can likely test this locally by running

    node --enable-fips && echo $?

with the current `node` binary from the main branch if compiled without
support for FIPS.

As confirmed by the `XXX` comment here, which was added three years ago
in commit b697160256, even if the error
queue was populated properly, the OpenSSL reason codes are not really
related to the Node.js `ExitCode` enumeration.

PR-URL: https://github.com/nodejs/node/pull/58379
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
This commit is contained in:
Tobias Nießen 2025-05-30 12:23:57 +01:00 committed by GitHub
parent 0854aa3324
commit 7622f0d050
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 29 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/node.cc
View File

@ -1167,10 +1167,7 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,
}
#endif
if (!crypto::ProcessFipsOptions()) {
// XXX: ERR_GET_REASON does not return something that is
// useful as an exit code at all.
result->exit_code_ =
static_cast<ExitCode>(ERR_GET_REASON(ERR_peek_error()));
result->exit_code_ = ExitCode::kGenericUserError;
result->early_return_ = true;
result->errors_.emplace_back(
"OpenSSL error when trying to enable FIPS:\n" +

29
test/parallel/test-crypto-fips.js
View File

@ -23,13 +23,16 @@ 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');
const kNoFailure = 0;
const kGenericUserError = 1;
let num_children_ok = 0;
function sharedOpenSSL() {
return process.config.variables.node_shared_openssl;
}
function testHelper(stream, args, expectedOutput, cmd, env) {
function testHelper(stream, args, expectedStatus, expectedOutput, cmd, env) {
const fullArgs = args.concat(['-e', `console.log(${cmd})`]);
const child = spawnSync(process.execPath, fullArgs, {
cwd: path.dirname(process.execPath),
@ -56,6 +59,7 @@ function testHelper(stream, args, expectedOutput, cmd, env) {
// Normal path where we expect either FIPS enabled or disabled.
assert.strictEqual(getFipsValue, expectedOutput);
}
assert.strictEqual(child.status, expectedStatus);
childOk(child);
}
@ -66,6 +70,7 @@ function testHelper(stream, args, expectedOutput, cmd, env) {
testHelper(
testFipsCrypto() ? 'stdout' : 'stderr',
['--enable-fips'],
testFipsCrypto() ? kNoFailure : kGenericUserError,
testFipsCrypto() ? FIPS_ENABLED : FIPS_ENABLE_ERROR_STRING,
'process.versions',
process.env);
@ -74,6 +79,7 @@ testHelper(
testHelper(
testFipsCrypto() ? 'stdout' : 'stderr',
['--force-fips'],
testFipsCrypto() ? kNoFailure : kGenericUserError,
testFipsCrypto() ? FIPS_ENABLED : FIPS_ENABLE_ERROR_STRING,
'process.versions',
process.env);
@ -85,6 +91,7 @@ if (!sharedOpenSSL()) {
testHelper(
'stdout',
[],
kNoFailure,
FIPS_DISABLED,
'require("crypto").getFips()',
{ ...process.env, 'OPENSSL_CONF': ' ' });
@ -94,6 +101,7 @@ if (!sharedOpenSSL()) {
testHelper(
'stderr',
[],
kGenericUserError,
'Calling crypto.setFips() is not supported in workers',
'new worker_threads.Worker(\'require("crypto").setFips(true);\', { eval: true })',
process.env);
@ -120,6 +128,7 @@ if (!sharedOpenSSL() && !hasOpenSSL3) {
testHelper(
'stdout',
[`--openssl-config=${CNF_FIPS_ON}`],
kNoFailure,
testFipsCrypto() ? FIPS_ENABLED : FIPS_DISABLED,
'require("crypto").getFips()',
process.env);
@ -128,6 +137,7 @@ if (!sharedOpenSSL() && !hasOpenSSL3) {
testHelper(
'stdout',
[],
kNoFailure,
testFipsCrypto() ? FIPS_ENABLED : FIPS_DISABLED,
'require("crypto").getFips()',
Object.assign({}, process.env, { 'OPENSSL_CONF': CNF_FIPS_ON }));
@ -136,6 +146,7 @@ if (!sharedOpenSSL() && !hasOpenSSL3) {
testHelper(
'stdout',
[`--openssl-config=${CNF_FIPS_ON}`],
kNoFailure,
testFipsCrypto() ? FIPS_ENABLED : FIPS_DISABLED,
'require("crypto").getFips()',
Object.assign({}, process.env, { 'OPENSSL_CONF': CNF_FIPS_OFF }));
@ -149,6 +160,7 @@ if (!hasOpenSSL3) {
testHelper(
'stdout',
[`--openssl-config=${CNF_FIPS_OFF}`],
kNoFailure,
FIPS_DISABLED,
'require("crypto").getFips()',
Object.assign({}, process.env, { 'OPENSSL_CONF': CNF_FIPS_ON }));
@ -157,6 +169,7 @@ if (!hasOpenSSL3) {
testHelper(
testFipsCrypto() ? 'stdout' : 'stderr',
['--enable-fips', `--openssl-config=${CNF_FIPS_OFF}`],
testFipsCrypto() ? kNoFailure : kGenericUserError,
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
'require("crypto").getFips()',
process.env);
@ -164,6 +177,7 @@ if (!hasOpenSSL3) {
testHelper(
testFipsCrypto() ? 'stdout' : 'stderr',
['--force-fips', `--openssl-config=${CNF_FIPS_OFF}`],
testFipsCrypto() ? kNoFailure : kGenericUserError,
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
'require("crypto").getFips()',
process.env);
@ -171,6 +185,7 @@ if (!hasOpenSSL3) {
testHelper(
testFipsCrypto() ? 'stdout' : 'stderr',
['--enable-fips'],
testFipsCrypto() ? kNoFailure : kGenericUserError,
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
'require("crypto").getFips()',
process.env);
@ -179,6 +194,7 @@ if (!hasOpenSSL3) {
testHelper(
testFipsCrypto() ? 'stdout' : 'stderr',
['--force-fips'],
testFipsCrypto() ? kNoFailure : kGenericUserError,
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
'require("crypto").getFips()',
process.env);
@ -187,6 +203,7 @@ if (!hasOpenSSL3) {
testHelper(
testFipsCrypto() ? 'stdout' : 'stderr',
['--enable-fips'],
testFipsCrypto() ? kNoFailure : kGenericUserError,
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
'require("crypto").getFips()',
Object.assign({}, process.env, { 'OPENSSL_CONF': CNF_FIPS_OFF }));
@ -195,6 +212,7 @@ if (!hasOpenSSL3) {
testHelper(
testFipsCrypto() ? 'stdout' : 'stderr',
['--force-fips'],
testFipsCrypto() ? kNoFailure : kGenericUserError,
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
'require("crypto").getFips()',
Object.assign({}, process.env, { 'OPENSSL_CONF': CNF_FIPS_OFF }));
@ -203,6 +221,7 @@ if (!hasOpenSSL3) {
testHelper(
testFipsCrypto() ? 'stdout' : 'stderr',
[],
testFipsCrypto() ? kNoFailure : kGenericUserError,
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
'(require("crypto").setFips(true),' +
'require("crypto").getFips())',
@ -212,6 +231,7 @@ if (!hasOpenSSL3) {
testHelper(
testFipsCrypto() ? 'stdout' : 'stderr',
[],
testFipsCrypto() ? kNoFailure : kGenericUserError,
testFipsCrypto() ? FIPS_DISABLED : FIPS_UNSUPPORTED_ERROR_STRING,
'(require("crypto").setFips(true),' +
'require("crypto").setFips(false),' +
@ -222,6 +242,7 @@ if (!hasOpenSSL3) {
testHelper(
testFipsCrypto() ? 'stdout' : 'stderr',
[`--openssl-config=${CNF_FIPS_OFF}`],
testFipsCrypto() ? kNoFailure : kGenericUserError,
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
'(require("crypto").setFips(true),' +
'require("crypto").getFips())',
@ -231,6 +252,7 @@ if (!hasOpenSSL3) {
testHelper(
'stdout',
[`--openssl-config=${CNF_FIPS_ON}`],
kNoFailure,
FIPS_DISABLED,
'(require("crypto").setFips(false),' +
'require("crypto").getFips())',
@ -240,6 +262,7 @@ if (!hasOpenSSL3) {
testHelper(
testFipsCrypto() ? 'stdout' : 'stderr',
['--enable-fips'],
testFipsCrypto() ? kNoFailure : kGenericUserError,
testFipsCrypto() ? FIPS_DISABLED : FIPS_UNSUPPORTED_ERROR_STRING,
'(require("crypto").setFips(false),' +
'require("crypto").getFips())',
@ -249,6 +272,7 @@ if (!hasOpenSSL3) {
testHelper(
'stderr',
['--force-fips'],
kGenericUserError,
testFipsCrypto() ? FIPS_ERROR_STRING2 : FIPS_UNSUPPORTED_ERROR_STRING,
'require("crypto").setFips(false)',
process.env);
@ -257,6 +281,7 @@ if (!hasOpenSSL3) {
testHelper(
testFipsCrypto() ? 'stdout' : 'stderr',
['--force-fips'],
testFipsCrypto() ? kNoFailure : kGenericUserError,
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
'(require("crypto").setFips(true),' +
'require("crypto").getFips())',
@ -266,6 +291,7 @@ if (!hasOpenSSL3) {
testHelper(
'stderr',
['--force-fips', '--enable-fips'],
kGenericUserError,
testFipsCrypto() ? FIPS_ERROR_STRING2 : FIPS_UNSUPPORTED_ERROR_STRING,
'require("crypto").setFips(false)',
process.env);
@ -274,6 +300,7 @@ if (!hasOpenSSL3) {
testHelper(
'stderr',
['--enable-fips', '--force-fips'],
kGenericUserError,
testFipsCrypto() ? FIPS_ERROR_STRING2 : FIPS_UNSUPPORTED_ERROR_STRING,
'require("crypto").setFips(false)',
process.env);