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

Consider requiring feature policy for SharedArrayBuffer in cross-origin frames #5435

Closed
domenic opened this issue Apr 3, 2020 · 18 comments · Fixed by #5752
Closed

Consider requiring feature policy for SharedArrayBuffer in cross-origin frames #5435

domenic opened this issue Apr 3, 2020 · 18 comments · Fixed by #5752
Labels
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

Comments

@domenic
Copy link
Member

domenic commented Apr 3, 2020

#4734 proposes that all crossOriginIsolated contexts be allowed to postMessage() SharedArrayBuffer. However, @ulan brought up the fact that this makes COOP+COEP adoption dangerous.

Consider the scenario where outer.com already embeds a semi-trusted inner.com. The decision to embed inner.com was made before COOP+COEP existed.

Now outer.com wants to use SharedArrayBuffer. So outer.com enables COOP+COEP, and convinces inner.com to add appropriate headers as well.

Suddenly outer.com is now vulnerable to Spectre attacks from inner.com. To restore the security they had in a pre-COOP+COEP world, they need to audit inner.com (and its transitive dependencies) to ensure that they do not abuse SharedArrayBuffer. In practice, this is likely not feasible.

We propose that SharedArrayBuffer not be automatically enabled in all crossOriginIsolated frames. Instead, we would define a new feature policy for SharedArrayBuffer and other potentially process-wide features (like the memory measurement API, or maybe precise performance.now()). This feature policy would have a default allowlist of 'self', so that same-origin frames can still use these APIs, but cross-origin frames would need to be explicitly delegated permission to use these APIs, with something like <iframe allow="powerful-features">. (Name TBD.)

/cc @mikewest since this seems rather related to https://github.com/mikewest/securer-contexts.

@domenic domenic added security/privacy There are security or privacy implications topic: cross-origin-opener-policy Issues and ideas around the new "inverse of rel=noopener" header topic: cross-origin-embedder-policy Issues and ideas around the new "require CORP for subresource requests and frames and etc" proposal labels Apr 3, 2020
@arturjanc
Copy link

/cc @annevk

@annevk
Copy link
Member

annevk commented Apr 6, 2020

I think in principle this is reasonable, especially if we expect process isolation to continue to be infeasible on certain platforms.

This will require a lot of new tests.

I don't think we should allow this to create a place where the SharedArrayBuffer constructor is not hidden, but postMessage() still throws. On the other hand, it definitely shouldn't affect the plan to disable document.domain and key agent clusters by origin.

It's also a bit weird that permission delegation is not per agent, so you could end up with globals within a single agent disagreeing on whether they have access to cross-origin isolated features. That's probably okay.

I tried to think a bit what kind of changes I would make to #4734:

  • I would add a cross-origin isolated permission with the default as per OP. (I'd really like to avoid terminology changes at this point.)
  • I would add a new concept that tells whether a given global/realm has the cross-origin isolated capability which depends on the agent cluster's cross-origin isolated and the cross-origin isolated permission.
  • I would make deleting the SharedArrayBuffer constructor conditional upon the cross-origin isolated capability.
  • I would make postMessage()'s check conditional upon the cross-origin isolated capability. I would not change the deserialization check. If an embedder A creates embeddees B1 and B2 with different permissions they have made a mistake that we cannot defend against.
  • I would make crossOriginIsolated's getter's true return value conditional upon the cross-origin isolated capability.

And we'd use the cross-origin isolated capability for timers and such. (Nobody tried to tackle cross-origin isolated timers yet I believe. Perhaps it should wait if this is happening.)

I'm not sure I want to commit to Firefox not shipping before all this is done and implemented. Not sure how tenable that is given the status quo. It would be doable to follow though. Hope that's understandable.

@annevk
Copy link
Member

annevk commented May 11, 2020

If you were to use allow="cross-origin-isolated 'none'" for a same-origin document that might yield surprising results. E.g., globalThis.SharedArrayBuffer is undefined, but globalThis.parent.SharedArrayBuffer is not. Specification-architecture wise this seems fine, but it might prevent certain implementation techniques as the shapes of the globals within an agent are no longer identical. whatwg/webidl#883 will make this worse. (Not sure anyone is pursuing such techniques, but figured I'd point it out.)

@yutakahirano
Copy link
Member

If you were to use allow="cross-origin-isolated 'none'" for a same-origin document that might yield surprising results. E.g., globalThis.SharedArrayBuffer is undefined, but globalThis.parent.SharedArrayBuffer is not.

I agree that it is surprising and I prefer ignoring the policy for the same-origin case.

@annevk
Copy link
Member

annevk commented Jun 8, 2020

I'm not sure that Feature Policy allows for that. And in fact, the header variant would allow you to disable the cross-origin isolated permission for a top-level context as well and I think we should not deviate from that.

What we could do though is that the availability of SharedArrayBuffer depends on cross-origin isolated rather than the cross-origin isolated permission. And that only postMessage() depends on the permission. You could still get weird cases, but at least the shapes of realms within an agent remain consistent.

cc @clelland

@yutakahirano
Copy link
Member

How about "other potentially process-wide features (like the memory measurement API, or maybe precise performance.now())"?

@annevk
Copy link
Member

annevk commented Jun 8, 2020

Well, I got the impression you all were proposing to gate those similarly? In Firefox I think we have a global available for most timers so it would be doable implementation-wise. Hiding SharedArrayBuffer would be feasible too, but it wasn't clear that's strictly needed as there's no compatibility concern here so might as well keep it non-hidden.

@yutakahirano
Copy link
Member

yutakahirano commented Jun 8, 2020

Let me make sure. Are the followings your preference?

  1. There are cross-origin isolated [concept], cross-origin isolated permission [concept] and crossOriginIsolated (global property).
  2. cross-origin isolated [concept] depends on COOP & COEP.
  3. cross-origin isolated permission [concept] depends on the feature policy.
  4. crossOriginIsolated (global property) exposes cross-origin isolated [concept] cross-origin isolated [concept] and cross-origin isolated permission [concept].
  5. SharedArrayBuffer constructor depends on cross-origin isolated[concept].
  6. postMessage with SharedArrayBuffer depends on cross-origin isolated [concept] and cross-origin isolated permission [concept].
  7. The memory measurement API depends on cross-origin isolated [concept] and cross-origin isolated permission [concept].

@annevk
Copy link
Member

annevk commented Jun 8, 2020

Yes, except I was thinking 4 would depend on both as well (that's what I meant with capability above) as you want that for feature testing postMessage(). Having said that, I'm pretty flexible here.

@yutakahirano
Copy link
Member

I agree that exposing cross-origin isolated [concept] && cross-origin isolated permission [concept] as crossOriginIsolated is better, given the original motivation.

@clelland
Copy link
Contributor

I'm not sure that Feature Policy allows for that. And in fact, the header variant would allow you to disable the cross-origin isolated permission for a top-level context as well and I think we should not deviate from that.

If you mean that Feature Permissions Policy doesn't allow us to just ignore the policy in same-origin documents, that's true. Restrictions on same-origin embeds are always a little weird, if the documents aren't isolated from each other, but the policy mechanism tries to at least be consistent.

Re: globals having different members; we originally specced Feature Policy to expose JS symbols (or not) depending on policy, but ended up never shipping a policy like that, as it always seemed better to just switch behaviour -- have window.sharedArrayBuffer fail or throw, rather than just not exist.

@yutakahirano
Copy link
Member

Thank you for the clarification, I now understand the consistency in the Permissions Policy is more important.

@yutakahirano yutakahirano mentioned this issue Jun 15, 2020
3 tasks
@yutakahirano
Copy link
Member

I'm implementing the permission at https://chromium-review.googlesource.com/c/chromium/src/+/2247940. I'm not shipping the feature so I can change the name later, but I'd like to get your opinions before landing the change. @domenic, @annevk, are you both fine with the name "cross-origin-isolated" as the permission name? Of course opinions from other people are welcome too.

@domenic
Copy link
Member Author

domenic commented Jun 19, 2020

I'm fine with it. It's not perfect (it's a weird name for a "permission") but it's probably better than creating some new term.

@annevk
Copy link
Member

annevk commented Jun 24, 2020

I have a hard time coming up with a better name, but I also already liked reusing this term for this, so yes, I'm fine.

@yutakahirano
Copy link
Member

Thank you!

annevk added a commit that referenced this issue Jun 29, 2020
A top-level navigation response with Cross-Origin-Opener-Policy set to same-origin and Cross-Origin-Embedder-Policy set to require-corp will create a cross-origin isolated browsing context group. And all agent clusters therein will be cross-origin isolated as well (shared and service workers can still not be, as they sit on the side).

This change also:

* Gates SharedArrayBuffer exposure behind that primitive for web compatibility reasons.
* Gates SharedArrayBuffer sharing behind that primitive.
* Exposes it through self.crossOriginIsolated.
* Makes document.domain return before it mutates the origin.
* Makes agent clusters keyed on origin.

Tests:

* web-platform-tests/wpt#17719
* web-platform-tests/wpt#17760
* web-platform-tests/wpt#17761
* web-platform-tests/wpt#17802
* web-platform-tests/wpt#17909
* web-platform-tests/wpt#18543
* web-platform-tests/wpt#20116
* web-platform-tests/wpt#22358

Closes #4732. Closes #5122. Closes #5444.

Follow-up: #5435.
annevk added a commit that referenced this issue Jul 8, 2020
A top-level navigation response with Cross-Origin-Opener-Policy set to same-origin and Cross-Origin-Embedder-Policy set to require-corp will create a cross-origin isolated browsing context group. And all agent clusters therein will be cross-origin isolated as well (shared and service workers can still not be, as they sit on the side).

This change also:

* Gates SharedArrayBuffer exposure behind that primitive for web compatibility reasons.
* Gates SharedArrayBuffer sharing behind that primitive.
* Exposes it through self.crossOriginIsolated.
* Makes document.domain return before it mutates the origin.
* Makes agent clusters keyed on origin.

Tests:

* web-platform-tests/wpt#17719
* web-platform-tests/wpt#17760
* web-platform-tests/wpt#17761
* web-platform-tests/wpt#17802
* web-platform-tests/wpt#17909
* web-platform-tests/wpt#18543
* web-platform-tests/wpt#20116
* web-platform-tests/wpt#22358

Closes #4732. Closes #5122. Closes #5444.

Follow-up: #5435 (and #5362).
@othermaciej
Copy link

Is the idea here that there would not necessarily be a user-exposed permission, just a way to delegate to subframes?

@clelland
Copy link
Contributor

Yep, that's the idea.

domenic pushed a commit that referenced this issue Aug 12, 2020
This allows a document to control whether nested documents can access
features that require cross-origin isolation, as an additional
restriction on top of requiring COOP+COEP.

Fixes #5435.
mfreed7 pushed a commit to mfreed7/html that referenced this issue Sep 11, 2020
A top-level navigation response with Cross-Origin-Opener-Policy set to same-origin and Cross-Origin-Embedder-Policy set to require-corp will create a cross-origin isolated browsing context group. And all agent clusters therein will be cross-origin isolated as well (shared and service workers can still not be, as they sit on the side).

This change also:

* Gates SharedArrayBuffer exposure behind that primitive for web compatibility reasons.
* Gates SharedArrayBuffer sharing behind that primitive.
* Exposes it through self.crossOriginIsolated.
* Makes document.domain return before it mutates the origin.
* Makes agent clusters keyed on origin.

Tests:

* web-platform-tests/wpt#17719
* web-platform-tests/wpt#17760
* web-platform-tests/wpt#17761
* web-platform-tests/wpt#17802
* web-platform-tests/wpt#17909
* web-platform-tests/wpt#18543
* web-platform-tests/wpt#20116
* web-platform-tests/wpt#22358

Closes whatwg#4732. Closes whatwg#5122. Closes whatwg#5444.

Follow-up: whatwg#5435 (and whatwg#5362).
mfreed7 pushed a commit to mfreed7/html that referenced this issue Sep 11, 2020
This allows a document to control whether nested documents can access
features that require cross-origin isolation, as an additional
restriction on top of requiring COOP+COEP.

Fixes whatwg#5435.
yutakahirano added a commit to yutakahirano/ServiceWorker that referenced this issue Oct 5, 2020
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Implementing whatwg/html#5435.

This change itself doesn't change the behavior (yet) as
Agent::IsCrossOriginIsolated() always returns false. Please see
https://crrev.com/c/2247463 for details. With subsequent changes this
will affect the value of WindowOrWorkerGlobalScope.crossOriginIsolated.

Bug: 1018680
Change-Id: I952197efe1b6a591cee39a28519402e22598928c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2247940
Auto-Submit: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#784696}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 23d3561c3e0fcd894052afd9a6c39c3e512e53b2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 a pull request may close this issue.

6 participants