From c74cf16b755ee6f5315f60c762daee3fa3b58cf3 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 21:19:32 +0200 Subject: [PATCH] Judge an opened file's kind and links below a home, and more ways to root A hard link swapped in for ~/.claude would have had root chown another account's file; fstat on the descriptor now refuses a second link, a fifo or an unexpected kind before anything is changed (hq ADR 0266, the re-review). The judge also finds polkit rules for every account, a runtime's API on TCP, setgid-to-root programs whoever owns them, setuid programs on every suid filesystem, and unprotected links; the rest is listed as not judged. --- internal/accounts/root_test.go | 26 ++++- internal/accounts/ways.go | 175 +++++++++++++++++++++++++++++- internal/accounts/ways_test.go | 56 ++++++++++ internal/apply/apply.go | 2 +- internal/apply/home_links.go | 22 ++++ internal/apply/home_links_test.go | 50 +++++++++ internal/apply/homes_linux.go | 73 ++++++++++++- internal/apply/homes_other.go | 6 + 8 files changed, 396 insertions(+), 14 deletions(-) diff --git a/internal/accounts/root_test.go b/internal/accounts/root_test.go index 07f4713..8e9a040 100644 --- a/internal/accounts/root_test.go +++ b/internal/accounts/root_test.go @@ -35,15 +35,33 @@ type agentMachine struct { setuid string findErr error packaged map[string]bool + // proc is /proc's files, aSafeProc unless a test changes them; findArgs sees find's arguments. + proc map[string]string + findArgs func([]string) } func (m *agentMachine) readFile(path string) ([]byte, error) { if t, ok := m.texts[path]; ok { return []byte(t), nil } + if t, ok := m.proc[path]; ok { + return []byte(t), nil + } return nil, fs.ErrNotExist } +// aSafeProc is what a machine with nothing to say has in /proc: links protected, nothing on the runtimes' +// ports, one root filesystem. +func aSafeProc() map[string]string { + return map[string]string{ + "/proc/sys/fs/protected_hardlinks": "1\n", + "/proc/sys/fs/protected_symlinks": "1\n", + "/proc/net/tcp": " sl local_address rem_address st tx_queue rx_queue tr tm->when retrnsmt uid timeout inode\n" + + " 0: 0100007F:1092 00000000:0000 0A 00000000:00000000 00:00000000 00000000 0 0 1 1\n", + "/proc/mounts": "/dev/sda2 / ext4 rw,relatime 0 0\nproc /proc proc rw,nosuid 0 0\ntmpfs /tmp tmpfs rw,nosuid 0 0\n", + } +} + func (m *agentMachine) readDir(path string) ([]fs.DirEntry, error) { names, ok := m.dirs[path] if !ok { @@ -85,6 +103,9 @@ func (m *agentMachine) run(_ context.Context, name string, args ...string) (stri case line == "sudo -l -U agent": return m.sudo, m.sudoErr case name == "find": + if m.findArgs != nil { + m.findArgs(args) + } return m.setuid, m.findErr case name == "pacman" && len(args) == 2 && args[0] == "-Qqo": if m.packaged[args[1]] { @@ -109,7 +130,8 @@ var agent = Account{Module: "claude-code", ID: "claude-code.agent", Name: "agent func clean() *agentMachine { return &agentMachine{uid: "1600", groups: "agent", gids: "1600", sudo: notAllowed, - files: map[string]FileMode{"/var/lib/mesh/node-tools/broker": {UID: 1500, GID: 1500, Perm: 0o600}}} + files: map[string]FileMode{"/var/lib/mesh/node-tools/broker": {UID: 1500, GID: 1500, Perm: 0o600}}, + proc: aSafeProc()} } func lookAgent(t *testing.T, m *agentMachine) Verdict { @@ -121,7 +143,7 @@ func lookAgent(t *testing.T, m *agentMachine) Verdict { t.Fatalf("the statement: %+v", st) } for _, a := range m.asked { - if !strings.HasPrefix(a, "id ") && a != "sudo -l -U agent" && !strings.HasPrefix(a, "find / -xdev") && + if !strings.HasPrefix(a, "id ") && a != "sudo -l -U agent" && !strings.HasPrefix(a, "find / ") && !strings.HasPrefix(a, "pacman -Qqo ") { t.Errorf("the judge asked something that is not a read: %q", a) } diff --git a/internal/accounts/ways.go b/internal/accounts/ways.go index 062e531..7fb25e1 100644 --- a/internal/accounts/ways.go +++ b/internal/accounts/ways.go @@ -30,10 +30,18 @@ var Judged = []string{ "any doas rule permitting it or one of its groups (/etc/doas.conf, /etc/opendoas.conf)", "any polkit rule naming it or one of its groups (/etc/polkit-1/rules.d, /usr/share/polkit-1/rules.d, " + "/etc/polkit-1/localauthority), which is how pkexec and systemd's own actions are granted", + "a polkit rule that grants every account: a .rules file that answers polkit.Result.YES and names no user " + + "and no group, or a .pkla whose Identity is unix-user:* or unix-group:* with a Result of yes", "write access to a container runtime's socket (docker, podman, containerd), by owner, group, other or " + "POSIX ACL", + "a container runtime's API listening on TCP port 2375 or 2376 (/proc/net/tcp, /proc/net/tcp6), which any " + + "local account reaches", "read access to a secret the mesh placed for another account, by owner, group, other or POSIX ACL", - "a setuid- or setgid-root program that no installed package owns, anywhere on the root filesystem", + "a program that no installed package owns and is setuid with owner root, or setgid with group root, " + + "whoever owns it, on every local filesystem mounted without nosuid (container and image layers, " + + "network and pseudo filesystems excepted)", + "hard links or symbolic links to other accounts' files left unprotected by the kernel " + + "(fs.protected_hardlinks or fs.protected_symlinks is 0)", } // NotJudged is what the judge does not look for: each is a way to root it would miss. @@ -46,6 +54,11 @@ var NotJudged = []string{ "file capabilities (setcap) on a program", "a setuid program a package installed that has a flaw of its own", "a terminal the account shares with a root process (TIOCSTI, sudo without use_pty)", + "a polkit rule that grants every account by logic the text does not show (a JavaScript condition " + + "other than a named user or group), or every account with an active local session", + "a container runtime's API on a TCP port other than 2375 and 2376, or on a unix socket not named here", + "a setuid program on a filesystem not judged: a container or image layer (overlay, squashfs), a network " + + "or FUSE filesystem", } // RuntimeSockets are the container runtimes' sockets: write access to one runs a container as root. @@ -189,6 +202,107 @@ func PolkitNames(text, account string, groups []string) bool { return false } +// PolkitGrantsEveryone says whether a polkit rule or authority file grants every account, naming none: a .rules +// file whose function answers polkit.Result.YES with no condition on a user or a group, or a .pkla whose +// Identity is every user or every group with a Result of yes. +func PolkitGrantsEveryone(path, text string) bool { + if strings.HasSuffix(path, ".pkla") { + for _, section := range strings.Split(text, "[") { + var every, yes bool + for _, line := range strings.Split(section, "\n") { + k, v, ok := strings.Cut(strings.TrimSpace(line), "=") + if !ok { + continue + } + v = strings.TrimSpace(v) + switch strings.TrimSpace(k) { + case "Identity": + for _, id := range strings.Split(v, ";") { + if id == "unix-user:*" || id == "unix-group:*" { + every = true + } + } + case "ResultAny", "ResultActive", "ResultInactive": + if v == "yes" { + yes = true + } + } + } + if every && yes { + return true + } + } + return false + } + if !strings.Contains(text, "polkit.Result.YES") { + return false + } + // A condition on a user or a group names whom it grants; one on an active local session grants only an + // account with a seat, which an agent started through sudo has not (listed under NotJudged). + for _, condition := range []string{".user", "isInGroup", "unix-user:", "unix-group:", ".active", ".local"} { + if strings.Contains(text, condition) { + return false + } + } + return true +} + +// RuntimeAPIPorts are the ports a container runtime's API listens on by convention: 2375 plain, 2376 TLS. +var RuntimeAPIPorts = []int{2375, 2376} + +// ListeningPorts is every local TCP port in the listening state, from /proc/net/tcp's or tcp6's text. +func ListeningPorts(text string) []int { + var out []int + for i, line := range strings.Split(text, "\n") { + f := strings.Fields(line) + if i == 0 || len(f) < 4 || f[3] != "0A" { + continue + } + _, port, ok := strings.Cut(f[1], ":") + if !ok { + continue + } + var n int + if _, err := fmt.Sscanf(port, "%X", &n); err == nil { + out = append(out, n) + } + } + return out +} + +// pseudoFS are filesystem types no setuid program is installed on, or that are a container's or an image's +// own layers: not searched. +var pseudoFS = map[string]bool{"proc": true, "sysfs": true, "devtmpfs": true, "devpts": true, "tmpfs": true, + "cgroup": true, "cgroup2": true, "securityfs": true, "pstore": true, "bpf": true, "debugfs": true, + "tracefs": true, "mqueue": true, "hugetlbfs": true, "configfs": true, "fusectl": true, "autofs": true, + "binfmt_misc": true, "efivarfs": true, "overlay": true, "squashfs": true, "nsfs": true, "ramfs": true, + "nfs": true, "nfs4": true, "cifs": true, "smb3": true, "9p": true, "virtiofs": true} + +// SuidMounts is every local filesystem a setuid program could run from: the mount points of /proc/mounts whose +// type is not pseudo, network or a container's layer, and that are not mounted nosuid. Root first. +func SuidMounts(text string) []string { + seen := map[string]bool{} + var out []string + for _, line := range strings.Split(text, "\n") { + f := strings.Fields(line) + if len(f) < 4 || pseudoFS[f[2]] || strings.HasPrefix(f[2], "fuse") { + continue + } + at := strings.ReplaceAll(strings.ReplaceAll(f[1], `\040`, " "), `\011`, "\t") + if strings.HasPrefix(at, "/var/lib/docker") || strings.HasPrefix(at, "/var/lib/containers") || + strings.HasPrefix(at, "/proc") || strings.HasPrefix(at, "/sys") { + continue + } + if contains(strings.Split(f[3], ","), "nosuid") || seen[at] { + continue + } + seen[at] = true + out = append(out, at) + } + sort.Slice(out, func(i, j int) bool { return out[i] == "/" || (out[j] != "/" && out[i] < out[j]) }) + return out +} + func contains(xs []string, s string) bool { for _, x := range xs { if x == s { @@ -224,12 +338,17 @@ func (c *SetuidCache) get(now time.Time, search func() ([]string, error)) ([]str // setuidSearch is every setuid- or setgid-root regular file on the root filesystem that no installed package // owns, found with find and asked of the package manager. Bounded: a search that does not finish is an // unanswered question, never "none". -func (e Exec) setuidSearch(ctx context.Context) ([]string, error) { +func (e Exec) setuidSearch(ctx context.Context, mounts []string) ([]string, error) { ctx, cancel := context.WithTimeout(ctx, 2*time.Minute) defer cancel() - out, err := e.Run(ctx, "find", "/", "-xdev", "(", "-path", "/proc", "-o", "-path", "/sys", "-o", + // Each mount point a starting point of its own, -xdev keeping each to its own filesystem: every local + // filesystem mounted without nosuid is searched once (the re-review of 2026-10-08), and a program setgid to + // root's group counts whoever owns it. + args := append(append([]string{}, mounts...), "-xdev", "(", "-path", "/proc", "-o", "-path", "/sys", "-o", "-path", "/var/lib/docker", "-o", "-path", "/var/lib/containers", ")", "-prune", "-o", - "-type", "f", "-user", "root", "-perm", "/6000", "-print") + "-type", "f", "(", "(", "-user", "root", "-perm", "-4000", ")", "-o", "(", "-group", "root", "-perm", + "-2000", ")", ")", "-print") + out, err := e.Run(ctx, "find", args...) if ctx.Err() != nil { return nil, fmt.Errorf("the search for setuid programs did not finish within two minutes") } @@ -312,7 +431,7 @@ func (e Exec) moreWays(ctx context.Context, account string, uid int, groups []st // polkit. for _, dir := range PolkitDirs { - var named []string + var named, everyone []string err := walkFiles(readDir, dir, func(path string) error { raw, err := read(path) if err != nil { @@ -320,6 +439,8 @@ func (e Exec) moreWays(ctx context.Context, account string, uid int, groups []st } if PolkitNames(string(raw), account, groups) { named = append(named, path) + } else if PolkitGrantsEveryone(path, string(raw)) { + everyone = append(everyone, path) } return nil }) @@ -329,6 +450,9 @@ func (e Exec) moreWays(ctx context.Context, account string, uid int, groups []st for _, p := range named { ways = append(ways, "a polkit rule names it or a group of it: "+p) } + for _, p := range everyone { + ways = append(ways, "a polkit rule grants every account: "+p) + } } // The container runtimes' sockets. @@ -384,13 +508,52 @@ func (e Exec) moreWays(ctx context.Context, account string, uid int, groups []st if e.Now != nil { now = e.Now } - unowned, err := e.Cache.get(now(), func() ([]string, error) { return e.setuidSearch(ctx) }) + mountsText, err := read("/proc/mounts") + if err != nil { + return nil, fmt.Errorf("the mounted filesystems could not be read: %w", err) + } + mounts := SuidMounts(string(mountsText)) + if len(mounts) == 0 { + return nil, errors.New("no filesystem a setuid program could run from was found in /proc/mounts") + } + unowned, err := e.Cache.get(now(), func() ([]string, error) { return e.setuidSearch(ctx, mounts) }) if err != nil { return nil, err } for _, p := range unowned { ways = append(ways, "a setuid-root program no package owns: "+p) } + + // The kernel's protection of links: off, any account may link another's file where root then acts on it. + for _, sysctl := range []string{"protected_hardlinks", "protected_symlinks"} { + raw, err := read("/proc/sys/fs/" + sysctl) + if err != nil { + return nil, fmt.Errorf("fs.%s could not be read: %w", sysctl, err) + } + if strings.TrimSpace(string(raw)) == "0" { + what := map[string]string{"protected_hardlinks": "hard links", "protected_symlinks": "symbolic links"}[sysctl] + ways = append(ways, fmt.Sprintf("%s to other accounts' files are not protected (fs.%s=0)", what, sysctl)) + } + } + + // A container runtime's API on TCP, which any account on the machine reaches. + for _, table := range []string{"/proc/net/tcp", "/proc/net/tcp6"} { + raw, err := read(table) + if errors.Is(err, fs.ErrNotExist) && table == "/proc/net/tcp6" { + continue // no IPv6 on this machine + } + if err != nil { + return nil, fmt.Errorf("the listening ports in %s could not be read: %w", table, err) + } + for _, port := range ListeningPorts(string(raw)) { + for _, api := range RuntimeAPIPorts { + if port == api { + ways = append(ways, fmt.Sprintf("a container runtime's API listens on TCP port %d, which any "+ + "local account reaches", port)) + } + } + } + } return ways, nil } diff --git a/internal/accounts/ways_test.go b/internal/accounts/ways_test.go index 16a83b1..7b9d7c8 100644 --- a/internal/accounts/ways_test.go +++ b/internal/accounts/ways_test.go @@ -49,6 +49,29 @@ func TestEachFurtherWayToRootIsSaid(t *testing.T) { {"secret by ACL", func(m *agentMachine) { m.acls = map[string][]ACLEntry{broker: {{User: false, ID: 1600, Read: true}}} }, "an ACL lets it read the secret " + broker}, + {"polkit for every account", func(m *agentMachine) { + m.dirs = map[string][]string{"/etc/polkit-1/rules.d": {"00-all.rules"}} + m.texts = map[string]string{"/etc/polkit-1/rules.d/00-all.rules": `polkit.addRule(function(action, subject) { return polkit.Result.YES; });`} + }, "a polkit rule grants every account: /etc/polkit-1/rules.d/00-all.rules"}, + {"pkla for every account", func(m *agentMachine) { + m.dirs = map[string][]string{"/etc/polkit-1/localauthority": {"all.pkla"}} + m.texts = map[string]string{"/etc/polkit-1/localauthority/all.pkla": "[all]\nIdentity=unix-user:*\nAction=*\nResultAny=yes\n"} + }, "a polkit rule grants every account: /etc/polkit-1/localauthority/all.pkla"}, + {"docker on TCP", func(m *agentMachine) { + m.proc["/proc/net/tcp6"] = " sl local_address rem_address st\n 0: 00000000000000000000000000000000:0947 00000000000000000000000000000000:0000 0A 0\n" + }, "a container runtime's API listens on TCP port 2375"}, + {"hard links unprotected", func(m *agentMachine) { + m.proc["/proc/sys/fs/protected_hardlinks"] = "0\n" + }, "hard links to other accounts' files are not protected (fs.protected_hardlinks=0)"}, + {"setuid on a second filesystem", func(m *agentMachine) { + m.proc["/proc/mounts"] += "/dev/sdb1 /srv xfs rw 0 0\n/dev/sdc1 /data ext4 rw,nosuid 0 0\n" + m.setuid = "/srv/bin/rootshell\n" + m.findArgs = func(args []string) { + if args[0] != "/" || args[1] != "/srv" || args[2] != "-xdev" { + panic(fmt.Sprintf("searched %v", args)) + } + } + }, "a setuid-root program no package owns: /srv/bin/rootshell"}, {"setuid no package owns", func(m *agentMachine) { m.setuid = "/usr/bin/sudo\n/usr/local/bin/rootshell\n" m.packaged = map[string]bool{"/usr/bin/sudo": true} @@ -169,3 +192,36 @@ func TestTheJudgedListIsWrittenDown(t *testing.T) { t.Fatal("what is judged and what is not are both written down") } } + +func TestDistributionRulesForASeatOrForSomebodyAreNotEveryone(t *testing.T) { + for _, rule := range []string{ + `polkit.addRule(function(a, s) { if (s.local && s.active) return polkit.Result.YES; });`, + `polkit.addRule(function(a, s) { if (s.isInGroup("wheel")) return polkit.Result.YES; });`, + `polkit.addAdminRule(function(a, s) { return ["unix-group:wheel"]; });`, + } { + if PolkitGrantsEveryone("/x.rules", rule) { + t.Errorf("not every account: %s", rule) + } + } + if PolkitGrantsEveryone("/x.pkla", "[a]\nIdentity=unix-group:wheel\nResultAny=yes\n") { + t.Error("a pkla for wheel is not every account") + } +} + +func TestAMachineMissingWhatTheJudgeReadsIsUnknown(t *testing.T) { + for _, file := range []string{"/proc/sys/fs/protected_hardlinks", "/proc/net/tcp", "/proc/mounts"} { + m := clean() + delete(m.proc, file) + if v := lookAgent(t, m); v.State != Unknown { + t.Errorf("%s unread: %+v", file, v) + } + } +} + +func TestSuidMountsLeaveOutNosuidPseudoAndContainerLayers(t *testing.T) { + got := SuidMounts("/dev/sdb1 /srv xfs rw 0 0\n/dev/sda2 / ext4 rw 0 0\noverlay /var/lib/docker/overlay2/x/merged overlay rw 0 0\n" + + "/dev/sdc1 /data ext4 rw,nosuid 0 0\nproc /proc proc rw 0 0\nhost:/x /mnt/nfs nfs4 rw 0 0\n/dev/sdd1 /my\\040disk ext4 rw 0 0\n") + if strings.Join(got, "|") != "/|/my disk|/srv" { + t.Fatalf("got %q", got) + } +} diff --git a/internal/apply/apply.go b/internal/apply/apply.go index e26adcf..0f99c69 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -996,7 +996,7 @@ func applyDirectory(r *declaration.Directory) (Outcome, error) { // Set explicitly even when it existed: MkdirAll applies the mode only on creation, and a // permission set at creation is not a permission maintained — a lesson this repository // already paid for once, with world-readable environment files. - if err := chmodPath(r.Path, mode); err != nil { + if err := chmodDirPath(r.Path, mode); err != nil { return out, err } diff --git a/internal/apply/home_links.go b/internal/apply/home_links.go index 8cf9a24..d62d789 100644 --- a/internal/apply/home_links.go +++ b/internal/apply/home_links.go @@ -122,6 +122,28 @@ func refuseLinksUnderHome(path string) error { return nil } +// HardLinkUnderHomeError is a regular file below a home with more than one link: the account that owns the +// home may have linked a file it does not own there, and root acting on it by name would act on that file +// (novox/hq ADR 0266). Refused, whatever the resource. +type HardLinkUnderHomeError struct { + Path string + Links uint64 +} + +func (e *HardLinkUnderHomeError) Error() string { + return fmt.Sprintf("%s has %d links: below a home a file with more than one is not owned, chmodded or read "+ + "as root, since one of its names may be a file the home's account does not own", e.Path, e.Links) +} + +// chmodDirPath is chmodPath for a directory resource: below a home, what is there must be a directory. +func chmodDirPath(path string, mode os.FileMode) error { + home := homeAbove(path) + if home == "" { + return os.Chmod(path, mode) + } + return chmodUnderAs(home, path, mode, kindDir) +} + // statPath is os.Stat, except below a home, where it never follows a link. func statPath(path string) (os.FileInfo, error) { if homeAbove(path) != "" { diff --git a/internal/apply/home_links_test.go b/internal/apply/home_links_test.go index e74a4f7..041500b 100644 --- a/internal/apply/home_links_test.go +++ b/internal/apply/home_links_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "strings" + "syscall" "testing" "github.com/novox/mesh-host/internal/store" @@ -203,3 +204,52 @@ func TestHomesAreAPersonsOrAnAgentsNotAServicesOrRoots(t *testing.T) { t.Fatal("strictly below a person's or an agent's home, and nothing else") } } + +// A hard link below a home to a file the home's account does not own is never owned, chmodded or read by +// name: fstat on the opened descriptor counts its links first (the re-review of 2026-10-08). +func TestAHardLinkedFileBelowAHomeIsRefused(t *testing.T) { + home, outside := aLinkedHome(t) + shadow := filepath.Join(outside, "shadow") + linked := filepath.Join(home, ".claude") + if err := os.Link(shadow, linked); err != nil { + t.Skipf("no hard link here: %v", err) + } + var hard *HardLinkUnderHomeError + if err := chmodPath(linked, 0o777); !errors.As(err, &hard) { + t.Fatalf("chmod of a hard-linked file below a home is refused, got %v", err) + } + if err := chownPath(linked, os.Getuid(), os.Getgid()); !errors.As(err, &hard) { + t.Fatalf("chown of it is refused, got %v", err) + } + if _, err := readPath(linked); !errors.As(err, &hard) { + t.Fatalf("reading it is refused, got %v", err) + } + if m := modeOfPath(t, shadow); m != 0o600 { + t.Fatalf("the other file's mode changed: %o", m) + } + // As the directory resource sees it: a file where ~/.claude should be is left alone. + if err := chmodDirPath(linked, 0o700); err == nil || !strings.Contains(err.Error(), "where a directory was expected") { + t.Fatalf("a file where a directory was expected is refused, got %v", err) + } +} + +func TestADirectoryWhereAFileWasExpectedAndAFifoAreRefused(t *testing.T) { + home, _ := aLinkedHome(t) + dir := filepath.Join(home, "d") + if err := os.Mkdir(dir, 0o755); err != nil { + t.Fatal(err) + } + if _, err := readPath(dir); err == nil || !strings.Contains(err.Error(), "where a file was expected") { + t.Fatalf("a directory where a file was expected is refused, got %v", err) + } + fifo := filepath.Join(home, "f") + if err := syscall.Mkfifo(fifo, 0o600); err != nil { + t.Skipf("no fifo here: %v", err) + } + if err := chmodPath(fifo, 0o644); err == nil || !strings.Contains(err.Error(), "neither a file nor a directory") { + t.Fatalf("a fifo is refused, got %v", err) + } + if err := chmodDirPath(dir, 0o700); err != nil { + t.Fatalf("a directory as a directory passes: %v", err) + } +} diff --git a/internal/apply/homes_linux.go b/internal/apply/homes_linux.go index b70f818..58ad05a 100644 --- a/internal/apply/homes_linux.go +++ b/internal/apply/homes_linux.go @@ -54,22 +54,85 @@ func linkOr(at, home, path, op string, err error) error { return &os.PathError{Op: op, Path: at, Err: err} } +// What a caller expects to find at a path below a home: either, a directory, or a regular file. +const ( + kindAny = iota + kindDir + kindFile +) + // openUnder opens the file or directory at path below home, following no link, for its metadata. -func openUnder(home, path string) (int, error) { +func openUnder(home, path string) (int, error) { return openUnderAs(home, path, kindAny) } + +// openUnderAs opens path below home as what the caller expects, and refuses what root must not act on +// (novox/hq ADR 0266): a directory is opened with O_DIRECTORY|O_NOFOLLOW; whatever was opened is judged by +// fstat on the descriptor itself, before any fchown, fchmod or read — so a name swapped after a check is +// judged as what it now is. Refused: anything but a directory or a regular file (a device, a fifo, a +// socket), a kind other than the one expected, and a regular file with more than one link. The account that +// owns the home can hard-link a file it does not own (root's, where fs.protected_hardlinks is off) into its +// home; owned or chmodded by name, root would hand that file over. +func openUnderAs(home, path string, want int) (int, error) { dir, err := openDirUnder(home, filepath.Dir(path)) if err != nil { return -1, err } defer unix.Close(dir) - fd, err := unix.Openat(dir, filepath.Base(path), unix.O_RDONLY|unix.O_NOFOLLOW|unix.O_NONBLOCK|unix.O_CLOEXEC, 0) + base := filepath.Base(path) + flags := unix.O_RDONLY | unix.O_NOFOLLOW | unix.O_NONBLOCK | unix.O_CLOEXEC + fd := -1 + if want != kindFile { + fd, err = unix.Openat(dir, base, flags|unix.O_DIRECTORY, 0) + if errors.Is(err, unix.ENOTDIR) && want == kindAny { + fd, err = unix.Openat(dir, base, flags, 0) + } + } else { + fd, err = unix.Openat(dir, base, flags, 0) + } if err != nil { - return -1, linkOr(path, home, path, "open", err) + err = linkOr(path, home, path, "open", err) + var link *LinkUnderHomeError + if want == kindDir && !errors.As(err, &link) && errors.Is(err, unix.ENOTDIR) { + return -1, fmt.Errorf("%s is not a directory where a directory was expected: below a home it is left alone", path) + } + return -1, err + } + if err := judgeOpened(fd, path, want); err != nil { + unix.Close(fd) + return -1, err } return fd, nil } +// judgeOpened is openUnderAs's verdict on what the descriptor holds. +func judgeOpened(fd int, path string, want int) error { + var st unix.Stat_t + if err := unix.Fstat(fd, &st); err != nil { + return &os.PathError{Op: "fstat", Path: path, Err: err} + } + switch st.Mode & unix.S_IFMT { + case unix.S_IFDIR: + if want == kindFile { + return fmt.Errorf("%s is a directory where a file was expected: below a home it is left alone", path) + } + case unix.S_IFREG: + if want == kindDir { + return fmt.Errorf("%s is a file where a directory was expected: below a home it is left alone", path) + } + if st.Nlink > 1 { + return &HardLinkUnderHomeError{Path: path, Links: uint64(st.Nlink)} + } + default: + return fmt.Errorf("%s is neither a file nor a directory: below a home it is left alone", path) + } + return nil +} + func chmodUnder(home, path string, mode os.FileMode) error { - fd, err := openUnder(home, path) + return chmodUnderAs(home, path, mode, kindAny) +} + +func chmodUnderAs(home, path string, mode os.FileMode, want int) error { + fd, err := openUnderAs(home, path, want) if err != nil { return err } @@ -93,7 +156,7 @@ func chownUnder(home, path string, uid, gid int) error { } func readUnder(home, path string) ([]byte, error) { - fd, err := openUnder(home, path) + fd, err := openUnderAs(home, path, kindFile) if err != nil { return nil, err } diff --git a/internal/apply/homes_other.go b/internal/apply/homes_other.go index 9e97a80..64974f1 100644 --- a/internal/apply/homes_other.go +++ b/internal/apply/homes_other.go @@ -17,6 +17,12 @@ func chmodUnder(home, path string, mode os.FileMode) error { return os.Chmod(path, mode) } +func chmodUnderAs(home, path string, mode os.FileMode, _ int) error { + return chmodUnder(home, path, mode) +} + +const kindDir = 1 + func chownUnder(home, path string, uid, gid int) error { if err := refuseLinksUnderHome(path); err != nil { return err