From c321020a559a2640799c4144ade6b4e69140e065 Mon Sep 17 00:00:00 2001 From: Aleksey Manevich Date: Wed, 24 Aug 2016 13:14:44 +0300 Subject: tighten secutiry --- src/firejail/fs.c | 120 +++++++++++++--------------------------------- src/firejail/fs_home.c | 25 ++++------ src/firejail/ls.c | 6 +-- src/firejail/pulseaudio.c | 2 +- src/firejail/util.c | 7 ++- src/include/euid_common.h | 6 ++- 6 files changed, 55 insertions(+), 111 deletions(-) diff --git a/src/firejail/fs.c b/src/firejail/fs.c index 6c87df1e9..21ab56bd8 100644 --- a/src/firejail/fs.c +++ b/src/firejail/fs.c @@ -29,16 +29,25 @@ static void fs_rdwr(const char *dir); +static void create_dir_as_root(const char *dir, mode_t mode) { + assert(dir); + if (arg_debug) + printf("Creating %s directory\n", dir); + + if (mkdir(dir, mode) == -1) + errExit("mkdir"); + + ASSERT_PERMS(dir, 0, 0, mode); +} + static void create_empty_dir(void) { struct stat s; if (stat(RUN_RO_DIR, &s)) { /* coverity[toctou] */ - int rv = mkdir(RUN_RO_DIR, S_IRUSR | S_IXUSR); - if (rv == -1) - errExit("mkdir"); - if (chown(RUN_RO_DIR, 0, 0) < 0) - errExit("chown"); + if (mkdir(RUN_RO_DIR, S_IRUSR | S_IXUSR) == -1) + errExit("mkdir"); + ASSERT_PERMS(RUN_RO_DIR, 0, 0, S_IRUSR | S_IXUSR); } } @@ -50,11 +59,16 @@ static void create_empty_file(void) { FILE *fp = fopen(RUN_RO_FILE, "w"); if (!fp) errExit("fopen"); - fclose(fp); - if (chown(RUN_RO_FILE, 0, 0) < 0) + + int fd = fileno(fp); + if (fd == -1) + errExit("fileno"); + if (fchown(fd, 0, 0) < 0) errExit("chown"); - if (chmod(RUN_RO_FILE, S_IRUSR) < 0) + if (fchmod(fd, S_IRUSR) < 0) errExit("chown"); + + fclose(fp); } } @@ -64,16 +78,7 @@ void fs_build_firejail_dir(void) { // CentOS 6 doesn't have /run directory if (stat(RUN_FIREJAIL_BASEDIR, &s)) { - if (arg_debug) - printf("Creating %s directory\n", RUN_FIREJAIL_BASEDIR); - /* coverity[toctou] */ - int rv = mkdir(RUN_FIREJAIL_BASEDIR, 0755); - if (rv == -1) - errExit("mkdir"); - if (chown(RUN_FIREJAIL_BASEDIR, 0, 0) < 0) - errExit("chown"); - if (chmod(RUN_FIREJAIL_BASEDIR, 0755) < 0) - errExit("chmod"); + create_dir_as_root(RUN_FIREJAIL_BASEDIR, 0755); } else { // check /tmp/firejail directory belongs to root end exit if doesn't! if (s.st_uid != 0 || s.st_gid != 0) { @@ -83,61 +88,23 @@ void fs_build_firejail_dir(void) { } if (stat(RUN_FIREJAIL_DIR, &s)) { - if (arg_debug) - printf("Creating %s directory\n", RUN_FIREJAIL_DIR); - /* coverity[toctou] */ - int rv = mkdir(RUN_FIREJAIL_DIR, 0755); - if (rv == -1) - errExit("mkdir"); - if (chown(RUN_FIREJAIL_DIR, 0, 0) < 0) - errExit("chown"); - if (chmod(RUN_FIREJAIL_DIR, 0755) < 0) - errExit("chmod"); + create_dir_as_root(RUN_FIREJAIL_DIR, 0755); } if (stat(RUN_FIREJAIL_NETWORK_DIR, &s)) { - if (arg_debug) - printf("Creating %s directory\n", RUN_FIREJAIL_NETWORK_DIR); - - if (mkdir(RUN_FIREJAIL_NETWORK_DIR, 0755) == -1) - errExit("mkdir"); - if (chown(RUN_FIREJAIL_NETWORK_DIR, 0, 0) < 0) - errExit("chown"); - if (chmod(RUN_FIREJAIL_NETWORK_DIR, 0755) < 0) - errExit("chmod"); + create_dir_as_root(RUN_FIREJAIL_NETWORK_DIR, 0755); } if (stat(RUN_FIREJAIL_BANDWIDTH_DIR, &s)) { - if (arg_debug) - printf("Creating %s directory\n", RUN_FIREJAIL_BANDWIDTH_DIR); - if (mkdir(RUN_FIREJAIL_BANDWIDTH_DIR, 0755) == -1) - errExit("mkdir"); - if (chown(RUN_FIREJAIL_BANDWIDTH_DIR, 0, 0) < 0) - errExit("chown"); - if (chmod(RUN_FIREJAIL_BANDWIDTH_DIR, 0755) < 0) - errExit("chmod"); + create_dir_as_root(RUN_FIREJAIL_BANDWIDTH_DIR, 0755); } if (stat(RUN_FIREJAIL_NAME_DIR, &s)) { - if (arg_debug) - printf("Creating %s directory\n", RUN_FIREJAIL_NAME_DIR); - if (mkdir(RUN_FIREJAIL_NAME_DIR, 0755) == -1) - errExit("mkdir"); - if (chown(RUN_FIREJAIL_NAME_DIR, 0, 0) < 0) - errExit("chown"); - if (chmod(RUN_FIREJAIL_NAME_DIR, 0755) < 0) - errExit("chmod"); + create_dir_as_root(RUN_FIREJAIL_NAME_DIR, 0755); } if (stat(RUN_FIREJAIL_X11_DIR, &s)) { - if (arg_debug) - printf("Creating %s directory\n", RUN_FIREJAIL_X11_DIR); - if (mkdir(RUN_FIREJAIL_X11_DIR, 0755) == -1) - errExit("mkdir"); - if (chown(RUN_FIREJAIL_X11_DIR, 0, 0) < 0) - errExit("chown"); - if (chmod(RUN_FIREJAIL_X11_DIR, 0755) < 0) - errExit("chmod"); + create_dir_as_root(RUN_FIREJAIL_X11_DIR, 0755); } create_empty_dir(); @@ -160,16 +127,7 @@ void fs_build_mnt_dir(void) { // create /run/firejail/mnt directory if (stat(RUN_MNT_DIR, &s)) { - if (arg_debug) - printf("Creating %s directory\n", RUN_MNT_DIR); - /* coverity[toctou] */ - int rv = mkdir(RUN_MNT_DIR, 0755); - if (rv == -1) - errExit("mkdir"); - if (chown(RUN_MNT_DIR, 0, 0) < 0) - errExit("chown"); - if (chmod(RUN_MNT_DIR, 0755) < 0) - errExit("chmod"); + create_dir_as_root(RUN_MNT_DIR, 0755); } // ... and mount tmpfs on top of it @@ -202,16 +160,12 @@ void fs_build_cp_command(void) { fprintf(stderr, "Error: invalid /bin/cp file\n"); exit(1); } - int rv = copy_file(fname, RUN_CP_COMMAND); + int rv = copy_file(fname, RUN_CP_COMMAND, 0, 0, 0755); if (rv) { fprintf(stderr, "Error: cannot access /bin/cp\n"); exit(1); } - /* coverity[toctou] */ - if (chown(RUN_CP_COMMAND, 0, 0)) - errExit("chown"); - if (chmod(RUN_CP_COMMAND, 0755)) - errExit("chmod"); + ASSERT_PERMS(RUN_CP_COMMAND, 0, 0, 0755); free(fname); } @@ -827,10 +781,7 @@ char *fs_check_overlay_dir(const char *subdirname, int allow_reuse) { /* coverity[toctou] */ if (mkdir(dirname, 0700)) errExit("mkdir"); - if (chown(dirname, getuid(), getgid()) < 0) - errExit("chown"); - if (chmod(dirname, 0700) < 0) - errExit("chmod"); + ASSERT_PERMS(dirname, getuid(), getgid(), 0700); } else if (is_link(dirname)) { fprintf(stderr, "Error: invalid ~/.firejail directory\n"); @@ -917,10 +868,7 @@ void fs_overlayfs(void) { errExit("asprintf"); if (mkdir(oroot, 0755)) errExit("mkdir"); - if (chown(oroot, 0, 0) < 0) - errExit("chown"); - if (chmod(oroot, 0755) < 0) - errExit("chmod"); + ASSERT_PERMS(oroot, 0, 0, 0755); struct stat s; char *basedir = RUN_MNT_DIR; @@ -1259,7 +1207,7 @@ void fs_chroot(const char *rootdir) { fprintf(stderr, "Error: invalid %s file\n", fname); exit(1); } - if (copy_file("/etc/resolv.conf", fname) == -1) + if (copy_file("/etc/resolv.conf", fname, 0, 0, 0644) == -1) fprintf(stderr, "Warning: /etc/resolv.conf not initialized\n"); } diff --git a/src/firejail/fs_home.c b/src/firejail/fs_home.c index d328d5f1c..75d69e021 100644 --- a/src/firejail/fs_home.c +++ b/src/firejail/fs_home.c @@ -43,9 +43,7 @@ static void skel(const char *homedir, uid_t u, gid_t g) { if (stat(fname, &s) == 0) return; if (stat("/etc/skel/.zshrc", &s) == 0) { - if (copy_file("/etc/skel/.zshrc", fname) == 0) { - if (chown(fname, u, g) == -1) - errExit("chown"); + if (copy_file("/etc/skel/.zshrc", fname, u, g, 0644) == 0) { fs_logger("clone /etc/skel/.zshrc"); } } @@ -73,9 +71,7 @@ static void skel(const char *homedir, uid_t u, gid_t g) { if (stat(fname, &s) == 0) return; if (stat("/etc/skel/.cshrc", &s) == 0) { - if (copy_file("/etc/skel/.cshrc", fname) == 0) { - if (chown(fname, u, g) == -1) - errExit("chown"); + if (copy_file("/etc/skel/.cshrc", fname, u, g, 0644) == 0) { fs_logger("clone /etc/skel/.cshrc"); } } @@ -104,10 +100,7 @@ static void skel(const char *homedir, uid_t u, gid_t g) { if (stat(fname, &s) == 0) return; if (stat("/etc/skel/.bashrc", &s) == 0) { - if (copy_file("/etc/skel/.bashrc", fname) == 0) { - /* coverity[toctou] */ - if (chown(fname, u, g) == -1) - errExit("chown"); + if (copy_file("/etc/skel/.bashrc", fname, u, g, 0644) == 0) { fs_logger("clone /etc/skel/.bashrc"); } } @@ -131,7 +124,7 @@ static int store_xauthority(void) { exit(1); } - int rv = copy_file(src, dest); + int rv = copy_file(src, dest, -1, -1, 0600); if (rv) { fprintf(stderr, "Warning: cannot transfer .Xauthority in private home directory\n"); return 0; @@ -167,7 +160,7 @@ static int store_asoundrc(void) { free(rp); } - int rv = copy_file(src, dest); + int rv = copy_file(src, dest, -1, -1, -0644); if (rv) { fprintf(stderr, "Warning: cannot transfer .asoundrc in private home directory\n"); return 0; @@ -184,7 +177,7 @@ static void copy_xauthority(void) { char *dest; if (asprintf(&dest, "%s/.Xauthority", cfg.homedir) == -1) errExit("asprintf"); - int rv = copy_file(src, dest); + int rv = copy_file(src, dest, -1, -1, 0600); if (rv) fprintf(stderr, "Warning: cannot transfer .Xauthority in private home directory\n"); else { @@ -207,7 +200,7 @@ static void copy_asoundrc(void) { char *dest; if (asprintf(&dest, "%s/.asoundrc", cfg.homedir) == -1) errExit("asprintf"); - int rv = copy_file(src, dest); + int rv = copy_file(src, dest, -1 , -1, 0644); if (rv) fprintf(stderr, "Warning: cannot transfer .asoundrc in private home directory\n"); else { @@ -360,11 +353,9 @@ int fs_copydir(const char *path, const struct stat *st, int ftype, struct FTW *s return(0); if (stat(path, &s) == 0) { if(ftype == FTW_F) { - if (copy_file(path, dest) == 0) { + if (copy_file(path, dest, u, g, 0644) == 0) { if (arg_debug) printf("copy from %s to %s\n", path, dest); - if (chown(dest, u, g) == -1) - errExit("chown"); fs_logger2("clone", path); } } diff --git a/src/firejail/ls.c b/src/firejail/ls.c index 09577fb0c..495aaf8e2 100644 --- a/src/firejail/ls.c +++ b/src/firejail/ls.c @@ -374,11 +374,7 @@ void sandboxfs(int op, pid_t pid, const char *path) { } // copy file EUID_ROOT(); - copy_file(src_fname, dest_fname); - if (chown(dest_fname, getuid(), getgid()) == -1) - errExit("chown"); - if (chmod(dest_fname, 0644) == -1) - errExit("chmod"); + copy_file(src_fname, dest_fname, getuid(), getgid(), 0644); printf("Transfer complete\n"); EUID_USER(); } diff --git a/src/firejail/pulseaudio.c b/src/firejail/pulseaudio.c index 908ef1d25..dd26d219c 100644 --- a/src/firejail/pulseaudio.c +++ b/src/firejail/pulseaudio.c @@ -114,7 +114,7 @@ void pulseaudio_init(void) { char *pulsecfg = NULL; if (asprintf(&pulsecfg, "%s/client.conf", RUN_PULSE_DIR) == -1) errExit("asprintf"); - if (copy_file("/etc/pulse/client.conf", pulsecfg)) + if (copy_file("/etc/pulse/client.conf", pulsecfg, -1, -1, 0644)) errExit("copy_file"); FILE *fp = fopen(pulsecfg, "a+"); if (!fp) diff --git a/src/firejail/util.c b/src/firejail/util.c index 24bb71e4c..22434e200 100644 --- a/src/firejail/util.c +++ b/src/firejail/util.c @@ -170,7 +170,7 @@ void logerr(const char *msg) { // return -1 if error, 0 if no error -int copy_file(const char *srcname, const char *destname) { +int copy_file(const char *srcname, const char *destname, uid_t uid, gid_t gid, mode_t mode) { assert(srcname); assert(destname); @@ -207,6 +207,11 @@ int copy_file(const char *srcname, const char *destname) { } } + if (fchown(dst, uid, gid) == -1) + errExit("fchown"); + if (fchmod(dst, mode) == -1) + errExit("fchmod"); + close(src); close(dst); return 0; diff --git a/src/include/euid_common.h b/src/include/euid_common.h index b6d341bf4..de5572fb1 100644 --- a/src/include/euid_common.h +++ b/src/include/euid_common.h @@ -37,11 +37,15 @@ extern uid_t firejail_uid; static inline void EUID_ROOT(void) { if (seteuid(0) == -1) fprintf(stderr, "Warning: cannot switch euid to root\n"); + if (setegid(0) == -1) + fprintf(stderr, "Warning: cannot switch egid to root\n"); } static inline void EUID_USER(void) { if (seteuid(firejail_uid) == -1) - fprintf(stderr, "Warning: cannot switch euid to user\n"); + errExit("seteuid"); + if (setegid(firejail_uid) == -1) + errExit("setegid"); } static inline void EUID_PRINT(void) { -- cgit v1.2.3-70-g09d2