Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use privilege separation #510

Open
KellerFuchs opened this issue May 12, 2016 · 19 comments
Open

Use privilege separation #510

KellerFuchs opened this issue May 12, 2016 · 19 comments
Labels
enhancement New feature request

Comments

@KellerFuchs
Copy link
Contributor

Would it be possible to split firejail in two processes?

  • firejail itself would still do all the profile parsing and so on, but isn't SUID anymore;
  • firejail-apply [0] receives a list of actions, in a very simple formal, executes them in order, drops privileges and execs the desired program.

This would likely help reduce the attack surface on the SUID binary.

[0] I'm sure there is a better name for it.

@netblue30
Copy link
Owner

netblue30 commented May 12, 2016

As soon as firejail starts, it drops the privileges only to rise and drop them as necessary. There are three processes running:

  • firejail front-end: it drops privileges in order to parse the command line arguments and the profile files, then it rises them to start the sandbox and configure networking, than it drops them again and starts monitoring the sandbox.
  • The sandbox process runs mostly as root. After it configures the filesystem, network and seccomp it drops privileges and starts the user application.
  • The application always starts and runs as user.

@netblue30 netblue30 added the information_old (Deprecated; use "doc-todo" or "needinfo" instead) Information was/is required label May 12, 2016
@KellerFuchs
Copy link
Contributor Author

KellerFuchs commented May 18, 2016

@netblue30 Sorry for the lack of response, I simply have lacked time so far to look at what firejail does as far as privsep is concerned.

@KellerFuchs
Copy link
Contributor Author

KellerFuchs commented May 18, 2016

@netblue30 As far as I see, the only privilege separation that is implemented is the use of your EUID_* functions. Since they do not change the saved set-user-id⁰, nothing prevents somebody gaining code execution in the firejail process (for instance by exploiting a buffer overflow in the parsing code) from running seteuid(0); system('/bin/sh') (or anything else, really).

If that's indeed the case, please re-open the issue.

[0]: Indeed, doing so would prevent firejail from performing EUID_ROOT() at any later point.

@netblue30 netblue30 reopened this May 18, 2016
@netblue30 netblue30 added enhancement New feature request and removed information_old (Deprecated; use "doc-todo" or "needinfo" instead) Information was/is required labels May 18, 2016
@netblue30
Copy link
Owner

You are right, I'll mark it as an enhancement.

@Sidnioulz
Copy link

@netblue30 you might find Wedge useful: http:https://static.usenix.org/event/nsdi08/tech/full_papers/bittau/bittau_html/ - http:https://www.scs.stanford.edu/~sorbo/

Since it's from academia the code might be unmaintained / the repo might be misconfigured, but I can ask one of the owners to make a public release if you need help reaching that. What I can tell is that these guys do not publish about software that does not work.

@netblue30
Copy link
Owner

Thank you for the information, I'll look into it.

@valoq
Copy link
Contributor

valoq commented Oct 10, 2016

Having firejail not drop privileges permanently is something that bothers me as well.

@netblue: I would like to work on this issue if you support this idea of permanent privilege separation and think its feasible.
Maybe you can point out how you would design the code to support this.

How about this:

  • have an unprivileged process parse all argument and profile files
  • a privileged suid sandbox process configures filesystem, network and seccomp, then drops privileges for good and starts the application

What are your thoughts on this?

@netblue30
Copy link
Owner

So far, parsing is done with very little privileges, but we go in the direction you described.

@KellerFuchs
Copy link
Contributor Author

KellerFuchs commented Oct 10, 2016

Well, as initially mentioned, everything (including parsing) is done with full privileges, since EUID_* are not a privilege-dropping mechanism.

Anyhow, I'm happy things are moving, and sad I couldn't make time to work on it myself.

@valoq
Copy link
Contributor

valoq commented Oct 11, 2016

As it is at the moment, parsing is done with full root privileges and an exploitable error in the parsing code will lead to full root access. The only way I know to change this is to have the parsing part of the code done by a process that never has root/suid privileges, or drops it for good before the parsing beginns.

As I understand the program so far, it should be feasible to have only a very small part of the code that is responsible for applying the sandbox mechanism, run as root/suid only to drop it immediately after for good.

@valoq
Copy link
Contributor

valoq commented Oct 16, 2016

There are two ideas for implementing proper privilege separation that I know of:

A.
Have an unprivileged process parse all configuration options, pass it to the privileged process that starts the sandbox, then drops privileges permanetly and starts the client within the sandbox.

This requires a lot of restructuring code as far as I can see.

B.
Exchange the current EUID_ROOT and EUID_USER functions as well as all code run as effective root at the moment for a passing mechanism like the one openssh uses.
http:https://www.citi.umich.edu/u/provos/ssh/privsep.html
This will delegate sections that need to be run as root to the privileged process, while most of the code will be run as an unprivileged user (possibly inside a chroot like opennssh does)

This is probably easier but I have no experience with this yet. And it requires a well defined interface between the two processes.

@KellerFuchs
Copy link
Contributor Author

@valoq A is basically one possible implementation of B, where the interface is one single call.

One issue is that the designer has to be very careful that the interface that is exposed cannot possibly be exploited for privilege escalation (for example tricking the privileged process into executing code as another user) or bypass system-wide restrictions defined in firejail.conf...

@valoq
Copy link
Contributor

valoq commented Oct 17, 2016

Having an interface for privilige separation will in any case improve security over what is the case right now (or did I missunderstand you?)

The problem I see is that one needs to have very good understanding of the code to create such an interface or separate those two prcesses completely like in case A.

I would like to know what netblue thinks about this.

@valoq
Copy link
Contributor

valoq commented Nov 19, 2016

Since this issue will be taken care of by netblue I would just like to write down an idea for future sandbox processing:

(note that this idea was not mine, I just wanted to write down what could be applied to firejail)

Since the future of sandboxing is with namespaces and user namespaces as substitute for suid bits, a design for firejail compatible with all linux distros could look like this:

Case 1: namespaces including unprivileged use of user namespaces is supported by the kernel

firejail can be started without suid bit as a normal user, creating a user namespace in which the process has root privileges and can therefore apply all other namepaces and features that require root access to complete the sandbox.

Case 2: namespaces are supported by the kernel, but user namespaces is not available to unprivileged users. This is the case with every major linux distro at the moment as the unprivileged use of user namespaces is considered a security risk until the feature has matured.

(the following strategy is used by the chrome sandbox)

have a suid helper binary (only when build on systems without unprivileged user namespaces available) that starts a user namespace and drops all privileges immediately afterwards. In the new user namespaces all other namespaces can be applied like in case 1. This required only a small change of the code to check for the availability of unprivileged user namespaces (if possible at the time of building the package) and create the suid helper binary if the check fails.

Case 3: namespaces are available but user namespaces are completely disabled. This is currently the case with the vanilla arch linux kernel (the linux-grsec kernel in arch has user namespaces for privileged users, see case 2)

use suid to apply all other namespaces just like it is that case with the current firejail. This will require to disable the part where the user namespaces are applied (I think this is already the case) and have the suid helper binary keep its privileged to apply the other namespaces. There needs to be a check for namespaces during runtime to allow custom kernels like linux-grsec to drop privileges of the suid helper binary and apply user namespaces like in case 2, while it is also possible to run the same build on a kernel without any support for user namespaces.

Case 4: no support for any namespaces: in this case firejail will not work. Kernels without namespaces support can also not run chrome, the new mozilla servo engine and some other common apps as well as pretty much any modern sandboxing application so this will not be a common occurrence and can be ignored

@genodeftest
Copy link
Contributor

genodeftest commented Jan 18, 2017

Also have a look at flatpak, which is using bubblewrap. I think firejail is 50% duplicate code with most of bubblewrap plus some rules. How about reusing bubblewrap and only maintaining firejail as rules-applying wrapper?

@KellerFuchs
Copy link
Contributor Author

@genodeftest As of currently, it's not possible, at least because bubblewrap inconditionally applies NO_NEW_PRIVS.
In general, I think it would be a good direction to move towards, but that's just my 2 cents.

@rusty-snake
Copy link
Collaborator

Any progress here?

@rusty-snake
Copy link
Collaborator

Related: #3412 (comment)

@crocket
Copy link
Contributor

crocket commented Sep 8, 2021

bubblewrap doesn't yet support executing programs in an existing network namespace. firejail does.

For firejail to wrap around bubblewrap, bubblewrap has to at least support executing programs in an existing network namespace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request
Projects
None yet
Development

No branches or pull requests

8 participants
@crocket @genodeftest @Sidnioulz @netblue30 @KellerFuchs @valoq @rusty-snake and others