Page MenuHomePhabricator

Enable wgBreakFrames across all projects
Open, MediumPublic

Description

This task is basically a public version of T112428 to solicit comments/tokens as to whether or not this is a good idea. I (and I think most members of the Security-Team) would like to see this enabled so as to marginally increase our defenses against clickjacking and limit the number of reports from "security researchers" we receive, which do nothing but waste our time. If there are compelling arguments AGAINST enabling this config variable, then I'm happy to decline the task and abandon the config patch.

Event Timeline

sbassett triaged this task as Medium priority.Jun 19 2020, 4:19 PM
sbassett moved this task from Backlog to In Progress on the user-sbassett board.

Change 606723 had a related patch set uploaded (by SBassett; owner: SBassett):
[operations/mediawiki-config@master] Enable wgBreakFrames across all projects

https://gerrit.wikimedia.org/r/606723

For reference, I believe CVE-2014-5243/T67778 was the last clickjacking issue in MediaWiki.

On T255881 there were some cases of legitimate framing for e.g. side-by-side comparisons. In general, we want to allow for wide reuse of our content, and framing is a really simple way of doing that. I like the suggestion of still allowing action=render to be framed, which I think should address all the legitimate cases, while closing the attack surface nearly everywhere else...is that reasonable? Or does leaving action=render open still cause problems?

On T255881 there were some cases of legitimate framing for e.g. side-by-side comparisons. In general, we want to allow for wide reuse of our content, and framing is a really simple way of doing that. I like the suggestion of still allowing action=render to be framed, which I think should address all the legitimate cases, while closing the attack surface nearly everywhere else...is that reasonable? Or does leaving action=render open still cause problems?

This sounds reasonable to me, certainly for now. Unfortunately $wgBreakFrames is a bit of a hammer, as-is. If OP->allowClickjacking() could override it, I think the solution would be simple for action=render. Otherwise I'm not quite sure what the cleanest approach might be. A new global? Seems a bit much. Creating some action exception logic for $wgBreakFrames? Defeats the purpose a bit IMO and I'm not sure just how ugly something like $wgBreakFramesAllowedActions would be.

Eh, I guess we'd have to explore something else anyways as there are probably too many important uses of allowClickjacking() right now, which $wgBreakFrames = true; would seemingly break.

This sounds reasonable to me, certainly for now. Unfortunately $wgBreakFrames is a bit of a hammer, as-is. If OP->allowClickjacking() could override it, I think the solution would be simple for action=render. Otherwise I'm not quite sure what the cleanest approach might be. A new global? Seems a bit much. Creating some action exception logic for $wgBreakFrames? Defeats the purpose a bit IMO and I'm not sure just how ugly something like $wgBreakFramesAllowedActions would be.

We currently have:

  • $wgBreakFrames
  • $wgEditPageFrameOptions
  • $wgApiFrameOptions

We could combine them all into:

$wgBreakFrames = [ 
'view' => 'DENY', // or maybe 'all' ??
'editpage' => 'DENY', 
'api' => 'DENY',

Then adding $wgBreakFrames['render'] = false; would be straightforward. It would be straightforward to have a layer that continues to support the old bool value as well for back-compat.

Eh, I guess we'd have to explore something else anyways as there are probably too many important uses of allowClickjacking() right now, which $wgBreakFrames = true; would seemingly break.

Based on that codesearch, it looks like there are 2 meanings for allowClickjacking(), one is "this page is safe to frame" and the other is "this page needs framing to work". So what if we have allowClickjacking() default to the "this page is safe to frame" case, in which a restrictive $wgBreakFrames policy would overrule it, and introduce a new requireClickjacking() with some security warning that would overrule $wgBreakFrames. Then we'd be free to tighten $wgBreakFrames as much as we want without breaking legitimate uses.

Change 606723 abandoned by SBassett:
[operations/mediawiki-config@master] Enable wgBreakFrames across all projects

Reason:
In favor of implementing T255881#6255282

https://gerrit.wikimedia.org/r/606723

Framing was disabled between March 2004 187d612dd48b4b83 and December 2006 439f59fb7b1d1d0. I recall receiving a complaint from a public library which was showing websites with library catalogue navigation wrapped around, probably T7306, 2006. That complaint influenced my decision on frame policy in 2006 and 2011. I suspect that there are a lot of minor community uses for frames. We could discover them by logging an event when a frame is detected (window.top!==window.self).

From https://www.hertzbleed.com/gpu.zip/ (via @matmarex in T131183#9202983):

Most sensitive websites already deny being embedded by cross-origin websites. As a result, they are not vulnerable to the pixel stealing attack we mounted using GPU.zip. However, some websites remain vulnerable. For example, if a user who is logged into Wikipedia visits a malicious webpage, that webpage can exploit GPU.zip to learn the user’s Wikipedia username (as we demonstrate in Section 5.4 of the paper).

5.4. Wikipedia username stealing example

As a proof-of-concept for a realistic attack, we demonstrate stealing a username. In this scenario the target iframe is Wikipedia, which shows the user’s username in the top corner. We run this PoC with multiple browser windows open, with one playing a YouTube video during the attack. Figure 17 shows the results of our attack on an Intel i7-8700 and an AMD Ryzen 7 4800U. We calculate the accuracy based on the ground truth after color binarization. Our attack is unoptimized, but completes in 30 minutes on the Ryzen with 97.0% accuracy. The Intel attack is significantly slower, at 215 minutes with 98.3% accuracy. Unlike previous works that are sensitive to noise in DVFS oscillation [24], [25], our PoC succeeds even in the presence of system noise, showcasing robustness.

Granted, "realistic attack" seems like a bit of a stretch:

However, from JavaScript we can only access coarse-grained timers. For the feasibility experiments in Section 5.2, we build Chrome with debug symbols (v8_symbol_level = 2), and use uprobe to expose the CPU time-stamp counter to Chrome. The time-stamp counter allows us to synchronize the rendering process and the sampling process and pinpoint the block of sampled data that belongs to the same frame.

Still I think the safe thing to do would be to disallow iframes for all logged-in pageviews (and handle the special case of CentralAuth updating the personal toolbar on a technically-logged-out pageview after logging in the user in the background).