amadan

carlos / platform Public

host: root's registry chown never follows a planted link (local escalation fix)

chownRegistryFiles ran os.Stat and os.Chown as root on /data/registry.db
and its -wal/-shm sidecars, following symlinks. /data belongs to the
service user, so the agent or a flat tenant could plant registry.db-wal as
a symlink, or a hard link, to a root file, and the next adopt or system
apply would hand it to carlos. Now each file is opened O_NOFOLLOW, must be a
plain single-link regular file, and is chowned through that descriptor.
Found by the Opus review of claim Plan 9 (P-1); it predates the claim work.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 869d4bc7775b7648e75a8444664bf4734a68b373 parent 17d5960
4 files changed, +121 −7
  • internal/host/registryowner.go +9 −7
  • internal/host/registryowner_chown_other.go +11 −0
  • internal/host/registryowner_chown_unix.go +48 −0
  • internal/host/registryowner_chown_unix_test.go +53 −0

CI passed — run details

diff --git a/internal/host/registryowner.go b/internal/host/registryowner.go
index 756b9e9d0..954979ab2 100644
--- a/internal/host/registryowner.go
+++ b/internal/host/registryowner.go
@@ -1,6 +1,7 @@
package host
import (
+ "errors"
"fmt"
"log"
"os"
@@ -102,21 +103,22 @@ func chownRegistryFiles(euid int, svcUser, path string) {
return
}
for _, p := range registryFiles(path) {
- fi, err := os.Stat(p)
- if err != nil {
+ changed, err := ownRegistryFile(p, uid, gid)
+ if errors.Is(err, os.ErrNotExist) {
// The -wal/-shm sidecars only exist while some connection is
// open. Their absence is the normal state, not a problem.
continue
}
- if fuid, fgid, ok := fileOwner(fi); ok && fuid == uid && fgid == gid {
- continue
- }
- if err := os.Chown(p, uid, gid); err != nil {
+ if err != nil {
// Logged and stepped over, never returned: the files after this
- // one may well be fixable, and none of it is worth the pass.
+ // one may well be fixable, and none of it is worth the pass. A
+ // symlink or hard link here is refused, never followed.
log.Printf("adopt: chown %s to %s: %v", p, svcUser, err)
continue
}
+ if !changed {
+ continue
+ }
// Worth a line every time it fires: on a healthy box it never does,
// so a run of these is the box telling you something armed root is
// creating the registry ahead of the edge.
diff --git a/internal/host/registryowner_chown_other.go b/internal/host/registryowner_chown_other.go
new file mode 100644
index 000000000..d90413cb5
--- /dev/null
+++ b/internal/host/registryowner_chown_other.go
@@ -0,0 +1,11 @@
+//go:build !unix
+
+package host
+
+import "errors"
+
+// ownRegistryFile is never reached off unix: chownRegistryFiles returns early
+// unless it runs as root, and the box side is Linux-only.
+func ownRegistryFile(string, int, int) (bool, error) {
+ return false, errors.New("registry ownership is unix-only")
+}
diff --git a/internal/host/registryowner_chown_unix.go b/internal/host/registryowner_chown_unix.go
new file mode 100644
index 000000000..0d7acec33
--- /dev/null
+++ b/internal/host/registryowner_chown_unix.go
@@ -0,0 +1,48 @@
+//go:build unix
+
+package host
+
+import (
+ "errors"
+ "fmt"
+ "os"
+ "syscall"
+)
+
+// errRegistryFileShape refuses a registry path that is not a plain,
+// single-link regular file. /data belongs to the service user, so anything
+// there may have been planted: a symlink or a hard link to a root-owned file
+// would otherwise hand that file to the service user when root chowns it.
+var errRegistryFileShape = errors.New("not a plain single-link regular file")
+
+// ownRegistryFile gives path to uid:gid without following a symlink and
+// without touching any file reachable by another name. It opens the path
+// O_NOFOLLOW, checks the opened file itself, and changes ownership through
+// that descriptor, so a swap between the check and the chown cannot redirect
+// it. A missing file reports os.ErrNotExist; changed is false when the file
+// was already owned as wanted.
+func ownRegistryFile(path string, uid, gid int) (changed bool, err error) {
+ f, err := os.OpenFile(path, os.O_RDONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK|syscall.O_CLOEXEC, 0)
+ if err != nil {
+ if errors.Is(err, syscall.ELOOP) {
+ return false, fmt.Errorf("%s: %w", path, errRegistryFileShape)
+ }
+ return false, err
+ }
+ defer f.Close()
+ fi, err := f.Stat()
+ if err != nil {
+ return false, err
+ }
+ st, ok := fi.Sys().(*syscall.Stat_t)
+ if !ok || !fi.Mode().IsRegular() || st.Nlink != 1 {
+ return false, fmt.Errorf("%s: %w", path, errRegistryFileShape)
+ }
+ if int(st.Uid) == uid && int(st.Gid) == gid {
+ return false, nil
+ }
+ if err := f.Chown(uid, gid); err != nil {
+ return false, err
+ }
+ return true, nil
+}
diff --git a/internal/host/registryowner_chown_unix_test.go b/internal/host/registryowner_chown_unix_test.go
new file mode 100644
index 000000000..1394519e8
--- /dev/null
+++ b/internal/host/registryowner_chown_unix_test.go
@@ -0,0 +1,53 @@
+//go:build unix
+
+package host
+
+import (
+ "errors"
+ "os"
+ "path/filepath"
+ "testing"
+)
+
+// Root chowns the registry files in the service user's /data. A planted
+// symlink or hard link must be refused, never followed, so it cannot hand a
+// root-owned file to the service user.
+func TestOwnRegistryFileRefusesPlantedLinks(t *testing.T) {
+ dir := t.TempDir()
+ uid, gid := os.Getuid(), os.Getgid()
+ victim := filepath.Join(dir, "victim")
+ if err := os.WriteFile(victim, []byte("root's"), 0600); err != nil {
+ t.Fatal(err)
+ }
+ link := filepath.Join(dir, "registry.db-wal")
+ if err := os.Symlink(victim, link); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := ownRegistryFile(link, uid, gid); !errors.Is(err, errRegistryFileShape) {
+ t.Fatalf("symlink followed or wrongly accepted: %v", err)
+ }
+ hard := filepath.Join(dir, "registry.db-shm")
+ if err := os.Link(victim, hard); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := ownRegistryFile(hard, uid, gid); !errors.Is(err, errRegistryFileShape) {
+ t.Fatalf("hard link accepted: %v", err)
+ }
+ sub := filepath.Join(dir, "registry.db")
+ if err := os.Mkdir(sub, 0700); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := ownRegistryFile(sub, uid, gid); !errors.Is(err, errRegistryFileShape) {
+ t.Fatalf("directory accepted: %v", err)
+ }
+ if _, err := ownRegistryFile(filepath.Join(dir, "absent"), uid, gid); !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("missing file: %v", err)
+ }
+ plain := filepath.Join(dir, "plain.db")
+ if err := os.WriteFile(plain, nil, 0600); err != nil {
+ t.Fatal(err)
+ }
+ if changed, err := ownRegistryFile(plain, uid, gid); err != nil || changed {
+ t.Fatalf("plain file already owned: changed=%v err=%v", changed, err)
+ }
+}