Merge pull request 'Count the home of an account a declaration makes for its own .ssh, never taking over another account's home (issue 1542)' (#125) from fix/1542-declared-account-is-a-home into main

internal/apply/declared_home_test.go

@@ -0,0 +1,158 @@
+package apply
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"git.novox.be/novox/mesh-sdk/go/declaration"
+)
+
+// A module that declares its own system account and a .ssh in that account's home is placed (novox/hq issue 1542):
+// the account is not in the user database yet when the declaration is judged, and its home is under /var/lib.
+func declaredHomeFixture(t *testing.T) (home string, user *declaration.User) {
+	t.Helper()
+	dir := t.TempDir()
+	passwd := filepath.Join(dir, "passwd")
+	if err := os.WriteFile(passwd, []byte("root:x:0:0::/root:/bin/sh\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	before := passwdFile
+	passwdFile = passwd
+	t.Cleanup(func() { passwdFile = before })
+	home = filepath.Join(dir, "lib", "mesh-build-go")
+	return home, &declaration.User{ID: "mesh-build-go.account", Type: declaration.TypeUser, Name: "mesh-build-go",
+		Shell: "/usr/bin/nologin", Home: home}
+}
+
+// 1542/AC1: the .ssh of a declared account, owned by it, is refused by nothing.
+func TestIssue1542ADeclaredAccountsSSHIsPlaced(t *testing.T) {
+	home, user := declaredHomeFixture(t)
+	ssh := &declaration.Directory{ID: "mesh-build-go.ssh-dir", Type: declaration.TypeDirectory, Path: home + "/.ssh",
+		Mode: "0700", Owner: "mesh-build-go"}
+	refused := refusedPlaces([]declaration.Resource{user, ssh})
+	if len(refused) != 0 {
+		t.Fatalf("a declared account's own .ssh was refused: %v", refused)
+	}
+	if err := (inputs{refused: refused}).refused[filepath.Clean(ssh.Path)]; err != nil {
+		t.Fatalf("the apply-time check refuses it: %v", err)
+	}
+	if line := JudgeDirectories(&declaration.Declaration{Resources: []declaration.Resource{user, ssh}}); strings.Contains(line, "refused already by the placement guard: ") {
+		t.Fatalf("the read-only judgement refuses it: %s", line)
+	}
+}
+
+// 1542/AC2: another account's directory below the declared home is refused as below any home, and a .ssh below
+// no home of the machine or the declaration is refused with today's words.
+func TestIssue1542OnlyTheDeclaredAccountsOwnIsPlaced(t *testing.T) {
+	home, user := declaredHomeFixture(t)
+	other := &declaration.Directory{ID: "notes.ssh-dir", Type: declaration.TypeDirectory, Path: home + "/.ssh",
+		Mode: "0700", Owner: "intruder"}
+	refused := refusedPlaces([]declaration.Resource{user, other})
+	err := refused[filepath.Clean(other.Path)]
+	if err == nil || !strings.Contains(err.Error(), "below mesh-build-go's home and declared intruder's") {
+		t.Fatalf("another account's directory below a declared home: %v", err)
+	}
+	stray := &declaration.Directory{ID: "mesh-build-go.ssh-dir", Type: declaration.TypeDirectory,
+		Path: filepath.Join(filepath.Dir(home), "elsewhere", ".ssh"), Mode: "0700", Owner: "mesh-build-go"}
+	refused = refusedPlaces([]declaration.Resource{user, stray})
+	err = refused[filepath.Clean(stray.Path)]
+	if err == nil || !strings.Contains(err.Error(), "a .ssh directory is placed only below its account's home, as that account's") {
+		t.Fatalf("a .ssh below no home: %v", err)
+	}
+	// Not a .ssh: a root-owned directory below the declared home is placed exactly as on main.
+	data := &declaration.Directory{ID: "mesh-build-go.data", Type: declaration.TypeDirectory, Path: home + "/data",
+		Mode: "0755", Owner: "root"}
+	if refused := refusedPlaces([]declaration.Resource{user, data}); len(refused) != 0 {
+		t.Fatalf("a root-owned directory below a declared home, not a .ssh, was refused: %v", refused)
+	}
+	// And without the account declared, the same .ssh is refused as it was.
+	own := &declaration.Directory{ID: "mesh-build-go.ssh-dir", Type: declaration.TypeDirectory, Path: home + "/.ssh",
+		Mode: "0700", Owner: "mesh-build-go"}
+	if refusedPlaces([]declaration.Resource{own})[filepath.Clean(own.Path)] == nil {
+		t.Fatal("a .ssh below an account nobody makes was placed")
+	}
+}
+
+// 1542/AC3: a declared account is no home to anyone but the directory placement guard.
+func TestIssue1542DeclaredAccountsAreNotHomesToTheOtherGuards(t *testing.T) {
+	home, user := declaredHomeFixture(t)
+	_ = refusedPlaces([]declaration.Resource{user})
+	if _, ok := accountsOfHomes()[home]; ok {
+		t.Fatal("a declared account became a home of the machine")
+	}
+	if homeAbove(home+"/.ssh") != "" {
+		t.Fatal("a declared account became a home for the links and the move-out")
+	}
+}
+
+// A declared account is added as a home and never takes one over (reviewer, issue 1542): the person's own .ssh
+// stays refused to every other account, whatever a declaration says about a user.
+func TestIssue1542ADeclaredAccountTakesNoExistingHome(t *testing.T) {
+	dir := t.TempDir()
+	passwd := filepath.Join(dir, "passwd")
+	body := "root:x:0:0::/root:/bin/sh\njochen:x:1000:1000::/home/jochen:/bin/sh\n" +
+		"svc:x:998:998::/var/lib/svc:/usr/bin/nologin\n"
+	if err := os.WriteFile(passwd, []byte(body), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	before := passwdFile
+	passwdFile = passwd
+	t.Cleanup(func() { passwdFile = before })
+	newHome := filepath.Join(dir, "lib", "mesh-build-go")
+	ssh := func(path, owner string) *declaration.Directory {
+		return &declaration.Directory{ID: "x.ssh", Type: declaration.TypeDirectory, Path: path, Mode: "0700", Owner: owner}
+	}
+	user := func(name, home string) *declaration.User {
+		return &declaration.User{ID: "x.user", Type: declaration.TypeUser, Name: name, Home: home}
+	}
+	// A declared user over a person's home, whoever it is named for.
+	for _, name := range []string{"evil", "postgres", "root"} {
+		d := ssh("/home/jochen/.ssh", name)
+		if refusedPlaces([]declaration.Resource{user(name, "/home/jochen"), d})[d.Path] == nil {
+			t.Errorf("%s, declared over jochen's home, was given jochen's .ssh", name)
+		}
+	}
+	// A declared home below a person's home.
+	d := ssh("/home/jochen/sub/.ssh", "evil")
+	if refusedPlaces([]declaration.Resource{user("evil", "/home/jochen/sub"), d})[d.Path] == nil {
+		t.Error("a home declared below jochen's home was counted")
+	}
+	// A name the machine has already, declared with another home: that is not its home.
+	d = ssh(newHome+"/e/.ssh", "svc")
+	if refusedPlaces([]declaration.Resource{user("svc", newHome+"/e"), d})[d.Path] == nil {
+		t.Error("an existing account's declared home was counted")
+	}
+	// A number is no name: chown -1 leaves the owner as it was.
+	for _, owner := range []string{"-1", "-1:0", "0", "1001"} {
+		d := ssh(newHome+"/.ssh", owner)
+		if refusedPlaces([]declaration.Resource{user("mesh-build-go", newHome), d})[d.Path] == nil {
+			t.Errorf("owner %q was placed below a declared home", owner)
+		}
+	}
+	// And the account that is new, by name, still is.
+	d = ssh(newHome+"/.ssh", "mesh-build-go")
+	if got := refusedPlaces([]declaration.Resource{user("mesh-build-go", newHome), d}); len(got) != 0 {
+		t.Errorf("a new account's own .ssh was refused: %v", got)
+	}
+}
+
+// The account the mesh made, as declared, is judged the same at the next apply (issue 1542): once it is in the
+// user database with exactly the declared home, its .ssh is still placed.
+func TestIssue1542TheAccountTheMeshMadeIsStillPlacedAtTheNextApply(t *testing.T) {
+	home, user := declaredHomeFixture(t)
+	ssh := &declaration.Directory{ID: "mesh-build-go.ssh-dir", Type: declaration.TypeDirectory, Path: home + "/.ssh",
+		Mode: "0700", Owner: "mesh-build-go"}
+	resources := []declaration.Resource{user, ssh}
+	if got := refusedPlaces(resources); len(got) != 0 {
+		t.Fatalf("first apply, the account not yet made: %v", got)
+	}
+	body := "root:x:0:0::/root:/bin/sh\nmesh-build-go:x:998:998::" + home + ":/usr/bin/nologin\n"
+	if err := os.WriteFile(passwdFile, []byte(body), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if got := refusedPlaces(resources); len(got) != 0 {
+		t.Fatalf("second apply, the account made as declared: %v", got)
+	}
+}

internal/apply/mesh_dirs.go

@@ -94,13 +94,14 @@ func groupNamed(gid uint32) string {
 func JudgeDirectories(d *declaration.Declaration) string {
 	var lines []string
 	var placed, refused, guarded, homed int
+	declared := declaredHomes(d.Resources)
 	for _, r := range d.Resources {
 		dir, ok := r.(*declaration.Directory)
 		if !ok {
 			continue
 		}
 		path := filepath.Clean(dir.Path)
-		if err := refusePath(path, placingDirectory, moduleOfID(dir.ID), dir.Owner); err != nil {
+		if err := refusePathWith(path, placingDirectory, moduleOfID(dir.ID), dir.Owner, declared); err != nil {
 			guarded++
 			lines = append(lines, path+": refused already by the placement guard: "+err.Error())
 			continue

internal/apply/placement_guard.go

@@ -384,6 +384,9 @@ func ownedByAccount(owner string, a homeAccount) bool {
 	if owner == a.Name {
 		return true
 	}
+	if a.UID < 0 {
+		return false // an account the declaration makes has no uid yet: by name alone, never by a number
+	}
 	user, _, _ := strings.Cut(owner, ":")
 	if uid, err := strconv.Atoi(user); err == nil {
 		return uid == a.UID
@@ -413,12 +416,91 @@ const (
 // refusePath says why a path is not placed or mounted; nil when it may be. module is the resource's module,
 // owner a directory's declared owner.
 func refusePath(path string, kind placing, module, owner string) error {
+	return refusePathWith(path, kind, module, owner, nil)
+}
+
+// declaredHomes is the home of each account the declaration itself makes (a user resource with a home), by the
+// account's name; the account is not in the user database yet when the declaration is judged, and the engine makes
+// it where it is declared, before the directory that is owned by it is reached (novox/hq issue 1542). Its uid is
+// not known, so a directory is owned by it by name alone. Only for the directory placement guard; no other
+// reader of accountsOfHomes sees these.
+func declaredHomes(resources []declaration.Resource) map[string]homeAccount {
+	var accounts []passwdEntry
+	var out map[string]homeAccount
+	for _, r := range resources {
+		u, ok := r.(*declaration.User)
+		if !ok || u.Name == "" {
+			continue
+		}
+		home := filepath.Clean(u.Home)
+		if u.Home == "" || !filepath.IsAbs(home) || home == "/" {
+			continue
+		}
+		if accounts == nil {
+			accounts = passwdEntries()
+		}
+		// Only ever adds a home. An account the machine has keeps the home it has (applyUser never moves it), so
+		// a declared home of one is not its home, unless it is the very home it has: the account an earlier apply
+		// made, as declared, which must be judged the same at every later apply. And a home at or below another
+		// account's, or at or above a protected root, is not a new one.
+		taken := false
+		for _, e := range accounts {
+			if e.name == u.Name && e.home != home {
+				taken = true
+			}
+			if e.name != u.Name && e.home != "" && atOrBelow(home, e.home) {
+				taken = true
+			}
+		}
+		for _, root := range protectedRoots {
+			taken = taken || home == root || below(root, home)
+		}
+		if taken {
+			continue
+		}
+		if out == nil {
+			out = map[string]homeAccount{}
+		}
+		out[home] = homeAccount{Name: u.Name, UID: -1}
+	}
+	return out
+}
+
+// passwdEntry is an account of the user database and its home, whatever its uid.
+type passwdEntry struct{ name, home string }
+
+// passwdEntries is every account of the user database with its home (empty and root homes as "").
+func passwdEntries() []passwdEntry {
+	out := []passwdEntry{}
+	f, err := os.Open(passwdFile)
+	if err != nil {
+		return out
+	}
+	defer f.Close()
+	sc := bufio.NewScanner(f)
+	for sc.Scan() {
+		fields := strings.Split(sc.Text(), ":")
+		if len(fields) < 6 {
+			continue
+		}
+		home := filepath.Clean(fields[5])
+		if home == "." || home == "/" || home == "/nonexistent" {
+			home = ""
+		}
+		out = append(out, passwdEntry{fields[0], home})
+	}
+	return out
+}
+
+// refusePathWith is refusePath with the homes of the accounts the declaration makes (declaredHomes) counted
+// among the machine's.
+func refusePathWith(path string, kind placing, module, owner string, declared map[string]homeAccount) error {
 	clean := filepath.Clean(path)
 	if !filepath.IsAbs(clean) {
 		return nil // the declaration refuses a relative path already; a named volume is not a path
 	}
 	for _, p := range []string{clean, resolved(clean)} {
-		if err := refuseOne(p, kind, module, owner); err != nil {
+		if err := refuseOne(p, kind, module, owner, declared); err != nil {
 			if p != clean {
 				err.Why = fmt.Sprintf("through a link, it is %s, and %s", p, err.Why)
 				err.Path = clean
@@ -429,7 +511,7 @@ func refusePath(path string, kind placing, module, owner string) error {
 	return nil
 }
 
-func refuseOne(path string, kind placing, module, owner string) *PlacementRefusedError {
+func refuseOne(path string, kind placing, module, owner string, declared map[string]homeAccount) *PlacementRefusedError {
 	for _, root := range protectedRoots {
 		if path == root || below(root, path) {
 			return &PlacementRefusedError{Path: path, Why: root + " is one of the machine's own directories, " +
@@ -482,6 +564,18 @@ func refuseOne(path string, kind placing, module, owner string) *PlacementRefuse
 		return nil
 	}
 	homes := accountsOfHomes()
+	if ssh && len(declared) > 0 {
+		merged := map[string]homeAccount{}
+		for home, a := range homes {
+			merged[home] = a
+		}
+		for home, a := range declared {
+			if _, held := merged[home]; !held {
+				merged[home] = a
+			}
+		}
+		homes = merged
+	}
 	var deepest string
 	for home := range homes {
 		if below(path, home) && len(home) > len(deepest) {
@@ -518,11 +612,12 @@ func moduleOfID(id string) string {
 // a file into it, because nothing judged the file's path.
 func refusedPlaces(resources []declaration.Resource) map[string]error {
 	out := map[string]error{}
+	declared := declaredHomes(resources)
 	for _, r := range resources {
 		var err error
 		switch res := r.(type) {
 		case *declaration.Directory:
-			err = refusePath(res.Path, placingDirectory, moduleOfID(res.ID), res.Owner)
+			err = refusePathWith(res.Path, placingDirectory, moduleOfID(res.ID), res.Owner, declared)
 			// 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