crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 #58942

panva · 2025-07-03T09:30:19Z

Background:

This bug surfaces when using linked OpenSSL >= 3.4
OpenSSL removed default digest lengths for SHAKE128 and SHAKE256 in Set secure defaults for the SHAKE 128/256 digest output length. openssl/openssl#23877 (first appeared in 3.4)
Before we had 3.5 in CI test: make test-crypto-hash compatible with OpenSSL > 3.4.0 #56160 skipped the tests that would break and let us know, @nodejs/crypto was not pinged there

This PR:

Adds a --pending-deprecate doc-only deprecation for creating SHAKE128/256 digests without an explicit output length
Reinstantes pre-3.4 defaults in crypto.createHash()
Pushes shake128/256 digests from crypto.hash() through crypto.createHash().update().digest() until crypto: support outputLength option in crypto.hash for XOF functions #58121 gets on track

After this lands a semver-major PRs that contain breaking changes and should be released in the next major version. will follow with a runtime deprecation.

This will need backporting all the way back to 20.x

Reverts: #56160
Fixes: #56159
Fixes: #58913
Refs: #58121

Note: FWIW #56160 should not have landed. Any OpenSSL version-related test changes should only accomodate changes in codes, not behaviour.

nodejs-github-bot · 2025-07-03T09:30:25Z

Review requested:

@nodejs/crypto

doc/api/deprecations.md

nodejs-github-bot · 2025-07-03T09:38:37Z

CI: https://ci.nodejs.org/job/node-test-pull-request/67797/

lib/internal/crypto/hash.js

codecov · 2025-07-03T11:23:44Z

Codecov Report

Attention: Patch coverage is 75.55556% with 11 lines in your changes missing coverage. Please review.

Project coverage is 90.08%. Comparing base (ec41686) to head (68fb47c).
Report is 21 commits behind head on main.

Files with missing lines	Patch %	Lines
src/crypto/crypto_hash.cc	0.00%	9 Missing and 1 partial ⚠️
lib/internal/crypto/hash.js	96.77%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #58942      +/-   ##
==========================================
+ Coverage   90.07%   90.08%   +0.01%     
==========================================
  Files         640      640              
  Lines      188473   188516      +43     
  Branches    36972    36986      +14     
==========================================
+ Hits       169763   169830      +67     
+ Misses      11427    11392      -35     
- Partials     7283     7294      +11

Files with missing lines	Coverage Δ
lib/internal/util.js	`96.75% <100.00%> (-0.10%)`	⬇️
lib/internal/crypto/hash.js	`98.43% <96.77%> (-0.23%)`	⬇️
src/crypto/crypto_hash.cc	`70.56% <0.00%> (-2.45%)`	⬇️

... and 30 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 · 2025-07-03T11:53:29Z

CI: https://ci.nodejs.org/job/node-test-pull-request/67802/

lib/internal/crypto/hash.js

panva · 2025-07-03T12:56:08Z

✅ [email protected] https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu2204_sharedlibs_openssl111_x64/51512/
✅ [email protected] https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu2204_sharedlibs_openssl35_x64/51512/

lib/internal/crypto/hash.js

Reverts: nodejs#56160 Fixes: nodejs#56159 Fixes: nodejs#58913 Refs: nodejs#58121

This reverts commit e799722.

This reverts commit e6a988d.

nodejs-github-bot · 2025-07-03T13:50:52Z

CI: https://ci.nodejs.org/job/node-test-pull-request/67809/

nodejs-github-bot · 2025-07-03T15:43:27Z

CI: https://ci.nodejs.org/job/node-test-pull-request/67815/

tniessen

Nice work @panva!

tniessen · 2025-07-04T11:11:00Z

lib/internal/crypto/hash.js

+  // TODO: ideally we have to ship https://github.com/nodejs/node/pull/58121 so
+  // that a proper DEP0198 deprecation can be done here as well.


Suggested change

// TODO: ideally we have to ship https://github.com/nodejs/node/pull/58121 so

// that a proper DEP0198 deprecation can be done here as well.

// TODO(panva): ideally we have to ship https://github.com/nodejs/node/pull/58121

// so that a proper DEP0198 deprecation can be done here as well.

panva · 2025-07-04T11:13:35Z

Thank you for the reviews. I think I'll wait for #58121 to ship and then include its code in the deprecation.

panva · 2025-07-04T12:03:17Z

Actually, this PR is backportable to 20.x as a fix, #58121 is a new feature in a Stability: 1.2 - Release candidate API that we don't have to backport to maintenance LTS.

I'll land this and update #58121 with the deprecation when this lands.

nodejs-github-bot · 2025-07-05T09:34:26Z

Landed in f5da8f8

Reverts: nodejs#56160 Fixes: nodejs#56159 Fixes: nodejs#58913 Refs: nodejs#58121 PR-URL: nodejs#58942 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

Reverts: #56160 Fixes: #56159 Fixes: #58913 Refs: #58121 PR-URL: #58942 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Jul 3, 2025

panva commented Jul 3, 2025

View reviewed changes

doc/api/deprecations.md Outdated Show resolved Hide resolved

panva commented Jul 3, 2025

View reviewed changes

doc/api/deprecations.md Outdated Show resolved Hide resolved

panva force-pushed the xof-openssl34 branch from b4e35d4 to 1898963 Compare July 3, 2025 10:06

panva marked this pull request as ready for review July 3, 2025 10:18

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 Jul 3, 2025

panva requested review from jasnell, tniessen and aduh95 July 3, 2025 10:26

aduh95 reviewed Jul 3, 2025

View reviewed changes

lib/internal/crypto/hash.js Outdated Show resolved Hide resolved

panva force-pushed the xof-openssl34 branch from 1898963 to 71eb03f Compare July 3, 2025 11:37

panva added lts-watch-v20.x PRs that may need to be released in v20.x lts-watch-v22.x PRs that may need to be released in v22.x labels Jul 3, 2025

panva force-pushed the xof-openssl34 branch from 71eb03f to e467910 Compare July 3, 2025 12:44

aduh95 reviewed Jul 3, 2025

View reviewed changes

lib/internal/crypto/hash.js Outdated Show resolved Hide resolved

panva force-pushed the xof-openssl34 branch from e467910 to 727c95a Compare July 3, 2025 12:53

aduh95 reviewed Jul 3, 2025

View reviewed changes

lib/internal/crypto/hash.js Outdated Show resolved Hide resolved

panva force-pushed the xof-openssl34 branch from 727c95a to 0abd2be Compare July 3, 2025 13:03

aduh95 approved these changes Jul 3, 2025

View reviewed changes

lib/internal/crypto/hash.js Outdated Show resolved Hide resolved

jasnell approved these changes Jul 3, 2025

View reviewed changes

panva added 3 commits July 3, 2025 15:37

crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4

d870b4f

Reverts: nodejs#56160 Fixes: nodejs#56159 Fixes: nodejs#58913 Refs: nodejs#58121

Revert "test: make test-crypto-hash compatible with OpenSSL > 3.4.0"

659b8ae

This reverts commit e799722.

Revert "test: disable openssl 3.4.0 incompatible tests"

68fb47c

This reverts commit e6a988d.

panva force-pushed the xof-openssl34 branch from 0abd2be to 68fb47c Compare July 3, 2025 13:37

aduh95 approved these changes Jul 3, 2025

View reviewed changes

panva added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jul 3, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 3, 2025

panva mentioned this pull request Jul 3, 2025

crypto: support outputLength option in crypto.hash for XOF functions #58121

Merged

tniessen approved these changes Jul 4, 2025

View reviewed changes

panva added the blocked PRs that are blocked by other issues or PRs. label Jul 4, 2025

panva added commit-queue Add this label to land a pull request using GitHub Actions. and removed blocked PRs that are blocked by other issues or PRs. labels Jul 4, 2025

joyeecheung approved these changes Jul 4, 2025

View reviewed changes

joyeecheung added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jul 4, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 5, 2025

nodejs-github-bot merged commit f5da8f8 into nodejs:main Jul 5, 2025
72 of 74 checks passed

panva mentioned this pull request Jul 5, 2025

[v22.x backport] crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 #58960

Open

panva added the backport-open-v22.x Indicate that the PR has an open backport label Jul 5, 2025

panva deleted the xof-openssl34 branch July 5, 2025 11:59

panva mentioned this pull request Jul 5, 2025

[v20.x backport] crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 #58961

Open

panva added the backport-open-v20.x Indicate that the PR has an open backport label Jul 5, 2025

github-actions bot mentioned this pull request Jul 8, 2025

2025-07-09, Version 24.4.0 (Current) #58993

Merged

panva mentioned this pull request Jul 9, 2025

crypto: runtime-deprecate default shake128/256 output lengths #59008

Merged

		// TODO: ideally we have to ship https://github.com/nodejs/node/pull/58121 so
		// that a proper DEP0198 deprecation can be done here as well.

Uh oh!

crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 #58942

crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 #58942

Conversation

panva commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 3, 2025

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 3, 2025

Uh oh!

Uh oh!

codecov bot commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nodejs-github-bot commented Jul 3, 2025

Uh oh!

Uh oh!

Uh oh!

panva commented Jul 3, 2025

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 3, 2025

Uh oh!

nodejs-github-bot commented Jul 3, 2025

Uh oh!

tniessen left a comment

Choose a reason for hiding this comment

Uh oh!

tniessen Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

panva commented Jul 4, 2025

Uh oh!

panva commented Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 5, 2025

Uh oh!

Uh oh!

panva commented Jul 3, 2025 •

edited

Loading

codecov bot commented Jul 3, 2025 •

edited

Loading

panva commented Jul 4, 2025 •

edited

Loading