From e8132d63fea6986cb6bcb2b78d95b1ada3ada708 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Aug 2017 20:40:26 +0200 Subject: [PATCH] seccomp: default to something resembling the current personality when locking it Let's lock the personality to the currently set one, if nothing is specifically specified. But do so with a grain of salt, and never default to any exotic personality here, but only PER_LINUX or PER_LINUX32. --- src/basic/process-util.c | 19 +++++++++++++++++++ src/basic/process-util.h | 2 ++ src/core/execute.c | 15 +++++++++++---- src/test/test-seccomp.c | 38 ++++++++++++++++++++++++++++++++++---- 4 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 2b23ac36c0..056a8b82aa 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -904,6 +904,25 @@ const char* personality_to_string(unsigned long p) { return architecture_to_string(architecture); } +int opinionated_personality(unsigned long *ret) { + int current; + + /* Returns the current personality, or PERSONALITY_INVALID if we can't determine it. This function is a bit + * opinionated though, and ignores all the finer-grained bits and exotic personalities, only distinguishing the + * two most relevant personalities: PER_LINUX and PER_LINUX32. */ + + current = personality(PERSONALITY_INVALID); + if (current < 0) + return -errno; + + if (((unsigned long) current & 0xffff) == PER_LINUX32) + *ret = PER_LINUX32; + else + *ret = PER_LINUX; + + return 0; +} + void valgrind_summary_hack(void) { #ifdef HAVE_VALGRIND_VALGRIND_H if (getpid_cached() == 1 && RUNNING_ON_VALGRIND) { diff --git a/src/basic/process-util.h b/src/basic/process-util.h index 913991b3cb..d71db2df7e 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -91,6 +91,8 @@ bool oom_score_adjust_is_valid(int oa); unsigned long personality_from_string(const char *p); const char *personality_to_string(unsigned long); +int opinionated_personality(unsigned long *ret); + int ioprio_class_to_string_alloc(int i, char **s); int ioprio_class_from_string(const char *s); diff --git a/src/core/execute.c b/src/core/execute.c index 4d285ff250..0cbaf9a00b 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1457,7 +1457,8 @@ static int apply_restrict_namespaces(Unit *u, const ExecContext *c) { } static int apply_lock_personality(const Unit* u, const ExecContext *c) { - unsigned long personality = c->personality; + unsigned long personality; + int r; assert(u); assert(c); @@ -1468,9 +1469,15 @@ static int apply_lock_personality(const Unit* u, const ExecContext *c) { if (skip_seccomp_unavailable(u, "LockPersonality=")) return 0; - /* If personality is not specified, use the default (Linux) */ - if (personality == PERSONALITY_INVALID) - personality = PER_LINUX; + personality = c->personality; + + /* If personality is not specified, use either PER_LINUX or PER_LINUX32 depending on what is currently set. */ + if (personality == PERSONALITY_INVALID) { + + r = opinionated_personality(&personality); + if (r < 0) + return r; + } return seccomp_lock_personality(personality); } diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c index 7ffbc4754e..262d0b712b 100644 --- a/src/test/test-seccomp.c +++ b/src/test/test-seccomp.c @@ -567,6 +567,7 @@ static void test_load_syscall_filter_set_raw(void) { } static void test_lock_personality(void) { + unsigned long current; pid_t pid; if (!is_seccomp_available()) @@ -574,26 +575,55 @@ static void test_lock_personality(void) { if (geteuid() != 0) return; + assert_se(opinionated_personality(¤t) >= 0); + + log_info("current personality=%lu", current); + pid = fork(); assert_se(pid >= 0); if (pid == 0) { - assert_se(seccomp_lock_personality(PER_LINUX) >= 0); + assert_se(seccomp_lock_personality(current) >= 0); - assert_se(personality(PER_LINUX) == PER_LINUX); + assert_se((unsigned long) personality(current) == current); + + errno = EUCLEAN; assert_se(personality(PER_LINUX | ADDR_NO_RANDOMIZE) == -1 && errno == EPERM); + + errno = EUCLEAN; assert_se(personality(PER_LINUX | MMAP_PAGE_ZERO) == -1 && errno == EPERM); + + errno = EUCLEAN; assert_se(personality(PER_LINUX | ADDR_COMPAT_LAYOUT) == -1 && errno == EPERM); + + errno = EUCLEAN; assert_se(personality(PER_LINUX | READ_IMPLIES_EXEC) == -1 && errno == EPERM); + + errno = EUCLEAN; assert_se(personality(PER_LINUX_32BIT) == -1 && errno == EPERM); + + errno = EUCLEAN; assert_se(personality(PER_SVR4) == -1 && errno == EPERM); + + errno = EUCLEAN; assert_se(personality(PER_BSD) == -1 && errno == EPERM); - assert_se(personality(PER_LINUX32) == -1 && errno == EPERM); + + errno = EUCLEAN; + assert_se(personality(current == PER_LINUX ? PER_LINUX32 : PER_LINUX) == -1 && errno == EPERM); + + errno = EUCLEAN; assert_se(personality(PER_LINUX32_3GB) == -1 && errno == EPERM); + + errno = EUCLEAN; assert_se(personality(PER_UW7) == -1 && errno == EPERM); + + errno = EUCLEAN; assert_se(personality(0x42) == -1 && errno == EPERM); + + errno = EUCLEAN; assert_se(personality(PERSONALITY_INVALID) == -1 && errno == EPERM); /* maybe remove this later */ - assert_se(personality(PER_LINUX) == PER_LINUX); + + assert_se((unsigned long) personality(current) == current); _exit(EXIT_SUCCESS); }