From db4c5b0f50e7d572116994fffe19af3967c8853e Mon Sep 17 00:00:00 2001 From: Kristóf Marussy Date: Sat, 7 Mar 2020 14:40:30 +0100 Subject: xdg-dbus-proxy hardening --- src/firejail/dbus.c | 59 ++++++++++++++++++++++++++++++++++++++++++++------ src/firejail/main.c | 4 +--- src/firejail/preproc.c | 12 +++++++++- 3 files changed, 64 insertions(+), 11 deletions(-) diff --git a/src/firejail/dbus.c b/src/firejail/dbus.c index bf1ae9a1e..9efd7bc85 100644 --- a/src/firejail/dbus.c +++ b/src/firejail/dbus.c @@ -22,18 +22,24 @@ #include #include #include +#include #include #include #include +#ifndef O_PATH +#define O_PATH 010000000 +#endif + #define DBUS_SOCKET_PATH_PREFIX "unix:path=" #define DBUS_USER_SOCKET_FORMAT "/run/user/%d/bus" #define DBUS_USER_SOCKET_PATH_FORMAT DBUS_SOCKET_PATH_PREFIX DBUS_USER_SOCKET_FORMAT #define DBUS_SYSTEM_SOCKET "/run/dbus/system_bus_socket" #define DBUS_SYSTEM_SOCKET_PATH DBUS_SOCKET_PATH_PREFIX DBUS_SYSTEM_SOCKET #define DBUS_SESSION_BUS_ADDRESS_ENV "DBUS_SESSION_BUS_ADDRESS" -#define DBUS_USER_PROXY_SOCKET_FORMAT RUN_FIREJAIL_DBUS_DIR "/%d-user" -#define DBUS_SYSTEM_PROXY_SOCKET_FORMAT RUN_FIREJAIL_DBUS_DIR "/%d-system" +#define DBUS_USER_DIR_FORMAT RUN_FIREJAIL_DBUS_DIR "/%d" +#define DBUS_USER_PROXY_SOCKET_FORMAT DBUS_USER_DIR_FORMAT "/%d-user" +#define DBUS_SYSTEM_PROXY_SOCKET_FORMAT DBUS_USER_DIR_FORMAT "/%d-system" #define DBUS_MAX_NAME_LENGTH 255 static pid_t dbus_proxy_pid = 0; @@ -144,7 +150,29 @@ static void write_profile(int fd, char const *prefix) { } } +static void dbus_create_user_dir(void) { + char *path; + if (asprintf(&path, DBUS_USER_DIR_FORMAT, (int) getuid()) == -1) + errExit("asprintf"); + struct stat s; + mode_t mode = 0700; + uid_t uid = getuid(); + gid_t gid = getgid(); + if (stat(path, &s)) { + if (arg_debug) + printf("Creating %s directory for DBus proxy sockets\n", path); + if (mkdir(path, mode) == -1 && errno != EEXIST) + errExit("mkdir"); + if (set_perms(path, uid, gid, mode)) + errExit("set_perms"); + ASSERT_PERMS(path, uid, gid, mode); + } + free(path); +} + void dbus_proxy_start(void) { + dbus_create_user_dir(); + int status_pipe[2]; if (pipe(status_pipe) == -1) errExit("pipe"); @@ -181,7 +209,8 @@ void dbus_proxy_start(void) { } else { write_arg(args_pipe[1], dbus_user_path_env); } - if (asprintf(&dbus_user_proxy_socket, DBUS_USER_PROXY_SOCKET_FORMAT, (int) getpid()) == -1) + if (asprintf(&dbus_user_proxy_socket, DBUS_USER_PROXY_SOCKET_FORMAT, + (int) getuid(), (int) getpid()) == -1) errExit("asprintf"); write_arg(args_pipe[1], dbus_user_proxy_socket); write_arg(args_pipe[1], "--filter"); @@ -190,7 +219,8 @@ void dbus_proxy_start(void) { if (arg_dbus_system == DBUS_POLICY_FILTER) { write_arg(args_pipe[1], DBUS_SYSTEM_SOCKET_PATH); - if (asprintf(&dbus_system_proxy_socket, DBUS_SYSTEM_PROXY_SOCKET_FORMAT, (int) getpid()) == -1) + if (asprintf(&dbus_system_proxy_socket, DBUS_SYSTEM_PROXY_SOCKET_FORMAT, + (int) getuid(), (int) getpid()) == -1) errExit("asprintf"); write_arg(args_pipe[1], dbus_system_proxy_socket); write_arg(args_pipe[1], "--filter"); @@ -245,8 +275,23 @@ void dbus_proxy_stop(void) { } static void socket_overlay(char *socket_path, char *proxy_path) { - if (mount(proxy_path, socket_path, NULL, MS_BIND, NULL) == -1) - errExit("mount"); + int fd = safe_fd(proxy_path, O_PATH | O_NOFOLLOW | O_CLOEXEC); + if (fd == -1) + errExit("opening DBus proxy socket"); + struct stat s; + if (fstat(fd, &s) == -1) + errExit("fstat"); + if (!S_ISSOCK(s.st_mode)) { + errno = ENOTSOCK; + errExit("mounting DBus proxy socket"); + } + char *proxy_fd_path; + if (asprintf(&proxy_fd_path, "/proc/self/fd/%d", fd) == -1) + errExit("asprintf"); + if (mount(proxy_path, socket_path, NULL, MS_BIND | MS_REC, NULL) == -1) + errExit("mount bind"); + free(proxy_fd_path); + close(fd); } static void disable_socket_dir(void) { @@ -281,7 +326,7 @@ void dbus_apply_policy(void) { } else { dbus_orig_user_socket_path = dbus_new_user_socket_path; } - char *dbus_orig_user_socket = dbus_user_path_env + strlen(DBUS_SOCKET_PATH_PREFIX); + char *dbus_orig_user_socket = dbus_orig_user_socket_path + strlen(DBUS_SOCKET_PATH_PREFIX); if (arg_dbus_user != DBUS_POLICY_ALLOW) { if (arg_dbus_user == DBUS_POLICY_FILTER) { diff --git a/src/firejail/main.c b/src/firejail/main.c index f59fc26e4..dc213b988 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -181,6 +181,7 @@ static void myexit(int rv) { // delete sandbox files in shared memory + dbus_proxy_stop(); EUID_ROOT(); delete_run_files(sandbox_pid); appimage_clear(); @@ -3023,9 +3024,6 @@ printf("**********************************\n"); // end of signal-safe code //***************************** - // stop dbus proxy (if any) - dbus_proxy_stop(); - // free globals if (cfg.profile) { ProfileEntry *prf = cfg.profile; diff --git a/src/firejail/preproc.c b/src/firejail/preproc.c index 61573b220..c0b09e945 100644 --- a/src/firejail/preproc.c +++ b/src/firejail/preproc.c @@ -59,7 +59,17 @@ void preproc_build_firejail_dir(void) { } if (stat(RUN_FIREJAIL_DBUS_DIR, &s)) { - create_empty_dir_as_root(RUN_FIREJAIL_DBUS_DIR, 01777); + create_empty_dir_as_root(RUN_FIREJAIL_DBUS_DIR, 0755); + if (arg_debug) + printf("Remounting the " RUN_FIREJAIL_DBUS_DIR + " directory as noexec\n"); + if (mount(RUN_FIREJAIL_DBUS_DIR, RUN_FIREJAIL_DBUS_DIR, NULL, + MS_BIND, NULL) == -1) + errExit("mounting " RUN_FIREJAIL_DBUS_DIR); + if (mount(NULL, RUN_FIREJAIL_DBUS_DIR, NULL, + MS_REMOUNT | MS_BIND | MS_NOSUID | MS_NOEXEC | MS_NODEV, + "mode=755,gid=0") == -1) + errExit("remounting " RUN_FIREJAIL_DBUS_DIR); } if (stat(RUN_FIREJAIL_APPIMAGE_DIR, &s)) { -- cgit v1.2.3-54-g00ecf