roothelper: walk validateManagedPath components, reject symlinks

validateManagedPath was textual-only: filepath.Clean + dest-prefix
match. That stopped `..` escapes but not the symlink-bypass attack
that motivated this fix — a daemon-UID attacker can write into
StateDir/RuntimeDir (it's their UID), so they can plant
`<StateDir>/redirect -> /etc` and any helper RPC that then operates
on `<StateDir>/redirect/...` resolves through the symlink at the
kernel and lands at /etc/... on the host.

Concretely the leaks this closed:
  * priv.create_dm_snapshot: rootfs/cow paths fed to losetup —
    losetup follows the symlink and attaches a host block device.
  * priv.launch_firecracker: kernel/initrd paths hard-linked into
    the chroot via `ln -f` — link(2) on Linux follows source
    symlinks, hard-linking host files into the jail.
  * priv.read_ext4_file / priv.write_ext4_files: image paths fed
    to debugfs / e2cp as root.
  * validateLaunchDrivePath: drive paths mknod'd or hard-linked.
  * validateJailerOpts: chroot base.

Fix: after the existing prefix match, walk every component below
the matched root with Lstat. Any existing symlink — leaf or
intermediate — fails the validator. ENOENT is tolerated because
several callers pass paths firecracker/the helper materialise
later (sockets, log files, kernel hard-link targets); whoever
materialises them goes through the same validation when the
helper-side primitive runs.

Subsumes most of validateNotSymlink's coverage but the explicit
call sites (methodEnsureSocketAccess, methodCleanupJailerChroot)
keep their belt-and-braces check — those paths must EXIST and
not be symlinks, which validateNotSymlink enforces strictly while
the broadened validateManagedPath tolerates ENOENT.

Race-free in practice: helper RPCs are short and the validator
fires on the same kernel state the next syscall sees. The helper
loop processes RPCs serially per-connection, and the validator
plus the syscall both run as root within microseconds of each
other.

Four new tests cover symlink leaf, symlink intermediate, missing
leaf (must pass), and the plain happy path. Smoke at JOBS=4 still
green — every legitimate daemon-supplied path passes the walk.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Thales Maciel 2026-04-28 15:26:56 -03:00
parent 0a079277ef
commit 4a56e6c7d6
No known key found for this signature in database
GPG key ID: 33112E6833C34679
2 changed files with 115 additions and 2 deletions

View file

@ -1037,6 +1037,7 @@ func (s *Server) validateManagedPath(path string, roots ...string) error {
return fmt.Errorf("path %q must be absolute", path)
}
cleaned := filepath.Clean(path)
var matched string
for _, root := range roots {
root = strings.TrimSpace(root)
if root == "" {
@ -1044,10 +1045,45 @@ func (s *Server) validateManagedPath(path string, roots ...string) error {
}
root = filepath.Clean(root)
if cleaned == root || strings.HasPrefix(cleaned, root+string(os.PathSeparator)) {
return nil
matched = root
break
}
}
return fmt.Errorf("path %q is outside banger-managed directories", path)
if matched == "" {
return fmt.Errorf("path %q is outside banger-managed directories", path)
}
// Walk each component below the matched root with Lstat and refuse
// symlinks. Without this, validation was textual-only: a daemon-UID
// attacker could plant a symlink under StateDir/RuntimeDir and get
// the helper to drive losetup, ln -f, debugfs, e2cp, fsck, etc. at
// the dereferenced target (host devices, /etc/shadow, …).
//
// ENOENT is tolerated: some callers pass paths that firecracker
// creates after this check (sockets, log files). Anything missing
// can't be a symlink at this instant; whoever materialises it later
// goes through the helper's create primitives, which validate again.
if cleaned == matched {
return nil
}
suffix := strings.TrimPrefix(cleaned, matched+string(os.PathSeparator))
cur := matched
for _, seg := range strings.Split(suffix, string(os.PathSeparator)) {
if seg == "" {
continue
}
cur = filepath.Join(cur, seg)
info, err := os.Lstat(cur)
if err != nil {
if os.IsNotExist(err) {
return nil
}
return fmt.Errorf("inspect %q: %w", cur, err)
}
if info.Mode()&os.ModeSymlink != 0 {
return fmt.Errorf("path %q has a symlink at %q", path, cur)
}
}
return nil
}
// validateExt4ImagePath accepts a path that is either inside the

View file

@ -237,6 +237,83 @@ func TestValidateDMSnapshotHandles(t *testing.T) {
}
}
// TestValidateManagedPathRejectsSymlinkLeaf pins the leaf-symlink
// rejection: even when the path string sits inside a managed root, a
// symlink at the final component must be refused. Otherwise a
// daemon-UID attacker could plant `<StateDir>/foo -> /etc/shadow` and
// get the helper to drive privileged tooling against host files.
func TestValidateManagedPathRejectsSymlinkLeaf(t *testing.T) {
t.Parallel()
srv := &Server{}
root := t.TempDir()
target := filepath.Join(t.TempDir(), "outside")
if err := os.WriteFile(target, []byte("secret"), 0o600); err != nil {
t.Fatalf("write target: %v", err)
}
link := filepath.Join(root, "leak")
if err := os.Symlink(target, link); err != nil {
t.Fatalf("symlink: %v", err)
}
err := srv.validateManagedPath(link, root)
if err == nil {
t.Fatal("validateManagedPath(symlink leaf) succeeded, want error")
}
}
// TestValidateManagedPathRejectsSymlinkIntermediate pins ancestor
// symlink rejection. Without the walk, an attacker plants
// `<StateDir>/dir -> /etc` and a path like `<StateDir>/dir/passwd`
// passes the textual prefix check but resolves to /etc/passwd at op
// time.
func TestValidateManagedPathRejectsSymlinkIntermediate(t *testing.T) {
t.Parallel()
srv := &Server{}
root := t.TempDir()
target := t.TempDir()
link := filepath.Join(root, "redirect")
if err := os.Symlink(target, link); err != nil {
t.Fatalf("symlink: %v", err)
}
err := srv.validateManagedPath(filepath.Join(link, "passwd"), root)
if err == nil {
t.Fatal("validateManagedPath(symlink intermediate) succeeded, want error")
}
}
// TestValidateManagedPathToleratesMissingLeaf confirms ENOENT does
// not flip the validator into a fail. Several callers pass paths
// firecracker (or the helper's own staging) creates AFTER validation
// — sockets, log files, kernel hard-link targets — and a strict
// existence check would break those flows.
func TestValidateManagedPathToleratesMissingLeaf(t *testing.T) {
t.Parallel()
srv := &Server{}
root := t.TempDir()
missing := filepath.Join(root, "deeper", "not-yet")
if err := srv.validateManagedPath(missing, root); err != nil {
t.Fatalf("validateManagedPath(missing leaf) = %v, want nil", err)
}
}
// TestValidateManagedPathPassesPlainSubpath is the happy path: a
// regular file inside a real subdir should sail through the new walk.
func TestValidateManagedPathPassesPlainSubpath(t *testing.T) {
t.Parallel()
srv := &Server{}
root := t.TempDir()
subdir := filepath.Join(root, "vms", "abc")
if err := os.MkdirAll(subdir, 0o755); err != nil {
t.Fatalf("mkdir: %v", err)
}
leaf := filepath.Join(subdir, "rootfs.ext4")
if err := os.WriteFile(leaf, []byte("data"), 0o644); err != nil {
t.Fatalf("write leaf: %v", err)
}
if err := srv.validateManagedPath(leaf, root); err != nil {
t.Fatalf("validateManagedPath(plain subpath) = %v, want nil", err)
}
}
func TestValidateLinuxIfaceName(t *testing.T) {
t.Parallel()