diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 55a83b6..d92657f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,6 +24,32 @@ jobs: - name: Compile binary via nix run: nix build + integration: + name: pivot integration (NixOS VM) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: DeterminateSystems/nix-installer-action@main + + - uses: cachix/cachix-action@v15 + with: + name: ananthb + authToken: ${{ secrets.CACHIX_AUTH_TOKEN }} + + # NixOS VM tests boot a guest under QEMU/KVM; expose /dev/kvm to the job. + - name: Enable KVM + run: | + echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", OPTIONS+="static_node=kvm"' \ + | sudo tee /etc/udev/rules.d/99-kvm4all.rules + sudo udevadm control --reload-rules + sudo udevadm trigger --name-match=kvm + + # Real-kernel coverage of the pivot path: pivot_root + mount ordering + # and the tar-extraction containment tests, run as root inside the VM. + - name: pivot_root + extraction VM test + run: nix build -L .#checks.x86_64-linux.nixos-pivot + goreleaser-check: name: goreleaser check + snapshot smoke test runs-on: ubuntu-latest diff --git a/README.md b/README.md index 2bae97e..bc960f8 100644 --- a/README.md +++ b/README.md @@ -14,10 +14,10 @@ replacement that stays reachable over Tailscale. # Nix nix run github:ananthb/xmorph -- --help -# Static binary (each release ships SHA256SUMS) -curl -LO https://github.com/ananthb/xmorph/releases/latest/download/xmorph-x86_64-linux -chmod +x xmorph-x86_64-linux -sudo mv xmorph-x86_64-linux /usr/local/bin/xmorph +# Static binary (each release ships a SHA256SUMS file alongside the archives) +curl -LO https://github.com/ananthb/xmorph/releases/latest/download/xmorph-x86_64-linux.tar.gz +tar xzf xmorph-x86_64-linux.tar.gz +sudo mv xmorph/xmorph /usr/local/bin/xmorph # From source (Go 1.26+) CGO_ENABLED=0 go build -o xmorph ./cmd/xmorph @@ -30,5 +30,6 @@ reprovisioning recipes at **[ananthb.github.io/xmorph](https://ananthb.github.io - [Rescue an unreachable host](docs/rescue.md) - [Reprovision to Flatcar / FCOS / Alpine / NixOS / Ubuntu](docs/reprovision/) +- [Testing (unit, NixOS VM, real-kernel dev loop)](docs/testing.md) Licensed under the [AGPL-3.0](LICENSE). diff --git a/docs/testing.md b/docs/testing.md new file mode 100644 index 0000000..2b8662e --- /dev/null +++ b/docs/testing.md @@ -0,0 +1,65 @@ +# Testing + +xmorph has three tiers of tests, matching how much of a real Linux kernel +each one needs. + +## 1. Unit tests + lint (any OS) + +```sh +nix run .#build # gofmt + go vet (GOOS=linux) + go test +``` + +On Linux this runs `go test ./...`; on macOS it runs the pure-Go tests +natively and cross-compiles the rest. This is what the `build` CI job runs. + +Tests that need root and mount namespaces (`internal/pivot`, and the +as-root tar-extraction cases in `internal/oci`) are guarded with +`if os.Geteuid() != 0 { t.Skip(...) }`, so they skip here rather than fail. + +## 2. Integration tests on a real kernel (CI) + +The pivot path — `pivot_root(2)`, mount ordering, tar extraction running as +root — can only be exercised against a real kernel with `CAP_SYS_ADMIN`. +The nix build sandbox can't do mounts, so the integration test binaries are +compiled by the `xmorphIntegrationTests` derivation and run inside a NixOS +VM test: + +```sh +nix build -L .#checks.x86_64-linux.nixos-pivot # pivot_root + extraction +nix flake check # all checks + every VM test +``` + +The `integration` CI job runs `nixos-pivot` on every push and PR (GitHub's +Linux runners expose `/dev/kvm`). This is the project's real-kernel +regression coverage — notably it guards the mount-ordering invariant that +`/proc`, `/sys`, and `/dev` stay visible after the pivot. + +## 3. Fast real-kernel iteration on macOS (developer option) + +NixOS VM tests are the source of truth, but they're slow to iterate on. On +Apple Silicon you can run the same Linux tests directly against a real +kernel using [Apple `container`](https://github.com/apple/container), which +puts each container in its own lightweight VM. + +One-time setup: + +```sh +container system start +container system kernel set --recommended +``` + +Then run any package's tests as root with mount capabilities: + +```sh +container run --rm --cap-add ALL \ + -v "$PWD:/src" \ + -v "$HOME/go/pkg/mod:/go/pkg/mod:ro" \ + -w /src \ + docker.io/library/golang:1.26 \ + sh -c 'GOFLAGS=-mod=mod GOPROXY=off go test -race ./...' +``` + +`--cap-add ALL` grants the `CAP_SYS_ADMIN` that `unshare`/`pivot_root` +require; mounting `$HOME/go/pkg/mod` read-only reuses your module cache so +the run is offline and fast. This is a convenience for local development — +CI relies on the NixOS VM test in tier 2, not on `container`. diff --git a/flake.nix b/flake.nix index 720648b..e116781 100644 --- a/flake.nix +++ b/flake.nix @@ -58,6 +58,32 @@ }; }; + # Integration test binaries that need a real kernel + root to run + # (mount namespaces, pivot_root). They are compiled here but executed + # inside the NixOS VM test below — the nix build sandbox can't do + # mounts, and GitHub CI has no Apple `container`. Developers on macOS + # can run the same binaries under `container`; see docs/testing.md. + xmorphIntegrationTests = buildGoModule { + pname = "xmorph-integration-tests"; + inherit version vendorHash; + src = ./.; + env.CGO_ENABLED = "0"; + doCheck = false; + buildPhase = '' + runHook preBuild + for pkg in pivot oci; do + go test -c -o "xmorph-$pkg.test" "./internal/$pkg" + done + runHook postBuild + ''; + installPhase = '' + runHook preInstall + mkdir -p "$out/bin" + install -m555 xmorph-*.test "$out/bin/" + runHook postInstall + ''; + }; + # xmorphLint: gofmt + go vet gate. Exposed as `apps.lint` so the # pre-commit hook and CI both go through `nix run .#lint`. xmorphLint = pkgs.writeShellApplication { @@ -201,6 +227,29 @@ ''; }; + # NixOS VM test: real pivot_root + mount ordering on a live kernel. + # Runs the pivot/oci integration test binaries as root inside the VM + # — this is the project's real-kernel coverage of the pivot path + # (the nix sandbox can't do mount namespaces). Guards the + # mount-ordering fix: essentials must stay visible after the pivot. + nixos-pivot = pkgs.testers.nixosTest { + name = "xmorph-pivot-integration"; + + nodes.machine = { ... }: { + environment.systemPackages = [ xmorphIntegrationTests ]; + virtualisation.memorySize = 2048; + }; + + testScript = '' + machine.wait_for_unit("multi-user.target") + # pivot_root + mount-ordering (needs root + CAP_SYS_ADMIN). + machine.succeed("xmorph-pivot.test -test.v") + # tar-extraction containment + setuid, exercised as root — the + # privilege level at which a symlink escape would actually bite. + machine.succeed("xmorph-oci.test -test.v") + ''; + }; + # NixOS VM test: headscale integration (offline, ~2-3 min) nixos-headscale = import ./nix/tests/headscale.nix { inherit pkgs; diff --git a/internal/cli/pivot.go b/internal/cli/pivot.go index ae5bb0b..250942c 100644 --- a/internal/cli/pivot.go +++ b/internal/cli/pivot.go @@ -6,8 +6,10 @@ import ( "fmt" "log/slog" "os" + "os/signal" "path/filepath" "runtime" + "syscall" "time" "github.com/ananthb/xmorph/internal/config" @@ -135,6 +137,18 @@ func runPivot(ctx context.Context, cfg *config.Config, stdout interface { } } + // From here on a dropped controlling session must not take xmorph with + // it. The transient-scope relocation above only fixes cgroup reaping; it + // does NOT detach the controlling TTY, so when the pivot severs the SSH + // connection (or init coordination stops sshd) the kernel delivers SIGHUP + // to this process group, whose default disposition is fatal. SIGPIPE is + // the same story for a slog write to a stderr fd whose reader just died. + // Ignore both now — we have already committed to proceeding (the SSH + // grace window above was the last chance to abort). SIGINT/SIGTERM are + // left alone until the destructive steps below so local Ctrl-C still + // aborts a long rootfs build. + signal.Ignore(syscall.SIGHUP, syscall.SIGPIPE) + if err := os.MkdirAll(cfg.WorkDir, 0o755); err != nil { return fmt.Errorf("create work dir: %w", err) } @@ -207,6 +221,12 @@ func runPivot(ctx context.Context, cfg *config.Config, stdout interface { // surfaced via the slog output — for now we just verify the file // is readable.) + // Point of no safe abort: everything below stops services, terminates + // userspace, and pivots. A stray Ctrl-C or a SIGTERM from a supervising + // unit here would strand the machine with services down and no new root, + // so stop honoring them now (SIGHUP/SIGPIPE were already ignored above). + signal.Ignore(syscall.SIGINT, syscall.SIGTERM) + if !cfg.SystemdMode { if !cfg.NoInitCoord && !initsys.ShouldSkipCoordination() { coord := initsys.NewCoordinator(time.Duration(cfg.Timeout) * time.Second) diff --git a/internal/oci/extract.go b/internal/oci/extract.go index 5fbd644..3758f6d 100644 --- a/internal/oci/extract.go +++ b/internal/oci/extract.go @@ -86,6 +86,15 @@ func ExtractTarball(path, targetDir string) error { // extractTar walks a tar stream and writes entries into targetDir, // resolving .wh.* whiteout markers against what's already on disk. +// +// Extraction runs as root into what becomes the live root filesystem, so +// containment is a security boundary, not a nicety: every path is resolved +// through secureJoin, which follows the symlinks earlier layers legitimately +// plant (e.g. usr-merge's /bin -> usr/bin) but clamps them inside targetDir, +// so a hostile layer cannot redirect a write or delete onto the host via an +// absolute/".." symlink. The final path component is always joined literally +// and any pre-existing entry there is unlinked (never followed) before we +// write, so a planted symlink at the target itself is replaced, not traversed. func extractTar(tr *tar.Reader, targetDir string) error { for { h, err := tr.Next() @@ -104,46 +113,59 @@ func extractTar(tr *tar.Reader, targetDir string) error { name = strings.TrimPrefix(name, "/") name = strings.TrimPrefix(name, "./") - // Reject path traversal. filepath.Clean has already normalized - // "../" but a leading "../" can still appear. - if strings.HasPrefix(name, "..") || strings.Contains(name, "/../") { + // Defense in depth: reject obvious traversal in the entry name. + // secureJoin below is the real guard (it also contains symlinked + // parents), but this rejects the blatant case early and clearly. + if name == ".." || strings.HasPrefix(name, "../") || strings.Contains(name, "/../") { return fmt.Errorf("tar entry %q escapes target", h.Name) } dir, base := filepath.Split(name) dir = strings.TrimSuffix(dir, "/") + // Resolve the entry's PARENT directory inside targetDir, following + // symlinked parents but clamping them to the root. + parent, err := secureJoin(targetDir, dir) + if err != nil { + return fmt.Errorf("resolve %q: %w", h.Name, err) + } + // Whiteout handling. if base == whiteoutOpaque { - parent := filepath.Join(targetDir, dir) if err := clearDirContents(parent); err != nil && !errors.Is(err, os.ErrNotExist) { return fmt.Errorf("opaque whiteout %s: %w", h.Name, err) } continue } if strings.HasPrefix(base, whiteoutPrefix) { - target := filepath.Join(targetDir, dir, strings.TrimPrefix(base, whiteoutPrefix)) + target := filepath.Join(parent, strings.TrimPrefix(base, whiteoutPrefix)) if err := os.RemoveAll(target); err != nil { return fmt.Errorf("whiteout %s: %w", h.Name, err) } continue } - target := filepath.Join(targetDir, name) + if err := ensureDir(parent); err != nil { + return err + } + target := filepath.Join(parent, base) switch h.Typeflag { case tar.TypeDir: - if err := os.MkdirAll(target, os.FileMode(h.Mode)&os.ModePerm); err != nil { + if err := os.MkdirAll(target, 0o755); err != nil { return fmt.Errorf("mkdir %s: %w", target, err) } - - case tar.TypeReg, tar.TypeRegA: - if err := ensureDir(filepath.Dir(target)); err != nil { + if err := chmodEntry(target, h); err != nil { return err } - // Remove any existing entry — later layers always win. + + case tar.TypeReg, tar.TypeRegA: + // Remove any existing entry first — later layers always win, and + // RemoveAll unlinks a planted symlink here rather than following + // it. O_EXCL then guarantees we create a fresh file, never open + // through a link. Mode is tightened later by chmodEntry. _ = os.RemoveAll(target) - f, err := os.OpenFile(target, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.FileMode(h.Mode)&os.ModePerm) + f, err := os.OpenFile(target, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) if err != nil { return fmt.Errorf("create %s: %w", target, err) } @@ -154,22 +176,25 @@ func extractTar(tr *tar.Reader, targetDir string) error { if err := f.Close(); err != nil { return fmt.Errorf("close %s: %w", target, err) } - - case tar.TypeSymlink: - if err := ensureDir(filepath.Dir(target)); err != nil { + if err := chmodEntry(target, h); err != nil { return err } + + case tar.TypeSymlink: _ = os.RemoveAll(target) if err := os.Symlink(h.Linkname, target); err != nil { return fmt.Errorf("symlink %s -> %s: %w", target, h.Linkname, err) } case tar.TypeLink: - if err := ensureDir(filepath.Dir(target)); err != nil { - return err - } _ = os.RemoveAll(target) - source := filepath.Join(targetDir, filepath.Clean(h.Linkname)) + // The hardlink source must also stay inside targetDir — otherwise + // a layer could hardlink a host file (e.g. /etc/shadow) into the + // rootfs and expose its contents. secureJoin clamps it. + source, err := secureJoin(targetDir, h.Linkname) + if err != nil { + return fmt.Errorf("resolve hardlink target %q: %w", h.Linkname, err) + } if err := os.Link(source, target); err != nil { return fmt.Errorf("hardlink %s -> %s: %w", target, source, err) } @@ -208,6 +233,94 @@ func ensureDir(dir string) error { return nil } +// secureJoin resolves unsafePath under root and returns the absolute path, +// following symlinks encountered along the way but treating root as "/" for +// them — an absolute symlink target is re-rooted at root and a "../" can +// never climb above root. This is the containment primitive that keeps a +// hostile tar layer from redirecting a write/delete onto the host through a +// symlink an earlier entry planted. Modeled on github.com/cyphar/filepath-securejoin. +// +// unsafePath is a slash-separated path relative to root (tar entry names +// always use "/"). The returned path may point at something that does not +// exist yet; non-existent components are taken literally. +func secureJoin(root, unsafePath string) (string, error) { + const maxLinks = 255 + linksWalked := 0 + // resolved is the portion already resolved, relative to root and always + // kept inside it. path is what remains to process. + resolved := "" + path := unsafePath + for path != "" { + var part string + if i := strings.IndexByte(path, '/'); i >= 0 { + part, path = path[:i], path[i+1:] + } else { + part, path = path, "" + } + switch part { + case "", ".": + continue + case "..": + resolved = filepath.Dir(resolved) + if resolved == "." || resolved == string(filepath.Separator) { + resolved = "" + } + continue + } + + next := filepath.Join(resolved, part) + full := filepath.Join(root, next) + fi, err := os.Lstat(full) + if err != nil { + if os.IsNotExist(err) { + // Nothing here yet — take it literally and keep going; any + // remaining components resolve underneath it. + resolved = next + continue + } + return "", err + } + if fi.Mode()&os.ModeSymlink == 0 { + resolved = next + continue + } + + linksWalked++ + if linksWalked > maxLinks { + return "", fmt.Errorf("too many symlinks while resolving %q", unsafePath) + } + dest, err := os.Readlink(full) + if err != nil { + return "", err + } + if filepath.IsAbs(dest) { + // Absolute link target is interpreted relative to root. + resolved = "" + path = filepath.Join(dest, path) + } else { + // Relative link target resolves against the link's parent dir, + // which is the current `resolved` (part was appended to it). + path = filepath.Join(dest, path) + } + } + return filepath.Join(root, resolved), nil +} + +// chmodEntry sets target's permission bits from the tar header, including the +// setuid/setgid/sticky bits. Extraction creates files/dirs with a tight base +// mode, so this restores the real mode (undoing both the ModePerm truncation +// and the umask narrowing that OpenFile/Mkdir would otherwise impose) — a +// setuid /usr/bin/sudo in the image stays setuid in the built rootfs. +func chmodEntry(target string, h *tar.Header) error { + fm := h.FileInfo().Mode() + perm := fm.Perm() + perm |= fm & (os.ModeSetuid | os.ModeSetgid | os.ModeSticky) + if err := os.Chmod(target, perm); err != nil { + return fmt.Errorf("chmod %s: %w", target, err) + } + return nil +} + // clearDirContents removes all children of dir but leaves dir itself. // Used for opaque-whiteout markers. func clearDirContents(dir string) error { diff --git a/internal/oci/extract_test.go b/internal/oci/extract_test.go index 55a0d9c..dbb3e50 100644 --- a/internal/oci/extract_test.go +++ b/internal/oci/extract_test.go @@ -153,6 +153,93 @@ func TestExtractTarPathTraversal(t *testing.T) { } } +// TestExtractTarSymlinkEscape verifies that a layer which plants a symlink +// pointing outside targetDir cannot then write (or delete) through it onto +// the host: the write must be contained inside targetDir. +func TestExtractTarSymlinkEscape(t *testing.T) { + root := t.TempDir() + outside := t.TempDir() + victim := filepath.Join(outside, "victim") + if err := os.WriteFile(victim, []byte("original"), 0o644); err != nil { + t.Fatal(err) + } + target := filepath.Join(root, "target") + if err := os.Mkdir(target, 0o755); err != nil { + t.Fatal(err) + } + + // Entry A plants an absolute symlink escape -> ; entry B tries + // to write escape/victim, which naively would clobber /victim. + data := makeTar(t, []tarEntry{ + {name: "escape", typ: tar.TypeSymlink, linkname: outside}, + {name: "escape/victim", typ: tar.TypeReg, mode: 0o644, body: []byte("pwned")}, + }) + if err := extractTar(tar.NewReader(bytes.NewReader(data)), target); err != nil { + t.Fatalf("extractTar: %v", err) + } + + if b, err := os.ReadFile(victim); err != nil || string(b) != "original" { + t.Errorf("host file escaped containment: %q err=%v", b, err) + } + // The write should have landed inside the target instead. + if b, err := os.ReadFile(filepath.Join(target, outside, "victim")); err != nil || string(b) != "pwned" { + t.Errorf("contained write missing: %q err=%v", b, err) + } +} + +// TestExtractTarHardlinkEscape verifies a layer cannot hardlink a host file +// into the rootfs (which would expose its contents). +func TestExtractTarHardlinkEscape(t *testing.T) { + root := t.TempDir() + outside := t.TempDir() + secret := filepath.Join(outside, "secret") + if err := os.WriteFile(secret, []byte("s3cr3t"), 0o600); err != nil { + t.Fatal(err) + } + data := makeTar(t, []tarEntry{ + {name: "leak", typ: tar.TypeLink, linkname: "../../../../../../" + secret}, + }) + // Either the link is refused, or it is contained inside root — never a + // link to the host secret. + _ = extractTar(tar.NewReader(bytes.NewReader(data)), root) + if fi, err := os.Lstat(filepath.Join(root, "leak")); err == nil { + if os.SameFile(fileInfoOf(t, secret), fi) { + t.Errorf("hardlink escaped to host secret") + } + } +} + +func fileInfoOf(t *testing.T, p string) os.FileInfo { + t.Helper() + fi, err := os.Stat(p) + if err != nil { + t.Fatal(err) + } + return fi +} + +// TestExtractTarPreservesSetuid checks that the setuid bit survives +// extraction (it must, or privileged binaries in the rootfs break). +func TestExtractTarPreservesSetuid(t *testing.T) { + dir := t.TempDir() + data := makeTar(t, []tarEntry{ + {name: "usr/bin/sudo", typ: tar.TypeReg, mode: 0o4755, body: []byte("elf")}, + }) + if err := extractTar(tar.NewReader(bytes.NewReader(data)), dir); err != nil { + t.Fatalf("extractTar: %v", err) + } + fi, err := os.Stat(filepath.Join(dir, "usr/bin/sudo")) + if err != nil { + t.Fatal(err) + } + if fi.Mode()&fs.ModeSetuid == 0 { + t.Errorf("setuid bit lost: mode=%v", fi.Mode()) + } + if fi.Mode().Perm() != 0o755 { + t.Errorf("perm = %o, want 0755", fi.Mode().Perm()) + } +} + func TestExtractTarLaterLayerWins(t *testing.T) { dir := t.TempDir() layer1 := makeTar(t, []tarEntry{ diff --git a/internal/pivot/pivot.go b/internal/pivot/pivot.go index 75ebfe9..0ba418e 100644 --- a/internal/pivot/pivot.go +++ b/internal/pivot/pivot.go @@ -50,13 +50,15 @@ func CreateMountNamespace() error { return nil } -// EnsureMountPoint bind-mounts target onto itself so pivot_root sees it -// as a "real" mount point (a kernel requirement). Idempotent: if target -// is already a mount, the bind is a no-op. +// EnsureMountPoint bind-mounts target onto itself so pivot_root sees it as a +// "real" mount point (a kernel requirement). Note this is NOT idempotent: a +// non-recursive self-bind always stacks a fresh mount over target that does +// not carry target's existing child mounts, so it must be done BEFORE any +// submounts (/proc, /sys, /dev) are established under target — otherwise it +// shadows them. Callers should invoke this exactly once, before populating +// target. See Prepare. func EnsureMountPoint(target string) error { if err := unix.Mount(target, target, "", unix.MS_BIND, ""); err != nil { - // EINVAL/EBUSY here usually means "already a mount" — let the - // caller decide whether to treat as fatal. return err } return nil @@ -94,15 +96,18 @@ func SetupEssentials(newRoot string) error { // PivotRoot executes the sequence that swaps the process's rootfs: // // 1. MakePrivate("/") -// 2. EnsureMountPoint(newRoot) + MakePrivate(newRoot) -// 3. pivot_root(newRoot, newRoot/oldRootMount) -// 4. chdir("/") +// 2. pivot_root(newRoot, newRoot/oldRootMount) +// 3. chdir("/") // -// EssentialMounts must already be set up by the caller (e.g. via -// SetupEssentials). After return, "/" is newRoot and the old root is -// at "/" + oldRootMount. +// The caller MUST have already run Prepare (or otherwise self-bound newRoot +// and set up EssentialMounts). PivotRoot deliberately does NOT re-bind +// newRoot onto itself: a second non-recursive self-bind here would stack a +// fresh mount over newRoot that shadows the /proc, /sys, and /dev mounts +// Prepare put there, so the pivoted system would boot without them. After +// return, "/" is newRoot and the old root is at "/" + oldRootMount. func PivotRoot(newRoot, oldRootMount string) error { - // Step 1. + // Step 1. Ensure "/" is private so pivot_root's mount changes don't + // propagate to peer namespaces. Idempotent with Prepare's rec-private. if err := MakePrivate("/"); err != nil { // MS_PRIVATE on "/" can fail in environments where "/" isn't // actually a mount (some containers). Log via return — caller @@ -110,16 +115,7 @@ func PivotRoot(newRoot, oldRootMount string) error { return fmt.Errorf("make / private: %w", err) } - // Step 2. - if err := EnsureMountPoint(newRoot); err != nil { - // If already a mount point, bind is harmless; continue. - _ = err - } - if err := MakePrivate(newRoot); err != nil { - return fmt.Errorf("make new root private: %w", err) - } - - // Step 3. pivot_root requires the old-root path to exist *inside* + // Step 2. pivot_root requires the old-root path to exist *inside* // new_root; the caller should have called ensureDir(newRoot + "/" + oldRootMount) // before now. putOld := joinPath(newRoot, oldRootMount) @@ -127,7 +123,7 @@ func PivotRoot(newRoot, oldRootMount string) error { return fmt.Errorf("pivot_root(%s, %s): %w", newRoot, putOld, err) } - // Step 4. + // Step 3. if err := unix.Chdir("/"); err != nil { return fmt.Errorf("chdir(/): %w", err) } diff --git a/internal/pivot/prepare.go b/internal/pivot/prepare.go index 077f31e..88c820c 100644 --- a/internal/pivot/prepare.go +++ b/internal/pivot/prepare.go @@ -33,15 +33,23 @@ func Prepare(opts PrepareOptions) error { return err } } - if err := SetupEssentials(opts.NewRoot); err != nil { - return fmt.Errorf("setup essentials: %w", err) + // Make newRoot a real mount point FIRST, then mount the essentials on + // top of it. Order matters: a non-recursive self-bind stacks a fresh + // mount over newRoot that does not carry child mounts, so if the + // essentials were mounted first they would be shadowed by the self-bind + // and the pivoted system would come up with no /proc, /sys, or /dev. + // (This is why pivot_root recipes always bind the new root onto itself + // before populating it.) pivot_root also requires newRoot to be a mount + // point, which this satisfies. + if err := EnsureMountPoint(opts.NewRoot); err != nil { + return fmt.Errorf("bind new root onto itself: %w", err) } - // Self-bind so pivot_root sees newRoot as a real mount point. The - // kernel rejects pivot_root unless this holds. - _ = EnsureMountPoint(opts.NewRoot) // tolerated: already-mounted is OK if err := MakePrivate(opts.NewRoot); err != nil { return fmt.Errorf("make new root private: %w", err) } + if err := SetupEssentials(opts.NewRoot); err != nil { + return fmt.Errorf("setup essentials: %w", err) + } return nil } diff --git a/internal/pivot/prepare_linux_test.go b/internal/pivot/prepare_linux_test.go new file mode 100644 index 0000000..708a108 --- /dev/null +++ b/internal/pivot/prepare_linux_test.go @@ -0,0 +1,176 @@ +//go:build linux + +package pivot + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "testing" + + "golang.org/x/sys/unix" +) + +// pivotChildEnv, when set, tells TestPivotRootChild to actually perform the +// pivot (it is otherwise a no-op that a normal `go test` run skips). Its +// value is the path to the new root. +const pivotChildEnv = "XMORPH_TEST_PIVOT_ROOT" + +// minimalRoot creates a directory that looks enough like a rootfs for +// Prepare to mount the essentials into it. +func minimalRoot(t *testing.T) string { + t.Helper() + root := t.TempDir() + for _, d := range []string{"bin", "lib"} { + if err := os.MkdirAll(filepath.Join(root, d), 0o755); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(root, "bin", "sh"), []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + return root +} + +// inNewMountNS runs fn on a dedicated OS thread that has its own mount +// namespace, so the mounts fn makes don't leak into the rest of the test +// binary. The thread is never unlocked, so it is discarded (along with its +// namespace) when the goroutine returns. preUnshare=false lets fn do its +// own unshare (e.g. via Prepare). +func inNewMountNS(preUnshare bool, fn func() error) error { + errc := make(chan error, 1) + go func() { + runtime.LockOSThread() // intentionally never unlocked + errc <- func() error { + if preUnshare { + if err := unix.Unshare(unix.CLONE_NEWNS); err != nil { + return fmt.Errorf("unshare: %w", err) + } + if err := MakePrivate("/"); err != nil { + return err + } + } + return fn() + }() + }() + return <-errc +} + +func mountsVisible(newRoot string) error { + // If /proc, /sys, /dev are actually mounted AND not shadowed, these + // paths (which only exist inside the respective mounts) resolve. + for _, p := range []string{"proc/self", "sys/kernel", "dev/null"} { + if _, err := os.Stat(filepath.Join(newRoot, p)); err != nil { + return fmt.Errorf("%s not visible under new root: %w", p, err) + } + } + return nil +} + +// TestPrepareEssentialsVisible is the regression test for the mount-ordering +// bug: the new root must be self-bound BEFORE the essentials are mounted, +// otherwise the non-recursive self-bind stacks a mount that shadows /proc, +// /sys, and /dev and the pivoted system comes up without them. +func TestPrepareEssentialsVisible(t *testing.T) { + if os.Geteuid() != 0 { + t.Skip("needs root + CAP_SYS_ADMIN for mount namespaces") + } + newRoot := minimalRoot(t) + err := inNewMountNS(false, func() error { + if err := Prepare(PrepareOptions{ + NewRoot: newRoot, + SkipVerify: true, + CreateNamespace: true, + }); err != nil { + return fmt.Errorf("Prepare: %w", err) + } + return mountsVisible(newRoot) + }) + if err != nil { + t.Fatal(err) + } +} + +// TestPivotRootEndToEnd performs a real pivot_root and asserts /proc, /sys, +// and /dev are mounted and usable *after* the swap — the full end-to-end +// version of the mount-ordering regression. It runs the pivot in a separate +// child process (re-execing this test binary) so the swap can't disturb the +// test driver's own root. +func TestPivotRootEndToEnd(t *testing.T) { + if os.Geteuid() != 0 { + t.Skip("needs root + CAP_SYS_ADMIN for pivot_root") + } + root := minimalRoot(t) + if err := os.MkdirAll(filepath.Join(root, "mnt", "oldroot"), 0o755); err != nil { + t.Fatal(err) + } + exe, err := os.Executable() + if err != nil { + t.Fatal(err) + } + cmd := exec.Command(exe, "-test.run", "TestPivotRootChild", "-test.v") + cmd.Env = append(os.Environ(), pivotChildEnv+"="+root) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("pivot child failed: %v\n%s", err, out) + } +} + +// TestPivotRootChild is the child half of TestPivotRootEndToEnd: it prepares +// and pivots into the root passed via pivotChildEnv, then verifies the +// essentials survived. Without the env var it is skipped, so it stays inert +// during a normal test run. +func TestPivotRootChild(t *testing.T) { + root := os.Getenv(pivotChildEnv) + if root == "" { + t.Skip("child half of TestPivotRootEndToEnd") + } + runtime.LockOSThread() // never unlocked: this process pivots and exits + if err := Prepare(PrepareOptions{ + NewRoot: root, + SkipVerify: true, + CreateNamespace: true, + }); err != nil { + t.Fatalf("Prepare: %v", err) + } + if err := PivotRoot(root, "mnt/oldroot"); err != nil { + t.Fatalf("PivotRoot: %v", err) + } + // "/" is now the pivoted root; the essentials must be present and usable. + for _, p := range []string{"/proc/self/status", "/sys/kernel", "/dev/null"} { + if _, err := os.Stat(p); err != nil { + t.Fatalf("%s missing after pivot_root: %v", p, err) + } + } +} + +// TestBuggyOrderShadowsEssentials documents *why* the order matters: mounting +// the essentials first and self-binding afterwards (the old sequence) shadows +// /proc, so proc/self is no longer reachable under the new root. If this ever +// stops holding, the ordering constraint in Prepare/PivotRoot can be relaxed. +func TestBuggyOrderShadowsEssentials(t *testing.T) { + if os.Geteuid() != 0 { + t.Skip("needs root + CAP_SYS_ADMIN for mount namespaces") + } + newRoot := minimalRoot(t) + err := inNewMountNS(true, func() error { + // Old (buggy) order: essentials, THEN self-bind. + if err := SetupEssentials(newRoot); err != nil { + return fmt.Errorf("SetupEssentials: %w", err) + } + if err := EnsureMountPoint(newRoot); err != nil { + return fmt.Errorf("EnsureMountPoint: %w", err) + } + if err := MakePrivate(newRoot); err != nil { + return err + } + if _, err := os.Stat(filepath.Join(newRoot, "proc/self")); err == nil { + return fmt.Errorf("expected the buggy order to shadow /proc, but proc/self is visible") + } + return nil + }) + if err != nil { + t.Fatal(err) + } +} diff --git a/internal/postpivot/configwrite.go b/internal/postpivot/configwrite.go index 8c30a07..00c4943 100644 --- a/internal/postpivot/configwrite.go +++ b/internal/postpivot/configwrite.go @@ -64,7 +64,13 @@ func WriteConfig(rootfsRoot string, cfg *Config) error { if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { return fmt.Errorf("mkdir for config: %w", err) } - f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644) + // 0600: this file holds the SSH root password and Tailscale auth key, so + // it must not be readable by unprivileged processes the entrypoint spawns. + // Run also unlinks it once loaded. O_EXCL after a best-effort remove keeps + // a pre-existing (possibly attacker-planted, wrong-mode) file from being + // reused. Mirrors the secret-handling intent of the pre-pivot staging. + _ = os.Remove(path) + f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) if err != nil { return fmt.Errorf("open config: %w", err) } diff --git a/internal/postpivot/run.go b/internal/postpivot/run.go index fa82fca..2f939d3 100644 --- a/internal/postpivot/run.go +++ b/internal/postpivot/run.go @@ -32,6 +32,12 @@ func Run(argv []string) int { if err != nil { fmt.Fprintf(os.Stderr, "xmorph --init: warning: %v\n", err) } + // The config carries the SSH password and Tailscale auth key; now that + // it's in memory, remove it from the rootfs so it doesn't linger + // world-visible for the life of the pivoted system. + if err := os.Remove(ConfigPath); err != nil && !os.IsNotExist(err) { + slog.Warn("could not remove init config after load", "path", ConfigPath, "err", err) + } if cfg != nil && cfg.FlushFirewall { FlushFirewall() diff --git a/nix/module.nix b/nix/module.nix index 66f10a9..35481d8 100644 --- a/nix/module.nix +++ b/nix/module.nix @@ -3,35 +3,49 @@ let cfg = config.services.xmorph; - # Common layer args shared between pivot and build - layerArgs = - (map (img: "--image ${img}") cfg.images) - ++ (map (r: "--rootfs ${r}") cfg.rootfs) + # Args accepted by BOTH `build` and `pivot`. SSH flags are deliberately + # NOT here: the build subcommand doesn't bind them (`xmorph build --ssh.enable` + # errors with "unknown flag"), so they must go to `pivot` only. + commonArgs = + (map (img: "--image=${img}") cfg.images) + ++ (map (r: "--rootfs=${r}") cfg.rootfs) ++ lib.optional cfg.tailscale.enable "--tailscale.enable" - ++ lib.optional (cfg.tailscale.enable && cfg.tailscale.authKeyFile != null) - "--tailscale.authkey=$(cat ${cfg.tailscale.authKeyFile})" ++ lib.optional (cfg.tailscale.enable && cfg.tailscale.server != null) "--tailscale.server=${cfg.tailscale.server}" - ++ lib.optional (cfg.tailscale.enable && cfg.tailscale.args != null) "--tailscale.args='${cfg.tailscale.args}'" - ++ lib.optional cfg.ssh.enable "--ssh.enable" + ++ lib.optional (cfg.tailscale.enable && cfg.tailscale.args != null) "--tailscale.args=${cfg.tailscale.args}" + ++ lib.optional cfg.verbose "--verbose"; + + # SSH args — pivot only. + sshArgs = + lib.optional cfg.ssh.enable "--ssh.enable" ++ lib.optional (cfg.ssh.enable && cfg.ssh.port != null) "--ssh.port=${toString cfg.ssh.port}" ++ lib.optional (cfg.ssh.enable && cfg.ssh.password != null) "--ssh.password=${cfg.ssh.password}" - ++ lib.optional (cfg.ssh.enable && cfg.ssh.authorizedKeys != null) "--ssh.authorized-keys='${cfg.ssh.authorizedKeys}'" - ++ lib.optional cfg.verbose "--verbose"; + ++ lib.optional (cfg.ssh.enable && cfg.ssh.authorizedKeys != null) "--ssh.authorized-keys=${cfg.ssh.authorizedKeys}"; - # Build command for cache pre-warming (no output, just cache) - xmorphBuildArgs = lib.concatStringsSep " " ( - [ "build" ] ++ layerArgs - ); + buildArgs = [ "build" ] ++ commonArgs; - # Pivot command - xmorphArgs = lib.concatStringsSep " " ( + pivotArgs = [ "pivot" "--systemd-mode" "--force" ] - ++ layerArgs - ++ lib.optional (cfg.entrypoint != null) "--entrypoint ${cfg.entrypoint}" - ++ (map (c: "--command ${c}") cfg.command) - ++ lib.optional (cfg.workDir != null) "--work-dir ${cfg.workDir}" - ++ lib.optional (cfg.logDir != null) "--log-dir ${cfg.logDir}" - ); + ++ commonArgs + ++ sshArgs + ++ lib.optional (cfg.entrypoint != null) "--entrypoint=${cfg.entrypoint}" + ++ (map (c: "--command=${c}") cfg.command) + ++ lib.optional (cfg.workDir != null) "--work-dir=${cfg.workDir}" + ++ lib.optional (cfg.logDir != null) "--log-dir=${cfg.logDir}"; + + needAuthKey = cfg.tailscale.enable && cfg.tailscale.authKeyFile != null; + + # systemd's ExecStart is not a shell, so `--tailscale.authkey=$(cat ...)` + # would be passed literally. Wrap the invocation in a small script that + # reads the key file at runtime (keeping the secret out of the nix store) + # and appends it as a real argument. + mkExecStart = name: baseArgs: pkgs.writeShellScript name '' + set -euo pipefail + ${lib.optionalString needAuthKey + ''authkey="$(cat ${lib.escapeShellArg (toString cfg.tailscale.authKeyFile)})"''} + exec ${cfg.package}/bin/xmorph ${lib.escapeShellArgs baseArgs} ${ + lib.optionalString needAuthKey ''--tailscale.authkey="$authkey"'' + } + ''; in { options.services.xmorph = { @@ -152,7 +166,7 @@ in Type = "oneshot"; RemainAfterExit = true; - ExecStart = "${cfg.package}/bin/xmorph ${xmorphArgs}"; + ExecStart = mkExecStart "xmorph-pivot-start" pivotArgs; CacheDirectory = "xmorph"; RuntimeDirectory = "xmorph"; @@ -176,7 +190,7 @@ in serviceConfig = { Type = "oneshot"; RemainAfterExit = true; - ExecStart = "${cfg.package}/bin/xmorph ${xmorphBuildArgs}"; + ExecStart = mkExecStart "xmorph-cache-warm-start" buildArgs; CacheDirectory = "xmorph"; RuntimeDirectory = "xmorph"; TimeoutStartSec = "infinity";