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

ShadowRealm Integration #42528

Open
legendecas opened this issue Mar 30, 2022 · 8 comments
Open

ShadowRealm Integration #42528

legendecas opened this issue Mar 30, 2022 · 8 comments
Labels
discuss Issues opened for discussions and feedbacks. realm Issues and PRs related to the ShadowRealm API and node::Realm

Comments

@legendecas
Copy link
Member

legendecas commented Mar 30, 2022

What is the problem this feature will solve?

ShadowRealm is a TC39 stage 3 proposal that introduces a new built-in to provide a distinct global environment, with its own global object containing its own intrinsics and built-ins. Its ability is similar to Node.js built-in vm.Context, however, ShadowRealm provides a stronger guarantee on object exchanges across realm boundaries.

As the ShadowRealm is a distinct global environment, Web integration is also under work (Like 1) to define which web APIs should be exposed in the ShadowRealm so that people can still utilize the Web APIs that still useful in the ShadowRealm. The line generally draw between the APIs that should be exposed in the ShadowRealm or not is generally if the APIs are side-effect-free to the host environment.

What is the feature you are proposing to solve the problem?

ShadowRealm is currently under active development in v8. The design doc of the Host API could be found at here. I'd believe in Node.js we may need to be able to re-evaluate the native modules in the ShadowRealm so that their object graph is not tangled with the main context, to not violate the principle of the design. /cc @nodejs/startup

I'd believe there are a bunch of modules that are useful and can stand in the line of side-effect-free, like EventEmitter, Stream, URL, etc. Also, the Web APIs that are available in Node.js like WebStream could also follow the Web specs to be exposed in the ShadowRealm. We may need to list all the possibilities that can be available in the ShadowRealm, and eventually make them available in the ShadowRealm.

Design Doc: https://docs.google.com/document/d/12_CkX6KbM9kt_lj1pdEgLB8-HQaozkJb7_nwQnHfTTg/edit?usp=sharing

/cc @nodejs/vm

@legendecas legendecas added the discuss Issues opened for discussions and feedbacks. label Mar 30, 2022
@lin72h

This comment was marked as off-topic.

@benjamingr
Copy link
Member

What sort of guarantees do we get in terms of isolation? Do we have a proven (as in formally) sandbox that is unescapable except with side-channels?

This enables some interesting use cases and I'm wondering if this means we can relax our "Do not use this for security" bits on vm (and build it on top of this)

@legendecas
Copy link
Member Author

legendecas commented Apr 4, 2022

What sort of guarantees do we get in terms of isolation? Do we have a proven (as in formally) sandbox that is unescapable except with side-channels?

The isolation guarantee of the ShadowRealm is about the isolation of the object graphs. This indicates that we will not accidentally exposes JavaScript vulnerable built-ins to the ShadowRealm, like what vm does:

let ctx = vm.createContext();
let fn = vm.runInContext('...', ctx);

fn({}) // <= the function created in the vm can get access to the OUTER `Object` and hijack its methods

However, since the code evaluated in the ShadowRealm is still sharing the heap with the code outside of the ShadowRealm. This means it is still vulnerable to Spectre, etc.

For Host APIs exposed in the ShadowRealm, technically, we are expecting them to be side-effects free -- and thus can not be utilized to initiate attacks on the host environment.

(@benjamingr I assume you closed the issue accidentally? I'm reopening this)

@legendecas legendecas reopened this Apr 4, 2022
@benjamingr
Copy link
Member

(@benjamingr I assume you closed the issue accidentally? I'm reopening this)

Yes, sorry.

@benjamingr
Copy link
Member

However, since the code evaluated in the ShadowRealm is still sharing the heap with the code outside of the ShadowRealm. This means it is still vulnerable to Spectre, etc.

Yeah I think side-channels in general are outside of scope of things like ShadowRealm - even if we run it in a worker (or in a container in a worker that's still not enough to mitigate side channel attacks).

IIRC @erights pointed out there is a way to avoid side-channels: we can choose not to expose APIs (like process.hrtime) that allow time measurement (this also includes stuff like Date.now(), performance.now(), some parts of http and a bunch of others).

@Jamesernator
Copy link

Jamesernator commented Apr 15, 2022

IIRC @erights pointed out there is a way to avoid side-channels: we can choose not to expose APIs (like process.hrtime) that allow time measurement (this also includes stuff like Date.now(), performance.now(), some parts of http and a bunch of others).

The SES proposal is designed to deal with all that sort've stuff, ShadowRealm by itself is not a sandbox (however it is usually a neccessary piece of one).

Note that the web is already planning on exposing all sorts of things like high resolution timers into ShadowRealms. It probably wouldn't be a good idea for Node to give the idea that ShadowRealm is itself a sandbox when it explictly won't be a sandbox in other environments.

aduh95 pushed a commit that referenced this issue May 2, 2022
Add initial shadow realm support behind an off-by-default flag
`--experimental-shadow-realm`.

PR-URL: #42869
Refs: #42528
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
@mhofman
Copy link

mhofman commented May 2, 2022

Is this the best issue to discuss what APIs should and shouldn't be exposed in ShadowRealms, or what impact ShadowRealm execution should have on the incubator realm?

In general user code in one realm should avoid being exposed to objects from another realm, when either or both are a ShadowRealm. The obvious case is incubator realm objects leaking into a ShadowRealm, but that also implies shielding user code in the incubator realm from ShadowRealm objects, and being extremely diligent in node internals that may handle a ShadowRealm object (it'd be best avoided if possible).

One thing that comes to mind are all the "global hooks" that node provides. For example, uncaughtException or unhandledRejection events on process, or async_hooks.

@legendecas
Copy link
Member Author

legendecas commented May 4, 2022

The obvious case is incubator realm objects leaking into a ShadowRealm, but that also implies shielding user code in the incubator realm from ShadowRealm objects, and being extremely diligent in node internals that may handle a ShadowRealm object (it'd be best avoided if possible).

@mhofman thank you for your ideas! I think this is a very crucial part of the feature too.

One thing that comes to mind are all the "global hooks" that node provides. For example, uncaughtException or unhandledRejection events on process, or async_hooks.

Events like uncaughtException or unhandledRejection are going to abort the process if there are no listeners for them. We can need to wrap the uncaught exception or unhandled rejection and emit them to the main context, and abort the process if there are no listeners.

Or we can route them to an eventemitter that acts as the process object in the main context, in the ShadowRealm. But it may be likely that unhandled exception or rejection can be ignored without anyone noticing them. I don't think it is good for events like uncaughtException or unhandledRejection to be easily ignored. One solution might be first routing the events to the ShadowRealm and if it is not handled, route it to the incubator context again, until it aborts the process. I think this can also be a problem to the Web: whatwg/html#7591.

For async_hooks, as ShadowRealm can schedule it's own promise task and call into the incubator contexts with wrapped function, triggering the hooks with a correct async resource is still crucial to propagate a correct async context for stable APIs like AsyncLocalStorage. For a very primitive thought, I'd find creating wraps for the async resource object and trigger hooks in main context with the wrapped objects (not related to the WrappedFunction or anything, it can be Node.js implementation-defined wraps) can be the most straightforward solution.

danielleadams pushed a commit that referenced this issue Jun 11, 2022
Add initial shadow realm support behind an off-by-default flag
`--experimental-shadow-realm`.

PR-URL: #42869
Refs: #42528
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
legendecas added a commit that referenced this issue Aug 1, 2022
PR-URL: #44056
Refs: #42528
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Feng Yu <[email protected]>
legendecas added a commit to legendecas/node that referenced this issue Aug 16, 2022
PR-URL: nodejs#44056
Refs: nodejs#42528
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Feng Yu <[email protected]>
legendecas added a commit to legendecas/node that referenced this issue Aug 17, 2022
PR-URL: nodejs#44056
Refs: nodejs#42528
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Feng Yu <[email protected]>
ruyadorno pushed a commit that referenced this issue Aug 21, 2022
PR-URL: #44056
Backport-PR-URL: #44251
Refs: #42528
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Feng Yu <[email protected]>
nodejs-github-bot pushed a commit that referenced this issue Aug 31, 2022
To distinguish per-context values from the node::Environment, split
those values to a new node::Realm structure and consolidate
bootstrapping methods with it.

PR-URL: #44179
Refs: #42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>
legendecas added a commit that referenced this issue Sep 7, 2022
Each Realm tracks its own cleanup hooks and drains the hooks when it is
going to be destroyed.

Moves the implementations of the cleanup queue to its own class so that
it can be used in `node::Realm` too.

PR-URL: #44379
Refs: #44348
Refs: #42528
Reviewed-By: Joyee Cheung <[email protected]>
Fyko pushed a commit to Fyko/node that referenced this issue Sep 15, 2022
PR-URL: nodejs#44056
Refs: nodejs#42528
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Feng Yu <[email protected]>
Fyko pushed a commit to Fyko/node that referenced this issue Sep 15, 2022
To distinguish per-context values from the node::Environment, split
those values to a new node::Realm structure and consolidate
bootstrapping methods with it.

PR-URL: nodejs#44179
Refs: nodejs#42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>
legendecas added a commit that referenced this issue Dec 28, 2022
Bindings that need to be loaded in distinct contexts of node::Realm in
the same node::Environment instance needs to share the per-isolate
templates.

Add a new binding registration callback, which is called with
per-IsolateData. This allows bindings to define templates and
share them across realms eagerly, and avoid accidental modification on
the templates when the per-context callback is called multiple times.

This also tracks loaded bindings and built-in modules with node::Realm.

PR-URL: #45547
Refs: #42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
legendecas added a commit that referenced this issue Dec 28, 2022
Create worker binding templates in the per-isolate initialization.

PR-URL: #45547
Refs: #42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
vitpavlenko pushed a commit to vitpavlenko/node that referenced this issue Dec 28, 2022
Bindings that need to be loaded in distinct contexts of node::Realm in
the same node::Environment instance needs to share the per-isolate
templates.

Add a new binding registration callback, which is called with
per-IsolateData. This allows bindings to define templates and
share them across realms eagerly, and avoid accidental modification on
the templates when the per-context callback is called multiple times.

This also tracks loaded bindings and built-in modules with node::Realm.

PR-URL: nodejs#45547
Refs: nodejs#42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
vitpavlenko pushed a commit to vitpavlenko/node that referenced this issue Dec 28, 2022
Create worker binding templates in the per-isolate initialization.

PR-URL: nodejs#45547
Refs: nodejs#42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this issue Jan 1, 2023
Bindings that need to be loaded in distinct contexts of node::Realm in
the same node::Environment instance needs to share the per-isolate
templates.

Add a new binding registration callback, which is called with
per-IsolateData. This allows bindings to define templates and
share them across realms eagerly, and avoid accidental modification on
the templates when the per-context callback is called multiple times.

This also tracks loaded bindings and built-in modules with node::Realm.

PR-URL: #45547
Refs: #42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this issue Jan 1, 2023
Create worker binding templates in the per-isolate initialization.

PR-URL: #45547
Refs: #42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
guangwong pushed a commit to noslate-project/node that referenced this issue Jan 3, 2023
PR-URL: nodejs/node#44056
Backport-PR-URL: nodejs/node#44542
Refs: nodejs/node#42528
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Feng Yu <[email protected]>
guangwong pushed a commit to noslate-project/node that referenced this issue Jan 3, 2023
PR-URL: nodejs/node#44056
Backport-PR-URL: nodejs/node#44542
Refs: nodejs/node#42528
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Feng Yu <[email protected]>
danielleadams pushed a commit that referenced this issue Jan 3, 2023
To distinguish per-context values from the node::Environment, split
those values to a new node::Realm structure and consolidate
bootstrapping methods with it.

PR-URL: #44179
Refs: #42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>
danielleadams pushed a commit that referenced this issue Jan 3, 2023
BaseObject is a wrapper around JS objects. These objects should be
created in a node::Realm and destroyed when their associated realm is
cleaning up.

PR-URL: #44348
Refs: #42528
Reviewed-By: Joyee Cheung <[email protected]>
RafaelGSS pushed a commit that referenced this issue Jan 4, 2023
Bindings that need to be loaded in distinct contexts of node::Realm in
the same node::Environment instance needs to share the per-isolate
templates.

Add a new binding registration callback, which is called with
per-IsolateData. This allows bindings to define templates and
share them across realms eagerly, and avoid accidental modification on
the templates when the per-context callback is called multiple times.

This also tracks loaded bindings and built-in modules with node::Realm.

PR-URL: #45547
Refs: #42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
RafaelGSS pushed a commit that referenced this issue Jan 4, 2023
Create worker binding templates in the per-isolate initialization.

PR-URL: #45547
Refs: #42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
RafaelGSS pushed a commit that referenced this issue Jan 5, 2023
Bindings that need to be loaded in distinct contexts of node::Realm in
the same node::Environment instance needs to share the per-isolate
templates.

Add a new binding registration callback, which is called with
per-IsolateData. This allows bindings to define templates and
share them across realms eagerly, and avoid accidental modification on
the templates when the per-context callback is called multiple times.

This also tracks loaded bindings and built-in modules with node::Realm.

PR-URL: #45547
Refs: #42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
RafaelGSS pushed a commit that referenced this issue Jan 5, 2023
Create worker binding templates in the per-isolate initialization.

PR-URL: #45547
Refs: #42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
fossamagna pushed a commit to fossamagna/node that referenced this issue Jan 23, 2023
To distinguish per-context values from the node::Environment, split
those values to a new node::Realm structure and consolidate
bootstrapping methods with it.

PR-URL: nodejs#44179
Refs: nodejs#42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>
cheungxi pushed a commit to cheungxi/node that referenced this issue Jan 29, 2023
To distinguish per-context values from the node::Environment, split
those values to a new node::Realm structure and consolidate
bootstrapping methods with it.

PR-URL: nodejs#44179
Refs: nodejs#42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>
legendecas added a commit that referenced this issue Mar 15, 2023
PR-URL: #46809
Refs: #42528
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
legendecas added a commit that referenced this issue Mar 15, 2023
This is the initial work to bootstrap Web interfaces that are defined
with extended attributes `[Exposed=*]`.

The ShadowRealm instances are garbage-collected once it is
unreachable. However, V8 can not infer the reference cycles between
the per-realm strong persistent function handles and the realm's
context handle. To allow the context to be gc-ed once it is not
reachable, the per-realm persistent handles are attached to the
context's global object and the persistent handles are set as weak.

PR-URL: #46809
Refs: #42528
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
targos pushed a commit that referenced this issue Mar 18, 2023
PR-URL: #46809
Refs: #42528
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
targos pushed a commit that referenced this issue Mar 18, 2023
This is the initial work to bootstrap Web interfaces that are defined
with extended attributes `[Exposed=*]`.

The ShadowRealm instances are garbage-collected once it is
unreachable. However, V8 can not infer the reference cycles between
the per-realm strong persistent function handles and the realm's
context handle. To allow the context to be gc-ed once it is not
reachable, the per-realm persistent handles are attached to the
context's global object and the persistent handles are set as weak.

PR-URL: #46809
Refs: #42528
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
legendecas added a commit to legendecas/node that referenced this issue Apr 12, 2023
PR-URL: nodejs#46809
Refs: nodejs#42528
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
legendecas added a commit to legendecas/node that referenced this issue Apr 12, 2023
This is the initial work to bootstrap Web interfaces that are defined
with extended attributes `[Exposed=*]`.

The ShadowRealm instances are garbage-collected once it is
unreachable. However, V8 can not infer the reference cycles between
the per-realm strong persistent function handles and the realm's
context handle. To allow the context to be gc-ed once it is not
reachable, the per-realm persistent handles are attached to the
context's global object and the persistent handles are set as weak.

PR-URL: nodejs#46809
Refs: nodejs#42528
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
legendecas added a commit to legendecas/node that referenced this issue May 23, 2023
Bindings that need to be loaded in distinct contexts of node::Realm in
the same node::Environment instance needs to share the per-isolate
templates.

Add a new binding registration callback, which is called with
per-IsolateData. This allows bindings to define templates and
share them across realms eagerly, and avoid accidental modification on
the templates when the per-context callback is called multiple times.

This also tracks loaded bindings and built-in modules with node::Realm.

PR-URL: nodejs#45547
Refs: nodejs#42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
legendecas added a commit to legendecas/node that referenced this issue May 23, 2023
Create worker binding templates in the per-isolate initialization.

PR-URL: nodejs#45547
Refs: nodejs#42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this issue Nov 10, 2023
Bindings that need to be loaded in distinct contexts of node::Realm in
the same node::Environment instance needs to share the per-isolate
templates.

Add a new binding registration callback, which is called with
per-IsolateData. This allows bindings to define templates and
share them across realms eagerly, and avoid accidental modification on
the templates when the per-context callback is called multiple times.

This also tracks loaded bindings and built-in modules with node::Realm.

PR-URL: #45547
Refs: #42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
Bindings that need to be loaded in distinct contexts of node::Realm in
the same node::Environment instance needs to share the per-isolate
templates.

Add a new binding registration callback, which is called with
per-IsolateData. This allows bindings to define templates and
share them across realms eagerly, and avoid accidental modification on
the templates when the per-context callback is called multiple times.

This also tracks loaded bindings and built-in modules with node::Realm.

PR-URL: nodejs/node#45547
Refs: nodejs/node#42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
Bindings that need to be loaded in distinct contexts of node::Realm in
the same node::Environment instance needs to share the per-isolate
templates.

Add a new binding registration callback, which is called with
per-IsolateData. This allows bindings to define templates and
share them across realms eagerly, and avoid accidental modification on
the templates when the per-context callback is called multiple times.

This also tracks loaded bindings and built-in modules with node::Realm.

PR-URL: nodejs/node#45547
Refs: nodejs/node#42528
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issues opened for discussions and feedbacks. realm Issues and PRs related to the ShadowRealm API and node::Realm
Projects
None yet
Development

No branches or pull requests

5 participants