Last Comment Bug 725490 - X-Frame-Options: SAMEORIGIN largely useless as implemented
: X-Frame-Options: SAMEORIGIN largely useless as implemented
Status: NEW
also applies to ALLOW-FROM
: dev-doc-needed, sec-want
Product: Core
Classification: Components
Component: Security (show other bugs)
: 10 Branch
: All All
: -- normal with 1 vote (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
http://tools.ietf.org/id/draft-gondro...
: 877724 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-08 14:59 PST by Michal Zalewski
Modified: 2014-01-12 18:10 PST (History)
23 users (show)
dev.akhawe+mozilla: needinfo? (lcamtuf)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
check ancestors for XFO - wip (3.12 KB, patch)
2012-02-08 20:23 PST, David Chan [:dchan]
no flags Details | Diff | Review
Make XFO check allancestors by default (7.27 KB, patch)
2013-03-02 16:18 PST, Devdatta Akhawe [:devd]
bzbarsky: review+
Details | Diff | Review
Make XFO check allancestors by default (7.40 KB, patch)
2013-03-05 10:45 PST, Devdatta Akhawe [:devd]
no flags Details | Diff | Review
Make XFO check allancestors by default (7.41 KB, patch)
2013-03-05 10:46 PST, Devdatta Akhawe [:devd]
no flags Details | Diff | Review
Make XFO check allancestors by default (7.89 KB, patch)
2013-06-09 00:49 PDT, Devdatta Akhawe [:devd]
dev.akhawe+mozilla: review+
Details | Diff | Review
Tests to check allancestors by default (3.07 KB, patch)
2013-06-09 10:54 PDT, Devdatta Akhawe [:devd]
mozbugs: review-
Details | Diff | Review
Make XFO check allancestors by default (7.90 KB, patch)
2013-06-09 11:02 PDT, Devdatta Akhawe [:devd]
dev.akhawe+mozilla: review+
Details | Diff | Review

Description Michal Zalewski 2012-02-08 14:59:16 PST
Currently, X-Frame-Options: SAMEORIGIN performs a check only against the top-level window. Therefore, it is possible to do have a framing chain such as good.com -> evil.com -> good.com [X-Frame-Options: SAMEORIGIN]

Demo:
http://lcamtuf.coredump.cx/xfo/

Unfortunately, this makes the mechanism largely pointless for a good number of sites. 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.

A simple way to fix it would be to make sure that the SAMEORIGIN check traverses all ancestors, rather than just looking at 'top'. I would expect this to be compatible with current uses of XFO, and actually make the feature useful.
Comment 1 David Chan [:dchan] 2012-02-08 20:23:27 PST
Created attachment 595625 [details] [diff] [review]
check ancestors for XFO - wip

It seems weird that XFO checks for the top level document by design. I'm not sure if the assertions made by the original Microsoft blog post are still true. [1] The top level check seems to cause problems with iframe sandboxes. A game site which may uses iframe sandbox to protect itself, would become vulnerable if their XFO is set to same origin. This is one of the more plausible attacks I can think of.

I've attached a patch which checks each of the ancestors of the document to see if any violate XFO: sameorigin . However there are still some quirks that need to be worked out, and tests written. It blocks the specific PoC, though the current patch will log the following error

"Security Error: Content at http://lcamtuf.coredump.cx/xfo/nested.cgi may not load data from http://beaver-peak.us/xfo2.html."

The error message is clearly wrong since it isn't nested.cgi causing the error.

[1] - http://blogs.msdn.com/b/ie/archive/2009/01/27/ie8-security-part-vii-clickjacking-defenses.aspx#9382230
Comment 2 Daniel Veditz [:dveditz] 2012-02-13 13:01:02 PST
We agree that the current XFO de facto specification of checking against the top frame is pretty useless, which is why we explicitly did it differently in the CSP frame-ancestors directive. That, now, is being deprecated in CSP in favor of a standardized [X-]Frame-Options where I certainly hope the header assertion is checked all the way to the top.

abarth: what is Chromium/webkit doing with XFO these days?
Comment 3 Michal Zalewski 2012-02-13 14:12:42 PST
Also checks top-level. I chatted with Adam and he wasn't excited about fixing XFO (essentially used the argument that it's worth the effort only if I can convince Mozilla or Microsoft to do that).

My take on this is that XFO is one of the relatively few new security mechanisms that are actually depended on heavily, and people have abandoned frame-busting JS in favor of it. So keeping it broken for the foreseeable future seems suboptimal, and the longer it stays in, the harder it will be to get rid of it.
Comment 4 Adam Barth 2012-02-13 17:58:33 PST
> abarth: what is Chromium/webkit doing with XFO these days?

The same as what IE did originally.

Who's the best person to talk with at IE these days about XFO?  Ideally everyone would agree that checking all the ancestors is better and we'd all change our implementations.  I'm wary of changing WebKit alone and would like at least one or two other vendors to change on a similar schedule.
Comment 5 Daniel Veditz [:dveditz] 2012-02-15 21:18:40 PST
I'd be happy to change if we can convince Microsoft or if it's specified in the IETF Frame-Options spec and that gets reasonably close to being accepted. Otherwise we break sites without much to stand on.
Comment 6 Michal Zalewski 2012-02-15 21:27:31 PST
I won't fight too vigorously, but keep in mind that as implemented right now, it offers a false sense of security to many sites that switched from framebusting JS to "X-F-O: SAMEORIGIN" - and that this property is fairly unintuitive.

From that perspective, it almost seems better to drop support for it if no changes are planned.

As for breakage, I suspect it's a lot less likely to cause trouble than most other security changes made in recent years (say, MIME type checking on CSS, frame descendant policy, better HTML parsing algorithms).
Comment 7 Daniel Veditz [:dveditz] 2012-02-29 17:06:26 PST
This issue is already understood amongst the people writing implementations (and attacking sites), we should un-hide this to give site authors a chance to defend themselves.

Any objections from the Chrome or webkit folks on doing that?
Comment 8 Adam Barth 2012-02-29 17:48:31 PST
> Any objections from the Chrome or webkit folks on doing that?

Not from me.
Comment 9 Sam Weinig 2012-02-29 17:54:43 PST
> Any objections from the Chrome or webkit folks on doing that?

None from me either.
Comment 10 David Chan [:dchan] 2012-04-27 10:49:39 PDT
What are the steps to move forward with changing XFO SAMEORIGIN behavior?
Comment 11 Michal Zalewski 2012-08-13 11:08:42 PDT
Hey guys,

Just FYI, we're getting an increasing number of vulnerability reports with exploits that abuse this behavior to effectively bypass XFO.
Comment 12 Michal Zalewski 2012-08-13 13:04:31 PDT
BTW, the draft spec is now updated with a possible solution:

http://tools.ietf.org/id/draft-gondrom-frame-options-02.txt

---
   In the case of SAMEORIGIN and ALLOW-FROM, there is also an optional
   flag "AllAncestors".  If this flag is set, it means that browsers
   MUST validate the URL of each hosting frame up to the top level and
   only allow the framing if all ancestor frames' origins are either the
   same as in SAMEORIGIN or included in the ALLOW-FROM list.
---
Comment 13 David Chan [:dchan] 2012-12-17 14:45:35 PST
I haven't had time to look at this in a while, unassigning so that someone else can take it
Comment 14 Devdatta Akhawe [:devd] 2013-02-24 00:19:25 PST
Couple of data points:

https://github.com/twitter/secureheaders/blob/master/lib/secure_headers/headers/x_frame_options.rb

https://github.com/evilpacket/helmet/blob/master/lib/middleware/xframe.js

Both the *libraries* are getting it wrong (if I am not wrong :). I argue that this should be changed to default to checking all ancestors. I think this is a case where Firefox should care less about the standard and more about doing the right thing for everyone. We can also have a setting where the x-frame-options is ignored if a CSP frame-options is present. Those people who really really want to check only the parent can switch to CSP. I can't imagine who they will be.

If I am not wrong, the effort to switch to checking all ancestors by default should be trivial. I am happy to put in the effort to make it to default check all ancestors. 

What do others think?
Comment 15 Ian Melven :imelven 2013-02-28 11:16:18 PST
(In reply to Devdatta Akhawe [:devd] from comment #14)
> Couple of data points:
> 
> https://github.com/twitter/secureheaders/blob/master/lib/secure_headers/
> headers/x_frame_options.rb
> 
> https://github.com/evilpacket/helmet/blob/master/lib/middleware/xframe.js
> 
> Both the *libraries* are getting it wrong (if I am not wrong :). I argue
> that this should be changed to default to checking all ancestors. I think
> this is a case where Firefox should care less about the standard and more
> about doing the right thing for everyone. We can also have a setting where
> the x-frame-options is ignored if a CSP frame-options is present. Those
> people who really really want to check only the parent can switch to CSP. I
> can't imagine who they will be.
> 
> If I am not wrong, the effort to switch to checking all ancestors by default
> should be trivial. I am happy to put in the effort to make it to default
> check all ancestors. 
> 
> What do others think?

the UI Safety frame options spec says the ancestors should be checked, see
http://www.w3.org/TR/UISafety/#frame-options

"If the directive-value contains the keyword-source 'self' alone to indicate that the resource may be embedded only if all ancestors are in the same origin as the protected resource."

so really i think it should be the other way around - if people want to check all ancestors, they should use CSP (especially since this is where frame-options is actively being worked on currently). That's just my personal opinion though, and there's some open issues in that section of the spec. Also see comment 5, I think breakage is a real issue here for sites currently using XFO.
Comment 16 Devdatta Akhawe [:devd] 2013-03-01 15:58:43 PST
My whole claim is that people don't know that they want to check all ancestors. Based on advice given online or for some other reason I don't know, everyone sets "XFO sameorigin" thinking it protects them against clickjacking. In many cases, it might not---as a result, things are broken. 

See the results on XFO at http://www.veracode.com/blog/2012/11/security-headers-report/

XFO is by far the most widely implemented security header and XFO:SameOrigin is the most common setting. I don't think it is the case that people choosing sameorigin decided "I don't really want to check allancestors". More likely, they didn't know about this threat.
Comment 17 Ian Melven :imelven 2013-03-01 17:11:46 PST
(In reply to Devdatta Akhawe [:devd] from comment #16)
> My whole claim is that people don't know that they want to check all
> ancestors. Based on advice given online or for some other reason I don't
> know, everyone sets "XFO sameorigin" thinking it protects them against
> clickjacking. In many cases, it might not---as a result, things are broken. 
> 
> See the results on XFO at
> http://www.veracode.com/blog/2012/11/security-headers-report/
> 
> XFO is by far the most widely implemented security header and XFO:SameOrigin
> is the most common setting. I don't think it is the case that people
> choosing sameorigin decided "I don't really want to check allancestors".
> More likely, they didn't know about this threat.

ok, maybe we should implement some telemetry to see if this would change the behavior of any sites ? (which is basically implementing this and ignoring the result of checking all ancestors at first)
Comment 18 Devdatta Akhawe [:devd] 2013-03-01 17:33:14 PST
I actually can't think of cases where things would break. Does anyone have examples  where things break?
Comment 19 Michal Zalewski 2013-03-01 17:43:36 PST
I don't think we'd see any breakage on our properties (and most of the XFO headers seen by Veracode are probably our various products / domains, as we set the header by default).

This would only affect a case where somebody has an interim non-same-origin frame in between two same-origin documents - and wants the frame to be shown in this case, but not when the outer document is missing... which seems unlikely.
Comment 20 Ian Melven :imelven 2013-03-01 17:50:22 PST
i agree it's unlikely, but it would be a lot easier to be sure about that if we had telemetry data to support that theory - then if it's true, we turn on the check for real - then maybe other browsers will start enforcing the check as well (is there a Webkit bug for this?)

I still think that we should implement the CSP frame-options directive which has no backwards compatibility concerns, there is code in CSP to do some of it already due to Gecko's non-spec frame-ancestors directive support, but i suppose that is a separate bug.
Comment 21 Devdatta Akhawe [:devd] 2013-03-01 18:36:08 PST
The problem with telemetry is that it has to go through all the channels and reach release, then we have to wait to get data, and then we can switch the defaults. we are talking at the minimum a year's latency. I think this change is far far less likely to cause breakage than changes made in the past (see comment 6) or changes being made currently (3p cookies).

Thanks for filing the bug! given the lack of adoption of CSP currently (see veracode link) and the time it will take for CSP1.1 to be available and adopted, it is also at least 1 year for frame-options to widely effective; likely far more.
Comment 22 Jeff Walden [:Waldo] (remove +bmo to email) 2013-03-01 18:44:15 PST
Is there a reason not to try implementing the change for Nightly and maybe Aurora users, and seeing if anyone reports bustage for a release or two?  No need to inflict it on beta/release without being reasonably certain it'll fly, but at least if people break from this there, we know more than when we started.
Comment 23 Ian Melven :imelven 2013-03-02 14:29:07 PST
(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #22)
> Is there a reason not to try implementing the change for Nightly and maybe
> Aurora users, and seeing if anyone reports bustage for a release or two?  No
> need to inflict it on beta/release without being reasonably certain it'll
> fly, but at least if people break from this there, we know more than when we
> started.

No - I think someone writing a patch for this and at least trying out some major sites with it would be a great start. Isaac Dawson's data as used in his post on the Veracode blog post is public (http://beacua.se/secheaders/secheaders_11022012.zip), so there's a good list of sites using XFO there to begin with there as well. 

I also agree this is much less likely to cause breakage than other recent changes, and your points about lack of CSP uptake are also well taken. 

(In reply to Devdatta Akhawe [:devd] from comment #21)
> The problem with telemetry is that it has to go through all the channels and
> reach release, then we have to wait to get data, and then we can switch the
> defaults. we are talking at the minimum a year's latency. 

I think that's an overexaggeration - telemetry can be uplifted (hopefully fairly quickly) and if we see something like no release users at all being broken by this change in the first month or so, that would be awfully compelling (to me at least.)

> I think this change is far far less likely to cause breakage than changes made > in the past (see comment 6) or changes being made currently (3p cookies).

yes, i agree with that.

> Thanks for filing the bug! given the lack of adoption of CSP currently (see
> veracode link) and the time it will take for CSP1.1 to be available and
> adopted, it is also at least 1 year for frame-options to widely effective;
> likely far more.

yes, that's true. I don't oppose changing the behavior if we have actual data to support the assertions of lack of breakage since Gecko would essentially be 'going first', but as has been said, let's start with a patch :)
Comment 24 Devdatta Akhawe [:devd] 2013-03-02 16:18:01 PST
Created attachment 720345 [details] [diff] [review]
Make XFO check allancestors by default
Comment 25 Devdatta Akhawe [:devd] 2013-03-02 16:21:18 PST
Comment on attachment 720345 [details] [diff] [review]
Make XFO check allancestors by default

Here's a patch. I will need to create tests. Asking for a review from Phil who wrote the original patch.

@jeff: no reason. I don't know how to do that.
Comment 26 Devdatta Akhawe [:devd] 2013-03-02 23:08:15 PST
Also, another datapoint: the MDN XFO docs also make the same mistake and give a possibly false sense of security 

https://developer.mozilla.org/en-US/docs/HTTP/X-Frame-Options
Comment 27 Phil Ames 2013-03-03 06:28:40 PST
I think you might have the wrong person -- I wrote a patch to support X-F-O Allow-From, not X-F-O in general.  While the patch looks reasonable, I'm not familiar enough with the Firefox code to say definitively that it will have the desired effect.

Perhaps someone from https://bugzilla.mozilla.org/show_bug.cgi?id=475530 might be more appropriate?  bsterne, or jst?
Comment 28 Devdatta Akhawe [:devd] 2013-03-03 09:57:06 PST
Comment on attachment 720345 [details] [diff] [review]
Make XFO check allancestors by default

asking bz instead.
Comment 29 Boris Zbarsky [:bz] 2013-03-04 19:59:05 PST
Comment on attachment 720345 [details] [diff] [review]
Make XFO check allancestors by default

>+    // XXXdevd We diverge from the spec for practical reason

How about we file a bug on the spec and reference it here?

>+    // We need to check the location of this window and the location of the each
>     // window, if we're not the top. 

s/the each window/each of our ancestors/

>+    // stop when we see a docshell whose  parent has a system principal, or a

Fix the spacing.

>         else {
>+          return false;

Why?  That doesn't obviously seem right to me...  At the very least needs documentation, though I suspect you just want to treat it as hitting the root.

>+        // If the X-Frame-Options value is SAMEORIGIN, then the parnet frame 

"parent".

r=me with those nits
Comment 30 Devdatta Akhawe [:devd] 2013-03-04 20:14:03 PST
Thanks! I will take care of all the issues save for the return false:

> Why?  That doesn't obviously seem right to me...  At the very least needs
> documentation, though I suspect you just want to treat it as hitting the
> root.

hmm..the "+return false" seems to be a quirk on the diff's part. I didn't add it, it is already there: http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDSURIContentListener.cpp#343 

I think jst suggested adding it in : https://bugzilla.mozilla.org/show_bug.cgi?id=581226#c15
 
I am not sure also why it is return false. I think the idea is that "this shouldn't happen. Lets fail close" Is that convincing enough or do you think this warrants more investigation?  I can ask jst/bsterne, but I doubt either will remember.
Comment 31 Boris Zbarsky [:bz] 2013-03-04 20:26:54 PST
> I think the idea is that "this shouldn't happen. Lets fail close"

OK, I can buy that.
Comment 32 Devdatta Akhawe [:devd] 2013-03-05 10:45:01 PST
Created attachment 721326 [details] [diff] [review]
Make XFO check allancestors by default
Comment 33 Devdatta Akhawe [:devd] 2013-03-05 10:46:17 PST
Created attachment 721327 [details] [diff] [review]
Make XFO check allancestors by default
Comment 34 Ian Melven :imelven 2013-03-07 13:14:53 PST
I'd like to see some mochitests for this change, please.
Comment 35 Ian Melven :imelven 2013-03-07 13:16:27 PST
Also, it looks like you can carry over bz's r+ on the latest patch :)
Comment 36 Daniel Veditz [:dveditz] 2013-03-29 13:15:21 PDT
update on the Veracode report linked from comment 16, they're re-run the survey:

http://www.veracode.com/blog/2013/03/security-headers-on-the-top-1000000-websites-march-2013-report/
Comment 37 Daniel Veditz [:dveditz] 2013-05-31 15:08:52 PDT
*** Bug 877724 has been marked as a duplicate of this bug. ***
Comment 38 Daniel Veditz [:dveditz] 2013-05-31 15:11:42 PDT
And while we're cautiously worrying over here about breaking web compatibility, it looks like IE has simply done this since IE 9 according to http://webstersprodigy.net/2012/09/13/clickjacking-google/ (thanks for the link, Johnathan). I tested IE 10 and it does seem to enforce SAMEORIGIN all the way to the top, as if the proposed "AllAncestors" flag were set.

We should just do it (and do the same for ALLOW-FROM)
Comment 39 Devdatta Akhawe [:devd] 2013-06-09 00:49:49 PDT
Created attachment 760181 [details] [diff] [review]
Make XFO check allancestors by default
Comment 40 Devdatta Akhawe [:devd] 2013-06-09 00:52:06 PDT
Comment on attachment 760181 [details] [diff] [review]
Make XFO check allancestors by default

Given the nature of the fix, do I need to replicate each and every XFO test but this time for all-ancestors? Or would just 1 test for all ancestors be sufficient and we can rely on existing tests for everything else? It seems quite unnecessary to create 2 copies of the same tests---both would exercise the same code base.
Comment 41 Devdatta Akhawe [:devd] 2013-06-09 10:54:12 PDT
Created attachment 760225 [details] [diff] [review]
Tests to check allancestors by default
Comment 42 Devdatta Akhawe [:devd] 2013-06-09 11:02:56 PDT
Created attachment 760227 [details] [diff] [review]
Make XFO check allancestors by default
Comment 43 Devdatta Akhawe [:devd] 2013-06-09 11:05:43 PDT
Comment on attachment 760227 [details] [diff] [review]
Make XFO check allancestors by default

carrying over r+ from bz
Comment 44 Devdatta Akhawe [:devd] 2013-06-09 11:07:40 PDT
Comment on attachment 760225 [details] [diff] [review]
Tests to check allancestors by default

Ian: Can you take a look at the test I added? Also, can you answer comment 40?
Comment 45 Devdatta Akhawe [:devd] 2013-06-12 15:10:52 PDT
try looks good to me.
https://tbpl.mozilla.org/?tree=Try&rev=674c7249e71f
Comment 46 Ian Melven :imelven 2013-06-14 20:07:13 PDT
For the record, I strongly support doing this given comment 38, I'm going to take a look at Dev's patches as soon as I can and have asked a few other people if they have time to take a look too.
Comment 47 David Chan [:dchan] 2013-07-15 10:20:03 PDT
I originally thought there might be some issues with mozbrowser iframes, but the code looks like it works for it. 

Would it be possible to add some tests for mozbrowser? I can help you with setting up the needed page permissions. I think a single test with mozbrowser at the top level would be sufficient. The main thing I want to make sure is covered is the break clause for the IsBrowserOrApp(). Granted the code flow should check the parent before setting curDocShell

mozbrowser frame origin = bar
 iframe origin = foo
   iframe origin = foo XFO = SAMORIGIN
Comment 48 Devdatta Akhawe [:devd] 2013-07-15 10:27:06 PDT
I am not sure that should be part of this bug---the mozbrowser issue existed before this too, right? I am surprised the previous implementation didn't have to write a mozbrowser test.
Comment 49 Garrett Robinson [:grobinson] 2013-09-11 16:51:17 PDT
Comment on attachment 760225 [details] [diff] [review]
Tests to check allancestors by default

Punting to Sid, as I do not feel qualified to review this.
Comment 50 Sid Stamm [:geekboy or :sstamm] 2013-09-12 10:11:05 PDT
Comment on attachment 760225 [details] [diff] [review]
Tests to check allancestors by default

Review of attachment 760225 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/test/file_x-frame-options_main.html
@@ +40,5 @@
>  </script>
>  
>  
> +<iframe id="grandchild-same-origin-deny" src="http://example.com/tests/content/base/test/file_x-frame-options_page.sjs?testid=grandchild-same-origin&xfo=gcs"></iframe><br>
> +

Change this to use addFrame (see code just above it) to make this more readable.

Please also put in a comment that explains how this test is going to differ from the rest; for example:
// this creates a gcs iframe that has a subframe for test sameorigin1 
// to test that a grandchild frame checks all ancestors.

Please additionally rename gcs to something that is more meaningful.  What this test is really doing is creating a subframe so maybe something like parent_for_sameorigin1 (means that this frame won't have an XFO, but its subframe will, and the xfo value will be keyed as sameorigin1).

::: content/base/test/file_x-frame-options_page.sjs
@@ +51,5 @@
>    }
> +  else if (query['xfo'] === 'gcs') {
> +    response.write("<iframe src='http://mochi.test:8888/tests/content/base/test/file_x-frame-options_page.sjs?testid=sameorigin1&xfo=sameorigin'></iframe>");
> +    return;
> +  }

else is a little misleading and not necessary here.  This block of code is doing something completely different than the if block above it, so please put in a comment to explain what this code is doing.

I think testid=sameorigin1 is already taken by another test in file_x-frame-options_main.html.  Please use a new for this test (allancestors or something).  You can reuse the xfo value, but don't reuse the testid value or it might confuse the test harness.

::: content/base/test/test_x-frame-options.html
@@ +143,5 @@
> +  frame = harness.contentDocument.getElementById("sameorigin1");
> +  var test4 = frame.contentDocument.getElementById("test").textContent;
> +  is(test4, "sameorigin1", "test sameorigin1");
> +  is(test12, null, "test allow-from-deny");
> +

This is confusing to me.  Please make a clear comment to explain how you're "checking allancestors" and use new variable names (test13, test4 are already defined and you're redefining them here).
Comment 51 Ian Melven :imelven 2013-10-21 11:00:42 PDT
Just as a heads up, Blink has made this change and has experienced some breakage already : see https://code.google.com/p/chromium/issues/detail?id=305244 

so it seems like our concerns about breakage weren't completely unfounded... 

dveditz has also proposed that frame-options be included as part of the CSP 1.1 spec, which I strongly support - we can spec things right for frame-options the first time and avoid the problems with XFO..
Comment 52 Devdatta Akhawe [:devd] 2013-10-21 13:24:08 PDT
The breakage is in Google and the comment 11 says that they need this behavior to stop exploits. So, I am not sure whether to count this as actual breakage. Maybe Michal can elaborate?
---
The status: I am currently swamped and haven't worked on Firefox for a while. I am not sure when I will get to it, so anyone else who has time should feel free to fix the test.
Comment 53 Devdatta Akhawe [:devd] 2014-01-12 18:10:44 PST
While the fix to the tests are not too hard, I am going to wait for Michal to elaborate on the breakage due to XFO allancestors and whether this patch should be blocked or not. While I think this patch is the right thing to do, we can't really afford to break google.

Note You need to log in before you can comment on or make changes to this bug.