carlos / platform Public

fix-moves-floor-gate

Squashed from fix-moves-floor-gate into main.
claude agent, operated by paul@keymail.dev 00fdf5411455b7bb5ebd66284d213e2172b9d53c parent 93ef4b5
12 files changed, +721 −23
  • approved_moves_copy_test.go +4 −0
  • docs/superpowers/plans/2026-10-06-moves-phase1-copy.md +11 −0
  • internal/host/host_test.go +97 −17
  • internal/host/movesfloor.go +3 −1
  • internal/host/movesfloorgate.go +76 −0
  • internal/host/movesfloorgate_linux.go +109 −0
  • internal/host/movesfloorgate_linux_test.go +387 −0
  • internal/host/movesfloorgate_other.go +10 −0
  • internal/host/units/carlos-system-recover.service +9 −2
  • internal/host/units/carlos-system-update.service +9 −2
  • system.go +4 −1
  • system_test.go +2 −0

CI passed — run details

diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go
index 14d7bac4b..ccd011d44 100644
--- a/approved_moves_copy_test.go
+++ b/approved_moves_copy_test.go
@@ -74,6 +74,7 @@ var movesCopyIDs = []string{
"MV-N300", "MV-N301", "MV-N302", "MV-N303", "MV-N304", "MV-N305", "MV-N306",
"MV-N307", "MV-N308", "MV-N309", "MV-N310", "MV-N311", "MV-N312",
"MV-N313", "MV-N314",
+ "MV-N315", "MV-N316", "MV-N317", "MV-N318",
}
// copyRow is one row of the record. The File cell is the file the sentence is
@@ -572,6 +573,9 @@ var movesScanFiles = []string{
"internal/host/movesfloor.go",
"internal/host/movesfloor_linux.go",
"internal/host/movesfloor_other.go",
+ "internal/host/movesfloorgate.go",
+ "internal/host/movesfloorgate_linux.go",
+ "internal/host/movesfloorgate_other.go",
"internal/host/movesabort.go",
"internal/host/movesarchive.go",
"internal/host/movescut.go",
diff --git a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md
index cbd4e559e..94f979f72 100644
--- a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md
+++ b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md
@@ -271,6 +271,17 @@ Columns: id, file, text, status.
| MV-N313 | internal/host/movesrollback.go | carlos move stuck: <host> | pending |
| MV-N314 | internal/host/movesrollback.go | The move <id> of <host> stopped for an operator on <box>, its <role>: <reason>. Run carlos move resolve --host <host> --retry once the cause is fixed. | pending |
+## Floor gate fix (#267 review)
+
+(root's floor gate for the system units, `carlos system floor-allows-prev`; its stderr reaches the unit's journal)
+
+| id | file | text | status |
+|---|---|---|---|
+| MV-N315 | internal/host/movesfloorgate.go | moves protocol floor: carlos.prev may not run: <error> | pending |
+| MV-N316 | internal/host/movesfloorgate.go | the verdict on carlos.prev does not name the floor on this box | pending |
+| MV-N317 | internal/host/movesfloorgate.go | <path> is not a regular file with one link, at most 256 bytes, owned by an expected account | pending |
+| MV-N318 | internal/host/movesfloorgate.go | floor-allows-prev takes two absolute paths: the floor file and the verdict file | pending |
+
## Plan 6
(completed by Plan 6 Task 7)
diff --git a/internal/host/host_test.go b/internal/host/host_test.go
index ba4d4534f..7560e7aba 100644
--- a/internal/host/host_test.go
+++ b/internal/host/host_test.go
@@ -13,6 +13,7 @@ import (
"os/exec"
"path/filepath"
"reflect"
+ "runtime"
"slices"
"strings"
"testing"
@@ -1320,11 +1321,14 @@ func TestSystemUnitsAreShapedLikeTheStageRelay(t *testing.T) {
u := read("carlos-system-update.service")
for _, want := range []string{"Type=oneshot", "StartLimitIntervalSec=0", "TimeoutStartSec=15min", "OnFailure=carlos-system-recover.service",
"EnvironmentFile=-/etc/carlos/host.env", "system apply", "carlos.prev", "MemoryMax=256M",
- "floor.min-release", "carlos.prev.allowed", "cmp -s"} {
+ "floor.min-release", "carlos.prev.allowed", floorGateCall} {
if !strings.Contains(u, want) {
t.Errorf("update unit lacks %q", want)
}
}
+ if strings.Contains(u, "cmp ") {
+ t.Errorf("update unit still compares the floor file with cmp, which follows a symlink and blocks on a FIFO")
+ }
for _, never := range []string{"ProtectSystem", "User="} {
if strings.Contains(u, never) {
t.Errorf("update unit must run as root with /opt writable; found %q", never)
@@ -1332,11 +1336,14 @@ func TestSystemUnitsAreShapedLikeTheStageRelay(t *testing.T) {
}
r := read("carlos-system-recover.service")
for _, want := range []string{"Type=oneshot", "Before=carlos-edge.service", "WantedBy=multi-user.target", "TimeoutStartSec=10min", "system apply --recover", "carlos.prev",
- "floor.min-release", "carlos.prev.allowed", "cmp -s"} {
+ "floor.min-release", "carlos.prev.allowed", floorGateCall} {
if !strings.Contains(r, want) {
t.Errorf("recover unit lacks %q", want)
}
}
+ if strings.Contains(r, "cmp ") {
+ t.Errorf("recover unit still compares the floor file with cmp, which follows a symlink and blocks on a FIFO")
+ }
}
// TestStatusPublishNeverReadsMetricsPrefixes is the privacy tripwire the
@@ -1767,12 +1774,48 @@ func TestPlanCarriesArmedList(t *testing.T) {
}
}
+// floorGateCall is how both system units ask whether carlos.prev may run
+// against an armed floor: carlos.prev's own floor gate (root's trusted
+// path, never a PATH lookup), with both files named in the unit.
+const floorGateCall = "/opt/carlos/carlos.prev system floor-allows-prev /data/.moves/floor.min-release /opt/carlos/carlos.prev.allowed"
+
+// floorGateHelperEnv makes this test binary act as the floor gate for the
+// unit stubs in TestSystemUnitsSelectPrevByTheFloor.
+const floorGateHelperEnv = "CARLOS_TEST_FLOOR_GATE_HELPER"
+
+// TestFloorGateHelperProcess is not a test: run with floorGateHelperEnv
+// set, it is `carlos system floor-allows-prev` with the test's own uid as
+// the expected owner of both files (a test cannot make root's or the
+// service account's files).
+func TestFloorGateHelperProcess(t *testing.T) {
+ if os.Getenv(floorGateHelperEnv) != "1" {
+ return
+ }
+ args := os.Args
+ for i, a := range args {
+ if a == "--" {
+ args = args[i+1:]
+ break
+ }
+ }
+ uid := []uint32{uint32(os.Getuid())}
+ os.Exit(floorGateMain(args, uid, uid, os.Stderr))
+}
+
// TestSystemUnitsSelectPrevByTheFloor runs each system unit's ExecStart
// shell (moves spec §11, round-6 finding 4) against a fake /opt/carlos and
// /data, with stub binaries that record which one ran. Without a floor file
// the selection is main's exactly; with one, carlos.prev runs only when its
-// verdict names the floor on disk, byte for byte.
+// verdict names the floor on disk, byte for byte, as carlos.prev's floor
+// gate reads them (the stubs hand the gate verb to the real Go code).
func TestSystemUnitsSelectPrevByTheFloor(t *testing.T) {
+ if runtime.GOOS != "linux" {
+ t.Skip("the floor gate refuses off Linux")
+ }
+ self, err := os.Executable()
+ if err != nil {
+ t.Fatal(err)
+ }
for _, unit := range []string{"carlos-system-recover.service", "carlos-system-update.service"} {
t.Run(unit, func(t *testing.T) {
b, err := unitsFS.ReadFile("units/" + unit)
@@ -1805,29 +1848,56 @@ func TestSystemUnitsSelectPrevByTheFloor(t *testing.T) {
name string
journal bool
floor, verdict string // "" means no file
+ floorKind string // "" a regular file; or symlink, dangling, directory, fifo
noPrev bool
want string
- floorKind string
+ gate string // which binary ran the floor gate, "" for none
+ prevNoVerb bool // carlos.prev predates the gate and rejects its verb
}{
- {"no journal", false, "", "", false, "carlos", ""},
- {"no floor", true, "", "", false, "carlos.prev", ""},
- {"no floor, no prev", true, "", "", true, "carlos", ""},
- {"floor and a verdict naming it", true, "v0.30.0\n", "v0.30.0\n", false, "carlos.prev", ""},
- {"a verdict for an older floor", true, "v0.30.0\n", "v0.29.0\n", false, "carlos", ""},
- {"floor and no verdict", true, "v0.30.0\n", "", false, "carlos", ""},
- {"no journal, armed and allowed", false, "v0.30.0\n", "v0.30.0\n", false, "carlos", ""},
- {"symlink floor with matching verdict", true, "v0.30.0\n", "v0.30.0\n", false, "carlos", "symlink"},
- {"dangling symlink floor", true, "", "", false, "carlos", "dangling"},
- {"directory floor", true, "", "", false, "carlos", "directory"},
+ {"no journal", false, "", "", "", false, "carlos", "", false},
+ {"no floor", true, "", "", "", false, "carlos.prev", "", false},
+ {"no floor, no prev", true, "", "", "", true, "carlos", "", false},
+ {"floor and a verdict naming it", true, "v0.30.0\n", "v0.30.0\n", "", false, "carlos.prev", "carlos.prev", false},
+ {"a verdict for an older floor", true, "v0.30.0\n", "v0.29.0\n", "", false, "carlos", "carlos.prev", false},
+ {"floor and no verdict", true, "v0.30.0\n", "", "", false, "carlos", "carlos.prev", false},
+ {"no journal, armed and allowed", false, "v0.30.0\n", "v0.30.0\n", "", false, "carlos", "", false},
+ {"armed, no prev", true, "v0.30.0\n", "v0.30.0\n", "", true, "carlos", "", false},
+ // The agent owns /data: anything but a regular file at the
+ // floor path is refused before the gate runs (#267), and the
+ // gate refuses one swapped in after that test (it never
+ // follows a symlink or opens a FIFO for a blocking read).
+ // [ -s ] is false for a dangling symlink and a FIFO, so
+ // without the shell's test both would read as "no floor".
+ {"symlink floor with matching verdict", true, "v0.30.0\n", "v0.30.0\n", "symlink", false, "carlos", "", false},
+ {"dangling symlink floor", true, "", "", "dangling", false, "carlos", "", false},
+ {"directory floor", true, "", "", "directory", false, "carlos", "", false},
+ {"fifo floor", true, "", "v0.30.0\n", "fifo", false, "carlos", "", false},
+ // A carlos.prev older than the gate answers its verb with
+ // an unknown-verb exit: a mismatch, so carlos runs.
+ {"a carlos.prev without the gate verb", true, "v0.30.0\n", "v0.30.0\n", "", false, "carlos", "carlos.prev", true},
} {
t.Run(tc.name, func(t *testing.T) {
root := t.TempDir()
opt, data := filepath.Join(root, "opt"), filepath.Join(root, "data")
- ran := filepath.Join(root, "ran")
+ ran, gate := filepath.Join(root, "ran"), filepath.Join(root, "gate")
os.MkdirAll(filepath.Join(opt, "system", "router"), 0o755)
os.MkdirAll(filepath.Join(data, ".moves"), 0o755)
+ // Each stub answers the gate verb by handing it to this
+ // test binary (TestFloorGateHelperProcess), noting which
+ // stub was asked; anything else records the stub as run.
stub := func(name string) {
- os.WriteFile(filepath.Join(opt, name), []byte("#!/bin/sh\necho "+name+" > "+ran+"\n"), 0o755)
+ answer := " shift 2\n" +
+ " exec " + self + " -test.run='^TestFloorGateHelperProcess$' -- \"$@\"\n"
+ if name == "carlos.prev" && tc.prevNoVerb {
+ answer = " echo 'system: unknown verb \"floor-allows-prev\"' >&2\n exit 2\n"
+ }
+ sh := "#!/bin/sh\n" +
+ "if [ \"$1\" = system ] && [ \"$2\" = floor-allows-prev ]; then\n" +
+ " echo " + name + " >> " + gate + "\n" +
+ answer +
+ "fi\n" +
+ "echo " + name + " > " + ran + "\n"
+ os.WriteFile(filepath.Join(opt, name), []byte(sh), 0o755)
}
stub("carlos")
if !tc.noPrev {
@@ -1852,6 +1922,10 @@ func TestSystemUnitsSelectPrevByTheFloor(t *testing.T) {
if err := os.Mkdir(floorPath, 0o755); err != nil {
t.Fatal(err)
}
+ case "fifo":
+ if out, err := exec.Command("mkfifo", floorPath).CombinedOutput(); err != nil {
+ t.Fatalf("mkfifo: %v: %s", err, out)
+ }
default:
if tc.floor != "" {
if err := os.WriteFile(floorPath, []byte(tc.floor), 0o644); err != nil {
@@ -1864,13 +1938,19 @@ func TestSystemUnitsSelectPrevByTheFloor(t *testing.T) {
}
script := strings.ReplaceAll(body, "/data/", data+"/")
script = strings.ReplaceAll(script, "/opt/carlos/", opt+"/")
- if out, err := exec.Command("/bin/sh", "-c", script).CombinedOutput(); err != nil {
+ cmd := exec.Command("/bin/sh", "-c", script)
+ cmd.Env = append(os.Environ(), floorGateHelperEnv+"=1")
+ if out, err := cmd.CombinedOutput(); err != nil {
t.Fatalf("%v: %s", err, out)
}
got, _ := os.ReadFile(ran)
if strings.TrimSpace(string(got)) != tc.want {
t.Fatalf("ran %q, want %s", strings.TrimSpace(string(got)), tc.want)
}
+ asked, _ := os.ReadFile(gate)
+ if strings.TrimSpace(string(asked)) != tc.gate {
+ t.Fatalf("the floor gate ran from %q, want %q", strings.TrimSpace(string(asked)), tc.gate)
+ }
})
}
})
diff --git a/internal/host/movesfloor.go b/internal/host/movesfloor.go
index 11b378d61..8ceec4921 100644
--- a/internal/host/movesfloor.go
+++ b/internal/host/movesfloor.go
@@ -48,7 +48,9 @@ import (
// The two system units choose carlos.prev for a recovery only when the
// floor line is absent, or equal byte for byte to the verdict. The version
// comparison happens once, here in Go (moves.ReleaseAtOrAbove, which fails
-// closed); the shell only compares two files.
+// closed); the units only compare two files, through carlos.prev's floor
+// gate (movesfloorgate.go), which never follows a symlink or waits on a
+// FIFO the agent planted.
//
// What this is not: a defence against a compromised agent. The agent owns
// /data and can remove the floor files there. The floor protects the
diff --git a/internal/host/movesfloorgate.go b/internal/host/movesfloorgate.go
new file mode 100644
index 000000000..8100a2848
--- /dev/null
+++ b/internal/host/movesfloorgate.go
@@ -0,0 +1,76 @@
+package host
+
+import (
+ "errors"
+ "fmt"
+ "io"
+ "os"
+ "os/user"
+ "path/filepath"
+ "strconv"
+)
+
+// The floor gate: `carlos system floor-allows-prev <floor file> <verdict
+// file>`, which the two system units (carlos-system-update.service and
+// carlos-system-recover.service) run as root, from carlos.prev, before
+// they let carlos.prev run a recovery against an armed floor (moves spec
+// §11; see movesfloor.go for the files). It replaces a `cmp -s` in the
+// units' shell, which reopened the agent's floor file after the shell had
+// tested it: between the two the agent, which owns /data, could put a
+// symlink there (root then compared a file of the agent's choosing, a
+// one-bit oracle) or a FIFO (root blocked until the unit timed out).
+//
+// The exit code is the answer: 0 when the verdict names the floor byte for
+// byte, 1 for everything else (a mismatch, a refused or unreadable file,
+// a misuse). The shell treats them all alike, so every failure here is
+// fail-closed: the current binary runs, and its own Go check decides.
+//
+// carlos.prev runs the gate, not carlos, because the units pick carlos.prev
+// on the premise that carlos is the candidate that did not work, and a
+// gate in carlos would fail with it. Running carlos.prev for the gate is no
+// new trust: root already runs `carlos.prev version` when a floor is armed
+// (rootJudgePrev). A carlos.prev too old to have the verb exits non-zero,
+// which the units read as a mismatch, so the current binary runs.
+
+// floorGateMaxBytes bounds both files. Each holds one release line,
+// "vMAJOR.MINOR.PATCH\n" (writeFloorFiles, judgePrevWith).
+const floorGateMaxBytes = 256
+
+var (
+ errFloorGateMismatch = errors.New("the verdict on carlos.prev does not name the floor on this box")
+ errFloorGateUnsafe = errors.New("is not a regular file with one link, at most 256 bytes, owned by an expected account")
+ errFloorGateUsage = errors.New("floor-allows-prev takes two absolute paths: the floor file and the verdict file")
+)
+
+// RunSystemFloorAllowsPrev is the verb's entry point. The floor file may be
+// owned by root (root's apply writes it) or by the service account (the
+// agent's refresh writes it); the verdict only by root.
+func RunSystemFloorAllowsPrev(args []string) {
+ os.Exit(floorGateMain(args, floorGateFloorOwners(), []uint32{0}, os.Stderr))
+}
+
+// floorGateMain checks args, runs the gate and says why on stderr when
+// carlos.prev may not run. It returns the exit code.
+func floorGateMain(args []string, floorOwners, verdictOwners []uint32, stderr io.Writer) int {
+ err := errFloorGateUsage
+ if len(args) == 2 && filepath.IsAbs(args[0]) && filepath.IsAbs(args[1]) {
+ err = floorAllowsPrev(args[0], args[1], floorOwners, verdictOwners)
+ }
+ if err != nil {
+ fmt.Fprintf(stderr, "moves protocol floor: carlos.prev may not run: %v\n", err)
+ return 1
+ }
+ return 0
+}
+
+// floorGateFloorOwners is root and, when the box has one, the service
+// account. A box where the account cannot be looked up accepts root alone.
+func floorGateFloorOwners() []uint32 {
+ owners := []uint32{0}
+ if u, err := user.Lookup(serviceUser); err == nil {
+ if uid, err := strconv.ParseUint(u.Uid, 10, 32); err == nil {
+ owners = append(owners, uint32(uid))
+ }
+ }
+ return owners
+}
diff --git a/internal/host/movesfloorgate_linux.go b/internal/host/movesfloorgate_linux.go
new file mode 100644
index 000000000..badf8c23e
--- /dev/null
+++ b/internal/host/movesfloorgate_linux.go
@@ -0,0 +1,109 @@
+//go:build linux
+
+package host
+
+import (
+ "bytes"
+ "fmt"
+ "io"
+ "os"
+ "path/filepath"
+ "slices"
+
+ "golang.org/x/sys/unix"
+
+ "github.com/carlosframework/platform/internal/safefile"
+)
+
+// floorAllowsPrev is the floor gate's judgement: nil only when the floor
+// file and the verdict file both open safely and hold the same bytes.
+// Anything else is an error, and every error means the same thing to the
+// units as a mismatch: carlos.prev does not run.
+//
+// The floor file lives under the agent's /data, so it is reached without
+// following a symlink anywhere on its path (safefile.OpenDir walks from /
+// with O_NOFOLLOW) and opened once, leaf O_NOFOLLOW and O_NONBLOCK: a
+// symlink fails the open, and a FIFO opens at once instead of parking root
+// until a writer appears, then fails the fstat. The verdict lives in root's
+// own /opt/carlos, whose directories only root can change, so only its
+// leaf is opened that way. Both are then fstat-checked on the descriptor
+// they will be read from (checkFloorGateStat), so nothing can be swapped
+// between the check and the read.
+func floorAllowsPrev(floorPath, verdictPath string, floorOwners, verdictOwners []uint32) error {
+ floor, err := readFloorGateFile(floorPath, floorOwners, true)
+ if err != nil {
+ return err
+ }
+ verdict, err := readFloorGateFile(verdictPath, verdictOwners, false)
+ if err != nil {
+ return err
+ }
+ if !bytes.Equal(floor, verdict) {
+ return errFloorGateMismatch
+ }
+ return nil
+}
+
+// readFloorGateFile opens path once and reads it from that descriptor,
+// refusing anything checkFloorGateStat does. With pinDirs every directory
+// on the path is opened without following a symlink too.
+func readFloorGateFile(path string, owners []uint32, pinDirs bool) ([]byte, error) {
+ const flags = unix.O_RDONLY | unix.O_NOFOLLOW | unix.O_NONBLOCK | unix.O_NOCTTY | unix.O_CLOEXEC
+ var fd int
+ var err error
+ if pinDirs {
+ dir, derr := safefile.OpenDir(filepath.Dir(path))
+ if derr != nil {
+ return nil, derr
+ }
+ fd, err = unix.Openat(int(dir.Fd()), filepath.Base(path), flags, 0)
+ dir.Close()
+ } else {
+ fd, err = unix.Open(path, flags, 0)
+ }
+ if err != nil {
+ return nil, &os.PathError{Op: "open", Path: path, Err: err}
+ }
+ f := os.NewFile(uintptr(fd), path)
+ defer f.Close()
+ var st unix.Stat_t
+ if err := unix.Fstat(fd, &st); err != nil {
+ return nil, &os.PathError{Op: "fstat", Path: path, Err: err}
+ }
+ if err := checkFloorGateStat(floorGateStat{mode: st.Mode, uid: st.Uid, nlink: uint64(st.Nlink), size: st.Size}, owners); err != nil {
+ return nil, fmt.Errorf("%s %w", path, err)
+ }
+ floorGateAfterStat(path)
+ // Read one byte past the limit, so a file that grew after the fstat is
+ // refused rather than cut short into a match.
+ b, err := io.ReadAll(io.LimitReader(f, floorGateMaxBytes+1))
+ if err != nil {
+ return nil, &os.PathError{Op: "read", Path: path, Err: err}
+ }
+ if len(b) > floorGateMaxBytes {
+ return nil, fmt.Errorf("%s %w", path, errFloorGateUnsafe)
+ }
+ return b, nil
+}
+
+// floorGateAfterStat runs between a file's fstat and its read; a test sets
+// it to grow the file in that window.
+var floorGateAfterStat = func(path string) {}
+
+// floorGateStat is what checkFloorGateStat judges, from an fstat.
+type floorGateStat struct {
+ mode uint32
+ uid uint32
+ nlink uint64
+ size int64
+}
+
+// checkFloorGateStat refuses all but a regular file with one link, at most
+// floorGateMaxBytes long, owned by one of owners.
+func checkFloorGateStat(st floorGateStat, owners []uint32) error {
+ if st.mode&unix.S_IFMT != unix.S_IFREG || st.nlink != 1 ||
+ st.size < 0 || st.size > floorGateMaxBytes || !slices.Contains(owners, st.uid) {
+ return errFloorGateUnsafe
+ }
+ return nil
+}
diff --git a/internal/host/movesfloorgate_linux_test.go b/internal/host/movesfloorgate_linux_test.go
new file mode 100644
index 000000000..2cc7d5901
--- /dev/null
+++ b/internal/host/movesfloorgate_linux_test.go
@@ -0,0 +1,387 @@
+//go:build linux
+
+package host
+
+import (
+ "errors"
+ "os"
+ "path/filepath"
+ "strings"
+ "syscall"
+ "testing"
+ "time"
+
+ "github.com/carlosframework/platform/internal/moves"
+)
+
+// The floor gate (carlos system floor-allows-prev) is what the two system
+// units run, as root, in place of `cmp -s` on the agent's floor file. The
+// agent owns /data, so between the shell's test and the compare it could
+// swap in a symlink (root compares a file of the agent's choosing, a
+// one-bit oracle) or a FIFO (root blocks until the unit times out). The
+// gate opens each file once, refuses anything but a small single-link
+// regular file with an expected owner, and compares what it read from that
+// same descriptor.
+
+type floorGateFixture struct {
+ data, opt string
+ floor, verdict string
+ floorOwners []uint32
+ verdictOwners []uint32
+ floorContent string
+ verdictContent string
+}
+
+func newFloorGateFixture(t *testing.T) *floorGateFixture {
+ t.Helper()
+ root := t.TempDir()
+ f := &floorGateFixture{
+ data: filepath.Join(root, "data"),
+ opt: filepath.Join(root, "opt"),
+ floorOwners: []uint32{uint32(os.Getuid())},
+ verdictOwners: []uint32{uint32(os.Getuid())},
+ floorContent: "v0.30.0\n",
+ verdictContent: "v0.30.0\n",
+ }
+ f.floor = filepath.Join(f.data, moves.FloorMinReleaseFile)
+ f.verdict = filepath.Join(f.opt, moves.PrevAllowedFile)
+ for _, d := range []string{filepath.Dir(f.floor), f.opt} {
+ if err := os.MkdirAll(d, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ }
+ return f
+}
+
+func (f *floorGateFixture) write(t *testing.T) {
+ t.Helper()
+ if err := os.WriteFile(f.floor, []byte(f.floorContent), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(f.verdict, []byte(f.verdictContent), 0o644); err != nil {
+ t.Fatal(err)
+ }
+}
+
+func (f *floorGateFixture) run() error {
+ return floorAllowsPrev(f.floor, f.verdict, f.floorOwners, f.verdictOwners)
+}
+
+// runWithin runs the gate and fails the test if it has not answered within
+// d: a gate that opens a FIFO for reading without O_NONBLOCK parks forever.
+func (f *floorGateFixture) runWithin(t *testing.T, d time.Duration) error {
+ t.Helper()
+ done := make(chan error, 1)
+ go func() { done <- f.run() }()
+ select {
+ case err := <-done:
+ return err
+ case <-time.After(d):
+ t.Fatalf("the floor gate did not answer within %v: it blocked on the file", d)
+ return nil
+ }
+}
+
+func TestFloorGateMatchingContentPasses(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.write(t)
+ if err := f.run(); err != nil {
+ t.Fatalf("a verdict naming the floor byte for byte was refused: %v", err)
+ }
+}
+
+func TestFloorGateMismatchFails(t *testing.T) {
+ for _, tc := range []struct{ name, floor, verdict string }{
+ {"an older verdict", "v0.30.0\n", "v0.29.0\n"},
+ {"a missing newline", "v0.30.0\n", "v0.30.0"},
+ {"an extra byte", "v0.30.0\n", "v0.30.0\n\n"},
+ {"an empty verdict", "v0.30.0\n", ""},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.floorContent, f.verdictContent = tc.floor, tc.verdict
+ f.write(t)
+ err := f.run()
+ if err == nil {
+ t.Fatal("a verdict that does not name the floor passed")
+ }
+ if !errors.Is(err, errFloorGateMismatch) {
+ t.Fatalf("err = %v, want the mismatch", err)
+ }
+ })
+ }
+}
+
+func TestFloorGateMissingFilesFail(t *testing.T) {
+ t.Run("no verdict", func(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.write(t)
+ os.Remove(f.verdict)
+ if err := f.run(); err == nil {
+ t.Fatal("no verdict passed")
+ }
+ })
+ t.Run("no floor", func(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.write(t)
+ os.Remove(f.floor)
+ if err := f.run(); err == nil {
+ t.Fatal("a floor removed after the shell's test passed")
+ }
+ })
+}
+
+func TestFloorGateRefusesASymlinkedFloor(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.write(t)
+ // The agent's pick: a file that happens to hold the verdict's bytes.
+ chosen := filepath.Join(f.data, "chosen")
+ if err := os.WriteFile(chosen, []byte(f.verdictContent), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ os.Remove(f.floor)
+ if err := os.Symlink(chosen, f.floor); err != nil {
+ t.Fatal(err)
+ }
+ if err := f.run(); err == nil {
+ t.Fatal("a symlink at the floor path was followed")
+ }
+}
+
+func TestFloorGateRefusesASymlinkedMovesDir(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.write(t)
+ elsewhere := filepath.Join(f.data, "elsewhere")
+ if err := os.Rename(filepath.Dir(f.floor), elsewhere); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Symlink(elsewhere, filepath.Dir(f.floor)); err != nil {
+ t.Fatal(err)
+ }
+ if err := f.run(); err == nil {
+ t.Fatal("a symlink in place of .moves was followed")
+ }
+}
+
+func TestFloorGateRefusesAFIFOWithoutBlocking(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.write(t)
+ os.Remove(f.floor)
+ if err := syscall.Mkfifo(f.floor, 0o644); err != nil {
+ t.Fatal(err)
+ }
+ if err := f.runWithin(t, 5*time.Second); err == nil {
+ t.Fatal("a FIFO at the floor path passed")
+ }
+}
+
+func TestFloorGateRefusesAFIFOVerdictWithoutBlocking(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.write(t)
+ os.Remove(f.verdict)
+ if err := syscall.Mkfifo(f.verdict, 0o644); err != nil {
+ t.Fatal(err)
+ }
+ if err := f.runWithin(t, 5*time.Second); err == nil {
+ t.Fatal("a FIFO at the verdict path passed")
+ }
+}
+
+func TestFloorGateRefusesASymlinkedVerdict(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.write(t)
+ other := filepath.Join(f.opt, "other")
+ if err := os.WriteFile(other, []byte(f.floorContent), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ os.Remove(f.verdict)
+ if err := os.Symlink(other, f.verdict); err != nil {
+ t.Fatal(err)
+ }
+ if err := f.run(); err == nil {
+ t.Fatal("a symlink at the verdict path was followed")
+ }
+}
+
+func TestFloorGateRefusesAHardLinkedFloor(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.write(t)
+ if err := os.Link(f.floor, filepath.Join(f.data, "second-name")); err != nil {
+ t.Fatal(err)
+ }
+ if err := f.run(); err == nil {
+ t.Fatal("a floor file with two links passed")
+ }
+}
+
+func TestFloorGateRefusesAHardLinkedVerdict(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.write(t)
+ if err := os.Link(f.verdict, filepath.Join(f.opt, "second-name")); err != nil {
+ t.Fatal(err)
+ }
+ if err := f.run(); err == nil {
+ t.Fatal("a verdict file with two links passed")
+ }
+}
+
+func TestFloorGateRefusesOversizedFiles(t *testing.T) {
+ // Equal bytes, which cmp -s would have passed: the size is refused
+ // before any compare.
+ f := newFloorGateFixture(t)
+ big := strings.Repeat("v", floorGateMaxBytes) + "\n"
+ f.floorContent, f.verdictContent = big, big
+ f.write(t)
+ if err := f.run(); err == nil {
+ t.Fatalf("a %d-byte floor passed", len(big))
+ }
+ t.Run("verdict alone", func(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.verdictContent = big
+ f.write(t)
+ if err := f.run(); err == nil {
+ t.Fatalf("a %d-byte verdict passed", len(big))
+ }
+ })
+ t.Run("at the limit", func(t *testing.T) {
+ f := newFloorGateFixture(t)
+ exact := strings.Repeat("v", floorGateMaxBytes)
+ f.floorContent, f.verdictContent = exact, exact
+ f.write(t)
+ if err := f.run(); err != nil {
+ t.Fatalf("a %d-byte floor and verdict were refused: %v", len(exact), err)
+ }
+ })
+}
+
+// A file that passes the fstat's size check and then grows before the read
+// is refused by the bounded read. Both files grow by the same bytes, so
+// without that bound the two reads, cut at the same length, would match.
+func TestFloorGateRefusesAFileThatGrowsAfterTheStat(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.write(t)
+ grown := map[string]bool{}
+ orig := floorGateAfterStat
+ floorGateAfterStat = func(path string) {
+ grown[path] = true
+ w, err := os.OpenFile(path, os.O_WRONLY|os.O_APPEND, 0)
+ if err != nil {
+ t.Error(err)
+ return
+ }
+ defer w.Close()
+ if _, err := w.WriteString(strings.Repeat("v", 2*floorGateMaxBytes)); err != nil {
+ t.Error(err)
+ }
+ }
+ t.Cleanup(func() { floorGateAfterStat = orig })
+ err := f.run()
+ if err == nil {
+ t.Fatal("a floor that grew past the limit after the fstat passed")
+ }
+ if !errors.Is(err, errFloorGateUnsafe) {
+ t.Fatalf("err = %v, want the bounded read's refusal", err)
+ }
+ if !grown[f.floor] {
+ t.Fatal("the hook never ran between the floor's fstat and its read")
+ }
+}
+
+func TestFloorGateRefusesAnUnexpectedOwner(t *testing.T) {
+ // Without root a test cannot chown, so the expected owners move instead.
+ other := uint32(os.Getuid()) + 1
+ t.Run("floor", func(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.write(t)
+ f.floorOwners = []uint32{other}
+ if err := f.run(); err == nil {
+ t.Fatal("a floor owned by an unexpected account passed")
+ }
+ })
+ t.Run("verdict", func(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.write(t)
+ f.verdictOwners = []uint32{other}
+ if err := f.run(); err == nil {
+ t.Fatal("a verdict owned by an unexpected account passed")
+ }
+ })
+ t.Run("no owner expected", func(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.write(t)
+ f.floorOwners = nil
+ if err := f.run(); err == nil {
+ t.Fatal("a floor passed with no expected owner")
+ }
+ })
+}
+
+// checkFloorGateStat is the fstat judgement on its own, so the owner rule
+// is pinned with real root and service uids a test process cannot create.
+func TestCheckFloorGateStat(t *testing.T) {
+ const svc = 998
+ ok := floorGateStat{mode: syscall.S_IFREG | 0o644, uid: svc, nlink: 1, size: 8}
+ owners := []uint32{0, svc}
+ if err := checkFloorGateStat(ok, owners); err != nil {
+ t.Fatalf("a good file was refused: %v", err)
+ }
+ asRoot := ok
+ asRoot.uid = 0
+ if err := checkFloorGateStat(asRoot, owners); err != nil {
+ t.Fatalf("a root-owned file (root's apply writes the floor) was refused: %v", err)
+ }
+ for name, mut := range map[string]func(*floorGateStat){
+ "another account": func(s *floorGateStat) { s.uid = 1000 },
+ "a FIFO": func(s *floorGateStat) { s.mode = syscall.S_IFIFO | 0o644 },
+ "a symlink": func(s *floorGateStat) { s.mode = syscall.S_IFLNK | 0o777 },
+ "a directory": func(s *floorGateStat) { s.mode = syscall.S_IFDIR | 0o755 },
+ "a device": func(s *floorGateStat) { s.mode = syscall.S_IFCHR | 0o644 },
+ "a socket": func(s *floorGateStat) { s.mode = syscall.S_IFSOCK | 0o644 },
+ "two links": func(s *floorGateStat) { s.nlink = 2 },
+ "no links": func(s *floorGateStat) { s.nlink = 0 },
+ "too big": func(s *floorGateStat) { s.size = floorGateMaxBytes + 1 },
+ "negative size": func(s *floorGateStat) { s.size = -1 },
+ } {
+ s := ok
+ mut(&s)
+ if err := checkFloorGateStat(s, owners); err == nil {
+ t.Errorf("%s passed", name)
+ }
+ }
+ if err := checkFloorGateStat(ok, []uint32{0}); err == nil {
+ t.Error("a service-owned file passed where only root is expected (the verdict)")
+ }
+}
+
+// The command's exit code is what the shell reads: 0 only for a match, and
+// every refusal, failure or misuse the same non-zero as a mismatch.
+func TestFloorGateMainExitCodes(t *testing.T) {
+ f := newFloorGateFixture(t)
+ f.write(t)
+ run := func(args ...string) int {
+ var b strings.Builder
+ return floorGateMain(args, f.floorOwners, f.verdictOwners, &b)
+ }
+ if code := run(f.floor, f.verdict); code != 0 {
+ t.Fatalf("a match exited %d", code)
+ }
+ mismatch := newFloorGateFixture(t)
+ mismatch.verdictContent = "v0.29.0\n"
+ mismatch.write(t)
+ codeMismatch := run(mismatch.floor, mismatch.verdict)
+ if codeMismatch == 0 {
+ t.Fatal("a mismatch exited 0")
+ }
+ for name, args := range map[string][]string{
+ "no arguments": nil,
+ "one argument": {f.floor},
+ "three arguments": {f.floor, f.verdict, f.verdict},
+ "a relative floor": {"data/.moves/floor.min-release", f.verdict},
+ "a relative verdict": {f.floor, "carlos.prev.allowed"},
+ "a missing verdict": {f.floor, filepath.Join(f.opt, "absent")},
+ } {
+ if code := run(args...); code != codeMismatch {
+ t.Errorf("%s exited %d, want the mismatch's %d", name, code, codeMismatch)
+ }
+ }
+}
diff --git a/internal/host/movesfloorgate_other.go b/internal/host/movesfloorgate_other.go
new file mode 100644
index 000000000..4f28ecdec
--- /dev/null
+++ b/internal/host/movesfloorgate_other.go
@@ -0,0 +1,10 @@
+//go:build !linux
+
+package host
+
+// The floor gate runs as root on a Linux box only (the system units). Off
+// Linux it refuses, which the units read as a mismatch: fail closed, like
+// openBeneath.
+func floorAllowsPrev(floorPath, verdictPath string, floorOwners, verdictOwners []uint32) error {
+ return errSystemUnsupported
+}
diff --git a/internal/host/units/carlos-system-recover.service b/internal/host/units/carlos-system-recover.service
index ea6465c10..fe418e72d 100644
--- a/internal/host/units/carlos-system-recover.service
+++ b/internal/host/units/carlos-system-recover.service
@@ -12,7 +12,14 @@
# floor is armed (/data/.moves/floor.min-release is there and not empty),
# carlos.prev runs only when its verdict file (/opt/carlos/carlos.prev.allowed,
# written by root after comparing versions in Go) names that same floor,
-# byte for byte.
+# byte for byte. A floor path that is a symlink or anything but a regular
+# file (a dangling link, a FIFO, a directory) never reads as "no floor":
+# the current binary runs (#267). carlos.prev's own floor gate (carlos
+# system floor-allows-prev) makes the comparison: it opens each file once,
+# without following a symlink or waiting on a FIFO, refuses anything but a
+# small single-link regular file with the expected owner, and exits 0 only
+# for a match. Not cmp: the agent owns /data and could swap the floor file
+# between the shell's tests and cmp's open.
# Otherwise the current binary runs, and its own check refuses to restore a
# carlos.prev below the floor. No floor file: the choice is exactly as
# before. The line carries no percent sign and no dollar sign, both of which
@@ -26,7 +33,7 @@ DefaultDependencies=yes
Type=oneshot
TimeoutStartSec=10min
EnvironmentFile=-/etc/carlos/host.env
-ExecStart=/bin/sh -c 'if [ -f /opt/carlos/system/router/journal.json ] && [ -x /opt/carlos/carlos.prev ] && [ ! -L /data/.moves/floor.min-release ] && { [ ! -e /data/.moves/floor.min-release ] || [ -f /data/.moves/floor.min-release ]; } && { ! [ -s /data/.moves/floor.min-release ] || cmp -s /data/.moves/floor.min-release /opt/carlos/carlos.prev.allowed; }; then exec /opt/carlos/carlos.prev system apply --recover; fi; exec /opt/carlos/carlos system apply --recover'
+ExecStart=/bin/sh -c 'if [ -f /opt/carlos/system/router/journal.json ] && [ -x /opt/carlos/carlos.prev ] && [ ! -L /data/.moves/floor.min-release ] && { [ ! -e /data/.moves/floor.min-release ] || [ -f /data/.moves/floor.min-release ]; } && { ! [ -s /data/.moves/floor.min-release ] || /opt/carlos/carlos.prev system floor-allows-prev /data/.moves/floor.min-release /opt/carlos/carlos.prev.allowed; }; then exec /opt/carlos/carlos.prev system apply --recover; fi; exec /opt/carlos/carlos system apply --recover'
ProtectHome=yes
PrivateTmp=yes
diff --git a/internal/host/units/carlos-system-update.service b/internal/host/units/carlos-system-update.service
index 98b61651c..e42678396 100644
--- a/internal/host/units/carlos-system-update.service
+++ b/internal/host/units/carlos-system-update.service
@@ -10,7 +10,14 @@
# floor is armed (/data/.moves/floor.min-release is there and not empty),
# carlos.prev runs only when its verdict file (/opt/carlos/carlos.prev.allowed,
# written by root after comparing versions in Go) names that same floor,
-# byte for byte.
+# byte for byte. A floor path that is a symlink or anything but a regular
+# file (a dangling link, a FIFO, a directory) never reads as "no floor":
+# the current binary runs (#267). carlos.prev's own floor gate (carlos
+# system floor-allows-prev) makes the comparison: it opens each file once,
+# without following a symlink or waiting on a FIFO, refuses anything but a
+# small single-link regular file with the expected owner, and exits 0 only
+# for a match. Not cmp: the agent owns /data and could swap the floor file
+# between the shell's tests and cmp's open.
# Otherwise the current binary runs, and its own check refuses to restore a
# carlos.prev below the floor. No floor file: the choice is exactly as
# before. The line carries no percent sign and no dollar sign, both of which
@@ -32,7 +39,7 @@ OnFailure=carlos-system-recover.service
Type=oneshot
TimeoutStartSec=15min
EnvironmentFile=-/etc/carlos/host.env
-ExecStart=/bin/sh -c 'if [ -f /opt/carlos/system/router/journal.json ] && [ -x /opt/carlos/carlos.prev ] && [ ! -L /data/.moves/floor.min-release ] && { [ ! -e /data/.moves/floor.min-release ] || [ -f /data/.moves/floor.min-release ]; } && { ! [ -s /data/.moves/floor.min-release ] || cmp -s /data/.moves/floor.min-release /opt/carlos/carlos.prev.allowed; }; then exec /opt/carlos/carlos.prev system apply; fi; exec /opt/carlos/carlos system apply'
+ExecStart=/bin/sh -c 'if [ -f /opt/carlos/system/router/journal.json ] && [ -x /opt/carlos/carlos.prev ] && [ ! -L /data/.moves/floor.min-release ] && { [ ! -e /data/.moves/floor.min-release ] || [ -f /data/.moves/floor.min-release ]; } && { ! [ -s /data/.moves/floor.min-release ] || /opt/carlos/carlos.prev system floor-allows-prev /data/.moves/floor.min-release /opt/carlos/carlos.prev.allowed; }; then exec /opt/carlos/carlos.prev system apply; fi; exec /opt/carlos/carlos system apply'
# --- hardening (the carlos-stage.service set) ---
ProtectHome=yes
diff --git a/system.go b/system.go
index c746c5567..ac3790880 100644
--- a/system.go
+++ b/system.go
@@ -49,7 +49,7 @@ func systemVerb(args []string) (verb string, rest []string, component string) {
// untouched in rest for runSystemAntivirusVerb to read.
// layout is a family too: migrate|revert|status stay in rest for
// runSystemLayout.
- case "guard-arm", "guard-exec", "journal-migrate", "update", "status", "rollback", "cancel-activation", "apply", "antivirus", "layout", "geoip", "help", "-h", "--help":
+ case "guard-arm", "guard-exec", "journal-migrate", "floor-allows-prev", "update", "status", "rollback", "cancel-activation", "apply", "antivirus", "layout", "geoip", "help", "-h", "--help":
return args[0], args[1:], component
}
return "", args, component
@@ -64,6 +64,9 @@ func runSystem(args []string) {
host.RunSystemGuardExec(rest)
case "journal-migrate":
host.RunSystemJournalMigrate(rest)
+ case "floor-allows-prev":
+ // Root's floor gate for the system units; not for operators.
+ host.RunSystemFloorAllowsPrev(rest)
case "apply":
host.RunSystemApply(rest)
diff --git a/system_test.go b/system_test.go
index 3a0d5f77c..a9af9f119 100644
--- a/system_test.go
+++ b/system_test.go
@@ -17,6 +17,8 @@ func TestSystemVerbAcceptsTheComponentForm(t *testing.T) {
{[]string{"router", "update"}, "update", "router", 0},
{[]string{"router", "status"}, "status", "router", 0},
{[]string{"apply", "--recover"}, "apply", "router", 1},
+ // The system units' floor gate: both file paths stay in rest.
+ {[]string{"floor-allows-prev", "/data/.moves/floor.min-release", "/opt/carlos/carlos.prev.allowed"}, "floor-allows-prev", "router", 2},
// antivirus is a family: its sub-verb stays in rest untouched.
{[]string{"antivirus", "status"}, "antivirus", "router", 1},
{[]string{"antivirus", "enable", "--console", "x"}, "antivirus", "router", 3},