crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 by panva · Pull Request #58942 · nodejs/node · GitHub

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.

Review requested:

@nodejs/crypto

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

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 87 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.

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

✅ openssl@1.1.1 https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu2204_sharedlibs_openssl111_x64/51512/
✅ openssl@3.5 https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu2204_sharedlibs_openssl35_x64/51512/

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

This reverts commit e799722.

This reverts commit e6a988d.

Landed in f5da8f8

Reverts: nodejs#56160 Fixes: nodejs#56159 Fixes: nodejs#58913 Refs: nodejs#58121 PR-URL: nodejs#58942 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Reverts: #56160 Fixes: #56159 Fixes: #58913 Refs: #58121 PR-URL: #58942 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Reverts: nodejs#56160 Fixes: nodejs#56159 Fixes: nodejs#58913 Refs: nodejs#58121 PR-URL: nodejs#58942 Backport-PR-URL: nodejs#58960 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

nodejs/node#58942

* chore: bump node in DEPS to v22.18.0 * crypto: fix inclusion of OPENSSL_IS_BORINGSSL define nodejs/node#58845 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58960 * permission: propagate permission model flags on spawn nodejs/node#58853 * esm: syncify default path of ModuleLoader\.load nodejs/node#57419 * src: remove fast API for InternalModuleStat nodejs/node#58489 * src: simplify adding fast APIs to ExternalReferenceRegistry nodejs/node#58896 * chore: fixup patch indices * src: fix internalModuleStat v8 fast path nodejs/node#58054 * test: add tests to ensure that node.1 is kept in sync with cli.md nodejs/node#58878 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58942 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

chore: bump node to v22.18.0 (main) (#47937) * chore: bump node in DEPS to v22.18.0 * crypto: fix inclusion of OPENSSL_IS_BORINGSSL define nodejs/node#58845 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58960 * permission: propagate permission model flags on spawn nodejs/node#58853 * esm: syncify default path of ModuleLoader\.load nodejs/node#57419 * src: remove fast API for InternalModuleStat nodejs/node#58489 * src: simplify adding fast APIs to ExternalReferenceRegistry nodejs/node#58896 * chore: fixup patch indices * src: fix internalModuleStat v8 fast path nodejs/node#58054 * test: add tests to ensure that node.1 is kept in sync with cli.md nodejs/node#58878 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58942 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

Reverts: #56160 Fixes: #56159 Fixes: #58913 Refs: #58121 PR-URL: #58942 Backport-PR-URL: #58961 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

* chore: bump node in DEPS to v22.18.0 * crypto: fix inclusion of OPENSSL_IS_BORINGSSL define nodejs/node#58845 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58960 * permission: propagate permission model flags on spawn nodejs/node#58853 * esm: syncify default path of ModuleLoader\.load nodejs/node#57419 * src: remove fast API for InternalModuleStat nodejs/node#58489 * src: simplify adding fast APIs to ExternalReferenceRegistry nodejs/node#58896 * chore: fixup patch indices * src: fix internalModuleStat v8 fast path nodejs/node#58054 * test: add tests to ensure that node.1 is kept in sync with cli.md nodejs/node#58878 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58942 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

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

Comment thread doc/api/deprecations.md Outdated

panva commented Jul 3, 2025

View reviewed changes

Comment thread doc/api/deprecations.md Outdated

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 aduh95, jasnell and tniessen July 3, 2025 10:26

aduh95 reviewed Jul 3, 2025

View reviewed changes

Comment thread lib/internal/crypto/hash.js Outdated

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

panva added lts-watch-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

Comment thread lib/internal/crypto/hash.js Outdated

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

Comment thread lib/internal/crypto/hash.js Outdated

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

Comment thread lib/internal/crypto/hash.js Outdated

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.

joyeecheung removed the request-ci Add this label to start a Jenkins CI on a PR. label 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

panva mentioned this pull request Jul 5, 2025

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

Merged

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

Closed

panva added the backport-open-v20.x 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

aduh95 added backported-to-v22.x PRs backported to the v22.x-staging branch. and removed backport-open-v22.x Indicate that the PR has an open backport lts-watch-v22.x PRs that may need to be released in v22.x labels Jul 21, 2025

github-actions Bot mentioned this pull request Jul 28, 2025

2025-07-29, Version 22.18.0 'Jod' (LTS) #59256

Merged

codebytere added a commit to electron/electron that referenced this pull request Aug 4, 2025

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

14c8441

nodejs/node#58942

marco-ippolito mentioned this pull request Aug 20, 2025

2025-09-03, Version 20.19.5 'Iron' (LTS) #59551

Merged

marco-ippolito removed the lts-watch-v20.x label Nov 19, 2025

crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 by panva · Pull Request #58942 · nodejs/node · GitHub

Uh oh!

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
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
panva:xof-openssl34

panva commented Jul 3, 2025 •
edited
Loading

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!

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 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 by panva · Pull Request #58942 · nodejs/node · GitHub

Uh oh!

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#58942nodejs-github-bot merged 3 commits intonodejs:mainnodejs/node:mainfrom panva:xof-openssl34panva/node:xof-openssl34Copy head branch name to clipboard

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 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4#58942
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
panva:xof-openssl34

panva commented Jul 3, 2025 •
edited
Loading

codecov Bot commented Jul 3, 2025 •
edited
Loading