July 2020 - tor-commits - lists.torproject.org

[tor/master] Fix seccomp sandbox rules for openat #27315
by nickm＠torproject.org 21 Jul '20

21 Jul '20

commit c79b4397d3839b77e85ceccc5a948f58c9fe37e6 Author: Daniel Pinto <danielpinto52(a)gmail.com> Date: Wed Jul 1 20:30:04 2020 +0100 Fix seccomp sandbox rules for openat #27315 The need for casting negative syscall arguments depends on the glibc version. This affects the rules for the openat syscall which uses the constant AT_FDCWD that is defined as a negative number. This commit adds logic to only apply the cast when necessary, on glibc versions from 2.27 onwards. --- changes/bug27315 | 6 ++++++ src/lib/sandbox/sandbox.c | 42 ++++++++++++++++++++++++++++++++---------- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/changes/bug27315 b/changes/bug27315 new file mode 100644 index 0000000000..8af3ac8559 --- /dev/null +++ b/changes/bug27315 @@ -0,0 +1,6 @@ + o Minor bugfixes (linux seccomp2 sandbox): + - Fix a regression on sandboxing rules for the openat() syscall. + The fix for bug 25440 fixed the problem on systems with glibc >= + 2.27 but broke tor on previous versions of glibc. We now apply + the correct seccomp rule according to the running glibc version. + Patch from Daniel Pinto. Fixes bug 27315; bugfix on 0.3.5.11. diff --git a/src/lib/sandbox/sandbox.c b/src/lib/sandbox/sandbox.c index 8f577b0660..76aacb0834 100644 --- a/src/lib/sandbox/sandbox.c +++ b/src/lib/sandbox/sandbox.c @@ -134,6 +134,10 @@ static sandbox_cfg_t *filter_dynamic = NULL; * the high bits of the value might get masked out improperly. */ #define SCMP_CMP_MASKED(a,b,c) \ SCMP_CMP4((a), SCMP_CMP_MASKED_EQ, ~(scmp_datum_t)(b), (c)) +/* For negative constants, the rule to add depends on the glibc version. */ +#define SCMP_CMP_NEG(a,op,b) (libc_negative_constant_needs_cast() ? \ + (SCMP_CMP((a), (op), (unsigned int)(b))) : \ + (SCMP_CMP_STR((a), (op), (b)))) /** Variable used for storing all syscall numbers that will be allowed with the * stage 1 general Tor sandbox. @@ -424,31 +428,49 @@ sb_mmap2(scmp_filter_ctx ctx, sandbox_cfg_t *filter) #endif #endif -/* Return true if we think we're running with a libc that always uses - * openat on linux. */ +/* Return true the libc version is greater or equal than + * major.minor. Returns false otherwise. */ static int -libc_uses_openat_for_everything(void) +is_libc_at_least(int major, int minor) { #ifdef CHECK_LIBC_VERSION const char *version = gnu_get_libc_version(); if (version == NULL) return 0; - int major = -1; - int minor = -1; + int libc_major = -1; + int libc_minor = -1; - tor_sscanf(version, "%d.%d", &major, &minor); - if (major >= 3) + tor_sscanf(version, "%d.%d", &libc_major, &libc_minor); + if (libc_major > major) return 1; - else if (major == 2 && minor >= 26) + else if (libc_major == major && libc_minor >= minor) return 1; else return 0; #else /* !(defined(CHECK_LIBC_VERSION)) */ + (void)major; + (void)minor; return 0; #endif /* defined(CHECK_LIBC_VERSION) */ } +/* Return true if we think we're running with a libc that always uses + * openat on linux. */ +static int +libc_uses_openat_for_everything(void) +{ + return is_libc_at_least(2, 26); +} + +/* 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 +libc_negative_constant_needs_cast(void) +{ + return is_libc_at_least(2, 27); +} + /** Allow a single file to be opened. If use_openat is true, * we're using a libc that remaps all the opens into openats. */ static int @@ -456,7 +478,7 @@ allow_file_open(scmp_filter_ctx ctx, int use_openat, const char *file) { if (use_openat) { return seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(openat), - SCMP_CMP(0, SCMP_CMP_EQ, (unsigned int)AT_FDCWD), + SCMP_CMP_NEG(0, SCMP_CMP_EQ, AT_FDCWD), SCMP_CMP_STR(1, SCMP_CMP_EQ, file)); } else { return seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(open), @@ -592,7 +614,7 @@ sb_openat(scmp_filter_ctx ctx, sandbox_cfg_t *filter) if (param != NULL && param->prot == 1 && param->syscall == SCMP_SYS(openat)) { rc = seccomp_rule_add_3(ctx, SCMP_ACT_ALLOW, SCMP_SYS(openat), - SCMP_CMP(0, SCMP_CMP_EQ, AT_FDCWD), + 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));

1 0

[tor/master] Fix seccomp sandbox rules for opening directories #40020
by nickm＠torproject.org 21 Jul '20

21 Jul '20

commit d28bfb2cd5665c38bd14d6a72848209dcd66faf9 Author: Daniel Pinto <danielpinto52(a)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.

1 0

[tor/maint-0.4.4] Merge remote-tracking branch 'tor-gitlab/mr/68' into maint-0.4.4
by nickm＠torproject.org 21 Jul '20

21 Jul '20

commit 5cd85feaa7f05b2dc6229b4a72216ee8ffdc3b66 Merge: 95d4bff784 d28bfb2cd5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 21 09:29:03 2020 -0400 Merge remote-tracking branch 'tor-gitlab/mr/68' into maint-0.4.4 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(-)

1 0

[tor/maint-0.4.4] Fix seccomp sandbox rules for openat #27315
by nickm＠torproject.org 21 Jul '20

21 Jul '20

commit c79b4397d3839b77e85ceccc5a948f58c9fe37e6 Author: Daniel Pinto <danielpinto52(a)gmail.com> Date: Wed Jul 1 20:30:04 2020 +0100 Fix seccomp sandbox rules for openat #27315 The need for casting negative syscall arguments depends on the glibc version. This affects the rules for the openat syscall which uses the constant AT_FDCWD that is defined as a negative number. This commit adds logic to only apply the cast when necessary, on glibc versions from 2.27 onwards. --- changes/bug27315 | 6 ++++++ src/lib/sandbox/sandbox.c | 42 ++++++++++++++++++++++++++++++++---------- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/changes/bug27315 b/changes/bug27315 new file mode 100644 index 0000000000..8af3ac8559 --- /dev/null +++ b/changes/bug27315 @@ -0,0 +1,6 @@ + o Minor bugfixes (linux seccomp2 sandbox): + - Fix a regression on sandboxing rules for the openat() syscall. + The fix for bug 25440 fixed the problem on systems with glibc >= + 2.27 but broke tor on previous versions of glibc. We now apply + the correct seccomp rule according to the running glibc version. + Patch from Daniel Pinto. Fixes bug 27315; bugfix on 0.3.5.11. diff --git a/src/lib/sandbox/sandbox.c b/src/lib/sandbox/sandbox.c index 8f577b0660..76aacb0834 100644 --- a/src/lib/sandbox/sandbox.c +++ b/src/lib/sandbox/sandbox.c @@ -134,6 +134,10 @@ static sandbox_cfg_t *filter_dynamic = NULL; * the high bits of the value might get masked out improperly. */ #define SCMP_CMP_MASKED(a,b,c) \ SCMP_CMP4((a), SCMP_CMP_MASKED_EQ, ~(scmp_datum_t)(b), (c)) +/* For negative constants, the rule to add depends on the glibc version. */ +#define SCMP_CMP_NEG(a,op,b) (libc_negative_constant_needs_cast() ? \ + (SCMP_CMP((a), (op), (unsigned int)(b))) : \ + (SCMP_CMP_STR((a), (op), (b)))) /** Variable used for storing all syscall numbers that will be allowed with the * stage 1 general Tor sandbox. @@ -424,31 +428,49 @@ sb_mmap2(scmp_filter_ctx ctx, sandbox_cfg_t *filter) #endif #endif -/* Return true if we think we're running with a libc that always uses - * openat on linux. */ +/* Return true the libc version is greater or equal than + * major.minor. Returns false otherwise. */ static int -libc_uses_openat_for_everything(void) +is_libc_at_least(int major, int minor) { #ifdef CHECK_LIBC_VERSION const char *version = gnu_get_libc_version(); if (version == NULL) return 0; - int major = -1; - int minor = -1; + int libc_major = -1; + int libc_minor = -1; - tor_sscanf(version, "%d.%d", &major, &minor); - if (major >= 3) + tor_sscanf(version, "%d.%d", &libc_major, &libc_minor); + if (libc_major > major) return 1; - else if (major == 2 && minor >= 26) + else if (libc_major == major && libc_minor >= minor) return 1; else return 0; #else /* !(defined(CHECK_LIBC_VERSION)) */ + (void)major; + (void)minor; return 0; #endif /* defined(CHECK_LIBC_VERSION) */ } +/* Return true if we think we're running with a libc that always uses + * openat on linux. */ +static int +libc_uses_openat_for_everything(void) +{ + return is_libc_at_least(2, 26); +} + +/* 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 +libc_negative_constant_needs_cast(void) +{ + return is_libc_at_least(2, 27); +} + /** Allow a single file to be opened. If use_openat is true, * we're using a libc that remaps all the opens into openats. */ static int @@ -456,7 +478,7 @@ allow_file_open(scmp_filter_ctx ctx, int use_openat, const char *file) { if (use_openat) { return seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(openat), - SCMP_CMP(0, SCMP_CMP_EQ, (unsigned int)AT_FDCWD), + SCMP_CMP_NEG(0, SCMP_CMP_EQ, AT_FDCWD), SCMP_CMP_STR(1, SCMP_CMP_EQ, file)); } else { return seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(open), @@ -592,7 +614,7 @@ sb_openat(scmp_filter_ctx ctx, sandbox_cfg_t *filter) if (param != NULL && param->prot == 1 && param->syscall == SCMP_SYS(openat)) { rc = seccomp_rule_add_3(ctx, SCMP_ACT_ALLOW, SCMP_SYS(openat), - SCMP_CMP(0, SCMP_CMP_EQ, AT_FDCWD), + 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));

1 0

[tor/maint-0.4.4] Fix seccomp sandbox rules for opening directories #40020
by nickm＠torproject.org 21 Jul '20

21 Jul '20

commit d28bfb2cd5665c38bd14d6a72848209dcd66faf9 Author: Daniel Pinto <danielpinto52(a)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.

1 0

[tor/release-0.4.4] Fix seccomp sandbox rules for opening directories #40020
by nickm＠torproject.org 21 Jul '20

21 Jul '20

commit d28bfb2cd5665c38bd14d6a72848209dcd66faf9 Author: Daniel Pinto <danielpinto52(a)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.

1 0

[tor/master] Merge remote-tracking branch 'tor-gitlab/mr/68' into maint-0.4.4
by nickm＠torproject.org 21 Jul '20

21 Jul '20

commit 5cd85feaa7f05b2dc6229b4a72216ee8ffdc3b66 Merge: 95d4bff784 d28bfb2cd5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 21 09:29:03 2020 -0400 Merge remote-tracking branch 'tor-gitlab/mr/68' into maint-0.4.4 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(-)

1 0

[tor/release-0.4.4] Fix seccomp sandbox rules for openat #27315
by nickm＠torproject.org 21 Jul '20

21 Jul '20

commit c79b4397d3839b77e85ceccc5a948f58c9fe37e6 Author: Daniel Pinto <danielpinto52(a)gmail.com> Date: Wed Jul 1 20:30:04 2020 +0100 Fix seccomp sandbox rules for openat #27315 The need for casting negative syscall arguments depends on the glibc version. This affects the rules for the openat syscall which uses the constant AT_FDCWD that is defined as a negative number. This commit adds logic to only apply the cast when necessary, on glibc versions from 2.27 onwards. --- changes/bug27315 | 6 ++++++ src/lib/sandbox/sandbox.c | 42 ++++++++++++++++++++++++++++++++---------- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/changes/bug27315 b/changes/bug27315 new file mode 100644 index 0000000000..8af3ac8559 --- /dev/null +++ b/changes/bug27315 @@ -0,0 +1,6 @@ + o Minor bugfixes (linux seccomp2 sandbox): + - Fix a regression on sandboxing rules for the openat() syscall. + The fix for bug 25440 fixed the problem on systems with glibc >= + 2.27 but broke tor on previous versions of glibc. We now apply + the correct seccomp rule according to the running glibc version. + Patch from Daniel Pinto. Fixes bug 27315; bugfix on 0.3.5.11. diff --git a/src/lib/sandbox/sandbox.c b/src/lib/sandbox/sandbox.c index 8f577b0660..76aacb0834 100644 --- a/src/lib/sandbox/sandbox.c +++ b/src/lib/sandbox/sandbox.c @@ -134,6 +134,10 @@ static sandbox_cfg_t *filter_dynamic = NULL; * the high bits of the value might get masked out improperly. */ #define SCMP_CMP_MASKED(a,b,c) \ SCMP_CMP4((a), SCMP_CMP_MASKED_EQ, ~(scmp_datum_t)(b), (c)) +/* For negative constants, the rule to add depends on the glibc version. */ +#define SCMP_CMP_NEG(a,op,b) (libc_negative_constant_needs_cast() ? \ + (SCMP_CMP((a), (op), (unsigned int)(b))) : \ + (SCMP_CMP_STR((a), (op), (b)))) /** Variable used for storing all syscall numbers that will be allowed with the * stage 1 general Tor sandbox. @@ -424,31 +428,49 @@ sb_mmap2(scmp_filter_ctx ctx, sandbox_cfg_t *filter) #endif #endif -/* Return true if we think we're running with a libc that always uses - * openat on linux. */ +/* Return true the libc version is greater or equal than + * major.minor. Returns false otherwise. */ static int -libc_uses_openat_for_everything(void) +is_libc_at_least(int major, int minor) { #ifdef CHECK_LIBC_VERSION const char *version = gnu_get_libc_version(); if (version == NULL) return 0; - int major = -1; - int minor = -1; + int libc_major = -1; + int libc_minor = -1; - tor_sscanf(version, "%d.%d", &major, &minor); - if (major >= 3) + tor_sscanf(version, "%d.%d", &libc_major, &libc_minor); + if (libc_major > major) return 1; - else if (major == 2 && minor >= 26) + else if (libc_major == major && libc_minor >= minor) return 1; else return 0; #else /* !(defined(CHECK_LIBC_VERSION)) */ + (void)major; + (void)minor; return 0; #endif /* defined(CHECK_LIBC_VERSION) */ } +/* Return true if we think we're running with a libc that always uses + * openat on linux. */ +static int +libc_uses_openat_for_everything(void) +{ + return is_libc_at_least(2, 26); +} + +/* 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 +libc_negative_constant_needs_cast(void) +{ + return is_libc_at_least(2, 27); +} + /** Allow a single file to be opened. If use_openat is true, * we're using a libc that remaps all the opens into openats. */ static int @@ -456,7 +478,7 @@ allow_file_open(scmp_filter_ctx ctx, int use_openat, const char *file) { if (use_openat) { return seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(openat), - SCMP_CMP(0, SCMP_CMP_EQ, (unsigned int)AT_FDCWD), + SCMP_CMP_NEG(0, SCMP_CMP_EQ, AT_FDCWD), SCMP_CMP_STR(1, SCMP_CMP_EQ, file)); } else { return seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(open), @@ -592,7 +614,7 @@ sb_openat(scmp_filter_ctx ctx, sandbox_cfg_t *filter) if (param != NULL && param->prot == 1 && param->syscall == SCMP_SYS(openat)) { rc = seccomp_rule_add_3(ctx, SCMP_ACT_ALLOW, SCMP_SYS(openat), - SCMP_CMP(0, SCMP_CMP_EQ, AT_FDCWD), + 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));

1 0

[tor/release-0.4.4] Merge remote-tracking branch 'tor-gitlab/mr/68' into maint-0.4.4
by nickm＠torproject.org 21 Jul '20

21 Jul '20

commit 5cd85feaa7f05b2dc6229b4a72216ee8ffdc3b66 Merge: 95d4bff784 d28bfb2cd5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 21 09:29:03 2020 -0400 Merge remote-tracking branch 'tor-gitlab/mr/68' into maint-0.4.4 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(-)

1 0

[tor/master] Merge branch 'maint-0.4.4'
by nickm＠torproject.org 21 Jul '20

21 Jul '20

commit 7d0e64016ea5b60f64b2f68029d7efa44f25c54d Merge: 2ee33765ae 5cd85feaa7 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 21 09:29:13 2020 -0400 Merge branch 'maint-0.4.4' 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(-)

1 0