diff options
author | smitsohu <smitsohu@gmail.com> | 2018-08-16 13:25:05 +0200 |
---|---|---|
committer | smitsohu <smitsohu@gmail.com> | 2018-08-16 13:25:05 +0200 |
commit | a924230110f868915fd2d81e22a1f3bfaaab5c2c (patch) | |
tree | 6fe497fe384f855909dc52d23c68c329e097da6f | |
parent | tests: skip fs_dev_shm.exp if /dev/shm is not writable (diff) | |
download | firejail-a924230110f868915fd2d81e22a1f3bfaaab5c2c.tar.gz firejail-a924230110f868915fd2d81e22a1f3bfaaab5c2c.tar.zst firejail-a924230110f868915fd2d81e22a1f3bfaaab5c2c.zip |
harden private-home mounting, small improvements
-rw-r--r-- | src/firejail/fs_etc.c | 2 | ||||
-rw-r--r-- | src/firejail/fs_home.c | 71 | ||||
-rw-r--r-- | src/firejail/main.c | 2 |
3 files changed, 37 insertions, 38 deletions
diff --git a/src/firejail/fs_etc.c b/src/firejail/fs_etc.c index bf60b56a7..af22e4c29 100644 --- a/src/firejail/fs_etc.c +++ b/src/firejail/fs_etc.c | |||
@@ -101,7 +101,7 @@ errexit: | |||
101 | static void duplicate(const char *fname, const char *private_dir, const char *private_run_dir) { | 101 | static void duplicate(const char *fname, const char *private_dir, const char *private_run_dir) { |
102 | assert(fname); | 102 | assert(fname); |
103 | 103 | ||
104 | if (*fname == '~' || *fname == '/' || strstr(fname, "..")) { | 104 | if (*fname == '~' || strchr(fname, '/') || strcmp(fname, "..") == 0) { |
105 | fprintf(stderr, "Error: \"%s\" is an invalid filename\n", fname); | 105 | fprintf(stderr, "Error: \"%s\" is an invalid filename\n", fname); |
106 | exit(1); | 106 | exit(1); |
107 | } | 107 | } |
diff --git a/src/firejail/fs_home.c b/src/firejail/fs_home.c index 3b5094ac9..03f3512b4 100644 --- a/src/firejail/fs_home.c +++ b/src/firejail/fs_home.c | |||
@@ -235,8 +235,29 @@ void fs_private_homedir(void) { | |||
235 | // mount bind private_homedir on top of homedir | 235 | // mount bind private_homedir on top of homedir |
236 | if (arg_debug) | 236 | if (arg_debug) |
237 | printf("Mount-bind %s on top of %s\n", private_homedir, homedir); | 237 | printf("Mount-bind %s on top of %s\n", private_homedir, homedir); |
238 | if (mount(private_homedir, homedir, NULL, MS_NOSUID | MS_NODEV | MS_BIND | MS_REC, NULL) < 0) | 238 | // get a file descriptor for private_homedir, fails if there is any symlink |
239 | int fd = safe_fd(private_homedir, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); | ||
240 | if (fd == -1) | ||
241 | errExit("safe_fd"); | ||
242 | // check if new home directory is owned by the user | ||
243 | struct stat s; | ||
244 | if (fstat(fd, &s) == -1) | ||
245 | errExit("fstat"); | ||
246 | if (s.st_uid != getuid()) { | ||
247 | fprintf(stderr, "Error: private directory is not owned by the current user\n"); | ||
248 | exit(1); | ||
249 | } | ||
250 | if ((S_IRWXU & s.st_mode) != S_IRWXU) | ||
251 | fwarning("no full permissions for private directory\n"); | ||
252 | // mount via the link in /proc/self/fd | ||
253 | char *proc; | ||
254 | if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) | ||
255 | errExit("asprintf"); | ||
256 | if (mount(proc, homedir, NULL, MS_NOSUID | MS_NODEV | MS_BIND | MS_REC, NULL) < 0) | ||
239 | errExit("mount bind"); | 257 | errExit("mount bind"); |
258 | free(proc); | ||
259 | close(fd); | ||
260 | |||
240 | fs_logger3("mount-bind", private_homedir, cfg.homedir); | 261 | fs_logger3("mount-bind", private_homedir, cfg.homedir); |
241 | fs_logger2("whitelist", cfg.homedir); | 262 | fs_logger2("whitelist", cfg.homedir); |
242 | // preserve mode and ownership | 263 | // preserve mode and ownership |
@@ -339,31 +360,10 @@ void fs_check_private_dir(void) { | |||
339 | free(tmp); | 360 | free(tmp); |
340 | 361 | ||
341 | if (!cfg.home_private | 362 | if (!cfg.home_private |
342 | || !is_dir(cfg.home_private) | 363 | || !is_dir(cfg.home_private)) { |
343 | || is_link(cfg.home_private) | ||
344 | || strstr(cfg.home_private, "..")) { | ||
345 | fprintf(stderr, "Error: invalid private directory\n"); | 364 | fprintf(stderr, "Error: invalid private directory\n"); |
346 | exit(1); | 365 | exit(1); |
347 | } | 366 | } |
348 | |||
349 | // check home directory and chroot home directory have the same owner | ||
350 | struct stat s2; | ||
351 | int rv = stat(cfg.home_private, &s2); | ||
352 | if (rv < 0) { | ||
353 | fprintf(stderr, "Error: cannot find %s directory\n", cfg.home_private); | ||
354 | exit(1); | ||
355 | } | ||
356 | |||
357 | struct stat s1; | ||
358 | rv = stat(cfg.homedir, &s1); | ||
359 | if (rv < 0) { | ||
360 | fprintf(stderr, "Error: cannot find %s directory, full path name required\n", cfg.homedir); | ||
361 | exit(1); | ||
362 | } | ||
363 | if (s1.st_uid != s2.st_uid) { | ||
364 | printf("Error: --private directory should be owned by the current user\n"); | ||
365 | exit(1); | ||
366 | } | ||
367 | } | 367 | } |
368 | 368 | ||
369 | //*********************************************************************************** | 369 | //*********************************************************************************** |
@@ -400,34 +400,33 @@ static char *check_dir_or_file(const char *name) { | |||
400 | } | 400 | } |
401 | return fname; | 401 | return fname; |
402 | } | 402 | } |
403 | else { | 403 | else // dangling link |
404 | fprintf(stderr, "Error: invalid file %s\n", name); | 404 | goto errexit; |
405 | exit(1); | ||
406 | } | ||
407 | } | 405 | } |
408 | else { | 406 | else { |
409 | // check the file is in user home directory, a full home directory is not allowed | 407 | // check the file is in user home directory, a full home directory is not allowed |
410 | char *rname = realpath(fname, NULL); | 408 | char *rname = realpath(fname, NULL); |
411 | if (!rname || | 409 | if (!rname || |
412 | strncmp(rname, cfg.homedir, strlen(cfg.homedir)) != 0 || | 410 | strncmp(rname, cfg.homedir, strlen(cfg.homedir)) != 0 || |
413 | strcmp(rname, cfg.homedir) == 0) { | 411 | strcmp(rname, cfg.homedir) == 0) |
414 | fprintf(stderr, "Error: invalid file %s\n", name); | 412 | goto errexit; |
415 | exit(1); | ||
416 | } | ||
417 | 413 | ||
418 | // only top files and directories in user home are allowed | 414 | // only top files and directories in user home are allowed |
419 | char *ptr = rname + strlen(cfg.homedir); | 415 | char *ptr = rname + strlen(cfg.homedir); |
420 | assert(*ptr != '\0'); | 416 | if (*ptr != '/') |
417 | goto errexit; | ||
421 | ptr = strchr(++ptr, '/'); | 418 | ptr = strchr(++ptr, '/'); |
422 | if (ptr) { | 419 | if (ptr) { |
423 | if (*ptr != '\0') { | 420 | fprintf(stderr, "Error: only top files and directories in user home are allowed\n"); |
424 | fprintf(stderr, "Error: only top files and directories in user home are allowed\n"); | 421 | exit(1); |
425 | exit(1); | ||
426 | } | ||
427 | } | 422 | } |
428 | free(fname); | 423 | free(fname); |
429 | return rname; | 424 | return rname; |
430 | } | 425 | } |
426 | |||
427 | errexit: | ||
428 | fprintf(stderr, "Error: invalid file %s\n", name); | ||
429 | exit(1); | ||
431 | } | 430 | } |
432 | 431 | ||
433 | static void duplicate(char *name) { | 432 | static void duplicate(char *name) { |
diff --git a/src/firejail/main.c b/src/firejail/main.c index b064155f4..4faef025a 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c | |||
@@ -1633,7 +1633,7 @@ int main(int argc, char **argv) { | |||
1633 | else if (strncmp(argv[i], "--private-srv=", 14) == 0) { | 1633 | else if (strncmp(argv[i], "--private-srv=", 14) == 0) { |
1634 | // extract private srv list | 1634 | // extract private srv list |
1635 | if (*(argv[i] + 14) == '\0') { | 1635 | if (*(argv[i] + 14) == '\0') { |
1636 | fprintf(stderr, "Error: invalid private-etc option\n"); | 1636 | fprintf(stderr, "Error: invalid private-srv option\n"); |
1637 | exit(1); | 1637 | exit(1); |
1638 | } | 1638 | } |
1639 | if (cfg.srv_private_keep) { | 1639 | if (cfg.srv_private_keep) { |