Skip to content

src: report why --enable-fips failed - #64979

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
panva:fips-startup-error
Aug 5, 2026
Merged

nodejs-github-bot merged 1 commit into
nodejs:mainfrom
panva:fips-startup-error

Conversation

@panva

@panva panva commented Aug 3, 2026

Copy link
Copy Markdown
Member

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.

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 <panva.ip@gmail.com>
@panva
panva requested review from anonrig and jasnell August 3, 2026 14:21
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Aug 3, 2026
@panva panva added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Aug 3, 2026
const FIPS_ENABLE_ERROR_STRING = 'OpenSSL error when trying to enable FIPS:';
const FIPS_ENABLE_ERROR_STRING =
hasOpenSSL3 ?
'--enable-fips requires an active OpenSSL provider named "fips"' :

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was something changed recently in Node.js that forced this? My recollection was that there is no restriction in OpenSSL 3 (the providers can be called anything (you could even have a provider called "fips" that doesn't support FIPS)) and Node.js didn't enforce that restriction either.

@panva panva Aug 3, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I traced it back to 1997aa3, so not recently. You're right that OpenSSL itself only cares about the fips=yes default property and maybe we can fix all in one swoop. 👀

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other words, yes this is the correct message to give as-is unless we change ProcessFipsOptions

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@panva panva removed request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. author ready PRs with CI started, the required approvals, and no outstanding review comments. labels Aug 3, 2026
@panva panva added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. and removed request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. author ready PRs with CI started, the required approvals, and no outstanding review comments. labels Aug 3, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 3, 2026
@nodejs-github-bot

This comment was marked as outdated.

@codecov

codecov Bot commented Aug 3, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 47.36842% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.27%. Comparing base (a937758) to head (f9f5222).
⚠️ Report is 21 commits behind head on main.

Files with missing lines Patch % Lines
src/crypto/crypto_util.cc 41.17% 9 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64979      +/-   ##
==========================================
- Coverage   90.28%   90.27%   -0.01%     
==========================================
  Files         762      762              
  Lines      247524   247534      +10     
  Branches    46684    46693       +9     
==========================================
+ Hits       223466   223470       +4     
- Misses      15490    15506      +16     
+ Partials     8568     8558      -10     
Files with missing lines Coverage Δ
src/crypto/crypto_util.h 67.43% <ø> (ø)
src/node.cc 76.35% <100.00%> (-0.58%) ⬇️
src/crypto/crypto_util.cc 72.00% <41.17%> (-1.39%) ⬇️

... and 21 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@panva panva added the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 3, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 5, 2026
@nodejs-github-bot
nodejs-github-bot merged commit b79dcdf into nodejs:main Aug 5, 2026
112 of 113 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in b79dcdf

@panva
panva deleted the fips-startup-error branch August 5, 2026 15:22
aduh95 pushed a commit that referenced this pull request Aug 13, 2026
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 <panva.ip@gmail.com>
PR-URL: #64979
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
aduh95 pushed a commit that referenced this pull request Aug 25, 2026
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 <panva.ip@gmail.com>
PR-URL: #64979
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
aduh95 pushed a commit that referenced this pull request Aug 27, 2026
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 <panva.ip@gmail.com>
PR-URL: #64979
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants