Move cross-origin opener policy into policy container

[domfarolino]

This PR does two things:

• Introduces policy container's cross-origin opener policy member
• Removes the two members that are redundant with it:
  • Document's cross-origin opener policy member
  • navigation params's cross-origin opener policy member

This change is mostly mechanical with a few exceptions:

• In the current spec's #creating-a-new-browsing-context > Step 15, we clone the creator Document's policy container and then conditionally inherit the cross-origin opener policy based on same-origin-ness. After this PR, we continue to inherit the policy container, and the conditionally censor the policy container's cross-origin opener policy based on cross-origin-ness
• The about:srcdoc navigation case (specific diff), where we always navigate with a blank COOP. Before this PR, in order to achieve this the spec just created a brand new cross-origin policy and passed that into navigation params. With this PR, by default the policy container (and therefore COOP) would be inherited from the parent. So in order to maintain the old behavior, this PR manually resets the inherited policy container's COOP back to "the brand new uninitialized COOP" (in step 7 of the "specific diff" link). I'm not sure if the old spec's behavior is exactly what we want, but either way this PR maintains it to be a pure clean-up.
• In "create navigation params by fetching", we manually set the "determined" policy container's COOP to responseCOOP so that it can be read later on when creating the new Document for the navigation. We used to put it on the navigation params but now we have to manually set it on the policy container that goes into the navigation params.

---

/browsers.html ( diff )
/browsing-the-web.html ( diff )
/document-lifecycle.html ( diff )
/document-sequences.html ( diff )
/dom.html ( diff )

[domfarolino]

@domenic Please note when I started this PR I was on vacation and I hastily wrote down this unfinished note (that I've since removed). Upon looking at it more today, I couldn't finish it because I'm not sure what case I had in mind that supposedly warranted a note. Besides the error document case, do you know of any situation where we wouldn't use the returned navigation params's COOP for the resulting document?

[domenic]

Besides the error document case, do you know of any situation where we wouldn't use the returned navigation params's COOP for the resulting document?

It should just be the #read-ua-inline case, yeah. (Which is also for PDF viewers or some other pages.) I don't think that needs special mention.

---

In general I think this PR is good but the number of places where we don't do the natural COOP inheritance is a bit worrying. I appreciate wanting to do a pure cleanup, but we should at least open an issue to track those. From what I can see they are:

• about:srcdoc navigation case does not inherit, but maybe it should
• "create navigation params by fetching": it would be much nicer if we could just get the result automatically from "determine navigation params policy container". I think the delta would be:
  • Storing COOP in history (case where historyPolicyContainer is not null, i.e. about: and data: URLs that we're traversing back to)
  • Inheriting COOP from initiator (cases where initiatorPolicyContainer is not null and we're navigating to an about:, data:, or blob: URL)
  • (Editorially: we'd probably change from passing in just a URL, to passing in response, so that in the fallback case we can return a new policy container whose COOP is derived from response, i.e. the logic stays inside "determine navigation params policy container".)

I'd like @antosart's thoughts on this, and also his review of this PR.

[antosart]

Thanks, I left a few comments. In general, I think it would be good to encapsulate the inheritance logic inside the policy container creation/inheritance algorithms.

As for how COOP inheritance should behave, I honestly do not know. I was having a look at chromium source code, and there I have the impression that we never use the response header's COOP for subframes (instead we inherit from the parent if same-origin, and use a new COOP if not). I also don't see srcdoc treated specially there.

I think that @camillelamy and @hemeryar thought a lot about COOP inheritance, so they might have some input?

[domfarolino]

Hmm, are your comments in a draft maybe? I can't see them.

I also don't see srcdoc treated specially there.

Does this imply that implementation-wise we just inherit COOP normally from the parent in the srcdoc case? If so then we should probably make the spec do that too. But I'm curious: what are the observable effects of this? Help me walk through a few scenarios so I can get it right in my head.

• Consider an about:srcdoc iframe that's same-origin with its top-level, and window.open()s any URL. I think about:srcdoc COOP inheritance would not be observable, since upon new top-level traversable creation we wouldn't be triggering this condition, therefore noopener would be false. Then upon actual browsing context/Document creation we'd hit this step 15 > substep 3, which uses the creator Document (aka srcdoc iframe)'s top-level browsing context's document's COOP for usage in the new top-level Document. So the srcdoc iframe's COOP never has an effect in this case — we just pull it from its top-level. Then if the newly-created Document goes cross-origin, the usual BCG swap matching logic kicks in comparing the current origin with the response origin, etc.
• Consider an about:srcdoc iframe that is cross-origin with its top-level — so the tree A(B(srcdoc)) I suppose. Then the condition above would be triggered and we'd immediately set noopener to true, placing the newly-opened Document in its own BCG. I think this is even observable in the case where the srcdoc iframe window.open()d a URL that is same-origin to itself.

Does that sound right? And furthermore do you know if we have tests for this? If so, I'm wondering how this would be observable.

Edit

I've crossed-out the second scenario after realizing that we DO NOT do COOP inheritance for cross-origin iframes period. So that scenario is not relevant.

[antosart]

Should we drop this and just always initialize policy container's coop to a new coop (instead of inheriting as we do for the other policies)? This would seem to match the current state of things.

If not, I think it would be preferable to handle this logic inside "determining navigation params policy container", and having a switch there: if the document's url is about:srcdoc, then don't inherit coop. Although I don't understand why srcdoc should be special (as opposed to, e.g., an empty document).

[domfarolino]

Should we drop this and just always initialize policy container's coop to a new coop

I'm confused, isn't that what this line is doing? <var>coop</var> is a new coop, so what this PR does is always initialize the policy container's COOP to a new COOP, instead of inheriting ever. But it sounds like we maybe shouldn't special-case about:srcdoc here, and should maybe conditionally clear policyContainer's COOP based on same-origin-ness (about:srcdoc can be cross-origin via responseOrigin due to sandbox flags). In that sense this is quite similar to https://github.com/whatwg/html/pull/9149/files#r1189506835 which handles the <embed> case.

[antosart]

I was just trying to say that, before your PR, I don't see any other place where we would inherit COOP according to policy container rules. For example, if A creates a data url child iframe, I don't think there is any mechanism, before your PR, to have the data url iframe inherit COOP from A. Your PR changes that, because the general policy container inheritance mechanism kicks in. So I am just saying, if we want to make this PR a no-op, would it make sense to change the inheritance algorithm of the policy container so that COOP is never inherited? This would avoid the special handling of srcdoc here. And we could sort out the correct inheritance in a separate PR.

[antosart]

sorry, I forgot to submit the review

[antosart]

@domfarolino what you write makes sense to me, but I don't see how it is specific to srcdoc (in fact, I wonder when the COOP of a child frame is actually observable). I can't find any WPTs specific to srcdoc under html/cross-origin-opener-policy.

As said, I am not an expert here. I could invest the time to learn more, but it probably makes more sense if @camillelamy, @hemeryar or @ArthurSonzogni chime in.

[hemeryar]

Hey there! I haven't read the patch but can give some extra info:

• COOP is never directly visible on subframes. It is ignored for all BC selection algorithms. It is only present to be inherited if the subframe opens a popup. Currently it is only inherited by same-origin iframes. On why COOP is not inherited on cross-origin iframes, this document gives some details: https://github.com/hemeryar/coi-with-popups/blob/main/docs/cross_origin_iframe_popup.MD, along with how we might change that in the future.

• Some tricky topics that might or might not have been discussed:

  • COOP is enforced at redirect time. Does that match with being in the PolicyContainer?
  • Is CSP parsed/enforced at the same time as COOP? What happens to pages setting CSP sandbox?

Hope that helps,
Arthur

[domfarolino]

In general I think this PR is good but the number of places where we don't do the natural COOP inheritance is a bit worrying. I appreciate wanting to do a pure cleanup, but we should at least open an issue to track those.

I've filed #9277 to continue discussion and spec progress on the three cases of probably-wrong inheritance that I we've identified here. Handling those separately from this PR sounds good to me if it's OK with everyone else. This PR now adds XXX boxes pointing to that issue in each of the three inheritance cases.

---

COOP is enforced at redirect time. Does that match with being in the PolicyContainer?

Can you clarify the last part of that question? If it helps, this is the part of the spec that computes the response policy container and COOP enforcement: https://html.spec.whatwg.org/multipage/browsing-the-web.html#create-navigation-params-by-fetching:~:text=Set%20responsePolicyContainer%20to,error%20and%20break.

Is CSP parsed/enforced at the same time as COOP? What happens to pages setting CSP sandbox?

We fail the navigation.

[antosart]

Consider the following example:

• A1 a.test (top-level), with COOP (set by response headers) COOP-a1
• A2 a.test (child iframe of A1), with COOP (set by response headers - has no effects because this is a child iframe, but still) COOP-a2
• A2 does window.open() and opens A3 about:blank (popup)

Before this PR: A3 gets COOP COOP-a1

After this PR: A3 gets COOP COOP-a2

I think if we want to fix it using inheritance, we must let A2 inherit from A1 always when it is same-origin (even for non-local schemes). I am not sure what the best approach is though.

[domfarolino]

Yeah good catch. The current spec's setup of always honoring the COOP headers on subframes (even for non-local schemes) and then having popups inherit their COOP from the initiator-subframe's top-level document is kinda odd. I think there are three options:

1. Have subframes, regardless of origin, unconditionally inherit their COOP from the top-level, and then make popups conditionally inherit their COOP from creator depending on whether creator is same-origin-with-top
2. Have subframes never inherit their creator's COOP, and have popups conditionally inherit their creator's top-level's COOP depending on whether creator is same-origin-with-top
3. Have subframes only inherit their COOP from the top-level if they are same-origin-with-top, and then have popups unconditionally inherit their creator's COOP

Personally I prefer (3) since it keeps COOP inheritance logic centralized in one place, and simplifies new browsing context/document creation here since we don't have to check creator/creator's top-level. Then here we can just always clone the policy container which has been properly redacted earlier if necessary. Spec'ing 3 seems to largely depend on the #create-navigation-params-by-fetching AI in #9277. So in the meantime in this PR we could maybe staple on some logic here to preserve the old behavior: conditionally wipe the COOP as we're doing, and otherwise forcibly inherit from creator's top-level's COOP. Then we can solve the real inheritance problem as part of the other issue. WDYT?

[antosart]

I was just trying to say that, before your PR, I don't see any other place where we would inherit COOP according to policy container rules. For example, if A creates a data url child iframe, I don't think there is any mechanism, before your PR, to have the data url iframe inherit COOP from A. Your PR changes that, because the general policy container inheritance mechanism kicks in. So I am just saying, if we want to make this PR a no-op, would it make sense to change the inheritance algorithm of the policy container so that COOP is never inherited? This would avoid the special handling of srcdoc here. And we could sort out the correct inheritance in a separate PR.