mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-04 10:48:20 +00:00
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 <martin.vogel.tech@gmail.com>
This commit is contained in:
1 parent
469c3dd996
commit
f9038b84eb
3 files changed
+30
-4
No files matched your search
+11
-1
@@ -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) {
|
||||
|
||||
+13
-1
@@ -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
|
||||
|
||||
+6
-2
@@ -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);
|
||||
|
||||
Reference in new issue
Block a user