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

Introduce "cross-origin-isolated" permission #5752

Merged

Conversation

yutakahirano
Copy link
Member

@yutakahirano yutakahirano commented Jul 22, 2020

Introduce "cross-origin-isolated" permission to allow a document to
control whether nested documents can access features that require
securer context, even when COOP+COEP are enabled.

Fixes #5435.

(See WHATWG Working Mode: Changes for more details.)


/infrastructure.html ( diff )
/webappapis.html ( diff )
/window-object.html ( diff )
/workers.html ( diff )

Introduce "cross-origin-isolated" permission to allow a document to
control whether nested documents can access to features that require
securer context, even when COOP+COEP are enabled.
@yutakahirano
Copy link
Member Author

yutakahirano commented Jul 22, 2020

@domenic @annevk Can you PTAL?

For documents We use #allowed-to-use.
For dedicated workers the permission is inherited from the ancestor document.

Shared and service workers don't inherit the policy. This comes from the fact that we don't inherit COEP for these workers, but web developers who want to use APIs requiring securer context to sniff process memory shared with the parent document can use service worker to do that, assuming the worker and the frame are hosted by the same process (this is not guaranteed but they often are). Do you think it's problematic? I thought about this for a while but couldn't find a good solution.

source Outdated Show resolved Hide resolved
@annevk
Copy link
Member

annevk commented Jul 22, 2020

Well, per the current model a shared/service worker that is cross-origin isolated would not be put in the same process with something that is not cross-origin isolated. It's not really clear how you delegate to shared/service workers though and whether shared workers need to be isolated the way they are for secure contexts. There's quite a few gotchas it seems with using permission delegation here (and perhaps in general when workers are important).

@yutakahirano
Copy link
Member Author

@domenic, do you have any opinions?

@domenic
Copy link
Member

domenic commented Jul 30, 2020

Sorry about not responding earlier. I can be more responsive on this going forward, now that I understand it more fully.

I think shared/service workers should be treated as "roots". Their cross-origin isolated status should only be controlled by themselves and the headers they set.

For dedicated workers, automatically inheriting from their parent seems fine, since they must be same-origin to their parent. (Or they can be data: URLs, but data: URLs also inherit this sort of thing, generally.)


On this PR generally, it seems like we will need to refactor things so that the crossOriginIsolated getter steps can be called from elsewhere. In particular I wonder if it's confusing to have separate "cross-origin isolated" (the boolean set at creation time) and "effectively cross-origin isolated" (what the getter steps return). Plus the "cross-origin isolated permission" boolean. I'm not sure what the best architecture is though, and perhaps it will be modified given my above comments...

@yutakahirano
Copy link
Member Author

How about the following?


Environment settings object

An environment settings object is an environment that additionally specifies algorithms for:
...

An effective cross-origin isolated: A boolean represents whether it is allowed to use APIs that requires cross-origin isolation.

Script settings for Window objects

The effective cross-origin isolated: Return the logical conjunction of the surrounding agent's agent cluster's cross-origin isolated and whether its associated Document is allowed to use the "cross-origin-isolated" permission.

Script settings for workers

The effective cross-origin isolated: Return worker global scope's effective cross-origin isolated.

(Note: worker global scope's effective cross-origin isolated is defined similarly to the current change).

The WindowOrWorkerGlobalScope mixin

self.crossOriginIsolated returns the relevant settings object's effective cross-origin isolated.

@domenic
Copy link
Member

domenic commented Aug 4, 2020

That sounds pretty good to me! Two notes:

@yutakahirano
Copy link
Member Author

Updated.

I like "agent cluster's cross-origin isolated" more than "COOP+COEP headers set". We use the value for important decisions (e.g., document.domain behavior) and while it may not be exposed via crossOriginIsolated I think "agent cluster's cross-origin isolated" is more important, and deserves a simpler name.

I will create PRs for service workers and worklets.

@domenic
Copy link
Member

domenic commented Aug 5, 2020

I like "agent cluster's cross-origin isolated" more than "COOP+COEP headers set". We use the value for important decisions (e.g., document.domain behavior)

Interesting. And I guess we don't want to turn document.domain back on if the permission isn't delegated. OK, that makes sense then.

I wonder then about the name "effective cross-origin isolated". Maybe "allowed to use cross-origin isolated features"? Any bikeshedding is welcome. (But there's no need to update the PR for naming stuff right now; we can wait until everyone is settled to go through more changes.)

I will create PRs for service workers and worklets.

Actually, since I already have PRs out for related changes, how about you just add comments to w3c/ServiceWorker#1527 and w3c/css-houdini-drafts#998 saying what the algorithms should be, and I can edit my PRs to include them?

source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Outdated
data-x="concept-url-scheme">scheme</span> is "<code data-x="">data</code>", then set
<var>worker global scope</var>'s <span
data-x="concept-WorkerGlobalScope-effective-cross-origin-isolated">effective cross-origin
isolated</span> to false.
Copy link
Member

Choose a reason for hiding this comment

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

What is the reasoning for this? data: URL workers have their own separate agent clusters, so I would think they should be allowed to use cross-origin isolated features...

Copy link
Member Author

Choose a reason for hiding this comment

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

This is because "data:" origins are opaque. We need these worker-specific logic because permission policy doesn't work with workers (w3c/webappsec-permissions-policy#207).

Copy link
Member

Choose a reason for hiding this comment

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

Right, they're opaque, so they should be treated as cross-origin isolated, right? So this should be set to true, if anything...

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm talking about hte "cross-origin-isolated" feature. The worker is cross origin compared to its owner, and it is not able to set permission policies for data: workers so the default policy ("self") is applied, which means effective cross-origin isolated is false.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I was confused with data: URL iframes, which are their own agent cluster. OK, this is probably fine... I'll do a more full re-review.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, upon re-reviewing I am still confused why we want to prevent data: URL workers from using cross-origin isolated features. I know they have a different origin. However, for other important things, we inherit from the parent, even though they are cross-origin. For example, for embedder policy.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we can choose either way. Ultimately that will be resolved in w3c/webappsec-permissions-policy#207 but to be honest I don't want to be blocked by the issue. I'm fine with either options, but starting with effective cross-origin isolated: false and changing the value to true if necessary (when w3c/webappsec-permissions-policy#207 is resolved) seems safer than the other option from the compatibility perspective.

Please note that for COEP we didn't have options. As the owner and the worker share the same agent cluster we needed to inherit the value when the owner had COEP: require-corp.

@annevk do you have opinions?

Copy link
Member

Choose a reason for hiding this comment

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

Got it. I understand now, and agree this is a good conservative default. I will push a commit adding a note about that reasoning.

@yutakahirano
Copy link
Member Author

Thank you the review! Somehow "commit suggestion" didn't work so I addressed the comments manually.

Reg feature and permission: I was under the impression that spec authors were replacing "feature" with "permission" (w3c/webappsec-permissions-policy#359), but I'm happy to accept your suggestion in this change.

@domenic
Copy link
Member

domenic commented Aug 6, 2020

@clelland are Permissions Policy things called "permissions" now, or still "features"? The spec still talks about features (example).

source Outdated
data-x="concept-url-scheme">scheme</span> is "<code data-x="">data</code>", then set
<var>worker global scope</var>'s <span
data-x="concept-WorkerGlobalScope-effective-cross-origin-isolated">effective cross-origin
isolated</span> to false.
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, upon re-reviewing I am still confused why we want to prevent data: URL workers from using cross-origin isolated features. I know they have a different origin. However, for other important things, we inherit from the parent, even though they are cross-origin. For example, for embedder policy.

source Outdated Show resolved Hide resolved
@domenic
Copy link
Member

domenic commented Aug 7, 2020

In IRC @annevk suggested "cross-origin isolated capability" as the best name for what this PR calls "effective cross-origin isolated". I am happy with that as the final name. I will push a commit renaming in that direction.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

I pushed a commit with editorial changes and additions. This LGTM. I believe also the discussion in #5435 was positive enough from @annevk that we should count that as Mozilla support; @annevk, please let us know if you disagree.

So I'll merge this on Monday or Tuesday unless I hear otherwise. In the meantime, you could file bugs on the various browser issue trackers, pointing to this PR and the web platform tests PR.

@domenic domenic self-assigned this Aug 10, 2020
@yutakahirano
Copy link
Member Author

Thank you! I filed a bug for each implementation.

@domenic domenic merged commit f781a90 into whatwg:master Aug 12, 2020
@domenic domenic added addition/proposal New features or enhancements security/privacy There are security or privacy implications topic: cross-origin-opener-policy Issues and ideas around the new "inverse of rel=noopener" header. labels Aug 12, 2020
@domenic domenic added the topic: cross-origin-embedder-policy Issues and ideas around the new "require CORP for subresource requests and frames and etc" proposal. label Aug 12, 2020
yutakahirano added a commit to web-platform-tests/wpt that referenced this pull request Aug 18, 2020
* Add tests for 'cross-origin-isolated' permission

Make sure self.crossOriginIsolated is affected by the permission.

See also: whatwg/html#5752
@annevk
Copy link
Member

annevk commented Aug 21, 2020

To follow-up here, Mozilla is indeed supportive of this and happy to follow Chrome.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 25, 2020
…rmission, a=testonly

Automatic update from web-platform-tests
Add tests for 'cross-origin-isolated' permission (#24600)

* Add tests for 'cross-origin-isolated' permission

Make sure self.crossOriginIsolated is affected by the permission.

See also: whatwg/html#5752
--

wpt-commits: 50255e662659a593628bc7dd95860fbdaa1e2a9e
wpt-pr: 24600
yutakahirano added a commit to yutakahirano/ServiceWorker that referenced this pull request Oct 5, 2020
ambroff pushed a commit to ambroff/gecko that referenced this pull request Nov 4, 2020
…rmission, a=testonly

Automatic update from web-platform-tests
Add tests for 'cross-origin-isolated' permission (#24600)

* Add tests for 'cross-origin-isolated' permission

Make sure self.crossOriginIsolated is affected by the permission.

See also: whatwg/html#5752
--

wpt-commits: 50255e662659a593628bc7dd95860fbdaa1e2a9e
wpt-pr: 24600
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements security/privacy There are security or privacy implications topic: cross-origin-embedder-policy Issues and ideas around the new "require CORP for subresource requests and frames and etc" proposal. topic: cross-origin-opener-policy Issues and ideas around the new "inverse of rel=noopener" header.
Development

Successfully merging this pull request may close these issues.

Consider requiring feature policy for SharedArrayBuffer in cross-origin frames
4 participants