F-LEAK third attempt: band-scoped fenced destroy (v0.110.0)
The per-VM ACL is consumed by the destroy it authorises (PVE remove_vm_access, LXC.pm:906), so it works once per slot. Fourth root-fenced exception, band-enforced in sudoers literally + in code + at the caller. API destroy still tried first.
This commit is contained in:
@@ -136,6 +136,39 @@ func (p *Privileged) CreateGoldenLXC(ctx context.Context, spec GoldenLXCSpec) er
|
||||
return p.run(ctx, "pct", args...)
|
||||
}
|
||||
|
||||
// DestroyScratchLXC destroys a restore-test scratch guest through the fenced root path, refusing any
|
||||
// vmid outside the caller-supplied scratch band.
|
||||
//
|
||||
// WHY THIS CANNOT BE THE API — and this is the fourth fenced exception, so the reasoning is recorded
|
||||
// in full. A restore-test whose restore FAILS leaves a scratch guest the API token cannot destroy:
|
||||
// `FelhomAgentGuest` is granted at /pool/felhom, and a guest joins that pool only when its restore
|
||||
// COMPLETES. A failed restore therefore leaves a guest that exists, is in no pool, and is out of
|
||||
// reach (403 VM.Allocate) while holding its disks.
|
||||
//
|
||||
// TWO API-SIDE FIXES WERE BUILT AND BOTH REFUTED BY LIVE TEST on 2026-07-28:
|
||||
// - Adopt the stranded guest into the pool, then retry. `PUT /pools/{pool}` ALSO requires
|
||||
// VM.Allocate on the VM being added, so pool membership cannot bootstrap its own authority.
|
||||
// - Grant the role per-path at /vms/990000..990009. Durable for exactly ONE use per slot: PVE's own
|
||||
// destroy calls AccessControl::remove_vm_access (LXC.pm:906), deleting every ACL at /vms/<vmid>
|
||||
// (AccessControl.pm:1898). The grant is consumed by the operation it authorises.
|
||||
//
|
||||
// The band ACLs are still provisioned (host-install v1.21.0) and the API path is still tried FIRST —
|
||||
// this is the fallback that makes teardown deterministic rather than once-per-slot.
|
||||
//
|
||||
// THE FENCE. The band is enforced in THREE places, deliberately: sudoers matches the vmid literally
|
||||
// (`pct destroy 99000[0-9] --purge` — even a compromised agent asking for 9201 is refused by sudo
|
||||
// itself), this method re-checks it before exec, and the caller checks its own journal provenance.
|
||||
// Unlike an ACL, none of these is consumed by use.
|
||||
func (p *Privileged) DestroyScratchLXC(ctx context.Context, vmid, bandMin, bandMax int) error {
|
||||
if bandMin <= 0 || bandMax < bandMin {
|
||||
return fmt.Errorf("proxmox: DestroyScratchLXC needs a configured scratch band, got [%d,%d]", bandMin, bandMax)
|
||||
}
|
||||
if vmid < bandMin || vmid > bandMax {
|
||||
return fmt.Errorf("proxmox: refusing to destroy vmid %d — outside the scratch band [%d,%d]", vmid, bandMin, bandMax)
|
||||
}
|
||||
return p.run(ctx, "pct", "destroy", strconv.Itoa(vmid), "--purge")
|
||||
}
|
||||
|
||||
// MountUSBByUUID mounts a filesystem by UUID at target (creating the mountpoint).
|
||||
//
|
||||
// WHY THIS CANNOT BE THE API: a physical host mount is not a Proxmox API op; it is
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
package proxmox
|
||||
|
||||
import (
|
||||
"context"
|
||||
"io"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// F-LEAK (Campaign 8): the fourth root-fenced exception. Its whole justification is that the band is
|
||||
// enforced rather than assumed, so these tests are about the REFUSALS, not the happy path.
|
||||
//
|
||||
// The band is checked in three independent places on purpose: sudoers matches the vmid literally
|
||||
// (`pct destroy 99000[0-9] --purge`), this method re-checks before exec, and the caller checks journal
|
||||
// provenance. These tests pin the middle one; the sudoers glob is proven live.
|
||||
|
||||
type recordingRunner struct {
|
||||
calls [][]string
|
||||
err error
|
||||
}
|
||||
|
||||
func (r *recordingRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) {
|
||||
r.calls = append(r.calls, append([]string{name}, args...))
|
||||
return nil, nil, r.err
|
||||
}
|
||||
|
||||
func (r *recordingRunner) RunStdin(_ context.Context, _ io.Reader, name string, args ...string) ([]byte, []byte, error) {
|
||||
r.calls = append(r.calls, append([]string{name}, args...))
|
||||
return nil, nil, r.err
|
||||
}
|
||||
|
||||
// A vmid inside the band is destroyed, with --purge so no config/ACL/firewall residue survives.
|
||||
//
|
||||
// RED-PROOF: drop "--purge" from the args → this fails with "destroy is not --purge", and the live
|
||||
// sudoers rule (which matches the FULL vector including --purge) would refuse the call outright.
|
||||
func TestDestroyScratchLXC_InBandDestroysWithPurge(t *testing.T) {
|
||||
r := &recordingRunner{}
|
||||
if err := NewPrivileged(r, "").DestroyScratchLXC(context.Background(), 990003, 990000, 990009); err != nil {
|
||||
t.Fatalf("in-band destroy failed: %v", err)
|
||||
}
|
||||
if len(r.calls) != 1 {
|
||||
t.Fatalf("want exactly 1 exec, got %d: %v", len(r.calls), r.calls)
|
||||
}
|
||||
got := strings.Join(r.calls[0], " ")
|
||||
if got != "pct destroy 990003 --purge" {
|
||||
t.Errorf("exec vector = %q, want %q (it must match the sudoers rule byte for byte)",
|
||||
got, "pct destroy 990003 --purge")
|
||||
}
|
||||
}
|
||||
|
||||
// THE ONE THAT MATTERS. A vmid outside the band must be refused WITHOUT EXECUTING ANYTHING — a real
|
||||
// customer guest, the golden image, a co-tenant's VM.
|
||||
//
|
||||
// RED-PROOF: remove the `vmid < bandMin || vmid > bandMax` check → this fails with
|
||||
// "REFUSAL FAILED: executed [pct destroy 9201 --purge] for out-of-band vmid 9201".
|
||||
func TestDestroyScratchLXC_RefusesOutOfBandWithoutExecuting(t *testing.T) {
|
||||
for _, vmid := range []int{
|
||||
1, // arbitrary
|
||||
9201, // the LIVE customer guest on both demo boxes
|
||||
9100, // golden image
|
||||
9999, // reserved
|
||||
989999, // one below the band
|
||||
990010, // one ABOVE the band — the off-by-one
|
||||
} {
|
||||
r := &recordingRunner{}
|
||||
err := NewPrivileged(r, "").DestroyScratchLXC(context.Background(), vmid, 990000, 990009)
|
||||
if err == nil {
|
||||
t.Errorf("vmid %d was NOT refused — the fence is open", vmid)
|
||||
}
|
||||
if len(r.calls) != 0 {
|
||||
t.Errorf("REFUSAL FAILED: executed %v for out-of-band vmid %d", r.calls, vmid)
|
||||
}
|
||||
if err != nil && !strings.Contains(err.Error(), "outside the scratch band") {
|
||||
t.Errorf("vmid %d refused with an unhelpful error: %v", vmid, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// An unconfigured or inverted band must refuse everything rather than defaulting to something. A zero
|
||||
// band is what a mis-wired caller looks like, and "destroy vmid 0" must never become reachable.
|
||||
//
|
||||
// RED-PROOF: drop the `bandMin <= 0 || bandMax < bandMin` check → the [0,0] case admits vmid 0 and
|
||||
// this fails with "an unconfigured band admitted vmid 0".
|
||||
func TestDestroyScratchLXC_RefusesUnconfiguredBand(t *testing.T) {
|
||||
cases := []struct{ vmid, min, max int }{
|
||||
{0, 0, 0}, // nothing configured at all
|
||||
{990000, 0, 0}, // band absent, real scratch vmid
|
||||
{990000, 0, 990009}, // min unset
|
||||
{990005, 990009, 990000}, // inverted
|
||||
{990000, -1, 990009}, // negative
|
||||
}
|
||||
for _, c := range cases {
|
||||
r := &recordingRunner{}
|
||||
if err := NewPrivileged(r, "").DestroyScratchLXC(context.Background(), c.vmid, c.min, c.max); err == nil {
|
||||
t.Errorf("band [%d,%d] admitted vmid %d — an unconfigured band must refuse", c.min, c.max, c.vmid)
|
||||
}
|
||||
if len(r.calls) != 0 {
|
||||
t.Errorf("an unconfigured band admitted vmid %d and EXECUTED %v", c.vmid, r.calls)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user