1566868 - Cross-Origin-Opener-Policy can be used to escape sandboxing

Status: RESOLVED FIXED

(Core :: DOM: Networking, defect, P2)

Description

[Anne (:annevk)]

If you create a sandboxed popup (sandbox needs to be able to create popups) and navigate that to a Cross-Origin-Opener-Policy document the result should be a network error. To ensure that when you use Cross-Origin-Opener-Policy, you can be guaranteed a clean slate.

In https://github.com/web-platform-tests/wpt/pull/17606 look for html/cross-origin-opener-policy/coop-sandbox.https.html.

Given that sandboxing is a security feature, we should fix that before release I think.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a task, but please change it back in case of error.

Comment 2

[Christoph Kerschbaumer [:ckerschb]]

Hey Anne, we are not supporting Cross-Origin-Opener-Policy as of now. Did you file by accident, or am I missing something?

Comment 3

[Junior [inactive]]

We support COOP with pref-off.

Comment 4

[Anne (:annevk)]

Indeed, this is a sandboxing bypass in an important new feature that we need to fix before shipping said feature (hence it blocking the "final" COOP bug).

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

It seems Bug 1543066 is filed in Networking, hence moving this one over as well.

Comment 6

[Junior [inactive]]

I'm wondering if it's a dup of Bug 1555036, though we can't pass html/cross-origin-opener-policy/coop-sandbox.https.html
I can't see SANDBOXED_AUXILIARY_NAVIGATION is set in docshell/httpchannel.

Comment 7

[Junior [inactive]]

Hello Anne,
(a) I'm confused with the summary of bug: Cross-Origin-Opener-Policy can be used to escape sandboxing
Looks like COOP headers could remove all the sandbox flags after navigation.
However, in html/cross-origin-opener-policy/coop-sandbox.https.html, we're testing the message shouldn't arrive, which indicates a network error.
I believe I misunderstand something (escape indicates network error instead of removal?) since I can't match the summary and the test.

(b)
I guess html/cross-origin-opener-policy/coop-sandbox.https.html is about 5.i in navigating in top-level coop spec

5. If the result of matching currentCOOP, currentOrigin, potentialCOOP, and potentialOrigin is false and one of the following is false

    doc is the initial about:blank document
    currentCOOP's unsafe-allow-outgoing is true
    potentialCOOP is null

then:

    i. If bc's popup sandboxing flag set is not empty, then navigate bc to a network error and abort these steps.

However, it requires that matching algorithm returns false.
In the test, coop-sandbox.https.html open a popup coop-coep.py. Four browsing contexts (top-level and iframe/opener and openee) are with COOP:same-origin and same origin.
Looks like we should not have a network error.

Which step I misinterpreted?

Comment 8

[Anne (:annevk)]

Thanks, I think you're right that the test case is incorrect. Same-origin sandboxed content with allow-same-origin and allow-scripts cannot be sandboxed so letting that escape the sandbox is fine. This mainly wants to protect against A (with COOP) embedding a sandboxed B (with allow-popups and allow-scripts and maybe event allow-same-origin) and that trying to popup B (with COOP) and succeeding (rather than getting a network error). Does that make sense?

Comment 9

[Junior [inactive]]

(In reply to Anne (:annevk) from comment #8)

Thanks, I think you're right that the test case is incorrect. Same-origin sandboxed content with allow-same-origin and allow-scripts cannot be sandboxed so letting that escape the sandbox is fine. This mainly wants to protect against A (with COOP) embedding a sandboxed B (with allow-popups and allow-scripts and maybe event allow-same-origin) and that trying to popup B (with COOP) and succeeding (rather than getting a network error). Does that make sense?

I suppose popup B's opener is the sandboxed iframe B.
It's same origin, so coop match returns true. No network error.

Even if A (with COOP) embedded sandboxed iframe B, and popup C (with COOP) with opener B.
It's cross origin, coop match returns false, so network error occurs.
I thought that's what we want, isn't it?

Comment 10

[Anne (:annevk)]

I think something went wrong with bug 1579012. If A embeds B and B popups B' then B's initial about:blank inherits from B, but the COOP match should be relative to A. It would probably be cleanest if we store the COOP policy on the top-level browsing context.

Needinfo'ing Nika to reaffirm and to help me keep track of the overall design goal here.

(So for these scenarios, for the first there should be a network error as there's no COOP match relative to B's embedder (A) and for the second there should be one as well.)

Comment 11

[Nika Layzell [:nika] (ni? for response)]

I think what you're saying is accurate. In general COOP has no meaning outside of toplevel documents, so when doing comparisons, we should generally always compare against the toplevel.

I haven't quite wrapped my head around what that means for sandboxed popups yet, but I think it'd be reasonable to always network error when loading a sandboxed popup which would otherwise perform a COOP switch, under the assumption that breaking out of the sandbox is bad.

Comment 12

[Junior [inactive]]

(a) For coop matching algorithm, we need to compare not only the top-level parent's COOP but also top-level parents' origin.
(b) Looks like all we need to do for sandbox is Bug 1555036.

Comment 13

[Junior [inactive]]

A tedious question:
this points out ctx->Canonical()->GetCurrentWindowGlobal()->DocumentPrincipal() is the principal of iframe's parent.

I'm testing a simple iframe (no sandbox).
I still get the origin of iframe instead of the origin of its parent.

GetCurrentEmbedderGlobal comes out a null origin.
Is there any way to get the origin of top-level window?

Comment 14

[Nika Layzell [:nika] (ni? for response)]

(In reply to Junior [:junior] from comment #13)

Is there any way to get the origin of top-level window?

IIRC this should generally work out (with the proper null principal checks woven in):

ctx->Top()->Canonical()->GetCurrentWindowGlobal()->DocumentPrincipal()

Comment 15

[Junior [inactive]]

Attachment: ctx->Top()

```

Comment 16

[Junior [inactive]]

nsHttpChannel::HasCrossOriginOpenerPolicyMismatch - doc:1 result:1 - compare:0
doc origin:https://coop-diff-origin.appspot.com/bug/firefox_iframe_B.html - res origin: https://coop-checker.appspot.com/bug/firefox_helper_A.html
ctx 0x1324e6580 ctx->Top() 0x1324e6580
top origin https://coop-diff-origin.appspot.com/bug/firefox_iframe_B.html

The log is from patch comment 15.
Test with https://coop-checker.appspot.com/bug/firefox_main_A.html and clicking button in iframe.

Top level window with origin: https://coop-checker.appspot.com
iframe origin: https://coop-diff-origin.appspot.com

Get get the same browsing context for ctx->Top() in parent process, which seems wrong.
This is after all the windows are created.

Child process looks fine: https://searchfox.org/mozilla-central/rev/deaf58acd27bc663750548e303ac2b461a821cf8/docshell/base/BrowsingContext.cpp#138
It looks not good, right?

Comment 17

[Junior [inactive]]

Per out of band discussion, I parse the wrong context tree.
Looks like we need to cache the top-level opener's origin in browsing context at this moment.

Comment 18

[Junior [inactive]]

Attachment: bug 1566868 - coop comparison with top-level parents

Comment 19

[Junior [inactive]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=790d72aa2e5fe53801351d2775244790509bb1d0

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e0427b37129b
coop comparison with top-level parents r=valentin

Comment 21

[Noemi Erli[:noemi_erli]]

Backed out changeset e0427b37129b (Bug 1566868) for turning Bug1528607 into permafail CLOSED TREE

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedJob=270326391&searchStr=os%2Cx%2C10.14%2Cshippable%2Copt%2Cmochitests%2Ctest-macosx1014-64-shippable%2Fopt-mochitest-browser-chrome-e10s-1%2Cm%28bc1%29&revision=e0427b37129bb0272be31d8d212db607db776f5c

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=270326391&repo=autoland&lineNumber=4793

Backout: https://hg.mozilla.org/integration/autoland/rev/b44da78c9070d87870a6fb5ce924b273dcae8369

Comment 22

[Junior [inactive]]

about:blank documents inherit cross-origin opener-policy from their creator's top-level browsing context's active document at the time of creation.
Note: When A embeds B and B creates a popup, this means the popup's initial about:blank document has origin B and A's cross-origin opener-policy.

We have a new spec clarification.

Comment 23

[Anne (:annevk)]

I made the clarification in comment 22 yesterday, but thanks to Junior I took another look at the problem as I had forgotten about the issues pointed out in comment 7.

I now think html/cross-origin-opener-policy/coop-sandbox.https.html is correct.

If a browsing context has a popup sandboxing flag set, it's a popup and it shouldn't be able to contain anything with a non-null COOP (note that Cross-Origin-Opener-Policy: unsafe-inherit might still work) as that would mean escaping the sandboxing, however mild that sandbox may be (to be clear, this is a reversal of my statement in comment 8).

Then, if a browsing context swap happened due to unsafe-allow-outgoing not being set, the new top-level browsing context still ought to be sandboxed. So we need to copy the popup sandboxing flag set over. (The alternative would be for sandboxed environments governed by COOP without unsafe-allow-outgoing to not be able to open popups at all.)

I have updated the navigation section at the end of https://gist.github.com/annevk/6f2dd8c79c77123f39797f6bdac43f3e to account for this.

(The one other thing about sandboxing that is captured in https://github.com/whatwg/html/issues/4921 was already okay in Firefox if I understood it correctly from Nika. Namely that CSP influences the origin of the document we are navigating to in time. So if you navigate to a document that has an opaque origin due to sandboxing it'll result in a non-match.)

I hope I have now considered all angles appropriately here.

Comment 24

[Junior [inactive]]

Attachment: Bug 1566868 - network error for non empty sandboxing flag and non-null COOP

Adjust for new spec change (comment 23)

Comment 25

[Junior [inactive]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a9f454b6f8480df4804302afb1c777bbb402e233

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/906731e277bb
network error for non empty sandboxing flag and non-null COOP r=valentin

Comment 27

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/906731e277bb