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

WIP: feature: Firejail as a service #3315

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

topimiettinen
Copy link
Collaborator

Run Firejail as socket activated service, so it doesn't need to be
setuid.

Run Firejail as socket activated service, so it doesn't need to be
setuid.
@topimiettinen
Copy link
Collaborator Author

Something happens but it does not work yet.

@FOSSONLY
Copy link

FOSSONLY commented Apr 4, 2020

Until now I thought that such a thing would require a change to the linux kernel itself. So e.g. some kind of extended interface for execve() to allow programs like firejail to intercept everything that is executed. I'm not sure how firejail should work as a service, I don't know enough about the proposed approach. But practically everything is very attractive if it allows you to avoid suid privileges while preserving the original functionality.

@topimiettinen
Copy link
Collaborator Author

In the concept there's a server process running with root privileges, but due to systemd socket activation, the server only starts when a client connects. The client is very simple, basically just send_args(): send arguments and credentials with SCM_CREDENTIALS. The server reads the client's privileges, then reads new argc and argv from the socket and launches Firejail with the new arguments. Both the client and the server could be small separate programs.

Launching apps this way means that they are actually started by systemd (PID 1) instead of being a child of window manager or shell in terminal. The new process will also not be part of the login session and it will not be a child of systemd --user either. Some limitations (for example cgroup limits like memory or number of processes, IO etc.) which come from being part of those sessions will not be enforced. So security-wise there are also disadvantages. Good thing is that Firejail already has some access controls so that the users which are allowed to run Firejail can be restricted.

Another advantage with this is that all sandboxing features of systemd (for example various seccomp filters) could be also used. Though it would be simple to make the server work without using socket activation, so it would work without systemd.

Missing features include: at least environment variables need to be transferred, maybe also stdin/stdout/stderr. SELinux context can be sent with SO_PEERSEC.

@kris7t
Copy link
Collaborator

kris7t commented Apr 5, 2020

This looks very interesting, especially using firejail in conjunction with systemd features.

However, I must wonder what is the security model here? At first look, a daemon that spawns a process with root privileges that performs complex operations according to user input (e.g., parse profiles, set up a sandbox according to passed arguments, manipulate mounts) is equivalent to (if not worse than, due to not being part of a user session) a setuid binary. I am not aware of any vulnerabilities that come from the mere presence of a setuid flag, instead of performing complex privileged operations according to non-privileged user input. Although it is true that environment sanitization in this daemon model is automatically better than with a setuid binary (environment variables must be explicitly sent over).

I found this discussion on LWN on setuid vs daemons. The points (enforcing a clean environment, enforcing a global rate limit) are definitely interesting, although they may have limited applicability fo firejail. As you noted, the environment should generally be transferred to the sandboxed process. A global quota on all firejail processes might be interesting, but somewhat of a niche use-case.

@topimiettinen
Copy link
Collaborator Author

Firejail already performs the input processing with EUID=UID, so this part should be equivalent. I tried to match the UID/EUID/SUID logic of setuid binaries. The server helper could also sanitize the received arguments, though this could be also done as preparatory part of Firejail's normal argument processing.

Ideally the environment variables should not be used by Firejail or its helper programs (fseccomp etc) at all, but they are just injected at last moment for the app. This way Firejail would not have to care about for example LD_ variables (though they probably should be rejected as well). Again, also setuid Firejail could save the environment variables, nuke the environment and apply the saved environment only at final execve().

Even better would be true privilege separation, also fully applicable and reusable for setuid Firejail model as well. Though a lot of processing is already done with unprivileged and sandboxed helpers.

Handling of supplementary groups is also missing and GID/EGID/SGID handling is buggy (Firejail is not set-gid).

@topimiettinen
Copy link
Collaborator Author

Opened #3319 to do basic checks for arguments and environment.

There's already some functionality to store environment variables elsewhere (see src/firejail/env.c), but the environment needs to be copied and then nuked. Though perhaps some variables should be kept (based on a whitelist), like LC_ALL to get localized messages from Firejail itself.

@topimiettinen
Copy link
Collaborator Author

Further items to consider:

Process niceness, realtime/IO/CPU scheduling data can be transferred but the server can't trust the values. These could be read from /proc but there could be TOCTOU issues.

Mount, PID, cgroup, user, network and other namespaces of the client could be joined but there could be TOCTOU issues.

Existing seccomp/BPF filters probably can't be re-applied. I don't know about kernel keyrings.

If the client has NoNewPrivileges flag set, it can't execute setuid binaries. This model would bypass the restriction. Though this may be exactly what we want: enable NoNewPrivileges globally. The app should have same NoNewPrivileges as the client (or even always forced to be set).

Signals should be passed from client to app. Exit code needs to be returned. Umask can be transferred, no trust issues there.

@rusty-snake
Copy link
Collaborator

What about distros like devuan w/o systemd?

@topimiettinen
Copy link
Collaborator Author

Systemd only creates the socket and launches Firejail when there's an incoming connection. The server can also do this easily without systemd.

The downsides are that the server will be waiting in the background even if not needed. Additional sandboxing features of systemd can't be used. The starting environment is also not so clearly defined, which could mean more sanitizing. The server should also behave as a traditional Unix daemon: fork, manage a pid file, close file descriptors, not just print to stdout/stderr (which is the usual way with systemd as the those are forwarded to journald/syslogd) but use system logging etc.

@FOSSONLY
Copy link

FOSSONLY commented Apr 6, 2020

I guess all these complications are among other things the reason why the Landlock LSM (https://landlock.io) was created. Certainly more complicated to implement (still in development), nor would the rulesets ever be as simple as with firejail, but from a security point of view the attack surface would be considerably reduced.

@topimiettinen
Copy link
Collaborator Author

Landlock is interesting, but its scope seems to be limited to filesystem access controls at least now. I also wonder if it will be easy to convince application developers to start using Landlock because in comparison, most applications (even security critical) do not manage their capabilities, use seccomp, integrate with SELinux to sandbox plugins etc. Even such non-intrusive changes as enabling anything more than basic hardening features in systemd service file are not well received in my experience by some developers in fear of breaking something. Application architectures also may not allow adding new sandboxing features, especially if existing unknown 3rd party plugins, libraries or tools can be expected to do anything they want.

@topimiettinen
Copy link
Collaborator Author

Opened #3322 to whitelist some environment variables and delete the rest. All variables will be re-applied for sandboxes.

@rusty-snake rusty-snake added the WIP: DON'T MERGE A PR that is still being worked on label Jul 16, 2020
@reinerh reinerh marked this pull request as draft February 22, 2021 00:39
@crocket
Copy link
Contributor

crocket commented Feb 25, 2022

I use OpenRC. Can this be used with OpenRC, too?

@rusty-snake
Copy link
Collaborator

#3315 (comment)

@glitsj16
Copy link
Collaborator

glitsj16 commented May 4, 2024

@topimiettinen

systemd v256 introduces run0 as non-SUID sudo replacement, using Polkit to manage authorization. Might be interesting to re-check its potential in the context of this PR.

Related resources:

@kmk3 kmk3 added the enhancement New feature request label Aug 25, 2024
@kmk3 kmk3 changed the title WIP: Firejail as a service WIP: feature: Firejail as a service Aug 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request WIP: DON'T MERGE A PR that is still being worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants