Keep some groups regardless of nogroups and restore nogroups on nvidia

[kmk3]

$ git log --reverse --pretty='* %s' master..
* Keep audio and video groups regardless of nogroups
* Keep render, lp, input and other groups regardless of nogroups
* Make nogroups work on nvidia again
* etc: Remove comments about nogroups and noroot on nvidia

This should fix audio on seatd (#4531/#4603) and also make nogroups work
regardless of no3d on nvidia (#841/#3644).

Note: This is a continuation of #4632.

Note2: I have not tested this yet.

Cc: @crocket (from #4603) @hlein (from #4632)

[kmk3]

Should also fix access to gamepads when using seatd:

@crocket commented on Oct 10:

Either nogroup or noroot breaks ALSA(/dev/snd) and gamepad(/dev/input) on retroarch on Gentoo Linux.

$ ls -alh /dev/snd
total 0
drwxr-xr-x  3 root root      280 Oct 10 07:51 ./
drwxr-xr-x 18 root root     4.1K Oct 10 07:51 ../
drwxr-xr-x  2 root root       80 Oct 10 07:51 by-path/
crw-rw----  1 root audio 116, 10 Oct 10 07:51 controlC0
crw-rw----  1 root audio 116,  4 Oct 10 07:51 controlC1
crw-rw----  1 root audio 116,  9 Oct 10 07:51 hwC0D0
crw-rw----  1 root audio 116,  3 Oct 10 07:51 hwC1D0
crw-rw----  1 root audio 116,  6 Oct 10 07:51 pcmC0D0c
crw-rw----  1 root audio 116,  5 Oct 10 07:59 pcmC0D0p
crw-rw----  1 root audio 116,  7 Oct 10 07:51 pcmC0D1p
crw-rw----  1 root audio 116,  8 Oct 10 07:51 pcmC0D2c
crw-rw----  1 root audio 116,  2 Oct 10 07:51 pcmC1D3p
crw-rw----  1 root audio 116,  1 Oct 10 07:51 seq
crw-rw----  1 root audio 116, 33 Oct 10 07:51 timer

$ ls -alh /dev/input
total 0
drwxr-xr-x  4 root root     520 Oct 10 07:51 ./
drwxr-xr-x 18 root root    4.1K Oct 10 07:51 ../
drwxr-xr-x  2 root root     220 Oct 10 07:51 by-id/
drwxr-xr-x  2 root root     220 Oct 10 07:51 by-path/
crw-rw----  1 root input 13, 64 Oct 10 07:51 event0
crw-rw----  1 root input 13, 65 Oct 10 07:51 event1
crw-rw----  1 root input 13, 74 Oct 10 07:51 event10
crw-rw----  1 root input 13, 75 Oct 10 07:51 event11
crw-rw----  1 root input 13, 76 Oct 10 07:51 event12
crw-rw----  1 root input 13, 77 Oct 10 07:51 event13
crw-rw----  1 root input 13, 78 Oct 10 07:51 event14
crw-rw----  1 root input 13, 79 Oct 10 07:51 event15
crw-rw----  1 root input 13, 80 Oct 10 07:51 event16
crw-rw----  1 root input 13, 81 Oct 10 07:51 event17
crw-rw----  1 root input 13, 66 Oct 10 07:51 event2
crw-rw----  1 root input 13, 67 Oct 10 07:51 event3
crw-rw----  1 root input 13, 68 Oct 10 07:51 event4
crw-rw----  1 root input 13, 69 Oct 10 07:51 event5
crw-rw----  1 root input 13, 70 Oct 10 07:51 event6
crw-rw----  1 root input 13, 71 Oct 10 07:51 event7
crw-rw----  1 root input 13, 72 Oct 10 07:51 event8
crw-rw----  1 root input 13, 73 Oct 10 07:51 event9
crw-rw-r--  1 root input 13,  0 Oct 10 07:51 js0
crw-rw-r--  1 root input 13,  1 Oct 10 07:51 js1
crw-rw----  1 root input 13, 63 Oct 10 07:51 mice
crw-rw----  1 root input 13, 32 Oct 10 07:51 mouse0

[crocket]

The concept behind this pull request should work, but it doesn't detect presence of (e)logind.

[glitsj16]

LGTM, but I don't have access to either gamepad or nvidia GPU to properly test...

[netblue30]

Merged, thanks!

[kmk3]

Well, this does not seem to work:

$ groups | grep audio >/dev/null && echo works
works
$ firejail --quiet --noprofile --nogroups groups | grep audio >/dev/null && echo works
$

Sorry, I should have opened this as a draft.

I suspect that it fails because of this:

src/firejail/util.c:

// drop privileges
// - for root group or if nogroups is set, supplementary groups are not configured
void drop_privs(int nogroups) {
	gid_t gid = getgid();
	if (arg_debug)
		printf("Drop privileges: pid %d, uid %d, gid %d, nogroups %d\n",  getpid(), getuid(), gid, nogroups);

	// configure supplementary groups
	EUID_ROOT();
	if (gid == 0 || nogroups) {
		if (setgroups(0, NULL) < 0)
			errExit("setgroups");
		if (arg_debug)
			printf("No supplementary groups\n");
	}
	else if (arg_noroot)
		clean_supplementary_groups(gid);

	// set uid/gid
	if (setresgid(-1, getgid(), getgid()) != 0)
		errExit("setresgid");
	if (setresuid(-1, getuid(), getuid()) != 0)
		errExit("setresuid");
}

Anyway, I'm working on it.

Note: If nogroups still works the same way, removing the nvidia workaround
means that this PR probably broke nvidia again. If I can't fix nogroups, I
suppose that I'll just revert that commit.

[kmk3]

@crocket commented on Nov 30:

The concept behind this pull request should work, but it doesn't detect
presence of (e)logind.

Good catch, that should probably be taken into consideration. Now that you
mention it, I remember writing something like if (!arg_nogroups || !have_seat_acls) a while back, but I forgot about it (and didn't immediately
find it on any related branch) when making this PR.

Relevant comment:

@topimiettinen commented on Oct 15:

if !arg_nosound:
    drop_group("audio")
if !arg_novideo:
    drop_group("video")
if !arg_nodvd:
    drop_group("cdrom")
if !arg_noinput:
    drop_group("input")
if !arg_no3d: // /dev/dri/card0 has group video
    drop_group("render")
if !arg_noprinters:
    drop_group("lp")

This could work well if also coupled with the logind check, so users on
distros with logind would not get extra permissions but others would get at
least some groups automatically when needed.

[rusty-snake]

IMHO nogroups should always mean no-groups. If we want selective nogroups, we should make this explicit, e.g. groups restricted.

[kmk3]

@rusty-snake commented on Nov 30:

IMHO nogroups should always mean no-groups. If we want selective
nogroups, we should make this explicit, e.g. groups restricted.

With #4732, the supplementary groups should always be dropped when (e)logind is
running (and nvidia is not used), so nothing should really change on such
systems compared to the behavior from before this PR.

I added the conditional group keeping because that should fix the audio/input
issues on seatd (and potentially also fix group-related issues on nvidia, if
any) without requiring a new command or changes to profiles (and because there
already was a nogroups workaround for nvidia anyway).

[rusty-snake]

Ahh Ok thanks. Then I'm fine with the change (if RELNOTEs/manpage document it).