'X-Frame-Options: SAMEORIGIN' should check all ancestor frames.

'X-Frame-Options: SAMEORIGIN' should check all ancestor frames.

Currently, XFO performs a same origin check only against the top-level
frame in a document's ancestor chain. As lcamtuf notes in [1], "Any site
that allows a rogue ad to be displayed in an IFRAME; or that frames
third-party content for other reasons (e.g., iGoogle, Image Search
results, Facebook gadgets), is effectively not protected, because the
framed content from evil.com can load and arbitrarily decorate any page
in the same origin as the top-level window, and entice the user to
interact with it."

This patch adjusts Blink's behavior to check each of a document's
ancestors, and blocks the load if any aren't same-origin with the
document being loaded.

[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=725490

BUG=250309
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=158466

[Mike West, 2013-07-26 22:10:08]

Hi Tom, Adam. This patch follows up on our earlier discussion around
X-Frame-Options and ancestors by blocking loads for good.com -> bad.com
->good.com when SAMEORIGIN is specified.

It looks like Mozilla has approved but not yet landed a patch[1] , and there
seems to be general agreement that this is the direction we'd like to go. I'd
suggest that we're early enough in the M30 cycle that we could land this change
and still have a relatively high confidence of evaluating its impact before it
hits stable.

WDYT?

-mike

[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=725490

[abarth-chromium, 2013-07-27 00:12:13]

What data did we get back from the UseCounter?

[Mike West, 2013-07-27 16:10:28]

On 2013/07/27 00:12:13, abarth wrote:
> What data did we get back from the UseCounter?

Assuming that I'm poking at UMA correctly (which I'm not sure I'd assume: the
"PageVisits" and "PageDestruction" data looks strange...), things look good from
the last 7 days of M28:

* We check for X-Frame-Options blockage on ~14% of pages.
* ~1% of those pages use SAMEORIGIN.
* 0.02% of those pages would have been effected by this patch.

[Mike West, 2013-07-27 19:08:50]

On 2013/07/27 16:10:28, Mike West wrote:
> On 2013/07/27 00:12:13, abarth wrote:
> > What data did we get back from the UseCounter?
> 
> Assuming that I'm poking at UMA correctly (which I'm not sure I'd assume: the
> "PageVisits" and "PageDestruction" data looks strange...), things look good
from
> the last 7 days of M28:
> 
> * We check for X-Frame-Options blockage on ~14% of pages.
> * ~1% of those pages use SAMEORIGIN.
> * 0.02% of those pages would have been effected by this patch.

Now that Adam's taught me to read the data, things look slightly different, and
quite different across platforms.

Across platforms, we check for XFO on ~20-25% of pages, and about 4% of _those_
pages use SAMEORIGIN.

On Windows, ~59% of those SAMEORIGIN pages would be effected by this patch. On
Mac, ~15%, and Linux, ~10%. Android: 

I have a hard time accounting for that discrepancy. It makes little sense,
honestly. I'm just going to assume I'm misreading the data. :)

[Tom Sepez, 2013-09-26 16:55:37]

Drive-by: ping.  What does it take to get this going again?  Is it time to roll
this into a dev release (only) and see who yells?

[Mike West, 2013-09-26 17:35:33]

On 2013/09/26 16:55:37, Tom Sepez wrote:
> Drive-by: ping.  What does it take to get this going again?  Is it time to
roll
> this into a dev release (only) and see who yells?

I'm happy to do that. I was a bit surprised at the UMA numbers; I'll look at
them again tomorrow to see if things have evened themselves out across
platforms.

Thanks for the ping.

[Mike West, 2013-09-27 09:47:18]

On 2013/09/26 17:35:33, Mike West wrote:
> On 2013/09/26 16:55:37, Tom Sepez wrote:
> > Drive-by: ping.  What does it take to get this going again?  Is it time to
> roll
> > this into a dev release (only) and see who yells?
> 
> I'm happy to do that. I was a bit surprised at the UMA numbers; I'll look at
> them again tomorrow to see if things have evened themselves out across
> platforms.
> 
> Thanks for the ping.

I've taken another look at the UMA stats for the last week:

We check for X-Frame-Options blockage on ~20% of pages. ~5% of _those_ pages use
SAMEORIGIN. This patch would effect another ~2% of _those_ pages (0.4% of all
pages).

That still seems absurdly high. It's impossible to tell from the data whether
they're cases where we'd really want to block the load, or honest configurations
that rely on the currently lax structure.

I'm with Tom: since we've just branched for M31, I'd like to land this change
and give it some time to bake in Canary/Dev to see what explodes. Adam, WDYT?

[abarth-chromium, 2013-09-27 16:46:16]

lgtm

I suspect we'll need to roll this back, but we can try it.

[commit-bot: I haz the power, 2013-09-27 16:46:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkwst@chromium.org/20822002/5001

[commit-bot: I haz the power, 2013-09-27 19:29:31]

Change committed as 158466