-
Notifications
You must be signed in to change notification settings - Fork 5
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
Special case redirects less #12
Comments
As currently written in the explainer and spec PR, we send a report if the redirect carries reporting headers. We also check at the end if the final page ended up in another BCG or not, and send reports accordingly. We can remove that final check though I am worried that we will miss potentially useful reports. For example, if we do A1->A2->B->A2->A1 and only A1 has reporting, we might miss the BCG switch. Also, one note, when we do A->B->C, the COOP spec as it is written does not compare B with C, but first A with B, and then A with C. So sending a report based on the comparison of B to C does not match what we currently have in the spec. |
That's interesting and seems a little weird, though I also cannot construct an example where the difference matters. But since it does for reporting I wonder if we should adjust that. What do you think? (I think if you want useful reporting you should probably deploy it for your entire origin, especially if you have redirect setups like that.) |
Thinking some more about it, comparing each redirect with each other or with the current page will yield different results in the same-origin-allow-popups case. For example:
The way the spec currently works, in step 2 we compare B (unsafe-none) with A (same-origin-allow-popups) and decide we don't switch (initial navigation). Then in step 3, we compare A (same-origin-allow-popups) with A (same-origin-allow-popups) and decide we don't switch. If we were to compare redirects to each other, the check in step 2 stays the same. However, in step 3 we compare A (same-origin-allow-popups) with B (unsafe-none) and this will return that we should switch BCG, because in this case the current COOP is unsafe-none while the navigation COOP is same-origin-allow-popups (and not the reverse). So by comparing each redirect we do end up with a BCG swap of the popup. I can see arguments for having one behavior or another, so we should agree on which one is the correct one and have the spec align on this. |
Thank you! I think due to confused deputy we should swap in that case. cc @arturjanc |
Note that it would also happen on a redirect to a same origin website. For example: A(same-origin-allow-popups) -> A(unsafe-none) -> A(same-origin-allow-popups) would also trigger a BCG switch if we compare redirects to each other. |
I have created whatwg/html#5739 to use the previous redirect rather than the current page when checking COOP (if we go in this direction), |
I thought about this a bit and I'm leaning towards being more worried about compatibility than about security here, so I think the current behavior (compare A with B, then compare A with C) is marginally better. Basically, from a security point of view, I can't think of a case where an attacker could redirect a document A through a same-origin redirector (with a COOP of 'unsafe-none') to land on a same-origin document C matching the COOP of A (and so retaining the BCG), where the attacker couldn't just directly navigate A to C. But when it comes to compatibility, I'd be worried that it's not always trivial to set COOP headers on redirect responses because sometimes they're generated by a different layer of the application stack or in middleware that writes out a redirect response before other request processors have a chance to run. This means that developers may run into cases where they unexpectedly lose the BCG even on benign same-origin navigations involving redirects. I don't feel very strongly about this though, especially if it would be simpler spec-wise, as Anne suggests. |
The only change is the one outlined before, about same-origin-allow-popups. In other cases, a badly configured redirect header will result in a bcg switch in both cases. It does make things simpler spec wise, since we don't need to keep track of the whole redirect chain and instead only have to reason about two URLs. Also, I did just modify Chrome's implementation to actually do that. |
Alright, let's go with this model; we can keep an eye out for any unforeseen consequences at the application level, and revisit this if there's a good reason to do so. |
This commit modifies the way we handle redirects with COOP. Instead of always comparing a response to the current Document, we will compare it to the previous redirect when enforcing COOP. See camillelamy/explainers#12 for context. Tests: web-platform-tests/wpt#24915.
This commit modifies the way we handle redirects with COOP. Instead of always comparing a response to the current Document, we will compare it to the previous redirect when enforcing COOP. See camillelamy/explainers#12 for context. Tests: web-platform-tests/wpt#24915.
If you navigate from A to B which redirects to C, why would we compare A, B, and C? Why not first compare A and B and report, and then compare B and C and report? That's also how the specification handles the switching and redirects could perfectly carry the reporting headers.
That would reduce the conceptual complexity a bit I think.
The text was updated successfully, but these errors were encountered: