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

[web-api] Limit cross-origin sharing of Wasm modules. #1352

Closed

Conversation

dtig
Copy link
Member

@dtig dtig commented Aug 16, 2021

This was discussed, and voted on in the June 22nd meeting (notes). The suggested solution was to limit sharing modules across origins. There is some discussion in the linked issue #1303, but it doesn't look like the HTML spec has a way to enforce an origin check in the serialization infrastructure.

This PR aims to store the origin as well as the agent cluster, and throw if there has been an attempt to post message across origin, the text right doesn't handle opaque origins as they are null when serialized. Another option is to be vague and include a same-origin check, but digging into it more, it's not clear how this would be implemented. Opening this PR to gather feedback, I'm also quite unfamiliar with the HTML spec, so links to existing infrastructure to do this correctly appreciated.

Closes #1303.

@dtig dtig changed the title Limit cross-origin sharing of Wasm modules. [web-api] Limit cross-origin sharing of Wasm modules. Aug 16, 2021
@Ms2ger
Copy link
Collaborator

Ms2ger commented Aug 17, 2021

I'll try to review this week

document/web-api/index.bs Outdated Show resolved Hide resolved
@dtig dtig requested a review from Ms2ger August 26, 2021 23:56
@dtig
Copy link
Member Author

dtig commented Aug 30, 2021

@Ms2ger Friendly ping for a review.

Copy link
Collaborator

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay!

@@ -150,6 +152,8 @@ The [=serialization steps=], given |value|, |serialized|, and |forStorage|, are:
1. Set |serialized|.\[[Bytes]] to the [=sub-serialization=] of |value|.\[[Bytes]].

1. Set |serialized|.\[[AgentCluster]] to the [=current Realm=]'s corresponding [=agent cluster=].

1. Set |serialized|.\[[Origin]] to be the [=origin-serialization=] of the user agent's [=origin=].
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A user agent doesn't have an origin - most likely this should be the origin of the relevant settings object for value.

@@ -150,6 +152,8 @@ The [=serialization steps=], given |value|, |serialized|, and |forStorage|, are:
1. Set |serialized|.\[[Bytes]] to the [=sub-serialization=] of |value|.\[[Bytes]].

1. Set |serialized|.\[[AgentCluster]] to the [=current Realm=]'s corresponding [=agent cluster=].

1. Set |serialized|.\[[Origin]] to be the [=origin-serialization=] of the user agent's [=origin=].
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also wondering if using the serialization is correct - this would perhaps allow transferring between two opaque origins. I'm not sure if we can store the origin itself here - @annevk?

@annevk
Copy link
Member

annevk commented Oct 15, 2021

I'd still like a reply to my question in #1303 (comment).

I read the minutes and it's clear my point there was not considered or addressed as it's a rather high-level discussion. And the minutes also seem to contain something that illustrates a rather fundamental misunderstanding of how this works:

Change Wasm spec to mention origins instead of agent clusters

As to @Ms2ger's question, normally an opaque origin would also imply a different agent cluster (and this PR keeps the agent cluster in tact, as it should), but there is this edge case that is still unsolved: whatwg/html#5254.

@rossberg
Copy link
Member

rossberg commented Aug 4, 2022

@dtig, this PR is stale, what's the status?

@rossberg
Copy link
Member

@dtig, any progress on this PR, or should we close it?

@dtig
Copy link
Member Author

dtig commented May 10, 2023

I'm closing this for now, because without infrastructure that the Wasm web spec can use, and better consensus on the checks themselves, this is out of scope for the Wasm CG. Will reopen if anything changes. Sorry for the slow response, I was unsure about the status of the various parts of the deprecations that are connected to this, and had some trouble with parsing the information across different parts of the spec.

@dtig dtig closed this May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[web-api] Deprecate cross-origin Module sharing
5 participants