Skip to content

crypto: avoid repeated OpenSSL lookups - #67109

Open
anonrig wants to merge 4 commits into
nodejs:mainfrom
anonrig:crypto-perf
Open

anonrig wants to merge 4 commits into
nodejs:mainfrom
anonrig:crypto-perf

Conversation

@anonrig

@anonrig anonrig commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

The first three commits remove OpenSSL lookups that were repeated on every call. The fourth adds tests for the new caches.

  1. crypto: cache digests for synchronous operations. createHmac(), createSign(), createVerify() and the synchronous one-shot sign, verify, HKDF and PBKDF2 functions looked up their digest with Digest::FromName(). That returns OpenSSL's legacy EVP_MD, so every EVP_DigestInit_ex() and HMAC init had to fetch the provider implementation again. They now use the per-Environment cache that createHash() and crypto.hash() already use. Jobs on the thread pool keep using Digest::FromName(). With the cache, parallel subtle.digest() throughput dropped 5% to 7%. Those benchmarks are bound by the main thread, and workers that finish sooner have to be woken up for each new job more often.
  2. crypto: skip clearing empty OpenSSL error queues. ClearErrorOnReturn called ERR_clear_error() twice per operation. That function resets all 16 slots of the error queue, with two free() calls each, even when the queue is empty, which is almost always the case. It now checks with ERR_peek_error() first. In a small C benchmark against OpenSSL 4.1, a call on an empty queue went from about 45-50 ns to about 9 ns.
  3. crypto: reuse HMAC contexts in createHmac(). HMACCtxPointer uses EVP_MAC, so each context fetched HMAC, and keying it with a digest name made the provider fetch the digest again. DigestCache now keeps a context without a key for each digest, and createHmac() duplicates it.

The first version of this PR also ran small Web Crypto jobs on the calling thread. I've split that out. It'll come back as its own PR, with HTTP latency and event loop responsiveness measurements under crypto load.

Benchmarks

benchmark/compare.js on an Apple M5 Max running macOS 26.7.1. The base is 48cb57d, and the branch merges cleanly with current main.

Benchmark Result
crypto/create-hmac.js +4% to +15% for all 12 configs (***), geomean +10.9%
crypto/class-construction.js DiffieHellmanGroup +25%, Hmac +13%, Sign +11%, Verify +11%
crypto/hkdf.js with sync=1 +6% to +11% for 10 and 64 byte outputs

These were 30 runs each. Nothing else is slower after Holm-Bonferroni correction:

  • 30 runs: webcrypto-digest.js (geomean -0.9%), webcrypto-hmac.js and hmac-throughput.js.
  • 20 runs: oneshot-sign.js, oneshot-verify.js, webcrypto-sign.js, webcrypto-verify.js and rsa-encrypt-decrypt-throughput.js.
  • 8 runs: every other file in benchmark/crypto. Two configs did show up after correction there: RSA public-key DER import in keyobject-serialization.js (-2.9%) and ecdh-compute-secret.js with scenario=first-after-generate (-1.5%). Neither reproduced as significant in 30 interleaved runs (-2.4% with z=1.35 and -3.5% with z=1.43, while A/A runs were within 1%).

createHash() doesn't run any changed code. Interleaved runs of the createHash configs in webcrypto-digest.js show about -2% (A/A: +0.2%), which I haven't been able to attribute. crypto/create-hash.js and the Hash case of crypto/class-construction.js show no significant change.

Tests

test: cover crypto digest and HMAC caches adds two tests:

  • test-crypto-hmac-independent-contexts.js creates several createHmac() instances with the same digest and different keys, interleaves their updates, and checks each result against the RFC 4231 vectors for SHA-256 and SHA-512.
  • test-crypto-digest-cache-fips.js warms the caches with MD5 through every API that the first commit changed, starts a SHA-256 HMAC, and then turns FIPS mode on. New MD5 operations must fail, and the HMAC that was already in progress must still produce the right result. After FIPS mode is turned off, MD5 results must match the ones from before. Without a FIPS provider, createHmac() is the only one of these operations that would reveal a stale cache entry, because the other ones also need a signature or KDF fetch that fails in FIPS mode. I checked that the test fails when the cache is never invalidated.

The existing crypto, webcrypto, WPT webcrypto, async-hooks, TLS and HTTPS tests pass locally, and each commit builds on its own.

This was developed with an AI coding agent (Grok), which also drafted this description.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

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

@nodejs-github-bot 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. dependencies PRs that add, update, or configure Node.js dependencies. needs-ci PRs that need a full CI run. labels Oct 29, 2026
@anonrig
anonrig requested review from jasnell and panva and removed request for panva October 29, 2026 15:10
@anonrig
anonrig marked this pull request as ready for review October 29, 2026 15:11
@anonrig anonrig added the commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. label Oct 29, 2026
@anonrig
anonrig requested review from mcollina and panva October 29, 2026 15:12
@panva

panva commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

Please split the inlining from general improvements.

I explored this in #63595, including operation-specific thresholds and a guard that sent fanout to the threadpool. The reason I stopped was HTTP server responsiveness. After @mcollina raised that concern, I measured it and found that accounting for responsiveness reduced the viable candidates to non-RSA key generation and symmetric operations, with thresholds below 4 KiB, sometimes 8 KiB, rather than 64 KiB. At that point I didn't think the remaining benefit justified the churn and maintenance burden. Closing comment.

The narrower scope and 1 KiB budget here are relevant differences, but crypto throughput alone doesn't establish that the responsiveness concern is resolved. Deferring promise settlement to a microtask still allows serial await loops to keep doing crypto without yielding to I/O, and this implementation also runs every eligible job in a fanout inline. The follow-up should measure HTTP latency and event-loop responsiveness under crypto load, alongside throughput.

There's also a specification question. The current digest() steps, for example, explicitly say "Return promise and perform the remaining steps in parallel", then queue a global task on the crypto task source to settle it. Returning a pending promise and settling from a microtask doesn't preserve that task scheduling. Reconsidering these requirements is itself an open spec discussion.

A separate synchronous WebCrypto API is being discussed as well.

@codecov

codecov Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.67%. Comparing base (a066ccf) to head (4a4eee7).
⚠️ Report is 58 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #67109      +/-   ##
==========================================
+ Coverage   90.64%   90.67%   +0.02%     
==========================================
  Files         797      797              
  Lines      278797   278809      +12     
  Branches    53627    53625       -2     
==========================================
+ Hits       252717   252799      +82     
+ Misses      16413    16378      -35     
+ Partials     9667     9632      -35     
Files with missing lines Coverage Δ
src/crypto/crypto_hash.cc 79.77% <100.00%> (+0.88%) ⬆️
src/crypto/crypto_hash.h 20.00% <ø> (ø)
src/crypto/crypto_hkdf.cc 66.66% <100.00%> (ø)
src/crypto/crypto_hmac.cc 70.55% <100.00%> (+1.84%) ⬆️
src/crypto/crypto_pbkdf2.cc 67.12% <100.00%> (ø)
src/crypto/crypto_sig.cc 69.18% <100.00%> (+0.55%) ⬆️

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

@anonrig anonrig changed the title crypto: run small Web Crypto jobs inline and cache OpenSSL lookups crypto: avoid repeated OpenSSL lookups Oct 29, 2026
@panva

panva commented Oct 29, 2026

Copy link
Copy Markdown
Member

Please add regression tests covering:

  • Two live HMAC instances using the same digest with different keys and interleaved updates, to verify their contexts remain independent.
  • Warmed digest/HMAC caches across FIPS enable/disable transitions, checking that new operations follow the current policy and an already initialized HMAC survives cache invalidation.

@anonrig anonrig removed the commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. label Oct 29, 2026
createHmac(), createSign(), createVerify() and the synchronous
one-shot sign, verify, HKDF and PBKDF2 functions looked up their
digest with Digest::FromName(). That returns OpenSSL's legacy EVP_MD,
so every EVP_DigestInit_ex() and HMAC init had to fetch the provider
implementation again through the method store.

Look these digests up through the per-Environment cache that
createHash() and crypto.hash() already use. Jobs on the thread pool
keep using Digest::FromName(): with the cache, Web Crypto digest and
HMAC throughput went down, because workers that finish sooner have to
be woken up for each new job more often.

Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
Assisted-by: Grok
ClearErrorOnReturn calls ERR_clear_error() when it is created and
again when it goes out of scope. That function resets all 16 slots of
the thread's error queue, including two free() calls per slot, even
when the queue is empty, which is almost always the case. Check for a
queued error first, which costs a fraction of that.

ClearErrorOnReturn guards most crypto operations, including every
CryptoJob, so this applies to all of them.

Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
Assisted-by: Grok
Every HMAC context was created with EVP_MAC_fetch() and keyed with the
digest passed by name, so the provider fetched the digest again on
each init. Both lookups go through OpenSSL's method store.

Keep an HMAC context without a key for each digest in the
per-Environment digest cache, and duplicate it in createHmac(). A
duplicated context already has its digest, so keying it with the same
digest no longer fetches anything.

Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
Assisted-by: Grok
Check that HMAC instances duplicated from the same cached context stay
independent when their updates are interleaved, and that warmed digest
and HMAC caches follow FIPS mode changes without breaking an HMAC that
is already in progress.

Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
Assisted-by: Grok
@panva panva added the commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. label Oct 29, 2026
@panva panva added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Oct 29, 2026
@github-actions github-actions Bot removed 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 29, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@panva panva added the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Oct 29, 2026
@github-actions github-actions Bot removed the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Oct 29, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@panva panva added the commit-queue PRs queued for automated landing through the Commit Queue. label Nov 2, 2026
@nodejs-github-bot nodejs-github-bot added the lacks-second-approval Commit Queue PRs awaiting a second collaborator approval or completion of the required wait. label Nov 2, 2026
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. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue PRs queued for automated landing through the Commit Queue. commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. crypto Issues and PRs related to the crypto subsystem. dependencies PRs that add, update, or configure Node.js dependencies. lacks-second-approval Commit Queue PRs awaiting a second collaborator approval or completion of the required wait. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants