diff --git a/docs/project.md b/docs/project.md index 48b053f..f7d3020 100644 --- a/docs/project.md +++ b/docs/project.md @@ -233,6 +233,7 @@ Takes callback functions (`IsKnownFn`, `GetDependenciesFn`) so it has no couplin - Async read loop for stdout/stderr with line buffering - Synchronous kill with graceful SIGTERM → SIGKILL escalation (5s timeout) - Unix domain socket for token delivery (scoped by `LOGOS_INSTANCE_ID`) +- **Token-listener authentication (CWE-940):** the socket path is predictable and world-writable, so before writing the auth token `sendTokenToProcess()` verifies the connected peer's credentials. The peer uid must match ours and, when the child pid is known, the peer pid must equal the spawned child — read via `SO_PEERCRED` on Linux and via `getpeereid()` + `getsockopt(SOL_LOCAL, LOCAL_PEERPID)` on macOS, so both platforms enforce the uid + pid gate. A mismatched peer is treated like a failed connect: the token is never written and the send fails closed, so a co-tenant pre-binding the path cannot intercept the secret. The named-path race is closed completely only by a future `socketpair()`-fd handoff. - A `std::mutex` (`s_processesMutex`) protects the `s_processes` map against concurrent access **ModuleContainer interface:** `id()` → `"subprocess"`, `canHandle()`, `launch()`, `sendToken()`, `terminate()`, `terminateAll()`, `hasModule()`, `pid()`, `getAllPids()` diff --git a/docs/spec.md b/docs/spec.md index 6be7033..ce17256 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -97,6 +97,8 @@ Each module runs in its own process for isolation: Since the remote object registry has no built-in security mechanisms, all RPC calls require an authentication token. This is transparent to module developers when using the SDK: 1. **Core → Module**: When a module is loaded, the core generates a UUID token and sends it to the module process via the container's `sendToken()` mechanism (currently a Unix domain socket managed by `SubprocessContainer`). On the child side, `SubprocessTokenReceiver` receives the token before the module loader initializes the plugin. The module uses this token to authenticate calls from the core. + + **Listener authentication (CWE-940).** The token socket lives at a predictable path (`$TMPDIR/logos_token_[_]`) in a world-writable temp directory, so a local co-tenant could pre-bind that path before the legitimate child calls `listen()`. A bare `connect()` only proves *something* is listening — not that it is the child the core spawned. Before writing the token, the parent therefore authenticates the connected peer's kernel-reported credentials: it requires the peer to run as the core's own effective uid, and — when the child's pid is known (the normal load path, where `launch()` records it before `sendToken()` runs) — to be exactly that process. The check uses `SO_PEERCRED` on Linux (uid + pid) and, on macOS, `getpeereid()` for the uid plus `getsockopt(SOL_LOCAL, LOCAL_PEERPID)` for the pid — so both platforms enforce the uid + pid gate. Any mismatch is fatal: the parent refuses to write the token rather than risk leaking it to a squatter, and the send fails. **Residual risk:** when the child pid is unknown (the placeholder path used by some callers) only the uid gate applies, so a same-uid co-tenant could still receive the token; and even with the pid gate a same-uid attacker can in principle race for the path between the child's `listen()` and the parent's `connect()`. Closing that window entirely requires handing the child a pre-connected `socketpair()` fd instead of a named path, eliminating the predictable socket altogether — a planned hardening. The child/receiver side (`SubprocessTokenReceiver`) has a symmetric exposure — it accepts the connecting peer without a credential check — and needs the mirror-image hardening; the `socketpair()` redesign would close both halves at once. 2. **Module → Module**: When modules need to communicate, they request authorization from the Capability Module, which issues a token and notifies both parties. The modules then use this token for subsequent requests. 3. **Token Storage**: Each module stores tokens in a thread-safe `TokenManager` (part of the SDK). `ModuleProxy` validates tokens before dispatching method calls. @@ -139,7 +141,7 @@ Every module ships a `metadata.json` referenced by Qt's `Q_PLUGIN_METADATA` macr a. The `ModuleLoader` resolves the host binary (e.g. `logos_host_qt`) and builds CLI arguments (including `--transport-set` if configured) b. The `ModuleContainer` launches the process with the resolved binary and arguments 7. Core generates a UUID authentication token -8. Core sends the token to the module via the runtime's `sendToken()` (delegates to the container) +8. Core sends the token to the module via the runtime's `sendToken()` (delegates to the container, which authenticates the receiving peer's credentials before writing the secret — see Token-Based Authentication) 9. Host process receives the token via `SubprocessTokenReceiver` (container concern), then loads the module plugin and calls `initLogos(LogosAPI*)` (loader/runtime concern) 10. The `LogosAPI` instance exposes `modulePath`, `instanceId`, and `instancePersistencePath` as properties 11. Host process registers the module with the remote object registry diff --git a/src/containers/subprocess/subprocess_container.cpp b/src/containers/subprocess/subprocess_container.cpp index f561046..ead0581 100644 --- a/src/containers/subprocess/subprocess_container.cpp +++ b/src/containers/subprocess/subprocess_container.cpp @@ -1,3 +1,11 @@ +// _GNU_SOURCE exposes struct ucred / SO_PEERCRED from on glibc. +// Must precede any system header. CMake's default -std=gnu++17 predefines it, +// but pin it here so the peer-credential check below compiles regardless of +// the C++ dialect a downstream consumer builds us with. +#ifndef _GNU_SOURCE +#define _GNU_SOURCE 1 +#endif + #include "subprocess_container.h" #include @@ -14,6 +22,7 @@ #include #include +#include #include #include "unix_socket_path.h" @@ -160,6 +169,132 @@ IoRuntime::~IoRuntime() { // QDir::tempPath() resolution is bypassed on both sides. using ::logos::unixSocketPath; +// --------------------------------------------------------------------------- +// Listener authentication (CWE-940) +// --------------------------------------------------------------------------- +// +// The token socket lives at a predictable path under a world-writable temp +// dir (see unix_socket_path.h) and we connect() to it in a retry loop. A +// successful connect() proves only that *something* is listening there — not +// that it is the child module we spawned. A local co-tenant can pre-bind that +// path before the real child calls listen(); our connect() then lands on the +// attacker's socket and, without this check, write()s the genuine auth token +// straight to them. They replay it as `authToken` to make authorized +// cross-module calls. +// +// Before writing the secret we verify the peer's kernel-reported credentials: +// - the peer must run as our own euid (no other user may receive the token); +// - when we know the child's pid (the common case — launch() records it +// before sendToken() runs), the peer must be exactly that process. +// +// Any failure is fatal: we refuse to write the token rather than risk leaking +// it. This is the sender/parent half of the handoff (CWE-940 / finding F-010); +// the child/receiver side (token_receiver.cpp) has a symmetric exposure that +// still needs the mirror-image peer check. +// +// The peer pid comes from SO_PEERCRED on Linux and getsockopt(SOL_LOCAL, +// LOCAL_PEERPID) on macOS, so both platforms enforce the uid+pid gate. +// +// Residual risk: when no child pid is recorded (e.g. the placeholder path used +// by some callers) only the uid gate applies, so a same-uid co-tenant could +// still receive the token. Even with the pid gate, a same-uid attacker can in +// principle race for the path between the child's listen() and our connect(). +// The race window is fully closed only by handing the child a pre-connected +// socketpair() fd instead of a named path — see the note in docs/spec.md. +// The peer-credential check is the defense-in-depth that closes the cross-uid +// theft this finding describes and, when the child pid is known, the same-uid +// squat as well. +// +// Returns true if the connected peer is acceptable, false if the token must +// not be written. `expectedPid <= 0` means "child pid unknown, skip the pid +// gate" (the uid gate still applies). +bool peerIsTrusted(int sock, int64_t expectedPid, const std::string& name) +{ +#if defined(__linux__) + struct ucred cred{}; + socklen_t len = sizeof(cred); + if (::getsockopt(sock, SOL_SOCKET, SO_PEERCRED, &cred, &len) != 0) { + fprintf(stderr, + "[SubprocessContainer] SO_PEERCRED failed for token peer of %s: %s\n", + name.c_str(), strerror(errno)); + return false; + } + if (cred.uid != ::geteuid()) { + fprintf(stderr, + "[SubprocessContainer] token peer uid mismatch for %s (peer uid %u != %u); " + "refusing to send token\n", + name.c_str(), static_cast(cred.uid), + static_cast(::geteuid())); + return false; + } + if (expectedPid > 0 && static_cast(cred.pid) != expectedPid) { + fprintf(stderr, + "[SubprocessContainer] token peer pid mismatch for %s (peer pid %d != child %lld); " + "refusing to send token\n", + name.c_str(), static_cast(cred.pid), + static_cast(expectedPid)); + return false; + } + return true; +#elif defined(__APPLE__) + // macOS has no SO_PEERCRED/ucred, but it exposes the two facts we need + // through separate APIs: getpeereid() for the peer's effective uid, and + // getsockopt(SOL_LOCAL, LOCAL_PEERPID) for the peer's pid (since macOS + // 10.8). Checking both gives parity with the Linux uid+pid gate, so a + // same-uid co-tenant squatting the path is rejected on the pid mismatch + // just as it is on Linux — not only the cross-uid theft. + uid_t peerUid = 0; + gid_t peerGid = 0; + if (::getpeereid(sock, &peerUid, &peerGid) != 0) { + fprintf(stderr, + "[SubprocessContainer] getpeereid failed for token peer of %s: %s\n", + name.c_str(), strerror(errno)); + return false; + } + if (peerUid != ::geteuid()) { + fprintf(stderr, + "[SubprocessContainer] token peer uid mismatch for %s (peer uid %u != %u); " + "refusing to send token\n", + name.c_str(), static_cast(peerUid), + static_cast(::geteuid())); + return false; + } + // When the child pid is known, enforce it too. LOCAL_PEERPID reports the + // pid of the process that connect()ed the socket — exactly the peer we are + // about to hand the token to. A getsockopt failure (e.g. EOPNOTSUPP on a + // pre-10.8 kernel) is fatal: we fail closed rather than silently downgrade + // to the uid-only gate and leak the token to a same-uid squatter. + if (expectedPid > 0) { + pid_t peerPid = 0; + socklen_t pidLen = sizeof(peerPid); + if (::getsockopt(sock, SOL_LOCAL, LOCAL_PEERPID, &peerPid, &pidLen) != 0) { + fprintf(stderr, + "[SubprocessContainer] LOCAL_PEERPID failed for token peer of %s: %s; " + "refusing to send token\n", + name.c_str(), strerror(errno)); + return false; + } + if (static_cast(peerPid) != expectedPid) { + fprintf(stderr, + "[SubprocessContainer] token peer pid mismatch for %s (peer pid %d != child %lld); " + "refusing to send token\n", + name.c_str(), static_cast(peerPid), + static_cast(expectedPid)); + return false; + } + } + return true; +#else + // Unknown platform: we cannot authenticate the peer, so we cannot make + // the security guarantee. Fail closed rather than leak the token. + (void)sock; (void)expectedPid; + fprintf(stderr, + "[SubprocessContainer] no peer-credential API on this platform; " + "refusing to send token for %s\n", name.c_str()); + return false; +#endif +} + // --------------------------------------------------------------------------- // Async read loop // --------------------------------------------------------------------------- @@ -478,6 +613,13 @@ bool SubprocessContainer::sendTokenToProcess(const std::string& name, using clock = std::chrono::steady_clock; const auto deadline = clock::now() + std::chrono::milliseconds(max_wait_ms); + // The child we spawned, if known. launch() -> startProcess() records the + // child's pid before sendToken() runs, so in the normal load path this is + // the genuine child's pid and lets us reject any other listener squatting + // the predictable socket path. -1 (placeholder / unknown) skips the pid + // gate but still enforces the uid gate in peerIsTrusted(). + const int64_t expectedPid = getProcessId(name); + int sock = -1; for (;;) { sock = ::socket(AF_UNIX, SOCK_STREAM, 0); @@ -490,8 +632,21 @@ bool SubprocessContainer::sendTokenToProcess(const std::string& name, addr.sun_family = AF_UNIX; strncpy(addr.sun_path, path.c_str(), sizeof(addr.sun_path) - 1); - if (::connect(sock, reinterpret_cast(&addr), sizeof(addr)) == 0) - break; + if (::connect(sock, reinterpret_cast(&addr), sizeof(addr)) == 0) { + // connect() only proves something is listening at this predictable, + // world-writable path — not that it is our child. Authenticate the + // peer before writing the secret (CWE-940). A trusted peer ends the + // loop; an untrusted one is treated like a failed connect, so a + // co-tenant squatting the path cannot steal the token and the real + // child can still claim the path before the deadline. + if (peerIsTrusted(sock, expectedPid, name)) + break; + ::close(sock); + sock = -1; + if (clock::now() >= deadline) break; + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + continue; + } ::close(sock); sock = -1; diff --git a/tests/test_subprocess_manager.cpp b/tests/test_subprocess_manager.cpp index 690d973..18ed81c 100644 --- a/tests/test_subprocess_manager.cpp +++ b/tests/test_subprocess_manager.cpp @@ -25,6 +25,7 @@ #include #include // for strncpy #include +#include // for poll() in the impostor accept loop #include #include #include @@ -389,3 +390,134 @@ TEST_F(ProcessManagerTest, SendToken_SucceedsAfterDelay) { EXPECT_TRUE(received.load()); EXPECT_EQ(receivedToken, "hello-token"); } + +// --------------------------------------------------------------------------- +// F-010 (CWE-940): the parent must NOT hand the auth token to an impostor +// that pre-binds the predictable token socket before the real child listens. +// +// Threat model: the token socket path ($TMPDIR/logos_token_) is +// predictable and lives in a world-writable temp dir. A local attacker can +// bind+listen there before the legitimate child module calls listen(). If the +// parent connects and writes the token without checking who is listening, the +// secret leaks and the attacker can replay it for authorized cross-module +// calls. +// +// We replicate the squat with an in-process impostor whose listener pid is the +// test process itself, while a *real* child (a sleep) is registered for the +// same name so the container knows the genuine child's pid. The peer-pid gate +// must reject the impostor: sendTokenToProcess must NOT write the token to it +// and must fail rather than leak the secret. The gate reads the peer pid via +// SO_PEERCRED on Linux and getsockopt(SOL_LOCAL, LOCAL_PEERPID) on macOS, so +// the same in-process impostor is rejected on both platforms — its pid is the +// test process, not the registered child. +// +// Pre-fix (no listener authentication) this test FAILS: the impostor receives +// the token and `ok` is true. Post-fix it passes: the impostor receives +// nothing and `ok` is false. +// +// Limited to Linux and macOS — the two platforms with a peer-credential API +// and a pid gate. On any other platform peerIsTrusted() fails closed without +// inspecting a pid, so this in-process impostor (same uid, different pid) +// cannot exercise the pid gate the test is asserting. +// --------------------------------------------------------------------------- + +#if defined(__linux__) || defined(__APPLE__) +TEST_F(ProcessManagerTest, SendToken_RejectsImpostorListenerSquattingSocket) { + const char* sleep = sleepBinary(); + if (!sleep) GTEST_SKIP() << "sleep binary not found"; + + const std::string name = "f010_impostor"; + const std::string path = tokenSocketPath(name); + ::unlink(path.c_str()); // clear any stale socket + + // The attacker pre-binds the predictable socket path and listens. Use a + // generous backlog and (below) a thread that accepts continuously: the + // parent's connect() is blocking, so an un-drained backlog could wedge it + // and mask the behaviour we're testing. We want connect() to keep + // succeeding so the parent's *authentication* decision is what's exercised. + int impostor = ::socket(AF_UNIX, SOCK_STREAM, 0); + ASSERT_GE(impostor, 0); + struct sockaddr_un addr{}; + addr.sun_family = AF_UNIX; + strncpy(addr.sun_path, path.c_str(), sizeof(addr.sun_path) - 1); + ASSERT_EQ(::bind(impostor, reinterpret_cast(&addr), sizeof(addr)), 0) + << "failed to bind impostor listener at " << path; + ASSERT_EQ(::listen(impostor, 64), 0); + struct SockGuard { + int fd; std::string p; + ~SockGuard() { if (fd >= 0) ::close(fd); ::unlink(p.c_str()); } + } guard{impostor, path}; + + // Spawn a real child for `name` so the container records its (genuine) + // pid. This is what makes the impostor distinguishable: its listener pid + // is this test process, not the registered child. (The impostor shares our + // uid, so the uid gate alone wouldn't reject it; the pid gate is what + // catches the squat — via SO_PEERCRED on Linux, LOCAL_PEERPID on macOS.) + const char* args[] = {"5", nullptr}; + ASSERT_EQ(logos_core_start_process(name.c_str(), sleep, args), 1); + const int64_t childPid = logos_core_get_process_id(name.c_str()); + ASSERT_GT(childPid, 0); + ASSERT_NE(childPid, static_cast(::getpid())) + << "impostor pid must differ from the real child pid for this test to " + "exercise the pid gate"; + + // Impostor accept loop: keep accepting (draining the backlog so the + // parent's blocking connect() never wedges) and read whatever arrives, + // until the main thread signals it is done sending. If the fix works the + // parent rejects us and closes without writing, so every accepted client + // yields zero bytes; pre-fix it writes the token and `stolen` captures it. + std::atomic stop{false}; + std::atomic impostorGotToken{false}; + std::mutex stolenMtx; + std::string stolen; + std::thread thief([&]() { + while (!stop.load()) { + struct pollfd pfd{impostor, POLLIN, 0}; + int pr = ::poll(&pfd, 1, 50); + if (pr <= 0 || !(pfd.revents & POLLIN)) continue; + int client = ::accept(impostor, nullptr, nullptr); + if (client < 0) continue; + std::string chunk; + char buf[256]; + for (;;) { + ssize_t n = ::read(client, buf, sizeof(buf)); + if (n > 0) { chunk.append(buf, static_cast(n)); continue; } + if (n < 0 && errno == EINTR) continue; + break; // EOF or error + } + ::close(client); + if (!chunk.empty()) { + std::lock_guard lk(stolenMtx); + stolen += chunk; + impostorGotToken.store(true); + } + } + }); + + const std::string secret = "f47ac10b-58cc-4372-a567-0e02b2c3d479"; + // Short budget: the legitimate child never binds (we squat the path), so + // after the impostor is rejected sendTokenToProcess runs out its retry + // budget and fails. Keep it small so the test is quick. + const int budget_ms = 400; + bool ok = SubprocessManager::sendTokenToProcess(name, secret, budget_ms); + + // Give the impostor a moment to surface any bytes the parent may have + // written on its final connection before we tear down. + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + stop.store(true); + thief.join(); + + std::string stolenCopy; + { std::lock_guard lk(stolenMtx); stolenCopy = stolen; } + + EXPECT_FALSE(impostorGotToken.load()) + << "SECURITY: auth token leaked to an impostor squatting the token " + "socket — the parent wrote the secret to a listener it never " + "authenticated (CWE-940). Stolen bytes: '" << stolenCopy << "'"; + EXPECT_NE(stolenCopy, secret) + << "SECURITY: impostor received the exact auth token"; + EXPECT_FALSE(ok) + << "sendTokenToProcess must fail rather than hand the token to an " + "unauthenticated peer"; +} +#endif // __linux__ || __APPLE__