From 2cffa33bcc3981c236269d881db4284a46d1b8b4 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 22:14:10 +0200 Subject: [PATCH] =?UTF-8?q?Keep=20a=20root=20without=20asking=20only=20whe?= =?UTF-8?q?n=20its=20data=20beneath=20is=20the=20library's=20already=20(hq?= =?UTF-8?q?=20ADR=200328=20=C2=A77)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A root in the group with setgid was kept without a warrant even when the data beneath it was not in the group, which §7 says the operator is asked about. A root is now kept without asking only when it is empty, or it and a bounded sample beneath it are all in the group with group write (and setgid on folders). On busybox, an account the mesh made is given its added groups with addgroup and delgroup; a change of its primary group is refused, saying why. --- internal/apply/library.go | 4 +- internal/apply/library_test.go | 92 +++++++++++++++++++++++------- internal/apply/walk_linux.go | 59 +++++++++++++++++++ internal/apply/walk_other.go | 2 + internal/system/identities.go | 58 +++++++++++++------ internal/system/identities_test.go | 38 ++++++++++++ 6 files changed, 212 insertions(+), 41 deletions(-) create mode 100644 internal/system/identities_test.go diff --git a/internal/apply/library.go b/internal/apply/library.go index db0ec1e..9e44753 100644 --- a/internal/apply/library.go +++ b/internal/apply/library.go @@ -193,7 +193,9 @@ func assessLibrary(ctx context.Context, l declaration.Library, rec store.Library switch { case facts.Empty: v.kind, v.how = libraryKeep, store.KeptEmpty - case int(facts.GID) == l.GID && facts.Mode&0o2070 == 0o2070: + case int(facts.GID) == l.GID && facts.Mode&0o2070 == 0o2070 && sampleInShape(l.Root, l.GID): + // The root and a bounded sample beneath it already in the group with group write and setgid: data that + // is the library's already, kept without asking. One entry of the sample out of shape asks (§7). v.kind, v.how = libraryKeep, store.KeptInShape default: // What is there holds data the mesh never took — a disk mounted over a root kept while it was empty below diff --git a/internal/apply/library_test.go b/internal/apply/library_test.go index 46ab9a1..15bc383 100644 --- a/internal/apply/library_test.go +++ b/internal/apply/library_test.go @@ -117,54 +117,53 @@ func statOf(t *testing.T, path string) unix.Stat_t { return st } -// Row 6: the root and every requested sub-path kept in the library's group with group rwx and setgid — those -// folders alone, never what is in them; owner, owner's and others' bits unchanged. +// Row 6: the root and every requested sub-path kept in the library's group with group rwx and setgid on every apply — +// those folders alone, never what is in them; owner, owner's and others' bits unchanged. func TestALibraryIsKeptInItsGroup(t *testing.T) { gid := testGID(t) libraries(t, nil, nil) root := filepath.Join(t.TempDir(), "media") - movies := filepath.Join(root, "movies") - if err := os.MkdirAll(movies, 0o750); err != nil { + if err := os.Mkdir(root, 0o755); err != nil { t.Fatal(err) } - for _, p := range []string{root, movies} { - if err := os.Chmod(p, 0o705); err != nil { - t.Fatal(err) - } + d := libraryDecl(t, libJSON("media", root, gid, true, false, "movies")) + if _, _, err := applyLib(t, d, store.State{}); err != nil { + t.Fatal(err) + } + // Kept since it was empty; an app then narrows a requested folder and writes beneath it. + movies := filepath.Join(root, "movies") + if err := os.Chown(movies, -1, os.Getgid()); err != nil { + t.Fatal(err) + } + if err := unix.Chmod(movies, 0o705); err != nil { + t.Fatal(err) } film := filepath.Join(movies, "film") if err := os.WriteFile(film, []byte("data"), 0o640); err != nil { t.Fatal(err) } - // In shape at its root (the operator's own hand, or the mesh's before): kept without asking. - if err := os.Chown(root, -1, gid); err != nil { - t.Fatal(err) - } - if err := unix.Chmod(root, 0o2775); err != nil { - t.Fatal(err) - } + filmBefore := statOf(t, film) before := statOf(t, movies) - d := libraryDecl(t, libJSON("media", root, gid, true, false, "movies")) report, _, err := applyLib(t, d, store.State{}) if err != nil { t.Fatal(err) } - if o := outcomeOf(report, "library:media"); o.Action == "waiting" { - t.Fatalf("a root in shape waited: %+v", o) + if o := outcomeOf(report, "library:media"); o.Action != "updated" { + t.Fatalf("a kept library was not put back: %+v", o) } after := statOf(t, movies) - if int(after.Gid) != gid || after.Mode&0o2070 != 0o2070 { + if int(after.Gid) != gid || after.Mode&0o7777 != 0o2775 { t.Fatalf("a requested path is not kept in the group with group rwx and setgid: gid %d, mode %o", after.Gid, after.Mode&0o7777) } if after.Uid != before.Uid || after.Mode&0o707 != before.Mode&0o707 { t.Errorf("the owner or the owner's and others' bits changed: %o to %o", before.Mode&0o7777, after.Mode&0o7777) } - if f := statOf(t, film); int(f.Gid) == gid || f.Mode&0o7777 != 0o640 { + if f := statOf(t, film); f.Mode != filmBefore.Mode || f.Gid != filmBefore.Gid { t.Error("a file under a kept path was changed without the take act") } - if rec, _ := libraryRecord("media"); !rec.Kept || rec.How != store.KeptInShape { - t.Errorf("not recorded kept in shape: %+v", rec) + if rec, _ := libraryRecord("media"); !rec.Kept || rec.How != store.KeptEmpty { + t.Errorf("not recorded kept as empty: %+v", rec) } } @@ -641,3 +640,52 @@ func TestALibraryIsNeverTheMachinesOwnTree(t *testing.T) { } } } + +// §7: a root in shape with old data beneath it not in the group is asked about, never kept without a warrant; a root +// and sample all in shape are kept; once taken, never asked again on that filesystem. +func TestARootInShapeWithDataBelowNotInTheGroupWaits(t *testing.T) { + gid := testGID(t) + libraries(t, nil, nil) + root := filepath.Join(t.TempDir(), "media") + movies := filepath.Join(root, "movies") + if err := os.MkdirAll(movies, 0o755); err != nil { + t.Fatal(err) + } + old := filepath.Join(movies, "old film") + if err := os.WriteFile(old, []byte("data"), 0o644); err != nil { + t.Fatal(err) + } + for _, p := range []string{root, movies} { + if err := os.Chown(p, -1, gid); err != nil { + t.Fatal(err) + } + if err := unix.Chmod(p, 0o2775); err != nil { + t.Fatal(err) + } + } + d := libraryDecl(t, libJSON("media", root, gid, true, false, "movies")) + report, _, err := applyLib(t, d, store.State{}) + if err != nil { + t.Fatal(err) + } + if o := outcomeOf(report, "library:media"); o.Action != "waiting" || !strings.Contains(o.Detail, KeepWords("media")) { + t.Fatalf("a root in shape with an old file below not in the group was kept without asking: %+v", o) + } + // The old file in the group too: the sample is in shape, and the library is kept. + if err := os.Chown(old, -1, gid); err != nil { + t.Fatal(err) + } + if err := unix.Chmod(old, 0o664); err != nil { + t.Fatal(err) + } + if report, _, _ = applyLib(t, d, store.State{}); outcomeOf(report, "library:media").Action == "waiting" { + t.Fatal("a root and sample in shape were not kept") + } + // Once kept, a file an app later leaves out of the group asks nothing again on that filesystem. + if err := os.WriteFile(filepath.Join(movies, "new"), nil, 0o600); err != nil { + t.Fatal(err) + } + if report, _, _ = applyLib(t, d, store.State{}); outcomeOf(report, "library:media").Action == "waiting" { + t.Fatal("a kept library asked again") + } +} diff --git a/internal/apply/walk_linux.go b/internal/apply/walk_linux.go index 1e8f5c7..b96d534 100644 --- a/internal/apply/walk_linux.go +++ b/internal/apply/walk_linux.go @@ -448,3 +448,62 @@ var MountUUID = func(point string) string { } return "" } + +// SampleLimits bound the sample that judges whether a root's data is already the library's: so many entries a +// folder, so many levels down, for so long. A sample that runs out of time is not in shape: it asks. +var SampleLimits = struct { + PerFolder, Levels int + Time time.Duration +}{PerFolder: 20, Levels: 3, Time: 2 * time.Second} + +// sampleInShape is whether the first entries of each folder under root, a few levels down, are all in the group gid +// with group read and write, and folders with group enter and setgid (ADR 0328 §7). Reads only, by descriptor: no +// link followed, no other filesystem entered. A file with more than one name is not judged (the take act leaves it). +func sampleInShape(root string, gid int) bool { + _, fd, st, err := openRoot(root, WalkLimits{Entries: 1, Time: time.Minute}, machineStats, nil) + if err != nil { + return false + } + deadline := time.Now().Add(SampleLimits.Time) + return sampleFolder(fd, uint64(st.Dev), gid, SampleLimits.Levels, deadline) +} + +// sampleFolder judges the folder open on fd's first entries, and the folders among them levels further. It closes fd. +func sampleFolder(fd int, dev uint64, gid, levels int, deadline time.Time) bool { + f := os.NewFile(uintptr(fd), "") + defer f.Close() + names, err := f.Readdirnames(SampleLimits.PerFolder) + if err != nil && !errors.Is(err, io.EOF) { + return false + } + for _, name := range names { + if time.Now().After(deadline) { + return false + } + var st unix.Stat_t + if err := unix.Fstatat(fd, name, &st, unix.AT_SYMLINK_NOFOLLOW); err != nil { + return false + } + if uint64(st.Dev) != dev { + continue + } + mode := uint32(st.Mode) + switch mode & unix.S_IFMT { + case unix.S_IFREG: + if st.Nlink == 1 && (int(st.Gid) != gid || mode&0o060 != 0o060) { + return false + } + case unix.S_IFDIR: + if int(st.Gid) != gid || mode&0o2070 != 0o2070 { + return false + } + if levels > 1 { + next, err := unix.Openat(fd, name, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0) + if err != nil || !sampleFolder(next, dev, gid, levels-1, deadline) { + return false + } + } + } + } + return true +} diff --git a/internal/apply/walk_other.go b/internal/apply/walk_other.go index ecd6d51..df8b725 100644 --- a/internal/apply/walk_other.go +++ b/internal/apply/walk_other.go @@ -35,3 +35,5 @@ var MountFsid = func(string) string { return "" } // MountUUID reads no UUID where a library is not kept. var MountUUID = func(string) string { return "" } + +func sampleInShape(string, int) bool { return false } diff --git a/internal/system/identities.go b/internal/system/identities.go index 3e3a363..7c13b67 100644 --- a/internal/system/identities.go +++ b/internal/system/identities.go @@ -88,25 +88,47 @@ func (alpine) CreateSystemAccount(ctx context.Context, run Runner, name string, return nil } -// ModifyAccount is not one busybox can do: it has no usermod to change an account's primary group, and removing -// an added group is a hand edit of /etc/group. Refused, saying so: the account is left as it is. -func (alpine) ModifyAccount(context.Context, Runner, string, int, []string) error { - return fmt.Errorf("this system cannot change an account's groups once it is made (busybox has no usermod): %w", - ErrUnsupported) -} - -// RemoveAccount removes an account with busybox's deluser. -func (alpine) RemoveAccount(ctx context.Context, run Runner, name string) error { - if _, err := run(ctx, "deluser", name); err != nil { - return fmt.Errorf("cannot remove the account %q: %w", name, err) +// ModifyAccount gives an account the mesh made exactly groups as its added groups, with busybox's addgroup and +// delgroup. Its primary group busybox cannot change (it has no usermod, and the mesh edits no passwd by hand): +// an account whose primary group must change is refused, saying so, and left as it is. +func (alpine) ModifyAccount(ctx context.Context, run Runner, name string, gid int, groups []string) error { + passwd, err := run(ctx, "getent", "passwd", name) + if err != nil { + return fmt.Errorf("cannot read the account %q: %w", name, err) } - return nil -} - -// RemoveGroup removes a group with busybox's delgroup. -func (alpine) RemoveGroup(ctx context.Context, run Runner, name string) error { - if _, err := run(ctx, "delgroup", name); err != nil { - return fmt.Errorf("cannot remove the group %q: %w", name, err) + fields := strings.Split(strings.TrimSpace(passwd), ":") + if len(fields) < 4 || fields[3] != strconv.Itoa(gid) { + return fmt.Errorf("this system cannot change the account %q's primary group to %d: busybox has no usermod, "+ + "and the mesh edits no passwd by hand: %w", name, gid, ErrUnsupported) + } + primary := "" + if g, err := run(ctx, "getent", "group", fields[3]); err == nil { + primary, _, _ = strings.Cut(strings.TrimSpace(g), ":") + } + out, err := run(ctx, "id", "-nG", name) + if err != nil { + return fmt.Errorf("cannot read the account %q's groups: %w", name, err) + } + have, want := map[string]bool{}, map[string]bool{} + for _, g := range strings.Fields(out) { + if g != primary && g != name { + have[g] = true + } + } + for _, g := range groups { + want[g] = true + if !have[g] { + if _, err := run(ctx, "addgroup", name, g); err != nil { + return fmt.Errorf("cannot put the account %q in %q: %w", name, g, err) + } + } + } + for g := range have { + if !want[g] { + if _, err := run(ctx, "delgroup", name, g); err != nil { + return fmt.Errorf("cannot take the account %q out of %q: %w", name, g, err) + } + } } return nil } diff --git a/internal/system/identities_test.go b/internal/system/identities_test.go new file mode 100644 index 0000000..4fe84b1 --- /dev/null +++ b/internal/system/identities_test.go @@ -0,0 +1,38 @@ +package system + +import ( + "context" + "errors" + "strings" + "testing" +) + +// busybox has no usermod (novox/hq ADR 0328 §4): an account the mesh made is given its added groups with addgroup +// and delgroup, and a change of its primary group is refused, saying why. +func TestBusyboxGivesAnAccountItsAddedGroupsAndRefusesAnotherPrimary(t *testing.T) { + var ran []string + run := func(_ context.Context, name string, args ...string) (string, error) { + ran = append(ran, name+" "+strings.Join(args, " ")) + switch { + case name == "getent" && args[0] == "passwd": + return "mesh-radarr:x:61001:62000::/:/sbin/nologin\n", nil + case name == "getent" && args[0] == "group": + return "mesh-media:x:62000:\n", nil + case name == "id": + return "mesh-media mesh-old\n", nil + } + return "", nil + } + if err := (alpine{}).ModifyAccount(context.Background(), run, "mesh-radarr", 62000, []string{"mesh-books"}); err != nil { + t.Fatal(err) + } + got := strings.Join(ran, "\n") + if !strings.Contains(got, "addgroup mesh-radarr mesh-books") || !strings.Contains(got, "delgroup mesh-radarr mesh-old") || + strings.Contains(got, "delgroup mesh-radarr mesh-media") { + t.Fatalf("not given exactly its added groups:\n%s", got) + } + err := (alpine{}).ModifyAccount(context.Background(), run, "mesh-radarr", 62005, nil) + if !errors.Is(err, ErrUnsupported) || !strings.Contains(err.Error(), "primary group") { + t.Fatalf("a change of primary group was not refused saying why: %v", err) + } +}