-
Notifications
You must be signed in to change notification settings - Fork 584
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
Comments
So I think the problem is that in the most common usecase, |
@dkavlakov: can you run groups in a terminal, something like this:
It could give us an idea what we need to add by default. |
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. |
@dkavlakov 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:
|
@netblue30 I think this is doable, but I'm not quite sure exactly how to go about adding this. I think having a |
|
@rusty-snake Did you mean 'gradual' control? |
sure, thk |
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 Is it possible for a future release that the groups you want to be available in the sandbox could be specified? I guess a workaround would be to create another user with more restrictive group permissions and |
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. |
@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. |
@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. |
udevd has 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. |
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. |
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. |
From:
I think that the following would be a rather self-descriptive way to use it:
It would enable granularity both for keeping individual users (and their Though for new whitelisting commands, I'd suggest splitting the "add to
The first two commands would be a noop without the later ones (so it would have This is probably a topic worth its own discussion, but since new commands are Misc: Originally (on a WIP proposal), I thought of using "enforce" for |
IMHO we talk about improving two similar but different commands:
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 |
@rusty-snake commented on Oct 26:
I'm not sure I understand the question (maybe because I'm not familiar with If you mean implementation-wise, as in hiding groups from /etc/groups inside
In my mind, this is what each command is (primarily) supposed to do (from
I know that it's probably not exactly how they work, but assuming that the Misc: Now I see that one might want to keep certain groups for user1, but not Maybe we could also have something like git does for subkeys (e.g.:
Though this is more unwieldy, so it's probably better to leave this part for |
I think
|
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.
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
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 Since vglusers is not in the supplementary groups that are kept with It would nice to be able to control what additional groups |
@JCallicoat commented on Jan 7:
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?
We could treat the "vglusers" group the same as the "render" group: Keep by
Given the current implementation, adding more hardcoded groups is the easiest But if it does not work well enough, maybe Do you know of any other group that firejail misses? Or of any other tools that override the default groups for device files? |
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.
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
I couldn't think of any offhand but looking in /dev on this box I do see /dev/vboxusb owner by |
@JCallicoat commented on Jan 7:
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?
I see, that makes sense. Not sure if/when Do you know if the group is ever applied to anything other than render devices?
That's one good argument for |
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.
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:
|
@JCallicoat commented on Jan 11:
I see; thanks for the details.
Glad it helped! Rambling
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 @hlein commented on Oct 2, 2020:
This seems to be set by udev. From /usr/lib/udev/rules.d/50-udev-default.rules
I'm not sure if this means that these nvidia devices allow both rendering and But it may also just be a workaround required by the nvidia drivers. And on firejail/src/firejail/fs_dev.c Line 53 in 121749f
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}, ConclusionSo considering the way firejail currently deals with these devices, I think it Anyway, I'll open a PR later. |
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
https://git.sr.ht/~kennylevinsen/pam_uaccess substitutes for logind's uaccess functionality. |
Is any progress being made on this topic (the "more gradual control over supplementary groups" part)? I would really like to be able to use |
@jonleivent commented on Jun 24:
What is the issue specifically? I have some been working on some group-related fixes that I intend to submit
I haven't worked on this, but I would also like to see something like |
Are you asking why I would want to use A specific instance: I discovered that KVM can be run in a 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 |
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.
I agree. But it currently does. If you want backward-compatibility, it might be best to add a new option, keeping
I'd be OK with that instead of |
@jonleivent commented on Jun 25:
Sorry, I meant what was the specific issue that
I see, so the problem is that
That indeed seems like a reasonable use use of groups and I agree that it @rusty-snake commented on Jun 25:
@jonleivent commented on Jun 25:
The reason that The last time I looked into this IIRC, the code seemed a bit tricky and Misc: I now noticed that IIRC one way to do it would be to refactor Anyway, this would be the result:
Misc: If
That command does seem more powerful, but I don't understand what would be the On a side note, if
|
And ... ? You can unshare the userns w/o dropping unmapped groups. |
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.
The text was updated successfully, but these errors were encountered: