Host power-off: refuse while another guest or VM runs; hold the heavy gate; fail closed (R-998)
gates / gates (push) Successful in 33s
gates / gates (push) Successful in 33s
Security review of 3403c87: (1) any guest's controller could switch off a shared host — demo-hp
runs a scratch guest and the Tester 1 VM. Now refused (other_guests_running) when /proc shows any
other lxc-start -n N or kvm -id N; an unreadable /proc refuses. (2) the checks ran once, so a
backup could start before the power-off: the heavy-operation gate is now taken on acceptance and
kept (released only if the power-off command fails). (3) an unwired gate or task reader refused
nothing; now refuses. Each red-proofed.
This commit is contained in:
@@ -4,6 +4,8 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
@@ -20,8 +22,15 @@ import (
|
|||||||
// - refused in the first 10 minutes after boot (no power-off loop);
|
// - refused in the first 10 minutes after boot (no power-off loop);
|
||||||
// - at most ONE accepted request per boot (poweroff.State, persisted, keyed by the boot);
|
// - at most ONE accepted request per boot (poweroff.State, persisted, keyed by the boot);
|
||||||
// - refused while a backup, restore test, OS update step, controller update, drive format, NAS verify
|
// - refused while a backup, restore test, OS update step, controller update, drive format, NAS verify
|
||||||
// or escrow ceremony runs, or a vzdump of the guest is in flight — each named in the 409.
|
// or escrow ceremony runs, or a vzdump of the guest is in flight — each named in the 409;
|
||||||
|
// - refused while ANY OTHER guest or VM runs on the host (security review 2026-10-13): one guest's
|
||||||
|
// controller must not switch off another household's guest, a scratch guest or a test VM (demo-hp
|
||||||
|
// carries the Tester 1 VM). Read from /proc (`lxc-start -n N`, `kvm -id N`), fail closed;
|
||||||
|
// - the heavy-operation gate is TAKEN on acceptance and never given back (the box is going down), so no
|
||||||
|
// backup, restore test or OS step can start between the checks and the power-off (security review).
|
||||||
|
// Released only if the power-off command itself fails.
|
||||||
//
|
//
|
||||||
|
// Every check fails CLOSED: an unwired gate, an unreadable task list or /proc refuses.
|
||||||
// On acceptance the record is written FIRST, the hub is given a report carrying it (bounded wait), and
|
// On acceptance the record is written FIRST, the hub is given a report carrying it (bounded wait), and
|
||||||
// only then: guest shutdown (time-limited), host power-off. Pinned by hostpoweroff_test.go.
|
// only then: guest shutdown (time-limited), host power-off. Pinned by hostpoweroff_test.go.
|
||||||
|
|
||||||
@@ -37,6 +46,7 @@ type HostPowerOffConfig struct {
|
|||||||
Power HostPowerer
|
Power HostPowerer
|
||||||
ReportNow func(ctx context.Context) bool // push a report carrying the record; true = the hub accepted it
|
ReportNow func(ctx context.Context) bool // push a report carrying the record; true = the hub accepted it
|
||||||
Uptime func() (time.Duration, bool) // time since host boot; false = unknown (refuse)
|
Uptime func() (time.Duration, bool) // time since host boot; false = unknown (refuse)
|
||||||
|
Running func() ([]int, error) // vmids of every running guest and VM on the host; default /proc
|
||||||
MinUptime time.Duration // default 10 min
|
MinUptime time.Duration // default 10 min
|
||||||
GuestStop time.Duration // pct shutdown --timeout; default 120 s
|
GuestStop time.Duration // pct shutdown --timeout; default 120 s
|
||||||
ReportMax time.Duration // longest wait for the hub; default 60 s
|
ReportMax time.Duration // longest wait for the hub; default 60 s
|
||||||
@@ -54,6 +64,7 @@ const (
|
|||||||
PowerOffDrive = "drive_operation_running"
|
PowerOffDrive = "drive_operation_running"
|
||||||
PowerOffEscrow = "escrow_ceremony_running"
|
PowerOffEscrow = "escrow_ceremony_running"
|
||||||
PowerOffBusy = "host_busy"
|
PowerOffBusy = "host_busy"
|
||||||
|
PowerOffOtherGuests = "other_guests_running"
|
||||||
PowerOffUnknown = "state_unknown"
|
PowerOffUnknown = "state_unknown"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -71,6 +82,9 @@ func (s *Server) SetHostPowerOff(c HostPowerOffConfig) {
|
|||||||
if c.Now == nil {
|
if c.Now == nil {
|
||||||
c.Now = time.Now
|
c.Now = time.Now
|
||||||
}
|
}
|
||||||
|
if c.Running == nil {
|
||||||
|
c.Running = func() ([]int, error) { return runningGuestsFromProc("/proc") }
|
||||||
|
}
|
||||||
s.powerOff = &c
|
s.powerOff = &c
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -101,8 +115,15 @@ func (s *Server) handleHostPowerOff(w http.ResponseWriter, r *http.Request, vmid
|
|||||||
writeStatus(w, http.StatusConflict, false, map[string]any{"reason": code, "detail": detail}, "host power-off refused: "+code)
|
writeStatus(w, http.StatusConflict, false, map[string]any{"reason": code, "detail": detail}, "host power-off refused: "+code)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
// Take the heavy-operation gate for good: nothing heavy may start between these checks and the power-off.
|
||||||
|
release, busy, free := s.inFlight.TryAcquire("host-poweroff")
|
||||||
|
if !free {
|
||||||
|
writeStatus(w, http.StatusConflict, false, map[string]any{"reason": PowerOffBusy, "detail": busy}, "host power-off refused: "+PowerOffBusy)
|
||||||
|
return
|
||||||
|
}
|
||||||
rec, err := c.State.Mark(c.Now(), vmid)
|
rec, err := c.State.Mark(c.Now(), vmid)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
release()
|
||||||
code := PowerOffUnknown
|
code := PowerOffUnknown
|
||||||
if errors.Is(err, poweroff.ErrAlreadyThisBoot) {
|
if errors.Is(err, poweroff.ErrAlreadyThisBoot) {
|
||||||
code = PowerOffAlreadyAsked
|
code = PowerOffAlreadyAsked
|
||||||
@@ -117,7 +138,7 @@ func (s *Server) handleHostPowerOff(w http.ResponseWriter, r *http.Request, vmid
|
|||||||
if base == nil {
|
if base == nil {
|
||||||
base = context.Background()
|
base = context.Background()
|
||||||
}
|
}
|
||||||
go s.runHostPowerOff(base, vmid)
|
go s.runHostPowerOff(base, vmid, release)
|
||||||
writeStatus(w, http.StatusAccepted, true, map[string]any{
|
writeStatus(w, http.StatusAccepted, true, map[string]any{
|
||||||
"vmid": vmid, "powering_off": true, "requested_at": rec.RequestedAt.Format(time.RFC3339),
|
"vmid": vmid, "powering_off": true, "requested_at": rec.RequestedAt.Format(time.RFC3339),
|
||||||
}, "")
|
}, "")
|
||||||
@@ -126,7 +147,7 @@ func (s *Server) handleHostPowerOff(w http.ResponseWriter, r *http.Request, vmid
|
|||||||
// runHostPowerOff: hub first (bounded), then the guest (time-limited), then the host. A failed guest
|
// runHostPowerOff: hub first (bounded), then the guest (time-limited), then the host. A failed guest
|
||||||
// shutdown does not stop the power-off — the household asked for it, and Proxmox's own shutdown stops
|
// shutdown does not stop the power-off — the household asked for it, and Proxmox's own shutdown stops
|
||||||
// any guest still running (pve-guests) before the host goes down.
|
// any guest still running (pve-guests) before the host goes down.
|
||||||
func (s *Server) runHostPowerOff(base context.Context, vmid int) {
|
func (s *Server) runHostPowerOff(base context.Context, vmid int, release func()) {
|
||||||
c := s.powerOff
|
c := s.powerOff
|
||||||
if c.ReportNow != nil {
|
if c.ReportNow != nil {
|
||||||
ctx, cancel := context.WithTimeout(base, c.ReportMax)
|
ctx, cancel := context.WithTimeout(base, c.ReportMax)
|
||||||
@@ -148,7 +169,8 @@ func (s *Server) runHostPowerOff(base context.Context, vmid int) {
|
|||||||
ctx, cancel = context.WithTimeout(base, 60*time.Second)
|
ctx, cancel = context.WithTimeout(base, 60*time.Second)
|
||||||
defer cancel()
|
defer cancel()
|
||||||
if err := c.Power.PowerOffHost(ctx); err != nil {
|
if err := c.Power.PowerOffHost(ctx); err != nil {
|
||||||
s.logger.Error("host-poweroff: host power-off command failed", "err", err)
|
s.logger.Error("host-poweroff: host power-off command failed — heavy-operation gate released", "err", err)
|
||||||
|
release()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -168,6 +190,22 @@ func (s *Server) powerOffBlocker(ctx context.Context, vmid int) (string, string)
|
|||||||
if up < c.MinUptime {
|
if up < c.MinUptime {
|
||||||
return PowerOffTooSoon, "host up " + up.Round(time.Second).String() + ", minimum " + c.MinUptime.String()
|
return PowerOffTooSoon, "host up " + up.Round(time.Second).String() + ", minimum " + c.MinUptime.String()
|
||||||
}
|
}
|
||||||
|
if s.inFlight == nil || s.staleLock == nil {
|
||||||
|
return PowerOffUnknown, "the heavy-operation gate or the Proxmox task reader is not wired"
|
||||||
|
}
|
||||||
|
running, err := c.Running()
|
||||||
|
if err != nil {
|
||||||
|
return PowerOffUnknown, "cannot list the running guests: " + err.Error()
|
||||||
|
}
|
||||||
|
var others []string
|
||||||
|
for _, v := range running {
|
||||||
|
if v != vmid {
|
||||||
|
others = append(others, strconv.Itoa(v))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if len(others) > 0 {
|
||||||
|
return PowerOffOtherGuests, "other guests or VMs running: " + strings.Join(others, ", ")
|
||||||
|
}
|
||||||
if busy := s.inFlight.Busy(); busy != "" {
|
if busy := s.inFlight.Busy(); busy != "" {
|
||||||
switch {
|
switch {
|
||||||
case strings.HasPrefix(busy, "backup:"):
|
case strings.HasPrefix(busy, "backup:"):
|
||||||
@@ -213,14 +251,52 @@ func (s *Server) powerOffBlocker(ctx context.Context, vmid int) (string, string)
|
|||||||
if esc {
|
if esc {
|
||||||
return PowerOffEscrow, "escrow ceremony running"
|
return PowerOffEscrow, "escrow ceremony running"
|
||||||
}
|
}
|
||||||
if s.staleLock != nil {
|
vz, err := s.staleLock.BackupRunning(ctx, vmid)
|
||||||
running, err := s.staleLock.BackupRunning(ctx, vmid)
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return PowerOffUnknown, "cannot list running Proxmox tasks: " + err.Error()
|
return PowerOffUnknown, "cannot list running Proxmox tasks: " + err.Error()
|
||||||
}
|
}
|
||||||
if running {
|
if vz {
|
||||||
return PowerOffBackup, "vzdump running for guest " + strconv.Itoa(vmid)
|
return PowerOffBackup, "vzdump running for guest " + strconv.Itoa(vmid)
|
||||||
}
|
}
|
||||||
}
|
|
||||||
return "", ""
|
return "", ""
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// runningGuestsFromProc lists the vmids of running containers (`lxc-start … -n N`) and VMs (`kvm -id N`) from
|
||||||
|
// the process table — readable by the non-root agent, unlike /run/qemu-server (measured on demo-hp 2026-10-13).
|
||||||
|
// An unreadable /proc is an error (fail closed); an unreadable single process is skipped (it exited).
|
||||||
|
func runningGuestsFromProc(procDir string) ([]int, error) {
|
||||||
|
ents, err := os.ReadDir(procDir)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
seen := map[int]bool{}
|
||||||
|
var out []int
|
||||||
|
for _, e := range ents {
|
||||||
|
if _, err := strconv.Atoi(e.Name()); err != nil {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
raw, err := os.ReadFile(filepath.Join(procDir, e.Name(), "cmdline"))
|
||||||
|
if err != nil || len(raw) == 0 {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
args := strings.Split(strings.TrimRight(string(raw), "\x00"), "\x00")
|
||||||
|
flag := ""
|
||||||
|
switch filepath.Base(args[0]) {
|
||||||
|
case "lxc-start":
|
||||||
|
flag = "-n"
|
||||||
|
case "kvm", "qemu-system-x86_64":
|
||||||
|
flag = "-id"
|
||||||
|
default:
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
for i := 1; i+1 < len(args); i++ {
|
||||||
|
if args[i] == flag {
|
||||||
|
if v, err := strconv.Atoi(args[i+1]); err == nil && !seen[v] {
|
||||||
|
seen[v] = true
|
||||||
|
out = append(out, v)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return out, nil
|
||||||
|
}
|
||||||
|
|||||||
@@ -3,8 +3,12 @@ package localapi
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
|
"errors"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"sort"
|
||||||
|
"strings"
|
||||||
"sync"
|
"sync"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
@@ -62,6 +66,7 @@ func newPowerRig(t *testing.T, uptime time.Duration) *powerRig {
|
|||||||
t.Helper()
|
t.Helper()
|
||||||
srv := newTestServerS(t, &fakeGuests{}, &fakeBackups{}, &fakeStore{}, nil)
|
srv := newTestServerS(t, &fakeGuests{}, &fakeBackups{}, &fakeStore{}, nil)
|
||||||
srv.inFlight = &backup.InFlight{}
|
srv.inFlight = &backup.InFlight{}
|
||||||
|
srv.staleLock = fakeVzdump{}
|
||||||
p := &fakePower{}
|
p := &fakePower{}
|
||||||
st := poweroff.New(filepath.Join(t.TempDir(), "poweroff.json"), func() string { return "1700000000" })
|
st := poweroff.New(filepath.Join(t.TempDir(), "poweroff.json"), func() string { return "1700000000" })
|
||||||
var reports []string
|
var reports []string
|
||||||
@@ -80,6 +85,7 @@ func newPowerRig(t *testing.T, uptime time.Duration) *powerRig {
|
|||||||
return true
|
return true
|
||||||
},
|
},
|
||||||
Uptime: func() (time.Duration, bool) { return uptime, true },
|
Uptime: func() (time.Duration, bool) { return uptime, true },
|
||||||
|
Running: func() ([]int, error) { return []int{8200}, nil },
|
||||||
Now: func() time.Time { return testNow },
|
Now: func() time.Time { return testNow },
|
||||||
})
|
})
|
||||||
return &powerRig{srv: srv, h: srv.Handler(), power: p, state: st, report: &reports}
|
return &powerRig{srv: srv, h: srv.Handler(), power: p, state: st, report: &reports}
|
||||||
@@ -118,6 +124,10 @@ func TestHostPowerOff_AcceptedOrder(t *testing.T) {
|
|||||||
if len(steps) != 2 || steps[0] != want[0] || steps[1] != want[1] {
|
if len(steps) != 2 || steps[0] != want[0] || steps[1] != want[1] {
|
||||||
t.Fatalf("steps = %v, want %v", steps, want)
|
t.Fatalf("steps = %v, want %v", steps, want)
|
||||||
}
|
}
|
||||||
|
// The heavy-operation gate stays taken: no backup can start between the checks and the power-off.
|
||||||
|
if _, busy, ok := r.srv.inFlight.TryAcquire("backup:primary"); ok || busy != "host-poweroff" {
|
||||||
|
t.Fatalf("after acceptance a backup could take the gate (ok=%v busy=%q)", ok, busy)
|
||||||
|
}
|
||||||
if len(*r.report) != 1 || (*r.report)[0] != "report" {
|
if len(*r.report) != 1 || (*r.report)[0] != "report" {
|
||||||
t.Fatalf("hub report = %v, want one report carrying the record", *r.report)
|
t.Fatalf("hub report = %v, want one report carrying the record", *r.report)
|
||||||
}
|
}
|
||||||
@@ -215,3 +225,58 @@ func TestHostPowerOff_ScopeAndConfig(t *testing.T) {
|
|||||||
t.Fatal("a refused request changed state")
|
t.Fatal("a refused request changed state")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Another guest or VM on the host refuses: one controller must not switch off someone else's guest (security
|
||||||
|
// review 2026-10-13; demo-hp runs a scratch guest and the Tester 1 VM). An unreadable process list refuses too.
|
||||||
|
func TestHostPowerOff_RefusedWhileOtherGuestsRun(t *testing.T) {
|
||||||
|
r := newPowerRig(t, time.Hour)
|
||||||
|
r.srv.powerOff.Running = func() ([]int, error) { return []int{8200, 9202, 341}, nil }
|
||||||
|
w := do(t, r.h, "POST", "/host/poweroff", "A", "")
|
||||||
|
if w.Code != http.StatusConflict || reasonOf(t, w.Body.Bytes()) != PowerOffOtherGuests {
|
||||||
|
t.Fatalf("got %d %s, want 409 %s", w.Code, w.Body.String(), PowerOffOtherGuests)
|
||||||
|
}
|
||||||
|
r.srv.powerOff.Running = func() ([]int, error) { return nil, errors.New("no /proc") }
|
||||||
|
w = do(t, r.h, "POST", "/host/poweroff", "A", "")
|
||||||
|
if w.Code != http.StatusConflict || reasonOf(t, w.Body.Bytes()) != PowerOffUnknown {
|
||||||
|
t.Fatalf("unreadable: got %d %s, want 409 %s", w.Code, w.Body.String(), PowerOffUnknown)
|
||||||
|
}
|
||||||
|
if len(r.power.got()) != 0 || r.state.Current() != nil {
|
||||||
|
t.Fatal("a refused request changed state")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Fail closed: an unwired vzdump reader refuses instead of skipping the check.
|
||||||
|
func TestHostPowerOff_FailsClosedWithoutTaskReader(t *testing.T) {
|
||||||
|
r := newPowerRig(t, time.Hour)
|
||||||
|
r.srv.staleLock = nil
|
||||||
|
w := do(t, r.h, "POST", "/host/poweroff", "A", "")
|
||||||
|
if w.Code != http.StatusConflict || reasonOf(t, w.Body.Bytes()) != PowerOffUnknown {
|
||||||
|
t.Fatalf("got %d %s, want 409 %s", w.Code, w.Body.String(), PowerOffUnknown)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The /proc reader finds containers and VMs by their real command lines (as measured on demo-hp).
|
||||||
|
func TestRunningGuestsFromProc(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
write := func(pid, cmd string) {
|
||||||
|
_ = os.MkdirAll(filepath.Join(dir, pid), 0o755)
|
||||||
|
_ = os.WriteFile(filepath.Join(dir, pid, "cmdline"), []byte(strings.ReplaceAll(cmd, " ", "\x00")+"\x00"), 0o644)
|
||||||
|
}
|
||||||
|
write("1927", "/usr/bin/lxc-start -F -n 9201")
|
||||||
|
write("1900", "/usr/bin/lxc-start -F -n 9202")
|
||||||
|
write("1823", "/usr/bin/kvm -id 341 -name night1004-tester1 -no-shutdown")
|
||||||
|
write("974", "/usr/libexec/lxc/lxc-monitord --daemon")
|
||||||
|
write("50", "/usr/bin/bash -c echo -n 5")
|
||||||
|
_ = os.MkdirAll(filepath.Join(dir, "self"), 0o755)
|
||||||
|
got, err := runningGuestsFromProc(dir)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
sort.Ints(got)
|
||||||
|
if len(got) != 3 || got[0] != 341 || got[1] != 9201 || got[2] != 9202 {
|
||||||
|
t.Fatalf("got %v, want [341 9201 9202]", got)
|
||||||
|
}
|
||||||
|
if _, err := runningGuestsFromProc(filepath.Join(dir, "missing")); err == nil {
|
||||||
|
t.Fatal("an unreadable /proc must be an error")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user