From 6c71bd596902f28ab492a100f56d1e141b023c04 Mon Sep 17 00:00:00 2001 From: Justine Tunney Date: Sun, 24 Jul 2022 05:54:26 -0700 Subject: [PATCH] Further improve unveil() implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change addresses review comments from Günther Noack on GitHub. We're now blacklisting truncate() and setxattr() since Landlock lets them operate on veiled files. The restriction has been lifted on using unveil() multiple times, since Landlock does that well. --- libc/calls/calls.h | 4 +- libc/calls/getegid.c | 12 +++-- libc/calls/geteuid.c | 12 +++-- libc/calls/getuid.c | 20 ++++--- libc/mem/unveil.c | 44 ++++++++++----- test/libc/mem/unveil_test.c | 105 ++++++++++++++++++++++++++++++++---- 6 files changed, 154 insertions(+), 43 deletions(-) diff --git a/libc/calls/calls.h b/libc/calls/calls.h index 7e03f4155..1bb55360d 100644 --- a/libc/calls/calls.h +++ b/libc/calls/calls.h @@ -103,7 +103,7 @@ int geteuid(void) nosideeffect; int getgid(void) nosideeffect; int gethostname(char *, size_t); int getloadavg(double *, int); -int getpgid(int) nosideeffect libcesque; +int getpgid(int) libcesque; int getpgrp(void) nosideeffect; int getpid(void) nosideeffect libcesque; int getppid(void); @@ -112,7 +112,7 @@ int getresgid(uint32_t *, uint32_t *, uint32_t *); int getresuid(uint32_t *, uint32_t *, uint32_t *); int getsid(int) nosideeffect libcesque; int gettid(void) libcesque; -int getuid(void) nosideeffect libcesque; +int getuid(void) libcesque; int ioprio_get(int, int); int ioprio_set(int, int, int); int kill(int, int); diff --git a/libc/calls/getegid.c b/libc/calls/getegid.c index e4516baca..73ec9dc4b 100644 --- a/libc/calls/getegid.c +++ b/libc/calls/getegid.c @@ -20,6 +20,8 @@ #include "libc/calls/strace.internal.h" #include "libc/calls/syscall-sysv.internal.h" #include "libc/dce.h" +#include "libc/runtime/runtime.h" +#include "libc/sysv/consts/auxv.h" /** * Returns effective group ID of calling process. @@ -27,10 +29,12 @@ */ int getegid(void) { int rc; - if (!IsWindows()) { - rc = sys_getegid(); - } else { - rc = getgid(); + if (!(rc = getauxval(AT_EGID))) { + if (!IsWindows()) { + rc = sys_getegid(); + } else { + rc = getgid(); + } } STRACE("%s() → %d% m", "getegid", rc); return rc; diff --git a/libc/calls/geteuid.c b/libc/calls/geteuid.c index 4db32a100..7c5d7ac1f 100644 --- a/libc/calls/geteuid.c +++ b/libc/calls/geteuid.c @@ -19,6 +19,8 @@ #include "libc/calls/calls.h" #include "libc/calls/strace.internal.h" #include "libc/calls/syscall-sysv.internal.h" +#include "libc/runtime/runtime.h" +#include "libc/sysv/consts/auxv.h" /** * Returns effective user ID of calling process. @@ -26,10 +28,12 @@ */ int geteuid(void) { int rc; - if (!IsWindows()) { - rc = sys_geteuid(); - } else { - rc = getuid(); + if (!(rc = getauxval(AT_EUID))) { + if (!IsWindows()) { + rc = sys_geteuid(); + } else { + rc = getuid(); + } } STRACE("%s() → %d% m", "geteuid", rc); return rc; diff --git a/libc/calls/getuid.c b/libc/calls/getuid.c index 4136a304e..f7b73cc06 100644 --- a/libc/calls/getuid.c +++ b/libc/calls/getuid.c @@ -53,10 +53,12 @@ static textwindows dontinline uint32_t GetUserNameHash(void) { */ int getuid(void) { int rc; - if (!IsWindows()) { - rc = sys_getuid(); - } else { - rc = GetUserNameHash(); + if (!(rc = getauxval(AT_UID))) { + if (!IsWindows()) { + rc = sys_getuid(); + } else { + rc = GetUserNameHash(); + } } STRACE("%s() → %d% m", "getuid", rc); return rc; @@ -73,10 +75,12 @@ int getuid(void) { */ int getgid(void) { int rc; - if (!IsWindows()) { - rc = sys_getgid(); - } else { - rc = GetUserNameHash(); + if (!(rc = getauxval(AT_GID))) { + if (!IsWindows()) { + rc = sys_getgid(); + } else { + rc = GetUserNameHash(); + } } STRACE("%s() → %d% m", "getgid", rc); return rc; diff --git a/libc/mem/unveil.c b/libc/mem/unveil.c index bb7a3f02c..ba6a26d8d 100644 --- a/libc/mem/unveil.c +++ b/libc/mem/unveil.c @@ -64,18 +64,15 @@ (LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_WRITE_FILE | \ LANDLOCK_ACCESS_FS_EXECUTE) -static const struct sock_filter kBlacklistLandlock[] = { - // clang-format off +static const struct sock_filter kUnveilBlacklist[] = { BPF_STMT(BPF_LD | BPF_W | BPF_ABS, OFF(arch)), BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, AUDIT_ARCH_X86_64, 1, 0), BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_KILL_PROCESS), BPF_STMT(BPF_LD | BPF_W | BPF_ABS, OFF(nr)), - BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, __NR_linux_landlock_create_ruleset, 2, 0), - BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, __NR_linux_landlock_add_rule, 1, 0), - BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, __NR_linux_landlock_restrict_self, 0, 1), + BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, __NR_linux_truncate, 1, 0), + BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, __NR_linux_setxattr, 0, 1), BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ERRNO | (1 & SECCOMP_RET_DATA)), BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ALLOW), - // clang-format on }; /** @@ -98,8 +95,8 @@ _Thread_local static struct { static int unveil_final(void) { int rc; struct sock_fprog sandbox = { - .filter = kBlacklistLandlock, - .len = ARRAYLEN(kBlacklistLandlock), + .filter = kUnveilBlacklist, + .len = ARRAYLEN(kUnveilBlacklist), }; if ((rc = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) != -1 && (rc = landlock_restrict_self(State.fd, 0)) != -1 && @@ -264,14 +261,13 @@ static int sys_unveil_linux(const char *path, const char *permissions) { * unveil("/etc", "r"); // make /etc readable too * unveil(0, 0); // commit and lock policy * - * Unveiling restricts a thread's view of the filesystem to a set of - * allowed paths with specific privileges. + * Unveiling restricts a view of the filesystem to a set of allowed + * paths with specific privileges. * * Once you start using unveil(), the entire file system is considered * hidden. You then specify, by repeatedly calling unveil(), which paths * should become unhidden. When you're finished, you call `unveil(0,0)` - * which commits your policy, after which further use is forbidden, in - * the current thread, as well as any threads or processes it spawns. + * which commits your policy. * * There are some differences between unveil() on Linux versus OpenBSD. * @@ -299,8 +295,28 @@ static int sys_unveil_linux(const char *path, const char *permissions) { * possible to use opendir() and go fishing for paths which weren't * previously known. * - * 5. Always specify at least one path. OpenBSD has unclear semantics - * when `pledge(0,0)` is used without any previous calls. + * 5. Use ftruncate() rather than truncate(). One of the backdoors with + * Landlock is it currently can't restrict truncate() and setxattr() + * which permits certain kinds of modifications to files outside the + * sandbox. When your policy is committed, we install a SECCOMP BPF + * filter to disable those calls, however similar trickery may be + * possible through other unaddressed calls like ioctl(). Using the + * pledge() function in addition to unveil() will solve this, since + * it installs a strong system call access policy. + * + * 6. Set your process-wide policy at startup from the main thread. On + * OpenBSD unveil() will apply process-wide even when called from a + * child thread; whereas with Linux, calling unveil() from a thread + * will cause your ruleset to only apply to that thread in addition + * to any descendent threads it creates. + * + * 7. Always specify at least one path. OpenBSD has unclear semantics + * when `unveil(0,0)` is used without any previous calls. + * + * 8. On OpenBSD calling `unveil(0,0)` will prevent unveil() from being + * used again. On Linux this is allowed, because Landlock is able to + * do that securely, i.e. the second ruleset can only be a subset of + * the previous ones. * * This system call is supported natively on OpenBSD and polyfilled on * Linux using the Landlock LSM[1]. diff --git a/test/libc/mem/unveil_test.c b/test/libc/mem/unveil_test.c index 40029e58a..e37a15639 100644 --- a/test/libc/mem/unveil_test.c +++ b/test/libc/mem/unveil_test.c @@ -38,6 +38,7 @@ #include "libc/sysv/consts/sig.h" #include "libc/sysv/consts/sock.h" #include "libc/testlib/testlib.h" +#include "libc/thread/spawn.h" #include "libc/x/x.h" STATIC_YOINK("zip_uri_support"); @@ -241,19 +242,107 @@ TEST(unveil, overlappingDirectories_inconsistentBehavior) { EXITS(0); } -TEST(unveil, usedTwice_forbidden) { +TEST(unveil, usedTwice_allowedOnLinux) { + if (IsOpenbsd()) return; + SPAWN(fork); + ASSERT_SYS(0, 0, mkdir("jail", 0755)); + ASSERT_SYS(0, 0, xbarf("jail/ok.txt", "hello", 5)); + ASSERT_SYS(0, 0, mkdir("garden", 0755)); + ASSERT_SYS(0, 0, xbarf("garden/secret.txt", "hello", 5)); + ASSERT_SYS(0, 0, mkdir("heaven", 0755)); + ASSERT_SYS(0, 0, xbarf("heaven/verysecret.txt", "hello", 5)); + ASSERT_SYS(0, 0, unveil("jail", "rw")); + ASSERT_SYS(0, 0, unveil("garden", "rw")); + ASSERT_SYS(0, 0, unveil(0, 0)); + ASSERT_SYS(0, 3, open("garden/secret.txt", O_RDONLY)); + ASSERT_SYS(0, 0, unveil("jail", "rw")); + ASSERT_SYS(0, 0, unveil("heaven", "rw")); // not allowed, superset of parent + ASSERT_SYS(0, 0, unveil(0, 0)); + ASSERT_SYS(0, 4, open("jail/ok.txt", O_RDONLY)); + ASSERT_SYS(EACCES, -1, open("garden/secret.txt", O_RDONLY)); + ASSERT_SYS(EACCES, -1, open("heaven/verysecret.txt", O_RDONLY)); + EXITS(0); +} + +TEST(unveil, truncate_isForbiddenBySeccomp) { SPAWN(fork); ASSERT_SYS(0, 0, mkdir("jail", 0755)); ASSERT_SYS(0, 0, mkdir("garden", 0755)); ASSERT_SYS(0, 0, xbarf("garden/secret.txt", "hello", 5)); ASSERT_SYS(0, 0, unveil("jail", "rw")); ASSERT_SYS(0, 0, unveil(0, 0)); - ASSERT_SYS(EACCES_OR_ENOENT, -1, open("garden/secret.txt", O_RDONLY)); - ASSERT_SYS(EPERM, -1, unveil("jail", "rw")); - ASSERT_SYS(EACCES_OR_ENOENT, -1, open("garden/secret.txt", O_RDONLY)); + ASSERT_SYS(IsOpenbsd() ? ENOENT : EPERM, -1, + truncate("garden/secret.txt", 0)); if (IsLinux()) { ASSERT_SYS(0, 0, stat("garden/secret.txt", &st)); - ASSERT_EQ(5, st.st_size); // wut linux metadata is accessible + ASSERT_EQ(5, st.st_size); + } + EXITS(0); +} + +TEST(unveil, ftruncate_isForbidden) { + if (IsOpenbsd()) return; // b/c O_PATH is a Linux thing + SPAWN(fork); + ASSERT_SYS(0, 0, mkdir("jail", 0755)); + ASSERT_SYS(0, 0, mkdir("garden", 0755)); + ASSERT_SYS(0, 0, xbarf("garden/secret.txt", "hello", 5)); + ASSERT_SYS(0, 0, unveil("jail", "rw")); + ASSERT_SYS(0, 0, unveil(0, 0)); + ASSERT_SYS(0, 3, open("garden/secret.txt", O_PATH)); + ASSERT_SYS(EBADF, -1, ftruncate(3, 0)); + ASSERT_SYS(0, 0, close(3)); + ASSERT_SYS(0, 0, stat("garden/secret.txt", &st)); + ASSERT_EQ(5, st.st_size); + EXITS(0); +} + +TEST(unveil, procfs_isForbiddenByDefault) { + if (IsOpenbsd()) return; + SPAWN(fork); + ASSERT_SYS(0, 0, mkdir("jail", 0755)); + ASSERT_SYS(0, 0, unveil("jail", "rw")); + ASSERT_SYS(0, 0, unveil(0, 0)); + ASSERT_SYS(EACCES, -1, open("/proc/self/cmdline", O_RDONLY)); + EXITS(0); +} + +int Worker(void *arg, int tid) { + ASSERT_SYS(EACCES_OR_ENOENT, -1, open("garden/secret.txt", O_RDONLY)); + return 0; +} + +TEST(unveil, isInheritedAcrossThreads) { + struct spawn t; + SPAWN(fork); + ASSERT_SYS(0, 0, mkdir("jail", 0755)); + ASSERT_SYS(0, 0, mkdir("garden", 0755)); + ASSERT_SYS(0, 0, xbarf("garden/secret.txt", "hello", 5)); + ASSERT_SYS(0, 0, unveil("jail", "rw")); + ASSERT_SYS(0, 0, unveil(0, 0)); + ASSERT_SYS(0, 0, _spawn(Worker, 0, &t)); + EXPECT_SYS(0, 0, _join(&t)); + EXITS(0); +} + +int Worker2(void *arg, int tid) { + ASSERT_SYS(0, 0, unveil("jail", "rw")); + ASSERT_SYS(0, 0, unveil(0, 0)); + ASSERT_SYS(EACCES_OR_ENOENT, -1, open("garden/secret.txt", O_RDONLY)); + return 0; +} + +TEST(unveil, isThreadSpecificOnLinux_isProcessWideOnOpenbsd) { + struct spawn t; + SPAWN(fork); + ASSERT_SYS(0, 0, mkdir("jail", 0755)); + ASSERT_SYS(0, 0, mkdir("garden", 0755)); + ASSERT_SYS(0, 0, xbarf("garden/secret.txt", "hello", 5)); + ASSERT_SYS(0, 0, _spawn(Worker2, 0, &t)); + EXPECT_SYS(0, 0, _join(&t)); + if (IsOpenbsd()) { + ASSERT_SYS(ENOENT, -1, open("garden/secret.txt", O_RDONLY)); + } else { + ASSERT_SYS(0, 3, open("garden/secret.txt", O_RDONLY)); } EXITS(0); } @@ -275,12 +364,6 @@ TEST(unveil, usedTwice_forbidden_worksWithPledge) { ASSERT_SYS(0, 0, unveil(0, 0)); ASSERT_SYS(EACCES_OR_ENOENT, -1, open("garden/secret.txt", O_RDONLY)); // verify the second filter is working - ASSERT_SYS(EPERM, -1, unveil("jail", "rw")); - if (IsLinux()) { - ASSERT_SYS( - EPERM, -1, - landlock_create_ruleset(0, 0, LANDLOCK_CREATE_RULESET_VERSION)); - } ASSERT_SYS(EACCES_OR_ENOENT, -1, open("garden/secret.txt", O_RDONLY)); // verify the first filter is still working *gotsome = true;