From 41faa97706d7edd6601b4c861c6b65e2d8356965 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 7 Jul 2026 19:54:11 +0000 Subject: [PATCH] fix(security): decide daemon auth locality from the accepting socket, not a spoofable REMOTE_ADDR header (1.1.23) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit crated grants full trust (bypassing bearer token AND pool ACL) to "Unix-socket peers" and detected them via req.get_header_value("REMOTE_ADDR").empty(). cpp-httplib stores request headers in a multimap and get_header_value returns the first match, so a remote TCP-listener client sending its own empty REMOTE_ADDR: header could shadow the server value and be mistaken for a local socket peer — a potential complete token-layer bypass (incl. privops uid=0) when the TCP listener is enabled. The same header keyed the rate-limit bucket. The daemon already runs two separate httplib::Server instances (TCP and UDS). registerRoutes now takes an isUnixListener flag and installs a pre-routing handler that erases any client-supplied X-Crated-Listener header and stamps the authoritative value (unix/tcp) from the accepting server instance. isUnixSocketPeer reads that marker instead of REMOTE_ADDR; getClientId keys on the marker plus req.remote_addr (filled by httplib from the accepted socket, not a header). A missing marker reads as untrusted TCP — fail-closed. Structural change on the httplib-coupled auth path (no pure unit surface); compile-gated by the FreeBSD build, warrants a runtime check against a live TCP+UDS daemon. Bumps to 1.1.23; CHANGELOG + trust-model (en/uk) updated; TODO item marked fixed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01X6t6tzVypHye5bDGLxzmZK --- CHANGELOG.md | 30 ++++++++++++++++++++++++++++++ TODO | 29 ++++++++++------------------- cli/args.cpp | 4 ++-- daemon/auth.cpp | 26 +++++++++++++++++--------- daemon/routes.cpp | 31 ++++++++++++++++++++++++++++--- daemon/routes.h | 9 ++++++++- daemon/server.cpp | 9 ++++++--- docs/trust-model.md | 4 ++-- docs/trust-model.uk.md | 4 ++-- 9 files changed, 105 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c2a412..11dbd21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,36 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). --- +## [1.1.23] — 2026-07-07 + +**Security: decide daemon auth locality from the accepting socket, not +a client-supplyable `REMOTE_ADDR` header.** + +`crated` grants full trust (bypassing the bearer token AND the pool +ACL) to "Unix-socket peers", and detected them with +`req.get_header_value("REMOTE_ADDR").empty()`. cpp-httplib stores +request headers in a multimap and `get_header_value` returns the first +match, so a remote client on the TCP listener that sent its own empty +`REMOTE_ADDR:` header could shadow the server-set value and be mistaken +for a local socket peer — a potential complete bypass of the token +layer (including `POST /api/v1/privops/:verb`, uid=0) when the TCP +listener is enabled. The same header keyed the rate-limit bucket, so a +client could also rotate its own bucket key. + +The daemon already runs two separate `httplib::Server` instances (TCP +and Unix socket). `registerRoutes` now takes an `isUnixListener` flag +and installs a pre-routing handler that, on every request, **erases any +client-supplied `X-Crated-Listener` header and stamps the authoritative +value** (`unix` / `tcp`) from the server instance that accepted the +connection. `isUnixSocketPeer` reads that marker instead of +`REMOTE_ADDR`; `getClientId` keys the rate limiter on the marker plus +`req.remote_addr` (the address httplib fills from the accepted socket, +not a header). A missing marker reads as untrusted TCP — fail-closed. + +This is a structural change on the httplib-coupled auth path (no pure +unit surface); it is compile-gated by the FreeBSD build and warrants a +runtime check against a live TCP+UDS daemon. + ## [1.1.22] — 2026-07-07 **Security & robustness: four fixes from a fresh project-wide audit.** diff --git a/TODO b/TODO index ac92965..a2be3ae 100644 --- a/TODO +++ b/TODO @@ -43,27 +43,18 @@ unit suite can't provide, so they are NOT being fixed blind): full FreeBSD workflow (or a self-hosted runner) to validate. Deferred from the 2026-07 second-pass audit (its four clear, testable -findings shipped as 1.1.22; these three remain because they touch the -auth and concurrency paths and want on-hardware validation before +findings shipped as 1.1.22; the auth-locality finding was fixed in +1.1.23 — see below; these two remain because they touch the +concurrency/confinement paths and want on-hardware validation before shipping — do NOT fix blind): -* (security, HIGH if TCP listener enabled) daemon/auth.cpp decides - connection locality — and thus full trust — from a header-map - lookup, `req.get_header_value("REMOTE_ADDR").empty()`, instead of - the connection itself. A UDS peer is treated as unauthenticated-but- - fully-trusted (bypasses bearer token AND pool ACL); the empty-string - signal is what a remote TCP client could shadow by sending its own - empty `REMOTE_ADDR:` header (cpp-httplib stores request headers in a - multimap; get_header_value returns the first match, which may be the - client's). Exploitability depends on the pkg-vendored cpp-httplib - version's handling of client-supplied reserved headers, which is not - in-repo. The daemon already runs SEPARATE TCP and UDS `httplib::Server` - instances (daemon/server.cpp), so the robust fix is to thread an - explicit `isUnixListener` flag into the auth path (or read the - non-spoofable `req.remote_addr` struct field), never a header lookup. - Also fix getClientId() (daemon/routes.cpp) which keys the rate-limit - bucket on the same header. Touches the auth path — validate against a - live TCP+UDS daemon before shipping. + [FIXED in 1.1.23] daemon/auth.cpp connection-locality decided from a + client-supplyable REMOTE_ADDR header. Replaced with a non-spoofable + X-Crated-Listener marker stamped per server instance (TCP vs UDS) by + registerRoutes' pre-routing handler; isUnixSocketPeer and getClientId + now read that marker / req.remote_addr instead of the header. Still + worth exercising against a live TCP+UDS daemon on a FreeBSD host to + confirm the pre-routing handler fires on every path. * (correctness/security, MED) lib/network_lease.cpp + network_lease6.cpp take the exclusive flock on the lease FILE's inode (openLocked opens diff --git a/cli/args.cpp b/cli/args.cpp index 739eeb2..5d078ad 100644 --- a/cli/args.cpp +++ b/cli/args.cpp @@ -753,7 +753,7 @@ Args parseArguments(int argc, char** argv, unsigned &processed) { args.noColor = true; break; } else if (strEq(argv[a], "--version")) { - std::cout << "crate 1.1.22" << std::endl; + std::cout << "crate 1.1.23" << std::endl; exit(0); } else if (auto argShort = isShort(argv[a])) { switch (argShort) { @@ -764,7 +764,7 @@ Args parseArguments(int argc, char** argv, unsigned &processed) { args.logProgress = true; break; case 'V': - std::cout << "crate 1.1.22" << std::endl; + std::cout << "crate 1.1.23" << std::endl; exit(0); default: err("unsupported short option '%s'", argv[a]); diff --git a/daemon/auth.cpp b/daemon/auth.cpp index a69c6fd..fe8d6b5 100644 --- a/daemon/auth.cpp +++ b/daemon/auth.cpp @@ -30,17 +30,25 @@ static std::string sha256hex(const std::string &input) { return "sha256:" + ss.str(); } -// Check if request comes from a Unix socket. cpp-httplib leaves -// REMOTE_ADDR empty for unix-socket peers; that's our signal. +// Check if the request arrived on the Unix-socket listener (local, +// trusted) rather than the TCP listener (remote, token-gated). // -// Tightening to actually verify the peer's uid/gid via -// getpeereid(2) requires the connection's underlying fd, which -// httplib doesn't expose. Until we fork that code path, the unix -// socket file's mode (default 0660 owned by root:wheel) is the -// effective access control. +// 1.1.23: this is decided from the `X-Crated-Listener` marker that +// registerRoutes' pre-routing handler stamps from the accepting server +// instance — AFTER erasing any client-supplied copy — so it is +// authoritative. The previous check keyed on `REMOTE_ADDR.empty()`, +// which a remote TCP client could shadow by sending its own empty +// `REMOTE_ADDR:` header (cpp-httplib stores request headers in a +// multimap; get_header_value returns the first match, the client's). +// A missing marker (pre-routing handler somehow skipped) reads as NOT a +// socket peer → token required → fail-closed. +// +// The unix socket file's mode (default 0660 owned by root:wheel) +// remains the access control for who may reach that listener; a +// getpeereid(2) uid/gid check would need the connection fd, which +// httplib doesn't expose (see the libnv privops socket for that). static bool isUnixSocketPeer(const httplib::Request &req) { - auto remoteAddr = req.get_header_value("REMOTE_ADDR"); - return remoteAddr.empty(); + return req.get_header_value("X-Crated-Listener") == "unix"; } bool isAuthorized(const httplib::Request &req, const Config &config, diff --git a/daemon/routes.cpp b/daemon/routes.cpp index 58d04d1..5b3dd35 100644 --- a/daemon/routes.cpp +++ b/daemon/routes.cpp @@ -73,8 +73,14 @@ static bool checkRateLimit(const std::string &clientId, const std::string &endpo } static std::string getClientId(const httplib::Request &req) { - auto addr = req.get_header_value("REMOTE_ADDR"); - return addr.empty() ? "unix" : addr; + // 1.1.23: key the rate-limit bucket on trustworthy signals, not a + // client-supplyable REMOTE_ADDR header (which let a client rotate its + // own bucket key). Unix-socket peers share one "unix" bucket; TCP + // peers key on req.remote_addr — the authoritative address httplib + // fills from the accepted socket, which a client cannot spoof. + if (req.get_header_value("X-Crated-Listener") == "unix") + return "unix"; + return req.remote_addr.empty() ? "tcp" : req.remote_addr; } // Cap constants live in daemon/rate_limit.h since 0.7.15. @@ -1023,7 +1029,26 @@ static void handlePrivOp(const httplib::Request &req, httplib::Response &res, // --- Route registration --- -void registerRoutes(httplib::Server &srv, const Config &config) { +void registerRoutes(httplib::Server &srv, const Config &config, + bool isUnixListener) { + // 1.1.23: stamp a non-spoofable listener marker on EVERY request + // before it reaches a handler. Trust locality — "this peer arrived on + // the root-owned Unix socket, treat as local admin" — must be decided + // from which server instance accepted the connection, never from a + // client-supplyable REMOTE_ADDR header (a remote TCP client could send + // its own empty REMOTE_ADDR and be mistaken for a socket peer). We + // erase any client-sent copy of the marker first, then set the + // authoritative value; auth (isUnixSocketPeer) and getClientId read + // it. If the pre-routing handler ever fails to run, the marker is + // absent → treated as untrusted TCP → fail-closed. + srv.set_pre_routing_handler( + [isUnixListener](const httplib::Request &req, httplib::Response &) { + auto &headers = const_cast(req).headers; + headers.erase("X-Crated-Listener"); + headers.emplace("X-Crated-Listener", isUnixListener ? "unix" : "tcp"); + return httplib::Server::HandlerResponse::Unhandled; + }); + // Health check (no rate limit) srv.Get("/healthz", [](const httplib::Request &, httplib::Response &res) { res.set_content("{\"status\":\"ok\"}", "application/json"); diff --git a/daemon/routes.h b/daemon/routes.h index f7620a9..c9b956a 100644 --- a/daemon/routes.h +++ b/daemon/routes.h @@ -11,6 +11,13 @@ namespace httplib { class Server; } namespace Crated { -void registerRoutes(httplib::Server &srv, const Config &config); +// 1.1.23: isUnixListener says which server instance this is — the +// Unix-socket listener (local, trusted) or the TCP listener (remote, +// token-gated). It is stamped onto every request as a non-spoofable +// marker so auth locality is decided from the accepting socket, not a +// client-supplyable REMOTE_ADDR header. Defaults to false (fail-closed: +// an un-flagged caller is treated as untrusted TCP). +void registerRoutes(httplib::Server &srv, const Config &config, + bool isUnixListener = false); } diff --git a/daemon/server.cpp b/daemon/server.cpp index 4026f87..339daa0 100644 --- a/daemon/server.cpp +++ b/daemon/server.cpp @@ -41,8 +41,9 @@ Server::Server(const Config &config) impl_->httpSrv = std::make_unique(); } - // Register all REST routes - registerRoutes(*impl_->httpSrv, config_); + // Register all REST routes. This is the TCP listener (remote, + // token-gated) — isUnixListener = false. + registerRoutes(*impl_->httpSrv, config_, /*isUnixListener=*/false); } Server::~Server() { @@ -66,7 +67,9 @@ void Server::start() { impl_->unixThread = std::thread([this]() { httplib::Server udsSrv; - registerRoutes(udsSrv, config_); + // This is the Unix-socket listener (local, root-owned socket) — + // isUnixListener = true, so its peers are treated as trusted. + registerRoutes(udsSrv, config_, /*isUnixListener=*/true); udsSrv.set_address_family(AF_UNIX); udsSrv.listen(config_.unixSocket, 0); }); diff --git a/docs/trust-model.md b/docs/trust-model.md index a1d9061..80a1bbe 100644 --- a/docs/trust-model.md +++ b/docs/trust-model.md @@ -4,7 +4,7 @@ operators on one machine) and contributors extending the privileged surface. -**Applies to:** 1.1.22 (rootless model + per-tenant authz series 1.1.12 → +**Applies to:** 1.1.23 (rootless model + per-tenant authz series 1.1.12 → 1.1.17 covering every privops verb that carries an operator-controlled ownership signal). For the ≤ 0.9.x setuid model and the migration, see [`rootless-migration.md`](rootless-migration.md). @@ -62,7 +62,7 @@ surface; 1.0.0 removed the setuid bit (`Makefile`, comment at the operator and delegates privileged operations to crated(8)"*). The single-trust-domain property did **not** disappear — it relocated. -Reasoning about isolation on 1.1.22 means reasoning about who can reach +Reasoning about isolation on 1.1.23 means reasoning about who can reach **privops**, not who can run `crate(1)`. --- diff --git a/docs/trust-model.uk.md b/docs/trust-model.uk.md index 9ba8ca3..582beca 100644 --- a/docs/trust-model.uk.md +++ b/docs/trust-model.uk.md @@ -4,7 +4,7 @@ (кілька операторів на одній машині), і контрибʼютори, які розширюють привілейовану поверхню. -**Стосується:** 1.1.22 (rootless-модель + серія per-tenant authz 1.1.12 → +**Стосується:** 1.1.23 (rootless-модель + серія per-tenant authz 1.1.12 → 1.1.17 покриває кожен privops-верб з operator-controlled ownership- сигналом). Про ≤ 0.9.x setuid-модель і міграцію див. [`rootless-migration.md`](rootless-migration.md). @@ -62,7 +62,7 @@ privops-сокета, ніколи admin-токен і ніколи Unix-сок privileged operations to crated(8)»*). Властивість «єдиний домен довіри» **не зникла** — вона переїхала. -Міркувати про ізоляцію на 1.1.22 — це міркувати про те, хто має доступ +Міркувати про ізоляцію на 1.1.23 — це міркувати про те, хто має доступ до **privops**, а не хто може запустити `crate(1)`. ---