search

w3c / webappsec

Web Application Security Working Group repo
https://www.w3.org/groups/wg/webappsec/
Other
607 stars 148 forks source link

CSP: frame-ancestors should check origins, not URLs #311

Closed dveditz closed 8 years ago

dveditz commented 9 years ago

In CSP section 7.7 for frame-ancestors step 3.2 describes comparing the policy's allowed frame ancestors against the URLs of the parent documents. This will run into trouble when the parent frame's URL is about:blank, about:srcdoc, or blob:. If we insist on using URL then we've effectively made those types of frames unable to have child frames containing documents with with a frame-ancestors policy. Adding any of those explicitly to your policy means you can be framed by anyone so sites must leave them off to stay safe if they care about being framed.

On the other hand, if we use origins then we run into trouble if the parent is a sandboxed iframe, which are specified to have a unique origin unless the allow-same-origin sandbox flag is set.

hillbrad commented 9 years ago

I think we should definitely use Origins, as re-framing parent-origin content from a sandboxed iframe is exactly the most important attack that traversing the full ancestor stack stops vs. XFO top-origin only checks. It is a feature, not a bug, that sandboxed iframes will always fail a frame-ancestors check unless it is fully permissive.

On Sun, Apr 26, 2015 at 10:54 PM Daniel Veditz notifications@github.com wrote:

In CSP section 7.7 for frame-ancestors step 3.2 describes comparing the policy's allowed frame ancestors against the URLs of the parent documents. This will run into trouble when the parent frame's URL is about:blank, about:srcdoc, or blob:. If we insist on using URL then we've effectively made those types of frames unable to have child frames containing documents with with a frame-ancestors policy. Adding any of those explicitly to your policy means you can be framed by anyone so sites must leave them off to stay safe if they care about being framed.

On the other hand, if we use origins then we run into trouble if the parent is a sandboxed iframe, which are specified to have a unique origin unless the allow-same-origin sandbox flag is set.

— Reply to this email directly or view it on GitHub https://github.com/w3c/webappsec/issues/311.

mikewest commented 9 years ago

Hrm. I thought it was a bug, not a feature, that sandboxed frames would fail frame-ancestors checks. We use the URL explicitly to deal with that case. We're using URL in "Privileged Contexts" for the same reason. shrug

We deal with blob: by using the origin of the URL (e.g. blob:[origin]/[guid] -> [origin]), so I don't think that's actually a problem. We ought to skip over srcdoc for the same reason, but I apparently forgot that language in the algorithm.

@hillbrad: If you feel strongly about it, I'm open to changing the behavior.

hillbrad commented 9 years ago

blob: and srcdoc content are essentially content at the origin of the document creating them, so inheriting it makes sense. But the whole point of using a sandboxed iframe (without allow-same-origin) is to get origin isolation for untrusted content. If it can still clickjack you... :(

I know many sites in practice use distinct sandbox origins anyway, but now that we have sandboxing available as part of CSP, also, the reasons to do that are much reduced. It would be nice if the necessity of that could eventually go away.

On Tue, Apr 28, 2015 at 6:27 AM Mike West notifications@github.com wrote:

Hrm. I thought it was a bug, not a feature, that sandboxed frames would fail frame-ancestors checks. We use the URL explicitly to deal with that case. We're using URL in "Privileged Contexts" for the same reason. shrug

We deal with blob: by using the origin of the URL (e.g. blob:[origin]/[guid] -> [origin]), so I don't think that's actually a problem. We ought to skip over srcdoc for the same reason, but I apparently forgot that language in the algorithm.

@hillbrad https://github.com/hillbrad: If you feel strongly about it, I'm open to changing the behavior.

— Reply to this email directly or view it on GitHub https://github.com/w3c/webappsec/issues/311#issuecomment-97064607.

sigkate commented 8 years ago

Is this on the roadmap for CSP3?

It sounds like the desired behavior is that for blob: and srcdoc is that they inherit the origin of the doc where they are defined, and that with sandboxed frames, @hillbrad prefers (and I concur) that they should be considered a unique origin for the purposes of frame-ancestors checks.

mikewest commented 8 years ago

Is this on the roadmap for CSP3?

frame-ancestors is shipping in Chrome and (I believe) Firefox. It's in the CSP2 doc, and will be in CSP3 as well, though I might extract it out to a module. Haven't really decided where the boundaries between "core" and "everything else" are yet.

@hillbrad prefers (and I concur) that they should be considered a unique origin for the purposes of frame-ancestors checks.

Yup. And I think it's reasonable; I'll change frame-ancestors. I'm not sure we can change x-frame-options at the same time, but if we can, we should.