1450740 - Don't unshare network namespace when using X11 and /tmp/.X11-unix isn't available

Status: RESOLVED FIXED

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

Description

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

I'm breaking this out of bug 1449594 because a workaround was already landed under that bug.  The problem is partially described in [1], but I forgot something: that situation can also happen if the browser is run with access to the X server's network namespace but without access to /tmp/.X11-unix.  (I did mention this in one of the commits for bug 1440206, but apparently didn't realize all of the implications at the time.)

In particular, this is a problem for container-like environments like Snap that allow access to the network namespace but create a new mount namespace with a private /tmp (and don't bind-mount /tmp/.X11-unix, because normally the abstract namespace sockets suffice).

(Note that if the browser is chroot()ed — more precisely, if the process's root directory isn't its mount namespace's root — then we're forbidden from creating new user namespaces as a protection against chroot escapes and none of this matters, but that's usually not the case in modern Linux containers.)

This situation should be detectable by checking whether the /tmp/.X11-unix directory exists.  It would also be possible to use getpeername() and hard-code some knowledge about how Xorg arranges its sockets to check for the specific socket that would be used, but that's more code and more possibly-fragile assumptions so I'd prefer not to if it's not necessary.


[1] https://searchfox.org/mozilla-central/rev/b80994a43e5d92c2f79160ece176127eed85dcc9/security/sandbox/linux/launch/SandboxLaunch.cpp#89-95

Comment 1

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

Self-contained STR:

unshare -Umr   # this starts a subshell; use it for the following commands
mount -t tmpfs tmpfs /tmp
firefox --new-instance

Comment 2

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

Attachment: Bug 1450740 - Don't sandbox network namespace when X11 named sockets aren't accessible.

Review commit: https://reviewboard.mozilla.org/r/233156/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/233156/

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8964424 [details]
Bug 1450740 - Don't sandbox network namespace when X11 named sockets aren't accessible.

https://reviewboard.mozilla.org/r/233156/#review238918

::: security/sandbox/linux/launch/SandboxLaunch.cpp:106
(Diff revision 1)
> +    // remote.  See also bug 1450740.
> +    //
> +    // Unfortunately, the Xorg client libraries prefer the abstract
> +    // addresses, so this isn't directly detectable by inspecting the
> +    // parent process's socket.  However, in a container with a
> +    // private /tmp that isn't running its own X server

Part of the comment went missing.

Comment 4

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

Comment on attachment 8964424 [details]
Bug 1450740 - Don't sandbox network namespace when X11 named sockets aren't accessible.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/233156/diff/1-2/

Comment 5

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

(In reply to Gian-Carlo Pascutto [:gcp] from comment #3)
> Comment on attachment 8964424 [details]
> > +    // private /tmp that isn't running its own X server
> 
> Part of the comment went missing.

Thanks; I'm not sure how that happened.  Fixed.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ddd9187ed66d
Don't sandbox network namespace when X11 named sockets aren't accessible. r=gcp

Comment 7

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/ddd9187ed66d

Comment 8

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

Can you test the latest Nightly with the Snap packaging and without the patch from bug 1449594?

Comment 9

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

(ni? myself to request uplift once this has had a day or two on Nightly)

Comment 10

[Johan Lorenzo [:jlorenzo]]

Sorry, we don't build Snaps against Nightly yet. This is due to historical reasons where nightlies have been really different from betas and releases. The work will be done in bug 1451694, though.

Comment 11

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

Comment on attachment 8964424 [details]
Bug 1450740 - Don't sandbox network namespace when X11 named sockets aren't accessible.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1126437 (sandbox improvements)
[User impact if declined]: In some container environments, browser may not display content (tab crash on every page), and the Snap package has a significantly weakened sandbox as a workaround.
[Is this code covered by automated tests?]: Sandboxing in general has tests; the Snap package doesn't.
[Has the fix been verified in Nightly?]: Yes, using the STR from comment #1.
[Needs manual test from QE? If yes, steps to reproduce]: Optional; STR is in comment #1.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Low risk.
[Why is the change risky/not risky?]: This just turns off a small part of the sandbox in an unusual case.
[String changes made/needed]: None.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8964424 [details]
Bug 1450740 - Don't sandbox network namespace when X11 named sockets aren't accessible.

Fix a Linux sandboxing regression. Approved for 60.0b11.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/73fab5fbc843