src: decouple KeyObject and CryptoKey and move CryptoKey to src#62924
Open
panva wants to merge 13 commits intonodejs:mainfrom
Open
src: decouple KeyObject and CryptoKey and move CryptoKey to src#62924panva wants to merge 13 commits intonodejs:mainfrom
panva wants to merge 13 commits intonodejs:mainfrom
Conversation
panva
added
crypto
Collaborator
|
Review requested:
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #62924 +/- ##
==========================================
- Coverage 89.63% 89.62% -0.01%
==========================================
Files 706 706
Lines 219219 219432 +213
Branches 42004 42088 +84
==========================================
+ Hits 196499 196671 +172
- Misses 14622 14632 +10
- Partials 8098 8129 +31
🚀 New features to boost your workflow:
|
addaleax
reviewed
Apr 24, 2026
Member
Author
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Introduces a C++ `NativeCryptoKey` class that holds the real CryptoKey slots (`handle_data_`, `algorithm_`, `usages_`, `extractable_`) and provides structured-clone and worker-transfer support through a dedicated `CryptoKeyTransferData`. New bindings `createCryptoKeyClass`, `getCryptoKeyHandle`, and `getCryptoKeySlots` expose the class and accessors to JS; a brand-tag class-ID pointer rejects spoofed receivers on the accessors. Signed-off-by: Filip Skokan <panva.ip@gmail.com>
JS-side `CryptoKey` now extends the native `NativeCryptoKey` created via
`createCryptoKeyClass`. `InternalCryptoKey` caches the
`[type, extractable, algorithm, usages, handle]` slot tuple in a
`#slots` private field on first access; the public getters and
`isCryptoKey` route through dedicated `getCryptoKey{Type,Extractable,
Algorithm,Usages,Handle}` helpers re-exported from this module. Symbol
property storage (`kKeyObject`, `kAlgorithm`, `kExtractable`,
`kKeyUsages`, `kCachedAlgorithm`, `kCachedKeyUsages`) is gone;
`kKeyObject` is dropped from `internal/crypto/util` exports.
`deepStrictEqual` on CryptoKey switches to the new accessors plus
`handle.equals()` (instead of structural compare on wrapper objects).
An ESLint rule forbids destructuring `getCryptoKeyHandle` from
`internalBinding('crypto')`; it must come from `internal/crypto/keys`
so the brand-check path is used.
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Every Web Crypto algorithm module drops `key[kKeyObject][kHandle]`
symbol access in favor of `getCryptoKey*` accessors and stores the raw
`KeyObjectHandle` directly (no more `SecretKeyObject`/`PublicKeyObject`/
`PrivateKeyObject` wrappers on the CryptoKey slot). Key generation
moves off the promisified `generateKey`/`generateKeyPair` paths onto
ones that return `KeyObjectHandle` (i.e. not wrapped in KeyObject):
- `SecretKeyGenJob` for AES, ChaCha20-Poly1305, HMAC/CMAC (aes.js,
chacha20_poly1305.js, mac.js).
- `EcKeyPairGenJob` for ECDSA, ECDH (ec.js).
- `NidKeyPairGenJob` for Ed25519/Ed448/X25519/X448, ML-DSA-{44,65,87},
ML-KEM-{512,768,1024} (cfrg.js, ml_dsa.js, ml_kem.js).
- `RsaKeyPairGenJob` for RSASSA-PKCS1-v1_5, RSA-PSS, RSA-OAEP (rsa.js).
`webcrypto_util.js` switches its import helpers (`importDerKey`,
`importJwkKey`, `importRawKey`) to return raw `KeyObjectHandle`s and
adds `importSecretKey` / `importJwkSecretKey` helpers for symmetric
imports. `webcrypto.js` follows the same pattern; its `getPublicKey`
builds a temporary wrapper and carries a TODO for future refactor.
`hkdf.js` switches from `promisify(hkdf)` to
`jobPromise(() => new HKDFJob(...))` directly. `diffiehellman.js`,
`argon2.js`, and `pbkdf2.js` are accessor-only migrations (the
`TODO(panva)` notes on argon2/pbkdf2 are orthogonal and left for a later
cleanup).
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Loosens two `AdditionalConfig` precondition checks so the new Web Crypto
keygen jobs added earlier (RsaKeyPairGenJob, EcKeyPairGenJob) can reuse
the shared traits without threading unused encoding args through the job
constructor.
- `RsaKeyGenTraits::AdditionalConfig` now CHECKs RSA key
type-dependant argument count accounting for being able to skip
unused parameters.
- `EcKeyGenTraits::AdditionalConfig` now defaults `param_encoding` to
`OPENSSL_EC_NAMED_CURVE`, this is not observable in existing
crypto.generateKeyPair(Sync) as its dispatch already applies the
same default. This is just so that a stray OPENSSL_EC_NAMED_CURVE
isn't needed in ec.js
Pure precondition relaxation — no new code paths. Existing
`generateKeyPair` callers still pass the same args and hit the same
branches.
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Enforces a 1024-byte maximum on `HkdfParams.info` at the WebIDL layer using refactored `validateMaxBufferLength`. Its error message is also fixed. Oversized `info` was already rejected via a different code path. It just relocates the rejection earlier into the WebIDL conversion step so the failure reproduces the OpenSSL's limitation early. Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Tightens `validateJwk` to require the per-`kty` string members up front
(before switching to C++), short-circuiting with the same
`Invalid keyData` message and `DataError` when the JWK is missing
required fields or passes a non-string value for one. In theory non-strings
are already rejected by WebIDL's JWK converter but this doesn't hurt.
- RSA: requires `n`, `e`; if `d` is present, also requires `p`, `q`,
`dp`, `dq`, and `qi`.
- EC: requires `crv`, `x`, `y`; optional `d`.
- OKP: requires `crv`, `x`; optional `d`.
- oct: requires `k`.
- AKP: requires `alg`, `pub`; optional `priv`.
Four export/import negative tests update their expected error text from
the later "Invalid JWK … Parameter and algorithm name mismatch" to the
new short-circuit "Invalid keyData" (for the case where `crv`/`alg` is
missing entirely). A new `{ kty: 'oct' }` missing-`k` negative is added
for ChaCha20-Poly1305. The tests check error messages but the error
class (DataError/DOMException) is the same everywhere.
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Adds four focused tests that check guarantees either introduced by the
native `CryptoKey` refactor or carried over from the existing state.
- `test-webcrypto-cryptokey-brand-check.js` - each of the four
prototype getters (`type`, `extractable`, `algorithm`, `usages`)
throws `ERR_INVALID_THIS` for foreign receivers (plain objects,
null-proto, primitives, null/undefined, functions, a subverted
`Symbol.hasInstance`, and a real `BaseObject` of a different kind).
`util.types.isCryptoKey()` remains accurate after a prototype
swap, confirming it cannot be spoofed.
- `test-webcrypto-cryptokey-clone-transfer.js` - exhaustive
structured-clone, `MessagePort.postMessage`, and
`Worker.postMessage` round-trips.
Verifies slot preservation, inspect-output equivalence, and that
crypto operations interoperate across clones including repeated
round-trips.
- `test-webcrypto-cryptokey-hidden-slots.js` - replaces all four
prototype getters with forged versions and confirms internal
consumers (export, inspect) still read the real native slots.
- `test-webcrypto-cryptokey-no-own-symbols.js`, asserts CryptoKey
instances expose no own symbol-keyed properties even after every
public getter has been touched (proof the `#slots` private field
plus native storage leaves the instance shape pristine).
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Adds two benchmarks exercising `subtle.sign` / `subtle.verify` across the main algorithm families: - ECDSA P-256 - RSASSA-PKCS1-v1_5 - RSA-PSS - Ed25519 - ML-DSA-44 (gated on OpenSSL >= 3.5) Each benchmark runs two modes (serial / parallel) × two key-reuse patterns (shared / unique per call) so regressions in slot-access hot paths (getCryptoKey* accessors) and in per-call key wrapping surface on both dimensions. Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactors Node.js' Web Crypto
CryptoKeyimplementation to decouple it fromKeyObjectby movingCryptoKey's internal-slot storage into a new nativeNativeCryptoKeybase and updating internal crypto code to use slot accessors instead of JS-visible properties.Paired with a future EOL #62321 deprecation (v26.x runtime-deprecation, v27.x EOL) this makes non-extractable CryptoKey key material unreachable in userland 🎉 thanks to removing public access to the
[[handle]]internal slot.Changes:
NativeCryptoKey(C++) plus JS-side slot helpers (getCryptoKey{Type,Extractable,Algorithm,Usages,Handle}) and updates Web Crypto/internal consumers accordingly.KeyObjectwrappers toKeyObjectHandleusage, including structured-clone/worker-transfer support.The PR is squash-merged, but the changes are split into logical commits to make review tractable.
Commits
Each commit has a description covering its scope and rationale, see individual commit messages for details. Commits are cumulative (except for fixups and applying feedback). No later commit from the ones listed below revises code introduced by an earlier one. In order:
src: decouple KeyObject and CryptoKey and move CryptoKey to src(empty, just a squash target)src,crypto: add NativeCryptoKeylib,crypto: rewire CryptoKey on the native classlib,crypto: migrate algorithm modules to native CryptoKeysrc,crypto: relax RSA/EC keygen arg checkslib,crypto: validate HkdfParams info length earlylib,crypto: add early structural JWK validationtest: add CryptoKey class regression testsbenchmark: add Web Crypto sign/verify benchmarks...rest is fixups and applying feedbackThe PR's changes UI is your friend, either filter commits, or filter files, or both, mark reviewed files as
Viewedto remember where you left off.Benchmark
https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1833/
No regression on any
parallelconfiguration; RSASSA-PKCS1-v1_5 verify is ~5–6% faster (***) on both shared-key and unique-key-per-call.serial-mode rows have ±20–60% margins and are inconclusive. The lone**regression candidate (EC sign, shared/parallel, −3.96%) falls within the multiple-comparison false-positive budget (≈0.62 expected at**+ across 30 comparisons).Signed-off-by: Filip Skokan <panva.ip@gmail.com>