Project

General

Profile

Bug #14623

Tor Browser sandbox breakout via X11 testing extensions

Added by yawning about 2 years ago. Updated over 1 year ago.

Status:
Confirmed
Priority:
Low
Assignee:
-
Category:
-
Target version:
-
Start date:
09/12/2017
Due date:
% Done:

10%

Feature Branch:
bugfix/14623-disable-X11-testing-extension
Type of work:
Code
Blueprint:
Starter:
Affected tool:
Browser

Description

The issue reported by Google Project Zero against the Tor Browser sandbox allows escape from the Tails browser's confinement via way of X11.

While this may be beyond Tails' threat model in this area (and to be frank, it was beyond mine, primarily as I view securing X a lost cause), this can be mitigated by disallowing access to the particularly unsafe X protocol extensions.

The easy way to accomplish this is to spawn the X server with `-tst` (See Xserver(1)), with the caveat that it will globally break applications that rely on that functionality (such as `xdotool`). The approach that I took, which is considerably more involved, is to intercept X protocol calls and whitelist the extensions that the browser is allowed to use, but my app operates under considerably different constraints.

See: https://bugs.chromium.org/p/project-zero/issues/detail?id=1293


Related issues

Related to Tails - Bug #14675: GNOME on-screen keyboard is broken without the X11 XTEST extensions Resolved 09/16/2017
Related to Tails - Feature #12213: Wayland in Tails 5.0 (Bullseye) In Progress 09/02/2017
Related to Tails - Bug #14712: Display backlight brightness regressions in 3.2~rc1 Resolved 09/24/2017
Blocks Tails - Feature #13234: Core work 2017Q3: Foundations Team Resolved 06/29/2017

Associated revisions

Revision 1f3367a4 (diff)
Added by intrigeri about 2 years ago

Disable X11 testing extension, aka. XTEST (refs: #14623).

This extension allow interaction with X11 server, e.g. sending keystrokes to
other windows. Let's plug that security issue, even though:

  • Disabling the XTEST extension does not prevent logging keystrokes sent to
    other windows, nor taking pictures of other pieces of the desktop.
  • With IBus, the accessibility bus, PulseAudio etc., our current sandboxing
    model is pretty limited. Wayland and Flatpak + Portals (or similar) will fix
    that some day.

Revision 706e8ef8 (diff)
Added by intrigeri about 2 years ago

Test suite: re-enable the X11 testing extension aka. XTEST (refs: #14623).

At least xdotool needs it.

Revision d89054fa
Added by anonym about 2 years ago

Merge remote-tracking branch 'origin/bugfix/14623-disable-X11-testing-extension' into devel

Fix-committed: #14623

History

#1 Updated by yawning about 2 years ago

My plan with this is to go for public disclosure in 30 days (2017/10/12), though this is probably fairly hard to exploit, and is also an obvious thing to check, so it being public immediately may be ok as well.

#2 Updated by intrigeri about 2 years ago

#3 Updated by intrigeri about 2 years ago

  • Subject changed from Disable X11 testing extensions. to Tor Browser sandbox breakout via X11 testing extensions
  • Status changed from New to Confirmed
  • Assignee set to intrigeri
  • Target version set to Tails_3.2

Thanks a lot for this report! I'll take a first look to evaluate how important we think it is in our threat model.

#4 Updated by intrigeri about 2 years ago

yawning wrote:

My plan with this is to go for public disclosure in 30 days (2017/10/12), though this is probably fairly hard to exploit, and is also an obvious thing to check, so it being public immediately may be ok as well.

X is explicitly out of scope for the limited "sandboxing" we currently apply to our apps, including Tor Browser, so IMO it's fine to disclose this publicly now: it won't teach any remotely competent adversary anything, and making it public might actually allow people to help us mitigate the problem.

yawning, may I make this ticket public? (Feel free to do it yourself if you agree.)

#5 Updated by intrigeri about 2 years ago

  • Feature Branch set to wip/bugfix/14623-disable-X11-testing-extension

X is explicitly out of scope

… and IBus + the accessibility bus + PulseAudio probably open even larger potential for escaping our current "sandbox".

Still, if we could disable the X11 testing extensions without too much effort (until #12213 happens), I think we should do it to raise the bar a bit.

We're currently using xdotool in our test suite only, so one way to handle this would be to disable the X11 testing extensions by default (hoping that doesn't break anything else we rely upon), and re-enable them during boot in our test suite: generally we dislike testing something different than what actual users face, but well, that might be one of these cases where an exception makes sense. Alternatively we could stop using xdotool in our test suite, but at first glance it seems to involve more work.

I've pushed a branch that disables the XTEST extension. I doubt we'll have time to adjust our test suite in time for 3.2, we'll see.

#6 Updated by intrigeri about 2 years ago

  • Status changed from Confirmed to In Progress

#7 Updated by yawning about 2 years ago

  • Private changed from Yes to No

intrigeri wrote:

yawning, may I make this ticket public? (Feel free to do it yourself if you agree.)

Done.

I agree that there isn't a great solution for other things currently, without using some sort of isolating X setup (or writing a hilarious amount of code to try to filter X protocol traffic). At least the situation can only get better over the next few years.

#8 Updated by intrigeri about 2 years ago

  • % Done changed from 0 to 20
  • Feature Branch changed from wip/bugfix/14623-disable-X11-testing-extension to bugfix/14623-disable-X11-testing-extension

Let's see what our test suite thinks. Chances are this makes it into 3.2 in the end :)

#9 Updated by intrigeri about 2 years ago

intrigeri wrote:

Let's see what our test suite thinks. Chances are this makes it into 3.2 in the end :)

Non-fragile subset of the test suite passed on my Jenkins, and my full test suite run should be over in 1-2 hours, shortly after the 3.2 freeze deadline but I'm still waiting for a few other branches from anonym anyway so it probably doesn't matter.

#10 Updated by intrigeri about 2 years ago

  • Assignee changed from intrigeri to anonym
  • % Done changed from 20 to 50
  • QA Check set to Ready for QA

Full test suite passed on my Jenkins (except #14586 as expected).

#11 Updated by anonym about 2 years ago

  • % Done changed from 50 to 100
  • QA Check changed from Ready for QA to Pass

Looks good:

$ xdotool key a
Warning: XTEST extension unavailable on '(null)'. Some functionality may be disabled; See 'man xdotool' for more info.
Xlib:  extension "XTEST" missing on display ":1".
Xlib:  extension "XTEST" missing on display ":1".

(Not merging yet due to bulk reviewing.)

#12 Updated by anonym about 2 years ago

  • Status changed from In Progress to Fix committed
  • Assignee deleted (anonym)

#13 Updated by intrigeri about 2 years ago

  • Related to Bug #14675: GNOME on-screen keyboard is broken without the X11 XTEST extensions added

#14 Updated by intrigeri about 2 years ago

#15 Updated by intrigeri almost 2 years ago

  • Related to Bug #14712: Display backlight brightness regressions in 3.2~rc1 added

#16 Updated by anonym almost 2 years ago

  • Status changed from Fix committed to Resolved

#17 Updated by intrigeri almost 2 years ago

  • Status changed from Resolved to Confirmed
  • Target version deleted (Tails_3.2)
  • % Done changed from 100 to 10
  • QA Check deleted (Pass)

The fix broke too much stuff so it got reverted in Tails 3.2 final.

#18 Updated by cypherpunks almost 2 years ago

this can be mitigated by disallowing access to the particularly unsafe X protocol extensions.

This is incorrect. It is still quite easy to screw with things if you have access to X11. While reducing attack surface by removing extensions is always a good idea, it is little more than a prayer to attempt to mitigate this by removing extensions.

Disabling XTEST only disables a single feature that is used for debugging. It does not actually prevent reading keystrokes, etc. Even using, say XGrabKeyboard() to attempt to block this is useless, as XQueryKeymap() can pretty trivially bypass that (see http://seclists.org/bugtraq/2005/Jun/3, yes from 2005). This also bypasses the disabling of XTEST. I'm sure you all know the debacle with the XSECURITY extension? It basically showed that it is impossible to secure the X11 protocol in the way we want through extensions, much less through simply removing extensions.

Please do not fall into snakeoil security features. The only solution, short of completely re-writing the X11 protocol yourself or using a nested X server, is to use Wayland (i.e. using a complete re-write of the X11 protocol by someone else). IIRC, there is already a ticket for moving to Wayland, so the most this could do is reduce attack surface area slightly.

This ticket is promoting snakeoil as it stands.

#19 Updated by u over 1 year ago

@FT do you plan to rework on the proposed branch?

#20 Updated by intrigeri over 1 year ago

@FT do you plan to rework on the proposed branch?

No we'll focus on Wayland.

#21 Updated by u over 1 year ago

  • Priority changed from Normal to Low

Lowering priority then.

Also available in: Atom PDF