aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLibravatar Kelvin M. Klann <kmk3.code@protonmail.com>2021-11-30 19:51:00 -0300
committerLibravatar Kelvin M. Klann <kmk3.code@protonmail.com>2021-12-07 14:52:45 -0300
commit7abce0b4c2891f68751cc18263709e90d48e097d (patch)
tree07e52ab8780ab0d7c0f8a72fa63190a19531dc2b
parentFix duplicated fwarning warnings (diff)
downloadfirejail-7abce0b4c2891f68751cc18263709e90d48e097d.tar.gz
firejail-7abce0b4c2891f68751cc18263709e90d48e097d.tar.zst
firejail-7abce0b4c2891f68751cc18263709e90d48e097d.zip
Fix keeping certain groups with nogroups
This amends commit b828a9047 ("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 6ddedeba0 ("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.
-rw-r--r--src/firejail/firejail.h1
-rw-r--r--src/firejail/main.c94
-rw-r--r--src/firejail/sandbox.c2
-rw-r--r--src/firejail/util.c40
4 files changed, 89 insertions, 48 deletions
diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h
index bbc496afc..c04a644f4 100644
--- a/src/firejail/firejail.h
+++ b/src/firejail/firejail.h
@@ -506,6 +506,7 @@ void errLogExit(char* fmt, ...) __attribute__((noreturn));
506void fwarning(char* fmt, ...); 506void fwarning(char* fmt, ...);
507void fmessage(char* fmt, ...); 507void fmessage(char* fmt, ...);
508long long unsigned parse_arg_size(char *str); 508long long unsigned parse_arg_size(char *str);
509int check_can_drop_all_groups();
509void drop_privs(int force_nogroups); 510void drop_privs(int force_nogroups);
510int mkpath_as_root(const char* path); 511int mkpath_as_root(const char* path);
511void extract_command_name(int index, char **argv); 512void extract_command_name(int index, char **argv);
diff --git a/src/firejail/main.c b/src/firejail/main.c
index e8815d5cd..0262db608 100644
--- a/src/firejail/main.c
+++ b/src/firejail/main.c
@@ -3155,62 +3155,64 @@ int main(int argc, char **argv, char **envp) {
3155 ptr += strlen(ptr); 3155 ptr += strlen(ptr);
3156 3156
3157 gid_t g; 3157 gid_t g;
3158 // add audio group 3158 if (!arg_nogroups || !check_can_drop_all_groups()) {
3159 if (!arg_nosound) { 3159 // add audio group
3160 g = get_group_id("audio"); 3160 if (!arg_nosound) {
3161 if (g) { 3161 g = get_group_id("audio");
3162 sprintf(ptr, "%d %d 1\n", g, g); 3162 if (g) {
3163 ptr += strlen(ptr); 3163 sprintf(ptr, "%d %d 1\n", g, g);
3164 ptr += strlen(ptr);
3165 }
3164 } 3166 }
3165 }
3166 3167
3167 // add video group 3168 // add video group
3168 if (!arg_novideo) { 3169 if (!arg_novideo) {
3169 g = get_group_id("video"); 3170 g = get_group_id("video");
3170 if (g) { 3171 if (g) {
3171 sprintf(ptr, "%d %d 1\n", g, g); 3172 sprintf(ptr, "%d %d 1\n", g, g);
3172 ptr += strlen(ptr); 3173 ptr += strlen(ptr);
3174 }
3173 } 3175 }
3174 }
3175 3176
3176 // add render group 3177 // add render group
3177 if (!arg_no3d) { 3178 if (!arg_no3d) {
3178 g = get_group_id("render"); 3179 g = get_group_id("render");
3179 if (g) { 3180 if (g) {
3180 sprintf(ptr, "%d %d 1\n", g, g); 3181 sprintf(ptr, "%d %d 1\n", g, g);
3181 ptr += strlen(ptr); 3182 ptr += strlen(ptr);
3183 }
3182 } 3184 }
3183 }
3184 3185
3185 // add lp group 3186 // add lp group
3186 if (!arg_noprinters) { 3187 if (!arg_noprinters) {
3187 g = get_group_id("lp"); 3188 g = get_group_id("lp");
3188 if (g) { 3189 if (g) {
3189 sprintf(ptr, "%d %d 1\n", g, g); 3190 sprintf(ptr, "%d %d 1\n", g, g);
3190 ptr += strlen(ptr); 3191 ptr += strlen(ptr);
3192 }
3191 } 3193 }
3192 }
3193 3194
3194 // add cdrom/optical groups 3195 // add cdrom/optical groups
3195 if (!arg_nodvd) { 3196 if (!arg_nodvd) {
3196 g = get_group_id("cdrom"); 3197 g = get_group_id("cdrom");
3197 if (g) { 3198 if (g) {
3198 sprintf(ptr, "%d %d 1\n", g, g); 3199 sprintf(ptr, "%d %d 1\n", g, g);
3199 ptr += strlen(ptr); 3200 ptr += strlen(ptr);
3200 } 3201 }
3201 g = get_group_id("optical"); 3202 g = get_group_id("optical");
3202 if (g) { 3203 if (g) {
3203 sprintf(ptr, "%d %d 1\n", g, g); 3204 sprintf(ptr, "%d %d 1\n", g, g);
3204 ptr += strlen(ptr); 3205 ptr += strlen(ptr);
3206 }
3205 } 3207 }
3206 }
3207 3208
3208 // add input group 3209 // add input group
3209 if (!arg_noinput) { 3210 if (!arg_noinput) {
3210 g = get_group_id("input"); 3211 g = get_group_id("input");
3211 if (g) { 3212 if (g) {
3212 sprintf(ptr, "%d %d 1\n", g, g); 3213 sprintf(ptr, "%d %d 1\n", g, g);
3213 ptr += strlen(ptr); 3214 ptr += strlen(ptr);
3215 }
3214 } 3216 }
3215 } 3217 }
3216 3218
diff --git a/src/firejail/sandbox.c b/src/firejail/sandbox.c
index 3887b5701..96fa4c81a 100644
--- a/src/firejail/sandbox.c
+++ b/src/firejail/sandbox.c
@@ -1225,7 +1225,7 @@ int sandbox(void* sandbox_arg) {
1225 //**************************************** 1225 //****************************************
1226 // drop privileges 1226 // drop privileges
1227 //**************************************** 1227 //****************************************
1228 drop_privs(arg_nogroups); 1228 drop_privs(0);
1229 1229
1230 // kill the sandbox in case the parent died 1230 // kill the sandbox in case the parent died
1231 prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0); 1231 prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0);
diff --git a/src/firejail/util.c b/src/firejail/util.c
index 55df44414..c1c31b43c 100644
--- a/src/firejail/util.c
+++ b/src/firejail/util.c
@@ -103,6 +103,41 @@ void errLogExit(char* fmt, ...) {
103 exit(1); 103 exit(1);
104} 104}
105 105
106// Returns whether all supplementary groups can be safely dropped
107int check_can_drop_all_groups() {
108 static int can_drop_all_groups = -1;
109
110 // Avoid needlessly checking (and printing) things twice
111 if (can_drop_all_groups != -1)
112 goto out;
113
114 // nvidia cards require video group; ignore nogroups
115 if (access("/dev/nvidiactl", R_OK) == 0 && arg_no3d == 0) {
116 fwarning("NVIDIA card detected, nogroups command ignored\n");
117 can_drop_all_groups = 0;
118 goto out;
119 }
120
121 /* When we are not sure that the system has working seat-based ACLs
122 * (e.g.: probably yes on (e)udev + (e)logind, probably not on eudev +
123 * seatd), supplementary groups (e.g.: audio and input) might be needed
124 * to avoid breakage (e.g.: audio or gamepads not working). See #4600
125 * and #4603.
126 */
127 if (access("/run/systemd/seats/", F_OK) != 0) {
128 fwarning("logind not detected, nogroups command ignored\n");
129 can_drop_all_groups = 0;
130 goto out;
131 }
132
133 if (arg_debug)
134 fprintf(stderr, "nogroups command not ignored\n");
135 can_drop_all_groups = 1;
136
137out:
138 return can_drop_all_groups;
139}
140
106static int find_group(gid_t group, const gid_t *groups, int ngroups) { 141static int find_group(gid_t group, const gid_t *groups, int ngroups) {
107 int i; 142 int i;
108 for (i = 0; i < ngroups; i++) { 143 for (i = 0; i < ngroups; i++) {
@@ -141,6 +176,9 @@ static void clean_supplementary_groups(gid_t gid) {
141 if (rv == -1) 176 if (rv == -1)
142 goto clean_all; 177 goto clean_all;
143 178
179 if (arg_nogroups && check_can_drop_all_groups())
180 goto clean_all;
181
144 // clean supplementary group list 182 // clean supplementary group list
145 gid_t new_groups[MAX_GROUPS]; 183 gid_t new_groups[MAX_GROUPS];
146 int new_ngroups = 0; 184 int new_ngroups = 0;
@@ -230,7 +268,7 @@ void drop_privs(int force_nogroups) {
230 if (arg_debug) 268 if (arg_debug)
231 printf("No supplementary groups\n"); 269 printf("No supplementary groups\n");
232 } 270 }
233 else if (arg_noroot) 271 else if (arg_noroot || arg_nogroups)
234 clean_supplementary_groups(gid); 272 clean_supplementary_groups(gid);
235 273
236 // set uid/gid 274 // set uid/gid