From 9f876f70ed13636a8f9a25ba043e8f682c6688f6 Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Tue, 8 Sep 2026 06:46:36 +0000 Subject: [PATCH] fix(daemon): trust the kernel overflow uid as an ancestor owner inside a restricted user namespace posix_directory_owner_trusted only accepted an ancestor owned by uid 0 or the caller's own euid. Inside a single-uid Docker userns-remap (and some WSL2 devcontainer setups), uid 0 has no entry in the container's uid map, so the kernel renders any host-owned path whose real owner is unmapped, including real root which owns "/", "/home" and "/tmp", as the kernel overflow uid instead. Every ancestor of a private cache directory then rendered as that overflow uid, so the trust check refused it and the daemon could never secure its private directory: every path failed with "secure CLI coordination could not be created". Extend posix_directory_owner_trusted to also accept the kernel's own overflow uid (read once from /proc/sys/kernel/overflowuid, falling back to the 65534 default if unreadable), but only when this process's own user namespace genuinely has no mapping for uid 0 (read from /proc/self/uid_map). Outside a restricted namespace the overflow uid names an ordinary account (services dropped to "nobody"), not a stand-in for unmapped root, so trusting it unconditionally would let anything writable by such an account own a trusted ancestor. The leaf private directory's own ownership check is untouched: it still requires an exact match against the caller's euid before it is chmod'd to 0700. Tests: two new Linux-only, root-required tests (fabricating an arbitrary-uid ancestor needs CAP_CHOWN). One confirms an overflow-uid ancestor is admitted when the process's own namespace has no mapping for uid 0; the other confirms the same ancestor still stays refused outside one, so the fix cannot regress the ordinary case. A real restricted namespace needs CAP_SYS_ADMIN to construct, which this sandbox does not grant even inside a fresh container (confirmed live: unshare --user fails EPERM here), so the namespace state is pinned through a test-only seam instead of unshare(1), consistent with how the file's own Windows tests already pin non-constructible OS state. Fixes #1830 Signed-off-by: Amir Fathi --- src/daemon/ipc.c | 64 +++++++++++++++++++ src/daemon/ipc_internal.h | 20 ++++++ tests/test_daemon_ipc.c | 125 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 209 insertions(+) diff --git a/src/daemon/ipc.c b/src/daemon/ipc.c index a706a7ca5..32f0d118d 100644 --- a/src/daemon/ipc.c +++ b/src/daemon/ipc.c @@ -1385,7 +1385,71 @@ static char *private_log_directory_path_copy(const char *directory_path) { return string_copy(directory_path); } +#ifdef __linux__ +/* Inside a single-uid Docker userns-remap (or some WSL2 devcontainer) setups, + * uid 0 has no entry in the container's uid map, so the kernel renders any + * host-owned path whose real owner is unmapped (including real root, which + * owns "/", "/home", "/tmp") as the overflow uid instead. 65534 is the + * kernel default but is configurable, so read it rather than assume it. */ +uid_t cbm_daemon_ipc_posix_kernel_overflow_uid(void) { + FILE *overflow_file = fopen("/proc/sys/kernel/overflowuid", "r"); + if (!overflow_file) { + return (uid_t)65534; + } + unsigned long parsed = 0; + int scanned = fscanf(overflow_file, "%lu", &parsed); + (void)fclose(overflow_file); + return scanned == 1 ? (uid_t)parsed : (uid_t)65534; +} + +/* -1 = no override (read /proc/self/uid_map for real), 0 = force "mapped" + * (ordinary namespace), 1 = force "unmapped" (restricted userns). A real + * restricted userns needs CAP_SYS_ADMIN to construct, which CI runners do + * not grant, so tests pin this instead of shelling out to unshare(1). */ +static atomic_int g_posix_uid_zero_unmapped_override_for_test = ATOMIC_VAR_INIT(-1); + +void cbm_daemon_ipc_posix_uid_zero_unmapped_override_set_for_test(int override) { + atomic_store_explicit(&g_posix_uid_zero_unmapped_override_for_test, override, + memory_order_release); +} + +/* The overflow uid is only a stand-in for "unmapped root" inside a restricted + * user namespace. Outside one, uid 0 is mapped to itself and the overflow uid + * is an ordinary, sometimes-real account (services that drop privileges to + * "nobody"); trusting it unconditionally would let any ancestor a compromised + * nobody-owned service can write into pass this check. So only relax the + * check when our own namespace genuinely has no mapping for uid 0, read from + * /proc/self/uid_map (one line per contiguous range: "inner outer count"). */ +bool cbm_daemon_ipc_posix_uid_zero_unmapped(void) { + int override = + atomic_load_explicit(&g_posix_uid_zero_unmapped_override_for_test, memory_order_acquire); + if (override >= 0) { + return override != 0; + } + FILE *uid_map_file = fopen("/proc/self/uid_map", "r"); + if (!uid_map_file) { + return false; + } + unsigned long inner = 0, outer = 0, count = 0; + bool zero_mapped = false; + while (fscanf(uid_map_file, "%lu %lu %lu", &inner, &outer, &count) == 3) { + if (inner == 0) { + zero_mapped = true; + break; + } + } + (void)fclose(uid_map_file); + return !zero_mapped; +} +#endif + static bool posix_directory_owner_trusted(uid_t owner) { +#ifdef __linux__ + if (owner == cbm_daemon_ipc_posix_kernel_overflow_uid() && + cbm_daemon_ipc_posix_uid_zero_unmapped()) { + return true; + } +#endif return owner == (uid_t)0 || owner == geteuid(); } diff --git a/src/daemon/ipc_internal.h b/src/daemon/ipc_internal.h index b791a6b02..1d0629769 100644 --- a/src/daemon/ipc_internal.h +++ b/src/daemon/ipc_internal.h @@ -10,6 +10,10 @@ #include #include +#ifdef __linux__ +#include +#endif + /* Windows daemon rendezvous addresses are generation-specific and * unguessable. These platform-neutral seams keep the SID/nonce derivation and * fixed record parser covered on every CI host; the Windows endpoint @@ -105,4 +109,20 @@ void cbm_daemon_ipc_windows_legacy_guard_release_failures_set_for_test(unsigned typedef void (*cbm_daemon_ipc_startup_gate_fn)(void *context); void cbm_daemon_ipc_startup_gate_set_for_test(cbm_daemon_ipc_startup_gate_fn gate, void *context); +#ifdef __linux__ +/* The kernel uid a single-uid userns-remap container renders an unmapped host + * owner as (default 65534, configurable via sysctl). Exposed so tests can + * assert against the real value instead of hardcoding it. */ +uid_t cbm_daemon_ipc_posix_kernel_overflow_uid(void); + +/* True when this process's own user namespace has no mapping for uid 0, i.e. + * we are inside a restricted userns-remap where the overflow uid stands in + * for an unmapped real owner rather than naming an ordinary account. */ +bool cbm_daemon_ipc_posix_uid_zero_unmapped(void); + +/* Test seam: -1 restores the real /proc/self/uid_map read, 0/1 pin the + * result without needing CAP_SYS_ADMIN to build an actual user namespace. */ +void cbm_daemon_ipc_posix_uid_zero_unmapped_override_set_for_test(int override); +#endif + #endif /* CBM_DAEMON_IPC_INTERNAL_H */ diff --git a/tests/test_daemon_ipc.c b/tests/test_daemon_ipc.c index 9bb550948..c8c1bfd0a 100644 --- a/tests/test_daemon_ipc.c +++ b/tests/test_daemon_ipc.c @@ -4950,6 +4950,123 @@ TEST(daemon_ipc_posix_world_writable_ancestor_still_refused_issue1537) { ASSERT_TRUE(refused); PASS(); } + +#ifdef __linux__ +/* #1830: inside a single-uid Docker userns-remap (some WSL2 devcontainers hit + * this too), uid 0 has no entry in the container's uid map, so the kernel + * renders every unmapped host owner, real root included, as the overflow uid. + * "/", "/home" and "/tmp" are root-owned, so the old owner==0-or-euid check + * refused every ancestor and the daemon could not start. */ +TEST(daemon_ipc_posix_kernel_overflow_uid_matches_proc_sys_kernel) { + FILE *overflow_file = fopen("/proc/sys/kernel/overflowuid", "r"); + unsigned long expected = 65534; + bool proc_readable = overflow_file != NULL; + if (overflow_file) { + proc_readable = fscanf(overflow_file, "%lu", &expected) == 1; + (void)fclose(overflow_file); + } + + uid_t actual = cbm_daemon_ipc_posix_kernel_overflow_uid(); + + ASSERT_TRUE(proc_readable); + ASSERT_EQ(actual, (uid_t)expected); + PASS(); +} + +TEST(daemon_ipc_posix_uid_zero_unmapped_reflects_real_process_by_default) { + /* The test binary itself is not built inside a restricted userns-remap + * (CI runners and dev machines map uid 0 identically), so the real + * /proc/self/uid_map read must report "mapped", the safe default that + * keeps posix_directory_owner_trusted() strict outside a real container. */ + cbm_daemon_ipc_posix_uid_zero_unmapped_override_set_for_test(-1); + bool unmapped = cbm_daemon_ipc_posix_uid_zero_unmapped(); + ASSERT_TRUE(!unmapped); + PASS(); +} + +TEST(daemon_ipc_posix_private_directory_admits_overflow_uid_ancestor_inside_restricted_userns) { + char parent[TEST_PATH_CAP]; + char cache[TEST_PATH_CAP]; + bool paths_ok = false; + bool is_root = geteuid() == 0; + bool ancestor_ready = !is_root; + bool secured = !is_root; + bool leaf_owner_private = !is_root; + + if (ipc_test_parent_new(parent, "posix-overflow-uid-ancestor")) { + int c = snprintf(cache, sizeof(cache), "%s/cache", parent); + paths_ok = c > 0 && c < (int)sizeof(cache); + } + if (paths_ok && is_root) { + /* Only root can fabricate an ancestor owned by a uid other than our + * own or 0; this is the one condition under test, so a non-root run + * cannot exercise it and is left passing on the checks it can do. + * Building an actual restricted userns needs CAP_SYS_ADMIN that CI + * runners do not grant, so the namespace state itself is pinned via + * the test seam rather than constructed with unshare(1). */ + cbm_daemon_ipc_posix_uid_zero_unmapped_override_set_for_test(1); + ancestor_ready = chown(parent, cbm_daemon_ipc_posix_kernel_overflow_uid(), + cbm_daemon_ipc_posix_kernel_overflow_uid()) == 0; + } + if (ancestor_ready && is_root) { + secured = cbm_daemon_ipc_private_directory_secure(cache); + struct stat leaf; + leaf_owner_private = secured && stat(cache, &leaf) == 0 && S_ISDIR(leaf.st_mode) && + leaf.st_uid == geteuid() && (leaf.st_mode & 07777) == 0700; + } + + (void)rmdir(cache); + if (is_root) { + (void)chown(parent, geteuid(), (gid_t)-1); + } + ipc_test_remove_flat_dir(parent); + cbm_daemon_ipc_posix_uid_zero_unmapped_override_set_for_test(-1); + + ASSERT_TRUE(paths_ok); + ASSERT_TRUE(ancestor_ready); + ASSERT_TRUE(secured); + ASSERT_TRUE(leaf_owner_private); + PASS(); +} + +TEST(daemon_ipc_posix_private_directory_refuses_overflow_uid_ancestor_outside_restricted_userns) { + /* The security case #1830's fix must not regress: on an ORDINARY host + * (uid 0 mapped, the common case), an ancestor owned by the overflow + * uid is an unrelated, sometimes-real account (services dropped to + * "nobody"), not a stand-in for unmapped root, so it must stay refused. */ + char parent[TEST_PATH_CAP]; + char cache[TEST_PATH_CAP]; + bool paths_ok = false; + bool is_root = geteuid() == 0; + bool ancestor_ready = !is_root; + bool refused = !is_root; + + if (ipc_test_parent_new(parent, "posix-overflow-uid-mapped-ancestor")) { + int c = snprintf(cache, sizeof(cache), "%s/cache", parent); + paths_ok = c > 0 && c < (int)sizeof(cache); + } + if (paths_ok && is_root) { + cbm_daemon_ipc_posix_uid_zero_unmapped_override_set_for_test(0); + ancestor_ready = chown(parent, cbm_daemon_ipc_posix_kernel_overflow_uid(), + cbm_daemon_ipc_posix_kernel_overflow_uid()) == 0; + } + if (ancestor_ready && is_root) { + refused = !cbm_daemon_ipc_private_directory_secure(cache); + } + + (void)rmdir(cache); + if (is_root) { + (void)chown(parent, geteuid(), (gid_t)-1); + } + ipc_test_remove_flat_dir(parent); + cbm_daemon_ipc_posix_uid_zero_unmapped_override_set_for_test(-1); + + ASSERT_TRUE(paths_ok); + ASSERT_TRUE(ancestor_ready); + ASSERT_TRUE(refused); + PASS(); +} +#endif /* __linux__ */ #endif /* !_WIN32 */ SUITE(daemon_ipc) { @@ -5019,5 +5136,13 @@ SUITE(daemon_ipc) { RUN_TEST(daemon_ipc_posix_private_directory_rejects_world_writable_ancestor); RUN_TEST(daemon_ipc_posix_private_log_rejects_symlinks_and_is_owner_only); RUN_TEST(daemon_ipc_posix_rejects_non_socket_and_symlink_endpoints); +#ifdef __linux__ + RUN_TEST(daemon_ipc_posix_kernel_overflow_uid_matches_proc_sys_kernel); + RUN_TEST(daemon_ipc_posix_uid_zero_unmapped_reflects_real_process_by_default); + RUN_TEST( + daemon_ipc_posix_private_directory_admits_overflow_uid_ancestor_inside_restricted_userns); + RUN_TEST( + daemon_ipc_posix_private_directory_refuses_overflow_uid_ancestor_outside_restricted_userns); +#endif #endif }