Skip to content

crypto: skip DH checks for recognized groups - #67071

Open
panva wants to merge 1 commit into
nodejs:mainfrom
panva:fix-dh-named-group-check
Open

panva wants to merge 1 commit into
nodejs:mainfrom
panva:fix-dh-named-group-check

Conversation

@panva

@panva panva commented Oct 27, 2026

Copy link
Copy Markdown
Member

Explicit DH parameters are recognized as named groups during OpenSSL provider import, but DHPointer::check() only skips primality checks for groups constructed by name. This makes createDiffieHellman() block for seconds on standard MODP groups.

Query the imported key's group name to restore the legacy DH_check() shortcut without changing key generation. Add a deterministic regression test with randomness disabled and cover explicit and padded parameters through the JavaScript API.

Fixes: #67067

Explicit DH parameters are recognized as named groups during OpenSSL
provider import, but DHPointer::check() only skips primality checks for
groups constructed by name. This makes createDiffieHellman() block for
seconds on standard MODP groups.

Query the imported key's group name to restore the legacy DH_check()
shortcut without changing key generation. Add a deterministic regression
test with randomness disabled and cover explicit and padded parameters
through the JavaScript API.

Fixes: nodejs#67067
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
@panva panva added the lts-watch-v24.x PRs that may need to be released in v24.x label Oct 27, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added dependencies PRs that add, update, or configure Node.js dependencies. needs-ci PRs that need a full CI run. labels Oct 27, 2026
@codecov

codecov Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.67%. Comparing base (75191c4) to head (d9f2aab).
⚠️ Report is 39 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #67071      +/-   ##
==========================================
- Coverage   92.97%   90.67%   -2.30%     
==========================================
  Files         428      797     +369     
  Lines      195284   278320   +83036     
  Branches    30205    53528   +23323     
==========================================
+ Hits       181560   252370   +70810     
- Misses      13397    16328    +2931     
- Partials      327     9622    +9295     

see 500 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 added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 27, 2026
@panva panva added the lts-watch-v26.x PRs that may need to be released in v26.x. label Oct 29, 2026
@panva
panva requested a review from jasnell October 30, 2026 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies PRs that add, update, or configure Node.js dependencies. lts-watch-v24.x PRs that may need to be released in v24.x lts-watch-v26.x PRs that may need to be released in v26.x. needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

crypto: createDiffieHellman(prime, generator) blocks for seconds with standard MODP primes (regression since 24.20.0 and 26.6.0)

2 participants