Skip to content

crypto: harden WebCrypto jobs and promise handling#63363

Open
panva wants to merge 3 commits into
nodejs:mainfrom
panva:webcrypto-job-mode
Open

crypto: harden WebCrypto jobs and promise handling#63363
panva wants to merge 3 commits into
nodejs:mainfrom
panva:webcrypto-job-mode

Conversation

@panva
Copy link
Copy Markdown
Member

@panva panva commented May 16, 2026

After quite the BoringSSL detour I'm coming back to Web Cryptography hardening related to #59699 (comment) and #59699 (comment) in hopes that we can eventually mark #59699 as resolved.


crypto: add WebCrypto CryptoJob mode

Add a WebCrypto-specific CryptoJob mode that returns a promise from run() and resolves it when native work is finished.

Encode job output directly as Web Crypto values, including CryptoKey instances and CryptoKeyPair dictionaries. Convert operation-specific setup failures from AdditionalConfig into OperationError rejections.


crypto: remove async from WebCrypto methods

Remove async function wrappers from SubtleCrypto methods while keeping their public promise-returning behaviour.

Route method entry points through a shared helper that converts synchronous validation errors into rejected promises. Let the internal implementations return native job promises directly.


crypto: pass CryptoKey handles to KDF jobs

Pass CryptoKey handles directly into KDF jobs instead of exporting secret bytes in lib.

Normalize HKDF, PBKDF2, and Argon2 around the same job construction pattern so WebCrypto derivation paths avoid extra key material copies and keep operation failures in native job handling.

panva added 3 commits May 16, 2026 10:57
Add a WebCrypto-specific CryptoJob mode that returns a promise from
run() and resolves it when native work is finished.

Encode job output directly as Web Crypto values, including CryptoKey
instances and CryptoKeyPair dictionaries. Convert operation-specific
setup failures from AdditionalConfig into OperationError rejections.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Remove async function wrappers from SubtleCrypto methods while keeping
their public promise-returning behaviour.

Route method entry points through a shared helper that converts
synchronous validation errors into rejected promises. Let the internal
implementations return native job promises directly.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Pass CryptoKey handles directly into KDF jobs instead of exporting
secret bytes in lib.

Normalize HKDF, PBKDF2, and Argon2 around the same job construction
pattern so WebCrypto derivation paths avoid extra key material copies
and keep operation failures in native job handling.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
@panva panva requested review from addaleax, anonrig and jasnell May 16, 2026 09:16
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto

@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 May 16, 2026
@panva panva added crypto Issues and PRs related to the crypto subsystem. security Issues and PRs related to security. webcrypto commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. dont-land-on-v25.x PRs that should not land on the v25.x-staging branch and should not be released in v25.x. labels May 16, 2026
@panva panva requested a review from tniessen May 16, 2026 09:49
@panva
Copy link
Copy Markdown
Member Author

panva commented May 16, 2026

First benchmark: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1847/

                                                                                                confidence improvement accuracy (*)    (**)   (***)
crypto/webcrypto-digest.js n=100000 method='SHA-1' data=10 sync='subtle'                               ***     51.49 %       ±1.88%  ±2.51%  ±3.27%
crypto/webcrypto-digest.js n=100000 method='SHA-1' data=100 sync='subtle'                              ***     46.56 %       ±1.65%  ±2.20%  ±2.87%
crypto/webcrypto-digest.js n=100000 method='SHA-1' data=20 sync='subtle'                               ***     50.30 %       ±1.62%  ±2.16%  ±2.81%
crypto/webcrypto-digest.js n=100000 method='SHA-1' data=50 sync='subtle'                               ***     47.82 %       ±1.97%  ±2.64%  ±3.46%
crypto/webcrypto-digest.js n=100000 method='SHA-256' data=10 sync='subtle'                             ***     53.30 %       ±1.74%  ±2.31%  ±3.01%
crypto/webcrypto-digest.js n=100000 method='SHA-256' data=100 sync='subtle'                            ***     46.80 %       ±1.43%  ±1.90%  ±2.48%
crypto/webcrypto-digest.js n=100000 method='SHA-256' data=20 sync='subtle'                             ***     50.61 %       ±1.51%  ±2.01%  ±2.62%
crypto/webcrypto-digest.js n=100000 method='SHA-256' data=50 sync='subtle'                             ***     48.72 %       ±1.67%  ±2.23%  ±2.92%
crypto/webcrypto-digest.js n=100000 method='SHA-384' data=10 sync='subtle'                             ***     50.38 %       ±1.63%  ±2.17%  ±2.83%
crypto/webcrypto-digest.js n=100000 method='SHA-384' data=100 sync='subtle'                            ***     45.03 %       ±1.85%  ±2.46%  ±3.21%
crypto/webcrypto-digest.js n=100000 method='SHA-384' data=20 sync='subtle'                             ***     51.67 %       ±1.48%  ±1.97%  ±2.56%
crypto/webcrypto-digest.js n=100000 method='SHA-384' data=50 sync='subtle'                             ***     47.72 %       ±1.76%  ±2.35%  ±3.07%
crypto/webcrypto-digest.js n=100000 method='SHA-512' data=10 sync='subtle'                             ***     50.89 %       ±1.37%  ±1.82%  ±2.38%
crypto/webcrypto-digest.js n=100000 method='SHA-512' data=100 sync='subtle'                            ***     46.94 %       ±1.85%  ±2.46%  ±3.20%
crypto/webcrypto-digest.js n=100000 method='SHA-512' data=20 sync='subtle'                             ***     48.71 %       ±1.61%  ±2.15%  ±2.80%
crypto/webcrypto-digest.js n=100000 method='SHA-512' data=50 sync='subtle'                             ***     48.82 %       ±1.57%  ±2.09%  ±2.73%
crypto/webcrypto-sign.js n=1000 keyReuse='shared' mode='parallel' keyType='ec'                         ***     14.77 %       ±4.90%  ±6.54%  ±8.53%
crypto/webcrypto-sign.js n=1000 keyReuse='shared' mode='parallel' keyType='ed25519'                    ***     -1.02 %       ±0.51%  ±0.68%  ±0.89%
crypto/webcrypto-sign.js n=1000 keyReuse='shared' mode='parallel' keyType='ml-dsa-44'                           0.10 %       ±1.23%  ±1.64%  ±2.13%
crypto/webcrypto-sign.js n=1000 keyReuse='shared' mode='parallel' keyType='rsa-pss'                            -0.09 %       ±0.21%  ±0.28%  ±0.37%
crypto/webcrypto-sign.js n=1000 keyReuse='shared' mode='parallel' keyType='rsassa-pkcs1-v1_5'                   0.04 %       ±0.38%  ±0.51%  ±0.66%
crypto/webcrypto-sign.js n=1000 keyReuse='shared' mode='serial' keyType='ec'                                    0.45 %       ±2.88%  ±3.85%  ±5.04%
crypto/webcrypto-sign.js n=1000 keyReuse='shared' mode='serial' keyType='ed25519'                               0.38 %       ±6.06%  ±8.07% ±10.53%
crypto/webcrypto-sign.js n=1000 keyReuse='shared' mode='serial' keyType='ml-dsa-44'                             7.75 %      ±26.04% ±34.67% ±45.15%
crypto/webcrypto-sign.js n=1000 keyReuse='shared' mode='serial' keyType='rsa-pss'                        *    -24.54 %      ±21.84% ±29.10% ±37.97%
crypto/webcrypto-sign.js n=1000 keyReuse='shared' mode='serial' keyType='rsassa-pkcs1-v1_5'              *    -23.13 %      ±18.31% ±24.41% ±31.88%
crypto/webcrypto-sign.js n=1000 keyReuse='unique' mode='parallel' keyType='ec'                         ***     12.74 %       ±5.34%  ±7.13%  ±9.34%
crypto/webcrypto-sign.js n=1000 keyReuse='unique' mode='parallel' keyType='ed25519'                            -0.87 %       ±1.98%  ±2.65%  ±3.48%
crypto/webcrypto-sign.js n=1000 keyReuse='unique' mode='parallel' keyType='ml-dsa-44'                           0.16 %       ±1.03%  ±1.37%  ±1.78%
crypto/webcrypto-sign.js n=1000 keyReuse='unique' mode='parallel' keyType='rsa-pss'                            -0.02 %       ±0.15%  ±0.21%  ±0.27%
crypto/webcrypto-sign.js n=1000 keyReuse='unique' mode='parallel' keyType='rsassa-pkcs1-v1_5'                   0.07 %       ±0.09%  ±0.11%  ±0.15%
crypto/webcrypto-verify.js n=1000 keyReuse='shared' mode='parallel' keyType='ec'                               -1.81 %       ±2.17%  ±2.92%  ±3.87%
crypto/webcrypto-verify.js n=1000 keyReuse='shared' mode='parallel' keyType='ed25519'                          -0.23 %       ±0.46%  ±0.61%  ±0.79%
crypto/webcrypto-verify.js n=1000 keyReuse='shared' mode='parallel' keyType='ml-dsa-44'                         0.42 %       ±2.50%  ±3.33%  ±4.35%
crypto/webcrypto-verify.js n=1000 keyReuse='shared' mode='parallel' keyType='rsa-pss'                  ***     50.85 %       ±2.68%  ±3.57%  ±4.64%
crypto/webcrypto-verify.js n=1000 keyReuse='shared' mode='parallel' keyType='rsassa-pkcs1-v1_5'        ***     45.80 %       ±2.30%  ±3.07%  ±4.03%
crypto/webcrypto-verify.js n=1000 keyReuse='shared' mode='serial' keyType='ec'                                  7.60 %      ±24.25% ±32.32% ±42.19%
crypto/webcrypto-verify.js n=1000 keyReuse='shared' mode='serial' keyType='ed25519'                           -15.55 %      ±17.62% ±23.51% ±30.74%
crypto/webcrypto-verify.js n=1000 keyReuse='shared' mode='serial' keyType='ml-dsa-44'                           0.43 %      ±19.65% ±26.15% ±34.03%
crypto/webcrypto-verify.js n=1000 keyReuse='shared' mode='serial' keyType='rsa-pss'                            -0.63 %       ±4.18%  ±5.57%  ±7.25%
crypto/webcrypto-verify.js n=1000 keyReuse='shared' mode='serial' keyType='rsassa-pkcs1-v1_5'                  -4.76 %       ±6.20%  ±8.30% ±10.90%
crypto/webcrypto-verify.js n=1000 keyReuse='unique' mode='parallel' keyType='ec'                                0.65 %       ±1.95%  ±2.60%  ±3.39%
crypto/webcrypto-verify.js n=1000 keyReuse='unique' mode='parallel' keyType='ed25519'                           0.08 %       ±0.42%  ±0.55%  ±0.72%
crypto/webcrypto-verify.js n=1000 keyReuse='unique' mode='parallel' keyType='ml-dsa-44'                         0.64 %       ±2.38%  ±3.18%  ±4.16%
crypto/webcrypto-verify.js n=1000 keyReuse='unique' mode='parallel' keyType='rsa-pss'                  ***     12.67 %       ±1.62%  ±2.16%  ±2.81%
crypto/webcrypto-verify.js n=1000 keyReuse='unique' mode='parallel' keyType='rsassa-pkcs1-v1_5'        ***      6.98 %       ±1.40%  ±1.86%  ±2.42%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 62 comparisons, you can thus
expect the following amount of false-positive results:
  3.10 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.62 false positives, when considering a   1% risk acceptance (**, ***),
  0.06 false positives, when considering a 0.1% risk acceptance (***)

@codecov
Copy link
Copy Markdown

codecov Bot commented May 16, 2026

Codecov Report

❌ Patch coverage is 85.31469% with 126 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.04%. Comparing base (2d6cbea) to head (9bdd823).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/crypto/crypto_util.h 49.29% 22 Missing and 14 partials ⚠️
src/crypto/crypto_keygen.h 78.87% 2 Missing and 13 partials ⚠️
src/crypto/crypto_keys.cc 54.54% 6 Missing and 4 partials ⚠️
src/crypto/crypto_argon2.cc 60.00% 5 Missing and 3 partials ⚠️
src/crypto/crypto_hkdf.cc 65.21% 5 Missing and 3 partials ⚠️
src/crypto/crypto_pbkdf2.cc 57.89% 2 Missing and 6 partials ⚠️
src/crypto/crypto_util.cc 78.94% 2 Missing and 6 partials ⚠️
src/crypto/crypto_turboshake.cc 0.00% 2 Missing and 3 partials ⚠️
src/crypto/crypto_aes.cc 20.00% 2 Missing and 2 partials ⚠️
src/crypto/crypto_kem.cc 75.00% 2 Missing and 2 partials ⚠️
... and 9 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63363      +/-   ##
==========================================
- Coverage   90.06%   90.04%   -0.02%     
==========================================
  Files         714      714              
  Lines      225502   225745     +243     
  Branches    42628    42715      +87     
==========================================
+ Hits       203089   203272     +183     
- Misses      14195    14208      +13     
- Partials     8218     8265      +47     
Files with missing lines Coverage Δ
lib/internal/crypto/aes.js 92.61% <100.00%> (+0.11%) ⬆️
lib/internal/crypto/argon2.js 98.70% <100.00%> (+2.75%) ⬆️
lib/internal/crypto/cfrg.js 94.67% <100.00%> (-0.24%) ⬇️
lib/internal/crypto/chacha20_poly1305.js 98.47% <100.00%> (+0.04%) ⬆️
lib/internal/crypto/diffiehellman.js 97.76% <100.00%> (+0.02%) ⬆️
lib/internal/crypto/ec.js 95.54% <100.00%> (-0.09%) ⬇️
lib/internal/crypto/hash.js 99.00% <100.00%> (ø)
lib/internal/crypto/hkdf.js 98.84% <100.00%> (+2.77%) ⬆️
lib/internal/crypto/mac.js 99.02% <100.00%> (+0.03%) ⬆️
lib/internal/crypto/ml_dsa.js 97.46% <100.00%> (-0.10%) ⬇️
... and 27 more

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

Renegade334

This comment was marked as off-topic.

@panva
Copy link
Copy Markdown
Member Author

panva commented May 16, 2026

@Renegade334 you are incorrect in your assessments. The public methods still reject, not throw. Nothing is changed on the surface as evidenced by the lack of test changes and pre-existing passing comprehensive test cov including WPTs.

@Renegade334
Copy link
Copy Markdown
Member

Renegade334 commented May 16, 2026

Oh good, code review hid webcrypto.js as a "large diff" 🤦‍♂️ I see the new machinery, apologies.

(FWIW PromiseResolve is still exposed to the same .constructor mutability issue as before, but there's not a lot we can do about that besides wait for a spec change.)

@panva
Copy link
Copy Markdown
Member Author

panva commented May 16, 2026

@Renegade334 i can do this across the board

// WebCrypto methods return promises, including for synchronous validation
// failures. Keep that conversion in one place so method bodies stay readable.
function callSubtleCryptoMethod(fn, receiver, args) {
  try {
    const result = ReflectApply(fn, receiver, args);
    // PromiseResolve(promise) reads promise.constructor. Avoid re-wrapping
    // native promises so Promise.prototype.constructor pollution is ignored.
    return isPromise(result) ? result : PromiseResolve(result);
  } catch (err) {
    return PromiseReject(err);
  }
}

That way PromiseResolve(result) is only called on what isn't a native job, which i now realize includes the key export jobs for which i can do a follow up to re-introduce a simpler native key export job now that everything goes through handles already.

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. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. dont-land-on-v25.x PRs that should not land on the v25.x-staging branch and should not be released in v25.x. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. security Issues and PRs related to security. webcrypto

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants