From 469c3dd996f23cdb629f1bbd6a751bc0f29a99f2 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 13 Sep 2026 09:20:11 +0200 Subject: [PATCH 1/5] fix(test): exec the #1830 userns probe instead of forking a primed process The single-uid userns smoke test forked a child, unshared CLONE_NEWUSER, wrote a 1:1 uid_map, then called cbm_daemon_ipc_private_directory_secure() directly in that child. The call cannot give the right answer there. posix_ancestor_overflow_uid() in src/daemon/ipc.c derives the overflow uid once per process under pthread_once, and the comment above it calls the /proc state "immutable". unshare(CLONE_NEWUSER) is precisely what changes /proc/self/uid_map, and pthread_once state survives fork(). Seven earlier call sites in this suite run the derivation first, so by the time the test forks, the child has already inherited the HOST answer -- no overflow uid -- and refuses the directory no matter what its namespace says. The test therefore asserted a product behaviour it never executed, and failed as a product refusal (exit 1) while the product was correct: production daemons start inside their namespace and derive the right answer on first use. Re-exec /proc/self/exe into a new --userns-secure-probe mode so the decision is made by a process that STARTED in the namespace, which is also the only shape production ever takes. A failed exec gets its own exit code 3 and a named FAIL, so a broken harness can never be misread as a quiet refusal. Verified both directions on linux/arm64 in the container leg as uid 1001: reverted, 48 passed / 1 failed on "expected WEXITSTATUS(status) == 1"; with the fix, 49 passed, 0 skipped. The non-root half of that recipe is now written down at the test, because it is the part that fooled me first: as root the mapped range covers uid 0, so root-owned /tmp stays 1:1 inside the namespace and never turns overflow. The first run of this reproduction passed with the fix REVERTED for exactly that reason. Only a non-root uid reaches the branch the test exists to cover. Signed-off-by: Martin Vogel --- tests/test_daemon_ipc.c | 33 +++++++++++++++++++++++++++++---- tests/test_main.c | 16 ++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/tests/test_daemon_ipc.c b/tests/test_daemon_ipc.c index e9f0a9046..9a10f838c 100644 --- a/tests/test_daemon_ipc.c +++ b/tests/test_daemon_ipc.c @@ -5133,7 +5133,16 @@ TEST(daemon_ipc_posix_overflow_ancestor_tolerated_only_in_single_uid_ns_issue183 * blocks unshare(CLONE_NEWUSER) on the Colima container leg; some kernels ship * user namespaces disabled. WHAT WAS TRIED when it skips: fork + unshare * CLONE_NEWUSER + a 1:1 uid_map write, which the sandbox denied (EPERM). The - * deterministic decision coverage above is what binds the fix. */ + * deterministic decision coverage above is what binds the fix. + * + * TO REPRODUCE IT LOCALLY two things are needed, and the second is easy to + * miss: run the container with --security-opt seccomp=unconfined so unshare is + * permitted, AND run the suite as a NON-ROOT uid. As root the mapped range + * covers uid 0, so root-owned /tmp stays 1:1 inside the namespace, never turns + * overflow, and this test passes without ever reaching the branch it exists to + * cover -- it passes just as happily with the fix reverted. Under `su tester` + * (uid 1001, the shape CI's runner user has) the ancestors do go overflow and + * the assertion becomes real. */ TEST(daemon_ipc_posix_single_uid_userns_real_smoke_issue1830) { #if defined(__linux__) uid_t host_uid = geteuid(); @@ -5164,9 +5173,20 @@ TEST(daemon_ipc_posix_single_uid_userns_real_smoke_issue1830) { _exit(2); } /* Inside the ns / and /tmp now show the overflow uid. With #1830 the - * daemon can still build its private tree there; without it, refused. */ - bool secured = cbm_daemon_ipc_private_directory_secure(probe); - _exit(secured ? 0 : 1); + * daemon can still build its private tree there; without it, refused. + * + * EXEC, do not just call. The overflow uid is derived once per process + * (pthread_once) from /proc/self/uid_map, and that state survives + * fork(): seven earlier call sites in this suite prime it with the HOST + * answer, so a forked child keeps "no overflow uid" and refuses no + * matter what its namespace says. That made this test fail on the only + * platform where it actually runs (ubuntu-22.04; macOS compile-gates it + * out, Colima's seccomp blocks unshare, and 23.10+ restricts + * unprivileged userns) -- while looking like a product regression. + * Re-exec so the decision is made by a process that STARTED here, which + * is also the only shape production ever takes. */ + (void)execl("/proc/self/exe", "test-runner", "--userns-secure-probe", probe, (char *)NULL); + _exit(3); /* exec failed -- distinct from secure(0)/refused(1)/skip(2) */ } if (child < 0) { FAIL("fork failed for userns smoke"); @@ -5178,6 +5198,11 @@ TEST(daemon_ipc_posix_single_uid_userns_real_smoke_issue1830) { if (WIFEXITED(status) && WEXITSTATUS(status) == 2) { SKIP_PLATFORM("user namespaces unavailable (no CLONE_NEWUSER / seccomp-blocked)"); } + /* A failed re-exec must never read as a product refusal: 3 is its own code + * so a broken probe is a loud harness failure, not a quiet "refused". */ + if (WIFEXITED(status) && WEXITSTATUS(status) == 3) { + FAIL("userns smoke could not re-exec /proc/self/exe for the probe"); + } ASSERT_TRUE(WIFEXITED(status)); ASSERT_EQ(0, WEXITSTATUS(status)); PASS(); diff --git a/tests/test_main.c b/tests/test_main.c index 42848f991..463a41c0a 100644 --- a/tests/test_main.c +++ b/tests/test_main.c @@ -943,6 +943,22 @@ int main(int argc, char **argv) { (void)puts("codebase-memory-mcp test-runner"); return 0; } + /* #1830 userns smoke probe -- see the test that spawns it. + * + * The ancestor overflow uid is derived ONCE per process (pthread_once in + * src/daemon/ipc.c) from /proc/self/uid_map. That file is not immutable: + * unshare(CLONE_NEWUSER) is exactly what changes it, and pthread_once state + * survives fork(). A forked child therefore keeps the HOST answer and never + * re-derives inside its new namespace, so a fork-only smoke test refuses and + * cannot pass once anything earlier in the suite has primed the cache -- + * seven call sites do. Re-exec into this probe so the decision is made by a + * process that STARTED inside the namespace, which is the production shape + * the test means to cover. */ +#if defined(__linux__) + if (argc == 3 && strcmp(argv[1], "--userns-secure-probe") == 0) { + return cbm_daemon_ipc_private_directory_secure(argv[2]) ? 0 : 1; + } +#endif int mcp_idxfailclosed_rc = tf_maybe_run_mcp_idxfailclosed_probe(argc, argv); if (mcp_idxfailclosed_rc >= 0) { return mcp_idxfailclosed_rc; From f9038b84eb3bf82c2dff4cc4ea7f1ec1d8d13f01 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 13 Sep 2026 09:56:54 +0200 Subject: [PATCH 2/5] fix(test,daemon): make the userns probe revert-sensitive and stop a false invariant shipping Follow-up to the re-exec fix, from the challenger read the frozen scope requires for security-boundary units. Three findings, all addressed here; two direction questions go to the maintainer separately. 1. src/daemon/ipc.c stated an invariant that is FALSE. The comment above posix_ancestor_overflow_uid() read "Derived once from immutable /proc state". /proc/self/uid_map is not immutable -- unshare(CLONE_NEWUSER) is exactly what rewrites it -- and pthread_once state survives fork() already marked done. The hazard was written up in the tests, which is the one place an ipc.c maintainer will never look, while production shipped the opposite claim. The comment now states the real guarantee: the cache is safe only because every caller runs in a freshly exec'd process, and it names what would break it. An audit of every process-creation site in src/ backs that: the daemon double-fork execs (daemon/bootstrap.c), cbm_fork_with_retry execs (foundation/subprocess.c), index workers go through cbm_subprocess_spawn, worker_pool.c never forks, and src/ contains no setns or unshare at all. No production path forks without exec and then reaches this check. 2. The probe now _exit()s instead of returning. The runner is built with -fsanitize=address, so returning runs the atexit chain, and a future leak anywhere in the prologue would exit 23 -- which the parent would have read as a verdict on a security test that has none. 3. The parent now treats 0/1/2/3 as the closed protocol and names anything else with the code it actually saw. A signal or a sanitizer exit is a broken harness, not a refusal, and must not be able to look like one. The probe guard also now matches its caller (linux AND test seams) instead of compiling dead code in a non-seams build. VERIFICATION. The challenger was right that the earlier proof was incomplete: it showed the new test goes green, never that the test still binds the #1830 relaxation. Stubbing posix_compute_ancestor_overflow_uid() to POSIX_NO_OVERFLOW_UID -- the relaxation removed -- and running as uid 1001: tolerance off, new probe 48 passed, 1 failed (refused, exit 1) tolerance on, new probe 49 passed, 0 skipped so the smoke test is revert-sensitive to the production decision, not merely to the test change. Worth recording: both deterministic #1830 tests PASSED with the tolerance stubbed off, because they drive the test-seam override rather than the real derivation. They bind the decision table; only this smoke test binds the wiring. macOS daemon_ipc 53 passed 1 skipped; linux/arm64 49 passed 0 skipped; lint-ci clean. Signed-off-by: Martin Vogel --- src/daemon/ipc.c | 12 +++++++++++- tests/test_daemon_ipc.c | 14 +++++++++++++- tests/test_main.c | 8 ++++++-- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/daemon/ipc.c b/src/daemon/ipc.c index 1243405c7..cec3b60b4 100644 --- a/src/daemon/ipc.c +++ b/src/daemon/ipc.c @@ -1572,7 +1572,17 @@ static void posix_overflow_init_once(void) { #endif /* __linux__ */ /* The overflow uid tolerated for ancestors in this process, or - * POSIX_NO_OVERFLOW_UID when none. Derived once from immutable /proc state. */ + * POSIX_NO_OVERFLOW_UID when none. + * + * Derived once per process. NOT from immutable state, whatever an earlier + * revision of this comment claimed: unshare(CLONE_NEWUSER) rewrites + * /proc/self/uid_map, and pthread_once state survives fork() already marked + * done. A child that forks WITHOUT exec and then changes namespace therefore + * keeps the parent answer. That is safe here only because every caller runs in + * a freshly exec'd process -- the daemon double-fork execs + * (daemon/bootstrap.c), workers go through cbm_subprocess_spawn's fork+exec, + * and src/ contains no setns/unshare at all. If you ever add a fork-without-exec + * path that reaches this check, this cache is WRONG and must go. */ static uid_t posix_ancestor_overflow_uid(void) { #ifdef CBM_ENABLE_TEST_SEAMS if (g_posix_overflow_override_active) { diff --git a/tests/test_daemon_ipc.c b/tests/test_daemon_ipc.c index 9a10f838c..2da597add 100644 --- a/tests/test_daemon_ipc.c +++ b/tests/test_daemon_ipc.c @@ -5203,7 +5203,19 @@ TEST(daemon_ipc_posix_single_uid_userns_real_smoke_issue1830) { if (WIFEXITED(status) && WEXITSTATUS(status) == 3) { FAIL("userns smoke could not re-exec /proc/self/exe for the probe"); } - ASSERT_TRUE(WIFEXITED(status)); + /* {0,1,2,3} is the whole protocol. Any other code -- a sanitizer exit (LSan + * defaults to 23), an abort, a signal -- is a broken harness, not a refusal, + * and must say so with the code it actually saw. */ + if (!WIFEXITED(status)) { + FAIL("userns probe did not exit normally (signalled) -- not a security verdict"); + } + if (WEXITSTATUS(status) != 0 && WEXITSTATUS(status) != 1) { + char unexpected[128]; + (void)snprintf(unexpected, sizeof(unexpected), + "userns probe exited %d -- not a security verdict", + WEXITSTATUS(status)); + FAIL(unexpected); + } ASSERT_EQ(0, WEXITSTATUS(status)); PASS(); #else diff --git a/tests/test_main.c b/tests/test_main.c index 463a41c0a..e25de3026 100644 --- a/tests/test_main.c +++ b/tests/test_main.c @@ -954,9 +954,13 @@ int main(int argc, char **argv) { * seven call sites do. Re-exec into this probe so the decision is made by a * process that STARTED inside the namespace, which is the production shape * the test means to cover. */ -#if defined(__linux__) +#if defined(__linux__) && defined(CBM_ENABLE_TEST_SEAMS) if (argc == 3 && strcmp(argv[1], "--userns-secure-probe") == 0) { - return cbm_daemon_ipc_private_directory_secure(argv[2]) ? 0 : 1; + /* _exit, not return: this process exists to answer ONE boolean. A + * return runs the atexit chain, and the runner is built with + * -fsanitize=address, so a future leak anywhere in the prologue would + * exit 23 and read as a security verdict on a test that has none. */ + _exit(cbm_daemon_ipc_private_directory_secure(argv[2]) ? 0 : 1); } #endif int mcp_idxfailclosed_rc = tf_maybe_run_mcp_idxfailclosed_probe(argc, argv); From d21e6cc762a1816d8c274174c9bba01601b888ee Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 13 Sep 2026 10:21:16 +0200 Subject: [PATCH 3/5] fix(daemon,ci): stop caching the overflow uid, and run the userns guard on the core matrix Three maintainer decisions from the challenger read on the #1830 unit. 1. DELETE THE CACHE (not just fix its comment). posix_ancestor_overflow_uid() memoised via pthread_once behind a comment claiming "immutable /proc state". Both halves were false: unshare(CLONE_NEWUSER) rewrites /proc/self/uid_map, and pthread_once state survives fork() already marked done. A process that forked and then entered a namespace kept the parent verdict and refused a directory it should have accepted. It was harmless only because every caller today runs in a freshly exec'd process -- the daemon double-fork execs, workers go through cbm_subprocess_spawn's fork+exec, worker_pool.c never forks, and src/ contains no setns or unshare at all. That made a security decision depend on an invariant nothing enforced. The next fork-without-exec caller would have inherited a stale verdict silently. Now derived fresh per call: two small /proc reads against an openat + fstat + fchmod + ACL check per path component in the same walk. With a test that would have caught the original bug and needs no namespace. daemon_ipc_posix_overflow_uid_is_never_cached_issue1830 counts real derivations through the production entry point and asserts the count rises on every ancestor check. Re-introducing a cache fails it by name on every platform. Proven both ways on linux/arm64 as uid 1001: cache re-introduced 49 passed 1 failed; removed 50 passed 0 skipped. Worth knowing why that test is not redundant: with the re-exec probe in place, the userns smoke test PASSES even with the cache restored, because the probe is a fresh process and immune to it by construction. The two tests are complementary -- the smoke test binds the namespace wiring, this one binds the absence of the cache. 2. RUN THE GUARD ON THE CORE MATRIX. Ubuntu 23.10+ ships kernel.apparmor_restrict_unprivileged_userns=1, so the #1830 smoke test skipped on every 24.04 leg and its only real-namespace coverage sat on the two ubuntu-22.04 legs. Those are broad-matrix only, so the test ran in NO pull request, and GitHub retires those images on 2027-04-17 -- after which it would have become a permanent silent skip, and a skip is not a red. A deliberate relaxation of a security check was guarded by a test that ran nowhere a change would be reviewed, on a clock. _test.yml now lifts the restriction with one sysctl on every ubuntu leg (hosted runners have passwordless sudo), best-effort so a runner that refuses simply returns to skipping. The O10 skip-whitelist comment no longer presents the AppArmor restriction as immovable. 3. KEEP THE TOLERANCE, CORRECT THE SAFETY ARGUMENT. The comment claimed that in a single-uid map "nobody reachable can have created it or can mutate it". The overflow uid is what EVERY unmapped host uid maps to, not only host root (user_namespaces(7)), so overflow-owned does not prove root-created: on a shared host another local user's sticky directory is indistinguishable from root-owned /tmp from inside the namespace. The tolerance stays, because there is no in-namespace discriminator that could tell them apart -- and narrowing it would refuse the legitimate root-owned 01777 /tmp that #1830 exists to accept, i.e. it would revert the fix in practice. What changes is the claim: the real guarantee is that no principal reachable from inside the namespace can create or mutate such an ancestor, and the leaf is never tolerated as overflow (0700, euid-rechecked), so a hostile host-side ancestor owner is bounded to denial of service and socket-path control. macOS daemon_ipc 53 passed 2 skipped; linux/arm64 50 passed 0 skipped; lint-ci clean. Signed-off-by: Martin Vogel --- .github/workflows/_test.yml | 15 +++++++++ src/daemon/ipc.c | 63 +++++++++++++++++++++++++------------ src/daemon/ipc.h | 6 ++++ tests/test_daemon_ipc.c | 51 +++++++++++++++++++++++++++++- 4 files changed, 114 insertions(+), 21 deletions(-) diff --git a/.github/workflows/_test.yml b/.github/workflows/_test.yml index d915eb243..893cc45e3 100644 --- a/.github/workflows/_test.yml +++ b/.github/workflows/_test.yml @@ -107,6 +107,21 @@ jobs: if: startsWith(matrix.os, 'ubuntu') run: sudo apt-get update && sudo apt-get install -y zlib1g-dev ccache + # Ubuntu 23.10+ ships kernel.apparmor_restrict_unprivileged_userns=1, which + # blocks unshare(CLONE_NEWUSER) for unprivileged users. Without this, the + # #1830 single-uid-userns smoke test SKIPS on every 24.04 leg, and its only + # real-namespace coverage lived on the two ubuntu-22.04 legs -- which are + # broad-matrix only (never PR CI) and which GitHub retires on 2027-04-17. + # A deliberate relaxation of a security check was therefore guarded by a + # test that ran in no PR and was on a clock. Hosted runners have + # passwordless sudo, so one sysctl puts it on the CORE matrix instead. + # Best-effort: a runner that refuses simply returns to skipping. + - name: Permit unprivileged user namespaces (Ubuntu) + if: startsWith(matrix.os, 'ubuntu') + run: | + sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 || \ + echo "could not lift the userns restriction; namespace tests will skip" + - name: Install ccache (macOS) if: startsWith(matrix.os, 'macos') run: command -v ccache >/dev/null 2>&1 || brew install ccache diff --git a/src/daemon/ipc.c b/src/daemon/ipc.c index cec3b60b4..f8d7d70b4 100644 --- a/src/daemon/ipc.c +++ b/src/daemon/ipc.c @@ -1463,9 +1463,21 @@ static char *private_log_directory_path_copy(const char *directory_path) { * * The overflow owner is tolerated for ANCESTORS ONLY, and ONLY when * /proc/self/uid_map is a single-uid map whose sole inside id is our euid. In - * that shape no in-namespace principal can be the overflow owner, so an - * overflow-owned ancestor is exactly as safe as a root-owned one on the host: - * nobody reachable can have created it or can mutate it. A multi-uid map + * that shape no IN-NAMESPACE principal can be the overflow owner. + * + * Be precise about what that does and does not buy, because an earlier version + * of this comment overstated it. The overflow uid is what EVERY unmapped host + * uid maps to, not only host root (user_namespaces(7)), so "overflow-owned" + * does NOT prove "created by root". On a shared host, a directory owned by + * another local user is indistinguishable from root-owned /tmp once you are + * inside the namespace -- and there is no in-namespace discriminator that could + * tell them apart, which is precisely why the tolerance is scoped the way it + * is rather than made smarter. The real guarantee is narrower and still + * sufficient: no principal REACHABLE FROM INSIDE the namespace can create or + * mutate such an ancestor, and the private directory itself is never tolerated + * as overflow (see below), so a hostile host-side owner of an ancestor is + * bounded to denial of service and socket-path control. It cannot reach the + * leaf, which stays 0700 and euid-rechecked. A multi-uid map * (rootless podman with subuids) still shows host root as overflow and is * refused — conservative scope, not the safety argument. There is deliberately * no environment escape hatch. @@ -1545,7 +1557,20 @@ static bool posix_read_small_proc_file(const char *path, char *buffer, size_t ca return true; } +#ifdef CBM_ENABLE_TEST_SEAMS +/* Counts real derivations. A cache here was a security hazard once (see the + * note above posix_ancestor_overflow_uid); the contract test asserts this + * climbs on EVERY call so re-introducing one fails loudly. */ +static unsigned g_posix_overflow_compute_count; +unsigned cbm_daemon_ipc_posix_overflow_compute_count_for_test(void) { + return g_posix_overflow_compute_count; +} +#endif + static uid_t posix_compute_ancestor_overflow_uid(void) { +#ifdef CBM_ENABLE_TEST_SEAMS + g_posix_overflow_compute_count++; +#endif char map[256]; if (!posix_read_small_proc_file("/proc/self/uid_map", map, sizeof(map)) || !posix_uid_map_is_single_uid(map, geteuid())) { @@ -1564,25 +1589,24 @@ static uid_t posix_compute_ancestor_overflow_uid(void) { return (uid_t)overflow; } -static uid_t g_posix_overflow_cached = POSIX_NO_OVERFLOW_UID; -static pthread_once_t g_posix_overflow_once = PTHREAD_ONCE_INIT; -static void posix_overflow_init_once(void) { - g_posix_overflow_cached = posix_compute_ancestor_overflow_uid(); -} #endif /* __linux__ */ -/* The overflow uid tolerated for ancestors in this process, or - * POSIX_NO_OVERFLOW_UID when none. +/* The overflow uid tolerated for ancestors, or POSIX_NO_OVERFLOW_UID when none. * - * Derived once per process. NOT from immutable state, whatever an earlier - * revision of this comment claimed: unshare(CLONE_NEWUSER) rewrites + * DERIVED FRESH ON EVERY CALL, deliberately. This used to memoise via + * pthread_once behind a comment claiming "immutable /proc state". That claim + * was false in both halves: unshare(CLONE_NEWUSER) rewrites * /proc/self/uid_map, and pthread_once state survives fork() already marked - * done. A child that forks WITHOUT exec and then changes namespace therefore - * keeps the parent answer. That is safe here only because every caller runs in - * a freshly exec'd process -- the daemon double-fork execs - * (daemon/bootstrap.c), workers go through cbm_subprocess_spawn's fork+exec, - * and src/ contains no setns/unshare at all. If you ever add a fork-without-exec - * path that reaches this check, this cache is WRONG and must go. */ + * done -- so a process that forked and then changed namespace kept the parent + * answer and refused a directory it should have accepted. It happened to be + * harmless because every caller today runs in a freshly exec'd process, but + * that made a security decision depend on an invariant nothing enforced, and + * the next fork-without-exec caller would have silently inherited a stale + * verdict. + * + * The cost of not caching is two small /proc reads per ancestor check, against + * an openat + fstat + fchmod + ACL check per path component in the same walk. + * Do not re-introduce a cache here; the contract test counts derivations. */ static uid_t posix_ancestor_overflow_uid(void) { #ifdef CBM_ENABLE_TEST_SEAMS if (g_posix_overflow_override_active) { @@ -1590,8 +1614,7 @@ static uid_t posix_ancestor_overflow_uid(void) { } #endif #if defined(__linux__) - (void)pthread_once(&g_posix_overflow_once, posix_overflow_init_once); - return g_posix_overflow_cached; + return posix_compute_ancestor_overflow_uid(); #else return POSIX_NO_OVERFLOW_UID; #endif diff --git a/src/daemon/ipc.h b/src/daemon/ipc.h index 9bd8d8a54..545ecb357 100644 --- a/src/daemon/ipc.h +++ b/src/daemon/ipc.h @@ -131,6 +131,12 @@ bool cbm_daemon_ipc_posix_uid_map_is_single_uid_for_test(const char *uid_map, un bool cbm_daemon_ipc_posix_ancestor_stat_ok_for_test(unsigned long owner, unsigned int mode, unsigned long euid, bool overflow_active, unsigned long overflow_uid); +#if defined(__linux__) +/* Number of REAL overflow-uid derivations so far. The value is meaningless on + * its own; the point is that it must rise on every ancestor check, proving no + * cache has crept back in. */ +unsigned cbm_daemon_ipc_posix_overflow_compute_count_for_test(void); +#endif #endif #endif diff --git a/tests/test_daemon_ipc.c b/tests/test_daemon_ipc.c index 2da597add..b451f9bb3 100644 --- a/tests/test_daemon_ipc.c +++ b/tests/test_daemon_ipc.c @@ -5124,6 +5124,49 @@ TEST(daemon_ipc_posix_overflow_ancestor_tolerated_only_in_single_uid_ns_issue183 PASS(); } +/* #1830 regression guard, and the test that would have caught the original bug + * with no namespace at all. The overflow uid used to be memoised per process + * via pthread_once. That made a SECURITY decision sticky: pthread_once state + * survives fork(), unshare(CLONE_NEWUSER) rewrites /proc/self/uid_map, so a + * process that forked and then entered a namespace kept the parent verdict. + * Nothing in the suite could see it, because every deterministic #1830 test + * drives the override seam rather than the real derivation -- they bind the + * decision TABLE, not the WIRING. + * + * So assert the wiring directly: two ancestor checks must perform two real + * derivations. Re-introduce any cache and the second call derives zero times + * and this fails by name, on every platform, with no namespace required. */ +TEST(daemon_ipc_posix_overflow_uid_is_never_cached_issue1830) { +#if defined(__linux__) + char parent[TEST_PATH_CAP]; + if (!ipc_test_parent_new(parent, "overflow-nocache")) { + FAIL("could not create the probe parent directory"); + } + char probe[TEST_PATH_CAP]; + (void)snprintf(probe, sizeof(probe), "%s/x", parent); + + /* The override seam short-circuits derivation, so it must be OFF here or + * the test would measure nothing. */ + cbm_daemon_ipc_posix_set_ancestor_overflow_uid_for_test(false, 0); + + unsigned before = cbm_daemon_ipc_posix_overflow_compute_count_for_test(); + (void)cbm_daemon_ipc_private_directory_secure(probe); + unsigned after_first = cbm_daemon_ipc_posix_overflow_compute_count_for_test(); + (void)cbm_daemon_ipc_private_directory_secure(probe); + unsigned after_second = cbm_daemon_ipc_posix_overflow_compute_count_for_test(); + + (void)rmdir(probe); + ipc_test_remove_flat_dir(parent); + + ASSERT_TRUE(after_first > before); + /* The decisive half: the SECOND call must derive again. */ + ASSERT_TRUE(after_second > after_first); + PASS(); +#else + SKIP_PLATFORM("the overflow-uid derivation is Linux-only"); +#endif +} + /* #1830 real end-to-end smoke: inside a single-uid user namespace the * root-owned ancestors (/, /tmp) are overflow-owned, and the daemon must still * create its private directory there. An overflow-owned ancestor cannot be @@ -5131,7 +5174,12 @@ TEST(daemon_ipc_posix_overflow_ancestor_tolerated_only_in_single_uid_ns_issue183 * namespace is actually available. O10 whitelisted-skip everywhere it is not: * macOS has no user namespaces (compile-gated out); Docker's default seccomp * blocks unshare(CLONE_NEWUSER) on the Colima container leg; some kernels ship - * user namespaces disabled. WHAT WAS TRIED when it skips: fork + unshare + * user namespaces disabled. NOT on that list any more: Ubuntu 23.10+ restricts + * unprivileged userns via AppArmor, which used to confine this test to the two + * ubuntu-22.04 legs -- broad-matrix only, so it ran in no PR and sat on + * GitHub's 2027-04-17 retirement clock. _test.yml now lifts that with + * kernel.apparmor_restrict_unprivileged_userns=0 on every ubuntu leg, so this + * runs on the CORE matrix. WHAT WAS TRIED when it skips: fork + unshare * CLONE_NEWUSER + a 1:1 uid_map write, which the sandbox denied (EPERM). The * deterministic decision coverage above is what binds the fix. * @@ -5346,6 +5394,7 @@ SUITE(daemon_ipc) { #ifdef CBM_ENABLE_TEST_SEAMS RUN_TEST(daemon_ipc_posix_uid_map_single_uid_parse_issue1830); RUN_TEST(daemon_ipc_posix_overflow_ancestor_tolerated_only_in_single_uid_ns_issue1830); + RUN_TEST(daemon_ipc_posix_overflow_uid_is_never_cached_issue1830); RUN_TEST(daemon_ipc_posix_single_uid_userns_real_smoke_issue1830); #endif RUN_TEST(daemon_ipc_posix_startup_lock_is_cross_process); From 085017ab2172c2f3d4cd3e27251033ecbc5b814b Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 13 Sep 2026 10:31:45 +0200 Subject: [PATCH 4/5] fix(daemon): reword the overflow-uid note so the security audit stops blocking it scripts/security-audit.sh greps src/ for the fork call literal and blocks any file that is not on the allow-list. The note explaining why the overflow uid is no longer cached quoted that literal while describing the hazard, so the audit refused src/daemon/ipc.c -- on a COMMENT, in a file that performs no such call. Reworded rather than allow-listed. An allow-list entry would have silenced the check for the whole file, permanently, so that one sentence could keep its punctuation; the scanner is right to be blunt here and the comment is the cheaper thing to change. The reason is recorded inline so the next person does not reintroduce it. Signed-off-by: Martin Vogel --- src/daemon/ipc.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/daemon/ipc.c b/src/daemon/ipc.c index f8d7d70b4..a92befb08 100644 --- a/src/daemon/ipc.c +++ b/src/daemon/ipc.c @@ -1596,9 +1596,12 @@ static uid_t posix_compute_ancestor_overflow_uid(void) { * DERIVED FRESH ON EVERY CALL, deliberately. This used to memoise via * pthread_once behind a comment claiming "immutable /proc state". That claim * was false in both halves: unshare(CLONE_NEWUSER) rewrites - * /proc/self/uid_map, and pthread_once state survives fork() already marked - * done -- so a process that forked and then changed namespace kept the parent - * answer and refused a directory it should have accepted. It happened to be + * /proc/self/uid_map, and pthread_once state survives a forked child already + * marked done -- so a process that forked and then changed namespace kept the + * parent answer and refused a directory it should have accepted. + * (Spelled without the call syntax on purpose: scripts/security-audit.sh + * blocks that literal in src/, and an allow-list entry to let a COMMENT pass + * would weaken a real check on a real file.) It happened to be * harmless because every caller today runs in a freshly exec'd process, but * that made a security decision depend on an invariant nothing enforced, and * the next fork-without-exec caller would have silently inherited a stale From ac8f5b8a8137f15e95478be71ee6742e928ba5bc Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 13 Sep 2026 10:35:41 +0200 Subject: [PATCH 5/5] ci(test): report whether unprivileged userns is actually available The sysctl that lifts the AppArmor restriction is best-effort, and the test runner prints per-shard aggregates only -- 'N passed, M skipped', never individual test names. So if the sysctl stopped working, the #1830 namespace test would quietly return to skipping and the log would stay green and look identical. That is the same false assurance that let a security relaxation ship behind a test which never executed. Probe it the way the test does (unprivileged, no sudo) and print the answer, so there is one greppable line of evidence per ubuntu leg. Deliberately not a gate: a runner that cannot lift the restriction should still run everything else. Signed-off-by: Martin Vogel --- .github/workflows/_test.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/_test.yml b/.github/workflows/_test.yml index 893cc45e3..7ccd56bf5 100644 --- a/.github/workflows/_test.yml +++ b/.github/workflows/_test.yml @@ -121,6 +121,17 @@ jobs: run: | sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 || \ echo "could not lift the userns restriction; namespace tests will skip" + # Then SAY whether it worked. The runner prints per-shard aggregates + # ("N passed, M skipped") and never individual test names, so a + # namespace test that silently returns to skipping is invisible in a + # green log -- the same false assurance that let #1830 ship with a + # test that never executed. This line is the evidence. Probed exactly + # as the test does it: unprivileged, no sudo. + if unshare -U -r true 2>/dev/null; then + echo "CBM_USERNS_AVAILABLE=yes" + else + echo "CBM_USERNS_AVAILABLE=no (namespace-dependent tests will skip)" + fi - name: Install ccache (macOS) if: startsWith(matrix.os, 'macos')