From 86a1ee93f3995e8dcbc1e90c0c6846561d9dd98b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 1 Dec 2023 15:53:06 +0100 Subject: [PATCH 1/4] core: fix comment --- src/core/exec-invoke.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index 1e08296b46..d4e284d764 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -5117,7 +5117,7 @@ int exec_invoke( #endif #if HAVE_SECCOMP - /* This really should remain as close to the execve() as possible, to make sure our own code is unaffected + /* This really should remain as close to the execve() as possible, to make sure our own code is affected * by the filter as little as possible. */ r = apply_syscall_filter(context, params, needs_ambient_hack); if (r < 0) { From 2331c02d06cae97b87637a0fc6bb4961b509ccf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 1 Dec 2023 19:03:23 +0100 Subject: [PATCH 2/4] core: when applying syscall filters, use ENOSYS for unknown calls glibc starting using fchmodat2 to implement fchmod with flags [1], but current version of libseccomp does not support fchmodat2 [2]. This is causing problems with programs sandboxed by systemd. libseccomp needs to know a syscall to be able to set any kind of filter for it, so for syscalls unknown by libseccomp we would always do the default action, i.e. either return the errno set by SystemCallErrorNumber or send a fatal signal. For glibc to ignore the unknown syscall and gracefully fall back to the older implementation, we need to return ENOSYS. In particular, tar now fails with the default SystemCallFilter="@system-service" sandbox [3]. This is of course a wider problem: any time the kernel gains new syscalls, before libseccomp and systemd have caught up, we'd behave incorrectly. Let's do the same as we already were doing in nspawn since 3573e032f26724949e86626eace058d006b8bf70, and do the "default action" only for syscalls which are known by us and libseccomp, and return ENOSYS for anything else. This means that users can start using a sandbox with the new syscalls only after libseccomp and systemd have been updated, but before that happens they behaviour that is backwards-compatible. [1] https://github.com/bminor/glibc/commit/65341f7bbea824d2ff9d37db15d8be162df42bd3 [2] https://github.com/seccomp/libseccomp/issues/406 [2] https://github.com/systemd/systemd/issues/30250 Fixes https://github.com/systemd/systemd/issues/30250. In seccomp_restrict_sxid() there's a chunk conditionalized with '#if defined(__SNR_fchmodat2)'. We need to kep that because seccomp_restrict_sxid() seccomp_restrict_suid_sgid() uses SCMP_ACT_ALLOW as the default action. --- src/shared/seccomp-util.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c index bb970d5264..95c704d225 100644 --- a/src/shared/seccomp-util.c +++ b/src/shared/seccomp-util.c @@ -1129,7 +1129,9 @@ int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* filter log_trace("Operating on architecture: %s", seccomp_arch_to_string(arch)); - r = seccomp_init_for_arch(&seccomp, arch, default_action); + /* We install ENOSYS as the default action, but it will only apply to syscalls which are not + * in the @known set. */ + r = seccomp_init_for_arch(&seccomp, arch, SCMP_ACT_ERRNO(ENOSYS)); if (r < 0) return r; @@ -1164,6 +1166,23 @@ int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* filter } } + NULSTR_FOREACH(name, syscall_filter_sets[SYSCALL_FILTER_SET_KNOWN].value) { + int id; + + id = seccomp_syscall_resolve_name(name); + if (id < 0) + continue; + + /* Ignore the syscall if it was already handled above */ + if (hashmap_contains(filter, INT_TO_PTR(id + 1))) + continue; + + r = seccomp_rule_add_exact(seccomp, default_action, id, 0); + if (r < 0 && r != -EDOM) /* EDOM means that the syscall is not available for arch */ + return log_debug_errno(r, "Failed to add rule for system call %s() / %d: %m", + name, id); + } + r = seccomp_load(seccomp); if (ERRNO_IS_NEG_SECCOMP_FATAL(r)) return r; From 309a4212cf53193051b56e194200d54db6edeb1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 1 Dec 2023 19:04:27 +0100 Subject: [PATCH 3/4] shared/seccomp-util: use the same error message for the same condition We were calling seccomp_syscall_resolve_name three times and using a slightly different error message in each of the cases. --- src/shared/seccomp-util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c index 95c704d225..d85275dbc6 100644 --- a/src/shared/seccomp-util.c +++ b/src/shared/seccomp-util.c @@ -1242,7 +1242,7 @@ int seccomp_parse_syscall_filter( return -EINVAL; log_syntax(unit, FLAGS_SET(flags, SECCOMP_PARSE_LOG) ? LOG_WARNING : LOG_DEBUG, filename, line, 0, - "Failed to parse system call, ignoring: %s", name); + "System call %s is not known, ignoring.", name); return 0; } @@ -2000,7 +2000,7 @@ int seccomp_filter_set_add(Hashmap *filter, bool add, const SyscallFilterSet *se id = seccomp_syscall_resolve_name(i); if (id == __NR_SCMP_ERROR) { - log_debug("Couldn't resolve system call, ignoring: %s", i); + log_debug("System call %s is not known, ignoring.", i); continue; } From e6c5386dee48c07d1f9a959ab0f3fc0e1ae43649 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 1 Dec 2023 23:13:54 +0100 Subject: [PATCH 4/4] core: turn on higher optimization level in seccomp This mirrors what d75615f398a0fbf986cf03924462863ca6ee2f9f did for nspawn. It isn't really a fatal failure if we can't set that, so ignore it in libseccomp cannot set the attribute. line OP JT JF K ================================= 0000: 0x20 0x00 0x00 0x00000004 ld $data[4] 0001: 0x15 0x00 0xb7 0x40000003 jeq 1073741827 true:0002 false:0185 0002: 0x20 0x00 0x00 0x00000000 ld $data[0] 0003: 0x15 0xb5 0x00 0x00000000 jeq 0 true:0185 false:0004 0004: 0x15 0xb4 0x00 0x00000001 jeq 1 true:0185 false:0005 0005: 0x15 0xb3 0x00 0x00000002 jeq 2 true:0185 false:0006 0006: 0x15 0xb2 0x00 0x00000003 jeq 3 true:0185 false:0007 0007: 0x15 0xb1 0x00 0x00000004 jeq 4 true:0185 false:0008 0008: 0x15 0xb0 0x00 0x00000005 jeq 5 true:0185 false:0009 0009: 0x15 0xaf 0x00 0x00000006 jeq 6 true:0185 false:0010 ... 0438: 0x15 0x03 0x00 0x000001be jeq 446 true:0442 false:0439 0439: 0x15 0x02 0x00 0x000001bf jeq 447 true:0442 false:0440 0440: 0x15 0x01 0x00 0x000001c0 jeq 448 true:0442 false:0441 0441: 0x06 0x00 0x00 0x00050026 ret ERRNO(38) 0442: 0x06 0x00 0x00 0x7fff0000 ret ALLOW line OP JT JF K ================================= 0000: 0x20 0x00 0x00 0x00000004 ld $data[4] 0001: 0x15 0x00 0x27 0x40000003 jeq 1073741827 true:0002 false:0041 0002: 0x20 0x00 0x00 0x00000000 ld $data[0] 0003: 0x25 0x01 0x00 0x000000b5 jgt 181 true:0005 false:0004 0004: 0x05 0x00 0x00 0x00000143 jmp 0328 0005: 0x25 0x00 0xa1 0x00000139 jgt 313 true:0006 false:0167 0006: 0x25 0x00 0x51 0x00000179 jgt 377 true:0007 false:0088 0007: 0x25 0x00 0x29 0x000001a0 jgt 416 true:0008 false:0049 0008: 0x25 0x00 0x13 0x000001b0 jgt 432 true:0009 false:0028 0009: 0x25 0x00 0x09 0x000001b8 jgt 440 true:0010 false:0019 ... 0551: 0x15 0x03 0x00 0x00000002 jeq 2 true:0555 false:0552 0552: 0x15 0x02 0x01 0x00000001 jeq 1 true:0555 false:0554 0553: 0x15 0x01 0x00 0x00000000 jeq 0 true:0555 false:0554 0554: 0x06 0x00 0x00 0x00050026 ret ERRNO(38) 0555: 0x06 0x00 0x00 0x7fff0000 ret ALLOW The program is longer but hopefully faster because of the binary search. --- src/nspawn/nspawn-seccomp.c | 2 +- src/shared/seccomp-util.c | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/nspawn/nspawn-seccomp.c b/src/nspawn/nspawn-seccomp.c index 34a8627848..fa05a8a5b4 100644 --- a/src/nspawn/nspawn-seccomp.c +++ b/src/nspawn/nspawn-seccomp.c @@ -169,7 +169,7 @@ static int add_syscall_filters( /* We have a large filter here, so let's turn on the binary tree mode if possible. */ r = seccomp_attr_set(ctx, SCMP_FLTATR_CTL_OPTIMIZE, 2); if (r < 0) - return r; + log_warning_errno(r, "Failed to set SCMP_FLTATR_CTL_OPTIMIZE, ignoring: %m"); #endif return 0; diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c index d85275dbc6..cd30e72ad5 100644 --- a/src/shared/seccomp-util.c +++ b/src/shared/seccomp-util.c @@ -1183,6 +1183,13 @@ int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* filter name, id); } +#if (SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR >= 5) || SCMP_VER_MAJOR > 2 + /* We have a large filter here, so let's turn on the binary tree mode if possible. */ + r = seccomp_attr_set(seccomp, SCMP_FLTATR_CTL_OPTIMIZE, 2); + if (r < 0) + log_warning_errno(r, "Failed to set SCMP_FLTATR_CTL_OPTIMIZE, ignoring: %m"); +#endif + r = seccomp_load(seccomp); if (ERRNO_IS_NEG_SECCOMP_FATAL(r)) return r;