From b9a06a085b13b12c3610da1a83af7ec96e72c504 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Wed, 14 Dec 2022 11:04:15 +0100 Subject: [PATCH 1/3] crypto: fix CryptoKey prototype WPT --- lib/internal/crypto/keys.js | 65 ++++++++++++++++--------------- test/wpt/status/WebCryptoAPI.json | 1 - 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js index 5e4ab819af5b3d..968116d6bf2ea7 100644 --- a/lib/internal/crypto/keys.js +++ b/lib/internal/crypto/keys.js @@ -56,7 +56,7 @@ const { } = require('internal/util/types'); const { - JSTransferable, + makeTransferable, kClone, kDeserialize, } = require('internal/worker/js_transferable'); @@ -630,14 +630,12 @@ function isKeyObject(obj) { } // Our implementation of CryptoKey is a simple wrapper around a KeyObject -// that adapts it to the standard interface. This implementation also -// extends the JSTransferable class, allowing the CryptoKey to be cloned -// to Workers. +// that adapts it to the standard interface. // TODO(@jasnell): Embedder environments like electron may have issues // here similar to other things like URL. A chromium provided CryptoKey // will not be recognized as a Node.js CryptoKey, and vice versa. It // would be fantastic if we could find a way of making those interop. -class CryptoKey extends JSTransferable { +class CryptoKey { constructor() { throw new ERR_ILLEGAL_CONSTRUCTOR(); } @@ -682,30 +680,6 @@ class CryptoKey extends JSTransferable { throw new ERR_INVALID_THIS('CryptoKey'); return ArrayFrom(this[kKeyUsages]); } - - [kClone]() { - const keyObject = this[kKeyObject]; - const algorithm = this.algorithm; - const extractable = this.extractable; - const usages = this.usages; - - return { - data: { - keyObject, - algorithm, - usages, - extractable, - }, - deserializeInfo: 'internal/crypto/keys:InternalCryptoKey' - }; - } - - [kDeserialize]({ keyObject, algorithm, usages, extractable }) { - this[kKeyObject] = keyObject; - this[kAlgorithm] = algorithm; - this[kKeyUsages] = usages; - this[kExtractable] = extractable; - } } ObjectDefineProperties(CryptoKey.prototype, { @@ -718,13 +692,14 @@ ObjectDefineProperties(CryptoKey.prototype, { // All internal code must use new InternalCryptoKey to create // CryptoKey instances. The CryptoKey class is exposed to end // user code but is not permitted to be constructed directly. -class InternalCryptoKey extends JSTransferable { +// Using makeTransferable also allows the CryptoKey to be +// cloned to Workers. +class InternalCryptoKey { constructor( keyObject, algorithm, keyUsages, extractable) { - super(); // Using symbol properties here currently instead of private // properties because (for now) the performance penalty of // private fields is still too high. @@ -732,9 +707,35 @@ class InternalCryptoKey extends JSTransferable { this[kAlgorithm] = algorithm; this[kExtractable] = extractable; this[kKeyUsages] = keyUsages; + + // eslint-disable-next-line no-constructor-return + return makeTransferable(this); } -} + [kClone]() { + const keyObject = this[kKeyObject]; + const algorithm = this.algorithm; + const extractable = this.extractable; + const usages = this.usages; + + return { + data: { + keyObject, + algorithm, + usages, + extractable, + }, + deserializeInfo: 'internal/crypto/keys:InternalCryptoKey' + }; + } + + [kDeserialize]({ keyObject, algorithm, usages, extractable }) { + this[kKeyObject] = keyObject; + this[kAlgorithm] = algorithm; + this[kKeyUsages] = usages; + this[kExtractable] = extractable; + } +} InternalCryptoKey.prototype.constructor = CryptoKey; ObjectSetPrototypeOf(InternalCryptoKey.prototype, CryptoKey.prototype); diff --git a/test/wpt/status/WebCryptoAPI.json b/test/wpt/status/WebCryptoAPI.json index 961d8b515d702f..0eee00e0705e34 100644 --- a/test/wpt/status/WebCryptoAPI.json +++ b/test/wpt/status/WebCryptoAPI.json @@ -23,7 +23,6 @@ "expected": [ "Crypto interface: existence and properties of interface object", "CryptoKey interface: existence and properties of interface object", - "CryptoKey interface: existence and properties of interface prototype object", "Window interface: attribute crypto" ] } From 7107b015449d0d4886b456d4745b4111d2d865cf Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Wed, 14 Dec 2022 11:51:31 +0100 Subject: [PATCH 2/3] test: add all WebCryptoAPI globals to WPTRunner's loadLazyGlobals --- test/common/wpt.js | 2 +- test/wpt/status/WebCryptoAPI.json | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/test/common/wpt.js b/test/common/wpt.js index 5e3f07a86db143..dcf5d943416950 100644 --- a/test/common/wpt.js +++ b/test/common/wpt.js @@ -397,7 +397,7 @@ class WPTRunner { 'CompressionStream', 'DecompressionStream', ]; if (Boolean(process.versions.openssl) && !process.env.NODE_SKIP_CRYPTO) { - lazyProperties.push('crypto'); + lazyProperties.push('crypto', 'Crypto', 'CryptoKey', 'SubtleCrypto'); } const script = lazyProperties.map((name) => `globalThis.${name};`).join('\n'); this.globalThisInitScripts.push(script); diff --git a/test/wpt/status/WebCryptoAPI.json b/test/wpt/status/WebCryptoAPI.json index 0eee00e0705e34..78a79a71920c6f 100644 --- a/test/wpt/status/WebCryptoAPI.json +++ b/test/wpt/status/WebCryptoAPI.json @@ -21,7 +21,6 @@ "idlharness.https.any.js": { "fail": { "expected": [ - "Crypto interface: existence and properties of interface object", "CryptoKey interface: existence and properties of interface object", "Window interface: attribute crypto" ] From dc6e9a8c323be689856087b5ba31bccc945779bb Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 16 Dec 2022 13:32:22 +0100 Subject: [PATCH 3/3] crypto: fix globalThis.crypto this check --- lib/internal/process/pre_execution.js | 11 ++++++++++- lib/internal/util.js | 5 ++++- test/wpt/status/WebCryptoAPI.json | 8 -------- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index c3e84854ccefdd..4c44927fb0c15a 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -23,6 +23,7 @@ const { } = require('internal/util'); const { + ERR_INVALID_THIS, ERR_MANIFEST_ASSERT_INTEGRITY, ERR_NO_CRYPTO, } = require('internal/errors').codes; @@ -278,7 +279,15 @@ function setupWebCrypto() { if (internalBinding('config').hasOpenSSL) { defineReplaceableLazyAttribute( - globalThis, 'internal/crypto/webcrypto', ['crypto'], false + globalThis, + 'internal/crypto/webcrypto', + ['crypto'], + false, + function cryptoThisCheck() { + if (this !== globalThis && this != null) + throw new ERR_INVALID_THIS( + 'nullish or must be the global object'); + } ); exposeLazyInterfaces( globalThis, 'internal/crypto/webcrypto', diff --git a/lib/internal/util.js b/lib/internal/util.js index 38002b421376ec..0d769e061fe448 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -552,7 +552,7 @@ function defineLazyProperties(target, id, keys, enumerable = true) { ObjectDefineProperties(target, descriptors); } -function defineReplaceableLazyAttribute(target, id, keys, writable = true) { +function defineReplaceableLazyAttribute(target, id, keys, writable = true, check) { let mod; for (let i = 0; i < keys.length; i++) { const key = keys[i]; @@ -560,6 +560,9 @@ function defineReplaceableLazyAttribute(target, id, keys, writable = true) { let setterCalled = false; function get() { + if (check !== undefined) { + FunctionPrototypeCall(check, this); + } if (setterCalled) { return value; } diff --git a/test/wpt/status/WebCryptoAPI.json b/test/wpt/status/WebCryptoAPI.json index 78a79a71920c6f..bd1db9eba685b7 100644 --- a/test/wpt/status/WebCryptoAPI.json +++ b/test/wpt/status/WebCryptoAPI.json @@ -17,13 +17,5 @@ }, "historical.any.js": { "skip": "Not relevant in Node.js context" - }, - "idlharness.https.any.js": { - "fail": { - "expected": [ - "CryptoKey interface: existence and properties of interface object", - "Window interface: attribute crypto" - ] - } } }