Fix keeping certain groups with nogroups

This amends commit b828a90 ("Keep audio and video groups regardless of
nogroups", 2021-11-28) from PR #4725.

The commit above did not change the behavior (the groups are still not
kept).  With this commit, it appears to work properly:

    $ groups | grep audio >/dev/null && echo kept
    kept
    # with check_can_drop_all_groups == 0
    $ firejail --quiet --noprofile --nogroups groups |
      grep audio >/dev/null && echo kept
    kept
    # with check_can_drop_all_groups == 1
    $ firejail --quiet --noprofile --nogroups groups |
      grep audio >/dev/null && echo kept
    $

Add a new check_can_drop_all_groups function to check whether the
supplementary groups can be safely dropped without potentially causing
issues with audio, 3D hardware acceleration or input (and maybe more).
It returns false if nvidia (and no `no3d`) is used or if (e)logind is
not running, as in either case the supplementary groups might be needed.

Note: With this, the behavior from before #4725 is restored on (e)logind
systems (when not using nvidia), as it makes the supplementary groups
always be dropped on such systems.

Note2: Even with the static variable, these checks still happen at least
twice.  It seems that it happens once per translation unit (and I think
that it may happen more times if there are multiple processes involved).

This also amends (/kind of reverts) commit 6ddedeb ("Make nogroups
work on nvidia again", 2021-11-29) from PR #4725, as it restores the
nvidia check from it into the new check_can_drop_all_groups function.

src/firejail/firejail.h

@@ -506,6 +506,7 @@ void errLogExit(char* fmt, ...) __attribute__((noreturn));
 void fwarning(char* fmt, ...);
 void fmessage(char* fmt, ...);
 long long unsigned parse_arg_size(char *str);
+int check_can_drop_all_groups();
 void drop_privs(int force_nogroups);
 int mkpath_as_root(const char* path);
 void extract_command_name(int index, char **argv);

src/firejail/main.c

@@ -3155,62 +3155,64 @@ int main(int argc, char **argv, char **envp) {
  ptr += strlen(ptr);
 
  gid_t g;
- // add audio group
- if (!arg_nosound) {
- g = get_group_id("audio");
- if (g) {
- sprintf(ptr, "%d %d 1\n", g, g);
- ptr += strlen(ptr);
+ if (!arg_nogroups || !check_can_drop_all_groups()) {
+ // add audio group
+ if (!arg_nosound) {
+ g = get_group_id("audio");
+ if (g) {
+ sprintf(ptr, "%d %d 1\n", g, g);
+ ptr += strlen(ptr);
+ }
  }
- }
 
- // add video group
- if (!arg_novideo) {
- g = get_group_id("video");
- if (g) {
- sprintf(ptr, "%d %d 1\n", g, g);
- ptr += strlen(ptr);
+ // add video group
+ if (!arg_novideo) {
+ g = get_group_id("video");
+ if (g) {
+ sprintf(ptr, "%d %d 1\n", g, g);
+ ptr += strlen(ptr);
+ }
  }
- }
 
- // add render group
- if (!arg_no3d) {
- g = get_group_id("render");
- if (g) {
- sprintf(ptr, "%d %d 1\n", g, g);
- ptr += strlen(ptr);
+ // add render group
+ if (!arg_no3d) {
+ g = get_group_id("render");
+ if (g) {
+ sprintf(ptr, "%d %d 1\n", g, g);
+ ptr += strlen(ptr);
+ }
  }
- }
 
- // add lp group
- if (!arg_noprinters) {
- g = get_group_id("lp");
- if (g) {
- sprintf(ptr, "%d %d 1\n", g, g);
- ptr += strlen(ptr);
+ // add lp group
+ if (!arg_noprinters) {
+ g = get_group_id("lp");
+ if (g) {
+ sprintf(ptr, "%d %d 1\n", g, g);
+ ptr += strlen(ptr);
+ }
  }
- }
 
- // add cdrom/optical groups
- if (!arg_nodvd) {
- g = get_group_id("cdrom");
- if (g) {
- sprintf(ptr, "%d %d 1\n", g, g);
- ptr += strlen(ptr);
- }
- g = get_group_id("optical");
- if (g) {
- sprintf(ptr, "%d %d 1\n", g, g);
- ptr += strlen(ptr);
+ // add cdrom/optical groups
+ if (!arg_nodvd) {
+ g = get_group_id("cdrom");
+ if (g) {
+ sprintf(ptr, "%d %d 1\n", g, g);
+ ptr += strlen(ptr);
+ }
+ g = get_group_id("optical");
+ if (g) {
+ sprintf(ptr, "%d %d 1\n", g, g);
+ ptr += strlen(ptr);
+ }
  }
- }
 
- // add input group
- if (!arg_noinput) {
- g = get_group_id("input");
- if (g) {
- sprintf(ptr, "%d %d 1\n", g, g);
- ptr += strlen(ptr);
+ // add input group
+ if (!arg_noinput) {
+ g = get_group_id("input");
+ if (g) {
+ sprintf(ptr, "%d %d 1\n", g, g);
+ ptr += strlen(ptr);
+ }
  }
  }
 

src/firejail/sandbox.c

@@ -1225,7 +1225,7 @@ int sandbox(void* sandbox_arg) {
  //****************************************
  // drop privileges
  //****************************************
- drop_privs(arg_nogroups);
+ drop_privs(0);
 
  // kill the sandbox in case the parent died
  prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0);

src/firejail/util.c

@@ -103,6 +103,41 @@ void errLogExit(char* fmt, ...) {
  exit(1);
 }
 
+// Returns whether all supplementary groups can be safely dropped
+int check_can_drop_all_groups() {
+ static int can_drop_all_groups = -1;
+
+ // Avoid needlessly checking (and printing) things twice
+ if (can_drop_all_groups != -1)
+ goto out;
+
+ // nvidia cards require video group; ignore nogroups
+ if (access("/dev/nvidiactl", R_OK) == 0 && arg_no3d == 0) {
+ fwarning("NVIDIA card detected, nogroups command ignored\n");
+ can_drop_all_groups = 0;
+ goto out;
+ }
+
+ /* When we are not sure that the system has working seat-based ACLs
+ * (e.g.: probably yes on (e)udev + (e)logind, probably not on eudev +
+ * seatd), supplementary groups (e.g.: audio and input) might be needed
+ * to avoid breakage (e.g.: audio or gamepads not working). See #4600
+ * and #4603.
+ */
+ if (access("/run/systemd/seats/", F_OK) != 0) {
+ fwarning("logind not detected, nogroups command ignored\n");
+ can_drop_all_groups = 0;
+ goto out;
+ }
+
+ if (arg_debug)
+ fprintf(stderr, "nogroups command not ignored\n");
+ can_drop_all_groups = 1;
+
+out:
+ return can_drop_all_groups;
+}
+
 static int find_group(gid_t group, const gid_t *groups, int ngroups) {
  int i;
  for (i = 0; i < ngroups; i++) {
@@ -141,6 +176,9 @@ static void clean_supplementary_groups(gid_t gid) {
  if (rv == -1)
  goto clean_all;
 
+ if (arg_nogroups && check_can_drop_all_groups())
+ goto clean_all;
+
  // clean supplementary group list
  gid_t new_groups[MAX_GROUPS];
  int new_ngroups = 0;
@@ -230,7 +268,7 @@ void drop_privs(int force_nogroups) {
  if (arg_debug)
  printf("No supplementary groups\n");
  }
- else if (arg_noroot)
+ else if (arg_noroot || arg_nogroups)
  clean_supplementary_groups(gid);
 
  // set uid/gid