Merge pull request #4732 from kmk3/fix-groups-misc3

Fix keeping certain groups with nogroups

src/firejail/firejail.h

@@ -506,7 +506,8 @@ void errLogExit(char* fmt, ...) __attribute__((noreturn));
 void fwarning(char* fmt, ...);
 void fmessage(char* fmt, ...);
 long long unsigned parse_arg_size(char *str);
-void drop_privs(int nogroups);
+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);
 void logsignal(int s);

src/firejail/main.c

@@ -2642,7 +2642,7 @@ int main(int argc, char **argv, char **envp) {
  else if (cfg.dns4 == NULL)
  cfg.dns4 = dns;
  else {
- fwarning("Warning: up to 4 DNS servers can be specified, %s ignored\n", dns);
+ fwarning("up to 4 DNS servers can be specified, %s ignored\n", dns);
  free(dns);
  }
  }
@@ -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/profile.c

@@ -1106,7 +1106,7 @@ int profile_check_line(char *ptr, int lineno, const char *fname) {
  else if (cfg.dns4 == NULL)
  cfg.dns4 = dns;
  else {
- fwarning("Warning: up to 4 DNS servers can be specified, %s ignored\n", dns);
+ fwarning("up to 4 DNS servers can be specified, %s ignored\n", dns);
  free(dns);
  }
  return 0;

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;
@@ -215,21 +253,22 @@ static void clean_supplementary_groups(gid_t gid) {
 
 
 // drop privileges
-// - for root group or if nogroups is set, supplementary groups are not configured
-void drop_privs(int nogroups) {
+// - for root group or if force_nogroups is set, supplementary groups are not configured
+void drop_privs(int force_nogroups) {
  gid_t gid = getgid();
  if (arg_debug)
- printf("Drop privileges: pid %d, uid %d, gid %d, nogroups %d\n", getpid(), getuid(), gid, nogroups);
+ printf("Drop privileges: pid %d, uid %d, gid %d, force_nogroups %d\n",
+ getpid(), getuid(), gid, force_nogroups);
 
  // configure supplementary groups
  EUID_ROOT();
- if (gid == 0 || nogroups) {
+ if (gid == 0 || force_nogroups) {
  if (setgroups(0, NULL) < 0)
  errExit("setgroups");
  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