From 5ffdcecb2d4c0faebfbdb8faf30894489ab4b8d2 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Tue, 28 Jul 2026 11:32:45 +0200 Subject: [PATCH] F-LEAK: record all three attempts; the ACL fix is consumed by use, the fenced destroy is the fix --- .../CAMPAIGN-8-backup-restore-2026-07-27.md | 67 ++++++++++++++----- documentation/backlog/OPEN-ITEMS.md | 2 +- 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/documentation/audits/CAMPAIGN-8-backup-restore-2026-07-27.md b/documentation/audits/CAMPAIGN-8-backup-restore-2026-07-27.md index 9b84c4b..c75bcc5 100644 --- a/documentation/audits/CAMPAIGN-8-backup-restore-2026-07-27.md +++ b/documentation/audits/CAMPAIGN-8-backup-restore-2026-07-27.md @@ -389,29 +389,60 @@ against the **587 s** the finding's original incident needed a human. The lock-d live during the replay. Scenario B proven on the same box — an `onboot:0` guest was left stopped across the whole window. -### F-LEAK — FIXED (host-install v1.21.0, 2026-07-28) — and the first fix was WRONG -`FelhomAgentGuest` is now granted at each `/vms/990000`…`/vms/990009` — the restore-test's scratch -band. The cause is structural rather than a missing privilege in the role: the role is granted at -`/pool/felhom`, and **a guest only joins that pool when its restore completes**, so a *failed* -restore-test leaves a guest that exists, is in no pool, and is outside the token's reach. +### F-LEAK — FIXED (agent v0.110.0 + host-install v1.21.0, 2026-07-28) — after TWO refuted attempts -**The first attempt was refuted live and is recorded because it looked right.** Agent v0.107.0 shipped -a teardown fallback that adopted the stranded guest into the pool and retried. It fired exactly as -designed and PVE refused it: `PUT /pools/felhom -> HTTP 500: permission denied at /vms/990000 (missing -privilege ...)`. `PUT /pools/{pool}` **also** requires `VM.Allocate` on the VM being added — pool -membership cannot bootstrap its own authority. That code was removed in agent v0.108.0. +**Three attempts, two refuted by live test.** Recorded in full because both wrong answers looked right +and one of them shipped. -**Why the grant is not a widening**, proven live at the same seam the defect lives in — and note that -PVE checks **permission before existence**, so these 403s are genuine refusals, not artifacts: +The cause is structural rather than a missing privilege in the role: `FelhomAgentGuest` is granted at +`/pool/felhom`, and **a guest joins that pool only when its restore completes**, so a *failed* +restore-test leaves a guest that exists, is in no pool, and is out of the token's reach entirely. -| `DELETE /nodes//lxc/` with the agent's own token | result | +**Attempt 1 — adopt the stranded guest into the pool, then retry** (agent v0.107.0, shipped). Fired +exactly as designed and PVE refused it: `PUT /pools/felhom -> HTTP 500: permission denied at /vms/990000 +(missing privilege ...)`. `PUT /pools/{pool}` **also** requires `VM.Allocate` on the VM being added — +**pool membership cannot bootstrap its own authority.** Removed in v0.108.0. + +**Attempt 2 — grant the role per-path at `/vms/990000..990009`** (host-install v1.21.0). It *works*, and +the live A/B proved it: grant removed → `403 (/vms/990000, VM.Allocate)`, guest still present; grant +restored → `200 UPID:...vzdestroy`, guest gone. **But it is durable for exactly one use per slot.** The +post-destroy check showed `/vms/990000` with **0 grants**, and PVE's own source says why: the destroy +path calls `AccessControl::remove_vm_access($vmid)` (`API2/LXC.pm:906`), which deletes every ACL at +`/vms/` (`AccessControl.pm:1898`). **The grant is consumed by the operation it authorises** — after +ten teardowns the band is ungranted and the defect returns. A per-VM ACL is the wrong tool; the pool +grant is durable only because it lives on a *pool* path, which `remove_vm_access` does not touch. + +**Attempt 3 — route the destroy through the authority that already performs the create** (agent +v0.110.0). `Privileged.DestroyScratchLXC` is the **fourth** root-fenced exception, and it is fenced +harder than the other three, in three places none of which is consumed by use: + +| layer | enforcement | |---|---| -| `990000` (stranded, pool-less, restored scratch) | **destroyed** — the defect's exact case | -| `990010` (one past the band) | **403** `Permission check failed (/vms/990010, VM.Allocate)` | -| `100` (arbitrary non-pool guest) | **403** same | +| **sudoers** | `/usr/sbin/pct destroy 99000[0-9] --purge` — sudo matches the vmid **literally** | +| **`DestroyScratchLXC`** | re-checks the band; refuses an unconfigured or inverted band rather than defaulting | +| **`teardownScratch`** | acts only on this journal entry's own scratch provenance | -Granting at `/vms` was rejected: it would authorise destroying every guest on the box, including a -co-tenant's. +The API destroy is still tried **first** and stays the normal path; the v1.21.0 band ACLs stay +provisioned so the common case needs no privileged call at all. This is the fallback that makes teardown +deterministic instead of once-per-slot. + +**The fence, proven live by asking sudo rather than executing** (executing the negative case against a +live guest is precisely what must not happen): + +| command | sudo verdict | +|---|---| +| `pct destroy 990000 / 990005 / 990009 --purge` | **PERMITTED** | +| `pct destroy 9201 --purge` (the live customer guest) | **REFUSED** | +| `pct destroy 9100 / 9999 / 1 --purge` | **REFUSED** | +| `pct destroy 990010 --purge` (one past the band) | **REFUSED** | +| `pct start 990000` / `pct set 990000 -memory 512` | **REFUSED** — the grant is this one op, not this vmid | + +Also proven at the API seam, and worth keeping because it establishes the *shape* of the refusal: a +`DELETE` of the non-existent `990010` returned **403**, not "does not exist" — PVE evaluates +**permission before existence**, so these are genuine authorization refusals rather than artifacts. + +Granting `VM.Allocate` at `/vms` was rejected throughout: it would authorise destroying every guest on +the box, including a co-tenant's. ### F-OBS — FIXED (controller v0.180.0, 2026-07-28) `deadapp-check` now emits a summary line at **INFO** every 20th scan (10 min at its 30 s cadence), diff --git a/documentation/backlog/OPEN-ITEMS.md b/documentation/backlog/OPEN-ITEMS.md index 932d4e1..85cc140 100644 --- a/documentation/backlog/OPEN-ITEMS.md +++ b/documentation/backlog/OPEN-ITEMS.md @@ -28,7 +28,7 @@ State: `BLOCKED` · `READY` · `WAITING-ON-OPERATOR` · `WATCHING`. Every row ha | **F-A1** | ~~A restore-test in flight made a healthy backup report as FAILED (HTTP 409 read as a tier failure): breaker armed + operator emailed, on both boxes~~ | **SHIPPED + PROVEN-LIVE** (controller v0.179.0, 2026-07-28) | — | 409 → contention: tier stays DUE, dropped before anything stops (15m), and BLOCKED alarm if contention outlives the agent's 120m ceiling (3h). Hub DB: 409 → **0** operator emails, real failure → **1** | — | | **R-100** | **A restic offsite tier that fails every night never goes stale on the hub.** `isStale()` reads only `LastRun` (`hub/internal/monitor/offsite.go:120,127,131`) and the controller writes `LastRun` **unconditionally**, outside the error branch (`controller/internal/backup/offbox.go:716`, with `LastStatus="error"` set at :725). So a tier failing nightly keeps a fresh clock, `isStale` is permanently false, and the operator's fleet-wide alarm plane is silent. `LastStatus` reaches the hub and is **only logged** (`offsite.go:270`) — it drives no checker and no notification | READY (S) | — | **This is F-CRIT-2's defect class on the hub, for the restic tier**: a failed run resetting the freshness clock. Gate staleness on the last *successful* run, not the last attempt. Found by Part 4 investigation 2026-07-28; NOT fixed (investigation-only scope) | CC | | **F-REBOOT** | ~~A guest rebooted during its backup does not come back — shutdown completes, start never happens, no self-heal; 9m47s total appliance outage with every alarm silent~~ | **SHIPPED + PROVEN-LIVE** (agent v0.107.0, 2026-07-28) | — | 60 s guest-power watchdog; `onboot` is the deliberate-stop discriminator (already the stale-lock path's, and what `pve-guests` consults), retry bounded 3x/1m-2m-4m then escalates once. Live on demo-hp: **120 s unattended** vs the incident's 587 s with a human; Scenario B proven (an `onboot:0` guest left stopped) | — | -| **F-LEAK** | ~~A failed restore-test cannot destroy its own scratch guest (403 `VM.Allocate`); the 10-slot VMID band shrinks silently~~ | **SHIPPED + PROVEN-LIVE** (host-install v1.21.0, 2026-07-28) | — | `FelhomAgentGuest` granted per-path at `/vms/990000..990009`. Cause was structural: the role is granted at `/pool/felhom` and a guest joins the pool only when its restore **completes**. Live A/B on demo-hp, same guest minutes apart: grant removed → `403 (/vms/990000, VM.Allocate)`; granted → `200 UPID:...vzdestroy`. `/vms/100` and `/vms/990010` still **403** (PVE checks permission before existence, so these are real refusals) | — | +| **F-LEAK** | ~~A failed restore-test cannot destroy its own scratch guest (403 `VM.Allocate`); the 10-slot VMID band shrinks silently~~ | **SHIPPED + PROVEN-LIVE** (agent v0.110.0 + host-install v1.21.0, 2026-07-28) | — | **Three attempts, two refuted live.** (1) Pool adoption: `PUT /pools/{pool}` also needs `VM.Allocate` on the VM — membership cannot bootstrap its own authority. (2) Per-path `/vms/990000..990009` ACLs: work, but PVE's destroy calls `remove_vm_access` (`LXC.pm:906`) which deletes every ACL at `/vms/` — **consumed by the op it authorises**, one use per slot. (3) SHIPPED: 4th root-fenced exception, band enforced in sudoers **literally** (`pct destroy 99000[0-9] --purge`) + in code + at the caller; API destroy still tried first. Live: band PERMITTED, `9201`/`9100`/`9999`/`990010`/`1` REFUSED, and `pct start 990000` REFUSED too | — | | **F-OBS** | ~~`deadapp-check` leaves NO positive observable on a default (info-level) box — "no alarms" was indistinguishable from "never ran"~~ | **SHIPPED + PROVEN-LIVE** (controller v0.180.0 + agent v0.109.0, 2026-07-28) | — | INFO summary every 20th scan carrying scans/evaluated/down. **Agent v0.109.0 fixes the same shape in the guest-power watchdog shipped hours earlier in v0.107.0** — it logged only at startup and when it acted, so its health could be read only from absence | — | | **R-89** | Retention as a per-customer **commercial** policy on the hub | READY (increment 2) | — | Policy object + reconciler → ep0 prune job; keep box tokens write-only | CC | | **R-92** | Hub PBS-DR gauge is 0.1 GB-granular — small deltas unverifiable | READY (XS) | — | Widen precision when retention becomes customer-visible | CC |