Skip to content

crypto: follow up on OpenSSL backend split#64547

Open
panva wants to merge 5 commits into
nodejs:mainfrom
panva:followup-crypto-split
Open

crypto: follow up on OpenSSL backend split#64547
panva wants to merge 5 commits into
nodejs:mainfrom
panva:followup-crypto-split

Conversation

@panva

@panva panva commented Jul 16, 2026

Copy link
Copy Markdown
Member

This follows up on #64211, which separated the OpenSSL >= 3, BoringSSL, and legacy OpenSSL backend paths.

It addresses issues identified during a subsequent review of that split (done with GPT-5.6 Sol). The changes restore unintentionally altered legacy behavior, improve handling of provider-backed keys and configure probe failures, and ensure temporary private key material is cleansed appropriately.

Refs: #64211

Details

build: handle OpenSSL macro probe failures

Return the existing unknown-version sentinel when the compiler cannot extract OpenSSL macros. This avoids slicing a missing legacy version number and preserves the pre-refactor configure failure behaviour.


crypto: retain legacy DH validation

Limit the g >= p rejection needed by the provider backend to OpenSSL 3. Preserve the legacy DiffieHellman object and expose the validation result through verifyError as before.


crypto: handle incomplete RSA private keys

Treat missing private RSA parameters as an export failure instead of passing null BIGNUM pointers to the JWK encoder. Also stop constructing a usable RSA view when reading an optional parameter itself fails.


crypto: cleanse provider private key copies

Clear provider-exported RSA, EC, and DH private BIGNUMs before freeing them. Also cleanse OSSL_PARAM builder copies and the plaintext DER intermediate used for encrypted traditional PEM output.


crypto: preserve RSA-PSS legacy pubkey DER

Recreate the historical rsaEncryption AlgorithmIdentifier used by X509Certificate legacy pubkey output for restricted and unrestricted RSA-PSS keys without relying on deprecated RSA APIs.

panva added 5 commits July 17, 2026 00:25
Return the existing unknown-version sentinel when the compiler cannot
extract OpenSSL macros. This avoids slicing a missing legacy version
number and preserves the pre-refactor configure failure behaviour.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Limit the g >= p rejection needed by the provider backend to OpenSSL 3.
Preserve the legacy DiffieHellman object and expose the validation
result through verifyError as before.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Treat missing private RSA parameters as an export failure instead of
passing null BIGNUM pointers to the JWK encoder. Also stop constructing
a usable RSA view when reading an optional parameter itself fails.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Clear provider-exported RSA, EC, and DH private BIGNUMs before freeing
them. Also cleanse OSSL_PARAM builder copies and the plaintext DER
intermediate used for encrypted traditional PEM output.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Recreate the historical rsaEncryption AlgorithmIdentifier used by
X509Certificate legacy pubkey output for restricted and unrestricted
RSA-PSS keys without relying on deprecated RSA APIs.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
@panva panva added crypto Issues and PRs related to the crypto subsystem. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Jul 16, 2026
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.24%. Comparing base (a27fe21) to head (ecd623a).

Files with missing lines Patch % Lines
src/crypto/crypto_rsa.cc 66.66% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64547      +/-   ##
==========================================
- Coverage   90.24%   90.24%   -0.01%     
==========================================
  Files         739      739              
  Lines      241698   241704       +6     
  Branches    45556    45546      -10     
==========================================
- Hits       218129   218122       -7     
- Misses      15103    15106       +3     
- Partials     8466     8476      +10     
Files with missing lines Coverage Δ
src/crypto/crypto_dh.cc 67.16% <ø> (ø)
src/crypto/crypto_rsa.cc 64.11% <66.66%> (+0.05%) ⬆️

... 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 nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 17, 2026
@panva

panva commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

cc @nodejs/crypto

@panva
panva requested review from jasnell and richardlau July 17, 2026 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. crypto Issues and PRs related to the crypto subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants