From 23e12f8e6cbdd094db3a607534b42760ca7804b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 21 Sep 2018 14:12:04 +0200 Subject: [PATCH 1/4] test-seccomp: move two similar tests closer --- src/test/test-seccomp.c | 62 ++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c index b2ac392dac..37815af6a4 100644 --- a/src/test/test-seccomp.c +++ b/src/test/test-seccomp.c @@ -133,6 +133,36 @@ static void test_filter_sets(void) { } } +static void test_filter_sets_ordered(void) { + size_t i; + + /* Ensure "@default" always remains at the beginning of the list */ + assert_se(SYSCALL_FILTER_SET_DEFAULT == 0); + assert_se(streq(syscall_filter_sets[0].name, "@default")); + + for (i = 0; i < _SYSCALL_FILTER_SET_MAX; i++) { + const char *k, *p = NULL; + + /* Make sure each group has a description */ + assert_se(!isempty(syscall_filter_sets[0].help)); + + /* Make sure the groups are ordered alphabetically, except for the first entry */ + assert_se(i < 2 || strcmp(syscall_filter_sets[i-1].name, syscall_filter_sets[i].name) < 0); + + NULSTR_FOREACH(k, syscall_filter_sets[i].value) { + + /* Ensure each syscall list is in itself ordered, but groups before names */ + assert_se(!p || + (*p == '@' && *k != '@') || + (((*p == '@' && *k == '@') || + (*p != '@' && *k != '@')) && + strcmp(p, k) < 0)); + + p = k; + } + } +} + static void test_restrict_namespace(void) { char *s = NULL; unsigned long ul; @@ -683,36 +713,6 @@ static void test_lock_personality(void) { assert_se(wait_for_terminate_and_check("lockpersonalityseccomp", pid, WAIT_LOG) == EXIT_SUCCESS); } -static void test_filter_sets_ordered(void) { - size_t i; - - /* Ensure "@default" always remains at the beginning of the list */ - assert_se(SYSCALL_FILTER_SET_DEFAULT == 0); - assert_se(streq(syscall_filter_sets[0].name, "@default")); - - for (i = 0; i < _SYSCALL_FILTER_SET_MAX; i++) { - const char *k, *p = NULL; - - /* Make sure each group has a description */ - assert_se(!isempty(syscall_filter_sets[0].help)); - - /* Make sure the groups are ordered alphabetically, except for the first entry */ - assert_se(i < 2 || strcmp(syscall_filter_sets[i-1].name, syscall_filter_sets[i].name) < 0); - - NULSTR_FOREACH(k, syscall_filter_sets[i].value) { - - /* Ensure each syscall list is in itself ordered, but groups before names */ - assert_se(!p || - (*p == '@' && *k != '@') || - (((*p == '@' && *k == '@') || - (*p != '@' && *k != '@')) && - strcmp(p, k) < 0)); - - p = k; - } - } -} - int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); @@ -721,6 +721,7 @@ int main(int argc, char *argv[]) { test_architecture_table(); test_syscall_filter_set_find(); test_filter_sets(); + test_filter_sets_ordered(); test_restrict_namespace(); test_protect_sysctl(); test_restrict_address_families(); @@ -730,7 +731,6 @@ int main(int argc, char *argv[]) { test_restrict_archs(); test_load_syscall_filter_set_raw(); test_lock_personality(); - test_filter_sets_ordered(); return 0; } From f09da7ccbc67efd4e1a7ac7f3bc1356fad27fc40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 21 Sep 2018 14:14:45 +0200 Subject: [PATCH 2/4] test-seccomp: log function names Various tests produce similar output, and the function names make it easier to see where the output is generated. --- src/test/test-seccomp.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c index 37815af6a4..5b2dfd766e 100644 --- a/src/test/test-seccomp.c +++ b/src/test/test-seccomp.c @@ -36,6 +36,8 @@ static void test_seccomp_arch_to_string(void) { uint32_t a, b; const char *name; + log_info("/* %s */", __func__); + a = seccomp_arch_native(); assert_se(a > 0); name = seccomp_arch_to_string(a); @@ -47,6 +49,8 @@ static void test_seccomp_arch_to_string(void) { static void test_architecture_table(void) { const char *n, *n2; + log_info("/* %s */", __func__); + NULSTR_FOREACH(n, "native\0" "x86\0" @@ -75,6 +79,8 @@ static void test_architecture_table(void) { } static void test_syscall_filter_set_find(void) { + log_info("/* %s */", __func__); + assert_se(!syscall_filter_set_find(NULL)); assert_se(!syscall_filter_set_find("")); assert_se(!syscall_filter_set_find("quux")); @@ -89,6 +95,8 @@ static void test_filter_sets(void) { unsigned i; int r; + log_info("/* %s */", __func__); + if (!is_seccomp_available()) { log_notice("Seccomp not available, skipping %s", __func__); return; @@ -136,6 +144,8 @@ static void test_filter_sets(void) { static void test_filter_sets_ordered(void) { size_t i; + log_info("/* %s */", __func__); + /* Ensure "@default" always remains at the beginning of the list */ assert_se(SYSCALL_FILTER_SET_DEFAULT == 0); assert_se(streq(syscall_filter_sets[0].name, "@default")); @@ -168,6 +178,8 @@ static void test_restrict_namespace(void) { unsigned long ul; pid_t pid; + log_info("/* %s */", __func__); + assert_se(namespace_flags_to_string(0, &s) == 0 && streq(s, "")); s = mfree(s); assert_se(namespace_flags_to_string(CLONE_NEWNS, &s) == 0 && streq(s, "mnt")); @@ -262,6 +274,8 @@ static void test_restrict_namespace(void) { static void test_protect_sysctl(void) { pid_t pid; + log_info("/* %s */", __func__); + if (!is_seccomp_available()) { log_notice("Seccomp not available, skipping %s", __func__); return; @@ -302,6 +316,8 @@ static void test_protect_sysctl(void) { static void test_restrict_address_families(void) { pid_t pid; + log_info("/* %s */", __func__); + if (!is_seccomp_available()) { log_notice("Seccomp not available, skipping %s", __func__); return; @@ -389,6 +405,8 @@ static void test_restrict_address_families(void) { static void test_restrict_realtime(void) { pid_t pid; + log_info("/* %s */", __func__); + if (!is_seccomp_available()) { log_notice("Seccomp not available, skipping %s", __func__); return; @@ -434,6 +452,8 @@ static void test_restrict_realtime(void) { static void test_memory_deny_write_execute_mmap(void) { pid_t pid; + log_info("/* %s */", __func__); + if (!is_seccomp_available()) { log_notice("Seccomp not available, skipping %s", __func__); return; @@ -482,6 +502,8 @@ static void test_memory_deny_write_execute_shmat(void) { int shmid; pid_t pid; + log_info("/* %s */", __func__); + if (!is_seccomp_available()) { log_notice("Seccomp not available, skipping %s", __func__); return; @@ -532,6 +554,8 @@ static void test_memory_deny_write_execute_shmat(void) { static void test_restrict_archs(void) { pid_t pid; + log_info("/* %s */", __func__); + if (!is_seccomp_available()) { log_notice("Seccomp not available, skipping %s", __func__); return; @@ -570,6 +594,8 @@ static void test_restrict_archs(void) { static void test_load_syscall_filter_set_raw(void) { pid_t pid; + log_info("/* %s */", __func__); + if (!is_seccomp_available()) { log_notice("Seccomp not available, skipping %s", __func__); return; @@ -666,6 +692,8 @@ static void test_lock_personality(void) { unsigned long current; pid_t pid; + log_info("/* %s */", __func__); + if (!is_seccomp_available()) { log_notice("Seccomp not available, skipping %s", __func__); return; @@ -714,7 +742,6 @@ static void test_lock_personality(void) { } int main(int argc, char *argv[]) { - test_setup_logging(LOG_DEBUG); test_seccomp_arch_to_string(); From b54f36c604472ffe08830ec4306fa2885b4a5424 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 24 Sep 2018 16:59:12 +0200 Subject: [PATCH 3/4] seccomp: reduce logging about failure to add syscall to seccomp Our logs are full of: Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call oldstat() / -10037, ignoring: Numerical argument out of domain Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call get_thread_area() / -10076, ignoring: Numerical argument out of domain Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call set_thread_area() / -10079, ignoring: Numerical argument out of domain Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call oldfstat() / -10034, ignoring: Numerical argument out of domain Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call oldolduname() / -10036, ignoring: Numerical argument out of domain Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call oldlstat() / -10035, ignoring: Numerical argument out of domain Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call waitpid() / -10073, ignoring: Numerical argument out of domain ... This is pointless and makes debug logs hard to read. Let's keep the logs in test code, but disable it in nspawn and pid1. This is done through a function parameter because those functions operate recursively and it's not possible to make the caller to log meaningfully. There should be no functional change, except the skipped debug logs. --- src/core/execute.c | 6 ++-- src/nspawn/nspawn-seccomp.c | 4 +-- src/shared/seccomp-util.c | 57 ++++++++++++++++++++----------------- src/shared/seccomp-util.h | 6 ++-- src/test/test-seccomp.c | 16 +++++------ 5 files changed, 47 insertions(+), 42 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index 35dd3898da..17c459fe43 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1432,7 +1432,7 @@ static int apply_syscall_filter(const Unit* u, const ExecContext *c, bool needs_ return r; } - return seccomp_load_syscall_filter_set_raw(default_action, c->syscall_filter, action); + return seccomp_load_syscall_filter_set_raw(default_action, c->syscall_filter, action, false); } static int apply_syscall_archs(const Unit *u, const ExecContext *c) { @@ -1515,7 +1515,7 @@ static int apply_protect_kernel_modules(const Unit *u, const ExecContext *c) { if (skip_seccomp_unavailable(u, "ProtectKernelModules=")) return 0; - return seccomp_load_syscall_filter_set(SCMP_ACT_ALLOW, syscall_filter_sets + SYSCALL_FILTER_SET_MODULE, SCMP_ACT_ERRNO(EPERM)); + return seccomp_load_syscall_filter_set(SCMP_ACT_ALLOW, syscall_filter_sets + SYSCALL_FILTER_SET_MODULE, SCMP_ACT_ERRNO(EPERM), false); } static int apply_private_devices(const Unit *u, const ExecContext *c) { @@ -1530,7 +1530,7 @@ static int apply_private_devices(const Unit *u, const ExecContext *c) { if (skip_seccomp_unavailable(u, "PrivateDevices=")) return 0; - return seccomp_load_syscall_filter_set(SCMP_ACT_ALLOW, syscall_filter_sets + SYSCALL_FILTER_SET_RAW_IO, SCMP_ACT_ERRNO(EPERM)); + return seccomp_load_syscall_filter_set(SCMP_ACT_ALLOW, syscall_filter_sets + SYSCALL_FILTER_SET_RAW_IO, SCMP_ACT_ERRNO(EPERM), false); } static int apply_restrict_namespaces(const Unit *u, const ExecContext *c) { diff --git a/src/nspawn/nspawn-seccomp.c b/src/nspawn/nspawn-seccomp.c index eb1964bb6d..b56c5b04a8 100644 --- a/src/nspawn/nspawn-seccomp.c +++ b/src/nspawn/nspawn-seccomp.c @@ -148,7 +148,7 @@ static int seccomp_add_default_syscall_filter( if (whitelist[i].capability != 0 && (cap_list_retain & (1ULL << whitelist[i].capability)) == 0) continue; - r = seccomp_add_syscall_filter_item(ctx, whitelist[i].name, SCMP_ACT_ALLOW, syscall_blacklist); + r = seccomp_add_syscall_filter_item(ctx, whitelist[i].name, SCMP_ACT_ALLOW, syscall_blacklist, false); if (r < 0) /* If the system call is not known on this architecture, then that's fine, let's ignore it */ log_debug_errno(r, "Failed to add rule for system call %s on %s, ignoring: %m", whitelist[i].name, seccomp_arch_to_string(arch)); @@ -157,7 +157,7 @@ static int seccomp_add_default_syscall_filter( } STRV_FOREACH(p, syscall_whitelist) { - r = seccomp_add_syscall_filter_item(ctx, *p, SCMP_ACT_ALLOW, syscall_blacklist); + r = seccomp_add_syscall_filter_item(ctx, *p, SCMP_ACT_ALLOW, syscall_blacklist, false); if (r < 0) log_debug_errno(r, "Failed to add rule for system call %s on %s, ignoring: %m", *p, seccomp_arch_to_string(arch)); else diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c index ff3537c5e9..c69b0e82c6 100644 --- a/src/shared/seccomp-util.c +++ b/src/shared/seccomp-util.c @@ -858,11 +858,9 @@ const SyscallFilterSet *syscall_filter_set_find(const char *name) { return NULL; } -static int seccomp_add_syscall_filter_set(scmp_filter_ctx seccomp, const SyscallFilterSet *set, uint32_t action, char **exclude); - -int seccomp_add_syscall_filter_item(scmp_filter_ctx *seccomp, const char *name, uint32_t action, char **exclude) { - int r; +static int seccomp_add_syscall_filter_set(scmp_filter_ctx seccomp, const SyscallFilterSet *set, uint32_t action, char **exclude, bool log_missing); +int seccomp_add_syscall_filter_item(scmp_filter_ctx *seccomp, const char *name, uint32_t action, char **exclude, bool log_missing) { assert(seccomp); assert(name); @@ -878,32 +876,36 @@ int seccomp_add_syscall_filter_item(scmp_filter_ctx *seccomp, const char *name, return -EINVAL; } - r = seccomp_add_syscall_filter_set(seccomp, other, action, exclude); - if (r < 0) - return r; + return seccomp_add_syscall_filter_set(seccomp, other, action, exclude, log_missing); + } else { - int id; + int id, r; id = seccomp_syscall_resolve_name(name); if (id == __NR_SCMP_ERROR) { - log_debug("System call %s is not known, ignoring.", name); + if (log_missing) + log_debug("System call %s is not known, ignoring.", name); return 0; } r = seccomp_rule_add_exact(seccomp, action, id, 0); - if (r < 0) + if (r < 0) { /* If the system call is not known on this architecture, then that's fine, let's ignore it */ - log_debug_errno(r, "Failed to add rule for system call %s() / %d, ignoring: %m", name, id); - } + if (log_missing) + log_debug_errno(r, "Failed to add rule for system call %s() / %d, ignoring: %m", + name, id); + } - return 0; + return 0; + } } static int seccomp_add_syscall_filter_set( scmp_filter_ctx seccomp, const SyscallFilterSet *set, uint32_t action, - char **exclude) { + char **exclude, + bool log_missing) { const char *sys; int r; @@ -912,7 +914,7 @@ static int seccomp_add_syscall_filter_set( assert(set); NULSTR_FOREACH(sys, set->value) { - r = seccomp_add_syscall_filter_item(seccomp, sys, action, exclude); + r = seccomp_add_syscall_filter_item(seccomp, sys, action, exclude, log_missing); if (r < 0) return r; } @@ -920,7 +922,7 @@ static int seccomp_add_syscall_filter_set( return 0; } -int seccomp_load_syscall_filter_set(uint32_t default_action, const SyscallFilterSet *set, uint32_t action) { +int seccomp_load_syscall_filter_set(uint32_t default_action, const SyscallFilterSet *set, uint32_t action, bool log_missing) { uint32_t arch; int r; @@ -938,7 +940,7 @@ int seccomp_load_syscall_filter_set(uint32_t default_action, const SyscallFilter if (r < 0) return r; - r = seccomp_add_syscall_filter_set(seccomp, set, action, NULL); + r = seccomp_add_syscall_filter_set(seccomp, set, action, NULL, log_missing); if (r < 0) { log_debug_errno(r, "Failed to add filter set, ignoring: %m"); continue; @@ -954,7 +956,7 @@ int seccomp_load_syscall_filter_set(uint32_t default_action, const SyscallFilter return 0; } -int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* set, uint32_t action) { +int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* set, uint32_t action, bool log_missing) { uint32_t arch; int r; @@ -967,7 +969,7 @@ int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* set, u SECCOMP_FOREACH_LOCAL_ARCH(arch) { _cleanup_(seccomp_releasep) scmp_filter_ctx seccomp = NULL; Iterator i; - void *id, *val; + void *syscall_id, *val; log_debug("Operating on architecture: %s", seccomp_arch_to_string(arch)); @@ -975,20 +977,23 @@ int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* set, u if (r < 0) return r; - HASHMAP_FOREACH_KEY(val, id, set, i) { + HASHMAP_FOREACH_KEY(val, syscall_id, set, i) { uint32_t a = action; - int e = PTR_TO_INT(val); + int id = PTR_TO_INT(syscall_id) - 1; + int error = PTR_TO_INT(val); - if (action != SCMP_ACT_ALLOW && e >= 0) - a = SCMP_ACT_ERRNO(e); + if (action != SCMP_ACT_ALLOW && error >= 0) + a = SCMP_ACT_ERRNO(error); - r = seccomp_rule_add_exact(seccomp, a, PTR_TO_INT(id) - 1, 0); + r = seccomp_rule_add_exact(seccomp, a, id, 0); if (r < 0) { /* If the system call is not known on this architecture, then that's fine, let's ignore it */ _cleanup_free_ char *n = NULL; - n = seccomp_syscall_resolve_num_arch(SCMP_ARCH_NATIVE, PTR_TO_INT(id) - 1); - log_debug_errno(r, "Failed to add rule for system call %s() / %d, ignoring: %m", strna(n), PTR_TO_INT(id) - 1); + n = seccomp_syscall_resolve_num_arch(SCMP_ARCH_NATIVE, id); + if (log_missing) + log_debug_errno(r, "Failed to add rule for system call %s() / %d, ignoring: %m", + strna(n), id); } } diff --git a/src/shared/seccomp-util.h b/src/shared/seccomp-util.h index eac857afb9..d8a36c4e21 100644 --- a/src/shared/seccomp-util.h +++ b/src/shared/seccomp-util.h @@ -58,10 +58,10 @@ const SyscallFilterSet *syscall_filter_set_find(const char *name); int seccomp_filter_set_add(Hashmap *s, bool b, const SyscallFilterSet *set); -int seccomp_add_syscall_filter_item(scmp_filter_ctx *ctx, const char *name, uint32_t action, char **exclude); +int seccomp_add_syscall_filter_item(scmp_filter_ctx *ctx, const char *name, uint32_t action, char **exclude, bool log_missing); -int seccomp_load_syscall_filter_set(uint32_t default_action, const SyscallFilterSet *set, uint32_t action); -int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* set, uint32_t action); +int seccomp_load_syscall_filter_set(uint32_t default_action, const SyscallFilterSet *set, uint32_t action, bool log_missing); +int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* set, uint32_t action, bool log_missing); typedef enum SeccompParseFlags { SECCOMP_PARSE_INVERT = 1 << 0, diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c index 5b2dfd766e..00cd216a4b 100644 --- a/src/test/test-seccomp.c +++ b/src/test/test-seccomp.c @@ -117,11 +117,11 @@ static void test_filter_sets(void) { if (pid == 0) { /* Child? */ int fd; - /* if we look at the default set (or one that includes it), whitelist instead of blacklist */ + /* If we look at the default set (or one that includes it), whitelist instead of blacklist */ if (IN_SET(i, SYSCALL_FILTER_SET_DEFAULT, SYSCALL_FILTER_SET_SYSTEM_SERVICE)) - r = seccomp_load_syscall_filter_set(SCMP_ACT_ERRNO(EUCLEAN), syscall_filter_sets + i, SCMP_ACT_ALLOW); + r = seccomp_load_syscall_filter_set(SCMP_ACT_ERRNO(EUCLEAN), syscall_filter_sets + i, SCMP_ACT_ALLOW, true); else - r = seccomp_load_syscall_filter_set(SCMP_ACT_ALLOW, syscall_filter_sets + i, SCMP_ACT_ERRNO(EUCLEAN)); + r = seccomp_load_syscall_filter_set(SCMP_ACT_ALLOW, syscall_filter_sets + i, SCMP_ACT_ERRNO(EUCLEAN), true); if (r < 0) _exit(EXIT_FAILURE); @@ -614,7 +614,7 @@ static void test_load_syscall_filter_set_raw(void) { 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) >= 0); + assert_se(seccomp_load_syscall_filter_set_raw(SCMP_ACT_ALLOW, NULL, SCMP_ACT_KILL, true) >= 0); assert_se(access("/", F_OK) >= 0); assert_se(poll(NULL, 0, 0) == 0); @@ -625,7 +625,7 @@ static void test_load_syscall_filter_set_raw(void) { assert_se(hashmap_put(s, UINT32_TO_PTR(__NR_faccessat + 1), INT_TO_PTR(-1)) >= 0); #endif - assert_se(seccomp_load_syscall_filter_set_raw(SCMP_ACT_ALLOW, s, SCMP_ACT_ERRNO(EUCLEAN)) >= 0); + 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); @@ -641,7 +641,7 @@ static void test_load_syscall_filter_set_raw(void) { assert_se(hashmap_put(s, UINT32_TO_PTR(__NR_faccessat + 1), INT_TO_PTR(EILSEQ)) >= 0); #endif - assert_se(seccomp_load_syscall_filter_set_raw(SCMP_ACT_ALLOW, s, SCMP_ACT_ERRNO(EUCLEAN)) >= 0); + 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 == EILSEQ); @@ -657,7 +657,7 @@ static void test_load_syscall_filter_set_raw(void) { assert_se(hashmap_put(s, UINT32_TO_PTR(__NR_ppoll + 1), INT_TO_PTR(-1)) >= 0); #endif - assert_se(seccomp_load_syscall_filter_set_raw(SCMP_ACT_ALLOW, s, SCMP_ACT_ERRNO(EUNATCH)) >= 0); + assert_se(seccomp_load_syscall_filter_set_raw(SCMP_ACT_ALLOW, s, SCMP_ACT_ERRNO(EUNATCH), true) >= 0); assert_se(access("/", F_OK) < 0); assert_se(errno == EILSEQ); @@ -674,7 +674,7 @@ static void test_load_syscall_filter_set_raw(void) { assert_se(hashmap_put(s, UINT32_TO_PTR(__NR_ppoll + 1), INT_TO_PTR(EILSEQ)) >= 0); #endif - assert_se(seccomp_load_syscall_filter_set_raw(SCMP_ACT_ALLOW, s, SCMP_ACT_ERRNO(EUNATCH)) >= 0); + assert_se(seccomp_load_syscall_filter_set_raw(SCMP_ACT_ALLOW, s, SCMP_ACT_ERRNO(EUNATCH), true) >= 0); assert_se(access("/", F_OK) < 0); assert_se(errno == EILSEQ); From 7e86bd73a47f2b8dd3d9a743e69fb0117f450ad8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 20 Sep 2018 14:19:41 +0200 Subject: [PATCH 4/4] seccomp: tighten checking of seccomp filter creation In seccomp code, the code is changed to propagate errors which are about anything other than unknown/unimplemented syscalls. I *think* such errors should not happen in normal usage, but so far we would summarilly ignore all errors, so that part is uncertain. If it turns out that other errors occur and should be ignored, this should be added later. In nspawn, we would count the number of added filters, but didn't use this for anything. Drop that part. The comments suggested that seccomp_add_syscall_filter_item() returned negative if the syscall is unknown, but this wasn't true: it returns 0. The error at this point can only be if the syscall was known but couldn't be added. If the error comes from our internal whitelist in nspawn, treat this as error, because it means that our internal table is wrong. If the error comes from user arguments, warn and ignore. (If some syscall is not known at current architecture, it is still silently ignored.) --- src/nspawn/nspawn-seccomp.c | 14 +++++--------- src/shared/seccomp-util.c | 26 ++++++++++++++++---------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/nspawn/nspawn-seccomp.c b/src/nspawn/nspawn-seccomp.c index b56c5b04a8..e7ef80f7d6 100644 --- a/src/nspawn/nspawn-seccomp.c +++ b/src/nspawn/nspawn-seccomp.c @@ -140,7 +140,7 @@ static int seccomp_add_default_syscall_filter( */ }; - int r, c = 0; + int r; size_t i; char **p; @@ -150,21 +150,17 @@ static int seccomp_add_default_syscall_filter( r = seccomp_add_syscall_filter_item(ctx, whitelist[i].name, SCMP_ACT_ALLOW, syscall_blacklist, false); if (r < 0) - /* If the system call is not known on this architecture, then that's fine, let's ignore it */ - log_debug_errno(r, "Failed to add rule for system call %s on %s, ignoring: %m", whitelist[i].name, seccomp_arch_to_string(arch)); - else - c++; + return log_error_errno(r, "Failed to add syscall filter item %s: %m", whitelist[i].name); } STRV_FOREACH(p, syscall_whitelist) { r = seccomp_add_syscall_filter_item(ctx, *p, SCMP_ACT_ALLOW, syscall_blacklist, false); if (r < 0) - log_debug_errno(r, "Failed to add rule for system call %s on %s, ignoring: %m", *p, seccomp_arch_to_string(arch)); - else - c++; + log_warning_errno(r, "Failed to add rule for system call %s on %s, ignoring: %m", + *p, seccomp_arch_to_string(arch)); } - return c; + return 0; } int setup_seccomp(uint64_t cap_list_retain, char **syscall_whitelist, char **syscall_blacklist) { diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c index c69b0e82c6..ca55441466 100644 --- a/src/shared/seccomp-util.c +++ b/src/shared/seccomp-util.c @@ -891,9 +891,13 @@ int seccomp_add_syscall_filter_item(scmp_filter_ctx *seccomp, const char *name, r = seccomp_rule_add_exact(seccomp, action, id, 0); if (r < 0) { /* If the system call is not known on this architecture, then that's fine, let's ignore it */ - if (log_missing) - log_debug_errno(r, "Failed to add rule for system call %s() / %d, ignoring: %m", - name, id); + bool ignore = r == -EDOM; + + if (!ignore || log_missing) + log_debug_errno(r, "Failed to add rule for system call %s() / %d%s: %m", + name, id, ignore ? ", ignoring" : ""); + if (!ignore) + return r; } return 0; @@ -941,10 +945,8 @@ int seccomp_load_syscall_filter_set(uint32_t default_action, const SyscallFilter return r; r = seccomp_add_syscall_filter_set(seccomp, set, action, NULL, log_missing); - if (r < 0) { - log_debug_errno(r, "Failed to add filter set, ignoring: %m"); - continue; - } + if (r < 0) + return log_debug_errno(r, "Failed to add filter set: %m"); r = seccomp_load(seccomp); if (IN_SET(r, -EPERM, -EACCES)) @@ -989,11 +991,15 @@ int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* set, u if (r < 0) { /* If the system call is not known on this architecture, then that's fine, let's ignore it */ _cleanup_free_ char *n = NULL; + bool ignore; n = seccomp_syscall_resolve_num_arch(SCMP_ARCH_NATIVE, id); - if (log_missing) - log_debug_errno(r, "Failed to add rule for system call %s() / %d, ignoring: %m", - strna(n), id); + ignore = r == -EDOM; + if (!ignore || log_missing) + log_debug_errno(r, "Failed to add rule for system call %s() / %d%s: %m", + strna(n), id, ignore ? ", ignoring" : ""); + if (!ignore) + return r; } }