diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index b664bbffa67..4a1585f6633 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -2252,6 +2252,9 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; + if (allow_list && e >= 0) + return -EINVAL; + r = seccomp_parse_syscall_filter(n, e, c->syscall_filter, @@ -2315,15 +2318,8 @@ int bus_exec_context_set_transient_property( } STRV_FOREACH(s, l) { - _cleanup_free_ char *n = NULL; - int e; - - r = parse_syscall_and_errno(*s, &n, &e); - if (r < 0) - return r; - - r = seccomp_parse_syscall_filter(n, - 0, /* errno not used */ + r = seccomp_parse_syscall_filter(*s, + -1, /* errno not used */ c->syscall_log, SECCOMP_PARSE_LOG | SECCOMP_PARSE_PERMISSIVE | invert_flag | diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index f9c334cc65e..c6fc4fe083f 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3209,13 +3209,20 @@ int config_parse_syscall_filter( if (r == -ENOMEM) return log_oom(); if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, "Invalid syntax, ignoring: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, r, + "Invalid syntax, ignoring: %s", rvalue); return 0; } r = parse_syscall_and_errno(word, &name, &num); if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse syscall:errno, ignoring: %s", word); + log_syntax(unit, LOG_WARNING, filename, line, r, + "Failed to parse syscall:errno, ignoring: %s", word); + continue; + } + if (!invert && num >= 0) { + log_syntax(unit, LOG_WARNING, filename, line, 0, + "Allow-listed system calls cannot take error number, ignoring: %s", word); continue; } @@ -3280,8 +3287,7 @@ int config_parse_syscall_log( p = rvalue; for (;;) { - _cleanup_free_ char *word = NULL, *name = NULL; - int num; + _cleanup_free_ char *word = NULL; r = extract_first_word(&p, &word, NULL, 0); if (r == 0) @@ -3293,14 +3299,8 @@ int config_parse_syscall_log( return 0; } - r = parse_syscall_and_errno(word, &name, &num); - if (r < 0 || num >= 0) { /* errno code not allowed */ - log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse syscall, ignoring: %s", word); - continue; - } - r = seccomp_parse_syscall_filter( - name, 0, c->syscall_log, + word, -1, c->syscall_log, SECCOMP_PARSE_LOG|SECCOMP_PARSE_PERMISSIVE| (invert ? SECCOMP_PARSE_INVERT : 0)| (c->syscall_log_allow_list ? SECCOMP_PARSE_ALLOW_LIST : 0), diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c index ab24baaf9e6..9813d82f956 100644 --- a/src/shared/seccomp-util.c +++ b/src/shared/seccomp-util.c @@ -1059,14 +1059,14 @@ 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, bool log_missing) { +int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* filter, uint32_t action, bool log_missing) { uint32_t arch; int r; - /* Similar to seccomp_load_syscall_filter_set(), but takes a raw Set* of syscalls, instead of a - * SyscallFilterSet* table. */ + /* Similar to seccomp_load_syscall_filter_set(), but takes a raw Hashmap* of syscalls, instead + * of a SyscallFilterSet* table. */ - if (hashmap_isempty(set) && default_action == SCMP_ACT_ALLOW) + if (hashmap_isempty(filter) && default_action == SCMP_ACT_ALLOW) return 0; SECCOMP_FOREACH_LOCAL_ARCH(arch) { @@ -1079,7 +1079,7 @@ int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* set, u if (r < 0) return r; - HASHMAP_FOREACH_KEY(val, syscall_id, set) { + HASHMAP_FOREACH_KEY(val, syscall_id, filter) { uint32_t a = action; int id = PTR_TO_INT(syscall_id) - 1; int error = PTR_TO_INT(val); @@ -1090,12 +1090,13 @@ int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* set, u else if (action == SCMP_ACT_LOG) a = SCMP_ACT_LOG; #endif - else if (action != SCMP_ACT_ALLOW && error >= 0) + else if (error >= 0) a = SCMP_ACT_ERRNO(error); 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 */ + /* 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; @@ -1113,7 +1114,8 @@ int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* set, u if (ERRNO_IS_SECCOMP_FATAL(r)) return r; if (r < 0) - log_debug_errno(r, "Failed to install filter set for architecture %s, skipping: %m", seccomp_arch_to_string(arch)); + log_debug_errno(r, "Failed to install systemc call filter for architecture %s, skipping: %m", + seccomp_arch_to_string(arch)); } return 0; @@ -1133,16 +1135,19 @@ int seccomp_parse_syscall_filter( assert(name); assert(filter); + if (!FLAGS_SET(flags, SECCOMP_PARSE_INVERT) && errno_num >= 0) + return -EINVAL; + if (name[0] == '@') { const SyscallFilterSet *set; const char *i; set = syscall_filter_set_find(name); if (!set) { - if (!(flags & SECCOMP_PARSE_PERMISSIVE)) + if (!FLAGS_SET(flags, SECCOMP_PARSE_PERMISSIVE)) return -EINVAL; - log_syntax(unit, flags & SECCOMP_PARSE_LOG ? LOG_WARNING : LOG_DEBUG, filename, line, 0, + log_syntax(unit, FLAGS_SET(flags, SECCOMP_PARSE_LOG) ? LOG_WARNING : LOG_DEBUG, filename, line, 0, "Unknown system call group, ignoring: %s", name); return 0; } @@ -1161,22 +1166,24 @@ int seccomp_parse_syscall_filter( id = seccomp_syscall_resolve_name(name); if (id == __NR_SCMP_ERROR) { - if (!(flags & SECCOMP_PARSE_PERMISSIVE)) + if (!FLAGS_SET(flags, SECCOMP_PARSE_PERMISSIVE)) return -EINVAL; - log_syntax(unit, flags & SECCOMP_PARSE_LOG ? LOG_WARNING : LOG_DEBUG, filename, line, 0, + log_syntax(unit, FLAGS_SET(flags, SECCOMP_PARSE_LOG) ? LOG_WARNING : LOG_DEBUG, filename, line, 0, "Failed to parse system call, ignoring: %s", name); return 0; } - /* If we previously wanted to forbid a syscall and now - * we want to allow it, then remove it from the list. */ - if (!(flags & SECCOMP_PARSE_INVERT) == !!(flags & SECCOMP_PARSE_ALLOW_LIST)) { + /* If we previously wanted to forbid a syscall and now we want to allow it, then remove + * it from the list. The entries in allow-list with non-negative error value will be + * handled with SCMP_ACT_ERRNO() instead of the default action. */ + if (!FLAGS_SET(flags, SECCOMP_PARSE_INVERT) == FLAGS_SET(flags, SECCOMP_PARSE_ALLOW_LIST) || + (FLAGS_SET(flags, SECCOMP_PARSE_INVERT | SECCOMP_PARSE_ALLOW_LIST) && errno_num >= 0)) { r = hashmap_put(filter, INT_TO_PTR(id + 1), INT_TO_PTR(errno_num)); if (r < 0) switch (r) { case -ENOMEM: - return flags & SECCOMP_PARSE_LOG ? log_oom() : -ENOMEM; + return FLAGS_SET(flags, SECCOMP_PARSE_LOG) ? log_oom() : -ENOMEM; case -EEXIST: assert_se(hashmap_update(filter, INT_TO_PTR(id + 1), INT_TO_PTR(errno_num)) == 0); break; diff --git a/src/test/meson.build b/src/test/meson.build index 62e31fd8309..ff40a8d10dd 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -224,9 +224,7 @@ tests += [ [['src/test/test-parse-argument.c']], - [['src/test/test-parse-util.c'], - [], - [libseccomp]], + [['src/test/test-parse-util.c']], [['src/test/test-sysctl-util.c']], diff --git a/src/test/test-execute.c b/src/test/test-execute.c index c0e046b5e21..239fcea5e3d 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -444,6 +444,7 @@ static void test_exec_systemcallfilter(Manager *m) { test(m, "exec-systemcallfilter-with-errno-name.service", errno_from_name("EILSEQ"), CLD_EXITED); test(m, "exec-systemcallfilter-with-errno-number.service", 255, CLD_EXITED); test(m, "exec-systemcallfilter-with-errno-multi.service", errno_from_name("EILSEQ"), CLD_EXITED); + test(m, "exec-systemcallfilter-with-errno-in-allow-list.service", errno_from_name("EILSEQ"), CLD_EXITED); test(m, "exec-systemcallfilter-override-error-action.service", SIGSYS, CLD_KILLED); test(m, "exec-systemcallfilter-override-error-action2.service", errno_from_name("EILSEQ"), CLD_EXITED); #endif diff --git a/src/test/test-parse-util.c b/src/test/test-parse-util.c index 756934acad5..b1b23e2fbfa 100644 --- a/src/test/test-parse-util.c +++ b/src/test/test-parse-util.c @@ -11,9 +11,6 @@ #include "log.h" #include "parse-util.h" #include "string-util.h" -#if HAVE_SECCOMP -#include "seccomp-util.h" -#endif static void test_parse_boolean(void) { assert_se(parse_boolean("1") == 1); @@ -783,60 +780,6 @@ static void test_parse_errno(void) { assert_se(parse_errno("EINVALaaa") == -EINVAL); } -static void test_parse_syscall_and_errno(void) { -#if HAVE_SECCOMP - _cleanup_free_ char *n = NULL; - int e; - - assert_se(parse_syscall_and_errno("uname:EILSEQ", &n, &e) >= 0); - assert_se(streq(n, "uname")); - assert_se(e == errno_from_name("EILSEQ") && e >= 0); - n = mfree(n); - - assert_se(parse_syscall_and_errno("uname:EINVAL", &n, &e) >= 0); - assert_se(streq(n, "uname")); - assert_se(e == errno_from_name("EINVAL") && e >= 0); - n = mfree(n); - - assert_se(parse_syscall_and_errno("@sync:4095", &n, &e) >= 0); - assert_se(streq(n, "@sync")); - assert_se(e == 4095); - n = mfree(n); - - /* If errno is omitted, then e is set to -1 */ - assert_se(parse_syscall_and_errno("mount", &n, &e) >= 0); - assert_se(streq(n, "mount")); - assert_se(e == -1); - n = mfree(n); - - /* parse_syscall_and_errno() does not check the syscall name is valid or not. */ - assert_se(parse_syscall_and_errno("hoge:255", &n, &e) >= 0); - assert_se(streq(n, "hoge")); - assert_se(e == 255); - n = mfree(n); - - assert_se(parse_syscall_and_errno("hoge:kill", &n, &e) >= 0); - assert_se(streq(n, "hoge")); - assert_se(e == SECCOMP_ERROR_NUMBER_KILL); - n = mfree(n); - - /* The function checks the syscall name is empty or not. */ - assert_se(parse_syscall_and_errno("", &n, &e) == -EINVAL); - assert_se(parse_syscall_and_errno(":255", &n, &e) == -EINVAL); - - /* errno must be a valid errno name or number between 0 and ERRNO_MAX == 4095, or "kill" */ - assert_se(parse_syscall_and_errno("hoge:4096", &n, &e) == -ERANGE); - assert_se(parse_syscall_and_errno("hoge:-3", &n, &e) == -ERANGE); - assert_se(parse_syscall_and_errno("hoge:12.3", &n, &e) == -EINVAL); - assert_se(parse_syscall_and_errno("hoge:123junk", &n, &e) == -EINVAL); - assert_se(parse_syscall_and_errno("hoge:junk123", &n, &e) == -EINVAL); - assert_se(parse_syscall_and_errno("hoge:255:EILSEQ", &n, &e) == -EINVAL); - assert_se(parse_syscall_and_errno("hoge:-EINVAL", &n, &e) == -EINVAL); - assert_se(parse_syscall_and_errno("hoge:EINVALaaa", &n, &e) == -EINVAL); - assert_se(parse_syscall_and_errno("hoge:", &n, &e) == -EINVAL); -#endif -} - static void test_parse_mtu(void) { uint32_t mtu = 0; @@ -914,7 +857,6 @@ int main(int argc, char *argv[]) { test_parse_nice(); test_parse_dev(); test_parse_errno(); - test_parse_syscall_and_errno(); test_parse_mtu(); test_parse_loadavg_fixed_point(); diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c index 10393b6a7cf..b1f917eb54e 100644 --- a/src/test/test-seccomp.c +++ b/src/test/test-seccomp.c @@ -41,6 +41,64 @@ # define SECCOMP_RESTRICT_ADDRESS_FAMILIES_BROKEN 0 #endif +static void test_parse_syscall_and_errno(void) { + _cleanup_free_ char *n = NULL; + int e; + + assert_se(parse_syscall_and_errno("uname:EILSEQ", &n, &e) >= 0); + assert_se(streq(n, "uname")); + assert_se(e == errno_from_name("EILSEQ") && e >= 0); + n = mfree(n); + + assert_se(parse_syscall_and_errno("uname:EINVAL", &n, &e) >= 0); + assert_se(streq(n, "uname")); + assert_se(e == errno_from_name("EINVAL") && e >= 0); + n = mfree(n); + + assert_se(parse_syscall_and_errno("@sync:4095", &n, &e) >= 0); + assert_se(streq(n, "@sync")); + assert_se(e == 4095); + n = mfree(n); + + /* If errno is omitted, then e is set to -1 */ + assert_se(parse_syscall_and_errno("mount", &n, &e) >= 0); + assert_se(streq(n, "mount")); + assert_se(e == -1); + n = mfree(n); + + /* parse_syscall_and_errno() does not check the syscall name is valid or not. */ + assert_se(parse_syscall_and_errno("hoge:255", &n, &e) >= 0); + assert_se(streq(n, "hoge")); + assert_se(e == 255); + n = mfree(n); + + /* 0 is also a valid errno. */ + assert_se(parse_syscall_and_errno("hoge:0", &n, &e) >= 0); + assert_se(streq(n, "hoge")); + assert_se(e == 0); + n = mfree(n); + + assert_se(parse_syscall_and_errno("hoge:kill", &n, &e) >= 0); + assert_se(streq(n, "hoge")); + assert_se(e == SECCOMP_ERROR_NUMBER_KILL); + n = mfree(n); + + /* The function checks the syscall name is empty or not. */ + assert_se(parse_syscall_and_errno("", &n, &e) == -EINVAL); + assert_se(parse_syscall_and_errno(":255", &n, &e) == -EINVAL); + + /* errno must be a valid errno name or number between 0 and ERRNO_MAX == 4095, or "kill" */ + assert_se(parse_syscall_and_errno("hoge:4096", &n, &e) == -ERANGE); + assert_se(parse_syscall_and_errno("hoge:-3", &n, &e) == -ERANGE); + assert_se(parse_syscall_and_errno("hoge:12.3", &n, &e) == -EINVAL); + assert_se(parse_syscall_and_errno("hoge:123junk", &n, &e) == -EINVAL); + assert_se(parse_syscall_and_errno("hoge:junk123", &n, &e) == -EINVAL); + assert_se(parse_syscall_and_errno("hoge:255:EILSEQ", &n, &e) == -EINVAL); + assert_se(parse_syscall_and_errno("hoge:-EINVAL", &n, &e) == -EINVAL); + assert_se(parse_syscall_and_errno("hoge:EINVALaaa", &n, &e) == -EINVAL); + assert_se(parse_syscall_and_errno("hoge:", &n, &e) == -EINVAL); +} + static void test_seccomp_arch_to_string(void) { uint32_t a, b; const char *name; @@ -1075,6 +1133,7 @@ static void test_restrict_suid_sgid(void) { int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); + test_parse_syscall_and_errno(); test_seccomp_arch_to_string(); test_architecture_table(); test_syscall_filter_set_find(); diff --git a/test/test-execute/exec-systemcallfilter-with-errno-in-allow-list.service b/test/test-execute/exec-systemcallfilter-with-errno-in-allow-list.service new file mode 100644 index 00000000000..4b2636eb44d --- /dev/null +++ b/test/test-execute/exec-systemcallfilter-with-errno-in-allow-list.service @@ -0,0 +1,9 @@ +[Unit] +Description=Test for SystemCallFilter with errno name (for issue #18916) + +[Service] +ExecStart=/usr/bin/python3 -c 'import os\ntry: os.uname()\nexcept Exception as e: exit(e.errno)' +Type=oneshot +SystemCallFilter=@system-service +SystemCallFilter=~uname:EILSEQ +SystemCallErrorNumber=EACCES