mirror of
https://github.com/agent-substrate/substrate.git
synced 2026-11-10 10:13:50 +08:00
atelet: drop privileged from the DaemonSet
atelet performs no privilege-requiring host operations: no mounts, netlink,
nftables, device access, or namespace manipulation (all of that lives in the
ateom worker pod). It only reads/writes the /var/lib/ateom-gvisor hostPath as
root, so it needs no Linux capabilities. Replace privileged:true with
runAsUser/runAsGroup 0 and capabilities drop ALL. runAsUser is pinned explicitly
so the root requirement no longer depends on the base image's default user.
Unpacking arbitrary actor images can require writing into image-defined
read-only directories (e.g. ko ships /ko-app as 0555), which root cannot do
without CAP_DAC_OVERRIDE. Instead of granting that cap:
- untar extracts directories owner-writable, then restores the image's modes
in a final pass (deepest-first), so directory and file modes are preserved
faithfully.
- bundle removal uses removeAllWritable, which re-adds owner write to each
directory before unlinking, so atelet can also delete a restored read-only
tree as plain root.
Validated end-to-end on a kind cluster.
This commit is contained in:
committed by
Haven Xia
parent
a1d46c11a6
commit
8fbc580e94
+4
-1
@@ -1000,8 +1000,11 @@ func writeFileAtomic(path string, data []byte, perm os.FileMode) error {
|
||||
func resetActorDirs(atespace, actorName string) error {
|
||||
// Explicitly leave runsc logs dir untouched.
|
||||
|
||||
// removeAllWritable, not os.RemoveAll: the bundle holds unpacked actor-image
|
||||
// rootfs whose directories keep the image's (possibly read-only) modes, which
|
||||
// atelet can't remove as plain root without first making them writable.
|
||||
bundleDir := ateompath.OCIBundleDir(atespace, actorName)
|
||||
if err := os.RemoveAll(bundleDir); err != nil {
|
||||
if err := removeAllWritable(bundleDir); err != nil {
|
||||
return wrapFileSystemErr("while deleting bundle dir: %w", err)
|
||||
}
|
||||
if err := os.MkdirAll(bundleDir, 0o700); err != nil {
|
||||
|
||||
+52
-3
@@ -21,10 +21,12 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"io/fs"
|
||||
"log/slog"
|
||||
"os"
|
||||
"path"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"strings"
|
||||
|
||||
"github.com/agent-substrate/substrate/cmd/atelet/internal/memorypullcache"
|
||||
@@ -64,7 +66,7 @@ func prepareOCIDirectory(ctx context.Context, pullCache *memorypullcache.MemoryP
|
||||
bundlePath := ateompath.OCIBundlePath(atespace, actorName, containerName)
|
||||
rootPath := path.Join(bundlePath, "rootfs")
|
||||
|
||||
if err := os.RemoveAll(rootPath); err != nil {
|
||||
if err := removeAllWritable(rootPath); err != nil {
|
||||
return fmt.Errorf("while clearing rootfs %q: %w", rootPath, err)
|
||||
}
|
||||
|
||||
@@ -309,6 +311,13 @@ func untar(ctx context.Context, tarData io.Reader, rootPath string) error {
|
||||
}
|
||||
defer root.Close()
|
||||
|
||||
// Directories are created owner-writable during extraction (so their children
|
||||
// can be written even when the image marks them read-only, e.g. ko ships
|
||||
// /ko-app as 0555) and their real modes are restored afterwards. This lets
|
||||
// atelet, running as plain root, unpack arbitrary actor images without
|
||||
// CAP_DAC_OVERRIDE. Keyed by name so a repeated dir entry's last mode wins.
|
||||
dirModes := map[string]os.FileMode{}
|
||||
|
||||
tarReader := tar.NewReader(tarData)
|
||||
for {
|
||||
hdr, err := tarReader.Next()
|
||||
@@ -360,12 +369,18 @@ func untar(ctx context.Context, tarData io.Reader, rootPath string) error {
|
||||
}
|
||||
|
||||
case tar.TypeDir:
|
||||
err := root.Mkdir(name, mode)
|
||||
// Create owner-writable so children can be written even when the image
|
||||
// marks the dir read-only; the real mode is restored after extraction
|
||||
// (see dirModes / the restore pass below).
|
||||
err := root.Mkdir(name, mode|0o700)
|
||||
if errors.Is(err, os.ErrExist) {
|
||||
// Ignore --- real images produced by ko seem to have directory entries placed multiple times?
|
||||
// OCI layers can repeat a directory entry (real ko images do); the
|
||||
// existing dir is already owner-writable, so let the later entry's
|
||||
// mode win at restore time.
|
||||
} else if err != nil {
|
||||
return fmt.Errorf("while creating directory=%q, mode=%v: %w", name, mode, err)
|
||||
}
|
||||
dirModes[name] = mode
|
||||
|
||||
case tar.TypeSymlink:
|
||||
// OCI image layers may re-define the same path across layers (e.g.
|
||||
@@ -421,5 +436,39 @@ func untar(ctx context.Context, tarData io.Reader, rootPath string) error {
|
||||
|
||||
}
|
||||
|
||||
// Restore the image's intended directory modes now that every child exists.
|
||||
// Deepest paths first: a child's path is always longer than its parent's, so
|
||||
// length-descending order guarantees a directory is restored before any of its
|
||||
// ancestors — restoring a parent to a non-traversable mode then can't block
|
||||
// restoring its children.
|
||||
dirs := make([]string, 0, len(dirModes))
|
||||
for name := range dirModes {
|
||||
dirs = append(dirs, name)
|
||||
}
|
||||
sort.Slice(dirs, func(i, j int) bool { return len(dirs[i]) > len(dirs[j]) })
|
||||
for _, name := range dirs {
|
||||
if err := root.Chmod(name, dirModes[name]); err != nil {
|
||||
return fmt.Errorf("while restoring mode %v on directory %q: %w", dirModes[name], name, err)
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// removeAllWritable removes path and everything under it, first making every
|
||||
// directory owner-writable so its children can be unlinked. atelet runs as plain
|
||||
// root (no CAP_DAC_OVERRIDE), so it cannot remove entries inside an image-defined
|
||||
// read-only directory (e.g. ko's restored 0555 /ko-app) — os.RemoveAll alone
|
||||
// fails there with EACCES. atelet owns these files, so chmod needs no capability.
|
||||
func removeAllWritable(path string) error {
|
||||
// Make dirs traversable/writable top-down (WalkDir visits a directory before
|
||||
// reading it, so chmod here lets the walk descend into otherwise-unreadable
|
||||
// dirs). Best-effort: ignore errors and let os.RemoveAll surface real ones.
|
||||
_ = filepath.WalkDir(path, func(p string, d fs.DirEntry, err error) error {
|
||||
if err == nil && d.IsDir() {
|
||||
_ = os.Chmod(p, 0o700)
|
||||
}
|
||||
return nil
|
||||
})
|
||||
return os.RemoveAll(path)
|
||||
}
|
||||
|
||||
@@ -499,6 +499,37 @@ func TestUntar_LaterEntryWins(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
// A read-only directory in the image (e.g. ko ships /ko-app as 0555) must still
|
||||
// get its child written AND keep the image's mode, so atelet can unpack arbitrary
|
||||
// actor images as plain root without CAP_DAC_OVERRIDE. removeAllWritable must
|
||||
// then still be able to delete the restored read-only tree. (Meaningful as a
|
||||
// non-root test run; as root the dir-permission checks are bypassed.)
|
||||
func TestUntar_ReadOnlyDir(t *testing.T) {
|
||||
entries := []tarEntry{
|
||||
{name: "ko-app", typeflag: tar.TypeDir, mode: 0o555},
|
||||
{name: "ko-app/counter", typeflag: tar.TypeReg, mode: 0o755, body: "bin"},
|
||||
}
|
||||
dir, err := runUntar(t, entries)
|
||||
if err != nil {
|
||||
t.Fatalf("untar into read-only dir: %v", err)
|
||||
}
|
||||
if got, _ := os.ReadFile(filepath.Join(dir, "ko-app/counter")); string(got) != "bin" {
|
||||
t.Errorf("ko-app/counter = %q, want %q", got, "bin")
|
||||
}
|
||||
info, err := os.Stat(filepath.Join(dir, "ko-app"))
|
||||
if err != nil {
|
||||
t.Fatalf("stat ko-app: %v", err)
|
||||
}
|
||||
if info.Mode().Perm() != 0o555 {
|
||||
t.Errorf("ko-app mode = %v, want the image's 0555 preserved", info.Mode().Perm())
|
||||
}
|
||||
// atelet must be able to delete the restored read-only tree (this also lets
|
||||
// t.TempDir's cleanup succeed, which plain os.RemoveAll could not on 0555).
|
||||
if err := removeAllWritable(filepath.Join(dir, "ko-app")); err != nil {
|
||||
t.Errorf("removeAllWritable on restored read-only dir: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUntar_PathTraversal(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
@@ -69,8 +69,21 @@ spec:
|
||||
image: ko://github.com/agent-substrate/substrate/cmd/atelet
|
||||
args:
|
||||
- --gcp-auth-for-image-pulls=true
|
||||
# atelet does no mounts, netlink, device, or namespace operations (those
|
||||
# live in the ateom worker pod) — it only reads/writes the
|
||||
# /var/lib/ateom-gvisor hostPath as root, so it needs no Linux
|
||||
# capabilities. (Unpacking arbitrary actor images can require writing into
|
||||
# image-defined read-only dirs; the untar in cmd/atelet/oci.go extracts
|
||||
# directories writable then restores the image's modes, and removal makes
|
||||
# them writable again first, so root suffices without CAP_DAC_OVERRIDE.)
|
||||
# runAsUser is pinned explicitly rather than relying on the base image's
|
||||
# default user.
|
||||
securityContext:
|
||||
privileged: true
|
||||
runAsUser: 0
|
||||
runAsGroup: 0
|
||||
capabilities:
|
||||
drop:
|
||||
- ALL
|
||||
env:
|
||||
- name: MY_NODE_NAME
|
||||
valueFrom:
|
||||
|
||||
Reference in New Issue
Block a user