Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: enable global WebCrypto by default #42083

Merged
merged 2 commits into from Sep 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.js
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
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
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
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
24 changes: 17 additions & 7 deletions lib/internal/crypto/webcrypto.js
Expand Up @@ -5,6 +5,7 @@ const {
JSONParse,
JSONStringify,
ObjectDefineProperties,
ObjectDefineProperty,
ReflectApply,
ReflectConstruct,
SafeSet,
Expand All @@ -28,6 +29,10 @@ const {
validateString,
} = require('internal/validators');

const {
getOptionValue,
} = require('internal/options');

const { TextDecoder, TextEncoder } = require('internal/encoding');

const {
Expand Down Expand Up @@ -792,15 +797,20 @@ ObjectDefineProperties(
writable: true,
value: randomUUID,
},
CryptoKey: {
__proto__: null,
enumerable: true,
configurable: true,
writable: true,
value: CryptoKey,
}
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
});

if (getOptionValue('--no-experimental-global-webcrypto')) {
// For backward compatibility, keep exposing CryptoKey in the Crypto prototype
// when using the flag.
ObjectDefineProperty(Crypto.prototype, 'CryptoKey', {
__proto__: null,
enumerable: true,
configurable: true,
writable: true,
value: CryptoKey,
});
}

ObjectDefineProperties(
SubtleCrypto.prototype, {
[SymbolToStringTag]: {
Expand Down
28 changes: 26 additions & 2 deletions lib/internal/main/eval_string.js
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'))`
targos marked this conversation as resolved.
Show resolved Hide resolved
) : source,
getOptionValue('--inspect-brk'),
print,
loadESM);
}
27 changes: 17 additions & 10 deletions lib/internal/process/pre_execution.js
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
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
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
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
@@ -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
@@ -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
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