From f833df38488ea40fc3d601ccefd64cfa3fce8bb4 Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Fri, 17 Sep 2021 13:05:32 +0200 Subject: [PATCH 1/4] seccomp: Always install filters for native architecture The commit 6597686865ff ("seccomp: don't install filters for archs that can't use syscalls") introduced a regression where filters may not be installed for the "native" architecture. This means that setting SystemCallArchitectures=native for a unit effectively disables the SystemCallFilter= and SystemCallLog= options. Conceptually, we have two filter stages: 1. architecture used for syscall (SystemCallArchitectures=) 2. syscall + architecture combination (SystemCallFilter=) The above commit tried to optimize the filter generation by skipping the second level filtering when it is not required. However, systemd will never fully block the "native" architecture using the first level filter. This makes the code a lot simpler, as systemd can execve() the target binary using its own architecture. And, it should be perfectly fine as the "native" architecture will always be the one with the most restrictive seccomp filtering. Said differently, the bug arises because (on x86_64): 1. x86_64 is permitted by libseccomp already 2. native != x86_64 3. the loop wants to block x86_64 because the permitted set only contains "native" (i.e. "native" != "x86_64") 4. x86_64 is marked as blocked in seccomp_local_archs Thereby we have an inconsistency, where it is marked as blocked in the seccomp_local_archs array but it is allowed by libseccomp. i.e. we will skip generating filter stage 2 without having stage 1 in place. The fix is simple, we just skip the native architecture when looping seccomp_local_archs. This way the inconsistency cannot happen. --- src/shared/seccomp-util.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c index 631ca5dd34..31d6b542c0 100644 --- a/src/shared/seccomp-util.c +++ b/src/shared/seccomp-util.c @@ -1789,6 +1789,10 @@ int seccomp_restrict_archs(Set *archs) { for (unsigned i = 0; seccomp_local_archs[i] != SECCOMP_LOCAL_ARCH_END; ++i) { uint32_t arch = seccomp_local_archs[i]; + /* See above comment, our "native" architecture is never blocked. */ + if (arch == seccomp_arch_native()) + continue; + /* That architecture might have already been blocked by a previous call to seccomp_restrict_archs. */ if (arch == SECCOMP_LOCAL_ARCH_BLOCKED) continue; From 08bf703cc1511817cdf67543c3b166dc8831ba8c Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Fri, 17 Sep 2021 14:00:39 +0200 Subject: [PATCH 2/4] test: Check that "native" architecture is always filtered --- src/test/test-seccomp.c | 61 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c index 023c4b6e0e..dc3088d4b0 100644 --- a/src/test/test-seccomp.c +++ b/src/test/test-seccomp.c @@ -890,6 +890,66 @@ static void test_load_syscall_filter_set_raw(void) { assert_se(wait_for_terminate_and_check("syscallrawseccomp", pid, WAIT_LOG) == EXIT_SUCCESS); } +static void test_native_syscalls_filtered(void) { + pid_t pid; + + log_info("/* %s */", __func__); + + if (!is_seccomp_available()) { + log_notice("Seccomp not available, skipping %s", __func__); + return; + } + if (!have_seccomp_privs()) { + log_notice("Not privileged, skipping %s", __func__); + return; + } + + pid = fork(); + assert_se(pid >= 0); + + if (pid == 0) { + _cleanup_set_free_ Set *arch_s = NULL; + _cleanup_hashmap_free_ Hashmap *s = NULL; + + /* Passing "native" or an empty set is equivalent, just do both here. */ + assert_se(arch_s = set_new(NULL)); + assert_se(seccomp_restrict_archs(arch_s) >= 0); + assert_se(set_put(arch_s, SCMP_ARCH_NATIVE) >= 0); + assert_se(seccomp_restrict_archs(arch_s) >= 0); + + assert_se(access("/", F_OK) >= 0); + assert_se(poll(NULL, 0, 0) == 0); + + assert_se(seccomp_load_syscall_filter_set_raw(SCMP_ACT_ALLOW, NULL, scmp_act_kill_process(), true) >= 0); + assert_se(access("/", F_OK) >= 0); + assert_se(poll(NULL, 0, 0) == 0); + + assert_se(s = hashmap_new(NULL)); +#if defined __NR_access && __NR_access >= 0 + assert_se(hashmap_put(s, UINT32_TO_PTR(__NR_access + 1), INT_TO_PTR(-1)) >= 0); + log_debug("has access()"); +#endif +#if defined __NR_faccessat && __NR_faccessat >= 0 + assert_se(hashmap_put(s, UINT32_TO_PTR(__NR_faccessat + 1), INT_TO_PTR(-1)) >= 0); + log_debug("has faccessat()"); +#endif +#if defined __NR_faccessat2 && __NR_faccessat2 >= 0 + assert_se(hashmap_put(s, UINT32_TO_PTR(__NR_faccessat2 + 1), INT_TO_PTR(-1)) >= 0); + log_debug("has faccessat2()"); +#endif + + assert_se(!hashmap_isempty(s)); + assert_se(seccomp_load_syscall_filter_set_raw(SCMP_ACT_ALLOW, s, SCMP_ACT_ERRNO(EUCLEAN), true) >= 0); + + assert_se(access("/", F_OK) < 0); + assert_se(errno == EUCLEAN); + + _exit(EXIT_SUCCESS); + } + + assert_se(wait_for_terminate_and_check("nativeseccomp", pid, WAIT_LOG) == EXIT_SUCCESS); +} + static void test_lock_personality(void) { unsigned long current; pid_t pid; @@ -1171,6 +1231,7 @@ int main(int argc, char *argv[]) { test_memory_deny_write_execute_shmat(); test_restrict_archs(); test_load_syscall_filter_set_raw(); + test_native_syscalls_filtered(); test_lock_personality(); test_restrict_suid_sgid(); From e975a94559900f47a3a5cc333682992616877a67 Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Fri, 24 Sep 2021 13:35:34 +0200 Subject: [PATCH 3/4] test: Add failing/non-failing syscall filter test setting architecture This adds a high level test verifying that syscall filtering in combination with a simple architecture filter for the "native" architecture works fine. --- src/test/test-execute.c | 2 ++ test/test-execute/exec-systemcallfilter-failing3.service | 9 +++++++++ .../exec-systemcallfilter-not-failing3.service | 8 ++++++++ 3 files changed, 19 insertions(+) create mode 100644 test/test-execute/exec-systemcallfilter-failing3.service create mode 100644 test/test-execute/exec-systemcallfilter-not-failing3.service diff --git a/src/test/test-execute.c b/src/test/test-execute.c index 99beb05d29..98989f405a 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -713,8 +713,10 @@ static void test_exec_systemcallfilter(Manager *m) { test(m, "exec-systemcallfilter-not-failing.service", 0, CLD_EXITED); test(m, "exec-systemcallfilter-not-failing2.service", 0, CLD_EXITED); + test(m, "exec-systemcallfilter-not-failing3.service", 0, CLD_EXITED); test(m, "exec-systemcallfilter-failing.service", SIGSYS, CLD_KILLED); test(m, "exec-systemcallfilter-failing2.service", SIGSYS, CLD_KILLED); + test(m, "exec-systemcallfilter-failing3.service", SIGSYS, CLD_KILLED); r = find_executable("python3", NULL); if (r < 0) { diff --git a/test/test-execute/exec-systemcallfilter-failing3.service b/test/test-execute/exec-systemcallfilter-failing3.service new file mode 100644 index 0000000000..b8c96704d2 --- /dev/null +++ b/test/test-execute/exec-systemcallfilter-failing3.service @@ -0,0 +1,9 @@ +[Unit] +Description=Test for SystemCallFilter + +[Service] +ExecStart=/bin/sh -c '/bin/echo "This should not be seen"' +Type=oneshot +LimitCORE=0 +SystemCallArchitectures=native +SystemCallFilter=~write open execve fexecve execveat exit_group close mmap munmap fstat DONOTEXIST diff --git a/test/test-execute/exec-systemcallfilter-not-failing3.service b/test/test-execute/exec-systemcallfilter-not-failing3.service new file mode 100644 index 0000000000..7d72f5ab83 --- /dev/null +++ b/test/test-execute/exec-systemcallfilter-not-failing3.service @@ -0,0 +1,8 @@ +[Unit] +Description=Test for SystemCallFilter + +[Service] +ExecStart=/bin/sh -c 'echo "Foo bar"' +Type=oneshot +SystemCallArchitectures=native +SystemCallFilter= From e72be068b159ff547cb74ea3f16f13b38bb5a02a Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Thu, 30 Sep 2021 14:14:19 +0200 Subject: [PATCH 4/4] test: use a less restrictive portable profile when running w/ sanitizers Since f833df3 we now actually use the seccomp rules defined in portable profiles. However, the default one is too restrictive for sanitizers, as it blocks certain syscall required by LSan. Mitigate this by using the 'trusted' profile when running TEST-29-PORTABLE under sanitizers. --- test/units/testsuite-29.sh | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/test/units/testsuite-29.sh b/test/units/testsuite-29.sh index 77bb6db15a..7775695e6d 100755 --- a/test/units/testsuite-29.sh +++ b/test/units/testsuite-29.sh @@ -4,6 +4,13 @@ set -eux set -o pipefail +ARGS=() +if [[ -v ASAN_OPTIONS || -v UBSAN_OPTIONS ]]; then + # If we're running under sanitizers, we need to use a less restrictive + # profile, otherwise LSan syscall would get blocked by seccomp + ARGS+=(--profile=trusted) +fi + export SYSTEMD_LOG_LEVEL=debug mkdir -p /run/systemd/system/systemd-portabled.service.d/ cat </run/systemd/system/systemd-portabled.service.d/override.conf @@ -11,7 +18,7 @@ cat </run/systemd/system/systemd-portabled.service.d/override.conf Environment=SYSTEMD_LOG_LEVEL=debug EOF -portablectl attach --now --runtime /usr/share/minimal_0.raw app0 +portablectl "${ARGS[@]}" attach --now --runtime /usr/share/minimal_0.raw app0 systemctl is-active app0.service systemctl is-active app0-foo.service @@ -21,7 +28,7 @@ systemctl is-active app0-bar.service && exit 1 set -e set -o pipefail -portablectl reattach --now --runtime /usr/share/minimal_1.raw app0 +portablectl "${ARGS[@]}" reattach --now --runtime /usr/share/minimal_1.raw app0 systemctl is-active app0.service systemctl is-active app0-bar.service @@ -42,7 +49,7 @@ portablectl list | grep -q -F "No images." unsquashfs -dest /tmp/minimal_0 /usr/share/minimal_0.raw unsquashfs -dest /tmp/minimal_1 /usr/share/minimal_1.raw -portablectl attach --copy=symlink --now --runtime /tmp/minimal_0 app0 +portablectl "${ARGS[@]}" attach --copy=symlink --now --runtime /tmp/minimal_0 app0 systemctl is-active app0.service systemctl is-active app0-foo.service @@ -52,7 +59,7 @@ systemctl is-active app0-bar.service && exit 1 set -e set -o pipefail -portablectl reattach --now --enable --runtime /tmp/minimal_1 app0 +portablectl "${ARGS[@]}" reattach --now --enable --runtime /tmp/minimal_1 app0 systemctl is-active app0.service systemctl is-active app0-bar.service @@ -68,21 +75,21 @@ portablectl detach --now --enable --runtime /tmp/minimal_1 app0 portablectl list | grep -q -F "No images." -portablectl attach --now --runtime --extension /usr/share/app0.raw /usr/share/minimal_0.raw app0 +portablectl "${ARGS[@]}" attach --now --runtime --extension /usr/share/app0.raw /usr/share/minimal_0.raw app0 systemctl is-active app0.service -portablectl reattach --now --runtime --extension /usr/share/app0.raw /usr/share/minimal_1.raw app0 +portablectl "${ARGS[@]}" reattach --now --runtime --extension /usr/share/app0.raw /usr/share/minimal_1.raw app0 systemctl is-active app0.service portablectl detach --now --runtime --extension /usr/share/app0.raw /usr/share/minimal_1.raw app0 -portablectl attach --now --runtime --extension /usr/share/app1.raw /usr/share/minimal_0.raw app1 +portablectl "${ARGS[@]}" attach --now --runtime --extension /usr/share/app1.raw /usr/share/minimal_0.raw app1 systemctl is-active app1.service -portablectl reattach --now --runtime --extension /usr/share/app1.raw /usr/share/minimal_1.raw app1 +portablectl "${ARGS[@]}" reattach --now --runtime --extension /usr/share/app1.raw /usr/share/minimal_1.raw app1 systemctl is-active app1.service @@ -95,7 +102,7 @@ mount /usr/share/app1.raw /tmp/app1 mount /usr/share/minimal_0.raw /tmp/rootdir mount -t overlay overlay -o lowerdir=/tmp/app1:/tmp/rootdir /tmp/overlay -portablectl attach --copy=symlink --now --runtime /tmp/overlay app1 +portablectl "${ARGS[@]}" attach --copy=symlink --now --runtime /tmp/overlay app1 systemctl is-active app1.service