crypto: harden WebCrypto jobs and promise handling#63363
Conversation
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>
|
Review requested:
|
|
First benchmark: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1847/ |
Codecov Report❌ Patch coverage is 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
🚀 New features to boost your workflow:
|
|
@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. |
|
Oh good, code review hid webcrypto.js as a "large diff" 🤦♂️ I see the new machinery, apologies. (FWIW |
|
@Renegade334 i can do this across the board 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. |
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.