Bug #14623

Tor Browser sandbox breakout via X11 testing extensions

Added by yawning 2017-09-12 07:53:37 . Updated 2018-01-16 14:48:40 .

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

10%

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

Starter:
Affected tool:
Browser
Deliverable for:

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

Subtasks

Related issues

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

History

#1 Updated by yawning 2017-09-13 03:39:18

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 2017-09-13 16:24:08

#3 Updated by intrigeri 2017-09-13 16:25:38

  • 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 2017-09-13 16:40:15

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 2017-09-13 20:38:21

  • 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 Feature #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 2017-09-13 23:31:46

  • Status changed from Confirmed to In Progress

Applied in changeset commit:1f3367a450cf4b1ef8600b21d24d485810c25d7e.

#7 Updated by yawning 2017-09-14 01:32:31

  • 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 2017-09-14 07:01:40

  • % 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 2017-09-14 09:50:47

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 2017-09-14 11:18:04

  • 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 Bug #14586 as expected).

#11 Updated by anonym 2017-09-15 16:11:45

  • % 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 2017-09-15 17:24:11

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

#13 Updated by intrigeri 2017-09-18 08:15:23

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

#14 Updated by intrigeri 2017-09-18 08:15:43

#15 Updated by intrigeri 2017-09-24 08:22:12

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

#16 Updated by anonym 2017-09-28 18:50:12

  • Status changed from Fix committed to Resolved

#17 Updated by intrigeri 2017-10-22 12:07:11

  • 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 2017-10-26 07:30:14

>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 Anonymous 2018-01-15 16:00:43

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

#20 Updated by intrigeri 2018-01-15 16:06:52

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

No we’ll focus on Wayland.

#21 Updated by Anonymous 2018-01-16 14:48:40

  • Priority changed from Normal to Low

Lowering priority then.