This is an automated email from the git hooks/post-receive script.
nickm pushed a change to branch main in repository tor.
from 8d5692a2f7 Changes file for mr 569 new 531275b0f3 sandbox: fix openat filtering on AArch64 new 8fd13f7a7b sandbox: filter {chown,chmod,rename} via their *at variant on Aarch64 new eb0749d649 sandbox: replace SCMP_CMP_NEG with masked equality checks new 42034ae9da changes: add entry for MR !574 new 853270a871 Merge remote-tracking branch 'tor-gitlab/mr/574'
The 5 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference.
Summary of changes: changes/aarch64_sandbox | 5 ++ src/lib/sandbox/sandbox.c | 128 ++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 117 insertions(+), 16 deletions(-) create mode 100644 changes/aarch64_sandbox
This is an automated email from the git hooks/post-receive script.
nickm pushed a commit to branch main in repository tor.
commit 531275b0f36b39fb6a5c6aaab47e26993ce6fa91 Author: Pierre Bourdon delroth@gmail.com AuthorDate: Sat Apr 30 11:52:59 2022 +0200
sandbox: fix openat filtering on AArch64
New glibc versions not sign-extending 32 bit negative constants seems to not be a thing on AArch64. I suspect that this might not be the only architecture where the sign-extensions is happening, and the correct fix might be instead to use a proper 32 bit comparison for the first openat parameter. For now, band-aid fix this so the sandbox can work again on AArch64. --- src/lib/sandbox/sandbox.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/src/lib/sandbox/sandbox.c b/src/lib/sandbox/sandbox.c index e87edd8e21..4681d4795a 100644 --- a/src/lib/sandbox/sandbox.c +++ b/src/lib/sandbox/sandbox.c @@ -518,7 +518,12 @@ libc_uses_openat_for_opendir(void) static int libc_negative_constant_needs_cast(void) { +#if defined(__aarch64__) && defined(__LP64__) + /* Existing glibc versions always sign-extend to 64 bits on AArch64. */ + return 0; +#else return is_libc_at_least(2, 27); +#endif }
/** Allow a single file to be opened. If <b>use_openat</b> is true,
This is an automated email from the git hooks/post-receive script.
nickm pushed a commit to branch main in repository tor.
commit 8fd13f7a7bfd4efc02d888ce9d10bcb6a80a03c8 Author: Pierre Bourdon delroth@gmail.com AuthorDate: Sat Apr 30 13:02:16 2022 +0200
sandbox: filter {chown,chmod,rename} via their *at variant on Aarch64
The chown/chmod/rename syscalls have never existed on AArch64, and libc implements the POSIX functions via the fchownat/fchmodat/renameat syscalls instead.
Add new filter functions for fchownat/fchmodat/renameat, not made architecture specific since the syscalls exists everywhere else too. However, in order to limit seccomp filter space usage, we only insert rules for one of {chown, chown32, fchownat} depending on the architecture (resp. {chmod, fchmodat}, {rename, renameat}). --- src/lib/sandbox/sandbox.c | 106 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 104 insertions(+), 2 deletions(-)
diff --git a/src/lib/sandbox/sandbox.c b/src/lib/sandbox/sandbox.c index 4681d4795a..cd55897334 100644 --- a/src/lib/sandbox/sandbox.c +++ b/src/lib/sandbox/sandbox.c @@ -614,6 +614,32 @@ sb_chmod(scmp_filter_ctx ctx, sandbox_cfg_t *filter) return 0; }
+static int +sb_fchmodat(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 + == SCMP_SYS(fchmodat)) { + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(fchmodat), + SCMP_CMP_NEG(0, SCMP_CMP_EQ, AT_FDCWD), + SCMP_CMP_STR(1, SCMP_CMP_EQ, param->value)); + if (rc != 0) { + log_err(LD_BUG,"(Sandbox) failed to add fchmodat syscall, received " + "libseccomp error %d", rc); + return rc; + } + } + } + + return 0; +} + #ifdef __i386__ static int sb_chown32(scmp_filter_ctx ctx, sandbox_cfg_t *filter) @@ -666,6 +692,32 @@ sb_chown(scmp_filter_ctx ctx, sandbox_cfg_t *filter) } #endif /* defined(__i386__) */
+static int +sb_fchownat(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 + == SCMP_SYS(fchownat)) { + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(fchownat), + SCMP_CMP_NEG(0, SCMP_CMP_EQ, AT_FDCWD), + SCMP_CMP_STR(1, SCMP_CMP_EQ, param->value)); + if (rc != 0) { + log_err(LD_BUG,"(Sandbox) failed to add fchownat syscall, received " + "libseccomp error %d", rc); + return rc; + } + } + } + + return 0; +} + /** * Function responsible for setting up the rename syscall for * the seccomp filter sandbox. @@ -697,6 +749,39 @@ sb_rename(scmp_filter_ctx ctx, sandbox_cfg_t *filter) return 0; }
+/** + * Function responsible for setting up the renameat syscall for + * the seccomp filter sandbox. + */ +static int +sb_renameat(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 == SCMP_SYS(renameat)) { + + rc = seccomp_rule_add_4(ctx, SCMP_ACT_ALLOW, SCMP_SYS(renameat), + SCMP_CMP_NEG(0, SCMP_CMP_EQ, AT_FDCWD), + SCMP_CMP_STR(1, SCMP_CMP_EQ, param->value), + SCMP_CMP_NEG(2, SCMP_CMP_EQ, AT_FDCWD), + SCMP_CMP_STR(3, SCMP_CMP_EQ, param->value2)); + if (rc != 0) { + log_err(LD_BUG,"(Sandbox) failed to add renameat syscall, received " + "libseccomp error %d", rc); + return rc; + } + } + } + + return 0; +} + /** * Function responsible for setting up the openat syscall for * the seccomp filter sandbox. @@ -1317,7 +1402,9 @@ static sandbox_filter_func_t filter_func[] = { #else sb_chown, #endif + sb_fchownat, sb_chmod, + sb_fchmodat, sb_open, sb_openat, sb_opendir, @@ -1325,6 +1412,7 @@ static sandbox_filter_func_t filter_func[] = { sb_ptrace, #endif sb_rename, + sb_renameat, #ifdef __NR_fcntl64 sb_fcntl64, #endif @@ -1592,10 +1680,24 @@ new_element(int syscall, char *value)
#ifdef __i386__ #define SCMP_chown SCMP_SYS(chown32) +#elif defined(__aarch64__) && defined(__LP64__) +#define SCMP_chown SCMP_SYS(fchownat) #else #define SCMP_chown SCMP_SYS(chown) #endif
+#if defined(__aarch64__) && defined(__LP64__) +#define SCMP_chmod SCMP_SYS(fchmodat) +#else +#define SCMP_chmod SCMP_SYS(chmod) +#endif + +#if defined(__aarch64__) && defined(__LP64__) +#define SCMP_rename SCMP_SYS(renameat) +#else +#define SCMP_rename SCMP_SYS(rename) +#endif + #ifdef __NR_stat64 #define SCMP_stat SCMP_SYS(stat64) #else @@ -1633,7 +1735,7 @@ sandbox_cfg_allow_chmod_filename(sandbox_cfg_t **cfg, char *file) { sandbox_cfg_t *elem = NULL;
- elem = new_element(SCMP_SYS(chmod), file); + elem = new_element(SCMP_chmod, file);
elem->next = *cfg; *cfg = elem; @@ -1659,7 +1761,7 @@ sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2) { sandbox_cfg_t *elem = NULL;
- elem = new_element2(SCMP_SYS(rename), file1, file2); + elem = new_element2(SCMP_rename, file1, file2);
elem->next = *cfg; *cfg = elem;
This is an automated email from the git hooks/post-receive script.
nickm pushed a commit to branch main in repository tor.
commit eb0749d64917fee6ff74c3810dbec8cd063f546c Author: Pierre Bourdon delroth@gmail.com AuthorDate: Wed May 4 07:19:40 2022 +0200
sandbox: replace SCMP_CMP_NEG with masked equality checks
For some syscalls the kernel ABI uses 32 bit signed integers. Whether these 32 bit integer values are sign extended or zero extended to the native 64 bit register sizes is undefined and dependent on the {arch, compiler, libc} being used. Instead of trying to detect which cases zero-extend and which cases sign-extend, this commit uses a masked equality check on the lower 32 bits of the value. --- src/lib/sandbox/sandbox.c | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-)
diff --git a/src/lib/sandbox/sandbox.c b/src/lib/sandbox/sandbox.c index cd55897334..b734977c3a 100644 --- a/src/lib/sandbox/sandbox.c +++ b/src/lib/sandbox/sandbox.c @@ -141,10 +141,12 @@ 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)))) +/* Negative constants aren't consistently sign extended or zero extended. + * Different compilers, libc, and architectures behave differently. For cases + * where the kernel ABI uses a 32 bit integer, this macro can be used to + * mask-compare only the lower 32 bits of the value. */ +#define SCMP_CMP_LOWER32_EQ(a,b) \ + SCMP_CMP4((a), SCMP_CMP_MASKED_EQ, 0xFFFFFFFF, (unsigned int)(b))
/** Variable used for storing all syscall numbers that will be allowed with the * stage 1 general Tor sandbox. @@ -513,19 +515,6 @@ libc_uses_openat_for_opendir(void) (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 -libc_negative_constant_needs_cast(void) -{ -#if defined(__aarch64__) && defined(__LP64__) - /* Existing glibc versions always sign-extend to 64 bits on AArch64. */ - return 0; -#else - return is_libc_at_least(2, 27); -#endif -} - /** Allow a single file to be opened. If <b>use_openat</b> is true, * we're using a libc that remaps all the opens into openats. */ static int @@ -533,7 +522,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_NEG(0, SCMP_CMP_EQ, AT_FDCWD), + SCMP_CMP_LOWER32_EQ(0, AT_FDCWD), SCMP_CMP_STR(1, SCMP_CMP_EQ, file)); } else { return seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(open), @@ -627,7 +616,7 @@ sb_fchmodat(scmp_filter_ctx ctx, sandbox_cfg_t *filter) if (param != NULL && param->prot == 1 && param->syscall == SCMP_SYS(fchmodat)) { rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(fchmodat), - SCMP_CMP_NEG(0, SCMP_CMP_EQ, AT_FDCWD), + SCMP_CMP_LOWER32_EQ(0, AT_FDCWD), SCMP_CMP_STR(1, SCMP_CMP_EQ, param->value)); if (rc != 0) { log_err(LD_BUG,"(Sandbox) failed to add fchmodat syscall, received " @@ -705,7 +694,7 @@ sb_fchownat(scmp_filter_ctx ctx, sandbox_cfg_t *filter) if (param != NULL && param->prot == 1 && param->syscall == SCMP_SYS(fchownat)) { rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(fchownat), - SCMP_CMP_NEG(0, SCMP_CMP_EQ, AT_FDCWD), + SCMP_CMP_LOWER32_EQ(0, AT_FDCWD), SCMP_CMP_STR(1, SCMP_CMP_EQ, param->value)); if (rc != 0) { log_err(LD_BUG,"(Sandbox) failed to add fchownat syscall, received " @@ -767,9 +756,9 @@ sb_renameat(scmp_filter_ctx ctx, sandbox_cfg_t *filter) param->syscall == SCMP_SYS(renameat)) {
rc = seccomp_rule_add_4(ctx, SCMP_ACT_ALLOW, SCMP_SYS(renameat), - SCMP_CMP_NEG(0, SCMP_CMP_EQ, AT_FDCWD), + SCMP_CMP_LOWER32_EQ(0, AT_FDCWD), SCMP_CMP_STR(1, SCMP_CMP_EQ, param->value), - SCMP_CMP_NEG(2, SCMP_CMP_EQ, AT_FDCWD), + SCMP_CMP_LOWER32_EQ(2, AT_FDCWD), SCMP_CMP_STR(3, SCMP_CMP_EQ, param->value2)); if (rc != 0) { log_err(LD_BUG,"(Sandbox) failed to add renameat syscall, received " @@ -799,7 +788,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_NEG(0, SCMP_CMP_EQ, AT_FDCWD), + SCMP_CMP_LOWER32_EQ(0, 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));
This is an automated email from the git hooks/post-receive script.
nickm pushed a commit to branch main in repository tor.
commit 42034ae9da2866c67ce8cb8522d6a619d8b21170 Author: Pierre Bourdon delroth@gmail.com AuthorDate: Wed May 4 07:31:06 2022 +0200
changes: add entry for MR !574 --- changes/aarch64_sandbox | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/changes/aarch64_sandbox b/changes/aarch64_sandbox new file mode 100644 index 0000000000..d1d64d6e6c --- /dev/null +++ b/changes/aarch64_sandbox @@ -0,0 +1,5 @@ + o Minor bugfixes (sandbox): + - Fix sandbox support on AArch64 systems. More "*at" variants of syscalls + are now supported. Signed 32 bit syscall parameters are checked more + precisely, which should lead to lower likelihood of breakages with future + compiler and libc releases. Fixes bug 40599; bugfix on 0.4.4.3-alpha.
This is an automated email from the git hooks/post-receive script.
nickm pushed a commit to branch main in repository tor.
commit 853270a8719db85d22e982fe638db0e2b62af658 Merge: 8d5692a2f7 42034ae9da Author: Nick Mathewson nickm@torproject.org AuthorDate: Wed May 4 10:34:03 2022 -0400
Merge remote-tracking branch 'tor-gitlab/mr/574'
changes/aarch64_sandbox | 5 ++ src/lib/sandbox/sandbox.c | 128 ++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 117 insertions(+), 16 deletions(-)
tor-commits@lists.torproject.org
-
gitolite role