1302711 - Investigate possibility to restrict sys_ioctl arguments

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect, P3)

Description

[Julian Hector [:tedd] [:jhector]]

This bug is for investigating if it is feasible to restrict ioctl() arguments using seccomp-bpf on Linux desktop.

icotl() is a very powerful system call and restricting its arguments would reduce the attack surface this system call exposes.

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

In particular, tty ioctls could be a problem — e.g., TIOCSTI (“Simulate Typed Input”) can inject arbitrary text into the terminal's input buffer, including shell commands.  This is a known problem for other sandboxes: http://www.openwall.com/lists/oss-security/2016/10/25/9

Unfortunately, we can't get rid of ioctl completely as long as we have GPU drivers in the content process.  And I don't even know how feasible it would be to change it to a default-deny policy (although if the kernel interface situation is that everything is either DRI or Nvidia, then maybe).

But we should at least be able block the entire 'T' class of ioctls (i.e., xxxx54xx in hex).

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

(In reply to Jed Davis [:jld] {⏰UTC-7} from comment #1)
> Unfortunately, we can't get rid of ioctl completely as long as we have GPU
> drivers in the content process.  And I don't even know how feasible it would
> be to change it to a default-deny policy (although if the kernel interface
> situation is that everything is either DRI or Nvidia, then maybe).

However, if there are other GPU interfaces besides those two, we'll probably have to explicitly allow them in the file broker policy (where “probably” becomes “definitely” once we stop having a default-allow policy for reads) before ioctls can be a concern.

For reference, DRI uses 'd' (0x64) and Nvidia seems to use 'F' (0x46).

Comment 3

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

nvidia is more complicated than that; on GL startup I'm seeing it open /dev/nvidia-modesetting and do an ioctl with type 0x6d ('m').

I also took a look at nvidia's OpenCL driver (because we're seeing media codec libraries try to use it, and it might share infrastructure with advanced GL features).  Just running the "clinfo" utility opens /dev/nvidia-uvm and uses ioctls of types 0x00 and 0x07.

So it might be best to just block the 'T' ioctls for now, because that's a few lines of code and fairly safe and will stop some known issues, and file a followup bug to lock this down more.

Comment 4

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

From the upstream issue linked in bug 1384292, it looks like they're going to keep doing the ioctls (i.e., fix the “broken” but not the “useless”).  I could just fail the SIOCGIF* family with EPERM instead and that should unblock the rest of this.

Comment 5

[Tom S. (please needinfo tschuster)]

I think with the removal of X11 and WebGL from the content process we might be able to significantly cut down the allowed IOCTL syscalls.

Comment 6

[Firefox User]

Any future plans or timeline for implementing this work?

Comment 7

[Tom Schuster (MoCo)]

Jed, is this blocked by GPU sandboxing on Linux?

Comment 8

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

I've been looking into this. Simply turning off the “allow if not type 'T'” rule seems to work, for the content and socket processes, on my system and on Try. But, I'm concerned about regressions from shipping this to the broader userbase, and I'd like to make this a new sandbox level (controlled by the security.sandbox.*.level prefs). Specifically, for content it should probably be level 6, with security.sandbox.content.headless retroactively turned into level 5, because ruling out the use of graphics drivers is a prerequisite for this.

And that's a little awkward, because security.sandbox.content.headless both modifies the sandbox policy and sets content processes' graphics to headless mode, so that may need to be disentangled.

Comment 9

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

Attachment: Bug 1302711 - Add Linux content sandbox level 6, for `ioctl` lockdown. r?gcp!

Comment 10

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

Attachment: Bug 1302711 - Add Linux socket process sandbox level 2, for `ioctl` lockdown. r?gcp!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/816a06dd64b6
Add Linux content sandbox level 6, for `ioctl` lockdown. r=gcp
https://hg.mozilla.org/integration/autoland/rev/daaebc45cb3c
Add Linux socket process sandbox level 2, for `ioctl` lockdown. r=gcp

Comment 12

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/816a06dd64b6
https://hg.mozilla.org/mozilla-central/rev/daaebc45cb3c

Comment 13

[Tom Schuster (MoCo)]

🎉 I think we might even want to add a release note for this?

Comment 14

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

Release Note Request (optional, but appreciated)
[Why is this notable]: Security improvement on Linux; Chrome-parity issue that's been called out on social media.
[Affects Firefox for Android]: no
[Suggested wording]: A new sandbox level on Linux, adding defense in depth to existing protections against access to device drivers (ioctl restrictions).
[Links (documentation, blog post, etc)]: none

Note: The parenthetical may not be strictly necessary, but I thought there might be value in having the keyword ioctl (the system call in question) in there for searchability.