Feature #11547

Tor Browser's AppArmor policy should not allow access to /dev/dri

Added by cypherpunks 2016-06-25 05:47:52 . Updated 2017-06-12 16:11:52 .

Status:
Resolved
Priority:
Low
Assignee:
Category:
Target version:
Tails_3.0
Start date:
2016-06-23
Due date:
% Done:

100%

Feature Branch:
Type of work:
Code
Blueprint:

Starter:
0
Affected tool:
Browser
Deliverable for:

Description

Currently, the torbrowser policy includes <abstractions/gnome>, which includes <abstractions/X>, which whitelists /dev/dri. Because the amnesia user is in the video group, all the dangerous ioctls are permitted. That on its own is bad enough, but there really shouldn’t be a need for Tor browser to access DRM nodes, unless you’re using hardware acceleration or something like WebGL or Flash. They are a gigantic source of vulnerabilities in the kernel when they can be accessed without special permissions. The X server already provides standard hardware acceleration (it’s not like disabling the DRM nodes is gonna give the browser VESA performance), so there should range from no perf hit at all, to no noticable perf hit.

Files

disallow-dri.patch (405 B) cypherpunks, 2016-07-18 20:53:34

Subtasks

Related issues

Blocked by Tails - Bug #12609: Bump APT snapshots serial for 3.0: 2017-06-09 Resolved 2017-05-27

History

#1 Updated by cypherpunks 2016-07-09 20:32:13

Actually it looks like Tor Browser has hardware acceleration disabled anyway, so the perf hit would be nil.

#2 Updated by intrigeri 2016-07-13 08:18:37

  • Tracker changed from Bug to Feature
  • Subject changed from Tor browser's AppArmor policy should not allow access to /dev/dri to Tor Browser's AppArmor policy should not allow access to /dev/dri
  • Status changed from New to Confirmed
  • Priority changed from Normal to Low
  • Starter changed from No to Yes
  • Affected tool set to Browser

It looks like a good idea at first glance! Patches welcome, the profile we ship lives in that Git repository: https://git-tails.immerda.ch/torbrowser-launcher/

#3 Updated by cypherpunks 2016-07-14 22:53:11

Unfortunately there might be some sort of bug in AppArmor in the way it interfaces with aufs. Even when, in the policy, I use the proper syntax to deny access to all actions against /dev/dri and /dev/dri/**, accesses are still permitted. When I asked in OFTC’s #apparmor, they did not have an answer for why that could be happening.

I’ll keep looking for a solution, since the official technique has no effect.

#4 Updated by intrigeri 2016-07-16 07:35:37

> Even when, […], accesses are still permitted.

Just curious: how do you check that?

#5 Updated by cypherpunks 2016-07-18 19:00:15

intrigeri wrote:
> > Even when, […], accesses are still permitted.
>
> Just curious: how do you check that?

I created a file, /dev/dri/hello.txt, and pointed Tor Browser at file:///dev/dri/hello.txt. The access should have been blocked, but instead I was presented with “Hello, world” in text/plain. I also tried accessing file:///dev/dri/card0, which, when blocked by AppArmor, will result in Tor Browser giving an error and the syslog recording the attempt. When it is not blocked, it simply hangs, trying to load the file (which obviously it cannot load, being that it is a character device). I used “strace -e open,openat -p ${tor_browser_pid}” as well.

#6 Updated by cypherpunks 2016-07-18 20:54:10

I think I’m just an idiot. Turns out I misunderstood http://wiki.apparmor.net/index.php/AppArmor_Core_Policy_Reference#deny and forgot to restart Tor Browser after reloading the policy. It works now.

Attached is a patch to /etc/apparmor.d/torbrowser which disallows DRM node access.

#7 Updated by intrigeri 2016-07-19 02:28:46

  • Starter changed from Yes to No

#8 Updated by intrigeri 2016-07-19 02:29:08

  • Assignee set to intrigeri
  • Target version set to Tails_2.6
  • QA Check set to Ready for QA

Thanks! Best would be to submit this as a pull request to our Tor Browser AppArmor profiles’ upstream. Do you want to do that? Worst case I’ll do it myself, but it’ll take more time.

#9 Updated by cypherpunks 2016-07-19 05:45:29

intrigeri wrote:
> Thanks! Best would be to submit this as a pull request to our Tor Browser AppArmor profiles’ upstream. Do you want to do that? Worst case I’ll do it myself, but it’ll take more time.

If you could do it whenever you have time that would be great.

#10 Updated by intrigeri 2016-07-21 06:40:27

  • Status changed from Confirmed to In Progress
  • % Done changed from 0 to 10

#11 Updated by intrigeri 2016-07-25 03:46:31

  • Target version deleted (Tails_2.6)
  • % Done changed from 10 to 20
  • QA Check deleted (Ready for QA)
  • Type of work changed from Code to Wait

Upstream pull request: https://github.com/micahflee/torbrowser-launcher/pull/242. We’ll get this change once it’s merged + there’s a new upstream release + it is uploaded to Debian + we merge it into our torbrowser AppArmor profile. I’ll track this, so keeping assigned to me. I don’t think it’s worth doing additional work to get this earlier in Tails, since such work will need to be reverted later on.

#12 Updated by cypherpunks 2017-03-11 08:19:17

It’s in 0.2.7-1 in the experimental branch, upstream, at least: https://sources.debian.net/src/torbrowser-launcher/

#13 Updated by intrigeri 2017-06-05 17:25:18

  • Target version set to Tails_3.0

According to myself above this should be fixed in Tails 3.0. I’ll double check this and update this ticket accordingly.

#14 Updated by intrigeri 2017-06-07 13:12:24

  • % Done changed from 20 to 90
  • Type of work changed from Wait to Code

That’s fixed in the devel branch, that uses an AppArmor profile based on the one from torbrowser-launcher 0.2.7-2.

#15 Updated by intrigeri 2017-06-07 13:13:42

  • blocked by Bug #12609: Bump APT snapshots serial for 3.0: 2017-06-09 added

#16 Updated by intrigeri 2017-06-09 20:17:07

  • Status changed from In Progress to Fix committed
  • Assignee deleted (intrigeri)
  • % Done changed from 90 to 100

#17 Updated by intrigeri 2017-06-12 16:11:52

  • Status changed from Fix committed to Resolved