Skip to content

[v26.x backport] crypto: fix various edge cases - #66391

Closed
panva wants to merge 2 commits into
nodejs:v26.x-stagingfrom
panva:backport-66237-to-v26.x
Closed

panva wants to merge 2 commits into
nodejs:v26.x-stagingfrom
panva:backport-66237-to-v26.x

Conversation

@panva

@panva panva commented Sep 29, 2026

Copy link
Copy Markdown
Member

Backports the rest of #66237

Convert algorithm dictionaries on the original receiver, with name
read once. Validate normalized parameters in their operation steps
after the method-level key checks and generation usage checks.

This also makes Argon2 validation use converted parallelism and keeps
later dictionary conversion errors ahead of semantic parameter errors.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
PR-URL: nodejs#66237
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Aviv Keller <me@aviv.sh>
Require cSHAKE and KMAC output lengths and KMAC key lengths to be
multiples of 8 bits. KMAC keys must be at least 32 bits. Share these
restrictions between operations and supports.

Use OpenSSL's KMAC provider for all supported inputs and its cSHAKE
implementation for non-empty function names or customization strings.
Keep using SHAKE when both cSHAKE parameters are empty. Remove the
custom Keccak framing, partial-bit handling, and short-key fallback.

Document the OpenSSL 4.0 requirement for non-empty cSHAKE parameters and
reject customization strings containing null bytes. Keep the documented
512-byte customization limit and let backend failures reach callers.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
PR-URL: nodejs#66237
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Aviv Keller <me@aviv.sh>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. v26.x Issues that can be reproduced on v26.x or PRs targeting the v26.x-staging branch. labels Sep 29, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@panva
panva requested a review from aduh95 September 29, 2026 10:05
@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.10526% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.66%. Comparing base (ca26282) to head (b81beca).
⚠️ Report is 1038 commits behind head on v26.x-staging.

Files with missing lines Patch % Lines
lib/internal/crypto/webidl.js 92.40% 5 Missing and 1 partial ⚠️
lib/internal/crypto/hash.js 50.00% 4 Missing ⚠️
src/crypto/crypto_kmac.cc 71.42% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##           v26.x-staging   #66391      +/-   ##
=================================================
+ Coverage          90.24%   90.66%   +0.42%     
=================================================
  Files                729      775      +46     
  Lines             242760   271437   +28677     
  Branches           46044    51940    +5896     
=================================================
+ Hits              219073   246096   +27023     
- Misses             15133    16232    +1099     
- Partials            8554     9109     +555     
Files with missing lines Coverage Δ
lib/internal/crypto/aes.js 94.59% <100.00%> (+0.84%) ⬆️
lib/internal/crypto/argon2.js 98.71% <100.00%> (+0.01%) ⬆️
lib/internal/crypto/cfrg.js 95.75% <100.00%> (-0.14%) ⬇️
lib/internal/crypto/chacha20_poly1305.js 98.26% <100.00%> (+0.03%) ⬆️
lib/internal/crypto/diffiehellman.js 97.94% <100.00%> (-0.01%) ⬇️
lib/internal/crypto/ec.js 96.94% <100.00%> (+0.04%) ⬆️
lib/internal/crypto/hkdf.js 100.00% <100.00%> (ø)
lib/internal/crypto/mac.js 99.03% <100.00%> (+0.01%) ⬆️
lib/internal/crypto/ml_dsa.js 97.33% <100.00%> (-0.05%) ⬇️
lib/internal/crypto/rsa.js 94.93% <100.00%> (+0.03%) ⬆️
... and 8 more

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

@panva panva changed the title [v26.x backport] edge case fixes [v26.x backport] crypto: edge case fixes Sep 29, 2026
@panva panva changed the title [v26.x backport] crypto: edge case fixes [v26.x backport] crypto: fix various edge cases Sep 29, 2026
@panva panva added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Sep 30, 2026
aduh95 pushed a commit that referenced this pull request Oct 3, 2026
Convert algorithm dictionaries on the original receiver, with name
read once. Validate normalized parameters in their operation steps
after the method-level key checks and generation usage checks.

This also makes Argon2 validation use converted parallelism and keeps
later dictionary conversion errors ahead of semantic parameter errors.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
PR-URL: #66237
Backport-PR-URL: #66391
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
aduh95 pushed a commit that referenced this pull request Oct 3, 2026
Require cSHAKE and KMAC output lengths and KMAC key lengths to be
multiples of 8 bits. KMAC keys must be at least 32 bits. Share these
restrictions between operations and supports.

Use OpenSSL's KMAC provider for all supported inputs and its cSHAKE
implementation for non-empty function names or customization strings.
Keep using SHAKE when both cSHAKE parameters are empty. Remove the
custom Keccak framing, partial-bit handling, and short-key fallback.

Document the OpenSSL 4.0 requirement for non-empty cSHAKE parameters and
reject customization strings containing null bytes. Keep the documented
512-byte customization limit and let backend failures reach callers.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
PR-URL: #66237
Backport-PR-URL: #66391
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
@aduh95

aduh95 commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Landed in 6c924ee...32ba6d5

@aduh95 aduh95 closed this Oct 3, 2026
@panva
panva deleted the backport-66237-to-v26.x branch October 3, 2026 13:46
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. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. v26.x Issues that can be reproduced on v26.x or PRs targeting the v26.x-staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants