diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 867293b..1ff9a5b 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -728,6 +728,13 @@ func ApplyMindingWindows( // The file guard (refusedPlaces) refuses what is below the directory whatever its module; this // keeps the module from going on as if its directory were there. Only the guard's refusals: a // directory that failed to be made is a failure like any other. + // A directory refused at the moment of use (novox/hq issue 358: a link another account could make + // found in its path by the walk) is a refusal like the guard's, from here on: it gates its module, and + // what is below it is refused whatever its module (applyOne's guard reads in.refused as it grows). + var refusal *PlacementRefusedError + if dir, ok := resource.(*declaration.Directory); ok && errors.As(err, &refusal) && in.refused[filepath.Clean(dir.Path)] == nil { + in.refused[filepath.Clean(dir.Path)] = err + } if dir, ok := resource.(*declaration.Directory); ok && in.refused[filepath.Clean(dir.Path)] != nil { if module, ours := moduleOf(resource.Identity()); ours { gated[module] = true @@ -1054,6 +1061,13 @@ func applyOne(ctx context.Context, sys system.System, r declaration.Resource, ru if err := in.refused[filepath.Clean(r.Target())]; err != nil { return begin(r), err } + // And below a directory refused during this apply (novox/hq issue 358), which the guard could not see + // before it began. + if _, isDir := r.(*declaration.Directory); !isDir { + if err := belowRefused(r.Target(), in.refused); err != nil { + return begin(r), err + } + } case *declaration.Container: if err := refuseMounts(res, in.refused); err != nil { return begin(r), err @@ -1147,6 +1161,18 @@ func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore if existed && !before.IsDir() { return out, fmt.Errorf("%s exists and is not a directory", r.Path) } + // Outside every home, nothing more is done — not even used as found, which lets what is declared below it go + // on — when the walk finds a link another account could make in its path now (novox/hq issue 358). A handed-over + // directory has its own, stricter walk below. + if meshDirsByDescriptor && existed && homeAbove(r.Path) == "" && !HandedOver()[handOverKey(r.ID, r.Path)] { + if err := judgeMeshDir(r.Path); err != nil { + var link *LinkInPathError + if errors.As(err, &link) { + return out, &PlacementRefusedError{Path: r.Path, Why: "it " + link.why()} + } + return out, err + } + } // Handed to the mesh by the operator (`nox node hand-over`, or `hand-over` at the machine): given the declared // owner and mode through a descriptor that followed no link (giveHandedOver) — **on every apply, not only the // first** (review of issue 356). After the first, the record is no longer used as found and would read as the @@ -1181,6 +1207,11 @@ func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore } } + // Outside every home, made and given its mode and owner only through a descriptor opened from /, following + // no link another account could have made (novox/hq issue 358, mesh_dirs.go). + if meshDirsByDescriptor && homeAbove(r.Path) == "" { + return applyMeshDirectory(r, out, mode, existed) + } if !existed { if err := makeDirs(r.Path, mode, r.Owner); err != nil { return out, err diff --git a/internal/apply/homes_other.go b/internal/apply/homes_other.go index cf5c92e..87cc3ec 100644 --- a/internal/apply/homes_other.go +++ b/internal/apply/homes_other.go @@ -8,6 +8,8 @@ package apply import ( "os" "path/filepath" + + "github.com/novox/mesh-host/internal/declaration" ) func chmodUnder(home, path string, mode os.FileMode) error { @@ -71,3 +73,12 @@ func judgeMeshDir(path string) error { return &PlacementRefusedError{Path: path, Why: "a mesh directory is judged only by a walk that follows no link " + "another account could make, which this system cannot do"} } + +// meshDirsByDescriptor is false where there is no openat with O_NOFOLLOW: a mesh directory is changed by path, as +// before novox/hq issue 358. The node-engine runs on Linux. +const meshDirsByDescriptor = false + +func applyMeshDirectory(r *declaration.Directory, out Outcome, _ os.FileMode, _ bool) (Outcome, error) { + return out, &PlacementRefusedError{Path: r.Path, Why: "a mesh directory is changed only through a descriptor " + + "that followed no link another account could make, which this system cannot open"} +} diff --git a/internal/apply/mesh_dirs_apply_linux.go b/internal/apply/mesh_dirs_apply_linux.go new file mode 100644 index 0000000..754c845 --- /dev/null +++ b/internal/apply/mesh_dirs_apply_linux.go @@ -0,0 +1,114 @@ +//go:build linux + +package apply + +// The apply half of mesh_dirs.go (novox/hq issue 358): a mesh directory outside every home made and given its mode +// and owner through the walk's descriptor. + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "golang.org/x/sys/unix" + + "github.com/novox/mesh-host/internal/declaration" +) + +// meshDirsByDescriptor says this system can open a directory component by component without following a link. +const meshDirsByDescriptor = true + +// openMeshDir opens the directory at path, from /, one component at a time with O_DIRECTORY|O_NOFOLLOW; a missing +// component is made in its parent's descriptor with mode, and made is called with each one made and its descriptor. +// A link in a parent is followed only when couldReplace and linkReplacer find that root alone could have made it, +// and its target is walked again from /; any other link is a LinkInPathError, and the last component being one is +// refused as not placed. Answers the directory's descriptor, which the caller closes, with the walk: what it made +// and the path it reached the directory at. +func openMeshDir(path string, mode os.FileMode, made func(dir string, fd int) error) (int, *meshWalk, error) { + w := &meshWalk{want: filepath.Clean(path), create: true, mode: mode, made: made} + fd, err := w.open() + return fd, w, err +} + +// applyMeshDirectory gives a mesh directory outside every home its declared mode and owner through a descriptor +// (novox/hq issue 358): opened by openMeshDir, made there when missing, judged a directory by fstat on it, changed +// with fchmod and fchown on it, and read back from it. The parents it makes inside the owner's home are the owner's, +// as makeDirs gives them (novox/hq ADR 0182). +func applyMeshDirectory(r *declaration.Directory, out Outcome, mode os.FileMode, existed bool) (Outcome, error) { + uid, gid, chown := 0, 0, r.Owner != "" + var err error + if chown { + if uid, gid, err = idsOf(r.Owner); err != nil { + return out, fmt.Errorf("%s should belong to %q: %w", r.Path, r.Owner, err) + } + } + clean := filepath.Clean(r.Path) + home := "" + if chown { + if h, herr := homeOf(r.Owner); herr == nil && h != "" { + home = filepath.Clean(h) + } + } + giveMade := func(dir string, fd int) error { + if dir == clean || home == "" || (dir != home && !strings.HasPrefix(dir, home+string(os.PathSeparator))) { + return nil + } + if err := unix.Fchown(fd, uid, gid); err != nil { + return fmt.Errorf("cannot give %s to %q: %w", dir, r.Owner, &os.PathError{Op: "chown", Path: dir, Err: err}) + } + return nil + } + fd, walk, err := openMeshDir(clean, mode, giveMade) + if err != nil { + var link *LinkInPathError + if errors.As(err, &link) { + return out, &PlacementRefusedError{Path: r.Path, Why: "it " + link.why()} + } + return out, err + } + defer unix.Close(fd) + // Made by this walk when its last component is: at the path the walk reached it at, which root-only links in its + // path may have moved from the path declared. + created := len(walk.Made) > 0 && walk.Made[len(walk.Made)-1] == walk.At + + var st unix.Stat_t + if err := unix.Fstat(fd, &st); err != nil { + return out, &os.PathError{Op: "fstat", Path: r.Path, Err: err} + } + beforeMode, beforeUID, beforeGID := os.FileMode(st.Mode&0o777), int(st.Uid), int(st.Gid) + // Set explicitly even when it existed: a mode given at creation is not a mode maintained. + if err := unix.Fchmod(fd, uint32(mode.Perm())); err != nil { + return out, &os.PathError{Op: "chmod", Path: r.Path, Err: err} + } + ownedAlready := !chown || (beforeUID == uid && beforeGID == gid) + if !ownedAlready { + if err := unix.Fchown(fd, uid, gid); err != nil { + return out, fmt.Errorf("cannot give %s to %q: %w", r.Path, r.Owner, &os.PathError{Op: "chown", Path: r.Path, Err: err}) + } + } + + // Read back, from the descriptor that was changed. + if err := unix.Fstat(fd, &st); err != nil { + return out, fmt.Errorf("made %s and cannot stat it: %w", r.Path, err) + } + if st.Mode&unix.S_IFMT != unix.S_IFDIR { + return out, fmt.Errorf("%s is not a directory after applying", r.Path) + } + if got := os.FileMode(st.Mode & 0o777); got != mode.Perm() { + return out, fmt.Errorf("%s is mode %o after setting %o", r.Path, got, mode.Perm()) + } + + out.Action = "unchanged" + switch { + case created || !existed: + out.Action = "created" + case !ownedAlready: + out.Action = "updated" + case beforeMode != mode.Perm(): + out.Action = "updated" + out.Detail = fmt.Sprintf("mode %o to %o", beforeMode, mode.Perm()) + } + return out, nil +} diff --git a/internal/apply/mesh_dirs_gate_test.go b/internal/apply/mesh_dirs_gate_test.go new file mode 100644 index 0000000..dfb64de --- /dev/null +++ b/internal/apply/mesh_dirs_gate_test.go @@ -0,0 +1,95 @@ +//go:build linux + +package apply + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/store" +) + +// Review 47 of novox/hq issue 358: a directory the walk refuses gates its module, and nothing below it is written, +// whether the link was there when the apply began or appeared while it ran. + +// aLinkedModule is a module's directory at /parent/state, its parent a link to /elsewhere, and a file of +// the module below the directory; and another module's file elsewhere. +func aLinkedModule(t *testing.T) (target, other, decl string) { + t.Helper() + base := t.TempDir() + target = filepath.Join(base, "elsewhere", "state") + if err := os.MkdirAll(target, 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(base, "elsewhere"), filepath.Join(base, "parent")); err != nil { + t.Fatal(err) + } + dir := filepath.Join(base, "parent", "state") + other = filepath.Join(base, "other.conf") + decl = `{"id":"mod.state","type":"directory","path":"` + dir + `","mode":"0700"},` + + `{"id":"mod.conf","type":"file","path":"` + dir + `/settings.conf","content":"a=1\n"},` + + `{"id":"other.conf","type":"file","path":"` + other + `","content":"kept apart\n"}` + return target, other, decl +} + +func holdsTheGate(t *testing.T, target, other string, report Report, err error) { + t.Helper() + var refusal *PlacementRefusedError + if !errors.As(err, &refusal) { + t.Fatalf("the directory behind the link was not refused: %v", err) + } + if _, statErr := os.Stat(filepath.Join(target, "settings.conf")); statErr == nil { + t.Error("a file was written into the link's target, below the directory the engine refused") + } + if info, _ := os.Stat(target); info.Mode().Perm() != 0o755 { + t.Errorf("the link's target was given mode %o (was 755)", info.Mode().Perm()) + } + if o := outcomeOf(report, "mod.conf"); o.Action != "skipped" { + t.Errorf("the file below the refused directory is not reported skipped: %+v", o) + } + if strings.Contains(err.Error(), "everything was attempted") || !strings.Contains(err.Error(), "the rest of mod was not attempted") { + t.Errorf("the report does not say the module was gated: %v", err) + } + if got, readErr := os.ReadFile(other); readErr != nil || string(got) != "kept apart\n" { + t.Errorf("another module's file was not applied: %q, %v", got, readErr) + } +} + +// The link there when the apply begins: refused by the guard before anything is applied. +func TestADirectoryBehindALinkGatesItsModuleThroughApply(t *testing.T) { + target, other, decl := aLinkedModule(t) + report, _, err := Apply(context.Background(), archHost(t), declare(t, decl), store.State{}, store.OriginDeclared, + noServices, nil, nil) + holdsTheGate(t, target, other, report, err) +} + +// The link found only at the moment of use: refused by the walk during the apply, and still a refusal that gates its +// module and what is below it. Simulated by trusting / for the guard's walk (two walks of /: one to the link, one +// from its target) and not for the apply's. +func TestADirectoryRefusedDuringTheApplyGatesItsModule(t *testing.T) { + target, other, decl := aLinkedModule(t) + wasDirs, wasLinks := couldReplace, linkReplacer + walksOfRoot := 0 + couldReplace = func(path string, _, _, _ uint32) string { + if path == "/" { + walksOfRoot++ + if walksOfRoot > 2 { + return "every account (a test's /)" + } + } + return "" + } + linkReplacer = func(string, uint32) string { return "" } + t.Cleanup(func() { couldReplace, linkReplacer = wasDirs, wasLinks }) + + report, _, err := Apply(context.Background(), archHost(t), declare(t, decl), store.State{}, store.OriginDeclared, + noServices, nil, nil) + if walksOfRoot < 3 { + t.Fatalf("the apply did not walk again at the moment of use (%d walks)", walksOfRoot) + } + holdsTheGate(t, target, other, report, err) +} diff --git a/internal/apply/mesh_dirs_test.go b/internal/apply/mesh_dirs_test.go new file mode 100644 index 0000000..b84abea --- /dev/null +++ b/internal/apply/mesh_dirs_test.go @@ -0,0 +1,91 @@ +//go:build linux + +package apply + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/declaration" + "github.com/novox/mesh-host/internal/store" +) + +// Defends novox/hq issue 358: a mesh directory outside every home is given its mode and owner only through a +// descriptor opened from /, so a parent swapped for a link between two applies cannot carry root's chmod and +// chown to the link's target — unless root alone could have made that link (/var/run is one on every node). + +// A link only root could have made — itself root's, in directories no other account may write — is followed, as +// /var/run to /run is on every node; relative or absolute. +func TestALinkOnlyRootCouldMakeIsFollowed(t *testing.T) { + for name, to := range map[string]func(base string) string{ + "absolute": func(base string) string { return filepath.Join(base, "elsewhere") }, + "relative": func(string) string { return "elsewhere" }, + } { + t.Run(name, func(t *testing.T) { + rootAlone(t, "") + base, r, previous := aMeshDirectory(t) + target := swapParentForLink(t, base, to(base)) + + out, err := applyDirectory(r, previous, true) + if err != nil { + t.Fatalf("a directory behind a root-only link was refused: %v", err) + } + if got := permOf(t, target); got != 0o700 { + t.Errorf("the directory behind a root-only link is mode %o, want 700", got) + } + if out.Action != "updated" { + t.Errorf("the apply said %q, want updated", out.Action) + } + }) + } +} + +// A link another account owns refuses the directory even in directories root alone may write, naming the account. +func TestALinkAnotherAccountOwnsIsRefused(t *testing.T) { + rootAlone(t, "the account mallory (it owns the link)") + base, r, previous := aMeshDirectory(t) + target := swapParentForLink(t, base, filepath.Join(base, "elsewhere")) + + _, err := applyDirectory(r, previous, true) + if err == nil || !strings.Contains(err.Error(), "mallory") { + t.Fatalf("a link another account owns was not refused naming it: %v", err) + } + if got := permOf(t, target); got != 0o755 { + t.Errorf("the link's target was given mode %o (was 755)", got) + } +} + +// A directory that is missing is made through the walk, below a link root alone could make. +func TestAMissingDirectoryIsMadeBehindARootOnlyLink(t *testing.T) { + rootAlone(t, "") + base := t.TempDir() + if err := os.Mkdir(filepath.Join(base, "real"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink("real", filepath.Join(base, "link")); err != nil { + t.Fatal(err) + } + r := &declaration.Directory{ID: "mod.state", Path: filepath.Join(base, "link", "a", "b"), Mode: "0750"} + out, err := applyDirectory(r, store.Applied{}, false) + if err != nil { + t.Fatal(err) + } + if out.Action != "created" { + t.Errorf("the apply said %q, want created", out.Action) + } + if got := permOf(t, filepath.Join(base, "real", "a", "b")); got != 0o750 { + t.Errorf("the made directory is mode %o, want 750", got) + } +} + +// rootAlone has every directory count as writable by root alone, as /, /var and /run are on a node, and a link's +// owner be linkOwner ("" for root): a test runs below /tmp, which every account may write. +func rootAlone(t *testing.T, linkOwner string) { + t.Helper() + wasDirs, wasLinks := couldReplace, linkReplacer + couldReplace = func(string, uint32, uint32, uint32) string { return "" } + linkReplacer = func(string, uint32) string { return linkOwner } + t.Cleanup(func() { couldReplace, linkReplacer = wasDirs, wasLinks }) +} diff --git a/internal/apply/placement_guard.go b/internal/apply/placement_guard.go index 01112d5..e23ef45 100644 --- a/internal/apply/placement_guard.go +++ b/internal/apply/placement_guard.go @@ -523,6 +523,20 @@ func refusedPlaces(resources []declaration.Resource) map[string]error { switch res := r.(type) { case *declaration.Directory: err = refusePath(res.Path, placingDirectory, moduleOfID(res.ID), res.Owner) + // And outside every home, the walk's judgement before anything is applied (novox/hq issue 358): a + // link another account could make in its path refuses it, and with it what is below it, whatever + // order they are declared in. The apply walks again at the moment of use. A directory handed over + // keeps its own, stricter walk (issue 356), which refuses any link at all. + if err == nil && meshDirsByDescriptor && homeAbove(res.Path) == "" && + !HandedOver()[handOverKey(res.ID, res.Path)] { + if judged := judgeMeshDir(res.Path); judged != nil { + err = judged + var link *LinkInPathError + if errors.As(judged, &link) { + err = &PlacementRefusedError{Path: res.Path, Why: "it " + link.why()} + } + } + } case *declaration.Access: err = refusePath(res.Path, placingAccess, moduleOfID(res.ID), "") default: diff --git a/internal/apply/replay358_test.go b/internal/apply/replay358_test.go new file mode 100644 index 0000000..4f3eef7 --- /dev/null +++ b/internal/apply/replay358_test.go @@ -0,0 +1,83 @@ +//go:build linux + +package apply + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/declaration" + "github.com/novox/mesh-host/internal/store" +) + +// The replay of novox/hq issue 358, written only with what the node-engine had before its fix, so it can be laid +// over the commit before (mesh-lab's replays register): a mesh directory outside every home was given its mode +// and owner by path, so a parent swapped for a link between two applies carried root's chmod to the link's target. + +// aMeshDirectory applies a directory declared at /parent/state once, as the mesh's own, and answers it +// with the record a second apply would read. +func aMeshDirectory(t *testing.T) (base string, r *declaration.Directory, previous store.Applied) { + t.Helper() + base = t.TempDir() + r = &declaration.Directory{ID: "mod.state", Path: filepath.Join(base, "parent", "state"), Mode: "0700"} + out, err := applyDirectory(r, store.Applied{}, false) + if err != nil { + t.Fatalf("the first apply: %v", err) + } + if out.Action != "created" { + t.Fatalf("the first apply said %q, want created", out.Action) + } + return base, r, store.Applied{ID: r.ID, Type: "directory", Target: r.Path} +} + +// swapParentForLink moves /parent away and puts a link to elsewhere (as written) in its place, where +// elsewhere holds a directory state of mode 0755: what an account that can write does between two applies. +func swapParentForLink(t *testing.T, base, to string) (target string) { + t.Helper() + elsewhere := filepath.Join(base, "elsewhere") + target = filepath.Join(elsewhere, "state") + if err := os.MkdirAll(target, 0o755); err != nil { + t.Fatal(err) + } + if err := os.Chmod(target, 0o755); err != nil { + t.Fatal(err) + } + if err := os.Rename(filepath.Join(base, "parent"), filepath.Join(base, "moved")); err != nil { + t.Fatal(err) + } + if err := os.Symlink(to, filepath.Join(base, "parent")); err != nil { + t.Fatal(err) + } + return target +} + +func permOf(t *testing.T, path string) os.FileMode { + t.Helper() + info, err := os.Lstat(path) + if err != nil { + t.Fatal(err) + } + return info.Mode().Perm() +} + +// The acceptance of issue 358: a parent swapped for a link after an apply leaves the link's target untouched, and +// the refusal names the link and who could have made it. +func TestAParentSwappedForALinkLeavesItsTargetUntouched(t *testing.T) { + base, r, previous := aMeshDirectory(t) + target := swapParentForLink(t, base, filepath.Join(base, "elsewhere")) + + _, err := applyDirectory(r, previous, true) + var refusal *PlacementRefusedError + if !errors.As(err, &refusal) { + t.Fatalf("the directory reached through a link was not refused: %v", err) + } + if !strings.Contains(err.Error(), filepath.Join(base, "parent")) || !strings.Contains(err.Error(), "could replace") { + t.Errorf("the refusal does not name the link and who could replace it: %v", err) + } + if got := permOf(t, target); got != 0o755 { + t.Errorf("the link's target was given mode %o (was 755): root's chmod followed the link", got) + } +}