Skip to content

Commit

Permalink
lib: enable global WebCrypto by default
Browse files Browse the repository at this point in the history
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: #42083
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
aduh95 committed Sep 17, 2022
1 parent 572d556 commit 6de2673
Show file tree
Hide file tree
Showing 16 changed files with 203 additions and 37 deletions.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ module.exports = {
CompressionStream: 'readable',
CountQueuingStrategy: 'readable',
CustomEvent: 'readable',
crypto: 'readable',
Crypto: 'readable',
CryptoKey: 'readable',
DecompressionStream: 'readable',
Expand Down
20 changes: 9 additions & 11 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -351,16 +351,6 @@ added:

Expose the [CustomEvent Web API][] on the global scope.

### `--experimental-global-webcrypto`

<!-- YAML
added:
- v17.6.0
- v16.15.0
-->

Expose the [Web Crypto API][] on the global scope.

### `--experimental-import-meta-resolve`

<!-- YAML
Expand Down Expand Up @@ -413,6 +403,14 @@ added: v18.0.0

Disable experimental support for the [Fetch API][].

### `--no-experimental-global-webcrypto`

<!-- YAML
added: REPLACEME
-->

Disable exposition of [Web Crypto API][] on the global scope.

### `--no-experimental-repl-await`

<!-- YAML
Expand Down Expand Up @@ -1839,7 +1837,6 @@ Node.js options that are allowed are:
* `--enable-source-maps`
* `--experimental-abortcontroller`
* `--experimental-global-customevent`
* `--experimental-global-webcrypto`
* `--experimental-import-meta-resolve`
* `--experimental-json-modules`
* `--experimental-loader`
Expand Down Expand Up @@ -1872,6 +1869,7 @@ Node.js options that are allowed are:
* `--no-addons`
* `--no-deprecation`
* `--no-experimental-fetch`
* `--no-experimental-global-webcrypto`
* `--no-experimental-repl-await`
* `--no-extra-info-on-fatal-exception`
* `--no-force-async-hooks-checks`
Expand Down
34 changes: 25 additions & 9 deletions doc/api/globals.md
Original file line number Diff line number Diff line change
Expand Up @@ -345,10 +345,14 @@ A browser-compatible implementation of [`CountQueuingStrategy`][].
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 {Crypto}. This global is available
only if the Node.js binary was compiled with including support for the
Expand All @@ -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][].

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -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.
.
Expand Down
28 changes: 26 additions & 2 deletions lib/internal/main/eval_string.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
// `--interactive`.

const {
ObjectDefineProperty,
RegExpPrototypeExec,
globalThis,
} = primordials;

Expand All @@ -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);
}
27 changes: 17 additions & 10 deletions lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const {

const {
ERR_MANIFEST_ASSERT_INTEGRITY,
ERR_NO_CRYPTO,
} = require('internal/errors').codes;
const assert = require('internal/assert');

Expand Down Expand Up @@ -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') });

}
}

Expand Down
3 changes: 2 additions & 1 deletion src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
21 changes: 21 additions & 0 deletions test/known_issues/test-cli-print-var-crypto.js
Original file line number Diff line number Diff line change
@@ -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/);
}));
7 changes: 6 additions & 1 deletion test/parallel/test-assert-checktag.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down
11 changes: 11 additions & 0 deletions test/parallel/test-bootstrap-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
66 changes: 66 additions & 0 deletions test/parallel/test-cli-eval.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/);
}));
2 changes: 1 addition & 1 deletion test/parallel/test-global-webcrypto-classes.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Flags: --experimental-global-webcrypto --expose-internals
// Flags: --expose-internals
'use strict';

const common = require('../common');
Expand Down
Loading

0 comments on commit 6de2673

Please sign in to comment.