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

Enforce CORP on "navigate" request mode #1113

Closed
shhnjk opened this issue Nov 5, 2020 · 14 comments
Closed

Enforce CORP on "navigate" request mode #1113

shhnjk opened this issue Nov 5, 2020 · 14 comments
Labels
security/privacy There are security or privacy implications

Comments

@shhnjk
Copy link

shhnjk commented Nov 5, 2020

There is an inconsistency in CORP, where CORP is not enforced on a request with navigate mode (e.g. a site embedded by an iframe). While I understand that X-Frame-Options or CSP: frame-ancestors can do this, there are few problems.

  1. Developer might think that a resource (e.g. image) is protected by CORP, but it can be leaked if that resource is loaded by an iframe (assuming that the browser doesn't support Site Isolation). While SharedArrayBuffer might not be available in this context, attacker can still take advantage of the amplification technique.
  2. Destination URL of a redirect can be leaked because XFO is not enforced in a redirect.

I think it'd be better to enforce CORP on a request with navigate mode as well.

@annevk
Copy link
Member

annevk commented Nov 5, 2020

See the discussion at #687 (comment). I'm not sure that's worth revisiting.

I think it makes sense to enforce XFO for redirects. HTML doesn't do that currently, but that could be changed. I'm not entirely sure I understand the analysis in the Chromium bug. Is the assumption that all those redirects do not want to be blocked? Why?

@shhnjk
Copy link
Author

shhnjk commented Nov 5, 2020

I'm not sure that's worth revisiting.

I do think it’s worth revisiting. The requirement of XFO together with CORP isn’t well explained anywhere in the spec or MDN (AFAICT). And implementing this would also allow developer to use “same-site” keyword which isn’t available in XFO. I think this change is more logical and easy for devs, than what we have today.

I think it makes sense to enforce XFO for redirects. HTML doesn't do that currently, but that could be changed.

Sure, but Chrome doesn’t want to implement this so far. And it’ll be much easier to just change CORP than landing a breaking change.

I'm not entirely sure I understand the analysis in the Chromium bug.

XFO should be enforced regardless of redirect :) Which isn’t the case today, and that would leak some secrets.

@annevk annevk added the security/privacy There are security or privacy implications label Nov 5, 2020
@annevk
Copy link
Member

annevk commented Nov 5, 2020

Well:

  1. Why is changing CORP more compatible? That's just as much of a breaking change and was actually considered as part of the design process and rejected.
  2. What are the reasons for Chromium not willing to change XFO?
  3. Why couldn't we add same-site to XFO if that's needed?

@shhnjk
Copy link
Author

shhnjk commented Nov 5, 2020

Why is changing CORP more compatible? That's just as much of a breaking change and was actually considered as part of the design process and rejected.

Because people adding CORP response header to resources loaded iframes (including redirects) would be lesser at this point then people specifying XFO. Why do you think enforcing CORP in requests from iframes is a breaking change (other than spec-wise)?

What are the reasons for Chromium not willing to change XFO?

You can read the thread in the bug, but they see 0.75% out of all subframe navigation to have XFO with redirect response status. Which is high enough that they won't be able to change the behavior. However, I would expect CORP to be much lesser in the same matrix.

Why couldn't we add same-site to XFO if that's needed?

If we are able to change the current behavior of XFO (i.e. enforce on redirect), then we can add same-site to XFO, and that's fine. However, I don't see that happening.

@shhnjk
Copy link
Author

shhnjk commented Nov 5, 2020

CC: @mikewest @arturjanc

@annevk
Copy link
Member

annevk commented Nov 6, 2020

Enforcing CORP for navigations might break navigations that are expected to succeed.

I had read the Chromium thread, and what I don't understand is why the conclusion is that blocking those fetches would be bad. It seems some more analysis would be needed to reach that conclusion.

@mikewest
Copy link
Member

mikewest commented Nov 9, 2020

With regard to XFO: I didn't do any additional research beyond getting a count. It's something that's broken applications in the past (both Google Drive and Dropbox both served XFO on redirect responses that were expected to turn into downloads, AFAIR), and given that experience, it didn't seem worth the time it would take to dive into HTTP Archive and/or add additional metrics to track things down more closely. I'd welcome analysis that showed it to be less likely to break things than my priors suggest.

@annevk
Copy link
Member

annevk commented Nov 9, 2020

Thanks, it does sound like there's a potential path there for "allowing a download if it turns out to be a download or is a download, and blocking otherwise", but maybe it was not the sole reason and that would also be a fair bit more involved than blocking.

@arturjanc
Copy link

I don't think I like the idea of CORP applying to frames for the following reasons:

  1. There are already two overlapping mechanisms that control embeddability (X-Frame-Options and frame-ancestors), and this would add a third one. CORP was explicitly designed as a complement to these mechanisms that applies only in no-cors mode, rather than as a general embedding control that would subsume them -- expanding its scope this way would be a significant design change after it already shipped.
  2. Bundling embedding and subresource controls makes the adoption of CORP more difficult. With this change, if you wanted to set CORP of same-site application-wide, you'd also disallow framing, which may not be what you want (and this is also backwards-incompatible for sites that already enabled CORP).
    • I guess we could come up with some inheritance logic where CORP only applies if the site fails to set X-Frame-Options or frame-ancestors, but this is complicated and doesn't solve the problem of XFO being ignored on redirects.

IMHO a better avenue here is to fix the handling of XFO on redirect responses and/or update the MDN page about CORP to tell developers they should set X-Frame-Options application-wide, which is already a best practice. Otherwise, given that the concern here is limited to browsers without OOPIFs, I don't think it's a good long-term approach to change the semantics of CORP (especially assuming browsers need to ship OOPIFs anyway because they can't depend on developers enabling CORP everywhere).

@youennf
Copy link
Collaborator

youennf commented Nov 23, 2020

I agree with @arturjanc. CORP is a subresource control mechanism, where you have a context/origin attached to the load. It seems best to keep CORP within this scope.
That does not preclude to either fix the existing navigation control mechanisms or complement them with new ones if the existing mechanisms are difficult to fix for compatibility reasons.

@shhnjk
Copy link
Author

shhnjk commented Nov 23, 2020

Okay, I agree with @arturjanc's 2nd reason that this change makes adoption of CORP more difficult.

However, I think we still have few gotchas for developers in frames vs resources protection.

  1. resourcepolicy.fyi, web.dev, and MDN, doesn't explain about requirement of XFO for protecting resources addition to CORP.
  2. Adding XFO to all sensitive HTML files won't save developers, because HTML files can be loaded using img tag, because Firefox doesn't seem to support CORB. This mean developers has to add CORP to all sensitive HTML files even though they don't see the change affecting the frame loading.

While it seems like enforce CORP on navigations are a bad idea, I think we need to do a better job of explaining what developers need to do (i.e. supply XFO AND CORP to all sensitive responses).

@mikewest
Copy link
Member

because Firefox doesn't seem to support CORB.

That's a little surprising. @annevk, is Firefox pushing forward on ORB instead? Something in this area seems necessary.

I think we need to do a better job of explaining what developers need to do (i.e. supply XFO AND CORP to all sensitive responses).

I think we'd be better served by spending time on changing the default behavior so that developers don't have to think about the risk unless they actually want their resources to be embeddable: https://github.com/mikewest/embedding-requires-opt-in/ is something I'd like us to do in the near future, for example. Doing the same for CORP is a huge lift, but perhaps there are alternatives I haven't thought about.

@annevk
Copy link
Member

annevk commented Nov 30, 2020

We would like to push ORB in due course, yes.

@annevk
Copy link
Member

annevk commented Jan 30, 2021

It seems like this can be closed? (We already have various issues tracking the final design of CORB/ORB.)

@annevk annevk closed this as completed May 25, 2022
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
Development

No branches or pull requests

5 participants