Firejail: Program in X11 sandbox can kill host X session

[iskunk]

In testing Firejail with Openbox, I found a rather dismaying issue. (I have verified that this occurs with Git d0a8395d, the current revision as of this writing.)

Start up Openbox in an X11 sandbox like so:

firejail --x11=xephyr --net=eth0 openbox

Right-click to bring up the WM menu, select "Exit", and confirm.

Not only does the Openbox-in-Xephyr session go away, the entire host X session [in which you started up Firejail in the first place] goes away. (I obtained this result with the Xfce desktop running on Ubuntu 18.04/bionic.)

And if a sandboxed Openbox can kill your login session, then any sandboxed (X11) program can kill your login session.

For what it's worth, this does not occur if you use whitelist or private.

[reinerh]

I can't reproduce it. I started firejail --x11=xephyr openbox inside awesome-wm, and exited openbox via its right-click menu, but my outer X session stayed active.

[iskunk]

It probably has to do with how the host WM/desktop manages its session, which is why I mentioned that this occurred with Xfce. Openbox is probably sending some sort of semi-standard "end session" message, which Xfce interpreted, but perhaps awesome-wm does not.

[rusty-snake]

@iskunk maybe firejail --nodbus --x11=xephyr openbox works.

[chiraag-nataraj]

@iskunk Is this still an issue?

[chiraag-nataraj]

I'll go ahead and close this for now. @iskunk, please feel free to re-open if you still have this issue or have further questions.

[iskunk]

Hi @chiraag-nataraj, apologies for the delay; I had to rebuild a test environment for this bug.

I tried this again, using firejail 0.9.60~rc2 (Git 43cbc6d5), and have verified that the issue still exists. The host X session remains vulnerable to a sandboxed X11 program.

I also tested with --nodbus, as @rusty-snake suggested. Unfortunately, this option did not improve matters.

(I cannot re-open this bug.)

[chiraag-nataraj]

Reopening!

[rusty-snake]

@iskunk can you try firejail --net=none --nonewprivs --nodbus --private --ipc-namespace --nogroups --shell=none --private-tmp --private-bin=openbox --private-etc=none --x11=xephyr openbox. If one (or more) of this args break openbox to start, try without this one.

[iskunk]

@rusty-snake, I had to drop --private-bin and --private-etc for Openbox to start, but with the remaining options, the session was not killed.

I then proceeded to remove options (except for --x11, of course) until I observed the host X session killed as originally reported.

The --private option appears to be significant. That one prevents the problem behavior. (Specifically, firejail --private --x11=xephyr openbox is good.)

That is not the only one, however. After further testing, I confirmed that firejail --net=none --private-tmp --x11=xephyr openbox does not exhibit the problem either. But if I remove either the --net or the --private-tmp, the problem returns.

Note that my test environment is an Xfce (Xubuntu) host X session with a default Openbox session in the sandbox, running on a VM. This issue should not be difficult to reproduce.

[rusty-snake]

@iskunk sorry for later responding, but now I have found a solution:
firejail --blacklist=${HOME}/.ICEauthority --x11=xephyr openbox

[iskunk]

@rusty-snake: Confirmed that blacklisting ~/.ICEauthority prevents the issue.

The various --x11 modes should probably include that one in their blacklisting, yes? The issue of a sandboxed environment killing the host X session is surely unexpected.

[rusty-snake]

Because it must be added to the openbox profile, but only when --x11 is present, I see three options:

1. add it with a comment to openbox and other affected WMs
2. add it to the documentation ([1], [2])
3. --x11 implieds blacklist ${HOME}/.ICEauthority

[iskunk]

I would think #3 is the best option, for the following reasons:

• This is not just a matter of (known) WMs possibly ending the session; badly-written programs, or even a hostile program may do this
• The failure mode (loss of an X session, with associated potential for data loss) makes it a bit more serious of a threat than can reasonably be addressed in documentation
• --x11 already blacklists relevant files like ~/.Xauthority; this would only be adding to that, rather than being an entirely new behavior

[rusty-snake]

_With #3 you link to an issue :grin:_

I also think that the 3. option the best solution is, if you hate the necessary C knowledge, you are welcome to open a PR, I can' t (yet) write C :cry: .

[iskunk]

Oh, nuts, didn't notice that :-]

I looked in the code. I can find where to add blacklisting for --x11=none, but the other cases are a bit less straightforward. There's a lot of special handling for ~/.Xauthority, for example.

I did figure that the /tmp/.ICE-unix/ socket directory should probably also be blocked by default in a sandboxed session. (I can't find a ready-made frontend program that demonstrates this, but it is theoretically possible that a misbehaving non-X program could make use of libICE to send an unwanted "end session" message.)

[rusty-snake]

private-tmp prenvents access to /tmp/.ICE-unix, but if a profile not has it …

[iskunk]

Yeah, that explains why --private-tmp made the issue go away (as well as --private with ~/.ICEauthority).

This does seem somewhat urgent as the authentication needed to mess with the X session is separate from XAUTHORITY; i.e. blocking that one does not protect the session.

Should we pull in netblue30? I don't think a fix for this should be complicated; it's mainly knowing where the blocking bits need to go...

[chiraag-nataraj]

@netblue30 and others, we should put this fix in...

[smitsohu]

I did figure that the /tmp/.ICE-unix/ socket directory should probably also be blocked by default in a sandboxed session.

I tend to agree. I would suggest to mount a tmpfs on this directory, so that just in case a program wants to talk to other programs in the same sandbox using whatever ICE based protocol, it is still able to do so.

But there is one problem, maybe two. First, in the case of session managers the sockets often seem to be no real files but in the abstract namespace (same as the sockets in /tmp/.X11-unix). This means a tmpfs (or a blacklist) is not effective in all cases, we need one of the net options to always and reliably shut down the access. So one "solution" might be to indeed align this feature with the x11 options (thematically).

Second, I'm a little bit worried if programs both inside and outside sandboxes all write to the same cookie database (aka ~/.ICEauthority). The sockets in /tmp/.ICE-unix are named after the pid of the process that created them, so if there are two or many pid namespaces, there could be a risk of file name collisions. Maybe Firejail should take care each sandbox has their own .ICEauthority file by setting an ICEAUTHORITY environment variable accordingly?

Is this overkill? Other ideas?

[smitsohu]

The sockets in /tmp/.ICE-unix are named after the pid of the process that created them, so if there are two or many pid namespaces, there could be a risk of file name collisions.

On the other hand, people are provoking similar situations with private-tmp already now, and there seem to be no reported problems.

[iskunk]

My experience to date with private-tmp is that programs sometimes complain with a message like "cannot connect to session manager." This would likewise happen if ICE is blocked.

It's not a showstopper for most X11 applications, and unless people are running e.g. full GNOME desktops within Firejail and complaining that they can't save their sessions properly, I don't think there's a pressing need to provide a separate instance of ICE et al. within an X11 sandbox.

[smitsohu]

Or maybe this is too complicated thinking. Should we just add a blacklist ${HOME}/.ICEauthority to disable-common.inc?

[smitsohu]

Alright, proposing a new ?HAS_X11 conditional, and putting the blacklist in disable-common.inc. Will put a fix in the next few days.

[smitsohu]

Oops. x11 xorg is not covered by the conditional, I'll have to add that option, too.

[rusty-snake]

for more flexibility in the future something like that would be helpful

?HAS(<LINE>): <LINE>
?HAS(seccomp): blacklist /...
?HAS(net none): x11 none

[smitsohu]

?HAS(net none):

How about something like ?HAS_NET ?

It tends to have influence on various other options...

That doesn't exclude e.g. ?HAS_NETNONE of course.

[rusty-snake]

Sure, HAS_NET or similar isn't bad, but my idea was more about conditions by line match (like ignore).

[smitsohu]

I'll add HAS_NET for the moment as it is useful and a particularly low hanging fruit :)

[matu3ba]

@smitsohu What is the status on this? Closing?