From b8e04acfaf22bf6498f099ad48269ad1c08c1e16 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 21:05:52 +0200 Subject: [PATCH] Let nfs-server's tests see their folders as the library owner's, so the root refusal is tested and not tripped when the build seat runs them as root --- modules/nfs-server/cmd/nfs-server/machine.go | 17 +++++++- modules/nfs-server/cmd/nfs-server/nfs_test.go | 19 ++++++++- .../nfs-server/cmd/nfs-server/review_test.go | 40 +++++++++++++++++++ 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/modules/nfs-server/cmd/nfs-server/machine.go b/modules/nfs-server/cmd/nfs-server/machine.go index 47f70e4e..857a50ed 100644 --- a/modules/nfs-server/cmd/nfs-server/machine.go +++ b/modules/nfs-server/cmd/nfs-server/machine.go @@ -102,6 +102,9 @@ type Machine struct { // WalkLimit bounds the look for a backup repository below a share. WalkLimit int owners map[string]Owner + // ownerOf reads a file's owner from its inode; nil is the inode as the kernel says it. Tests set it + // so the folders they make stand for a non-root owner, whoever runs them. + ownerOf func(info os.FileInfo) (uid, gid int) // clean is when a look below each real path last found no backup repository (LookKept). clean map[string]time.Time cleanMu sync.Mutex @@ -136,9 +139,11 @@ func (m *Machine) Stat(path string) (Owner, error) { return Owner{}, err } o := Owner{Dir: info.IsDir()} - if st, ok := info.Sys().(*syscall.Stat_t); ok { - o.UID, o.GID = int(st.Uid), int(st.Gid) + if m.ownerOf != nil { + o.UID, o.GID = m.ownerOf(info) + return o, nil } + o.UID, o.GID = inodeOwner(info) return o, nil } @@ -290,3 +295,11 @@ func (m *Machine) ServiceState() string { } return "unknown" } + +// inodeOwner is a file's owner as its inode says. +func inodeOwner(info os.FileInfo) (uid, gid int) { + if st, ok := info.Sys().(*syscall.Stat_t); ok { + return int(st.Uid), int(st.Gid) + } + return -1, -1 +} diff --git a/modules/nfs-server/cmd/nfs-server/nfs_test.go b/modules/nfs-server/cmd/nfs-server/nfs_test.go index b81f81af..dbc45970 100644 --- a/modules/nfs-server/cmd/nfs-server/nfs_test.go +++ b/modules/nfs-server/cmd/nfs-server/nfs_test.go @@ -32,7 +32,7 @@ func newFake(t *testing.T) *fake { } func (f *fake) machine() *Machine { - return &Machine{Root: f.root, UID: f.uid, owners: f.owners, Now: func() time.Time { return f.now }, Run: func(_ context.Context, name string, args ...string) Ran { + return &Machine{Root: f.root, UID: f.uid, owners: f.owners, ownerOf: asTheLibrarysOwner, Now: func() time.Time { return f.now }, Run: func(_ context.Context, name string, args ...string) Ran { line := strings.Join(append([]string{name}, args...), " ") f.ran = append(f.ran, line) if (line == "exportfs -ra" || line == "sudo -n exportfs -ra") && f.onExportfs != nil { @@ -121,7 +121,7 @@ func TestAnExportLineSquashesEveryClientToTheFoldersOwnerToTheMeshRangeOnly(t *t if len(s.Shares) != 2 || len(s.Refused) != 0 { t.Fatalf("status: %+v", s) } - uid, gid := os.Getuid(), os.Getgid() + uid, gid := 1001, 2000 // the owner the tests' folders stand for (asTheLibrarysOwner) text := f.read(ExportFile) for _, want := range []string{ fmt.Sprintf("/srv/library %s(rw,sync,secure,no_subtree_check,all_squash,anonuid=%d,anongid=%d)", wsAddress, uid, gid), @@ -432,3 +432,18 @@ func TestAnActNeedingRootGoesThroughSudoOnlyWhenNotRoot(t *testing.T) { t.Errorf("%v", err) } } + +// asTheLibrarysOwner is how the tests see the folders they make: owned by the library's owner (1001:2000), +// not by whoever runs them. The build seat runs the suite as root, where every temporary folder is root's +// and the refusal of a root-owned share would fire first; on a person's machine they are that person's. +// A folder the test runner does not own keeps its real owner, so a real root-owned folder is still root's. +func asTheLibrarysOwner(info os.FileInfo) (uid, gid int) { + uid, gid = inodeOwner(info) + if uid == os.Getuid() { + uid = 1001 + } + if gid == os.Getgid() { + gid = 2000 + } + return uid, gid +} diff --git a/modules/nfs-server/cmd/nfs-server/review_test.go b/modules/nfs-server/cmd/nfs-server/review_test.go index 266bf00b..c0893d11 100644 --- a/modules/nfs-server/cmd/nfs-server/review_test.go +++ b/modules/nfs-server/cmd/nfs-server/review_test.go @@ -75,6 +75,46 @@ func TestAShareWhoseOwnerIsRootIsRefused(t *testing.T) { } } +// Finding 1, on a real inode: a folder root owns, read as the kernel says, is refused. Run as root (the build +// seat, or `unshare -r`), the test's own temporary folder is root's; run as anyone else, a system folder is. +func TestARealRootOwnedFolderIsRefused(t *testing.T) { + f := newFake(t) + f.mountinfo("/") + share := "/srv/mine" + f.dir(share) + root := f.root + if os.Getuid() != 0 { + info, err := os.Stat("/usr") + if err != nil { + t.Skip("no /usr to stand for a root-owned folder:", err) + } + if uid, _ := inodeOwner(info); uid != 0 { + t.Skipf("/usr is not root's here (uid %d); run as root or under unshare -r", uid) + } + root, share = "/", "/usr" + } + f.file(ConfigFile, grantsConfig("system="+share, "system=ws:rw")) + f.roster("ws 10.0.0.3") + f.wants(map[string][]Want{"ws": {{Share: "system", Access: "rw"}}}) + m := f.machine() + m.ownerOf = nil // the inode as the kernel says it + if root == "/" { + // Judge only reads: the folder's real path, its inode and the mount table. Nothing is written. + sys := &Machine{Root: "/", Run: func(context.Context, string, ...string) Ran { return Ran{} }} + _, refused, _ := sys.Judge(Config{Shares: []Share{{Name: "system", Path: share, Access: "rw"}}, + Grants: map[string]map[string]string{"system": {"ws": "rw"}}, Range: "10.0.0.0/24"}, + map[string]string{"ws": "10.0.0.3"}, map[string][]Want{"ws": {{Share: "system", Access: "rw"}}}) + if len(refused) != 1 || !strings.Contains(refused[0].Why, "owned by root") { + t.Fatalf("the system folder %s, root's on its inode, is not refused for being root's: %+v", share, refused) + } + return + } + s := m.Pass(context.Background(), nil) + if !strings.Contains(refusedWhy(s, "system"), "root") { + t.Errorf("a folder root owns on its inode is not refused: %+v", s) + } +} + // Finding 2: a symlink is judged where it points, and the export names the real folder. func TestASymlinkIsJudgedAndExportedAtItsRealPath(t *testing.T) { f := newFake(t)