amadan

carlos / platform Public

activator: refuse a symlink at a flat row's <host>.env on every instance-key path, in the open itself (O_NOFOLLOW)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude ba7c661c6082c54109436a9122df6ee30907de28 parent 4c2e881
4 files changed, +194 −3
  • internal/activator/instancekey.go +31 −3
  • internal/activator/instancekey_test.go +119 −0
  • internal/activator/ownenv_other.go +20 −0
  • internal/activator/ownenv_unix.go +24 −0
diff --git a/internal/activator/instancekey.go b/internal/activator/instancekey.go
index 2ab5ae99f..2f614677c 100644
--- a/internal/activator/instancekey.go
+++ b/internal/activator/instancekey.go
@@ -25,12 +25,17 @@ package activator
// are already there;
// - the value is never logged, never put in an error, and never leaves
// the file except in the launch environment.
+// - a flat row's own file is opened O_NOFOLLOW on every path, read and
+// append alike (openOwnEnvNoFollow): a symlink there is refused, never
+// followed into a file a same-uid sibling chose. A nested row's file
+// gets the same from instancefs.
import (
"crypto/rand"
"encoding/base64"
"errors"
"fmt"
+ "io"
"os"
"path/filepath"
"strings"
@@ -190,7 +195,24 @@ func openInstanceEnv(dir, name string, beneath, create bool) (*os.File, error) {
if beneath {
return instancefs.OpenBeneath(dir, name, flags, perm)
}
- return os.OpenFile(filepath.Join(dir, name), flags, perm)
+ return openOwnEnvNoFollow(filepath.Join(dir, name), flags, perm)
+}
+
+// errOwnEnvSymlink is a flat row's own env file found to be a symlink. The
+// ensure refuses it on every path (mint, existing key, backfill, the claim
+// settle): following it would append to, chmod and fsync a file a same-uid
+// sibling chose (spec §7.2).
+var errOwnEnvSymlink = errors.New("activator: the instance env file is a symlink, refused")
+
+// readOwnEnv reads a flat row's own env file through openOwnEnvNoFollow,
+// so the read refuses a link exactly as the append does.
+func readOwnEnv(path string) ([]byte, error) {
+ f, err := openOwnEnvNoFollow(path, os.O_RDONLY, 0)
+ if err != nil {
+ return nil, err
+ }
+ defer f.Close()
+ return io.ReadAll(f)
}
// settleInstanceEnv is the durability step every successful ensure ends
@@ -223,7 +245,13 @@ func ensureInstanceKey(dir, name string, beneath, create bool) (InstanceKeyOutco
}
return os.ReadFile(filepath.Join(dir, n))
}
- raw, err := read(name)
+ var raw []byte
+ var err error
+ if beneath {
+ raw, err = read(name)
+ } else {
+ raw, err = readOwnEnv(path)
+ }
switch {
case errors.Is(err, os.ErrNotExist):
if !create {
@@ -306,7 +334,7 @@ func (b *ExecBackend) settleClaimInstanceEnv(rt registry.Route) error {
if p.layout == LayoutNested {
f, err = instancefs.OpenBeneath(p.dataDir, name, os.O_RDONLY, 0)
} else {
- f, err = os.Open(p.envFile)
+ f, err = openOwnEnvNoFollow(p.envFile, os.O_RDONLY, 0)
}
if err != nil {
return fmt.Errorf("open %s: %w", p.envFile, err)
diff --git a/internal/activator/instancekey_test.go b/internal/activator/instancekey_test.go
index 263d8a9dd..8e2bdae0e 100644
--- a/internal/activator/instancekey_test.go
+++ b/internal/activator/instancekey_test.go
@@ -335,3 +335,122 @@ func TestReplicaRowsGetNoPlatformKey(t *testing.T) {
t.Fatal("a replica row was given a platform key")
}
}
+
+// The flat ensure never follows a symlink planted at <host>.env (spec §7.2,
+// the hazard TestEnsureAdminTokenRefusesToChmodThroughASymlink guards). On
+// every path, mint, existing key, backfill and the claim settle, it refuses
+// and leaves the link's target's mode and bytes exactly as they were.
+func TestEnsureInstanceKeyRefusesToFollowASymlink(t *testing.T) {
+ wellFormed := InstanceKeyVar + "=" + strings.Repeat("A", 43) + "=\n"
+ for _, tc := range []struct {
+ name string
+ target string // the target's contents
+ mode os.FileMode
+ call func(b *ExecBackend, rt registry.Route, link string) error
+ }{
+ {"mint", "A=1\n", 0o644, func(_ *ExecBackend, _ registry.Route, link string) error {
+ _, err := EnsureInstanceKey(link)
+ return err
+ }},
+ {"existing key", wellFormed, 0o644, func(_ *ExecBackend, _ registry.Route, link string) error {
+ _, err := EnsureInstanceKey(link)
+ return err
+ }},
+ {"backfill mint", "A=1\n", 0o644, func(b *ExecBackend, rt registry.Route, _ string) error {
+ _, err := b.BackfillInstanceKey(rt)
+ return err
+ }},
+ {"backfill existing key", wellFormed, 0o644, func(b *ExecBackend, rt registry.Route, _ string) error {
+ _, err := b.BackfillInstanceKey(rt)
+ return err
+ }},
+ {"claim settle", wellFormed, 0o600, func(b *ExecBackend, rt registry.Route, _ string) error {
+ return b.settleClaimInstanceEnv(rt)
+ }},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ b := &ExecBackend{DataDir: t.TempDir()}
+ rt := registry.Route{Host: "shop.example.com", Socket: "/run/carlos/shop.sock"}
+ target := filepath.Join(t.TempDir(), "someone-elses-file")
+ if err := os.WriteFile(target, []byte(tc.target), tc.mode); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Chmod(target, tc.mode); err != nil {
+ t.Fatal(err)
+ }
+ link := filepath.Join(b.DataDir, rt.Host+".env")
+ if err := os.Symlink(target, link); err != nil {
+ t.Skipf("this filesystem will not make symlinks: %v", err)
+ }
+ err := tc.call(b, rt, link)
+ if !errors.Is(err, errOwnEnvSymlink) {
+ t.Fatalf("err = %v, want errOwnEnvSymlink", err)
+ }
+ if strings.Contains(err.Error(), strings.Repeat("A", 43)) {
+ t.Fatal("the refusal put the key in its error")
+ }
+ st, err := os.Stat(target)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if st.Mode().Perm() != tc.mode {
+ t.Fatalf("mode of the symlink's TARGET = %o, want the untouched %o", st.Mode().Perm(), tc.mode)
+ }
+ if raw, _ := os.ReadFile(target); string(raw) != tc.target {
+ t.Fatalf("the symlink's TARGET changed: %q -> %q", tc.target, raw)
+ }
+ })
+ }
+}
+
+// A dangling link is refused too: the mint's O_CREATE must not create the
+// file it points at.
+func TestEnsureInstanceKeyRefusesADanglingSymlink(t *testing.T) {
+ dir := t.TempDir()
+ target := filepath.Join(t.TempDir(), "not-yet")
+ link := filepath.Join(dir, "shop.example.com.env")
+ if err := os.Symlink(target, link); err != nil {
+ t.Skipf("this filesystem will not make symlinks: %v", err)
+ }
+ if _, err := EnsureInstanceKey(link); !errors.Is(err, errOwnEnvSymlink) {
+ t.Fatalf("err = %v, want errOwnEnvSymlink", err)
+ }
+ if _, err := os.Lstat(target); !os.IsNotExist(err) {
+ t.Fatalf("the mint created the link's target: %v", err)
+ }
+}
+
+// The refusal is the open's own, not a check before it: the helper refuses
+// a link for every flag set the ensure uses, so no Lstat-to-open window
+// exists on unix.
+func TestOpenOwnEnvNoFollowRefusesALinkForEveryFlagSet(t *testing.T) {
+ dir := t.TempDir()
+ target := filepath.Join(dir, "target")
+ if err := os.WriteFile(target, []byte("A=1\n"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ link := filepath.Join(dir, "h.env")
+ if err := os.Symlink(target, link); err != nil {
+ t.Skipf("this filesystem will not make symlinks: %v", err)
+ }
+ for _, flags := range []int{os.O_RDONLY, os.O_APPEND | os.O_WRONLY, os.O_APPEND | os.O_WRONLY | os.O_CREATE} {
+ perm := os.FileMode(0)
+ if flags&os.O_CREATE != 0 {
+ perm = 0o600
+ }
+ f, err := openOwnEnvNoFollow(link, flags, perm)
+ if err == nil {
+ f.Close()
+ t.Fatalf("flags %#x: opened through a symlink", flags)
+ }
+ if !errors.Is(err, errOwnEnvSymlink) {
+ t.Fatalf("flags %#x: err = %v, want errOwnEnvSymlink", flags, err)
+ }
+ }
+ // An ordinary file still opens.
+ f, err := openOwnEnvNoFollow(target, os.O_RDONLY, 0)
+ if err != nil {
+ t.Fatal(err)
+ }
+ f.Close()
+}
diff --git a/internal/activator/ownenv_other.go b/internal/activator/ownenv_other.go
new file mode 100644
index 000000000..035ea007b
--- /dev/null
+++ b/internal/activator/ownenv_other.go
@@ -0,0 +1,20 @@
+//go:build !unix
+
+package activator
+
+import (
+ "fmt"
+ "os"
+)
+
+// openOwnEnvNoFollow is the unix helper's stand-in where there is no
+// O_NOFOLLOW (windows). The agent only runs on Linux; this keeps the
+// cross-compiled builds working and still refuses a link it can see, by
+// the same Lstat check EnsureAdminToken makes. It is check-then-open, so
+// it is weaker than the unix open, which is the one that matters.
+func openOwnEnvNoFollow(path string, flag int, perm os.FileMode) (*os.File, error) {
+ if fi, err := os.Lstat(path); err == nil && fi.Mode()&os.ModeSymlink != 0 {
+ return nil, fmt.Errorf("%w: %s", errOwnEnvSymlink, path)
+ }
+ return os.OpenFile(path, flag, perm)
+}
diff --git a/internal/activator/ownenv_unix.go b/internal/activator/ownenv_unix.go
new file mode 100644
index 000000000..bc17196ba
--- /dev/null
+++ b/internal/activator/ownenv_unix.go
@@ -0,0 +1,24 @@
+//go:build unix
+
+package activator
+
+import (
+ "errors"
+ "fmt"
+ "os"
+
+ "golang.org/x/sys/unix"
+)
+
+// openOwnEnvNoFollow opens a flat row's own <host>.env and refuses a
+// symlink at it in the open itself (O_NOFOLLOW), so there is no window
+// between a check and the open for a same-uid sibling (spec §7.2) to plant
+// a link in. A refused link is errOwnEnvSymlink. A nested row's file needs
+// none of this: instancefs.OpenBeneath already refuses links.
+func openOwnEnvNoFollow(path string, flag int, perm os.FileMode) (*os.File, error) {
+ f, err := os.OpenFile(path, flag|unix.O_NOFOLLOW, perm)
+ if errors.Is(err, unix.ELOOP) {
+ return nil, fmt.Errorf("%w: %s", errOwnEnvSymlink, path)
+ }
+ return f, err
+}