amadan
moves floor gate: compare the floor file in Go, not with cmp
The system units tested /data/.moves/floor.min-release in the shell and then reopened it through cmp -s. The agent owns /data, so between the two it could swap in a symlink (root compared a file of its choosing, a one-bit oracle) or a FIFO (root blocked until the unit timed out), during an interrupted update with a journal and an executable carlos.prev. Both units now run carlos.prev's floor gate, `carlos system floor-allows-prev <floor> <verdict>`: each file opened once, O_NOFOLLOW and O_NONBLOCK (the floor's directories pinned without following a symlink too), fstat-checked on that descriptor (regular, one link, at most 256 bytes, owned by root or the service account for the floor, root for the verdict), read from it and compared byte for byte. Exit 0 only for a match; every refusal, failure or misuse exits 1, the same as a mismatch. The no-floor path stays pure shell. Off Linux the gate refuses. Found in the #267 review (P3). Copy rows MV-N315 to MV-N318 pending.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
12 files changed,
+668
−20
approved_moves_copy_test.go+4 −0docs/superpowers/plans/2026-10-06-moves-phase1-copy.md+11 −0internal/host/host_test.go+86 −14internal/host/movesfloor.go+3 −1internal/host/movesfloorgate.go+76 −0internal/host/movesfloorgate_linux.go+104 −0internal/host/movesfloorgate_linux_test.go+354 −0internal/host/movesfloorgate_other.go+10 −0internal/host/units/carlos-system-recover.service+7 −2internal/host/units/carlos-system-update.service+7 −2system.go+4 −1system_test.go+2 −0
diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go| index 1cb1a1556..82f1d90b1 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 2313b98e3..9fe591b3a 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" |
| @@ -1315,11 +1316,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) |
| @@ -1327,11 +1331,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 |
| @@ -1762,12 +1769,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) |
| @@ -1800,25 +1843,41 @@ func TestSystemUnitsSelectPrevByTheFloor(t *testing.T) { |
| name string |
| journal bool |
| floor, verdict string // "" means no file |
| + floorLink bool // the floor path is a symlink to a file holding floor |
| noPrev bool |
| want string |
| + gate string // which binary ran the floor gate, "" for none |
| }{ |
| - {"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"}, |
| + {"no journal", false, "", "", false, false, "carlos", ""}, |
| + {"no floor", true, "", "", false, false, "carlos.prev", ""}, |
| + {"no floor, no prev", true, "", "", false, true, "carlos", ""}, |
| + {"floor and a verdict naming it", true, "v0.30.0\n", "v0.30.0\n", false, false, "carlos.prev", "carlos.prev"}, |
| + {"a verdict for an older floor", true, "v0.30.0\n", "v0.29.0\n", false, false, "carlos", "carlos.prev"}, |
| + {"floor and no verdict", true, "v0.30.0\n", "", false, false, "carlos", "carlos.prev"}, |
| + {"no journal, armed and allowed", false, "v0.30.0\n", "v0.30.0\n", false, false, "carlos", ""}, |
| + {"armed, no prev", true, "v0.30.0\n", "v0.30.0\n", false, true, "carlos", ""}, |
| + // The agent owns /data: a symlink there must not choose the |
| + // file root compares (cmp -s followed it). |
| + {"a symlinked floor naming the verdict", true, "v0.30.0\n", "v0.30.0\n", true, false, "carlos", "carlos.prev"}, |
| } { |
| 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) |
| + sh := "#!/bin/sh\n" + |
| + "if [ \"$1\" = system ] && [ \"$2\" = floor-allows-prev ]; then\n" + |
| + " echo " + name + " >> " + gate + "\n" + |
| + " shift 2\n" + |
| + " exec " + self + " -test.run='^TestFloorGateHelperProcess$' -- \"$@\"\n" + |
| + "fi\n" + |
| + "echo " + name + " > " + ran + "\n" |
| + os.WriteFile(filepath.Join(opt, name), []byte(sh), 0o755) |
| } |
| stub("carlos") |
| if !tc.noPrev { |
| @@ -1828,20 +1887,33 @@ func TestSystemUnitsSelectPrevByTheFloor(t *testing.T) { |
| os.WriteFile(filepath.Join(opt, "system", "router", "journal.json"), []byte("{}"), 0o600) |
| } |
| if tc.floor != "" { |
| - os.WriteFile(filepath.Join(data, moves.FloorMinReleaseFile), []byte(tc.floor), 0o644) |
| + floor := filepath.Join(data, moves.FloorMinReleaseFile) |
| + if tc.floorLink { |
| + chosen := filepath.Join(data, "chosen") |
| + os.WriteFile(chosen, []byte(tc.floor), 0o644) |
| + os.Symlink(chosen, floor) |
| + } else { |
| + os.WriteFile(floor, []byte(tc.floor), 0o644) |
| + } |
| } |
| if tc.verdict != "" { |
| os.WriteFile(filepath.Join(opt, moves.PrevAllowedFile), []byte(tc.verdict), 0o644) |
| } |
| 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..993075b97 |
| --- /dev/null |
| +++ b/internal/host/movesfloorgate_linux.go |
| @@ -0,0 +1,104 @@ |
| +//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) |
| + } |
| + // 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 |
| +} |
| + |
| +// 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..1054f0d01 |
| --- /dev/null |
| +++ b/internal/host/movesfloorgate_linux_test.go |
| @@ -0,0 +1,354 @@ |
| +//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) |
| + } |
| + }) |
| +} |
| + |
| +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 3da2b581b..ae72c07af 100644 |
| --- a/internal/host/units/carlos-system-recover.service |
| +++ b/internal/host/units/carlos-system-recover.service |
| @@ -12,7 +12,12 @@ |
| # 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. carlos.prev's own floor gate (carlos system |
| +# floor-allows-prev) makes that 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 test 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 +31,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 ] && { ! [ -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 ] && { ! [ -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 9bc8d25f9..24f7679bd 100644 |
| --- a/internal/host/units/carlos-system-update.service |
| +++ b/internal/host/units/carlos-system-update.service |
| @@ -10,7 +10,12 @@ |
| # 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. carlos.prev's own floor gate (carlos system |
| +# floor-allows-prev) makes that 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 test 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 +37,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 ] && { ! [ -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 ] && { ! [ -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}, |