commit d28bfb2cd5665c38bd14d6a72848209dcd66faf9 Author: Daniel Pinto <danielpinto52@gmail.com> Date: Wed Jul 1 23:51:39 2020 +0100 Fix seccomp sandbox rules for opening directories #40020 Different versions of glibc use either open or openat for the opendir function. This commit adds logic to use the correct rule for each glibc version, namely: - Until 2.14 open is used - From 2.15 to to 2.21 openat is used - From 2.22 to 2.26 open is used - From 2.27 onwards openat is used --- changes/bug40020 | 9 ++++++ src/app/main/main.c | 15 ++++++--- src/lib/sandbox/sandbox.c | 77 ++++++++++++++++++++++++++++++++++++++++++++--- src/lib/sandbox/sandbox.h | 7 +++++ 4 files changed, 100 insertions(+), 8 deletions(-) diff --git a/changes/bug40020 b/changes/bug40020 new file mode 100644 index 0000000000..ca6ee2b85b --- /dev/null +++ b/changes/bug40020 @@ -0,0 +1,9 @@ + o Minor bugfixes (linux seccomp2 sandbox): + - Makes the seccomp sandbox allow the correct syscall for opendir + according to the running glibc version. The opendir function + either uses open or openat but the current code does not + differenciate between opendir and open calls. This adds a new + seccomp sandbox rule for opendir. This fixes crashes when + reloading torrc with sandbox enabled when running on glibc + 2.15 to 2.21 and 2.26. Patch from Daniel Pinto. Fixes bug 40020; + bugfix on 0.3.5.11. diff --git a/src/app/main/main.c b/src/app/main/main.c index 67f2181cd5..aceba78cfc 100644 --- a/src/app/main/main.c +++ b/src/app/main/main.c @@ -989,6 +989,9 @@ sandbox_init_filter(void) #define OPEN(name) \ sandbox_cfg_allow_open_filename(&cfg, tor_strdup(name)) +#define OPENDIR(dir) \ + sandbox_cfg_allow_opendir_dirname(&cfg, tor_strdup(dir)) + #define OPEN_DATADIR(name) \ sandbox_cfg_allow_open_filename(&cfg, get_datadir_fname(name)) @@ -1006,7 +1009,7 @@ sandbox_init_filter(void) } while (0) #define OPEN_KEY_DIRECTORY() \ - sandbox_cfg_allow_open_filename(&cfg, tor_strdup(options->KeyDirectory)) + OPENDIR(options->KeyDirectory) #define OPEN_CACHEDIR(name) \ sandbox_cfg_allow_open_filename(&cfg, get_cachedir_fname(name)) #define OPEN_CACHEDIR_SUFFIX(name, suffix) do { \ @@ -1020,7 +1023,7 @@ sandbox_init_filter(void) OPEN_KEYDIR(name suffix); \ } while (0) - OPEN(options->DataDirectory); + OPENDIR(options->DataDirectory); OPEN_KEY_DIRECTORY(); OPEN_CACHEDIR_SUFFIX("cached-certs", ".tmp"); @@ -1067,7 +1070,11 @@ sandbox_init_filter(void) } SMARTLIST_FOREACH(options->FilesOpenedByIncludes, char *, f, { - OPEN(f); + if (file_status(f) == FN_DIR) { + OPENDIR(f); + } else { + OPEN(f); + } }); #define RENAME_SUFFIX(name, suffix) \ @@ -1180,7 +1187,7 @@ sandbox_init_filter(void) * directory that holds it. */ char *dirname = tor_strdup(port->unix_addr); if (get_parent_directory(dirname) == 0) { - OPEN(dirname); + OPENDIR(dirname); } tor_free(dirname); sandbox_cfg_allow_chmod_filename(&cfg, tor_strdup(port->unix_addr)); diff --git a/src/lib/sandbox/sandbox.c b/src/lib/sandbox/sandbox.c index 76aacb0834..1903da70e8 100644 --- a/src/lib/sandbox/sandbox.c +++ b/src/lib/sandbox/sandbox.c @@ -276,6 +276,12 @@ static int filter_nopar_gen[] = { SCMP_SYS(poll) }; +/* opendir is not a syscall but it will use either open or openat. We do not + * want the decision to allow open/openat to be the callers reponsability, so + * we create a phony syscall number for opendir and sb_opendir will choose the + * correct syscall. */ +#define PHONY_OPENDIR_SYSCALL -2 + /* These macros help avoid the error where the number of filters we add on a * single rule don't match the arg_cnt param. */ #define seccomp_rule_add_0(ctx,act,call) \ @@ -455,14 +461,24 @@ is_libc_at_least(int major, int minor) #endif /* defined(CHECK_LIBC_VERSION) */ } -/* Return true if we think we're running with a libc that always uses - * openat on linux. */ +/* Return true if we think we're running with a libc that uses openat for the + * open function on linux. */ static int -libc_uses_openat_for_everything(void) +libc_uses_openat_for_open(void) { return is_libc_at_least(2, 26); } +/* Return true if we think we're running with a libc that uses openat for the + * opendir function on linux. */ +static int +libc_uses_openat_for_opendir(void) +{ + // libc 2.27 and above or between 2.15 (inclusive) and 2.22 (exclusive) + return is_libc_at_least(2, 27) || + (is_libc_at_least(2, 15) && !is_libc_at_least(2, 22)); +} + /* Return true if we think we're running with a libc that needs to cast * negative arguments like AT_FDCWD for seccomp rules. */ static int @@ -496,7 +512,7 @@ sb_open(scmp_filter_ctx ctx, sandbox_cfg_t *filter) int rc; sandbox_cfg_t *elem = NULL; - int use_openat = libc_uses_openat_for_everything(); + int use_openat = libc_uses_openat_for_open(); // for each dynamic parameter filters for (elem = filter; elem != NULL; elem = elem->next) { @@ -629,6 +645,38 @@ sb_openat(scmp_filter_ctx ctx, sandbox_cfg_t *filter) return 0; } +static int +sb_opendir(scmp_filter_ctx ctx, sandbox_cfg_t *filter) +{ + int rc; + sandbox_cfg_t *elem = NULL; + + // for each dynamic parameter filters + for (elem = filter; elem != NULL; elem = elem->next) { + smp_param_t *param = elem->param; + + if (param != NULL && param->prot == 1 && param->syscall + == PHONY_OPENDIR_SYSCALL) { + if (libc_uses_openat_for_opendir()) { + rc = seccomp_rule_add_3(ctx, SCMP_ACT_ALLOW, SCMP_SYS(openat), + SCMP_CMP_NEG(0, SCMP_CMP_EQ, AT_FDCWD), + SCMP_CMP_STR(1, SCMP_CMP_EQ, param->value), + SCMP_CMP(2, SCMP_CMP_EQ, O_RDONLY|O_NONBLOCK|O_LARGEFILE| + O_DIRECTORY|O_CLOEXEC)); + } else { + rc = allow_file_open(ctx, 0, param->value); + } + if (rc != 0) { + log_err(LD_BUG,"(Sandbox) failed to add openat syscall, received " + "libseccomp error %d", rc); + return rc; + } + } + } + + return 0; +} + /** * Function responsible for setting up the socket syscall for * the seccomp filter sandbox. @@ -1128,6 +1176,7 @@ static sandbox_filter_func_t filter_func[] = { sb_chmod, sb_open, sb_openat, + sb_opendir, sb_rename, #ifdef __NR_fcntl64 sb_fcntl64, @@ -1447,6 +1496,19 @@ sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file) return 0; } +int +sandbox_cfg_allow_opendir_dirname(sandbox_cfg_t **cfg, char *dir) +{ + sandbox_cfg_t *elem = NULL; + + elem = new_element(PHONY_OPENDIR_SYSCALL, dir); + + elem->next = *cfg; + *cfg = elem; + + return 0; +} + /** * Function responsible for going through the parameter syscall filters and * call each function pointer in the list. @@ -1752,6 +1814,13 @@ sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file) return 0; } +int +sandbox_cfg_allow_opendir_dirname(sandbox_cfg_t **cfg, char *dir) +{ + (void)cfg; (void)dir; + return 0; +} + int sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file) { diff --git a/src/lib/sandbox/sandbox.h b/src/lib/sandbox/sandbox.h index 5bec09a36a..8542b57f9c 100644 --- a/src/lib/sandbox/sandbox.h +++ b/src/lib/sandbox/sandbox.h @@ -135,6 +135,13 @@ int sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2); */ int sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file); +/** + * Function used to add a opendir allowed filename to a supplied configuration. + * The (char*) specifies the path to the allowed dir; we steal the pointer to + * that dir. + */ +int sandbox_cfg_allow_opendir_dirname(sandbox_cfg_t **cfg, char *dir); + /** * Function used to add a stat/stat64 allowed filename to a configuration. * The (char*) specifies the path to the allowed file; that pointer is stolen.