From 6de2673a9f9420e87066422375278d0156079889 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 22 Feb 2022 23:25:59 +0100 Subject: [PATCH] lib: enable global WebCrypto by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enables `--experimental-global-webcrypto` by default, and ensures that the classic `node:crypto` core module is still available in `--eval` or `--print` contexts. PR-URL: https://github.com/nodejs/node/pull/42083 Reviewed-By: Matteo Collina Reviewed-By: Filip Skokan Reviewed-By: Michaël Zasso --- .eslintrc.js | 1 + doc/api/cli.md | 20 +++--- doc/api/globals.md | 34 +++++++--- doc/node.1 | 3 + lib/internal/main/eval_string.js | 28 +++++++- lib/internal/process/pre_execution.js | 27 +++++--- src/node_options.cc | 3 +- src/node_options.h | 2 +- test/common/index.js | 4 +- .../known_issues/test-cli-print-var-crypto.js | 21 ++++++ test/parallel/test-assert-checktag.js | 7 +- test/parallel/test-bootstrap-modules.js | 11 ++++ test/parallel/test-cli-eval.js | 66 +++++++++++++++++++ .../parallel/test-global-webcrypto-classes.js | 2 +- .../parallel/test-global-webcrypto-disbled.js | 10 +++ test/parallel/test-global.js | 1 + 16 files changed, 203 insertions(+), 37 deletions(-) create mode 100644 test/known_issues/test-cli-print-var-crypto.js create mode 100644 test/parallel/test-global-webcrypto-disbled.js diff --git a/.eslintrc.js b/.eslintrc.js index 1dedcd1bd76e15..2a10875dd34ebd 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -322,6 +322,7 @@ module.exports = { CompressionStream: 'readable', CountQueuingStrategy: 'readable', CustomEvent: 'readable', + crypto: 'readable', Crypto: 'readable', CryptoKey: 'readable', DecompressionStream: 'readable', diff --git a/doc/api/cli.md b/doc/api/cli.md index 18daed918dafe0..b1b0103cfddddb 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -351,16 +351,6 @@ added: Expose the [CustomEvent Web API][] on the global scope. -### `--experimental-global-webcrypto` - - - -Expose the [Web Crypto API][] on the global scope. - ### `--experimental-import-meta-resolve` + +Disable exposition of [Web Crypto API][] on the global scope. + ### `--no-experimental-repl-await` -> Stability: 1 - Experimental. Enable this API with the -> [`--experimental-global-webcrypto`][] CLI flag. +> Stability: 1 - Experimental. Disable this API with the +> [`--no-experimental-global-webcrypto`][] CLI flag. A browser-compatible implementation of {Crypto}. This global is available only if the Node.js binary was compiled with including support for the @@ -360,10 +364,14 @@ only if the Node.js binary was compiled with including support for the added: - v17.6.0 - v16.15.0 +changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/42083 + description: No longer behind `--experimental-global-webcrypto` CLI flag. --> -> Stability: 1 - Experimental. Enable this API with the -> [`--experimental-global-webcrypto`][] CLI flag. +> Stability: 1 - Experimental. Disable this API with the +> [`--no-experimental-global-webcrypto`][] CLI flag. A browser-compatible implementation of the [Web Crypto API][]. @@ -373,10 +381,14 @@ A browser-compatible implementation of the [Web Crypto API][]. added: - v17.6.0 - v16.15.0 +changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/42083 + description: No longer behind `--experimental-global-webcrypto` CLI flag. --> -> Stability: 1 - Experimental. Enable this API with the -> [`--experimental-global-webcrypto`][] CLI flag. +> Stability: 1 - Experimental. Disable this API with the +> [`--no-experimental-global-webcrypto`][] CLI flag. A browser-compatible implementation of {CryptoKey}. This global is available only if the Node.js binary was compiled with including support for the @@ -725,10 +737,14 @@ The WHATWG [`structuredClone`][] method. added: - v17.6.0 - v16.15.0 +changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/42083 + description: No longer behind `--experimental-global-webcrypto` CLI flag. --> -> Stability: 1 - Experimental. Enable this API with the -> [`--experimental-global-webcrypto`][] CLI flag. +> Stability: 1 - Experimental. Disable this API with the +> [`--no-experimental-global-webcrypto`][] CLI flag. A browser-compatible implementation of {SubtleCrypto}. This global is available only if the Node.js binary was compiled with including support for the @@ -870,8 +886,8 @@ A browser-compatible implementation of [`WritableStreamDefaultWriter`][]. [Web Crypto API]: webcrypto.md [`--experimental-global-customevent`]: cli.md#--experimental-global-customevent -[`--experimental-global-webcrypto`]: cli.md#--experimental-global-webcrypto [`--no-experimental-fetch`]: cli.md#--no-experimental-fetch +[`--no-experimental-global-webcrypto`]: cli.md#--no-experimental-global-webcrypto [`AbortController`]: https://developer.mozilla.org/en-US/docs/Web/API/AbortController [`ByteLengthQueuingStrategy`]: webstreams.md#class-bytelengthqueuingstrategy [`CompressionStream`]: webstreams.md#class-compressionstream diff --git a/doc/node.1 b/doc/node.1 index 50a80742c458f9..3e2602673a83e4 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -165,6 +165,9 @@ Use this flag to enable ShadowRealm support. .It Fl -no-experimental-fetch Disable experimental support for the Fetch API. . +.It Fl -no-experimental-global-webcrypto +Disable exposition of the Web Crypto API on the global scope. +. .It Fl -no-experimental-repl-await Disable top-level await keyword support in REPL. . diff --git a/lib/internal/main/eval_string.js b/lib/internal/main/eval_string.js index e7baef452632dc..e92e088b8e0ceb 100644 --- a/lib/internal/main/eval_string.js +++ b/lib/internal/main/eval_string.js @@ -4,6 +4,8 @@ // `--interactive`. const { + ObjectDefineProperty, + RegExpPrototypeExec, globalThis, } = primordials; @@ -25,9 +27,31 @@ const print = getOptionValue('--print'); const loadESM = getOptionValue('--import').length > 0; if (getOptionValue('--input-type') === 'module') evalModule(source, print); -else +else { + // For backward compatibility, we want the identifier crypto to be the + // `node:crypto` module rather than WebCrypto. + const isUsingCryptoIdentifier = + getOptionValue('--experimental-global-webcrypto') && + RegExpPrototypeExec(/\bcrypto\b/, source) !== null; + const shouldDefineCrypto = isUsingCryptoIdentifier && internalBinding('config').hasOpenSSL; + + if (isUsingCryptoIdentifier && !shouldDefineCrypto) { + // This is taken from `addBuiltinLibsToObject`. + const object = globalThis; + const name = 'crypto'; + const setReal = (val) => { + // Deleting the property before re-assigning it disables the + // getter/setter mechanism. + delete object[name]; + object[name] = val; + }; + ObjectDefineProperty(object, name, { __proto__: null, set: setReal }); + } evalScript('[eval]', - source, + shouldDefineCrypto ? ( + print ? `let crypto=require("node:crypto");{${source}}` : `(crypto=>{{${source}}})(require('node:crypto'))` + ) : source, getOptionValue('--inspect-brk'), print, loadESM); +} diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index 23d4dbcf6cd8e1..6ad5502b61ded1 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -25,6 +25,7 @@ const { const { ERR_MANIFEST_ASSERT_INTEGRITY, + ERR_NO_CRYPTO, } = require('internal/errors').codes; const assert = require('internal/assert'); @@ -247,23 +248,29 @@ function setupFetch() { // removed. function setupWebCrypto() { if (process.config.variables.node_no_browser_globals || - !getOptionValue('--experimental-global-webcrypto')) { + getOptionValue('--no-experimental-global-webcrypto')) { return; } - let webcrypto; - ObjectDefineProperty(globalThis, 'crypto', - { __proto__: null, ...ObjectGetOwnPropertyDescriptor({ - get crypto() { - webcrypto ??= require('internal/crypto/webcrypto'); - return webcrypto.crypto; - } - }, 'crypto') }); if (internalBinding('config').hasOpenSSL) { - webcrypto ??= require('internal/crypto/webcrypto'); + const webcrypto = require('internal/crypto/webcrypto'); + ObjectDefineProperty(globalThis, 'crypto', + { __proto__: null, ...ObjectGetOwnPropertyDescriptor({ + get crypto() { + return webcrypto.crypto; + } + }, 'crypto') }); exposeInterface(globalThis, 'Crypto', webcrypto.Crypto); exposeInterface(globalThis, 'CryptoKey', webcrypto.CryptoKey); exposeInterface(globalThis, 'SubtleCrypto', webcrypto.SubtleCrypto); + } else { + ObjectDefineProperty(globalThis, 'crypto', + { __proto__: null, ...ObjectGetOwnPropertyDescriptor({ + get crypto() { + throw new ERR_NO_CRYPTO(); + } + }, 'crypto') }); + } } diff --git a/src/node_options.cc b/src/node_options.cc index d25cc36feaa469..e5daab80196ae3 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -370,7 +370,8 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { AddOption("--experimental-global-webcrypto", "expose experimental Web Crypto API on the global scope", &EnvironmentOptions::experimental_global_web_crypto, - kAllowedInEnvironment); + kAllowedInEnvironment, + true); AddOption("--experimental-json-modules", "", NoOp{}, kAllowedInEnvironment); AddOption("--experimental-loader", "use the specified module as a custom loader", diff --git a/src/node_options.h b/src/node_options.h index 004bcae837c406..bd0799829a9aa4 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -110,7 +110,7 @@ class EnvironmentOptions : public Options { bool enable_source_maps = false; bool experimental_fetch = true; bool experimental_global_customevent = false; - bool experimental_global_web_crypto = false; + bool experimental_global_web_crypto = true; bool experimental_https_modules = false; std::string experimental_specifier_resolution; bool experimental_wasm_modules = false; diff --git a/test/common/index.js b/test/common/index.js index 310f0c30dc933b..b3c063b57ab41c 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -357,7 +357,9 @@ if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') { const leaked = []; for (const val in global) { - if (!knownGlobals.includes(global[val])) { + // globalThis.crypto is a getter that throws if Node.js was compiled + // without OpenSSL. + if (val !== 'crypto' && !knownGlobals.includes(global[val])) { leaked.push(val); } } diff --git a/test/known_issues/test-cli-print-var-crypto.js b/test/known_issues/test-cli-print-var-crypto.js new file mode 100644 index 00000000000000..bd81f8102b1b08 --- /dev/null +++ b/test/known_issues/test-cli-print-var-crypto.js @@ -0,0 +1,21 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); + +if (!common.hasCrypto) { + assert.fail('When Node.js is compiled without OpenSSL, overriding the global ' + + 'crypto is allowed on string eval'); +} + +const child = require('child_process'); +const nodejs = `"${process.execPath}"`; + +// Trying to define a variable named `crypto` using `var` triggers an exception. + +child.exec( + `${nodejs} ` + + '-p "var crypto = {randomBytes:1};typeof crypto.randomBytes"', + common.mustSucceed((stdout) => { + assert.match(stdout, /^number/); + })); diff --git a/test/parallel/test-assert-checktag.js b/test/parallel/test-assert-checktag.js index c0b80adffcaa1a..a8b076049b61e9 100644 --- a/test/parallel/test-assert-checktag.js +++ b/test/parallel/test-assert-checktag.js @@ -1,5 +1,10 @@ 'use strict'; -require('../common'); +const common = require('../common'); + +if (!common.hasCrypto) { + common.skip('missing crypto'); +} + const assert = require('assert'); // Disable colored output to prevent color codes from breaking assertion diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index e2fc58cdf26f57..88a43b51473ff2 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -206,6 +206,17 @@ if (common.hasIntl) { expectedModules.add('NativeModule url'); } +if (common.hasCrypto) { + expectedModules.add('Internal Binding crypto') + .add('NativeModule internal/crypto/hash') + .add('NativeModule internal/crypto/hashnames') + .add('NativeModule internal/crypto/keys') + .add('NativeModule internal/crypto/random') + .add('NativeModule internal/crypto/util') + .add('NativeModule internal/crypto/webcrypto') + .add('NativeModule internal/streams/lazy_transform'); +} + if (process.features.inspector) { expectedModules.add('Internal Binding inspector'); expectedModules.add('NativeModule internal/inspector_async_hook'); diff --git a/test/parallel/test-cli-eval.js b/test/parallel/test-cli-eval.js index e95fee008d6e3a..dcc2b8fcde3ca4 100644 --- a/test/parallel/test-cli-eval.js +++ b/test/parallel/test-cli-eval.js @@ -288,3 +288,69 @@ child.exec( common.mustSucceed((stdout) => { assert.strictEqual(stdout, '.mjs file\n'); })); + +if (common.hasCrypto) { + // Assert that calls to crypto utils work without require. + child.exec( + `${nodejs} ` + + '-e "console.log(crypto.randomBytes(16).toString(\'hex\'))"', + common.mustSucceed((stdout) => { + assert.match(stdout, /[0-9a-f]{32}/i); + })); + child.exec( + `${nodejs} ` + + '-p "crypto.randomBytes(16).toString(\'hex\')"', + common.mustSucceed((stdout) => { + assert.match(stdout, /[0-9a-f]{32}/i); + })); +} +// Assert that overriding crypto works. +child.exec( + `${nodejs} ` + + '-p "crypto=Symbol(\'test\')"', + common.mustSucceed((stdout) => { + assert.match(stdout, /Symbol\(test\)/i); + })); +child.exec( + `${nodejs} ` + + '-e "crypto = {};console.log(\'randomBytes\', typeof crypto.randomBytes)"', + common.mustSucceed((stdout) => { + assert.match(stdout, /randomBytes\sundefined/); + })); +// Assert that overriding crypto with a local variable works. +child.exec( + `${nodejs} ` + + '-e "const crypto = {};console.log(\'randomBytes\', typeof crypto.randomBytes)"', + common.mustSucceed((stdout) => { + assert.match(stdout, /randomBytes\sundefined/); + })); +child.exec( + `${nodejs} ` + + '-e "let crypto = {};console.log(\'randomBytes\', typeof crypto.randomBytes)"', + common.mustSucceed((stdout) => { + assert.match(stdout, /randomBytes\sundefined/); + })); +child.exec( + `${nodejs} ` + + '-e "var crypto = {};console.log(\'randomBytes\', typeof crypto.randomBytes)"', + common.mustSucceed((stdout) => { + assert.match(stdout, /randomBytes\sundefined/); + })); +child.exec( + `${nodejs} ` + + '-p "const crypto = {randomBytes:1};typeof crypto.randomBytes"', + common.mustSucceed((stdout) => { + assert.match(stdout, /^number/); + })); +child.exec( + `${nodejs} ` + + '-p "let crypto = {randomBytes:1};typeof crypto.randomBytes"', + common.mustSucceed((stdout) => { + assert.match(stdout, /^number/); + })); +child.exec( + `${nodejs} --no-experimental-global-webcrypto ` + + '-p "var crypto = {randomBytes:1};typeof crypto.randomBytes"', + common.mustSucceed((stdout) => { + assert.match(stdout, /^number/); + })); diff --git a/test/parallel/test-global-webcrypto-classes.js b/test/parallel/test-global-webcrypto-classes.js index 083592bd92278c..ac42ff1d82e2d1 100644 --- a/test/parallel/test-global-webcrypto-classes.js +++ b/test/parallel/test-global-webcrypto-classes.js @@ -1,4 +1,4 @@ -// Flags: --experimental-global-webcrypto --expose-internals +// Flags: --expose-internals 'use strict'; const common = require('../common'); diff --git a/test/parallel/test-global-webcrypto-disbled.js b/test/parallel/test-global-webcrypto-disbled.js new file mode 100644 index 00000000000000..ebbb4afa9a03c5 --- /dev/null +++ b/test/parallel/test-global-webcrypto-disbled.js @@ -0,0 +1,10 @@ +// Flags: --no-experimental-global-webcrypto +'use strict'; + +require('../common'); +const assert = require('assert'); + +assert.strictEqual(typeof crypto, 'undefined'); +assert.strictEqual(typeof Crypto, 'undefined'); +assert.strictEqual(typeof CryptoKey, 'undefined'); +assert.strictEqual(typeof SubtleCrypto, 'undefined'); diff --git a/test/parallel/test-global.js b/test/parallel/test-global.js index 5437751cbcc520..9ac9b4f7287327 100644 --- a/test/parallel/test-global.js +++ b/test/parallel/test-global.js @@ -57,6 +57,7 @@ builtinModules.forEach((moduleName) => { 'setTimeout', 'structuredClone', 'fetch', + 'crypto', ]; assert.deepStrictEqual(new Set(Object.keys(global)), new Set(expected)); }