From ceb95f55ccde2999f58ab7133ea242008a21dde8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 5 Jul 2026 17:56:49 +0000 Subject: [PATCH] fix(privops): fail closed when getpeereid fails on the libnv socket (1.1.19, security) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit daemon/privops_listener.cpp left peerUid = 0 on a getpeereid failure and proceeded to dispatch. With rootless_per_user enabled the authorize-before-dispatch gate is `if (rootlessPerUser && operatorUid > 0)`, so peerUid = 0 was treated as the admin/host-wide path and EVERY per-tenant gate was skipped — a fail-open on identity loss (e.g. the peer exiting between accept and getpeereid, or a kernel edge). The old comment even asserted "getpeereid only fails on non-AF_UNIX sockets which we don't accept", which is not guaranteed. The listener now rejects the connection with 403 when getpeereid fails AND rootless_per_user is on, instead of running the verb unauthenticated. When per-user enforcement is off there is nothing to gate (peerUid only fed the audit trail), so a failure there stays a harmless no-op and the connection proceeds unchanged. A root operator whose getpeereid SUCCEEDS with uid 0 is unaffected — that remains the intended admin treatment; only a genuine getpeereid FAILURE is rejected. errno is captured before applyConnectionRights (Capsicum) can clobber it, for the diagnostic log. Closes the "fail closed on identity loss" guardrail in docs/trust-model.md (item 3). Note: this is daemon-side (crated), FreeBSD-only, and not exercised by the automated lite CI (which links only crate(1)); verified by careful review + the manual full FreeBSD build. docs/trust-model.{md,uk.md} mark the guardrail done; CHANGELOG [1.1.19] + --version bump. https://claude.ai/code/session_01X6t6tzVypHye5bDGLxzmZK --- CHANGELOG.md | 19 +++++++++++++++++++ cli/args.cpp | 4 ++-- daemon/privops_listener.cpp | 28 ++++++++++++++++++++++++---- docs/trust-model.md | 14 +++++++++----- docs/trust-model.uk.md | 14 +++++++++----- 5 files changed, 63 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8962e1e..f749429 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,25 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). --- +## [1.1.19] — 2026-06-10 + +**Security: fail closed when `getpeereid` fails on the privops libnv +socket.** `daemon/privops_listener.cpp` left `peerUid = 0` on a +`getpeereid` failure and proceeded to dispatch. With +`rootless_per_user` enabled the authorize-before-dispatch gate keys on +`operatorUid > 0`, so `peerUid = 0` was treated as the admin/host-wide +path and **every** per-tenant gate was skipped — a fail-open on +identity loss (e.g. the peer exiting between `accept` and `getpeereid`). + +The listener now rejects such a connection with `403` when per-user +enforcement is on, rather than running the verb unauthenticated. When +`rootless_per_user` is off there is nothing to gate (`peerUid` only fed +the audit trail), so a failure there is harmless and the connection +proceeds unchanged. A root operator whose `getpeereid` *succeeds* with +uid 0 is unaffected — that path is still the intended admin treatment; +only a genuine `getpeereid` *failure* is rejected. Closes the +"fail closed on identity loss" guardrail in `docs/trust-model.md`. + ## [1.1.18] — 2026-06-10 **Fix: per-user network sub-CIDR silently aliased colliding uids.** diff --git a/cli/args.cpp b/cli/args.cpp index d65c9ce..c66c7a9 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.18" << std::endl; + std::cout << "crate 1.1.19" << 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.18" << std::endl; + std::cout << "crate 1.1.19" << std::endl; exit(0); default: err("unsupported short option '%s'", argv[a]); diff --git a/daemon/privops_listener.cpp b/daemon/privops_listener.cpp index 55adec5..7ece00c 100644 --- a/daemon/privops_listener.cpp +++ b/daemon/privops_listener.cpp @@ -84,22 +84,42 @@ void writeErrorResponse(int fd, int status, const std::string &body) { void handleConnection(int connFd, bool rootlessPerUser) { // getpeereid (unix-socket peer credentials). uint32_t peerUid = 0; + bool havePeerUid = false; + int peerErrno = 0; { uid_t uid = (uid_t)-1; gid_t gid = (gid_t)-1; if (::getpeereid(connFd, &uid, &gid) == 0) { peerUid = (uint32_t)uid; + havePeerUid = true; + } else { + peerErrno = errno; // capture before applyConnectionRights clobbers it } - // Failure leaves peerUid = 0 — audit hook is no-op, dispatch - // still works, but the operator gets no per-user trail. This - // is the correct degradation: getpeereid only fails on - // non-AF_UNIX sockets which we don't accept. } // Capsicum: limit fd to recv/send/shutdown. Same rationale as // control_socket plane (0.7.14). Sandbox::applyConnectionRights(connFd); + // Fail closed on identity loss. When per-user enforcement is on, the + // authorize-before-dispatch gate keys on peerUid; a getpeereid + // failure (peer exited between accept and getpeereid, a kernel edge) + // must NOT degrade to peerUid = 0, because the dispatcher treats + // uid 0 as the admin/host-wide path and skips EVERY per-tenant gate. + // Reject rather than silently authorize. (When per-user enforcement + // is off there is nothing to gate — peerUid only fed the audit + // trail — so a failure there is harmless and we proceed.) + if (rootlessPerUser && !havePeerUid) { + std::cerr << "privops_listener: getpeereid failed on a rootless " + "per-user connection — rejecting (fail closed): " + << std::strerror(peerErrno) << std::endl; + writeErrorResponse(connFd, 403, + "{\"error\":\"forbidden: could not determine peer uid " + "(getpeereid failed); refusing to run unauthenticated\"}"); + ::close(connFd); + return; + } + nvlist_t *req = nvlist_recv(connFd, 0); if (!req) { writeErrorResponse(connFd, 400, diff --git a/docs/trust-model.md b/docs/trust-model.md index 306de5b..cca3cec 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.18 (rootless model + per-tenant authz series 1.1.12 → +**Applies to:** 1.1.19 (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.18 means reasoning about who can reach +Reasoning about isolation on 1.1.19 means reasoning about who can reach **privops**, not who can run `crate(1)`. --- @@ -329,9 +329,13 @@ security boundary: against `env.pathPrefix`). 3. **Fail closed on identity loss.** On any path that authorizes, a - `getpeereid` failure must deny. It may degrade to a no-op only for - identity-tagged side effects that are not access decisions (e.g. the - audit tail — its current behavior). + `getpeereid` failure must deny. *Done (1.1.19):* the libnv listener + rejects a connection with `403` when `getpeereid` fails and + `rootless_per_user` is on, instead of degrading to `peerUid = 0` + (which the dispatcher would treat as the admin/host-wide path and + skip every per-tenant gate). It degrades to a no-op only when + per-user enforcement is off — there `peerUid` fed only the audit + trail, not an access decision. The path-scoped verbs in (1) remain host-wide for now. A multi-tenant deployment that exposes them to operators directly still needs a diff --git a/docs/trust-model.uk.md b/docs/trust-model.uk.md index 2c5b91a..a1b8f74 100644 --- a/docs/trust-model.uk.md +++ b/docs/trust-model.uk.md @@ -4,7 +4,7 @@ (кілька операторів на одній машині), і контрибʼютори, які розширюють привілейовану поверхню. -**Стосується:** 1.1.18 (rootless-модель + серія per-tenant authz 1.1.12 → +**Стосується:** 1.1.19 (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.18 — це міркувати про те, хто має доступ +Міркувати про ізоляцію на 1.1.19 — це міркувати про те, хто має доступ до **privops**, а не хто може запустити `crate(1)`. --- @@ -321,9 +321,13 @@ privops безпечним для **взаємно недовірливих** о slash-anchored prefix проти `env.pathPrefix`). 3. **Fail closed при втраті ідентичності.** На будь-якому шляху, що - авторизує, збій `getpeereid` має давати deny. Деградувати до no-op - можна лише для side-effect'ів з тегом ідентичності, що не є рішеннями - доступу (напр. audit-хвіст — його теперішня поведінка). + авторизує, збій `getpeereid` має давати deny. *Зроблено (1.1.19):* + libnv-лістенер відбиває з'єднання `403`, коли `getpeereid` падає, а + `rootless_per_user` увімкнено, замість деградації до `peerUid = 0` + (що диспетчер трактував би як admin/host-wide шлях і пропустив би + кожен пер-тенант гейт). Деградує до no-op лише коли пер-user + enforcement вимкнено — там `peerUid` живив тільки audit-хвіст, не + рішення доступу. Доки jid-scoped верби з (1) не гейтнуті, мультитенантне розгортання, де оператори мають самі створювати jail'и, має посередничати створення jail