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

Allow to compile firejail non-setuid #1846

Closed
Vincent43 opened this issue Mar 29, 2018 · 20 comments
Closed

Allow to compile firejail non-setuid #1846

Vincent43 opened this issue Mar 29, 2018 · 20 comments

Comments

@Vincent43
Copy link
Collaborator

Vincent43 commented Mar 29, 2018

Currently there are two ways to build sandboxes in linux:

  • setuid wrapper
  • user namespaces

Firejail is traditionally compiled as setuid binary but it can also use user namespaces when they're available. Both have their own security issues.

Some distros like Fedora or Opensuse are reluctant to add setuid binaries to official repos which means no firejail for their users. If we add possibility to compile firejail as non-setuid binary (--nosetuid ?) which uses unprivileged user namespaces to create sandbox then will be more chance that those distros will accept official firejail package. Also all users who are afraid of setuid bit will have alternative. Bubblewrap already provides this choice (on Archlinux and Debian it's compiled as setuid, on Fedora it uses namespaces).

@Vincent43 Vincent43 added the enhancement New feature request label Mar 30, 2018
@curiosity-seeker
Copy link
Contributor

I'm not an expert - but here are some thoughts:

  1. I remember that @netblue30 once wrote:

There are two different technologies you can use today to setup a sandbox: SUID and user namespaces. Quite funny, both of them are terribly insecure. User namespace has the advantage when things go wrong you can blame it on kernel people. For Firejail we use SUID, at least this one we can fix ourselves.

  1. Doesn't this still hold true? The concerns regarding user namespaces still exist if you look at newer posts in the related Arch Linux bug. Is it possible to overcome these concerns by applying the Bubblewrap approach?

Bubblewrap could be viewed as setuid implementation of a subset of user namespaces. Emphasis on subset - specifically relevant to the above CVE, bubblewrap does not allow control over iptables.

  1. What about suggestion b) here? Would that be a reasonable way to go? What would be the practical consequences - would it be no longer possible to sandbox, e.g., dnsmasq or unbound?

@Fred-Barclay
Copy link
Collaborator

I think it's worth considering, with a few reservations: 😄

  1. As @chiraag-nataraj points out, in the past we couldn't reliably count on user namespaces alone. It's my understanding that the combination of setuid and namespaces that is helpful here - i.e. they complement each other so that namespace escapes are blocked by the SUID options, and vice versa.

  2. How would this affect sandbox parameters that probably do require setuid? Whitelisting and blacklisting can (I think) be done pretty safely with normal user privileges, but I expect things like seccomp, protocol, dbus control, net control... quite a few of our features rely on setuid or root privs (I believe - please correct if I'm wrong!).

If someone compiles with --nosetuid, might they be lulled into a false sense of security with the belief that seccomp and the other above options are working, when they're really not? Esp. in cases like Fedora, where (if they use this approach) the user may not even be aware that it's running without the setuid bit.

  1. With the above note and my quite possible misunderstanding, if we look at taking this further, can we compile a list of precisely which features require root/setuid and which will work without?

@Vincent43
Copy link
Collaborator Author

@curiosity-seeker
That's true that both approaches can be insecure. Some distros choose namespaces, others setuid. For distros which prefers namespaces firejail is currently unavailable. Bubblewrap approach is just less code, less features. If firejail would mimic this then there would be no point to exist at all.

What about suggestion b) here? Would that be a reasonable way to go? What would be the practical consequences - would it be no longer possible to sandbox, e.g., dnsmasq or unbound?

This is interesting approach. System daemons can still be sandboxed as they're started by root which has always access to firejail binary. However this is packaging issue which still can be unacceptable for some distros.

@Fred-Barclay
I guess non-setuid firejail will have slightly less features which should be documented alongside compile time option. I think this would be still better than nothing.

With the above note and my quite possible misunderstanding, if we look at taking this further, can we compile a list of precisely which features require root/setuid and which will work without?

That would be very helpful to decide if this approach make any sense to follow. Unfortunately I'm unable to compile such list.

@curiosity-seeker
Copy link
Contributor

This is interesting approach. System daemons can still be sandboxed as they're started by root which has always access to firejail binary.

Yes, you're right. of course.

@curiosity-seeker
Copy link
Contributor

curiosity-seeker commented Apr 2, 2018

I've tried what is suggested above in b).
I created the group firejail, added my user to it and added two lines to the script I use to update the Firejail git version:

cd ~/firejail
git pull
./configure --prefix=/usr
make
sudo make install

sudo chown root:seeker /usr/bin/firejail
sudo chmod 4750 /usr/bin/firejail

Works well. No problems so far.

@Vincent43
Copy link
Collaborator Author

You probably mean sudo chown root:firejail /usr/bin/firejail instead of sudo chown root:seeker /usr/bin/firejail.

@curiosity-seeker
Copy link
Contributor

Yes, sorry - typo :-(

@netblue30
Copy link
Owner

Non-suid install here: ff1599f

You configure as usuall, and add --disable-suid:

$ ./configure --prefix=/usr --disable-suid

The variant with mode 4750 and group firejail seems to be working fine, but I'll do some more testing. After you add yourself to the group just remember to logout and login again.

@Vincent43
Copy link
Collaborator Author

Vincent43 commented Apr 2, 2018

@netblue30 can you elaborate what are consequences of --disable-suid for firejail functionality?

BTW: I think we could make mode 4750 and group firejail official recommendation.

@Fred-Barclay
Copy link
Collaborator

@Vincent43 The issue I see with mode 4750 and group firejail is that every single user on a system (I believe) will have to be manually added to the firejail group. Whereas with the current method, every user can run firejail.

It's certainly a great alternative, but I'm not sure it's suited for official recommendation (at least, not as the default).
Then again, probably a lot of desktop Linux systems are single-user anyway.

@Vincent43
Copy link
Collaborator Author

Yes, it can be an issue on multi-user systems. On Archlinux it can be fixed by using users group instead of firejail group which every real user belongs to by default.

@Fred-Barclay
Copy link
Collaborator

If I'm seeing it correctly, my user doesn't belong to users on Arch:

$ groups fred
wheel fred

@Vincent43
Copy link
Collaborator Author

Hm, maybe they changed it recently.

@netblue30
Copy link
Owner

netblue30 commented Apr 3, 2018

can you elaborate what are consequences of --disable-suid for firejail functionality

The sandbox will work only if you start it as root (for example if you run servers).

BTW: I think we could make mode 4750 and group firejail official recommendation.

Will do! However, it will work only for more technical users. For non-technical users I have this plan:

  • regular SUID install
  • the user runs "sudo firecfg"; this adds the user name to a file /etc/firejail/firejail.users; it will create the file if it's not there
  • when you start the sandbox, firejail checks /etc/firejail/users and refuses to run if the user is not in the list;
  • this can be used in parallel with 4750/root:firejail above

Question: is anybody using --git-install? I intend to remove it.

@Fred-Barclay
Copy link
Collaborator

Fred-Barclay commented Apr 3, 2018

Question: is anybody using --git-install? I intend to remove it.

I'm not. I just clone and build from source manually.

The sandbox will work only if you start it as root (for example if you run servers).

Good to know! So everything will still work as usual?

  • regular SUID install
  • the user runs "sudo firecfg"; this adds the user name to a file /etc/firejail/firejail.users; it will create the file if it's not there
  • when you start the sandbox, firejail checks /etc/firejail/users and refuses to run if the user is not in the list;

What advantages does this offer over the current method?
If I understand correctly this would handle the potential privilege escalation discussed in (b) of https://bugzilla.suse.com/show_bug.cgi?id=1059013#c9 - is that correct?

Can we have an option for people who don't use sudo firecfg to sandbox programs? Something like sudo firecfg --add-users so users are added to /etc/firejail/firejail.users but symlinks aren't created.

Can we pass a list of users when running the code to add users to /etc/firejail/firejail.users? sudo firecfg --add-users dustin lucas mike eleven or something like that. That should make it simpler to set up for multi-user machines.
If we require each user to run sudo firecfg, users without admin rights can't add themselves to the file and so can't use firejail. If an admin can specify user accounts to be added to the list, that would solve this potential issue.

@netblue30
Copy link
Owner

netblue30 commented Apr 3, 2018

The sandbox will work only if you start it as root (for example if you run servers).

Good to know! So everything will still work as usual?

Yes.

What advantages does this offer over the current method?
If I understand correctly this would handle the potential privilege escalation discussed in (b) of https://bugzilla.suse.com/show_bug.cgi?id=1059013#c9 - is that correct?

Yes, it will handle the same problem. It is easier to install than 4750/root:firejail, and we can make it invisible to the users.

sudo firecfg --add-users dustin lucas mike eleven

Sure, it is very easy to implement. I should also add a --remove-users. Or we can allow root user edit /etc/firejail/firejail.user with a text editor.

@Vincent43
Copy link
Collaborator Author

The /etc/firejail/firejail.user seems unnecessary to me. We can just make sudo firecfg to add user to firejail group (which can be created during install with sysusers.d) instead. And sudo firecfg --add-users dustin lucas mike eleven will add those users to firejail group instead of creating firejail specific files.

This way it would be transparent for users but won't need duplication of existing functionality in firejail

@Fred-Barclay
Copy link
Collaborator

What @Vincent43 said, with the exception that I would prefer us using something other than sysusers.d. There are still some distros out there where systemd isn't necessarily installed (Gentoo) or almost definitely isn't present (Devuan, the Arch and Manjaro OpenRC folks at Artix, and so on).

@Vincent43 Vincent43 added in testing A bugfix that is being tested and removed enhancement New feature request labels Apr 9, 2018
@Vincent43 Vincent43 removed the in testing A bugfix that is being tested label Apr 21, 2018
@curiosity-seeker
Copy link
Contributor

Kees Cook writes in his excellent blog regarding security enhancements in kernel 5.1:

SafeSetID LSM
Micah Morton added the new SafeSetID LSM, which provides a way to narrow the power associated with the CAP_SETUID capability. Normally a process with CAP_SETUID can become any user on the system, including root, which makes it a meaningless capability to hand out to non-root users in order for them to “drop privileges” to some less powerful user. There are trees of processes under Chrome OS that need to operate under different user IDs and other methods of accomplishing these transitions safely weren’t sufficient. Instead, this provides a way to create a system-wide policy for user ID transitions via setuid() (and group transitions via setgid()) when a process has the CAP_SETUID capability, making it a much more useful capability to hand out to non-root processes that need to make uid or gid transitions.

Could this become relevant for Firejail?

@Vincent43
Copy link
Collaborator Author

No, firejail needs to be root.

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

No branches or pull requests

4 participants