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

more gradual control over supplementary groups #2042

Open
dkavlakov opened this issue Jul 16, 2018 · 33 comments
Open

more gradual control over supplementary groups #2042

dkavlakov opened this issue Jul 16, 2018 · 33 comments
Labels
enhancement New feature request

Comments

@dkavlakov
Copy link

The set of global Firefox-specific settings imported in the latest palemoon.profile includes a "nogroups" line, that prevents group-based device permissions from working and leaves Palemoon without access to sound when ALSA is used. Unlike Firefox there is no forced Pulse Audio dependency in Palemoon (including the future 29 series) and it's profile should allow ALSA sound to be used.

@chiraag-nataraj chiraag-nataraj added the bug Something isn't working label Jul 16, 2018
@chiraag-nataraj chiraag-nataraj changed the title No ALSA sound in palemoon due to imported Firefox globals No ALSA sound in various browsers due to nogroups Jul 16, 2018
@chiraag-nataraj
Copy link
Collaborator

So I think the problem is that in the most common usecase, nogroups provides some additional security. Removing it by default when it works for most people (and provides additional security) doesn't make much sense imo. At the same time, I recognize that it screws over people who are currently using ALSA (and will continue to do so). The problem is that nogroups doesn't just allow ALSA to be used, right. It could enable other things which are far more nefarious. If you can document which specific groups are needed (by default, anyway), we might be able to bring in an --enable-alsa flag (and corresponding enable-alsa profile option) so that you can get ALSA sound back without having to disable nogroups.

@netblue30
Copy link
Owner

@dkavlakov: can you run groups in a terminal, something like this:

$ groups
netblue tty sudo autologin

It could give us an idea what we need to add by default.

@dkavlakov
Copy link
Author

dkavlakov commented Jul 20, 2018

lp cdrom floppy tape audio dip video plugdev users netdev lpadmin scanner kvm sambashare wireshark vboxusers bluetooth

This is a pretty typical example of the pre-systemd-takeover Debian group permissions model.

Is it security risk? It may become in case of serious misuse of the filesystem group permissions. Is it less secure than your current settings? No! Right now to have sound in Palemoon and Chromium one is supposed to install huge amount of bloatware that includes even an additional, self-sufficient privilege elevation system, managed by huge set of policies, written by multiple third parties . Getting rid of this realy does provide additional security.

[edit] Ok, looking at the "sudo" group membership in the upper example i am forced to agree that with sufficient amount of Ubuntu in the mind one can easily turn group permissions into a threat. This however also proves that having more installed packages with the word "security" in their description (like sudo in the example) does not necessary give you more security.

@chiraag-nataraj
Copy link
Collaborator

chiraag-nataraj commented Jul 21, 2018

@dkavlakov
I hope you realize that pulseaudio is separate from systemd, was adopted well before systemd, and is a completely separate project. The fact that Lennart was a huge force behind both is the only thing that connects the two.

That being said, yes, pulseaudio (like most programs on the "modern" desktop) is not designed with security in mind. dbus is a huge security issue. But this is nowhere near an issue unique to pulseaudio or dbus. Most programs don't sandbox themselves. Many system daemons run as root or install SUID executables. Indeed, I would posit that systemd (love it or hate it) has made some of these issues better in that it's exposed some of the same technologies that firejail uses (such as namespaces) in a way such that some distributions and projects have adopted stricter defaults than they were doing under sysvinit. Socket activation means that systemd can set up the sockets and pass them to the program (which means the program no longer needs to run as root just to set up sockets). The same is true for things like temporary directories. So even if many programs aren't taking advantage of it right now, these features mean that they can take advantage of it in the future. Further, the drop-in system ensures I can add stricter defaults to my system (as I have done with several daemons) without having to futz around with the files installed by the system (ensuring I don't need to do manual merging of changes on upgrade).

If you're going to shit on systemd and pulseaudio, at least stick to the facts. I'm neutral on systemd (I like some aspects, but dislike others) and pulseaudio, but I can't stand this mindless hatred when some of these features are objectively a good thing in that they lead to more secure defaults and fewer programs running as root. That is unquestionably a good thing.

[Edit] Also, I literally have most of the same groups in my user, and I'm running a pure systemd system:

$ groups
chiraag tty disk dialout cdrom floppy sudo audio dip video plugdev netdev bluetooth lpadmin scanner kvm boinc

@chiraag-nataraj
Copy link
Collaborator

@netblue30 I think this is doable, but I'm not quite sure exactly how to go about adding this. I think having a --groups= predicate on the command-line and a groups predicate in profiles would be useful. It could retain only those groups while cleaning everything else and would provide a setting other than the binary on/off that we have right now.

@chiraag-nataraj chiraag-nataraj added enhancement New feature request and removed bug Something isn't working labels Jul 25, 2018
@chiraag-nataraj
Copy link
Collaborator

--groups=none or groups none could function equivalently to the current --nogroups and nogroups options we have right now.

@rusty-snake rusty-snake changed the title No ALSA sound in various browsers due to nogroups more gradulant control over supplementary groups Jan 25, 2020
@glitsj16
Copy link
Collaborator

@rusty-snake Did you mean 'gradual' control?

@rusty-snake
Copy link
Collaborator

sure, thk

@rusty-snake rusty-snake changed the title more gradulant control over supplementary groups more gradual control over supplementary groups Jan 25, 2020
@Leebre
Copy link

Leebre commented Mar 12, 2020

Hi, I am a new user to firejail. I like it a lot and was going to make a suggestion related to this, but saw this existing issue. Another example from my own use is Skype - it needs access to both the audio and video groups in order to make video calls. So, it won't work correctly with the --nogroups option enabled.

Is it possible for a future release that the groups you want to be available in the sandbox could be specified? --nogroups seems a bit 'all-or-nothing'.

I guess a workaround would be to create another user with more restrictive group permissions and su to them first. But that might lead to difficulties if you need to access anything in the first user's $home directory.

@jonleivent
Copy link

@Leebre

I guess a workaround would be to create another user with more restrictive group permissions and su to them first. But that might lead to difficulties if you need to access anything in the first user's $home directory.

I use the "workaround" you describe for other reasons, but I think it is very much worth the effort to do this for the extra security it brings. The "separate users from each other" security model is built into Linux and very well tested and adhered to, so, if you have the Linux know-how to use it, why abandon it just because we have sandboxing these days? Firejail sandboxing without user separation is convenient and easy to set up - and this is firejail's chief benefit over other techniques - but if you can combine multiple techniques to get additive security, go for it.

To deal with access to the first user's home dir, I have the first user be a member in each auxiliary user's group. That way I can selectively enable access, and keep track of who can access what very easily.

As for the audio (and video) issues vs. security, I would love to see firejail implement bridges for these - much the same as it has the ability to bridge network and X11.

@Leebre
Copy link

Leebre commented Mar 14, 2020

@jonleivent thanks for your advice on this. I agree, it does make sense to use the existing Linux user/group privileges as well as firejail, so I will give that a try. Still, more granular control of group permissions within firejail might be something worth considering for future development.

@jonleivent
Copy link

@Leebre I agree with having detailed control over group permissions. There are popular apps, for instance virtualbox, that require certain group permission, and it would be nice to drop all unnecessary group permission while whitelisting the necessary ones.

@topimiettinen
Copy link
Collaborator

udevd has uaccess rules, which are used by systemd-logind (maybe also elogind) to change ACLs of the devices (audio, video or cdrom). When a user logs in, the ACLs give access to the devices and the ACLs are removed on logout. This is safer than the older method of using special groups to give access, because once the group rights are given, the user could keep them forever by making a setgid executable. Also, a remote user could use the groups to mess remotely with locally logged in user.

But the group method is still used for other purposes, so the selective groups would be nice. One (a bit theoretical) catch is that the group membership could be used to limit access (g=rx vs o=rwx), but since we allow dropping all groups it's already broken.

@yardenac
Copy link

Agreed, granular group settings should be a no-brainer. The overlap between nogroup and noroot are confusing and unnecessary, and this would solve both problems.

@crocket
Copy link
Contributor

crocket commented Sep 11, 2021

I had to debug ALSA sound issue for a few hours until I figured out the issue by searching issues and pull requests.

Neglecting ALSA users isn't cool.

@kmk3
Copy link
Collaborator

kmk3 commented Oct 26, 2021

From:

@kmk3 on Oct 26:

@muziker on Oct 26:

Thanks! That's really helpful. Is it possible to have firejail keep
users:groups for only particular users/groups, so others are nobody:nobody?

It would be possible if we had something like groups.keep foo,bar.

Most recent discussion on that:

@rusty-snake on Oct 26:

Partly, groups.keep (#2042) is only about clearing/keeping supplementary
groups. This would also require to keep other uids/gids in the private user
ns like noroot.keep 1001.

I think that the following would be a rather self-descriptive way to use it:

users.keep foo,bar
groups.keep foo,bar

It would enable granularity both for keeping individual users (and their
respective user groups) and for keeping system groups.

Though for new whitelisting commands, I'd suggest splitting the "add to
whitelist" command from the "enforce/enable whitelist" command, so that the
former (i.e.: the something.keep commands) could be safely used on include
profiles. Example commands:

users.keep foo,bar
groups.keep foo,bar

users.enforce
groups.enforce

The first two commands would be a noop without the later ones (so it would have
the same effect of trying to whitelist a non-existent file, for example).

This is probably a topic worth its own discussion, but since new commands are
being suggested, might as well put a remark in here.


Misc: Originally (on a WIP proposal), I thought of using "enforce" for
whitelist.enforce, seccomp.enforce and others (with the same goal of adding
new subcommands to split the functionality of each command), so I used the same
name above for consistency.

@rusty-snake
Copy link
Collaborator

IMHO we talk about improving two similar but different commands:

  • noroot sets up a new user ns with only the current user and a few supplementary groups
  • nogroups removes supplementary groups from the current user

Is your proposal regarding keeping additional uids/gids in a private user ns (which I assume) or selectively keeping supplementary groups?


In all that, keep the following question in mind: What is the function of nogroups if you keep additional uids with noroot? Only for the current user or for all users?

@kmk3
Copy link
Collaborator

kmk3 commented Oct 26, 2021

@rusty-snake commented on Oct 26:

IMHO we talk about improving two similar but different commands:

  • noroot sets up a new user ns with only the current user and a few
    supplementary groups
  • nogroups removes supplementary groups from the current user

Is your proposal regarding keeping additional uids/gids in a private user ns
(which I assume) or selectively keeping supplementary groups?

I'm not sure I understand the question (maybe because I'm not familiar with
everything that a user ns entails), but I assume that you mean non-system
users/groups (e.g.: "rusty") vs system groups (e.g.: "audio"). I thought about
using the same command (groups.keep) to whitelist any group in /etc/groups
(so that only the whitelisted groups would exist in the sandbox).

If you mean implementation-wise, as in hiding groups from /etc/groups inside
the sandbox vs making the user not be part of any system groups inside the
sandbox (through whatever means this is currently achieved with), I hadn't
thought about the distinction. I meant mostly the former, but I think both
could be done at once for the same command. Would that make it too
complicated?

In all that, keep the following question in mind: What is the function of
nogroups if you keep additional uids with noroot? Only for the current
user or for all users?

In my mind, this is what each command is (primarily) supposed to do (from
having previously looked at some of the code and the man pages):

  • noroot: Hide all other non-system users/groups (including root) -> hide
    other dirs in /home (and hide /root) and hide other non-system UIDs and GIDs.
  • nogroups: Hide all (non-whitelisted / non-hardcoded) system groups.

I know that it's probably not exactly how they work, but assuming that the
above was the case, I guess that they would be the equivalent of
users.enforce and groups.enforce from my previous comment. So they would
still do the above, but without dropping the users in users.keep and the
groups in groups.keep.


Misc: Now I see that one might want to keep certain groups for user1, but not
for user2. I hadn't thought about this scenario, just about dropping groups
for all users in the sandbox (so it would be "system-wide" inside the sandbox).

Maybe we could also have something like git does for subkeys (e.g.:
branch.<name>.remote):

groups.user1.keep foo,bar
groups.user2.keep foo

Though this is more unwieldy, so it's probably better to leave this part for
later.

@crocket
Copy link
Contributor

crocket commented Oct 27, 2021

I think noroot should not touch groups for simplicity. If noroot touches groups, we lose separation of concern, and it becomes more difficult to reason about profiles.

nogroups should touch groups, or nogroups should account for noroot.

kmk3 added a commit to kmk3/firejail that referenced this issue Nov 30, 2021
Currently, on systems that use seat managers that do not implement
seat-based ACLs (such as seatd), sound is broken whenever `nogroups` is
used.  This happens because without ACLs, access to the audio devices in
/dev is controlled by the standard group permissions and the "audio"
group is always dropped when `nogroups` is used.  This patch makes the
"audio" and "video" groups be dropped if and only if `noaudio` and
`novideo` are in effect, respectively (and independently of `nogroups`).
See netblue30#4603 and the linked issues/discussions for details.

Note: This is a continuation of commit ea564eb ("Consider nosound and
novideo when keeping groups") / PR netblue30#4632.

Relates to netblue30#2042 and netblue30#4531.
kmk3 added a commit to kmk3/firejail that referenced this issue Nov 30, 2021
Mappings of command -> group that this commit adds:

* no3d -> render
* noprinters -> lp
* nodvd -> cdrom (Debian[1] and Gentoo[2]), optical (Arch[3])
* noinput -> input

Mappings that were considered but that are not added:

* notv -> ? (unknown group)
* nou2f -> ? (devices are apparently owned by root; see netblue30#4603)

Based on @rusty-snake's suggestion:
netblue30#4603 (comment)

See the previous commit ("Keep audio and video groups regardless of
nogroups") for details.

Relates to netblue30#2042 and netblue30#4632.

[1] https://wiki.debian.org/SystemGroups
[2] https://api.gentoo.org/uid-gid.txt
[3] https://wiki.archlinux.org/title/Users_and_groups
@JCallicoat
Copy link

I just spent a while going down rabbit holes trying to figure out why webrender was broken in firefox. It ended up being that /dev/nvidia* files were owned by the vglusers group (gid 1000 on my system) because of using virtualgl. The virtualgl server creates udev rules to set that group on the devices.

Since vglusers is not in the supplementary groups that are kept with noroot, the /dev/nvidia* files were getting the nobody group and firefox couldn't access them. When I changed the group ownership of the files to render or video (or set ignore noroot) then everything worked.

It would nice to be able to control what additional groups noroot will keep for cases like this. It looks like there was some discussion about it that at #4603 but I guess it was decided against?

@kmk3
Copy link
Collaborator

kmk3 commented Jan 7, 2022

@JCallicoat commented on Jan 7:

I just spent a while going down rabbit holes trying to figure out why
webrender was broken in firefox. It ended up being that /dev/nvidia* files
were owned by the vglusers group (gid 1000 on my system) because of using
virtualgl. The virtualgl server creates udev rules to set that group on the
devices.

Interesting, I never heard of this group.

On what distro/version?

I did not find such udev rules on Artix:

$ grep vglusers /usr/lib/udev/rules.d/*
$
$ pacman -Flq virtualgl | grep udev
$

Where are they defined? Can you post them here?

Since vglusers is not in the supplementary groups that are kept with
noroot, the /dev/nvidia* files were getting the nobody group and firefox
couldn't access them. When I changed the group ownership of the files to
render or video (or set ignore noroot) then everything worked.

We could treat the "vglusers" group the same as the "render" group: Keep by
default and only drop when no3d is used.

It would nice to be able to control what additional groups noroot will keep
for cases like this. It looks like there was some discussion about it that at
#4603 but I guess it was decided against?

Given the current implementation, adding more hardcoded groups is the easiest
way to fix group-related issues (see #4725/#4732).

But if it does not work well enough, maybe groups.keep could be added
eventually.

Do you know of any other group that firejail misses?

Or of any other tools that override the default groups for device files?

@JCallicoat
Copy link

Interesting, I never heard of this group.

I suspect it's may be somewhat niche for people to be using virtualgl. I mainly use it for passing through to a real X server from a nested server like Xypher / Xpra or from containers with X socket passed through.

I don't use it much anymore and forgot about the group ownership and udev rules and different configs it generates. The setup script actually does a whole bunch of stuff https://github.com/VirtualGL/virtualgl/blob/6f0b90be02d13171dfdfffb112485f4091a5904f/server/vglserver_config#L393.

We could treat the "vglusers" group the same as the "render" group: Keep by
default and only drop when no3d is used.

I honestly don't know how big the virtualgl user base is to say if it's worthwhile to special case it, I just thought it would be another data point when considering the groups.keep idea.

Do you know of any other group that firejail misses?

Or of any other tools that override the default groups for device files?

I couldn't think of any offhand but looking in /dev on this box I do see /dev/vboxusb owner by root:vboxusers.

@kmk3
Copy link
Collaborator

kmk3 commented Jan 9, 2022

@JCallicoat commented on Jan 7:

Interesting, I never heard of this group.

I suspect it's may be somewhat niche for people to be using virtualgl. I
mainly use it for passing through to a real X server from a nested server
like Xypher / Xpra or from containers with X socket passed through.

I don't use it much anymore and forgot about the group ownership and udev
rules and different configs it generates. The setup script actually does a
whole bunch of stuff
https://github.com/VirtualGL/virtualgl/blob/6f0b90be02d13171dfdfffb112485f4091a5904f/server/vglserver_config#L393.

vglserver_config

	elif [ "$UNAME_S" = "Linux" ]; then
	# [...]
		if [ "$FBDEVVGLUSERSONLY" = "1" ]; then
			if [ -e /dev/nvidia0 -o -e /dev/nvidiactl ]; then
				echo ... Granting write permission to /dev/nvidia* for vglusers group ...
				chmod 660 /dev/nvidia*
				chown root:vglusers /dev/nvidia*
			fi
			if ls /dev/dri/card* >/dev/null 2>&1; then
				echo ... Granting write permission to /dev/dri/card* for vglusers group ...
				chmod 660 /dev/dri/card*
				chown root:vglusers /dev/dri/card*
			fi
			if ls /dev/dri/renderD* >/dev/null 2>&1; then
				echo ... Granting write permission to /dev/dri/renderD* for vglusers group ...
				chmod 660 /dev/dri/renderD*
				chown root:vglusers /dev/dri/renderD*
			fi
		else
			if [ -e /dev/nvidia0 -o -e /dev/nvidiactl ]; then
				echo ... Granting write permission to /dev/nvidia* for all users ...
				chmod 666 /dev/nvidia*
				chown root:root /dev/nvidia*
			fi
			if ls /dev/dri/card* >/dev/null 2>&1; then
				echo ... Granting write permission to /dev/dri/card* for all users ...
				chmod 666 /dev/dri/card*
				chown root:root /dev/dri/card*
			fi
			if ls /dev/dri/renderD* >/dev/null 2>&1; then
				echo ... Granting write permission to /dev/dri/renderD* for all users ...
				chmod 666 /dev/dri/renderD*
				chown root:root /dev/dri/renderD*
			fi
		fi
		if [ -d /etc/udev/rules.d ]; then
			if [ "$FBDEVVGLUSERSONLY" = "1" ]; then
				echo "KERNEL==\"card*|renderD*\", MODE=\"0660\", OWNER=\"root\", GROUP=\"vglusers\"" >/etc/udev/rules.d/99-virtualgl-dri.rules
			else
				echo "KERNEL==\"card*|renderD*\", MODE=\"0666\", OWNER=\"root\", GROUP=\"root\"" >/etc/udev/rules.d/99-virtualgl-dri.rules
			fi
		fi
	# [...]
	fi

Well, that's quite the party going on in /dev.

Is this done only on the guest? Does it do that on the host with xephyr/xpra?

Do you have to reboot to restore the original group ownership?

We could treat the "vglusers" group the same as the "render" group: Keep by
default and only drop when no3d is used.

I honestly don't know how big the virtualgl user base is to say if it's
worthwhile to special case it, I just thought it would be another data point
when considering the groups.keep idea.

I see, that makes sense.

Not sure if/when groups.keep would be implemented though, so treating
"vglusers" the same as the "render" group could be a workaround until then,
especially if both target the same devices.

Do you know if the group is ever applied to anything other than render devices?

Do you know of any other group that firejail misses? Or of any other tools
that override the default groups for device files?

I couldn't think of any offhand but looking in /dev on this box I do see
/dev/vboxusb owner by root:vboxusers.

That's one good argument for groups.keep, as it's an application-specific
group and that device indeed should probably only be accessible by virtualbox.

@JCallicoat
Copy link

Well, that's quite the party going on in /dev.

Is this done only on the guest? Does it do that on the host with xephyr/xpra?

Do you have to reboot to restore the original group ownership?

I know right haha? It's done on the host X server which renders the GL there for the "remote" client by launching programs with vglrun inside the client. The config script also has an uninstall option but it sets the ownership on all the device files to root:root instead of root:video for /dev/nvidia* and root:render for /dev/dri/* heh. If I were to remove it I would manually reset the ownership and remove the files it drops in /etc and reboot.

I totally forgot about it changing the groups. and reading your comment here #3644 (comment) when I was trying to figure out why webrender was throwing errors in firefox made me remember.

Not sure if/when groups.keep would be implemented though, so treating
"vglusers" the same as the "render" group could be a workaround until then,
especially if both target the same devices.

That would definitely work for my use case. These are the files with the groups changed by the virtualgl config script and the udev rules it creates:

crw-rw---- 1 root vglusers 195,   0 Jan  7 11:40 /dev/nvidia0
crw-rw---- 1 root vglusers 195, 255 Jan  7 11:40 /dev/nvidiactl
crw-rw---- 1 root vglusers 195, 254 Jan  7 11:40 /dev/nvidia-modeset
crw-rw----+ 1 root vglusers 226,   0 Jan  7 11:40 /dev/dri/card0
crw-rw----  1 root vglusers 226, 128 Jan  7 11:40 /dev/dri/renderD128

@kmk3
Copy link
Collaborator

kmk3 commented Jan 12, 2022

@JCallicoat commented on Jan 11:

Well, that's quite the party going on in /dev.

Is this done only on the guest? Does it do that on the host with
xephyr/xpra?

Do you have to reboot to restore the original group ownership?

I know right haha? It's done on the host X server which renders the GL there
for the "remote" client by launching programs with vglrun inside the client.
The config script also has an uninstall option but it sets the ownership on
all the device files to root:root instead of root:video for /dev/nvidia* and
root:render for /dev/dri/* heh. If I were to remove it I would manually reset
the ownership and remove the files it drops in /etc and reboot.

I see; thanks for the details.

I totally forgot about it changing the groups. and reading your comment here #3644 (comment)
when I was trying to figure out why webrender was throwing errors in firefox
made me remember.

Glad it helped!

Rambling

Not sure if/when groups.keep would be implemented though, so treating
"vglusers" the same as the "render" group could be a workaround until then,
especially if both target the same devices.

That would definitely work for my use case. These are the files with the
groups changed by the virtualgl config script and the udev rules it creates:

crw-rw---- 1 root vglusers 195,   0 Jan  7 11:40 /dev/nvidia0
crw-rw---- 1 root vglusers 195, 255 Jan  7 11:40 /dev/nvidiactl
crw-rw---- 1 root vglusers 195, 254 Jan  7 11:40 /dev/nvidia-modeset
crw-rw----+ 1 root vglusers 226,   0 Jan  7 11:40 /dev/dri/card0
crw-rw----  1 root vglusers 226, 128 Jan  7 11:40 /dev/dri/renderD128

Good to know. Default ownership on Artix (with AMD):

$ pacman -Q mesa udev
mesa 21.3.3-2
udev 250-2
$ find /dev -group render | LC_ALL=C sort
/dev/dri/renderD128
/dev/kfd
$ find /dev -group video | LC_ALL=C sort
/dev/dri/card0
/dev/fb0

So as you said, the default ownership for nvidia devices on your system is
"root:video", which also seems to be the default on Gentoo:

@hlein commented on Oct 2, 2020:

There are several, all but /dev/nvidia-caps/ get group 'video':

drwxr-xr-x   2 root root            80 Sep 21 13:54 nvidia-caps/
crw-rw----   1 root video     195, 254 Sep 23 11:17 nvidia-modeset
crw-rw----   1 root video     195,   0 Sep 23 11:17 nvidia0
crw-rw----   1 root video     195,   1 Sep 23 11:17 nvidia1
crw-rw----   1 root video     195, 255 Sep 23 11:17 nvidiactl

I have two cards, so that's likely the nvidiaN's, and I guess a single
nvidiactl and nvidia-modeset created by the driver.

I have not tried giving access to only a subset of those (like, maybe write
access to nvidiaN but not to nvidiactl would be sufficient?).

This seems to be set by udev. From /usr/lib/udev/rules.d/50-udev-default.rules
(udev 250-2 on Artix):

SUBSYSTEM=="video4linux", GROUP="video"
SUBSYSTEM=="graphics", GROUP="video"
SUBSYSTEM=="drm", KERNEL!="renderD*", GROUP="video"
SUBSYSTEM=="dvb", GROUP="video"
SUBSYSTEM=="media", GROUP="video"
SUBSYSTEM=="cec", GROUP="video"

SUBSYSTEM=="drm", KERNEL=="renderD*", GROUP="render", MODE="0666"
SUBSYSTEM=="kfd", GROUP="render", MODE="0666"

I'm not sure if this means that these nvidia devices allow both rendering and
video capture. In which case maybe "vglusers" should only be kept only if
there is neither no3d nor novideo.

But it may also just be a workaround required by the nvidia drivers. And on
firejail they are treated as DEV_3D devices (which are controlled by no3d
and are usually owned by the "render" group):

static DevEntry dev[] = {

static DevEntry dev[] = {
	{"/dev/snd", RUN_DEV_DIR "/snd", DEV_SOUND},	// sound device
	{"/dev/dri", RUN_DEV_DIR "/dri", DEV_3D},		// 3d device
	{"/dev/nvidia0", RUN_DEV_DIR "/nvidia0", DEV_3D},
	{"/dev/nvidia1", RUN_DEV_DIR "/nvidia1", DEV_3D},
	{"/dev/nvidia2", RUN_DEV_DIR "/nvidia2", DEV_3D},
	{"/dev/nvidia3", RUN_DEV_DIR "/nvidia3", DEV_3D},
	{"/dev/nvidia4", RUN_DEV_DIR "/nvidia4", DEV_3D},
	{"/dev/nvidia5", RUN_DEV_DIR "/nvidia5", DEV_3D},
	{"/dev/nvidia6", RUN_DEV_DIR "/nvidia6", DEV_3D},
	{"/dev/nvidia7", RUN_DEV_DIR "/nvidia7", DEV_3D},
	{"/dev/nvidia8", RUN_DEV_DIR "/nvidia8", DEV_3D},
	{"/dev/nvidia9", RUN_DEV_DIR "/nvidia9", DEV_3D},
	{"/dev/nvidiactl", RUN_DEV_DIR "/nvidiactl", DEV_3D},
	{"/dev/nvidia-modeset", RUN_DEV_DIR "/nvidia-modeset", DEV_3D},
	{"/dev/nvidia-uvm", RUN_DEV_DIR "/nvidia-uvm", DEV_3D},
	{"/dev/video0", RUN_DEV_DIR "/video0", DEV_VIDEO}, // video camera devices
	{"/dev/video1", RUN_DEV_DIR "/video1", DEV_VIDEO},
	{"/dev/video2", RUN_DEV_DIR "/video2", DEV_VIDEO},
	{"/dev/video3", RUN_DEV_DIR "/video3", DEV_VIDEO},
	{"/dev/video4", RUN_DEV_DIR "/video4", DEV_VIDEO},
	{"/dev/video5", RUN_DEV_DIR "/video5", DEV_VIDEO},
	{"/dev/video6", RUN_DEV_DIR "/video6", DEV_VIDEO},
	{"/dev/video7", RUN_DEV_DIR "/video7", DEV_VIDEO},
	{"/dev/video8", RUN_DEV_DIR "/video8", DEV_VIDEO},
	{"/dev/video9", RUN_DEV_DIR "/video9", DEV_VIDEO},

Conclusion

So considering the way firejail currently deals with these devices, I think it
would make sense to just treat "vglusers" the same as the "render" group
indeed. Though let me know if I missed anything.

Anyway, I'll open a PR later.

kmk3 added a commit to kmk3/firejail that referenced this issue Jan 12, 2022
virtualgl[1] runs `chown root:vglusers` on `/dev/nvidia*` and on devices
usually owned by the "render" group[2].  This makes them unavailable in
the sandbox if `noroot` (which causes groups to be dropped) is used.

Since firejail classifies all of the aforementioned devices as being
`DEV_3D` on fs_dev.c (which means that they are controlled by `no3d`),
treat the "vglusers" group the same as the "render" group (by always
keeping "vglusers" unless `no3d` is used).

See the discussion on netblue30#2042 (from this comment[3] onwards).

[1] https://virtualgl.org
[2] https://github.com/VirtualGL/virtualgl/blob/6f0b90be02d13171dfdfffb112485f4091a5904f/server/vglserver_config#L393
[3] netblue30#2042 (comment)

Reported-by: @JCallicoat
@crocket
Copy link
Contributor

crocket commented Mar 10, 2022

https://git.sr.ht/~kennylevinsen/pam_uaccess substitutes for logind's uaccess functionality.

@jonleivent
Copy link

Is any progress being made on this topic (the "more gradual control over supplementary groups" part)?

I would really like to be able to usenoroot more often, and then also selectively restrict supplementary groups.

@kmk3
Copy link
Collaborator

kmk3 commented Jun 25, 2022

@jonleivent commented on Jun 24:

Is any progress being made on this topic (the "more gradual control over
supplementary groups" part)?

I would really like to be able to usenoroot more often

What is the issue specifically?

I have some been working on some group-related fixes that I intend to submit
after the ongoing build improvements, which may or may not relate to your
issue.

and then also selectively restrict supplementary groups.

I haven't worked on this, but I would also like to see something like
groups.keep eventually.

@jonleivent
Copy link

@kmk3:

What is the issue specifically?

Are you asking why I would want to use noroot more often? I thought it was generally understood that noroot containers are safer. I guess my whole premise is mistaken if noroot containers are not safer.

A specific instance: I discovered that KVM can be run in a noroot firejail provided one chmods or chowns the /dev/kvm socket so that the KVM user does not need to be in the kvm group to use it. Then a noroot and nogroups firejail can be wrapped around KVM and it works. However, it shouldn't be necessary to alter the external-to-firejail environment of KVM by chmoding /dev/kvm (which also needs to be redone at startup every time) just to get the safest firejail environment for KVM.

KVM is an instance of an app that uses groups in a very normal linux way: to enable sysadms to provide permission on a user-by-user basis through groups. It would be nice if firejail accommodated this usage in an expandable way, in other words, not by building into firejail support only for particular predefined groups.

Suppose, for instance, firejail had a noroot-with-group-file option that took a file parameter in the format of /etc/group, checked that its contents are a permissible subset of the existing /etc/group (no new membership is added: only users are removed from groups and/or whole groups are removed), and used it within the container (along with removing the root user).

@rusty-snake
Copy link
Collaborator

rusty-snake commented Jun 25, 2022

which also needs to be redone at startup every time

OT: udev-rules


noroot-with-group-file

noroot should not mess with groups at all IMHO.

@jonleivent
Copy link

@rusty-snake

OT: udev-rules

True enough. The point being that it takes a modification to something (be that /etc/rc.local, or /usr/lib/udev/rules.d) that modifies the environment external to firejail to accomplish the task of giving KVM a safer environment within firejail.

noroot shound not mess with groups at all IMHO

I agree. But it currently does. If you want backward-compatibility, it might be best to add a new option, keeping noroot with its current, albeit mistaken, functionality. Because if you modify noroot to not alter groups at all, then even if you simultaneously modify all profiles to reflect that, some people who have their own profiles that use noroot will experience a lowering of protections possibly without realizing it.

@kmk3

I haven't worked on this, but I would also like to see something like
groups.keep eventually.

I'd be OK with that instead of noroot-with-group-file. It's probably easier to accomplish, less prone to error, and easier to use within the provided /etc/firejail profiles framework. It has less functionality than noroot-with-group-file, as the latter can drop users from groups without dropping those groups. But maybe that added functionality isn't worth it.

@kmk3
Copy link
Collaborator

kmk3 commented Jun 27, 2022

@jonleivent commented on Jun 25:

@kmk3:

What is the issue specifically?

Are you asking why I would want to use noroot more often?

Sorry, I meant what was the specific issue that noroot was causing.

I thought it was generally understood that noroot containers are safer. I
guess my whole premise is mistaken if noroot containers are not safer.

A specific instance: I discovered that KVM can be run in a noroot firejail
provided one chmods or chowns the /dev/kvm socket so that the KVM user does
not need to be in the kvm group to use it. Then a noroot and nogroups
firejail can be wrapped around KVM and it works. However, it shouldn't be
necessary to alter the external-to-firejail environment of KVM by chmoding
/dev/kvm (which also needs to be redone at startup every time) just to get
the safest firejail environment for KVM.

I see, so the problem is that noroot causes the kvm group to be dropped,
which results in permission issues.

KVM is an instance of an app that uses groups in a very normal linux way: to
enable sysadms to provide permission on a user-by-user basis through groups.
It would be nice if firejail accommodated this usage in an expandable way, in
other words, not by building into firejail support only for particular
predefined groups.

That indeed seems like a reasonable use use of groups and I agree that it
should be supported.

@rusty-snake commented on Jun 25:

noroot-with-group-file

noroot should not mess with groups at all IMHO.

@jonleivent commented on Jun 25:

noroot shound not mess with groups at all IMHO

I agree. But it currently does. If you want backward-compatibility, it might
be best to add a new option, keeping noroot with its current, albeit
mistaken, functionality. Because if you modify noroot to not alter groups
at all, then even if you simultaneously modify all profiles to reflect that,
some people who have their own profiles that use noroot will experience a
lowering of protections possibly without realizing it.

The reason that noroot drops groups is that it sets up a user namespace where
the root user/group do not exist (see "User and group ID mappings" in
user_namespaces(7)). Whereas nogroups just uses setgroups(2) to drop
groups.

The last time I looked into this IIRC, the code seemed a bit tricky and
dropping all groups with noroot is likely the least annoying way to do it,
considering how it's currently written and the format of gid_map. But
noroot doesn't really have to drop all (non-whitelisted) groups and I think
that it could be made to drop only the root user/group.

Misc: I now noticed that gid_map has a limit of 340 lines (see
user_namespaces(7)), which would likely mean up to 340 groups in a simple
implementation; not sure if this would ever be a problem. Unless maybe if
running in an Android-style system, where each program has its own user/group.

IIRC one way to do it would be to refactor noroot / nogroups to use the
same mechanism(s) to drop groups and then split the processing of noroot from
nogroups. That is, first drop the root user/group (if noroot is enabled),
then do another pass to deal with the other groups.

Anyway, this would be the result:

  • noroot: drops root user/group
  • nogroups: keeps whitelisted groups, keeps/drops supplementary groups based
    on the relevant options (such as nosound -> drops audio group), drops all
    other groups

Suppose, for instance, firejail had a noroot-with-group-file option that
took a file parameter in the format of /etc/group, checked that its
contents are a permissible subset of the existing /etc/group (no new
membership is added: only users are removed from groups and/or whole groups
are removed), and used it within the container (along with removing the root
user).

Misc: If noroot is changed as per above, I think just group-file file would
be a more appropriate name.

I haven't worked on this, but I would also like to see something like
groups.keep eventually.

I'd be OK with that instead of noroot-with-group-file. It's probably easier
to accomplish, less prone to error, and easier to use within the provided
/etc/firejail profiles framework. It has less functionality than
noroot-with-group-file, as the latter can drop users from groups without
dropping those groups. But maybe that added functionality isn't worth it.

That command does seem more powerful, but I don't understand what would be the
use case, so I'm not sure if it would warrant the extra complexity. That is,
inside of the sandbox, why/when would it matter in which supplementary groups
other users are?

On a side note, if group.keep accumulates, the following should be possible:

groups.keep foo
groups.keep bar
groups.keep baz

@rusty-snake
Copy link
Collaborator

The reason that noroot drops groups is that it sets up a user namespace where
the root user/group do not exist (see "User and group ID mappings" in
user_namespaces(7)).

And ... ?

You can unshare the userns w/o dropping unmapped groups.

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