crypto: evict getCiphers/getHashes cache on setFips/setEngine by srikanth-karthi · Pull Request #62985 · nodejs/node · GitHub

Summary

getCiphers() and getHashes() used cachedResult() which memoizes on first call and never clears. Two operations mutate the OpenSSL algorithm set but did not invalidate the cache:

setFips(true/false) — restricts/restores the visible cipher/hash list to FIPS-approved algorithms only
setEngine(id, flags) — loads an OpenSSL engine that may register additional ciphers/hashes

This meant that after calling either function, getCiphers() and getHashes() would continue returning the stale pre-call snapshot.

Changes

Replace the two cachedResult() calls for getCiphers/getHashes in lib/internal/crypto/util.js with manual let _ciphersCache / let _hashesCache variables — mirroring the existing _hashCache / getHashCache() pattern already in the same file.
Add evictCipherHashCache() that resets both variables to undefined.
Call evictCipherHashCache() from setEngine() (after a successful engine load) and from setFips() (after setFipsCrypto(val)).
getCurves intentionally keeps using cachedResult() — curves are not affected by FIPS mode or engine loading.

getCiphers() and getHashes() used cachedResult() which memoizes once and never clears. setFips() changes which algorithms OpenSSL exposes (FIPS-approved only vs. all), and setEngine() can register additional ciphers/hashes from a loaded engine, but neither invalidated the cache. Replace the two cachedResult() calls with manual cache variables (_ciphersCache, _hashesCache) that mirror the existing _hashCache pattern. Add evictCipherHashCache() and call it from both setFips() and setEngine() after they mutate OpenSSL state. getCurves is intentionally left using cachedResult — curves are not affected by FIPS mode or engine loading. Fixes: nodejs#62982

Review requested:

@nodejs/crypto

@srikanth-karthi thank you, this looks good on first a cursory look but can you add a test? we do have fips enabled runners in CI that could use one.

As #62982 details mention: under BoringSSL, internal code in conditionalAlgorithms calls getHashes and getCiphers at load.

node/lib/internal/crypto/util.js

Lines 405 to 410 in bb85d23

    
           'ChaCha20-Poly1305': !process.features.openssl_is_boringssl || 
        
             ArrayPrototypeIncludes(getCiphers(), 'chacha20-poly1305'), 
        
           'cSHAKE128': !process.features.openssl_is_boringssl || 
        
             ArrayPrototypeIncludes(getHashes(), 'shake128'), 
        
           'cSHAKE256': !process.features.openssl_is_boringssl || 
        
             ArrayPrototypeIncludes(getHashes(), 'shake256'),

Those results are also stale

BoringSSL has no fips mode. And when it's not boringssl and the ciphers "become unavailable" due to a toggle switch they'll start failing with OperationError and the only inconsistency will be SubtleCrypto.supports reporting true when it should be false.

The "fips to non-fips" case 🤷

Imho not worth the churn.

Thanks for the context. Agreed — under BoringSSL there's no FIPS mode so conditionalAlgorithms is stable for the process lifetime, and for OpenSSL the SubtleCrypto.supports() inconsistency after a FIPS toggle is an accepted limitation given the churn required to fix it. I'll leave conditionalAlgorithms out of scope for this PR.

@ChALkeR what do you think about keeping the identity of the cached result the same? that way consumer can actually rely on their returned value and not get tripped up by someone elses' concern flipping fips mode.

Add a FIPS-only regression test for nodejs#62982 that confirms getCiphers() and getHashes() reflect the restricted FIPS algorithm set after setFips(true) and restore the full list after setFips(false). Signed-off-by: srikanth-karthi <srikanthkarthi2003@gmail.com>

Capture initialFips state and explicitly disable FIPS before recording the baseline algorithm lists, so the test works correctly on systems where FIPS is enabled by default. Restore the original state on exit. Signed-off-by: srikanth-karthi <srikanthkarthi2003@gmail.com>

Each test runs in its own process so restoring the initial FIPS state is unnecessary. Signed-off-by: srikanth-karthi <srikanthkarthi2003@gmail.com>

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.66%. Comparing base (85ae4cd) to head (76e61cb).
⚠️ Report is 1085 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #62985   +/-   ##
=======================================
  Coverage   89.66%   89.66%           
=======================================
  Files         707      707           
  Lines      219501   219523   +22     
  Branches    42087    42092    +5     
=======================================
+ Hits       196807   196832   +25     
  Misses      14588    14588           
+ Partials     8106     8103    -3

Files with missing lines	Coverage Δ
lib/crypto.js	93.04% <100.00%> (+0.81%)	⬆️
lib/internal/crypto/util.js	95.89% <100.00%> (+0.09%)	⬆️

... and 28 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.

This pull request has been marked as stale due to 90 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

setFips() invalidation is superseded by #65484, I don't believe dealing with setEngine()-triggered invalidation is worth the churn since engines are on the way out of the codebase (#63966, #64777 (comment)).

nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Apr 27, 2026

panva reviewed Apr 27, 2026

View reviewed changes

Comment thread test/parallel/test-crypto-ciphers-hashes-fips-cache.js Outdated

panva added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Apr 27, 2026

panva reviewed Apr 27, 2026

View reviewed changes

Comment thread test/parallel/test-crypto-ciphers-hashes-fips-cache.js Outdated

srikanth-karthi added 3 commits April 27, 2026 16:26

test: drop FIPS state restore at end of cipher/hash cache test

76e61cb

Each test runs in its own process so restoring the initial FIPS state is unnecessary. Signed-off-by: srikanth-karthi <srikanthkarthi2003@gmail.com>

srikanth-karthi force-pushed the fix-cipher-hash-cache-eviction branch from 967e31f to 76e61cb Compare April 27, 2026 10:57

github-actions Bot added the stale label Aug 6, 2026

panva closed this Aug 23, 2026

crypto: evict getCiphers/getHashes cache on setFips/setEngine by srikanth-karthi · Pull Request #62985 · nodejs/node · GitHub

Uh oh!

Uh oh!

crypto: evict getCiphers/getHashes cache on setFips/setEngine - #62985

crypto: evict getCiphers/getHashes cache on setFips/setEngine#62985
srikanth-karthi wants to merge 4 commits into
nodejs:mainfrom
srikanth-karthi:fix-cipher-hash-cache-eviction

srikanth-karthi commented Apr 27, 2026 •
edited
Loading

Uh oh!

Uh oh!

nodejs-github-bot commented Apr 27, 2026

Uh oh!

panva commented Apr 27, 2026 •
edited
Loading

Uh oh!

Uh oh!

ChALkeR commented Apr 27, 2026 •
edited
Loading

Uh oh!

Uh oh!

panva commented Apr 27, 2026 •
edited
Loading

Uh oh!

Uh oh!

srikanth-karthi commented Apr 27, 2026

Uh oh!

Uh oh!

Uh oh!

panva commented Apr 27, 2026

Uh oh!

codecov Bot commented Apr 27, 2026 •
edited
Loading

Uh oh!

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

panva commented Aug 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

crypto: evict getCiphers/getHashes cache on setFips/setEngine by srikanth-karthi · Pull Request #62985 · nodejs/node · GitHub

Uh oh!

Uh oh!

crypto: evict getCiphers/getHashes cache on setFips/setEngine - #62985

crypto: evict getCiphers/getHashes cache on setFips/setEngine#62985srikanth-karthi wants to merge 4 commits intonodejs:mainnodejs/node:mainfrom srikanth-karthi:fix-cipher-hash-cache-evictionsrikanth-karthi/node:fix-cipher-hash-cache-evictionCopy head branch name to clipboard

Conversation

srikanth-karthi commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Uh oh!

nodejs-github-bot commented Apr 27, 2026

Uh oh!

panva commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChALkeR commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

panva commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srikanth-karthi commented Apr 27, 2026

Uh oh!

Uh oh!

Uh oh!

panva commented Apr 27, 2026

Uh oh!

codecov Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

panva commented Aug 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

crypto: evict getCiphers/getHashes cache on setFips/setEngine#62985
srikanth-karthi wants to merge 4 commits into
nodejs:mainfrom
srikanth-karthi:fix-cipher-hash-cache-eviction

srikanth-karthi commented Apr 27, 2026 •
edited
Loading

panva commented Apr 27, 2026 •
edited
Loading

ChALkeR commented Apr 27, 2026 •
edited
Loading

panva commented Apr 27, 2026 •
edited
Loading

codecov Bot commented Apr 27, 2026 •
edited
Loading