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

Should pages with different COOP be able to get unload event start/end. #169

Closed
hemeryar opened this issue Feb 8, 2022 · 13 comments · Fixed by whatwg/html#7884
Closed

Comments

@hemeryar
Copy link
Member

hemeryar commented Feb 8, 2022

The use of Cross-Origin-Opener-Policy header may lead to BrowsingContext group swaps. In that case, Chrome does not report the values, see https://bugs.chromium.org/p/chromium/issues/detail?id=1156559.

It seems reasonable that pages in different BrowsingContext groups, potentially in different processes with different crossOriginIsolated values don't have access to each other's timing. In that case, this needs to be spec'd somehow.

@yoavweiss
Copy link
Contributor

OK, looking at context group switches, it seems like this can happen if the two pages have different COOP values.
I agree that it makes sense to treat this case similarly to the cross-origin case, and not expose the unload timings here.

@noamr - thoughts?

@noamr
Copy link
Contributor

noamr commented Feb 9, 2022

OK, looking at context group switches, it seems like this can happen if the two pages have different COOP values. I agree that it makes sense to treat this case similarly to the cross-origin case, and not expose the unload timings here.

@noamr - thoughts?

Yea this makes sense to me that unloadEventStart is hidden for BCG switches. Will draft something for the HTML spec.

@noamr
Copy link
Contributor

noamr commented Mar 28, 2022

Maybe rather than not reporting these values at all, the unload event timing should be coarsened by the most strict cross-origin isolated capability of the unloading and the loading document? That would be more in line with how this is handled for service-workers, iframes etc.

@yoavweiss
Copy link
Contributor

@hemeryar - thoughts on the above?

@hemeryar
Copy link
Member Author

In general I think coarsening is appropriate when we can use the API in multiple contexts with different security guarantees. Here it's giving information across contexts with different security level. Imagine a crossOriginIsolated page has an unload of 200ms if the user is logged in and 1ms otherwise, even with coarsening we would leak information, unless we make it coarse to the points it is worthless.

In this specific case, Arthur Sonzogni pointed in the bug: "Unloading and loading are now running in parallel instead of sequentially." That's because we're using different processes, so I think the values might not even make sense.

@yoavweiss
Copy link
Contributor

In this specific case, Arthur Sonzogni pointed in the bug: "Unloading and loading are now running in parallel instead of sequentially." That's because we're using different processes, so I think the values might not even make sense.

That's an interesting point, as it means that the value is not of interest to the following page.
@noamr - thoughts?

@noamr
Copy link
Contributor

noamr commented May 1, 2022

In general I think coarsening is appropriate when we can use the API in multiple contexts with different security guarantees. Here it's giving information across contexts with different security level. Imagine a crossOriginIsolated page has an unload of 200ms if the user is logged in and 1ms otherwise, even with coarsening we would leak information, unless we make it coarse to the points it is worthless.

What can leak here is if the unload does something sensitive with a COEP embedded resource while unloading and that high-resolution time is now accessible to the newly loaded event.
Coarsening that time should be sufficient to mitigate this IMO.

In this specific case, Arthur Sonzogni pointed in the bug: "Unloading and loading are now running in parallel instead of sequentially." That's because we're using different processes, so I think the values might not even make sense.

I think this should be spelled out in the spec. This whole step is currently synchronous in the document unloading step.

I believe the following can work:

If BCG is replaced due to COOP:
-> The user agent may unload the previous document in parallel, in which case unload event timing will not be measured.
-> If those timings are present (the unloading is synchronous), they need to be coarsened if either of the BCs is cross-origin isolated.

noamr added a commit to noamr/html that referenced this issue May 3, 2022
Sometimes a navigation causes a BCG swap, e.g. when the
cross-origin isolated capability changes.

In that case:
- the timestamps should coarsened based on both documents
- Some user-agents unload in parallel, which should not become
  part of the new document's timing info
- Time origin should be according to the new global's time origin, but coarsened by both.

Closes w3c/navigation-timing#169
@noamr
Copy link
Contributor

noamr commented May 3, 2022

In this specific case, Arthur Sonzogni pointed in the bug: "Unloading and loading are now running in parallel instead of sequentially." That's because we're using different processes, so I think the values might not even make sense.

That's an interesting point, as it means that the value is not of interest to the following page. @noamr - thoughts?

whatwg/html#7884

@domenic
Copy link
Contributor

domenic commented May 3, 2022

If we provide unload timing info based on whether or not a BCG switch occurs, I believe this makes BCG switches much more observable than they were before.

In particular, my understanding---CCing @rakina and @altimin to correct me if I'm wrong---is that currently BCG switches are only web-observable for windows with openers, who see the opener relationship severed. And as such, that is the only time they are specced to take place. But in Chromium, we perform BCG switches much more frequently, basically whenever we can.

If we make unload timing nulled out whenever a BCG switch occurs, I think then Chromium's departure from the spec becomes observable, i.e. the spec would start constraining our freedom to switch BCGs often. That seems bad.

So---again, if my understanding is correct---I think we'd need to go one of a few possible routes:

  • Allow (or require???) BCG switches under a larger variety of circumstances than the spec currently does. Then say unload timing info is always null for BCG switches.

  • Allow (or require???) BCG switches under a larger variety of circumstances than the spec currently does. Then say unload timing info is sometimes null for BCG switches, or sometimes is the coarsened version.

  • Keep spec-BCG switches like they are currently. Then say that unload timing info can be null at implementation-defined times (most of which will happen to correspond to implementation BCG switches).

/cc @annevk who I think has mentioned a desire to expand the spec's notion of BCG switches in the past.

@noamr
Copy link
Contributor

noamr commented May 4, 2022

If we provide unload timing info based on whether or not a BCG switch occurs, I believe this makes BCG switches much more observable than they were before.

This is already as observable, by calling window.crossOriginIsolated and seeing that the values are different, run a quick high-resolution measuring of something and see if it's coarsened differently, or try to open one of them as a popup and see that you don't have access to the window object. Note that all of this is only relevant in the context of the same origin as unload timing is zeroed across origins.

In particular, my understanding---CCing @rakina and @altimin to correct me if I'm wrong---is that currently BCG switches are only web-observable for windows with openers, who see the opener relationship severed. And as such, that is the only time they are specced to take place. But in Chromium, we perform BCG switches much more frequently, basically whenever we can.

If we make unload timing nulled out whenever a BCG switch occurs, I think then Chromium's departure from the spec becomes observable, i.e. the spec would start constraining our freedom to switch BCGs often. That seems bad.

It also wouldn't make the BCG switch less observable. The unloading document can measure their own unload event timing and see that it's been nulled for the new document's navigation timing.

So---again, if my understanding is correct---I think we'd need to go one of a few possible routes:

  • Allow (or require???) BCG switches under a larger variety of circumstances than the spec currently does. Then say unload timing info is always null for BCG switches.
  • Allow (or require???) BCG switches under a larger variety of circumstances than the spec currently does. Then say unload timing info is sometimes null for BCG switches, or sometimes is the coarsened version.

So, the BCG switch will be observable but not the reason for it? as mentioned before, a COIP switch is already observable so this would obscure something the document already knows.

  • Keep spec-BCG switches like they are currently. Then say that unload timing info can be null at implementation-defined times (most of which will happen to correspond to implementation BCG switches).

I believe all these options obscure the observable information a bit but don't really fix it. Maybe that's OK, it's not a cross-origin leak.

I don't think there is a way to make parallelizing the unload not observable when it's a same-origin navigation, apart for maybe avoid doing that if there is an unload event, as if the developer really wanted that information they could send beacons to a service worker before/after the load event and see that it's parallel with metrics in the new document.

@rakina
Copy link
Member

rakina commented May 4, 2022

But in Chromium, we perform BCG switches much more frequently, basically whenever we can.

Just to clarify, we do BCG switches when we have a need for it: when the current document might be able to get BFCached, or when "swapping in" a prerendered document. For the first case we won't fire unload anyways so there is nothing to set. For the latter case the prerendered document will already exist before the "swapped out" previous document is unloaded, so there is nothing to set also (I think?). So I think we kinda avoided the problem here? Would be different if it's "pagehide timing" instead, where we do fire pagehide for the BFCache case.

Also linking whatwg/html#6563 (comment) as it's kinda related.

@noamr
Copy link
Contributor

noamr commented May 4, 2022

But in Chromium, we perform BCG switches much more frequently, basically whenever we can.

Just to clarify, we do BCG switches when we have a need for it: when the current document might be able to get BFCached, or when "swapping in" a prerendered document. For the first case we won't fire unload anyways so there is nothing to set. For the latter case the prerendered document will already exist before the "swapped out" previous document is unloaded, so there is nothing to set also (I think?). So I think we kinda avoided the problem here? Would be different if it's "pagehide timing" instead, where we do fire pagehide for the BFCache case.

Also linking whatwg/html#6563 (comment) as it's kinda related.

OK, that's different from the different-COOP case.
As you say, Both BFCache and prerender are irrelevant use cases for unloading.

@hemeryar
Copy link
Member Author

hemeryar commented May 4, 2022

I think the PR looks reasonable. It ends up not being a security measure as much as a sanity improvement for multi-process navigations. Thanks Noam!

noamr added a commit to noamr/html that referenced this issue May 24, 2022
Sometimes a navigation causes a BCG swap, e.g. when the
cross-origin isolated capability changes.

In that case:
- the timestamps should coarsened based on both documents
- Some user-agents unload in parallel, which should not become
  part of the new document's timing info
- Time origin should be according to the new global's time origin, but coarsened by both.

Closes w3c/navigation-timing#169
noamr added a commit to noamr/html that referenced this issue May 24, 2022
Sometimes a navigation causes a BCG swap, e.g. when the
cross-origin isolated capability changes.

In that case:
- the timestamps should coarsened based on both documents
- Some user-agents unload in parallel, which should not become
  part of the new document's timing info
- Time origin should be according to the new global's time origin, but coarsened by both.

Closes w3c/navigation-timing#169
domenic pushed a commit to whatwg/html that referenced this issue May 31, 2022
Sometimes a navigation causes a BCG swap, e.g. when the cross-origin isolated capability changes. In that case:

* The timestamps should coarsened based on both documents.
* Some user-agents unload in parallel, which should not become part of the new document's timing info.
* Time origin should be according to the new global's time origin, but coarsened by both.

Closes w3c/navigation-timing#169.
mfreed7 pushed a commit to mfreed7/html that referenced this issue Jun 3, 2022
Sometimes a navigation causes a BCG swap, e.g. when the cross-origin isolated capability changes. In that case:

* The timestamps should coarsened based on both documents.
* Some user-agents unload in parallel, which should not become part of the new document's timing info.
* Time origin should be according to the new global's time origin, but coarsened by both.

Closes w3c/navigation-timing#169.
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 a pull request may close this issue.

5 participants