From cadea945e4c7de237ef878d2eaff6600bf37c1c7 Mon Sep 17 00:00:00 2001 From: Viorel Munteanu Date: Fri, 4 Sep 2026 13:35:35 +0000 Subject: [PATCH] Fix [integration][net] rootless tests to not assume kernel capability Found via real-device testing (the actual Android target), not assumed: both tests hardcoded assumptions that don't hold on every kernel. 1. "a fresh network namespace has only loopback" assumed exactly 3 lines of /proc/net/dev (2-line header + one "lo" entry) -- the real target device's kernel auto-creates several harmless placeholder tunnel interfaces (sit0, ip6tnl0, ip_vti0, ip6_vti0) in *every* fresh network namespace, alongside loopback. The namespace is still genuinely isolated (confirmed: none of the *host's* real interfaces leak in) -- the test's assumption was just wrong for this kernel. Replaced the exact-count check with a readlink-based /proc/self/ns/net identity comparison (proves genuine isolation regardless of kernel config) plus a simple "loopback is present" check, dropping the brittle count assertion entirely. 2. "pid/uts/ipc namespaces differ from this process's own" assumed all three are always readable via /proc/self/ns/. The real target device has neither PID nor IPC namespace support *as a kernel feature at all* -- confirmed directly: even this test process's own `readlink /proc/self/ns/pid`, run completely outside any container, fails outright there. This matches this project's own already-documented standing lesson (neither CONFIG_CHECKPOINT_RESTORE nor pid namespace support on this target). Comparing against a namespace type the kernel doesn't expose at all wouldn't prove anything either way. Both tests now build their expectations from detect_bwrap_unshare_args() (bwrap.h) -- this host's own live kernel-capability probe, the exact same one build_bwrap_args() itself already gates on -- rather than assuming a fixed set of namespace types is always available. "user" is deliberately excluded from the generic per-type check: build_bwrap_args() never requests --unshare-user when running as root, so asserting on it would be wrong specifically when these tests are run as root (as they are on the real device). Verified: passes repeatably on this dev machine (all 6 namespace types supported, 8 assertions/2 test cases either way -- same coverage as before, just derived instead of hardcoded), full combined suite and meson test both still clean. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Gv3s5jckJKzh6JkMoi2Akz --- tests/integration/test_rootless_run.cpp | 94 ++++++++++++++++++++----- 1 file changed, 76 insertions(+), 18 deletions(-) diff --git a/tests/integration/test_rootless_run.cpp b/tests/integration/test_rootless_run.cpp index d93a5fe..5c22e58 100644 --- a/tests/integration/test_rootless_run.cpp +++ b/tests/integration/test_rootless_run.cpp @@ -19,11 +19,32 @@ // invocation goes through -- mount, resolve, run_bwrap, unmount, cleanup, // all for real, just called in-process instead of via a subprocess. // Confirms bwrap's *default* sandboxing (no -n/-p involved at all) is -// genuinely isolating: a fresh network namespace with no interfaces but -// loopback, and pid/uts/ipc namespaces that differ from this test +// genuinely isolating: a network namespace with its own identity (not the +// host's), with loopback present, and every other namespace type this +// kernel supports namespacing at all also differing from this test // process's own. Needs a real runnable image (find_busybox_fixture()) but // no root -- everything here works the same way a plain `-r image.tar -- // ` already does unprivileged. +// +// **Deliberately checks against this host's own live kernel capability +// (detect_bwrap_unshare_args(), bwrap.h) rather than assuming every +// namespace type is supported, exactly like production code (build_bwrap_args() +// itself) already has to.** Two real, non-obvious findings from running this +// on the actual Android target device, not assumed: (1) that kernel +// auto-creates several harmless placeholder tunnel interfaces (`sit0`, +// `ip6tnl0`, `ip_vti0`, `ip6_vti0`) in *every* fresh network namespace, +// alongside loopback -- confirmed genuinely isolated regardless (no real +// host interfaces present), just not "loopback-only" the way this dev +// machine's own kernel is; an earlier version of this test asserted an +// exact interface count and failed there for exactly this reason. (2) That +// device has neither PID nor IPC namespace support *as a kernel feature at +// all* -- confirmed directly: even this test process's own `readlink +// /proc/self/ns/pid` fails outright there (not merely "unshared or not"), +// matching this project's own already-documented standing lesson that this +// target has neither `CONFIG_CHECKPOINT_RESTORE` nor pid namespace support. +// Comparing against a namespace type the kernel doesn't expose at all +// wouldn't prove anything either way, so both checks below only ever look +// at types `detect_bwrap_unshare_args()` reports as real. #include @@ -35,6 +56,7 @@ #include +#include "bwrap.h" #include "cli_args.h" #include "commands.h" #include "config_file.h" @@ -111,42 +133,78 @@ std::string run_in_fixture(const std::filesystem::path& image, const std::vector } // namespace -TEST_CASE("rootless -r/--run: a fresh network namespace has only loopback", "[integration][net]") { +TEST_CASE("rootless -r/--run: network namespace is genuinely isolated, loopback present", "[integration][net]") { auto image = find_busybox_fixture(); if (!image) { SKIP("no busybox fixture (images/busybox.tar) -- see tests/setup-tests.py"); } + auto supported = detect_bwrap_unshare_args(); + if (std::find(supported.begin(), supported.end(), "--unshare-net") == supported.end()) { + SKIP("this kernel doesn't support net namespaces"); + } ScratchXdgDirs scratch; // bwrap's own sandbox mounts --proc /proc and --dev /dev, but *not* // /sys at all (confirmed directly: `ls /sys/class/net` inside the // sandbox fails outright, "No such file or directory") -- so // /proc/net/dev is what's actually available to enumerate interfaces - // from inside. Format: two header lines, then one ": ..." - // line per interface. - auto output = run_in_fixture( - *image, {"sh", "-c", "echo BEGIN-TEST-OUTPUT; cat /proc/net/dev; echo END-TEST-OUTPUT"}); + // from inside. Format: two header lines, then one ": ..." line + // per interface -- deliberately not asserted on for an exact count + // (see this file's own top-of-file comment for why). + auto output = run_in_fixture(*image, {"sh", "-c", + "echo BEGIN-TEST-OUTPUT; readlink /proc/self/ns/net; " + "cat /proc/net/dev; echo END-TEST-OUTPUT"}); auto lines = extract_marked_lines(output); - REQUIRE(lines.size() == 3); // 2-line header + exactly one interface - CHECK(lines[2].substr(0, lines[2].find(':')).find("lo") != std::string::npos); + REQUIRE(lines.size() >= 3); // ns/net line + 2-line /proc/net/dev header, at least + CHECK(lines[0] != read_own_namespace_link("net")); + + bool has_lo = false; + for (size_t i = 3; i < lines.size(); ++i) { + if (lines[i].substr(0, lines[i].find(':')).find("lo") != std::string::npos) { + has_lo = true; + } + } + CHECK(has_lo); } -TEST_CASE("rootless -r/--run: pid/uts/ipc namespaces differ from this process's own", "[integration][net]") { +TEST_CASE("rootless -r/--run: every kernel-supported namespace type differs from this process's own", + "[integration][net]") { auto image = find_busybox_fixture(); if (!image) { SKIP("no busybox fixture (images/busybox.tar) -- see tests/setup-tests.py"); } + // "user" is excluded: build_bwrap_args() deliberately never requests + // --unshare-user when running as root (bwrap.cpp), so it wouldn't + // actually be isolated in that case even though the kernel might + // support it -- this test runs at whatever privilege invoked it + // (rootful when run via `doas`/on the real device, rootless + // otherwise), so asserting on "user" here would be wrong in the root + // case. "net" is covered by the previous test case, more specifically. + std::vector to_check; + for (const auto& flag : detect_bwrap_unshare_args()) { + std::string type = flag.substr(std::string("--unshare-").size()); + if (type == "pid" || type == "uts" || type == "ipc" || type == "cgroup") { + to_check.push_back(type); + } + } + if (to_check.empty()) { + SKIP("this kernel supports none of pid/uts/ipc/cgroup namespaces"); + } + + std::string script = "echo BEGIN-TEST-OUTPUT; "; + for (const auto& type : to_check) { + script += "readlink /proc/self/ns/" + type + "; "; + } + script += "echo END-TEST-OUTPUT"; + ScratchXdgDirs scratch; - auto output = run_in_fixture(*image, {"sh", "-c", - "echo BEGIN-TEST-OUTPUT; readlink /proc/self/ns/pid; " - "readlink /proc/self/ns/uts; readlink /proc/self/ns/ipc; " - "echo END-TEST-OUTPUT"}); + auto output = run_in_fixture(*image, {"sh", "-c", script}); auto lines = extract_marked_lines(output); - REQUIRE(lines.size() == 3); - CHECK(lines[0] != read_own_namespace_link("pid")); - CHECK(lines[1] != read_own_namespace_link("uts")); - CHECK(lines[2] != read_own_namespace_link("ipc")); + REQUIRE(lines.size() == to_check.size()); + for (size_t i = 0; i < to_check.size(); ++i) { + CHECK(lines[i] != read_own_namespace_link(to_check[i].c_str())); + } }