From 8c9df1abbe69446f874fa2034f6e1a4926b048ed Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 4 May 2026 11:52:07 +0200 Subject: [PATCH 1/3] crypto: harden KeyObject internal slots Move KeyObject type and handle storage behind NativeKeyObject and expose it to JS through a module-private slot reader, mirroring the CryptoKey hardening. Cache the native slot tuple in a private field and lazily derive secret and asymmetric metadata from the cached KeyObjectHandle. Update internal crypto, QUIC, and comparison callers to use private helpers instead of public KeyObject accessors. Keep getKeyObjectSlots restricted to internal/crypto/keys with an ESLint guard. Add regression coverage for brand checks, hidden slots, clone and transfer behavior, own-property reflection, and post-clone crypto operations. Extend the CryptoKey brand test to assert getSlots is not reachable through the public constructor or prototype chain. Signed-off-by: Filip Skokan --- lib/eslint.config_partial.mjs | 4 + lib/internal/crypto/aes.js | 7 +- lib/internal/crypto/cfrg.js | 8 +- lib/internal/crypto/chacha20_poly1305.js | 4 +- lib/internal/crypto/ec.js | 8 +- lib/internal/crypto/hkdf.js | 7 +- lib/internal/crypto/keys.js | 316 +++++++++++++----- lib/internal/crypto/mac.js | 7 +- lib/internal/crypto/ml_dsa.js | 8 +- lib/internal/crypto/ml_kem.js | 8 +- lib/internal/crypto/rsa.js | 8 +- lib/internal/crypto/x509.js | 10 +- lib/internal/quic/quic.js | 7 +- lib/internal/util/comparisons.js | 13 +- src/crypto/README.md | 12 +- src/crypto/crypto_keys.cc | 43 ++- src/crypto/crypto_keys.h | 14 + src/crypto/crypto_util.cc | 6 +- src/env_properties.h | 1 + .../test-crypto-keyobject-brand-check.js | 96 ++++++ .../test-crypto-keyobject-clone-transfer.js | 138 ++++++++ .../test-crypto-keyobject-hidden-slots.js | 213 ++++++++++++ .../test-crypto-keyobject-no-own-symbols.js | 42 +++ .../test-webcrypto-cryptokey-brand-check.js | 6 + 24 files changed, 858 insertions(+), 128 deletions(-) create mode 100644 test/parallel/test-crypto-keyobject-brand-check.js create mode 100644 test/parallel/test-crypto-keyobject-clone-transfer.js create mode 100644 test/parallel/test-crypto-keyobject-hidden-slots.js create mode 100644 test/parallel/test-crypto-keyobject-no-own-symbols.js diff --git a/lib/eslint.config_partial.mjs b/lib/eslint.config_partial.mjs index 61632a7ee447e6..3c9bf0bc4e177c 100644 --- a/lib/eslint.config_partial.mjs +++ b/lib/eslint.config_partial.mjs @@ -75,6 +75,10 @@ export default [ selector: "VariableDeclarator[init.type='CallExpression'][init.callee.name='internalBinding'][init.arguments.0.value='crypto'] > ObjectPattern > Property[key.name='getCryptoKeySlots']", message: "Use `const { getCryptoKeySlots } = require('internal/crypto/keys');` instead of destructuring it from `internalBinding('crypto')`.", }, + { + selector: "VariableDeclarator[init.type='CallExpression'][init.callee.name='internalBinding'][init.arguments.0.value='crypto'] > ObjectPattern > Property[key.name='getKeyObjectSlots']", + message: "Use `const { getKeyObjectSlots } = require('internal/crypto/keys');` instead of destructuring it from `internalBinding('crypto')`.", + }, ], 'no-restricted-globals': [ 'error', diff --git a/lib/internal/crypto/aes.js b/lib/internal/crypto/aes.js index 123babf6fd570d..73fdde03d73ba8 100644 --- a/lib/internal/crypto/aes.js +++ b/lib/internal/crypto/aes.js @@ -30,7 +30,6 @@ const { getUsagesMask, hasAnyNotIn, jobPromise, - kHandle, } = require('internal/crypto/util'); const { @@ -41,6 +40,8 @@ const { InternalCryptoKey, getCryptoKeyAlgorithm, getCryptoKeyHandle, + getKeyObjectHandle, + getKeyObjectSymmetricKeySize, } = require('internal/crypto/keys'); const { @@ -219,9 +220,9 @@ function aesImportKey( let length; switch (format) { case 'KeyObject': { - length = keyData.symmetricKeySize * 8; + length = getKeyObjectSymmetricKeySize(keyData) * 8; validateKeyLength(length); - handle = keyData[kHandle]; + handle = getKeyObjectHandle(keyData); break; } case 'raw-secret': diff --git a/lib/internal/crypto/cfrg.js b/lib/internal/crypto/cfrg.js index f3e3e008b629f3..8d26a2888200ff 100644 --- a/lib/internal/crypto/cfrg.js +++ b/lib/internal/crypto/cfrg.js @@ -28,7 +28,6 @@ const { getUsagesUnion, hasAnyNotIn, jobPromise, - kHandle, } = require('internal/crypto/util'); const { @@ -38,6 +37,8 @@ const { const { getCryptoKeyHandle, getCryptoKeyType, + getKeyObjectHandle, + getKeyObjectType, InternalCryptoKey, } = require('internal/crypto/keys'); @@ -188,8 +189,9 @@ function cfrgImportKey( const usagesSet = new SafeSet(keyUsages); switch (format) { case 'KeyObject': { - verifyAcceptableCfrgKeyUse(name, keyData.type === 'public', usagesSet); - handle = keyData[kHandle]; + verifyAcceptableCfrgKeyUse( + name, getKeyObjectType(keyData) === 'public', usagesSet); + handle = getKeyObjectHandle(keyData); break; } case 'spki': { diff --git a/lib/internal/crypto/chacha20_poly1305.js b/lib/internal/crypto/chacha20_poly1305.js index ca7ec501bf4a0c..1bd173cab36191 100644 --- a/lib/internal/crypto/chacha20_poly1305.js +++ b/lib/internal/crypto/chacha20_poly1305.js @@ -14,7 +14,6 @@ const { getUsagesMask, hasAnyNotIn, jobPromise, - kHandle, } = require('internal/crypto/util'); const { @@ -24,6 +23,7 @@ const { const { InternalCryptoKey, getCryptoKeyHandle, + getKeyObjectHandle, } = require('internal/crypto/keys'); const { @@ -87,7 +87,7 @@ function c20pImportKey( let handle; switch (format) { case 'KeyObject': { - handle = keyData[kHandle]; + handle = getKeyObjectHandle(keyData); break; } case 'raw-secret': { diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js index e399071228e4e8..983bfde2e8efa6 100644 --- a/lib/internal/crypto/ec.js +++ b/lib/internal/crypto/ec.js @@ -34,7 +34,6 @@ const { hasAnyNotIn, jobPromise, normalizeHashName, - kHandle, kNamedCurveAliases, } = require('internal/crypto/util'); @@ -47,6 +46,8 @@ const { getCryptoKeyAlgorithm, getCryptoKeyHandle, getCryptoKeyType, + getKeyObjectHandle, + getKeyObjectType, } = require('internal/crypto/keys'); const { @@ -189,8 +190,9 @@ function ecImportKey( const usagesSet = new SafeSet(keyUsages); switch (format) { case 'KeyObject': { - verifyAcceptableEcKeyUse(name, keyData.type === 'public', usagesSet); - handle = keyData[kHandle]; + verifyAcceptableEcKeyUse( + name, getKeyObjectType(keyData) === 'public', usagesSet); + handle = getKeyObjectHandle(keyData); break; } case 'spki': { diff --git a/lib/internal/crypto/hkdf.js b/lib/internal/crypto/hkdf.js index 4203e3ee21c701..424c56fd894961 100644 --- a/lib/internal/crypto/hkdf.js +++ b/lib/internal/crypto/hkdf.js @@ -29,6 +29,7 @@ const { const { createSecretKey, getCryptoKeyHandle, + getKeyObjectHandle, isKeyObject, } = require('internal/crypto/keys'); @@ -75,10 +76,10 @@ const validateParameters = hideStackFrames((hash, key, salt, info, length) => { function prepareKey(key) { if (isKeyObject(key)) - return key; + return getKeyObjectHandle(key); if (isAnyArrayBuffer(key)) - return createSecretKey(key); + return getKeyObjectHandle(createSecretKey(key)); key = toBuf(key); @@ -96,7 +97,7 @@ function prepareKey(key) { key); } - return createSecretKey(key); + return getKeyObjectHandle(createSecretKey(key)); } function hkdf(hash, key, salt, info, length, callback) { diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js index 03ace95df21dca..fd0fa7185bc113 100644 --- a/lib/internal/crypto/keys.js +++ b/lib/internal/crypto/keys.js @@ -3,10 +3,8 @@ const { ArrayPrototypeSlice, ObjectDefineProperties, - ObjectDefineProperty, ObjectSetPrototypeOf, SafeSet, - Symbol, SymbolToStringTag, Uint8Array, } = primordials; @@ -14,6 +12,8 @@ const { const { KeyObjectHandle, createNativeKeyObjectClass, + // eslint-disable-next-line no-restricted-syntax -- intended here + getKeyObjectSlots: nativeGetKeyObjectSlots, createCryptoKeyClass, // eslint-disable-next-line no-restricted-syntax -- intended here getCryptoKeySlots: nativeGetCryptoKeySlots, @@ -57,7 +57,6 @@ const { } = require('internal/errors'); const { - kHandle, getArrayBufferOrView, bigIntArrayToUnsignedBigInt, normalizeAlgorithm, @@ -76,18 +75,12 @@ const { customInspectSymbol: kInspect, getDeprecationWarningEmitter, kEnumerableProperty, + kEmptyObject, lazyDOMException, } = require('internal/util'); const { inspect } = require('internal/util/inspect'); -// Module-local symbol used by KeyObject to store its `type` string -// ("secret"/"public"/"private"). It is also used by `isKeyObject` to -// distinguish KeyObject instances from other types. CryptoKey no longer -// uses any module-local symbol slots - its state lives in C++ internal -// fields on `NativeCryptoKey`. -const kKeyType = Symbol('kKeyType'); - const emitDEP0203 = getDeprecationWarningEmitter( 'DEP0203', 'Passing a CryptoKey to node:crypto functions is deprecated.', @@ -112,6 +105,32 @@ for (const m of [[kKeyEncodingPKCS1, 'pkcs1'], [kKeyEncodingPKCS8, 'pkcs8'], [kKeyEncodingSPKI, 'spki'], [kKeyEncodingSEC1, 'sec1']]) encodingNames[m[0]] = m[1]; +// KeyObject state lives on the native NativeKeyObject base class. JS reads +// the native type enum and a KeyObjectHandle in one call and caches that +// slot tuple in a private field so no forgeable own Symbols are exposed on +// public KeyObject instances. +let getKeyObjectSlots; // Populated by the createNativeKeyObjectClass callback. + +const kKeyObjectSlotType = 0; +const kKeyObjectSlotHandle = 1; +// The native slot tuple stops at kKeyObjectSlotHandle. The remaining entries +// are JS-side lazy cache slots derived from the KeyObjectHandle on first use. +const kKeyObjectSlotSymmetricKeySize = 2; +const kKeyObjectSlotAsymmetricKeyType = 3; +const kKeyObjectSlotAsymmetricKeyDetails = 4; + +function normalizeKeyDetails(details = kEmptyObject) { + if (details.publicExponent !== undefined) { + return { + __proto__: null, + ...details, + publicExponent: + bigIntArrayToUnsignedBigInt(new Uint8Array(details.publicExponent)), + }; + } + return details; +} + // Creating the KeyObject class is a little complicated due to inheritance // and the fact that KeyObjects should be transferable between threads, // which requires the KeyObject base class to be implemented in C++. @@ -125,6 +144,8 @@ const { } = createNativeKeyObjectClass((NativeKeyObject) => { // Publicly visible KeyObject class. class KeyObject extends NativeKeyObject { + #slots; + constructor(type, handle) { if (type !== 'secret' && type !== 'public' && type !== 'private') throw new ERR_INVALID_ARG_VALUE('type', type); @@ -132,20 +153,10 @@ const { throw new ERR_INVALID_ARG_TYPE('handle', 'object', handle); super(handle); - - this[kKeyType] = type; - - ObjectDefineProperty(this, kHandle, { - __proto__: null, - value: handle, - enumerable: false, - configurable: false, - writable: false, - }); } get type() { - return this[kKeyType]; + return getKeyObjectType(this); } static from(key) { @@ -168,8 +179,25 @@ const { 'otherKeyObject', 'KeyObject', otherKeyObject); } - return otherKeyObject.type === this.type && - this[kHandle].equals(otherKeyObject[kHandle]); + const slots = getKeyObjectSlots(this); + const otherSlots = getKeyObjectSlots(otherKeyObject); + return slots[kKeyObjectSlotType] === otherSlots[kKeyObjectSlotType] && + slots[kKeyObjectSlotHandle].equals( + otherSlots[kKeyObjectSlotHandle]); + } + + static { + getKeyObjectSlots = (key) => { + if (!key || typeof key !== 'object') + throw new ERR_INVALID_THIS('KeyObject'); + if (#slots in key) { + const cached = key.#slots; + if (cached !== undefined) return cached; + } + const slots = nativeGetKeyObjectSlots(key); + key.#slots = slots; + return slots; + }; } } @@ -189,19 +217,20 @@ const { } get symmetricKeySize() { - return this[kHandle].getSymmetricKeySize(); + return getKeyObjectSymmetricKeySize(this); } export(options) { + const handle = getKeyObjectHandle(this); if (options !== undefined) { validateObject(options, 'options'); validateOneOf( options.format, 'options.format', [undefined, 'buffer', 'jwk']); if (options.format === 'jwk') { - return this[kHandle].exportJwk({}, false); + return handle.exportJwk({}, false); } } - return this[kHandle].export(); + return handle.export(); } toCryptoKey(algorithm, extractable, keyUsages) { @@ -266,37 +295,13 @@ const { } } - const kAsymmetricKeyType = Symbol('kAsymmetricKeyType'); - const kAsymmetricKeyDetails = Symbol('kAsymmetricKeyDetails'); - - function normalizeKeyDetails(details = {}) { - if (details.publicExponent !== undefined) { - return { - ...details, - publicExponent: - bigIntArrayToUnsignedBigInt(new Uint8Array(details.publicExponent)), - }; - } - return details; - } - class AsymmetricKeyObject extends KeyObject { get asymmetricKeyType() { - return this[kAsymmetricKeyType] ||= this[kHandle].getAsymmetricKeyType(); + return getKeyObjectAsymmetricKeyType(this); } get asymmetricKeyDetails() { - switch (this.asymmetricKeyType) { - case 'rsa': - case 'rsa-pss': - case 'dsa': - case 'ec': - return this[kAsymmetricKeyDetails] ||= normalizeKeyDetails( - this[kHandle].keyDetail({}), - ); - default: - return {}; - } + return { ...getKeyObjectAsymmetricKeyDetails(this) }; } toCryptoKey(algorithm, extractable, keyUsages) { @@ -370,23 +375,27 @@ const { export(options) { switch (options?.format) { case 'jwk': - return this[kHandle].exportJwk({}, false); + return getKeyObjectHandle(this).exportJwk({}, false); case 'raw-public': { - if (this.asymmetricKeyType === 'ec') { + const handle = getKeyObjectHandle(this); + const asymmetricKeyType = getKeyObjectAsymmetricKeyType(this); + if (asymmetricKeyType === 'ec') { const { type = 'uncompressed' } = options; validateOneOf(type, 'options.type', ['compressed', 'uncompressed']); const form = type === 'compressed' ? POINT_CONVERSION_COMPRESSED : POINT_CONVERSION_UNCOMPRESSED; - return this[kHandle].exportECPublicRaw(form); + return handle.exportECPublicRaw(form); } - return this[kHandle].rawPublicKey(); + return handle.rawPublicKey(); } default: { + const asymmetricKeyType = getKeyObjectAsymmetricKeyType(this); + const handle = getKeyObjectHandle(this); const { format, type, - } = parsePublicKeyEncoding(options, this.asymmetricKeyType); - return this[kHandle].export(format, type); + } = parsePublicKeyEncoding(options, asymmetricKeyType); + return handle.export(format, type); } } } @@ -405,23 +414,27 @@ const { } switch (options?.format) { case 'jwk': - return this[kHandle].exportJwk({}, false); + return getKeyObjectHandle(this).exportJwk({}, false); case 'raw-private': { - if (this.asymmetricKeyType === 'ec') { - return this[kHandle].exportECPrivateRaw(); + const handle = getKeyObjectHandle(this); + const asymmetricKeyType = getKeyObjectAsymmetricKeyType(this); + if (asymmetricKeyType === 'ec') { + return handle.exportECPrivateRaw(); } - return this[kHandle].rawPrivateKey(); + return handle.rawPrivateKey(); } case 'raw-seed': - return this[kHandle].rawSeed(); + return getKeyObjectHandle(this).rawSeed(); default: { + const asymmetricKeyType = getKeyObjectAsymmetricKeyType(this); + const handle = getKeyObjectHandle(this); const { format, type, cipher, passphrase, - } = parsePrivateKeyEncoding(options, this.asymmetricKeyType); - return this[kHandle].export(format, type, cipher, passphrase); + } = parsePrivateKeyEncoding(options, asymmetricKeyType); + return handle.export(format, type, cipher, passphrase); } } } @@ -635,8 +648,9 @@ function getKeyTypes(allowKeyObject, bufferOnly = false) { function prepareAsymmetricKey(key, ctx, name = 'key') { if (isKeyObject(key)) { // Best case: A key object, as simple as that. - validateAsymmetricKeyType(key.type, ctx, key); - return { data: key[kHandle] }; + const type = getKeyObjectType(key); + validateAsymmetricKeyType(type, ctx, key); + return { data: getKeyObjectHandle(key) }; } if (isCryptoKey(key)) { emitDEP0203(); @@ -653,8 +667,9 @@ function prepareAsymmetricKey(key, ctx, name = 'key') { // The 'key' property can be a KeyObject as well to allow specifying // additional options such as padding along with the key. if (isKeyObject(data)) { - validateAsymmetricKeyType(data.type, ctx, data); - return { data: data[kHandle] }; + const type = getKeyObjectType(data); + validateAsymmetricKeyType(type, ctx, data); + return { data: getKeyObjectHandle(data) }; } if (isCryptoKey(data)) { emitDEP0203(); @@ -718,9 +733,10 @@ function preparePublicOrPrivateKey(key, name) { function prepareSecretKey(key, encoding, bufferOnly = false) { if (!bufferOnly) { if (isKeyObject(key)) { - if (key.type !== 'secret') - throw new ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE(key.type, 'secret'); - return key[kHandle]; + const type = getKeyObjectType(key); + if (type !== 'secret') + throw new ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE(type, 'secret'); + return getKeyObjectHandle(key); } if (isCryptoKey(key)) { emitDEP0203(); @@ -766,8 +782,132 @@ function createPrivateKey(key) { return new PrivateKeyObject(handle); } +function keyObjectTypeToString(type) { + switch (type) { + case kKeyTypeSecret: return 'secret'; + case kKeyTypePublic: return 'public'; + case kKeyTypePrivate: return 'private'; + default: { + const assert = require('internal/assert'); + assert.fail('Unreachable code'); + } + } +} + +// The helpers below return a KeyObject's native-backed slot values, +// populating the per-instance cache on first access via a single native +// call. The public getters delegate to these helpers, and internal +// consumers use them directly to avoid user-replaceable public accessors. +// Derived metadata such as key size and asymmetric key details is expanded +// lazily from the cached KeyObjectHandle. The public asymmetric key details +// getter returns a clone so the cached details object stays internal. + +/** + * Returns the KeyObject's native type slot as a string. + * @param {KeyObject} key + * @returns {'secret' | 'public' | 'private'} + */ +function getKeyObjectType(key) { + return keyObjectTypeToString(getKeyObjectSlots(key)[kKeyObjectSlotType]); +} + +/** + * Returns the KeyObjectHandle wrapping the KeyObject's underlying key + * material. + * @param {KeyObject} key + * @returns {KeyObjectHandle} + */ +function getKeyObjectHandle(key) { + return getKeyObjectSlots(key)[kKeyObjectSlotHandle]; +} + +/** + * Returns the KeyObject's symmetric key size, bypassing the public + * `symmetricKeySize` getter. The value is derived lazily from the cached + * KeyObjectHandle. + * @param {SecretKeyObject} key + * @returns {number} + */ +function getKeyObjectSymmetricKeySize(key) { + const slots = getKeyObjectSlots(key); + if (slots[kKeyObjectSlotType] !== kKeyTypeSecret) + throw new ERR_INVALID_THIS('SecretKeyObject'); + + let cached = slots[kKeyObjectSlotSymmetricKeySize]; + if (cached === undefined) { + cached = slots[kKeyObjectSlotHandle].getSymmetricKeySize(); + slots[kKeyObjectSlotSymmetricKeySize] = cached; + } + return cached; +} + +/** + * Returns the KeyObject's asymmetric key type, bypassing the public + * `asymmetricKeyType` getter. The value is derived lazily from the cached + * KeyObjectHandle. + * @param {PublicKeyObject|PrivateKeyObject} key + * @returns {string} + */ +function getKeyObjectAsymmetricKeyType(key) { + const slots = getKeyObjectSlots(key); + if (slots[kKeyObjectSlotType] === kKeyTypeSecret) + throw new ERR_INVALID_THIS('AsymmetricKeyObject'); + + let cached = slots[kKeyObjectSlotAsymmetricKeyType]; + if (cached === undefined) { + cached = slots[kKeyObjectSlotHandle].getAsymmetricKeyType(); + slots[kKeyObjectSlotAsymmetricKeyType] = cached; + } + return cached; +} + +/** + * Returns the KeyObject's cached asymmetric key details, bypassing the + * public `asymmetricKeyDetails` getter (which returns a cloned copy). + * The value is derived lazily from the cached KeyObjectHandle. + * @param {PublicKeyObject|PrivateKeyObject} key + * @returns {object} + */ +function getKeyObjectAsymmetricKeyDetails(key) { + const slots = getKeyObjectSlots(key); + if (slots[kKeyObjectSlotType] === kKeyTypeSecret) + throw new ERR_INVALID_THIS('AsymmetricKeyObject'); + + let cached = slots[kKeyObjectSlotAsymmetricKeyDetails]; + if (cached === undefined) { + let asymmetricKeyType = slots[kKeyObjectSlotAsymmetricKeyType]; + if (asymmetricKeyType === undefined) { + asymmetricKeyType = slots[kKeyObjectSlotHandle].getAsymmetricKeyType(); + slots[kKeyObjectSlotAsymmetricKeyType] = asymmetricKeyType; + } + switch (asymmetricKeyType) { + case 'rsa': + case 'rsa-pss': + case 'dsa': + case 'ec': + cached = normalizeKeyDetails( + slots[kKeyObjectSlotHandle].keyDetail({ __proto__: null }), + ); + break; + default: + cached = kEmptyObject; + break; + } + slots[kKeyObjectSlotAsymmetricKeyDetails] = cached; + } + return cached; +} + function isKeyObject(obj) { - return obj != null && obj[kKeyType] !== undefined; + if (obj == null || typeof obj !== 'object') + return false; + + try { + getKeyObjectSlots(obj); + return true; + } catch { + return false; + } } // CryptoKey is a plain JS class whose prototype's [[Prototype]] is @@ -869,19 +1009,20 @@ const { class InternalCryptoKey extends NativeCryptoKey { #slots; - static getSlots(key) { - if (!key || typeof key !== 'object') - throw new ERR_INVALID_THIS('CryptoKey'); - if (#slots in key) { - const cached = key.#slots; - if (cached !== undefined) return cached; - } - const slots = nativeGetCryptoKeySlots(key); - key.#slots = slots; - return slots; + static { + getSlots = (key) => { + if (!key || typeof key !== 'object') + throw new ERR_INVALID_THIS('CryptoKey'); + if (#slots in key) { + const cached = key.#slots; + if (cached !== undefined) return cached; + } + const slots = nativeGetCryptoKeySlots(key); + key.#slots = slots; + return slots; + }; } } - getSlots = InternalCryptoKey.getSlots; // Hide NativeCryptoKey from user code. InternalCryptoKey.prototype.constructor = CryptoKey; ObjectSetPrototypeOf(InternalCryptoKey.prototype, CryptoKey.prototype); @@ -1032,7 +1173,7 @@ function importGenericSecretKey( let handle; switch (format) { case 'KeyObject': { - handle = keyData[kHandle]; + handle = getKeyObjectHandle(keyData); break; } case 'raw-secret': @@ -1072,6 +1213,11 @@ module.exports = { PublicKeyObject, PrivateKeyObject, isKeyObject, + getKeyObjectType, + getKeyObjectHandle, + getKeyObjectSymmetricKeySize, + getKeyObjectAsymmetricKeyType, + getKeyObjectAsymmetricKeyDetails, isCryptoKey, getCryptoKeyType, getCryptoKeyExtractable, diff --git a/lib/internal/crypto/mac.js b/lib/internal/crypto/mac.js index 6554e05a892ff4..57576f729b7b41 100644 --- a/lib/internal/crypto/mac.js +++ b/lib/internal/crypto/mac.js @@ -20,7 +20,6 @@ const { hasAnyNotIn, jobPromise, normalizeHashName, - kHandle, } = require('internal/crypto/util'); const { @@ -31,6 +30,8 @@ const { InternalCryptoKey, getCryptoKeyAlgorithm, getCryptoKeyHandle, + getKeyObjectHandle, + getKeyObjectSymmetricKeySize, } = require('internal/crypto/keys'); const { @@ -106,8 +107,8 @@ function macImportKey( let length; switch (format) { case 'KeyObject': { - length = keyData.symmetricKeySize * 8; - handle = keyData[kHandle]; + length = getKeyObjectSymmetricKeySize(keyData) * 8; + handle = getKeyObjectHandle(keyData); break; } case 'raw-secret': diff --git a/lib/internal/crypto/ml_dsa.js b/lib/internal/crypto/ml_dsa.js index 173ef666c29736..bd93327f93aa5f 100644 --- a/lib/internal/crypto/ml_dsa.js +++ b/lib/internal/crypto/ml_dsa.js @@ -29,7 +29,6 @@ const { getUsagesUnion, hasAnyNotIn, jobPromise, - kHandle, } = require('internal/crypto/util'); const { @@ -39,6 +38,8 @@ const { const { getCryptoKeyHandle, getCryptoKeyType, + getKeyObjectHandle, + getKeyObjectType, InternalCryptoKey, } = require('internal/crypto/keys'); @@ -151,8 +152,9 @@ function mlDsaImportKey( const usagesSet = new SafeSet(keyUsages); switch (format) { case 'KeyObject': { - verifyAcceptableMlDsaKeyUse(name, keyData.type === 'public', usagesSet); - handle = keyData[kHandle]; + verifyAcceptableMlDsaKeyUse( + name, getKeyObjectType(keyData) === 'public', usagesSet); + handle = getKeyObjectHandle(keyData); break; } case 'spki': { diff --git a/lib/internal/crypto/ml_kem.js b/lib/internal/crypto/ml_kem.js index 67f5ddd0ff2499..3725c23d7c924d 100644 --- a/lib/internal/crypto/ml_kem.js +++ b/lib/internal/crypto/ml_kem.js @@ -29,7 +29,6 @@ const { getUsagesUnion, hasAnyNotIn, jobPromise, - kHandle, } = require('internal/crypto/util'); const { @@ -39,6 +38,8 @@ const { const { getCryptoKeyHandle, getCryptoKeyType, + getKeyObjectHandle, + getKeyObjectType, InternalCryptoKey, } = require('internal/crypto/keys'); @@ -147,8 +148,9 @@ function mlKemImportKey( const usagesSet = new SafeSet(keyUsages); switch (format) { case 'KeyObject': { - verifyAcceptableMlKemKeyUse(name, keyData.type === 'public', usagesSet); - handle = keyData[kHandle]; + verifyAcceptableMlKemKeyUse( + name, getKeyObjectType(keyData) === 'public', usagesSet); + handle = getKeyObjectHandle(keyData); break; } case 'spki': { diff --git a/lib/internal/crypto/rsa.js b/lib/internal/crypto/rsa.js index a019e70e0268b7..6034ed64e69514 100644 --- a/lib/internal/crypto/rsa.js +++ b/lib/internal/crypto/rsa.js @@ -36,7 +36,6 @@ const { jobPromise, normalizeHashName, validateMaxBufferLength, - kHandle, } = require('internal/crypto/util'); const { @@ -48,6 +47,8 @@ const { getCryptoKeyAlgorithm, getCryptoKeyHandle, getCryptoKeyType, + getKeyObjectHandle, + getKeyObjectType, } = require('internal/crypto/keys'); const { @@ -217,8 +218,9 @@ function rsaImportKey( let handle; switch (format) { case 'KeyObject': { - verifyAcceptableRsaKeyUse(algorithm.name, keyData.type === 'public', usagesSet); - handle = keyData[kHandle]; + verifyAcceptableRsaKeyUse( + algorithm.name, getKeyObjectType(keyData) === 'public', usagesSet); + handle = getKeyObjectHandle(keyData); break; } case 'spki': { diff --git a/lib/internal/crypto/x509.js b/lib/internal/crypto/x509.js index fcec607fb648de..cd5b5457e3ca67 100644 --- a/lib/internal/crypto/x509.js +++ b/lib/internal/crypto/x509.js @@ -18,6 +18,8 @@ const { const { PublicKeyObject, + getKeyObjectHandle, + getKeyObjectType, isKeyObject, } = require('internal/crypto/keys'); @@ -374,17 +376,17 @@ class X509Certificate { checkPrivateKey(pkey) { if (!isKeyObject(pkey)) throw new ERR_INVALID_ARG_TYPE('pkey', 'KeyObject', pkey); - if (pkey.type !== 'private') + if (getKeyObjectType(pkey) !== 'private') throw new ERR_INVALID_ARG_VALUE('pkey', pkey); - return this[kHandle].checkPrivateKey(pkey[kHandle]); + return this[kHandle].checkPrivateKey(getKeyObjectHandle(pkey)); } verify(pkey) { if (!isKeyObject(pkey)) throw new ERR_INVALID_ARG_TYPE('pkey', 'KeyObject', pkey); - if (pkey.type !== 'public') + if (getKeyObjectType(pkey) !== 'public') throw new ERR_INVALID_ARG_VALUE('pkey', pkey); - return this[kHandle].verify(pkey[kHandle]); + return this[kHandle].verify(getKeyObjectHandle(pkey)); } toLegacyObject() { diff --git a/lib/internal/quic/quic.js b/lib/internal/quic/quic.js index a24ba4d39c6e69..46aef7ba00dc3b 100644 --- a/lib/internal/quic/quic.js +++ b/lib/internal/quic/quic.js @@ -97,6 +97,8 @@ const { } = require('internal/blob'); const { + getKeyObjectHandle, + getKeyObjectType, isKeyObject, } = require('internal/crypto/keys'); @@ -140,7 +142,6 @@ const { kTrailers, kVersionNegotiation, kInspect, - kKeyObjectHandle, kWantsHeaders, kWantsTrailers, } = require('internal/quic/symbols'); @@ -2209,11 +2210,11 @@ function processIdentityOptions(identity, label) { const keyInputs = ArrayIsArray(keys) ? keys : [keys]; for (const key of keyInputs) { if (isKeyObject(key)) { - if (key.type !== 'private') { + if (getKeyObjectType(key) !== 'private') { throw new ERR_INVALID_ARG_VALUE(`${label}.keys`, key, 'must be a private key'); } - ArrayPrototypePush(keyHandles, key[kKeyObjectHandle]); + ArrayPrototypePush(keyHandles, getKeyObjectHandle(key)); } else { throw new ERR_INVALID_ARG_TYPE(`${label}.keys`, 'KeyObject', key); } diff --git a/lib/internal/util/comparisons.js b/lib/internal/util/comparisons.js index 2f21740fcb84bf..80d39756cf155a 100644 --- a/lib/internal/util/comparisons.js +++ b/lib/internal/util/comparisons.js @@ -132,6 +132,8 @@ let getCryptoKeyType; let getCryptoKeyExtractable; let getCryptoKeyAlgorithm; let getCryptoKeyUsagesMask; +let getKeyObjectHandle; +let getKeyObjectType; const kStrict = 2; const kStrictWithoutPrototypes = 3; @@ -410,7 +412,16 @@ function objectComparisonStart(val1, val2, mode, memos) { return false; } } else if (isKeyObject(val1)) { - if (!isKeyObject(val2) || !val1.equals(val2)) { + if (getKeyObjectHandle === undefined) { + ({ + getKeyObjectHandle, + getKeyObjectType, + } = require('internal/crypto/keys')); + } + if (!isKeyObject(val2) || + getKeyObjectType(val1) !== getKeyObjectType(val2) || + !getKeyObjectHandle(val1).equals(getKeyObjectHandle(val2)) + ) { return false; } } else if (isCryptoKey(val1)) { diff --git a/src/crypto/README.md b/src/crypto/README.md index 672b15c13d4962..263a512cdefc9b 100644 --- a/src/crypto/README.md +++ b/src/crypto/README.md @@ -155,18 +155,22 @@ a Secret key. It is the shared backing representation used by `KeyObject`, #### `KeyObjectHandle` -`KeyObjectHandle` is the JavaScript-visible C++ handle for a +`KeyObjectHandle` is the internal JavaScript-visible C++ handle for a `KeyObjectData`. It exposes operations that internal JavaScript uses to initialize, inspect, compare, and export key material. Native code passes `KeyObjectData` across threads and jobs; a `KeyObjectHandle` is created when -JavaScript needs access to those operations. +JavaScript needs access to those operations and is kept out of user-visible +`KeyObject` own properties. #### `KeyObject` A `KeyObject` is the public Node.js-specific API for keys. It extends a native `NativeKeyObject`, which stores `KeyObjectData` for structured -cloning, and it owns one `KeyObjectHandle` used by the JavaScript API -surface. +cloning. The JavaScript API surface reads its key type and a +`KeyObjectHandle` through a hidden native-backed slot tuple, caching that +tuple in a private field outside user-visible own properties. Derived +metadata, such as symmetric key size and asymmetric key details, is read +from the cached handle and appended lazily to the same private-field cache. #### `CryptoKey` diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc index 92bb7dbb9714ce..269539c7915480 100644 --- a/src/crypto/crypto_keys.cc +++ b/src/crypto/crypto_keys.cc @@ -1686,18 +1686,26 @@ void NativeKeyObject::Initialize(Environment* env, Local target) { target, "createNativeKeyObjectClass", NativeKeyObject::CreateNativeKeyObjectClass); + SetMethod( + env->context(), target, "getKeyObjectSlots", NativeKeyObject::GetSlots); } void NativeKeyObject::RegisterExternalReferences( ExternalReferenceRegistry* registry) { registry->Register(NativeKeyObject::CreateNativeKeyObjectClass); + registry->Register(NativeKeyObject::GetSlots); registry->Register(NativeKeyObject::New); } +bool NativeKeyObject::HasInstance(Environment* env, Local value) { + auto t = env->crypto_key_object_constructor_template(); + return !t.IsEmpty() && t->HasInstance(value); +} + void NativeKeyObject::New(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK_EQ(args.Length(), 1); - CHECK(args[0]->IsObject()); + CHECK(KeyObjectHandle::HasInstance(env, args[0])); KeyObjectHandle* handle = Unwrap(args[0].As()); CHECK_NOT_NULL(handle); new NativeKeyObject(env, args.This(), handle->Data()); @@ -1716,6 +1724,8 @@ void NativeKeyObject::CreateNativeKeyObjectClass( NewFunctionTemplate(isolate, NativeKeyObject::New); t->InstanceTemplate()->SetInternalFieldCount( NativeKeyObject::kInternalFieldCount); + CHECK(env->crypto_key_object_constructor_template().IsEmpty()); + env->set_crypto_key_object_constructor_template(t); Local ctor; if (!t->GetFunction(env->context()).ToLocal(&ctor)) @@ -1737,6 +1747,34 @@ void NativeKeyObject::CreateNativeKeyObjectClass( args.GetReturnValue().Set(ret); } +// Returns the key's native hidden slot tuple as a single Array: +// [type enum, handle]. JS-side helpers call this once per key to prime +// a per-instance cache; derived metadata is appended lazily from JS by +// calling methods on the returned KeyObjectHandle. +void NativeKeyObject::GetSlots(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + CHECK_EQ(args.Length(), 1); + if (!HasInstance(env, args[0])) { + THROW_ERR_INVALID_THIS(env, "Value of \"this\" must be of type KeyObject"); + return; + } + + NativeKeyObject* native = Unwrap(args[0].As()); + CHECK_NOT_NULL(native); + + Local handle; + if (!KeyObjectHandle::Create(env, native->handle_data_).ToLocal(&handle)) { + return; + } + + Isolate* isolate = env->isolate(); + Local slots[] = { + Uint32::NewFromUnsigned(isolate, native->handle_data_.GetKeyType()), + handle, + }; + args.GetReturnValue().Set(Array::New(isolate, slots, arraysize(slots))); +} + BaseObjectPtr NativeKeyObject::KeyObjectTransferData::Deserialize( Environment* env, Local context, @@ -1776,7 +1814,7 @@ BaseObjectPtr NativeKeyObject::KeyObjectTransferData::Deserialize( if (!key_ctor->NewInstance(context, 1, &handle).ToLocal(&key)) return {}; - return BaseObjectPtr(Unwrap(key.As())); + return BaseObjectPtr(Unwrap(key.As())); } BaseObject::TransferMode NativeKeyObject::GetTransferMode() const { @@ -1897,6 +1935,7 @@ void NativeCryptoKey::GetSlots(const FunctionCallbackInfo& args) { } Local obj = args[0].As(); NativeCryptoKey* native = Unwrap(obj); + CHECK_NOT_NULL(native); Local handle; if (!KeyObjectHandle::Create(env, native->handle_data_).ToLocal(&handle)) { diff --git a/src/crypto/crypto_keys.h b/src/crypto/crypto_keys.h index bab640138e909d..8bba206a08239e 100644 --- a/src/crypto/crypto_keys.h +++ b/src/crypto/crypto_keys.h @@ -189,6 +189,11 @@ class KeyObjectHandle : public BaseObject { KeyObjectData data_; }; +// NativeKeyObject is the native base class for the Node.js-specific +// `KeyObject`. It holds the underlying KeyObjectData for structured +// cloning and exposes the native hidden slot tuple that JS needs: +// [type enum, KeyObjectHandle]. JS primes a per-instance private-field +// cache from that result and lazily appends derived metadata there. class NativeKeyObject : public BaseObject { public: static void Initialize(Environment* env, v8::Local target); @@ -198,6 +203,15 @@ class NativeKeyObject : public BaseObject { static void CreateNativeKeyObjectClass( const v8::FunctionCallbackInfo& args); + // True if `value` is a real NativeKeyObject instance. Uses the + // FunctionTemplate stored on the Environment as a brand check. + // Used by `GetSlots` to validate its receiver. + static bool HasInstance(Environment* env, v8::Local value); + + // Returns [type, handle] in one call so JS can prime a per-instance cache + // on first access. Derived metadata is not returned from native here. + static void GetSlots(const v8::FunctionCallbackInfo& args); + SET_NO_MEMORY_INFO() SET_MEMORY_INFO_NAME(NativeKeyObject) SET_SELF_SIZE(NativeKeyObject) diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index 0e743135e8de15..53d6142917dc58 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -479,9 +479,9 @@ ByteSource ByteSource::FromBuffer(Local buffer, bool ntc) { ByteSource ByteSource::FromSecretKeyBytes( Environment* env, Local value) { - // A key can be passed as a string, buffer or KeyObject with type 'secret'. - // If it is a string, we need to convert it to a buffer. We are not doing that - // in JS to avoid creating an unprotected copy on the heap. + // JS normalizes secret KeyObject/CryptoKey inputs to a KeyObjectHandle. + // Strings are converted here instead of in JS to avoid creating an + // unprotected copy on the heap. return value->IsString() || IsAnyBufferSource(value) ? ByteSource::FromStringOrBuffer(env, value) : ByteSource::FromSymmetricKeyObjectHandle(value); diff --git a/src/env_properties.h b/src/env_properties.h index 60a5d75708ac85..6530f89ec918ac 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -409,6 +409,7 @@ V(cpu_usage_template, v8::DictionaryTemplate) \ V(crypto_cryptokey_constructor_template, v8::FunctionTemplate) \ V(crypto_key_object_handle_constructor, v8::FunctionTemplate) \ + V(crypto_key_object_constructor_template, v8::FunctionTemplate) \ V(env_proxy_template, v8::ObjectTemplate) \ V(env_proxy_ctor_template, v8::FunctionTemplate) \ V(ephemeral_key_template, v8::DictionaryTemplate) \ diff --git a/test/parallel/test-crypto-keyobject-brand-check.js b/test/parallel/test-crypto-keyobject-brand-check.js new file mode 100644 index 00000000000000..ac0cf1b65f709b --- /dev/null +++ b/test/parallel/test-crypto-keyobject-brand-check.js @@ -0,0 +1,96 @@ +'use strict'; + +// KeyObject instances are backed by NativeKeyObject and must be +// recognized by native brand, not by public prototype shape or +// forgeable own properties. + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('node:assert'); +const { + createHmac, + createSecretKey, + generateKeyPairSync, + KeyObject, +} = require('node:crypto'); +const { types: { isKeyObject } } = require('node:util'); + +const invalidThis = { code: 'ERR_INVALID_THIS', name: 'TypeError' }; + +function getter(proto, name) { + return Object.getOwnPropertyDescriptor(proto, name).get; +} + +{ + const secret = createSecretKey(Buffer.alloc(16)); + const { publicKey } = generateKeyPairSync('rsa', { modulusLength: 1024 }); + + const type = getter(KeyObject.prototype, 'type'); + const symmetricKeySize = + getter(Object.getPrototypeOf(secret), 'symmetricKeySize'); + const asymmetricProto = Object.getPrototypeOf(Object.getPrototypeOf(publicKey)); + const asymmetricKeyType = getter(asymmetricProto, 'asymmetricKeyType'); + const asymmetricKeyDetails = getter(asymmetricProto, 'asymmetricKeyDetails'); + + assert.strictEqual(isKeyObject(secret), true); + assert.strictEqual(isKeyObject(publicKey), true); + assert.strictEqual(Object.hasOwn(KeyObject, 'getSlots'), false); + for (const key of [secret, publicKey]) { + for (let proto = Object.getPrototypeOf(key); + proto !== null; + proto = Object.getPrototypeOf(proto)) { + assert.strictEqual(Object.hasOwn(proto, 'getSlots'), false); + assert.strictEqual('getSlots' in proto, false); + if (Object.hasOwn(proto, 'constructor')) { + assert.strictEqual(Object.hasOwn(proto.constructor, 'getSlots'), false); + assert.strictEqual(proto.constructor.getSlots, undefined); + } + } + } + + for (const value of [{}, { __proto__: null }, 1, null, undefined, + Buffer.alloc(1), function() {}]) { + assert.throws(() => type.call(value), invalidThis); + assert.throws(() => symmetricKeySize.call(value), invalidThis); + assert.throws(() => asymmetricKeyType.call(value), invalidThis); + assert.throws(() => asymmetricKeyDetails.call(value), invalidThis); + } + + assert.throws(() => symmetricKeySize.call(publicKey), invalidThis); + assert.throws(() => asymmetricKeyType.call(secret), invalidThis); + assert.throws(() => asymmetricKeyDetails.call(secret), invalidThis); + + const spoofed = {}; + Object.setPrototypeOf(spoofed, Object.getPrototypeOf(secret)); + assert.strictEqual(spoofed instanceof KeyObject, true); + assert.strictEqual(isKeyObject(spoofed), false); + assert.throws(() => type.call(spoofed), invalidThis); + assert.throws(() => symmetricKeySize.call(spoofed), invalidThis); + assert.throws(() => createHmac('sha256', spoofed), { + code: 'ERR_INVALID_ARG_TYPE', + }); + + const originalHasInstance = + Object.getOwnPropertyDescriptor(KeyObject, Symbol.hasInstance); + Object.defineProperty(KeyObject, Symbol.hasInstance, { + configurable: true, + value: () => true, + }); + try { + const buf = Buffer.alloc(16); + assert.strictEqual(buf instanceof KeyObject, true); + assert.strictEqual(isKeyObject(buf), false); + assert.throws(() => type.call(buf), invalidThis); + assert.throws(() => symmetricKeySize.call(buf), invalidThis); + assert.throws(() => asymmetricKeyType.call(buf), invalidThis); + assert.throws(() => asymmetricKeyDetails.call(buf), invalidThis); + } finally { + if (originalHasInstance === undefined) { + delete KeyObject[Symbol.hasInstance]; + } else { + Object.defineProperty(KeyObject, Symbol.hasInstance, originalHasInstance); + } + } +} diff --git a/test/parallel/test-crypto-keyobject-clone-transfer.js b/test/parallel/test-crypto-keyobject-clone-transfer.js new file mode 100644 index 00000000000000..1d68e4b9911a0a --- /dev/null +++ b/test/parallel/test-crypto-keyobject-clone-transfer.js @@ -0,0 +1,138 @@ +'use strict'; + +// KeyObject instances must survive structured cloning with their +// native backing data and hidden JS slot semantics preserved. + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('node:assert'); +const { once } = require('node:events'); +const { + createHmac, + createSecretKey, + generateKeyPairSync, + sign, + verify, +} = require('node:crypto'); +const { MessageChannel, Worker } = require('node:worker_threads'); +const { types: { isKeyObject } } = require('node:util'); + +function assertNoOwnKeys(key) { + assert.deepStrictEqual(Object.getOwnPropertySymbols(key), []); + assert.deepStrictEqual(Object.getOwnPropertyNames(key), []); + assert.deepStrictEqual(Reflect.ownKeys(key), []); +} + +function assertSameKeyObject(original, clone) { + assert.notStrictEqual(original, clone); + assert.strictEqual(isKeyObject(clone), true); + assert.strictEqual(original.type, clone.type); + assert.strictEqual(original.equals(clone), true); + assert.deepStrictEqual(original, clone); + if (clone.type === 'secret') { + assert.strictEqual(original.symmetricKeySize, clone.symmetricKeySize); + } else { + assert.strictEqual(original.asymmetricKeyType, clone.asymmetricKeyType); + assert.deepStrictEqual( + original.asymmetricKeyDetails, + clone.asymmetricKeyDetails); + } + assertNoOwnKeys(original); + assertNoOwnKeys(clone); +} + +async function roundTripViaMessageChannel(key) { + const { port1, port2 } = new MessageChannel(); + port1.postMessage(key); + const [received] = await once(port2, 'message'); + port1.close(); + port2.close(); + return received; +} + +async function roundTripViaWorker(key) { + const worker = new Worker(` + 'use strict'; + const { parentPort } = require('node:worker_threads'); + const { types: { isKeyObject } } = require('node:util'); + + parentPort.once('message', ({ key, expectedType }) => { + try { + if (!isKeyObject(key) || key.type !== expectedType) { + throw new Error('KeyObject slot mismatch in worker'); + } + parentPort.postMessage({ key }); + } catch (err) { + parentPort.postMessage({ error: err.stack || err.message }); + } + }); + `, { eval: true }); + + worker.postMessage({ key, expectedType: key.type }); + const [msg] = await once(worker, 'message'); + await worker.terminate(); + + assert.strictEqual(msg.error, undefined, msg.error); + return msg.key; +} + +function hmacDigest(key) { + return createHmac('sha256', key).update('payload').digest('hex'); +} + +(async () => { + const secret = createSecretKey(Buffer.alloc(16)); + const { publicKey, privateKey } = generateKeyPairSync('rsa', { + modulusLength: 1024, + }); + + for (const key of [secret, publicKey, privateKey]) { + const cloned = structuredClone(key); + assertSameKeyObject(key, cloned); + + const viaPort = await roundTripViaMessageChannel(key); + assertSameKeyObject(key, viaPort); + + const clonedAgain = structuredClone(viaPort); + assertSameKeyObject(key, clonedAgain); + + const viaWorker = await roundTripViaWorker(key); + assertSameKeyObject(key, viaWorker); + } + + const secretClones = [ + secret, + structuredClone(secret), + await roundTripViaMessageChannel(secret), + await roundTripViaWorker(secret), + ]; + const digest = hmacDigest(secret); + for (const key of secretClones) { + assert.strictEqual(hmacDigest(key), digest); + } + + const data = Buffer.from('payload'); + const publicClones = [ + publicKey, + structuredClone(publicKey), + await roundTripViaMessageChannel(publicKey), + await roundTripViaWorker(publicKey), + ]; + const privateClones = [ + privateKey, + structuredClone(privateKey), + await roundTripViaMessageChannel(privateKey), + await roundTripViaWorker(privateKey), + ]; + + const signature = sign('sha256', data, privateKey); + for (const key of publicClones) { + assert.strictEqual(verify('sha256', data, key, signature), true); + } + for (const key of privateClones) { + const clonedSignature = sign('sha256', data, key); + assert.strictEqual(verify('sha256', data, publicKey, clonedSignature), true); + } +})().then(common.mustCall()); diff --git a/test/parallel/test-crypto-keyobject-hidden-slots.js b/test/parallel/test-crypto-keyobject-hidden-slots.js new file mode 100644 index 00000000000000..1ea243ba0ab818 --- /dev/null +++ b/test/parallel/test-crypto-keyobject-hidden-slots.js @@ -0,0 +1,213 @@ +'use strict'; + +// KeyObject public getters and methods are configurable JS properties. +// Internal consumers must read key state through hidden native-backed +// slots, not through user-replaceable accessors. + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('node:assert'); +const { + createCipheriv, + createDecipheriv, + createHmac, + createPrivateKey, + createPublicKey, + createSecretKey, + createSign, + createVerify, + diffieHellman, + generateKeyPairSync, + hkdfSync, + KeyObject, + privateDecrypt, + publicEncrypt, + sign, + verify, + X509Certificate, +} = require('node:crypto'); +const { readFileSync } = require('node:fs'); +const fixtures = require('../common/fixtures'); + +function updateFinal(cipher, data = Buffer.alloc(16)) { + return Buffer.concat([cipher.update(data), cipher.final()]); +} + +{ + const secret = createSecretKey(Buffer.alloc(16)); + const secretProto = Object.getPrototypeOf(secret); + const originalType = + Object.getOwnPropertyDescriptor(KeyObject.prototype, 'type'); + const originalSize = + Object.getOwnPropertyDescriptor(secretProto, 'symmetricKeySize'); + + Object.defineProperty(KeyObject.prototype, 'type', { + configurable: true, + get() { return 'public'; }, + }); + Object.defineProperty(secretProto, 'symmetricKeySize', { + configurable: true, + get() { return 1; }, + }); + + try { + assert.strictEqual(secret.type, 'public'); + assert.strictEqual(secret.symmetricKeySize, 1); + + assert.strictEqual( + createHmac('sha256', secret).update('payload').digest('hex').length, + 64); + + const ciphertext = updateFinal( + createCipheriv('aes-128-ecb', secret, null)); + const plaintext = updateFinal( + createDecipheriv('aes-128-ecb', secret, null), ciphertext); + assert.deepStrictEqual(plaintext, Buffer.alloc(16)); + + assert.strictEqual( + hkdfSync('sha256', secret, Buffer.alloc(0), Buffer.alloc(0), 16) + .byteLength, + 16); + + const cryptoKey = secret.toCryptoKey( + { name: 'AES-GCM' }, true, ['encrypt']); + assert.strictEqual(cryptoKey.algorithm.length, 128); + } finally { + Object.defineProperty(KeyObject.prototype, 'type', originalType); + Object.defineProperty(secretProto, 'symmetricKeySize', originalSize); + } +} + +{ + const { + privateKey: ecPrivateKey, + publicKey, + } = generateKeyPairSync('ec', { namedCurve: 'P-256' }); + const asymmetricProto = Object.getPrototypeOf(Object.getPrototypeOf(publicKey)); + const originalAsymmetricKeyType = + Object.getOwnPropertyDescriptor(asymmetricProto, 'asymmetricKeyType'); + + Object.defineProperty(asymmetricProto, 'asymmetricKeyType', { + configurable: true, + get() { return 'rsa'; }, + }); + + try { + assert.strictEqual(publicKey.asymmetricKeyType, 'rsa'); + assert.strictEqual( + publicKey.export({ format: 'raw-public', type: 'compressed' }).length, + 33); + + assert.strictEqual( + diffieHellman({ privateKey: ecPrivateKey, publicKey }).byteLength, + 32); + } finally { + Object.defineProperty( + asymmetricProto, 'asymmetricKeyType', originalAsymmetricKeyType); + } +} + +{ + const { publicKey } = generateKeyPairSync('rsa', { + modulusLength: 1024, + }); + + const details = publicKey.asymmetricKeyDetails; + assert.strictEqual(details.modulusLength, 1024); + assert.strictEqual(details.publicExponent, 65537n); + + details.modulusLength = 1; + details.publicExponent = 3n; + details.extra = true; + + const freshDetails = publicKey.asymmetricKeyDetails; + assert.notStrictEqual(freshDetails, details); + assert.strictEqual(freshDetails.modulusLength, 1024); + assert.strictEqual(freshDetails.publicExponent, 65537n); + assert.strictEqual(freshDetails.extra, undefined); +} + +{ + Object.defineProperty(Object.prototype, 'publicExponent', { + configurable: true, + value: new Uint8Array([1, 0, 1]), + }); + + try { + const { publicKey } = generateKeyPairSync('ec', { namedCurve: 'P-256' }); + assert.deepStrictEqual(publicKey.asymmetricKeyDetails, { + namedCurve: 'prime256v1', + }); + assert.strictEqual( + Object.hasOwn(publicKey.asymmetricKeyDetails, 'publicExponent'), + false); + } finally { + delete Object.prototype.publicExponent; + } +} + +{ + const { privateKey, publicKey } = generateKeyPairSync('rsa', { + modulusLength: 1024, + }); + const originalType = + Object.getOwnPropertyDescriptor(KeyObject.prototype, 'type'); + const data = Buffer.from('payload'); + + Object.defineProperty(KeyObject.prototype, 'type', { + configurable: true, + get() { return 'secret'; }, + }); + + try { + assert.strictEqual(privateKey.type, 'secret'); + assert.strictEqual(publicKey.type, 'secret'); + + const signature = sign('sha256', data, privateKey); + assert.strictEqual(verify('sha256', data, publicKey, signature), true); + + const signer = createSign('sha256'); + signer.update(data); + const streamSignature = signer.sign(privateKey); + const verifier = createVerify('sha256'); + verifier.update(data); + assert.strictEqual(verifier.verify(publicKey, streamSignature), true); + + const ciphertext = publicEncrypt(publicKey, data); + assert.deepStrictEqual(privateDecrypt(privateKey, ciphertext), data); + + assert.strictEqual(publicKey.equals(createPublicKey(privateKey)), true); + + const x509 = new X509Certificate( + readFileSync(fixtures.path('keys', 'agent1-cert.pem'))); + const x509PrivateKey = createPrivateKey( + readFileSync(fixtures.path('keys', 'agent1-key.pem'))); + const ca = new X509Certificate( + readFileSync(fixtures.path('keys', 'ca1-cert.pem'))); + + assert.strictEqual(x509.checkPrivateKey(x509PrivateKey), true); + assert.strictEqual(x509.verify(ca.publicKey), true); + } finally { + Object.defineProperty(KeyObject.prototype, 'type', originalType); + } +} + +{ + const a = createSecretKey(Buffer.alloc(16)); + const b = createSecretKey(Buffer.alloc(16, 1)); + const originalEquals = + Object.getOwnPropertyDescriptor(KeyObject.prototype, 'equals'); + + Object.defineProperty(KeyObject.prototype, 'equals', { + configurable: true, + value: () => true, + }); + + try { + assert.notDeepStrictEqual(a, b); + } finally { + Object.defineProperty(KeyObject.prototype, 'equals', originalEquals); + } +} diff --git a/test/parallel/test-crypto-keyobject-no-own-symbols.js b/test/parallel/test-crypto-keyobject-no-own-symbols.js new file mode 100644 index 00000000000000..f1539c6a0f7ab5 --- /dev/null +++ b/test/parallel/test-crypto-keyobject-no-own-symbols.js @@ -0,0 +1,42 @@ +'use strict'; + +// KeyObject instances must not expose own string or Symbol properties, even +// after the native slot tuple and lazy metadata cache have been populated. + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('node:assert'); +const { + createSecretKey, + generateKeyPairSync, +} = require('node:crypto'); + +function assertNoOwnKeys(key) { + assert.deepStrictEqual(Object.getOwnPropertySymbols(key), []); + assert.deepStrictEqual(Object.getOwnPropertyNames(key), []); + assert.deepStrictEqual(Reflect.ownKeys(key), []); +} + +{ + const secret = createSecretKey(Buffer.alloc(16)); + const { publicKey, privateKey } = generateKeyPairSync('rsa', { + modulusLength: 1024, + }); + + for (const key of [secret, publicKey, privateKey]) { + const type = key.type; + assert.notStrictEqual(type, undefined); + if (type === 'secret') { + assert.strictEqual(key.symmetricKeySize, 16); + key.export(); + } else { + assert.notStrictEqual(key.asymmetricKeyType, undefined); + assert.notStrictEqual(key.asymmetricKeyDetails, undefined); + key.export({ format: 'pem', type: 'pkcs1' }); + } + key.equals(key); + assertNoOwnKeys(key); + } +} diff --git a/test/parallel/test-webcrypto-cryptokey-brand-check.js b/test/parallel/test-webcrypto-cryptokey-brand-check.js index 5b5c0386693865..3fe8aaa181a226 100644 --- a/test/parallel/test-webcrypto-cryptokey-brand-check.js +++ b/test/parallel/test-webcrypto-cryptokey-brand-check.js @@ -46,6 +46,12 @@ const { subtle } = globalThis.crypto; assert.notStrictEqual(getter.call(key), undefined, `baseline ${name}`); }); assert.strictEqual(isCryptoKey(key), true); + assert.strictEqual(Object.hasOwn(CryptoKey, 'getSlots'), false); + const internalProto = Object.getPrototypeOf(key); + assert.strictEqual(Object.hasOwn(internalProto, 'getSlots'), false); + assert.strictEqual('getSlots' in internalProto, false); + assert.strictEqual(internalProto.constructor, CryptoKey); + assert.strictEqual(Object.getPrototypeOf(internalProto), CryptoKey.prototype); const invalidThis = { code: 'ERR_INVALID_THIS', name: 'TypeError' }; From 046ee73e7637fdeb43ef17ce49202dc4478acbcf Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 4 May 2026 11:53:08 +0200 Subject: [PATCH 2/3] crypto: harden CryptoKey algorithm slots Clone CryptoKey algorithm dictionaries into null-prototype objects before storing or caching them internally. Copy nested hash dictionaries and publicExponent bytes so internal consumers and transferred keys do not observe user-mutable input objects or polluted Object.prototype fields. Keep public algorithm and inspect output as ordinary objects. Make the clone path check only own hash and publicExponent properties. Signed-off-by: Filip Skokan --- lib/internal/crypto/keys.js | 36 ++++++++-- .../test-webcrypto-cryptokey-hidden-slots.js | 65 +++++++++++++++++++ 2 files changed, 97 insertions(+), 4 deletions(-) diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js index fd0fa7185bc113..277eb7db9ca788 100644 --- a/lib/internal/crypto/keys.js +++ b/lib/internal/crypto/keys.js @@ -3,6 +3,7 @@ const { ArrayPrototypeSlice, ObjectDefineProperties, + ObjectPrototypeHasOwnProperty, ObjectSetPrototypeOf, SafeSet, SymbolToStringTag, @@ -928,6 +929,8 @@ function isKeyObject(obj) { // CryptoKey's hidden class pristine. The `getCryptoKey{Type, // Extractable,Algorithm,Usages,Handle}` helpers index into that // array and convert native enums/masks back to Web Crypto strings. +// The internal algorithm object is stored as a null-prototype clone +// so it cannot observe polluted Object.prototype properties. // The public `algorithm` getter caches a cloned dictionary and the // public `usages` getter caches a synthesized array (as Web Crypto // requires repeat reads to return the same object so a consumer's @@ -945,9 +948,27 @@ const kSlotUsages = 7; function cloneAlgorithm(raw) { const cloned = { ...raw }; - if (cloned.hash !== undefined) cloned.hash = { ...cloned.hash }; - if (cloned.publicExponent !== undefined) + if (ObjectPrototypeHasOwnProperty(cloned, 'hash') && + cloned.hash !== undefined) { + cloned.hash = { ...cloned.hash }; + } + if (ObjectPrototypeHasOwnProperty(cloned, 'publicExponent') && + cloned.publicExponent !== undefined) { + cloned.publicExponent = new Uint8Array(cloned.publicExponent); + } + return cloned; +} + +function cloneInternalAlgorithm(raw) { + const cloned = { __proto__: null, ...raw }; + if (ObjectPrototypeHasOwnProperty(cloned, 'hash') && + cloned.hash !== undefined) { + cloned.hash = { __proto__: null, ...cloned.hash }; + } + if (ObjectPrototypeHasOwnProperty(cloned, 'publicExponent') && + cloned.publicExponent !== undefined) { cloned.publicExponent = new Uint8Array(cloned.publicExponent); + } return cloned; } @@ -972,8 +993,8 @@ const { return `CryptoKey ${inspect({ type: getCryptoKeyType(this), extractable: getCryptoKeyExtractable(this), - algorithm: getCryptoKeyAlgorithm(this), - usages: getCryptoKeyUsages(this), + algorithm: cloneAlgorithm(getCryptoKeyAlgorithm(this)), + usages: ArrayPrototypeSlice(getCryptoKeyUsages(this), 0), }, opts)}`; } @@ -1009,6 +1030,12 @@ const { class InternalCryptoKey extends NativeCryptoKey { #slots; + constructor(handle, algorithm, usagesMask, extractable) { + if (algorithm !== undefined) + algorithm = cloneInternalAlgorithm(algorithm); + super(handle, algorithm, usagesMask, extractable); + } + static { getSlots = (key) => { if (!key || typeof key !== 'object') @@ -1018,6 +1045,7 @@ const { if (cached !== undefined) return cached; } const slots = nativeGetCryptoKeySlots(key); + slots[kSlotAlgorithm] = cloneInternalAlgorithm(slots[kSlotAlgorithm]); key.#slots = slots; return slots; }; diff --git a/test/parallel/test-webcrypto-cryptokey-hidden-slots.js b/test/parallel/test-webcrypto-cryptokey-hidden-slots.js index 4e042fef697d67..792a1a59c4c5eb 100644 --- a/test/parallel/test-webcrypto-cryptokey-hidden-slots.js +++ b/test/parallel/test-webcrypto-cryptokey-hidden-slots.js @@ -47,6 +47,71 @@ common.expectWarning({ false, ['sign', 'verify'], ); + const { publicKey: rsaPublicKey } = await subtle.generateKey( + { + name: 'RSA-PSS', + modulusLength: 1024, + publicExponent: new Uint8Array([1, 0, 1]), + hash: 'SHA-256', + }, + true, + ['sign', 'verify'], + ); + + // Public algorithm/usages objects are mutable, but they must be + // separate from the native-backed internal slots. + rsaPublicKey.algorithm.name = 'FORGED-ALGORITHM'; + rsaPublicKey.algorithm.hash.name = 'FORGED-HASH'; + rsaPublicKey.algorithm.publicExponent[0] = 0xff; + rsaPublicKey.usages.push('forged-usage'); + + const clonedRsaPublicKey = structuredClone(rsaPublicKey); + assert.strictEqual(clonedRsaPublicKey.algorithm.name, 'RSA-PSS'); + assert.strictEqual(clonedRsaPublicKey.algorithm.hash.name, 'SHA-256'); + assert.deepStrictEqual( + clonedRsaPublicKey.algorithm.publicExponent, + new Uint8Array([1, 0, 1])); + assert.deepStrictEqual(clonedRsaPublicKey.usages, ['verify']); + + const rsaJwk = await subtle.exportKey('jwk', rsaPublicKey); + assert.strictEqual(rsaJwk.alg, 'PS256'); + assert.deepStrictEqual(rsaJwk.key_ops, ['verify']); + + Object.defineProperties(Object.prototype, { + hash: { + configurable: true, + value: { name: 'FORGED-HASH' }, + }, + publicExponent: { + configurable: true, + value: new Uint8Array([0xff]), + }, + }); + + try { + const aesKey = await subtle.generateKey( + { name: 'AES-GCM', length: 128 }, + true, + ['encrypt'], + ); + assert.deepStrictEqual(aesKey.algorithm, { + name: 'AES-GCM', + length: 128, + }); + assert.strictEqual(Object.hasOwn(aesKey.algorithm, 'hash'), false); + assert.strictEqual( + Object.hasOwn(aesKey.algorithm, 'publicExponent'), + false); + + const clonedAesKey = structuredClone(aesKey); + assert.deepStrictEqual(clonedAesKey.algorithm, { + name: 'AES-GCM', + length: 128, + }); + } finally { + delete Object.prototype.hash; + delete Object.prototype.publicExponent; + } // Snapshot the real values BEFORE tampering. const realType = key.type; From 717e624a6266369a72a880b8257ba34287f98714 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 4 May 2026 11:52:37 +0200 Subject: [PATCH 3/3] tools: prevent lib code from reading KeyObject and CryptoKey accessors Add ESLint rules that reject public KeyObject and CryptoKey accessor reads after internal brand checks. Internal code must use the private key helpers so it reads native-backed slots instead of user-replaceable properties. Add a separate rule that rejects instanceof checks against KeyObject and CryptoKey constructors, including the global CryptoKey constructor. Signed-off-by: Filip Skokan --- lib/eslint.config_partial.mjs | 3 + ...st-eslint-no-cryptokey-public-accessors.js | 88 ++++++ ...slint-no-keyobject-cryptokey-instanceof.js | 78 +++++ ...st-eslint-no-keyobject-public-accessors.js | 85 ++++++ .../no-cryptokey-public-accessors.js | 277 +++++++++++++++++ .../no-keyobject-cryptokey-instanceof.js | 122 ++++++++ .../no-keyobject-public-accessors.js | 284 ++++++++++++++++++ 7 files changed, 937 insertions(+) create mode 100644 test/parallel/test-eslint-no-cryptokey-public-accessors.js create mode 100644 test/parallel/test-eslint-no-keyobject-cryptokey-instanceof.js create mode 100644 test/parallel/test-eslint-no-keyobject-public-accessors.js create mode 100644 tools/eslint-rules/no-cryptokey-public-accessors.js create mode 100644 tools/eslint-rules/no-keyobject-cryptokey-instanceof.js create mode 100644 tools/eslint-rules/no-keyobject-public-accessors.js diff --git a/lib/eslint.config_partial.mjs b/lib/eslint.config_partial.mjs index 3c9bf0bc4e177c..d4c3fd688314c2 100644 --- a/lib/eslint.config_partial.mjs +++ b/lib/eslint.config_partial.mjs @@ -424,6 +424,9 @@ export default [ 'node-core/lowercase-name-for-primitive': 'error', 'node-core/non-ascii-character': 'error', 'node-core/no-array-destructuring': 'error', + 'node-core/no-cryptokey-public-accessors': 'error', + 'node-core/no-keyobject-cryptokey-instanceof': 'error', + 'node-core/no-keyobject-public-accessors': 'error', 'node-core/prefer-primordials': [ 'error', { name: 'AggregateError' }, diff --git a/test/parallel/test-eslint-no-cryptokey-public-accessors.js b/test/parallel/test-eslint-no-cryptokey-public-accessors.js new file mode 100644 index 00000000000000..1b0cd7f930f569 --- /dev/null +++ b/test/parallel/test-eslint-no-cryptokey-public-accessors.js @@ -0,0 +1,88 @@ +'use strict'; + +const common = require('../common'); +common.skipIfEslintMissing(); + +const RuleTester = require('../../tools/eslint/node_modules/eslint').RuleTester; +const rule = require('../../tools/eslint-rules/no-cryptokey-public-accessors'); + +new RuleTester().run('no-cryptokey-public-accessors', rule, { + valid: [ + 'foo.algorithm;', + ` + const { isCryptoKey, getCryptoKeyAlgorithm } = + require('internal/crypto/keys'); + if (isCryptoKey(key)) { + getCryptoKeyAlgorithm(key); + } + `, + ` + const { CryptoKey } = require('internal/crypto/keys'); + class Key extends CryptoKey { + get type() { return 'secret'; } + } + `, + ` + key = webidl.converters.KeyFormat(key); + key.algorithm; + `, + ], + invalid: [ + { + code: ` + const { isCryptoKey } = require('internal/crypto/keys'); + if (isCryptoKey(key)) { + key.type; + } + `, + errors: [{ messageId: 'noPublicAccessor' }], + }, + { + code: ` + const { isCryptoKey: check } = require('internal/crypto/keys'); + if (check(key) && key.extractable) {} + `, + errors: [{ messageId: 'noPublicAccessor' }], + }, + { + code: ` + const { isCryptoKey } = require('internal/crypto/keys'); + if (!isCryptoKey(key)) { + throw new TypeError(); + } + key.algorithm.name; + `, + errors: [{ messageId: 'noPublicAccessor' }], + }, + { + code: ` + const keys = require('internal/crypto/keys'); + if (!keys.isCryptoKey(key)) throw new TypeError(); + key['usages']; + `, + errors: [{ messageId: 'noPublicAccessor' }], + }, + { + code: ` + key = webidl.converters.CryptoKey(key); + key.algorithm; + `, + errors: [{ messageId: 'noPublicAccessor' }], + }, + { + code: ` + const key = webidl.converters.CryptoKey(value); + key.usages; + `, + errors: [{ messageId: 'noPublicAccessor' }], + }, + { + code: ` + class CryptoKey { + inspect() { return this.algorithm; } + } + `, + errors: [{ messageId: 'noPublicAccessor' }], + }, + ], +}); diff --git a/test/parallel/test-eslint-no-keyobject-cryptokey-instanceof.js b/test/parallel/test-eslint-no-keyobject-cryptokey-instanceof.js new file mode 100644 index 00000000000000..29b70c9ec58ab8 --- /dev/null +++ b/test/parallel/test-eslint-no-keyobject-cryptokey-instanceof.js @@ -0,0 +1,78 @@ +'use strict'; + +const common = require('../common'); +common.skipIfEslintMissing(); + +const RuleTester = require('../../tools/eslint/node_modules/eslint').RuleTester; +const rule = require('../../tools/eslint-rules/no-keyobject-cryptokey-instanceof'); + +new RuleTester().run('no-keyobject-cryptokey-instanceof', rule, { + valid: [ + 'key instanceof Buffer;', + 'key instanceof KeyObject;', + ` + const { isKeyObject } = require('internal/crypto/keys'); + isKeyObject(key); + `, + ` + const { isCryptoKey } = require('internal/crypto/keys'); + isCryptoKey(key); + `, + ], + invalid: [ + { + code: ` + const { KeyObject } = require('internal/crypto/keys'); + key instanceof KeyObject; + `, + errors: [{ messageId: 'noKeyObjectInstanceof' }], + }, + { + code: ` + const { KeyObject: KO } = require('internal/crypto/keys'); + key instanceof KO; + `, + errors: [{ messageId: 'noKeyObjectInstanceof' }], + }, + { + code: ` + const keys = require('internal/crypto/keys'); + key instanceof keys.KeyObject; + `, + errors: [{ messageId: 'noKeyObjectInstanceof' }], + }, + { + code: ` + key instanceof CryptoKey; + `, + errors: [{ messageId: 'noCryptoKeyInstanceof' }], + }, + { + code: ` + const { CryptoKey } = require('internal/crypto/keys'); + key instanceof CryptoKey; + `, + errors: [{ messageId: 'noCryptoKeyInstanceof' }], + }, + { + code: ` + const { CryptoKey: CK } = require('internal/crypto/webcrypto'); + key instanceof CK; + `, + errors: [{ messageId: 'noCryptoKeyInstanceof' }], + }, + { + code: ` + const webcrypto = require('internal/crypto/webcrypto'); + key instanceof webcrypto.CryptoKey; + `, + errors: [{ messageId: 'noCryptoKeyInstanceof' }], + }, + { + code: ` + key instanceof globalThis.CryptoKey; + `, + errors: [{ messageId: 'noCryptoKeyInstanceof' }], + }, + ], +}); diff --git a/test/parallel/test-eslint-no-keyobject-public-accessors.js b/test/parallel/test-eslint-no-keyobject-public-accessors.js new file mode 100644 index 00000000000000..2420ae48763995 --- /dev/null +++ b/test/parallel/test-eslint-no-keyobject-public-accessors.js @@ -0,0 +1,85 @@ +'use strict'; + +const common = require('../common'); +common.skipIfEslintMissing(); + +const RuleTester = require('../../tools/eslint/node_modules/eslint').RuleTester; +const rule = require('../../tools/eslint-rules/no-keyobject-public-accessors'); + +new RuleTester().run('no-keyobject-public-accessors', rule, { + valid: [ + 'foo.type;', + ` + const { isKeyObject, getKeyObjectType } = + require('internal/crypto/keys'); + if (isKeyObject(key)) { + getKeyObjectType(key); + } + `, + ` + const { isKeyObject } = require('internal/crypto/keys'); + if (format === 'raw-public') { + key.asymmetricKeyType; + } + `, + ` + const { KeyObject } = require('internal/crypto/keys'); + class Key extends KeyObject { + get type() { return 'secret'; } + } + `, + ], + invalid: [ + { + code: ` + const { isKeyObject } = require('internal/crypto/keys'); + if (isKeyObject(key)) { + key.type; + } + `, + errors: [{ messageId: 'noPublicAccessor' }], + }, + { + code: ` + const { isKeyObject: check } = require('internal/crypto/keys'); + if (check(key) && key.symmetricKeySize === 32) {} + `, + errors: [{ messageId: 'noPublicAccessor' }], + }, + { + code: ` + const { isKeyObject } = require('internal/crypto/keys'); + if (!isKeyObject(otherKeyObject)) { + throw new TypeError(); + } + otherKeyObject.asymmetricKeyType; + `, + errors: [{ messageId: 'noPublicAccessor' }], + }, + { + code: ` + const keys = require('internal/crypto/keys'); + if (!keys.isKeyObject(otherKeyObject)) throw new TypeError(); + otherKeyObject.asymmetricKeyDetails; + `, + errors: [{ messageId: 'noPublicAccessor' }], + }, + { + code: ` + class SecretKeyObject extends KeyObject { + export() { return this.symmetricKeySize; } + } + `, + errors: [{ messageId: 'noPublicAccessor' }], + }, + { + code: ` + const { isKeyObject } = require('internal/crypto/keys'); + if (isKeyObject(key)) { + key.equals(otherKey); + } + `, + errors: [{ messageId: 'noPublicAccessor' }], + }, + ], +}); diff --git a/tools/eslint-rules/no-cryptokey-public-accessors.js b/tools/eslint-rules/no-cryptokey-public-accessors.js new file mode 100644 index 00000000000000..cec9198d740fc3 --- /dev/null +++ b/tools/eslint-rules/no-cryptokey-public-accessors.js @@ -0,0 +1,277 @@ +/** + * @file Prevent internal code from using public CryptoKey accessors. + */ +'use strict'; + +const { isRequireCall, isString } = require('./rules-utils.js'); + +const CRYPTO_KEYS_MODULE = 'internal/crypto/keys'; +const WEBCRYPTO_MODULE = 'internal/crypto/webcrypto'; + +const accessors = new Map([ + ['type', 'getCryptoKeyType(key)'], + ['extractable', 'getCryptoKeyExtractable(key)'], + ['algorithm', 'getCryptoKeyAlgorithm(key)'], + ['usages', 'getCryptoKeyUsages(key)'], +]); + +const cryptoKeyClassNames = new Set([ + 'CryptoKey', + 'InternalCryptoKey', +]); + +function isCryptoKeyModuleRequire(node) { + return node?.type === 'CallExpression' && + isRequireCall(node) && + isString(node.arguments[0]) && + (node.arguments[0].value === CRYPTO_KEYS_MODULE || + node.arguments[0].value === WEBCRYPTO_MODULE); +} + +function getPropertyName(node) { + if (!node) return undefined; + if (node.computed) { + return node.property.type === 'Literal' ? node.property.value : undefined; + } + return node.property.name; +} + +function getIdentifierArgument(node) { + const arg = node.arguments[0]; + return arg?.type === 'Identifier' ? arg.name : undefined; +} + +function isNodeWithin(node, ancestor) { + return node.range[0] >= ancestor.range[0] && + node.range[1] <= ancestor.range[1]; +} + +function exits(statement) { + if (!statement) return false; + switch (statement.type) { + case 'BlockStatement': + return statement.body.length > 0 && exits(statement.body.at(-1)); + case 'ReturnStatement': + case 'ThrowStatement': + return true; + default: + return false; + } +} + +function findStatementInBlock(node) { + let current = node; + while (current?.parent) { + if ((current.parent.type === 'BlockStatement' || + current.parent.type === 'Program') && + current.parent.body.includes(current)) { + return { block: current.parent, statement: current }; + } + current = current.parent; + } +} + +function isWebIDLCryptoKeyConverter(node) { + if (node?.type !== 'CallExpression') return false; + if (node.callee.type !== 'MemberExpression') return false; + if (getPropertyName(node.callee) !== 'CryptoKey') return false; + + const converter = node.callee.object; + return converter?.type === 'MemberExpression' && + getPropertyName(converter) === 'converters'; +} + +module.exports = { + meta: { + messages: { + noPublicAccessor: 'Use `{{replacement}}` instead of the public CryptoKey `{{property}}` accessor.', + }, + schema: [], + }, + + create(context) { + const isCryptoKeyNames = new Set(); + const namespaceNames = new Set(); + const knownCryptoKeyNames = new Set(); + const knownCryptoKeyClassNames = new Set(cryptoKeyClassNames); + + function isIsCryptoKeyCall(node) { + if (node?.type !== 'CallExpression') return false; + + if (node.callee.type === 'Identifier') { + return isCryptoKeyNames.has(node.callee.name); + } + + if (node.callee.type === 'MemberExpression' && + !node.callee.computed && + node.callee.object.type === 'Identifier' && + namespaceNames.has(node.callee.object.name)) { + return node.callee.property.name === 'isCryptoKey'; + } + + return false; + } + + function getConsequentCryptoKeys(test) { + const names = new Set(); + if (isIsCryptoKeyCall(test)) { + names.add(getIdentifierArgument(test)); + } else if (test?.type === 'LogicalExpression' && test.operator === '&&') { + for (const name of getConsequentCryptoKeys(test.left)) { + names.add(name); + } + for (const name of getConsequentCryptoKeys(test.right)) { + names.add(name); + } + } + names.delete(undefined); + return names; + } + + function getAlternateCryptoKeys(test) { + const names = new Set(); + if (test?.type === 'UnaryExpression' && + test.operator === '!' && + isIsCryptoKeyCall(test.argument)) { + names.add(getIdentifierArgument(test.argument)); + } + names.delete(undefined); + return names; + } + + function isCryptoKeyFactory(node) { + if (node?.type === 'NewExpression' && + node.callee.type === 'Identifier') { + return knownCryptoKeyClassNames.has(node.callee.name); + } + + return isWebIDLCryptoKeyConverter(node); + } + + function isInCryptoKeyBranch(name, node) { + for (let current = node.parent; current; current = current.parent) { + if (current.type !== 'IfStatement') continue; + if (isNodeWithin(node, current.consequent) && + getConsequentCryptoKeys(current.test).has(name)) { + return true; + } + if (current.alternate && + isNodeWithin(node, current.alternate) && + getAlternateCryptoKeys(current.test).has(name)) { + return true; + } + } + return false; + } + + function followsExitingCryptoKeyGuard(name, node) { + const location = findStatementInBlock(node); + if (!location) return false; + const index = location.block.body.indexOf(location.statement); + for (let i = 0; i < index; i++) { + const statement = location.block.body[i]; + if (statement.type === 'IfStatement' && + exits(statement.consequent) && + getAlternateCryptoKeys(statement.test).has(name)) { + return true; + } + } + return false; + } + + function followsLogicalCryptoKeyCheck(name, node) { + for (let current = node; current.parent; current = current.parent) { + const parent = current.parent; + if (parent.type !== 'LogicalExpression' || parent.operator !== '&&') { + continue; + } + if (parent.right === current && + getConsequentCryptoKeys(parent.left).has(name)) { + return true; + } + } + return false; + } + + function isInsideCryptoKeyClass(node) { + for (let current = node.parent; current; current = current.parent) { + if (current.type !== 'ClassDeclaration' && + current.type !== 'ClassExpression') { + continue; + } + + const className = current.id?.name; + const superName = current.superClass?.type === 'Identifier' ? + current.superClass.name : undefined; + return knownCryptoKeyClassNames.has(className) || + knownCryptoKeyClassNames.has(superName); + } + return false; + } + + function isKnownCryptoKey(node) { + if (node.type === 'ThisExpression') { + return isInsideCryptoKeyClass(node); + } + + if (node.type !== 'Identifier') return false; + return knownCryptoKeyNames.has(node.name) || + isInCryptoKeyBranch(node.name, node) || + followsLogicalCryptoKeyCheck(node.name, node) || + followsExitingCryptoKeyGuard(node.name, node); + } + + return { + VariableDeclarator(node) { + if (isCryptoKeyModuleRequire(node.init)) { + if (node.id.type === 'Identifier') { + namespaceNames.add(node.id.name); + return; + } + + if (node.id.type !== 'ObjectPattern') return; + + for (const property of node.id.properties) { + if (property.type !== 'Property') continue; + const keyName = property.key.name ?? property.key.value; + if (property.value.type !== 'Identifier') continue; + const localName = property.value.name; + if (keyName === 'isCryptoKey') { + isCryptoKeyNames.add(localName); + } else if (cryptoKeyClassNames.has(keyName)) { + knownCryptoKeyClassNames.add(localName); + } + } + return; + } + + if (node.id.type === 'Identifier' && isCryptoKeyFactory(node.init)) { + knownCryptoKeyNames.add(node.id.name); + } + }, + + AssignmentExpression(node) { + if (node.left.type === 'Identifier' && isCryptoKeyFactory(node.right)) { + knownCryptoKeyNames.add(node.left.name); + } + }, + + MemberExpression(node) { + const property = getPropertyName(node); + const replacement = accessors.get(property); + if (replacement === undefined) return; + if (!isKnownCryptoKey(node.object)) return; + + context.report({ + node: node.property, + messageId: 'noPublicAccessor', + data: { + property, + replacement, + }, + }); + }, + + }; + }, +}; diff --git a/tools/eslint-rules/no-keyobject-cryptokey-instanceof.js b/tools/eslint-rules/no-keyobject-cryptokey-instanceof.js new file mode 100644 index 00000000000000..19e13437247a5e --- /dev/null +++ b/tools/eslint-rules/no-keyobject-cryptokey-instanceof.js @@ -0,0 +1,122 @@ +/** + * @file Prevent internal code from brand-checking keys with instanceof. + */ +'use strict'; + +const { isRequireCall, isString } = require('./rules-utils.js'); + +const CRYPTO_KEYS_MODULE = 'internal/crypto/keys'; +const WEBCRYPTO_MODULE = 'internal/crypto/webcrypto'; + +const keyObjectClassNames = new Set([ + 'KeyObject', + 'SecretKeyObject', + 'AsymmetricKeyObject', + 'PublicKeyObject', + 'PrivateKeyObject', +]); + +const cryptoKeyClassNames = new Set([ + 'CryptoKey', + 'InternalCryptoKey', +]); + +function isKeyModuleRequire(node) { + return node?.type === 'CallExpression' && + isRequireCall(node) && + isString(node.arguments[0]) && + (node.arguments[0].value === CRYPTO_KEYS_MODULE || + node.arguments[0].value === WEBCRYPTO_MODULE); +} + +function getPropertyName(node) { + if (!node) return undefined; + if (node.computed) { + return node.property.type === 'Literal' ? node.property.value : undefined; + } + return node.property.name; +} + +module.exports = { + meta: { + messages: { + noKeyObjectInstanceof: 'Use `isKeyObject(value)` instead of `value instanceof KeyObject`.', + noCryptoKeyInstanceof: 'Use `isCryptoKey(value)` instead of `value instanceof CryptoKey`.', + }, + schema: [], + }, + + create(context) { + const namespaceNames = new Set(); + const keyObjectConstructorNames = new Set(); + const cryptoKeyConstructorNames = new Set(['CryptoKey']); + + function registerRequire(node) { + if (!isKeyModuleRequire(node.init)) return; + + if (node.id.type === 'Identifier') { + namespaceNames.add(node.id.name); + return; + } + + if (node.id.type !== 'ObjectPattern') return; + + for (const property of node.id.properties) { + if (property.type !== 'Property') continue; + const keyName = property.key.name ?? property.key.value; + if (property.value.type !== 'Identifier') continue; + const localName = property.value.name; + if (keyObjectClassNames.has(keyName)) { + keyObjectConstructorNames.add(localName); + } else if (cryptoKeyClassNames.has(keyName)) { + cryptoKeyConstructorNames.add(localName); + } + } + } + + function constructorKind(node) { + if (node.type === 'Identifier') { + if (keyObjectConstructorNames.has(node.name)) return 'KeyObject'; + if (cryptoKeyConstructorNames.has(node.name)) return 'CryptoKey'; + return undefined; + } + + if (node.type !== 'MemberExpression') return undefined; + + const property = getPropertyName(node); + if (node.object.type === 'Identifier') { + if (namespaceNames.has(node.object.name)) { + if (keyObjectClassNames.has(property)) return 'KeyObject'; + if (cryptoKeyClassNames.has(property)) return 'CryptoKey'; + } + if (node.object.name === 'globalThis' && + cryptoKeyClassNames.has(property)) { + return 'CryptoKey'; + } + } + + return undefined; + } + + return { + VariableDeclarator: registerRequire, + + BinaryExpression(node) { + if (node.operator !== 'instanceof') return; + + const kind = constructorKind(node.right); + if (kind === 'KeyObject') { + context.report({ + node, + messageId: 'noKeyObjectInstanceof', + }); + } else if (kind === 'CryptoKey') { + context.report({ + node, + messageId: 'noCryptoKeyInstanceof', + }); + } + }, + }; + }, +}; diff --git a/tools/eslint-rules/no-keyobject-public-accessors.js b/tools/eslint-rules/no-keyobject-public-accessors.js new file mode 100644 index 00000000000000..64b93f4a70e65e --- /dev/null +++ b/tools/eslint-rules/no-keyobject-public-accessors.js @@ -0,0 +1,284 @@ +/** + * @file Prevent internal code from using public KeyObject accessors. + */ +'use strict'; + +const { isRequireCall, isString } = require('./rules-utils.js'); + +const KEYOBJECT_MODULE = 'internal/crypto/keys'; + +const accessors = new Map([ + ['type', 'getKeyObjectType(key)'], + ['symmetricKeySize', 'getKeyObjectSymmetricKeySize(key)'], + ['asymmetricKeyType', 'getKeyObjectAsymmetricKeyType(key)'], + ['asymmetricKeyDetails', 'getKeyObjectAsymmetricKeyDetails(key)'], + ['equals', 'getKeyObjectType(key) and getKeyObjectHandle(key)'], +]); + +const keyObjectClassNames = new Set([ + 'KeyObject', + 'SecretKeyObject', + 'AsymmetricKeyObject', + 'PublicKeyObject', + 'PrivateKeyObject', +]); + +const keyObjectFactoryNames = new Set([ + 'createSecretKey', + 'createPublicKey', + 'createPrivateKey', +]); + +function isInternalCryptoKeysRequire(node) { + return node?.type === 'CallExpression' && + isRequireCall(node) && + isString(node.arguments[0]) && + node.arguments[0].value === KEYOBJECT_MODULE; +} + +function getPropertyName(node) { + if (!node) return undefined; + if (node.computed) { + return node.property.type === 'Literal' ? node.property.value : undefined; + } + return node.property.name; +} + +function getIdentifierArgument(node) { + const arg = node.arguments[0]; + return arg?.type === 'Identifier' ? arg.name : undefined; +} + +function isNodeWithin(node, ancestor) { + return node.range[0] >= ancestor.range[0] && + node.range[1] <= ancestor.range[1]; +} + +function exits(statement) { + if (!statement) return false; + switch (statement.type) { + case 'BlockStatement': + return statement.body.length > 0 && exits(statement.body.at(-1)); + case 'ReturnStatement': + case 'ThrowStatement': + return true; + default: + return false; + } +} + +function findStatementInBlock(node) { + let current = node; + while (current?.parent) { + if ((current.parent.type === 'BlockStatement' || + current.parent.type === 'Program') && + current.parent.body.includes(current)) { + return { block: current.parent, statement: current }; + } + current = current.parent; + } +} + +module.exports = { + meta: { + messages: { + noPublicAccessor: 'Use `{{replacement}}` instead of the public KeyObject `{{property}}` accessor.', + }, + schema: [], + }, + + create(context) { + const isKeyObjectNames = new Set(); + const namespaceNames = new Set(); + const knownKeyObjectNames = new Set(); + const knownKeyObjectClassNames = new Set(keyObjectClassNames); + + function isIsKeyObjectCall(node) { + if (node?.type !== 'CallExpression') return false; + + if (node.callee.type === 'Identifier') { + return isKeyObjectNames.has(node.callee.name); + } + + if (node.callee.type === 'MemberExpression' && + !node.callee.computed && + node.callee.object.type === 'Identifier' && + namespaceNames.has(node.callee.object.name)) { + return node.callee.property.name === 'isKeyObject'; + } + + return false; + } + + function getConsequentKeyObjects(test) { + const names = new Set(); + if (isIsKeyObjectCall(test)) { + names.add(getIdentifierArgument(test)); + } else if (test?.type === 'LogicalExpression' && test.operator === '&&') { + for (const name of getConsequentKeyObjects(test.left)) { + names.add(name); + } + for (const name of getConsequentKeyObjects(test.right)) { + names.add(name); + } + } + names.delete(undefined); + return names; + } + + function getAlternateKeyObjects(test) { + const names = new Set(); + if (test?.type === 'UnaryExpression' && + test.operator === '!' && + isIsKeyObjectCall(test.argument)) { + names.add(getIdentifierArgument(test.argument)); + } + names.delete(undefined); + return names; + } + + function isKeyObjectFactory(node) { + if (node?.type === 'NewExpression' && + node.callee.type === 'Identifier') { + return knownKeyObjectClassNames.has(node.callee.name); + } + + if (node?.type !== 'CallExpression') return false; + + if (node.callee.type === 'Identifier') { + return keyObjectFactoryNames.has(node.callee.name); + } + + if (node.callee.type !== 'MemberExpression') return false; + const object = node.callee.object; + const property = getPropertyName(node.callee); + if (object.type === 'Identifier' && + knownKeyObjectClassNames.has(object.name)) { + return property === 'from'; + } + return object.type === 'Identifier' && + namespaceNames.has(object.name) && + keyObjectFactoryNames.has(property); + } + + function isInKeyObjectBranch(name, node) { + for (let current = node.parent; current; current = current.parent) { + if (current.type !== 'IfStatement') continue; + if (isNodeWithin(node, current.consequent) && + getConsequentKeyObjects(current.test).has(name)) { + return true; + } + if (current.alternate && + isNodeWithin(node, current.alternate) && + getAlternateKeyObjects(current.test).has(name)) { + return true; + } + } + return false; + } + + function followsExitingKeyObjectGuard(name, node) { + const location = findStatementInBlock(node); + if (!location) return false; + const index = location.block.body.indexOf(location.statement); + for (let i = 0; i < index; i++) { + const statement = location.block.body[i]; + if (statement.type === 'IfStatement' && + exits(statement.consequent) && + getAlternateKeyObjects(statement.test).has(name)) { + return true; + } + } + return false; + } + + function followsLogicalKeyObjectCheck(name, node) { + for (let current = node; current.parent; current = current.parent) { + const parent = current.parent; + if (parent.type !== 'LogicalExpression' || parent.operator !== '&&') { + continue; + } + if (parent.right === current && + getConsequentKeyObjects(parent.left).has(name)) { + return true; + } + } + return false; + } + + function isInsideKeyObjectClass(node) { + for (let current = node.parent; current; current = current.parent) { + if (current.type !== 'ClassDeclaration' && + current.type !== 'ClassExpression') { + continue; + } + + const className = current.id?.name; + const superName = current.superClass?.type === 'Identifier' ? + current.superClass.name : undefined; + return knownKeyObjectClassNames.has(className) || + knownKeyObjectClassNames.has(superName); + } + return false; + } + + function isKnownKeyObject(node) { + if (node.type === 'ThisExpression') { + return isInsideKeyObjectClass(node); + } + + if (node.type !== 'Identifier') return false; + return knownKeyObjectNames.has(node.name) || + isInKeyObjectBranch(node.name, node) || + followsLogicalKeyObjectCheck(node.name, node) || + followsExitingKeyObjectGuard(node.name, node); + } + + return { + VariableDeclarator(node) { + if (isInternalCryptoKeysRequire(node.init)) { + if (node.id.type === 'Identifier') { + namespaceNames.add(node.id.name); + return; + } + + if (node.id.type !== 'ObjectPattern') return; + + for (const property of node.id.properties) { + if (property.type !== 'Property') continue; + const keyName = property.key.name ?? property.key.value; + if (property.value.type !== 'Identifier') continue; + const localName = property.value.name; + if (keyName === 'isKeyObject') { + isKeyObjectNames.add(localName); + } else if (keyObjectClassNames.has(keyName)) { + knownKeyObjectClassNames.add(localName); + } + } + return; + } + + if (node.id.type === 'Identifier' && isKeyObjectFactory(node.init)) { + knownKeyObjectNames.add(node.id.name); + } + }, + + MemberExpression(node) { + const property = getPropertyName(node); + const replacement = accessors.get(property); + if (replacement === undefined) return; + if (!isKnownKeyObject(node.object)) return; + + context.report({ + node: node.property, + messageId: 'noPublicAccessor', + data: { + property, + replacement, + }, + }); + }, + + }; + }, +};