R-861: narrow the agent's root grants — exact sudo patterns, felhom-priv-apply content checker, fixed hook/parent files in the bundle, signed self-update verified as root, escrow root reads pinned

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS
2026-10-05 12:00:43 +02:00
parent 61345790ed
commit 6ab1e7c56c
39 changed files with 1657 additions and 400 deletions
@@ -121,32 +121,33 @@ func (b *GuestBinder) EnsureSharedParent(ctx context.Context) error {
 	// only the script (the unit was unchanged), so the earlier unit-only gate never redeployed it —
 	// leaving hosts running the pre-fix script (no make-private), whose self-bind stays in root's shared
 	// peer group and DOUBLES every drive bind. Comparing both files closes that deploy gap.
-	if sharedParentInstallStale(sharedParentUnitPath, sharedParentScriptPath) {
-		if ierr := b.installSharedParentUnit(ctx); ierr != nil {
-			b.logger.Warn("shared-parent: boot-persistence (re)install failed (live setup OK; survives until host reboot)", "err", ierr)
-		}
+	if err := b.ensureSharedParentBoot(ctx, sharedParentUnitPath, sharedParentScriptPath, sharedParentWantsLink); err != nil {
+		b.logger.Warn("shared-parent: boot persistence not enabled (live setup OK; survives until host reboot)", "err", err)
 	}
 	return nil
 }

-// stageTemp writes content to a fresh random-named temp file (os.CreateTemp pattern — `*` is replaced
-// by a random string) and returns its path. Caller removes it after the privileged `install`.
-func stageTemp(pattern, content string) (string, error) {
-	f, err := os.CreateTemp("", pattern)
-	if err != nil {
-		return "", err
+// sharedParentWantsLink exists once `systemctl enable felhom-shared-parent.service` ran (WantedBy=pve-guests.service).
+const sharedParentWantsLink = "/etc/systemd/system/pve-guests.service.wants/felhom-shared-parent.service"
+
+// ensureSharedParentBoot (R-861, agent v0.146.0) — the boot script and its unit are ROOT-OWNED FIXED files that arrive
+// with the signed config bundle (configs/felhom-shared-parent.{sh,service}, byte-identical to the constants above,
+// pinned by TestSharedParentFilesEqualTheBundle). The agent NEVER installs them any more: until v0.146.0 it installed
+// them from /tmp, and a script a root unit runs at boot was a root shell for a compromised agent. Here it only checks
+// them, and enables the unit (a fixed sudoers line) when the bundle has put it in place but nothing has enabled it — a
+// fresh install. Missing or different files: one warning, nothing run (the next bundle brings them).
+func (b *GuestBinder) ensureSharedParentBoot(ctx context.Context, unitPath, scriptPath, wantsLink string) error {
+	if sharedParentInstallStale(unitPath, scriptPath) {
+		return fmt.Errorf("%s or %s is missing or differs from this agent's — it arrives with the signed config bundle (agent_config_update); the agent does not install it (R-861)", scriptPath, unitPath)
 	}
-	name := f.Name()
-	if _, err := f.WriteString(content); err != nil {
-		f.Close()
-		os.Remove(name)
-		return "", err
+	if _, err := os.Lstat(wantsLink); err == nil {
+		return nil
 	}
-	if err := f.Close(); err != nil {
-		os.Remove(name)
-		return "", err
+	if err := b.run(ctx, "systemctl", "enable", "felhom-shared-parent.service"); err != nil {
+		return fmt.Errorf("enable unit: %w", err)
 	}
-	return name, nil
+	b.logger.Info("shared-parent: boot-persistence unit enabled (files from the config bundle)")
+	return nil
 }

 // sharedParentInstallStale reports whether the on-disk boot script OR unit is missing or differs from
@@ -162,37 +163,6 @@ func sharedParentInstallStale(unitPath, scriptPath string) bool {
 	return false
 }

-// installSharedParentUnit writes the script + unit (from agent-written temps) and enables the unit so the
-// shared parent is re-established on every host boot before pve-guests. Idempotent. The temps are
-// RANDOM-named os.CreateTemp files (audit B1): a fixed, predictable /tmp name could be pre-created by
-// another local user and rewritten between our write and root's install (TOCTOU into a root-executed
-// boot script). The final modes come from `install -m`, so the 0600 temps are fine.
-func (b *GuestBinder) installSharedParentUnit(ctx context.Context) error {
-	tmpScript, err := stageTemp("felhom-shared-parent-*.sh", sharedParentScript)
-	if err != nil {
-		return fmt.Errorf("write temp script: %w", err)
-	}
-	defer os.Remove(tmpScript)
-	if err := b.run(ctx, "install", "-m", "0755", "--", tmpScript, sharedParentScriptPath); err != nil {
-		return fmt.Errorf("install script: %w", err)
-	}
-	tmpUnit, err := stageTemp("felhom-shared-parent-*.service", sharedParentUnit)
-	if err != nil {
-		return fmt.Errorf("write temp unit: %w", err)
-	}
-	defer os.Remove(tmpUnit)
-	if err := b.run(ctx, "install", "-m", "0644", "--", tmpUnit, sharedParentUnitPath); err != nil {
-		return fmt.Errorf("install unit: %w", err)
-	}
-	if err := b.run(ctx, "systemctl", "daemon-reload"); err != nil {
-		return fmt.Errorf("daemon-reload: %w", err)
-	}
-	if err := b.run(ctx, "systemctl", "enable", "felhom-shared-parent.service"); err != nil {
-		return fmt.Errorf("enable unit: %w", err)
-	}
-	return nil
-}
-
 // AttachDrive binds a drive's felhom-data namespace under the stable parent so it appears live in the
 // guest at the returned stable path (via propagation — no pct, no reboot). `where` is the drive's RAW
 // host PVE mount (/mnt/<name>); only `<where>/felhom-data` crosses into the guest (confinement). The
@@ -4,94 +4,82 @@ import (
 	"context"
 	"io"
 	"os"
-	"regexp"
+	"path/filepath"
 	"testing"
 )

-// stagingRecorderRunner is a fake proxmox.Runner recording every call vector and snapshotting each
-// install SOURCE file's content at call time (the deferred os.Remove erases it afterwards).
-type stagingRecorderRunner struct {
-	calls      [][]string
-	srcContent map[string]string // install dest → staged source content
-}
+// R-861 (agent v0.146.0): the shared-parent boot script and unit arrive with the signed config bundle; the agent never
+// installs them. It checks them and enables the unit when nothing has.
+//
+// RED-PROOF (audits/hub-safety-2026-10-05/partF/red-proof.txt): put the old installSharedParentUnit call back (an
+// `install` of a /tmp file) → TestSharedParentBoot_NeverInstalls fails.

-func (r *stagingRecorderRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) {
+type callRecorder struct{ calls [][]string }
+
+func (r *callRecorder) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) {
 	r.calls = append(r.calls, append([]string{name}, args...))
-	if name == "install" && len(args) >= 2 {
-		src, dest := args[len(args)-2], args[len(args)-1]
-		if r.srcContent == nil {
-			r.srcContent = map[string]string{}
-		}
-		b, _ := os.ReadFile(src)
-		r.srcContent[dest] = string(b)
-	}
 	return nil, nil, nil
 }

-func (r *stagingRecorderRunner) RunStdin(ctx context.Context, _ io.Reader, name string, args ...string) ([]byte, []byte, error) {
+func (r *callRecorder) RunStdin(ctx context.Context, _ io.Reader, name string, args ...string) ([]byte, []byte, error) {
 	return r.Run(ctx, name, args...)
 }

-// installSources returns the install-call source paths keyed by destination.
-func (r *stagingRecorderRunner) installSources() map[string][]string {
-	out := map[string][]string{}
-	for _, c := range r.calls {
-		if c[0] == "install" && len(c) >= 3 {
-			src, dest := c[len(c)-2], c[len(c)-1]
-			out[dest] = append(out[dest], src)
-		}
+func bootFiles(t *testing.T, script, unit string) (string, string, string) {
+	t.Helper()
+	d := t.TempDir()
+	sp, up := filepath.Join(d, "felhom-shared-parent.sh"), filepath.Join(d, "felhom-shared-parent.service")
+	if script != "" {
+		_ = os.WriteFile(sp, []byte(script), 0o755)
 	}
-	return out
+	if unit != "" {
+		_ = os.WriteFile(up, []byte(unit), 0o644)
+	}
+	return sp, up, filepath.Join(d, "wants-link")
 }

-// TestInstallSharedParent_RandomTempName is the audit-B1 negative test for the shared-parent boot
-// persistence install: both staged install SOURCES (script + unit) must be RANDOM os.CreateTemp names
-// (felhom-shared-parent-<random>.sh / .service), never the fixed, pre-creatable /tmp names (a local
-// TOCTOU into a root-executed boot script), and two consecutive installs must use DIFFERENT paths.
-func TestInstallSharedParent_RandomTempName(t *testing.T) {
-	r := &stagingRecorderRunner{}
+func TestSharedParentBoot_NeverInstalls(t *testing.T) {
+	for _, c := range []struct{ name, script, unit string }{
+		{"both missing", "", ""},
+		{"script differs", "#!/bin/sh\necho old\n", sharedParentUnit},
+		{"unit missing", sharedParentScript, ""},
+	} {
+		r := &callRecorder{}
+		b := NewGuestBinder(r, nil)
+		sp, up, link := bootFiles(t, c.script, c.unit)
+		if err := b.ensureSharedParentBoot(context.Background(), up, sp, link); err == nil {
+			t.Errorf("%s: no error — the operator would not learn the bundle is missing", c.name)
+		}
+		if len(r.calls) != 0 {
+			t.Errorf("%s: the agent ran %v — it must install nothing (R-861)", c.name, r.calls)
+		}
+	}
+}
+
+func TestSharedParentBoot_EnablesOnceTheBundleBroughtTheFiles(t *testing.T) {
+	r := &callRecorder{}
 	b := NewGuestBinder(r, nil)
-	if err := b.installSharedParentUnit(context.Background()); err != nil {
-		t.Fatalf("installSharedParentUnit #1: %v", err)
+	sp, up, link := bootFiles(t, sharedParentScript, sharedParentUnit)
+	if err := b.ensureSharedParentBoot(context.Background(), up, sp, link); err != nil {
+		t.Fatal(err)
 	}
-	if err := b.installSharedParentUnit(context.Background()); err != nil {
-		t.Fatalf("installSharedParentUnit #2: %v", err)
+	if len(r.calls) != 1 || len(r.calls[0]) != 3 || r.calls[0][0] != "systemctl" || r.calls[0][1] != "enable" ||
+		r.calls[0][2] != "felhom-shared-parent.service" {
+		t.Fatalf("want exactly `systemctl enable felhom-shared-parent.service`, got %v", r.calls)
 	}
+	_ = os.Symlink(up, link)
+	r.calls = nil
+	if err := b.ensureSharedParentBoot(context.Background(), up, sp, link); err != nil || len(r.calls) != 0 {
+		t.Fatalf("already enabled: want no calls, got %v (%v)", r.calls, err)
+	}
+}

-	srcs := r.installSources()
-	cases := []struct {
-		dest    string
-		random  *regexp.Regexp
-		fixed   *regexp.Regexp
-		content string
-	}{
-		{sharedParentScriptPath, regexp.MustCompile(`felhom-shared-parent-[^/\\]+\.sh$`),
-			regexp.MustCompile(`felhom-shared-parent\.sh$`), sharedParentScript},
-		{sharedParentUnitPath, regexp.MustCompile(`felhom-shared-parent-[^/\\]+\.service$`),
-			regexp.MustCompile(`felhom-shared-parent\.service$`), sharedParentUnit},
-	}
-	for _, tc := range cases {
-		got := srcs[tc.dest]
-		if len(got) != 2 {
-			t.Fatalf("dest %s: expected 2 install calls, got %d (%v)", tc.dest, len(got), got)
-		}
-		for i, src := range got {
-			if !tc.random.MatchString(src) {
-				t.Errorf("dest %s call %d: source %q does not match the random temp pattern", tc.dest, i, src)
-			}
-			if tc.fixed.MatchString(src) {
-				t.Errorf("dest %s call %d: source %q is the FIXED predictable temp name (B1 TOCTOU)", tc.dest, i, src)
-			}
-			if _, err := os.Stat(src); err == nil {
-				t.Errorf("dest %s: staged temp %q left behind (defer os.Remove missing)", tc.dest, src)
-			}
-		}
-		if got[0] == got[1] {
-			t.Errorf("dest %s: two consecutive installs staged through the SAME source %q — must be random per call", tc.dest, got[0])
-		}
-		// Non-hollow: the staged file carried the real content at install time.
-		if r.srcContent[tc.dest] != tc.content {
-			t.Errorf("dest %s: staged content mismatch (got %d bytes, want %d)", tc.dest, len(r.srcContent[tc.dest]), len(tc.content))
+// The bundle's copies are byte-identical to the constants the agent compares against.
+func TestSharedParentFilesEqualTheBundle(t *testing.T) {
+	for file, want := range map[string]string{"felhom-shared-parent.sh": sharedParentScript, "felhom-shared-parent.service": sharedParentUnit} {
+		got, err := os.ReadFile(filepath.Join("..", "..", "configs", file))
+		if err != nil || string(got) != want {
+			t.Errorf("configs/%s differs from the agent's constant (err %v)", file, err)
 		}
 	}
 }