Last Comment Bug 707893 - Add a (temporary) global pref to ignore X-Frame-Options
: Add a (temporary) global pref to ignore X-Frame-Options
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: unspecified
: x86 Linux
: -- normal (vote)
: mozilla12
Assigned To: Justin Lebar (not reading bugmail)
:
:
Mentors:
: 712974 (view as bug list)
Depends on: 704037
Blocks: 770239
  Show dependency treegraph
 
Reported: 2011-12-06 00:10 PST by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2012-07-02 10:10 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (3.06 KB, patch)
2011-12-22 10:32 PST, Justin Lebar (not reading bugmail)
no flags Details | Diff | Splinter Review
Patch v2 (3.65 KB, patch)
2011-12-22 11:51 PST, Justin Lebar (not reading bugmail)
bzbarsky: review+
Details | Diff | Splinter Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-12-06 00:10:17 PST
For testing the browser app in b2g, we'd like to temporarily disable X-Frame-Options checking.  This might be generally useful (maybe), so if we do this, I'd be happy to make it a pref and land on m-c.  We won't need this workaround for too long.

If anyone strongly objects, or no one else would find it useful, we can close out this bug.
Comment 1 Ben Francis [:benfrancis] 2011-12-22 04:44:29 PST
I was going to set this as blocking browser-as-webapp (https://bugzilla.mozilla.org/show_bug.cgi?id=693515) but what's described here is just a workaround.

If this is a workaround, what is the long term solution? Should all <iframe browser> elements ignore X-Frame-Options and continue to render the page?
Comment 2 Justin Lebar (not reading bugmail) 2011-12-22 07:09:42 PST
(In reply to Ben Francis from comment #1)
> If this is a workaround, what is the long term solution? Should all <iframe
> browser> elements ignore X-Frame-Options and continue to render the page?

Yes, I think so.

Is this really getting in your way at the moment?
Comment 3 Ben Francis [:benfrancis] 2011-12-22 08:08:14 PST
No it's not getting in my way at all because I just test the browser on pages without X-Frame-Options, but it will certainly get in users' way. 

Google, Facebook, Twitter, YouTube, GMail, Google Calendar, Github... most popular web apps disallow rendering inside iFrames in order to prevent phishing. That makes it a blocker as far as a useful browser is concerned.
Comment 4 Justin Lebar (not reading bugmail) 2011-12-22 08:11:17 PST
The question is: Should we hack around it, or fix it properly?  We can probably fix it properly, unless it's getting in your way right now and you need a fix.
Comment 5 Justin Lebar (not reading bugmail) 2011-12-22 08:27:53 PST
It sounds like from IRC that cjones thinks this will be In The Way within a week.
Comment 6 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-12-22 08:50:48 PST
<cjones> hi bz, are you ok with a quick hack to disable x-frame-options checking, behind a pref?
<cjones> this is to unblock the browser app for b2g, for a quarterly goal
<cjones> the downside is ugly, sorry
<cjones> upside is we can start testing the browser in real phones sooner
* jfkthame (jfkthame@7CAC30AF.5BC345F5.9542EC20.IP) has joined #developers
* Mano has quit (Ping timeout)
<pr0pagandhi> any plans for an iOS version?
* Wes (chatzilla@A1FEE3E8.E3DA2587.9A5171B3.IP) has joined #developers
<smaug> ted: ^
* smontagu (chatzilla@moz-13AE8AF9.red.bezeqint.net) has joined #developers
* mcote|chiro is now known as mcote
<khuey> I think that project died with native UI for android
<bz> cjones: worksforme
* victorporof (victorporo@3E36101D.6BD22D89.79933D60.IP) has joined #developers
<khuey> hmm
* victorporof has quit (Connection reset by peer)
<bz> cjones: can you make it check system prefs only?
<bz> cjones: so users can't set it by accident?

Justin might be able to get to this today.  If not, please un-assign.

When we have bug 704037 or a followup, we will remove this hack.
Comment 7 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-12-22 08:51:00 PST
*** Bug 712974 has been marked as a duplicate of this bug. ***
Comment 8 Justin Lebar (not reading bugmail) 2011-12-22 10:32:43 PST
Created attachment 583853 [details] [diff] [review]
Patch v1
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2011-12-22 10:39:13 PST
Comment on attachment 583853 [details] [diff] [review]
Patch v1

Won't this possibly still pick up user.js values for this pref?
Comment 10 Justin Lebar (not reading bugmail) 2011-12-22 10:42:39 PST
I thought lockPref() was supposed to prevent that?
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2011-12-22 11:07:16 PST
I don't know.  Does it take effect dynamically?
Comment 12 Justin Lebar (not reading bugmail) 2011-12-22 11:20:57 PST
I added 

  user_pref("b2g.ignoreXFrameOptions", true);

to my user.js.  Now when I view the pref in about:memory, it has status "locked".  Checking what the value is returned by the pref branch...
Comment 13 Justin Lebar (not reading bugmail) 2011-12-22 11:26:20 PST
And with that pref set in user.js and the lockPref() call in place, the pref reads as false.  When I take out the lock, it reads as true.

Now, it appears that Google has other framebusting code in place, so the page still doesn't load properly with this pref set...
Comment 14 Justin Lebar (not reading bugmail) 2011-12-22 11:51:31 PST
Created attachment 583883 [details] [diff] [review]
Patch v2

Move x-frame-options check into CheckFrameOptions, which has the advantage of working.

(It worked sometimes as in patch v1, but it works consistently this way.)
Comment 15 Justin Lebar (not reading bugmail) 2011-12-22 11:57:30 PST
Comment on attachment 583883 [details] [diff] [review]
Patch v2

I checked that this works on desktop (if I comment out lockPref()) and that when the lock is there, my setting in user_pref is ignored.
Comment 16 Boris Zbarsky [:bz] (still a bit busy) 2011-12-22 13:19:55 PST
Comment on attachment 583883 [details] [diff] [review]
Patch v2

r=me
Comment 17 Phil Ringnalda (:philor) 2011-12-24 22:13:26 PST
https://hg.mozilla.org/mozilla-central/rev/00f4c8c5bcc7
Comment 18 Ben Francis [:benfrancis] 2011-12-25 09:06:26 PST
Cool, thanks :)

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