Skip to content

Commit

Permalink
Merge pull request #4732 from kmk3/fix-groups-misc3
Browse files Browse the repository at this point in the history 
Fix keeping certain groups with nogroups
  • Loading branch information
@netblue30
netblue30 authored Dec 8, 2021
2 parents 72c5890 + 7abce0b commit 1afa38f
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 55 deletions.
3 changes: 2 additions & 1 deletion src/firejail/firejail.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
96 changes: 49 additions & 47 deletions src/firejail/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/firejail/profile.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/firejail/sandbox.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
49 changes: 44 additions & 5 deletions src/firejail/util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 1afa38f

Please sign in to comment.