diff --git a/cmd/felis/initforwarding.go b/cmd/felis/initforwarding.go index 1f60c2a..8445365 100644 --- a/cmd/felis/initforwarding.go +++ b/cmd/felis/initforwarding.go @@ -20,18 +20,14 @@ const forwardingSecretEnv = "FELIS_FORWARDING_SECRET" // config/ and server.properties live under it. const defaultForwardingDataDir = "/data" -// fwd*Mode make the written config readable AND rewritable by the main server -// container, whose UID we do not control (an arbitrary user image). The -// initContainer runs as root (see buildStatefulSet) so it can write into a data -// volume of unknown ownership; 0666/0777 then let a non-root Paper rewrite the -// same files on boot. -// -// This relies on the initContainer running as root to write into a volume of -// unknown ownership; that is how the operator schedules it. If that ever changes, -// give the server pod an fsGroup so the shared volume is group-writable instead. +// fwd*Mode are the modes the written config lands with. The initContainer runs as +// the same uid as the server container (naming.GameUID, pinned by the operator in +// the pod securityContext) after the prepare-data initContainer has handed the +// whole volume to that uid, so owner read/write is all the server needs to rewrite +// these files on boot and nothing else on the node gets write access to them. const ( - fwdFileMode os.FileMode = 0o666 - fwdDirMode os.FileMode = 0o777 + fwdFileMode os.FileMode = 0o644 + fwdDirMode os.FileMode = 0o755 ) // cmdInitForwarding is the felis-image initContainer entrypoint that makes an @@ -96,9 +92,8 @@ func writePaperGlobal(dataDir, secret string) error { if err := os.MkdirAll(dir, fwdDirMode); err != nil { return fmt.Errorf("create %s: %w", dir, err) } - // MkdirAll honours the process umask (root's is typically 022 → 0755); chmod - // does not, and a non-root main container must be able to place/replace the - // file in this directory on boot. + // MkdirAll honours the process umask; chmod does not, so a directory an older + // release left at 0777 is brought back to fwdDirMode here. if err := os.Chmod(dir, fwdDirMode); err != nil { return fmt.Errorf("chmod %s: %w", dir, err) } @@ -188,8 +183,8 @@ func upsertProperty(content []byte, key, value string) []byte { } // writeFileMode writes data then forces the mode, since WriteFile honours the -// umask (root's is typically 022 → 0644) but a non-root main container must be -// able to rewrite these files on boot. +// umask and leaves an existing file's mode alone: a file an older release wrote +// world-writable (0666) is tightened back to fwdFileMode on the next boot. func writeFileMode(path string, data []byte) error { if err := os.WriteFile(path, data, fwdFileMode); err != nil { return fmt.Errorf("write %s: %w", path, err) diff --git a/cmd/felis/initforwarding_test.go b/cmd/felis/initforwarding_test.go index d6671d8..816d6fc 100644 --- a/cmd/felis/initforwarding_test.go +++ b/cmd/felis/initforwarding_test.go @@ -164,8 +164,9 @@ func TestUpsertPropertyAppends(t *testing.T) { } } -// The written files must be group/world writable so a non-root main container can -// rewrite them. chmod semantics are POSIX-only, so this asserts on non-Windows. +// The written files land at fwdFileMode: owner-writable for the game uid the init +// shares with the server container, and no longer world-writable. chmod semantics +// are POSIX-only, so this asserts on non-Windows. func TestWriteForwardingFileModes(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("POSIX file modes not represented on Windows") diff --git a/cmd/felis/initvolume.go b/cmd/felis/initvolume.go new file mode 100644 index 0000000..b004756 --- /dev/null +++ b/cmd/felis/initvolume.go @@ -0,0 +1,117 @@ +package main + +import ( + "errors" + "flag" + "fmt" + "io" + "io/fs" + "os" + "syscall" + + "felis.lolicon.best/internal/naming" +) + +// cmdInitVolume is the felis-image `prepare-data` initContainer entrypoint: it +// hands every entry of a server's world volume to the game uid/gid before the +// server container starts. The operator runs the server itself as naming.GameUID, +// so a world written by an earlier release (whose server ran as root), a restore +// Job (which extracts as root), or a storage provisioner that creates the volume +// root-owned would otherwise leave files the server cannot write — a world that +// boots and then fails every save. +// +// fsGroup covers only part of this: kubelet applies it to volume types that +// support ownership management, and a k3s local-path PV is a hostPath underneath, +// which it skips. A walk from inside the pod works for every volume type. +// +// Only mismatched entries are touched, so a volume already owned by the game uid +// costs one lstat per entry and no writes. The walk runs inside an os.Root at the +// data dir and uses lchown, so a symlink a plugin planted is re-owned as a link +// and never followed out of the volume. +// +// A single entry that cannot be chowned is reported and skipped: failing the pod +// over one odd file would keep the whole server down, while the server itself +// reports the one file it cannot write. Only an unreadable data dir fails. +func cmdInitVolume(args []string, stdout, stderr io.Writer) int { + fs := flag.NewFlagSet("init-volume", flag.ContinueOnError) + fs.SetOutput(stderr) + dataDir := fs.String("data", defaultForwardingDataDir, "world volume mount to hand to the game uid") + uid := fs.Int64("uid", naming.GameUID, "owner uid for every entry") + gid := fs.Int64("gid", naming.GameGID, "owner gid for every entry") + if err := fs.Parse(args); err != nil { + return 2 + } + root, err := os.OpenRoot(*dataDir) + if err != nil { + fmt.Fprintf(stderr, "felis init-volume: open %s: %v\n", *dataDir, err) + return 1 + } + defer root.Close() + res, err := chownTree(root, int(*uid), int(*gid), root.Lchown) + if err != nil { + fmt.Fprintf(stderr, "felis init-volume: %v\n", err) + return 1 + } + for _, f := range res.failures { + fmt.Fprintf(stderr, "felis init-volume: %s\n", f) + } + fmt.Fprintf(stdout, "felis init-volume: %d entries checked, %d handed to %d:%d, %d failed\n", + res.checked, res.changed, *uid, *gid, len(res.failures)) + return 0 +} + +// chownResult tallies one walk; failures is capped so a volume of thousands of +// unownable files cannot flood the pod log. +type chownResult struct { + checked int + changed int + failures []string +} + +const maxReportedChownFailures = 20 + +// chownTree walks root and calls chown on every entry (the root dir included) +// whose owner is not uid:gid. It returns an error only when the root itself +// cannot be read; per-entry failures are collected in the result. +func chownTree(root *os.Root, uid, gid int, chown func(name string, uid, gid int) error) (chownResult, error) { + var res chownResult + fail := func(name string, err error) { + if len(res.failures) < maxReportedChownFailures { + res.failures = append(res.failures, fmt.Sprintf("%s: %v", name, err)) + } else if len(res.failures) == maxReportedChownFailures { + res.failures = append(res.failures, "further failures not listed") + } + } + err := fs.WalkDir(root.FS(), ".", func(name string, d fs.DirEntry, walkErr error) error { + if walkErr != nil { + if name == "." { + return walkErr + } + fail(name, walkErr) + // A directory that cannot be listed is skipped as a whole; a file + // error has nothing below it to skip. + if d != nil && d.IsDir() { + return fs.SkipDir + } + return nil + } + res.checked++ + info, err := d.Info() + if err != nil { + fail(name, err) + return nil + } + if st, ok := info.Sys().(*syscall.Stat_t); ok && int(st.Uid) == uid && int(st.Gid) == gid { + return nil + } + if err := chown(name, uid, gid); err != nil { + if !errors.Is(err, fs.ErrNotExist) { // gone mid-walk: nothing left to own + fail(name, err) + } + return nil + } + res.changed++ + return nil + }) + return res, err +} diff --git a/cmd/felis/initvolume_test.go b/cmd/felis/initvolume_test.go new file mode 100644 index 0000000..8787fde --- /dev/null +++ b/cmd/felis/initvolume_test.go @@ -0,0 +1,124 @@ +package main + +import ( + "bytes" + "os" + "path/filepath" + "runtime" + "slices" + "strconv" + "testing" +) + +// openTree builds a small world under a temp dir: nested dirs, a file, and a +// symlink pointing out of the volume that the walk must not follow. +func openTree(t *testing.T) *os.Root { + t.Helper() + dir := t.TempDir() + for _, d := range []string{"world/region", "plugins"} { + if err := os.MkdirAll(filepath.Join(dir, d), 0o755); err != nil { + t.Fatal(err) + } + } + for _, f := range []string{"level.dat", "world/region/r.0.0.mca"} { + if err := os.WriteFile(filepath.Join(dir, f), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + } + outside := t.TempDir() + if err := os.Symlink(outside, filepath.Join(dir, "plugins", "escape")); err != nil { + t.Fatal(err) + } + root, err := os.OpenRoot(dir) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { root.Close() }) + return root +} + +// Every entry owned by someone else is handed over, the root dir included, and a +// symlink is re-owned as a link rather than walked into. +func TestChownTreeHandsOverMismatchedEntries(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX ownership not represented on Windows") + } + root := openTree(t) + var got []string + res, err := chownTree(root, os.Getuid()+1, os.Getgid(), func(name string, uid, gid int) error { + got = append(got, name) + return nil + }) + if err != nil { + t.Fatalf("chownTree: %v", err) + } + want := []string{".", "level.dat", "plugins", "plugins/escape", "world", "world/region", "world/region/r.0.0.mca"} + slices.Sort(got) + if !slices.Equal(got, want) { + t.Errorf("chowned %v, want %v", got, want) + } + if res.changed != len(want) || res.checked != len(want) || len(res.failures) != 0 { + t.Errorf("result = %+v, want %d checked and changed, no failures", res, len(want)) + } +} + +// A volume already owned by the game uid costs no chown at all: this is the steady +// state every restart after the first one hits. +func TestChownTreeSkipsMatchingOwner(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX ownership not represented on Windows") + } + root := openTree(t) + calls := 0 + res, err := chownTree(root, os.Getuid(), os.Getgid(), func(string, int, int) error { + calls++ + return nil + }) + if err != nil { + t.Fatalf("chownTree: %v", err) + } + if calls != 0 || res.changed != 0 { + t.Errorf("chown called %d times on an already-owned tree (result %+v)", calls, res) + } +} + +// One entry that refuses the chown is reported and the walk carries on: a single +// odd file must not keep the whole server from starting. +func TestChownTreeContinuesPastFailures(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX ownership not represented on Windows") + } + root := openTree(t) + res, err := chownTree(root, os.Getuid()+1, os.Getgid(), func(name string, uid, gid int) error { + if name == "level.dat" { + return os.ErrPermission + } + return nil + }) + if err != nil { + t.Fatalf("chownTree: %v", err) + } + if len(res.failures) != 1 || res.changed != 6 { + t.Errorf("result = %+v, want 1 failure and 6 changed", res) + } +} + +// Against a real directory the owner already matches, so the command succeeds +// without needing CAP_CHOWN — the path every test runner (non-root) can take. +func TestCmdInitVolumeOwnedTree(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX ownership not represented on Windows") + } + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "server.properties"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + var out, errb bytes.Buffer + code := cmdInitVolume([]string{"--data", dir, "--uid", strconv.Itoa(os.Getuid()), "--gid", strconv.Itoa(os.Getgid())}, &out, &errb) + if code != 0 { + t.Fatalf("exit %d, stderr %q", code, errb.String()) + } + if code := cmdInitVolume([]string{"--data", filepath.Join(dir, "missing")}, &out, &errb); code != 1 { + t.Errorf("missing data dir exit = %d, want 1", code) + } +} diff --git a/cmd/felis/run.go b/cmd/felis/run.go index 62be2ab..5ecc186 100644 --- a/cmd/felis/run.go +++ b/cmd/felis/run.go @@ -63,6 +63,7 @@ var commands = map[string]func(args []string, stdout, stderr io.Writer) int{ "breakGlass": cmdBreakGlass, "bootstrap-assets": cmdBootstrapAssets, "init-forwarding": cmdInitForwarding, + "init-volume": cmdInitVolume, "version": cmdVersion, "update": cmdUpdate, } diff --git a/cmd/felis/run_test.go b/cmd/felis/run_test.go index 3364631..7552490 100644 --- a/cmd/felis/run_test.go +++ b/cmd/felis/run_test.go @@ -38,9 +38,12 @@ func TestRunUnknownCommand(t *testing.T) { } // undocumentedCommands are routable on purpose but kept out of the usage text: they -// are called by deploy/bootstrap.sh, not by a human at a prompt. Listing them here is -// what makes their absence from usage a deliberate decision rather than an oversight. -var undocumentedCommands = map[string]bool{"bootstrap-assets": true, "init-forwarding": true} +// are called by deploy/bootstrap.sh or the operator's initContainers, not by a human +// at a prompt. Listing them here is what makes their absence from usage a deliberate +// decision rather than an oversight. +var undocumentedCommands = map[string]bool{ + "bootstrap-assets": true, "init-forwarding": true, "init-volume": true, +} // The usage text and the dispatch table must describe the same set of commands. // diff --git a/deploy/limbo/Dockerfile b/deploy/limbo/Dockerfile index 250da10..7b4374b 100644 --- a/deploy/limbo/Dockerfile +++ b/deploy/limbo/Dockerfile @@ -78,6 +78,13 @@ COPY deploy/limbo/entrypoint.sh /usr/local/bin/felis-entrypoint.sh # The operator mounts the world PVC at /data. Runtime state lives there; /limbo # remains the immutable image seed copied into the volume by the entrypoint. WORKDIR /data +# Run as the game uid (naming.GameUID in the Go tree). The operator pins the same uid in +# the pod securityContext whatever USER an image declares; declaring it here as well +# keeps a plain `docker run` of this image off root, and chowning the empty /data seed +# lets that run write its world. The jar seed above stays root-owned and read-only to +# the server. +RUN chown 1000:1000 /data +USER 1000:1000 ENV FELIS_HEALTH_PORT=8080 # FELIS_GAME_PORT is the port the entrypoint pins Limbo to; it MUST equal the operator's diff --git a/deploy/lobby/Dockerfile b/deploy/lobby/Dockerfile index 466db28..2d75ca7 100644 --- a/deploy/lobby/Dockerfile +++ b/deploy/lobby/Dockerfile @@ -82,6 +82,13 @@ COPY deploy/lobby/entrypoint.sh /usr/local/bin/felis-entrypoint.sh # The operator mounts the world PVC at /data. Runtime state lives there; /paper # remains the immutable image seed copied into the volume by the entrypoint. WORKDIR /data +# Run as the game uid (naming.GameUID in the Go tree). The operator pins the same uid in +# the pod securityContext whatever USER an image declares; declaring it here as well +# keeps a plain `docker run` of this image off root, and chowning the empty /data seed +# lets that run write its world. The jar seed above stays root-owned and read-only to +# the server. +RUN chown 1000:1000 /data +USER 1000:1000 # FELIS_GAME_PORT is the port the entrypoint pins Paper to; it MUST equal the operator's # GamePort (internal/operator/builders.go). Default 25565 — override only in lockstep diff --git a/deploy/paper/Dockerfile b/deploy/paper/Dockerfile index e70f80a..656b71e 100644 --- a/deploy/paper/Dockerfile +++ b/deploy/paper/Dockerfile @@ -5,7 +5,7 @@ # internal/store/migrations/0019_recommended_paper.sql). It is NOT a system server: it # carries no felis-paper /menu plugin, no LuckPerms, and no forwarding-secret gate. # -# It writes NO Velocity forwarding config itself. The operator injects a root +# It writes NO Velocity forwarding config itself. The operator injects a # `felis init-forwarding` initContainer into every USER server (internal/operator/ # builders.go: buildStatefulSet) that writes config/paper-global.yml + server.properties # online-mode=false onto the /data PVC before this container starts. That external step is @@ -54,6 +54,13 @@ COPY deploy/paper/entrypoint.sh /usr/local/bin/felis-entrypoint.sh # on the PVC. /paper stays the immutable image seed: the jar is never copied onto the # volume, so the panel file editor (which sees only /data) cannot tamper with it. WORKDIR /data +# Run as the game uid (naming.GameUID in the Go tree). The operator pins the same uid in +# the pod securityContext whatever USER an image declares; declaring it here as well +# keeps a plain `docker run` of this image off root, and chowning the empty /data seed +# lets that run write its world. The jar seed above stays root-owned and read-only to +# the server. +RUN chown 1000:1000 /data +USER 1000:1000 # FELIS_GAME_PORT is the port the entrypoint pins Paper to; it MUST equal the operator's # GamePort (internal/operator/builders.go). Default 25565 — override only in lockstep with diff --git a/deploy/paper/entrypoint.sh b/deploy/paper/entrypoint.sh index e23c6fd..ebb2c51 100644 --- a/deploy/paper/entrypoint.sh +++ b/deploy/paper/entrypoint.sh @@ -3,7 +3,7 @@ # # A plain Paper backend for a user's OWN world — NOT a system server. Unlike deploy/limbo # and deploy/lobby it writes no Velocity forwarding config and has no secret gate: the -# operator injects a root `felis init-forwarding` initContainer that writes +# operator injects a `felis init-forwarding` initContainer that writes # config/paper-global.yml + server.properties online-mode=false onto /data BEFORE this # container starts, so forwarding is configured externally and this stays a drop-in Paper # image. With no initContainer (no FELIS_IMAGE) Paper just boots standalone-online — diff --git a/internal/backupjob/backup.go b/internal/backupjob/backup.go index cd2d82e..29b524b 100644 --- a/internal/backupjob/backup.go +++ b/internal/backupjob/backup.go @@ -88,13 +88,11 @@ type Config struct { CPULimit string MemLimit string // RunAsUser / RunAsGroup / FSGroup are the Pod's runtime identity. They default - // to ROOT (0:0) for the same reason the operator's forwarding-init container - // runs as root: the world volume is written by the game image's own UID (root - // for every Paper image we ship), and Paper saves files a non-root uid can - // never read — level.dat is written mode 0600 (tar walk: permission denied, - // verified live). DAC_OVERRIDE on the container covers images whose UID is - // neither root nor ours. Set 0/0/0 explicitly for root; FSGroup is omitted - // when zero. + // to ROOT (0:0): the world volume is written by the game uid (naming.GameUID), + // or by root in a world an older release wrote, and Paper saves files no other + // non-root uid can read — level.dat is written mode 0600 (tar walk: permission + // denied, verified live). DAC_OVERRIDE on the container reads them whichever + // uid owns them. Set 0/0/0 explicitly for root; FSGroup is omitted when zero. RunAsUser int64 RunAsGroup int64 FSGroup int64 diff --git a/internal/backupjob/jobspec.go b/internal/backupjob/jobspec.go index 307bceb..d2db9a6 100644 --- a/internal/backupjob/jobspec.go +++ b/internal/backupjob/jobspec.go @@ -183,7 +183,7 @@ func BackupJob(p JobParams) (*batchv1.Job, error) { ServiceAccountName: p.ServiceAccount, AutomountServiceAccountToken: boolPtr(false), // Root by default (see Config.RunAsUser): the world volume's - // owner is the game image's UID, so only an owner-matching or + // owner is the game uid, so only an owner-matching or // DAC-overriding uid can read it. FSGroup is omitted when unset // so a root pod never triggers a volume chgrp. SecurityContext: backupPodSecurityContext(p), @@ -248,9 +248,9 @@ func int32Ptr(i int32) *int32 { return &i } func int64Ptr(i int64) *int64 { return &i } // backupPodSecurityContext pins the Pod identity. RunAsNonRoot is false because -// the default identity is root: worlds are owned by the game image's UID (root -// for the images we ship), and Paper writes mode-0600 files a non-root reader -// cannot open. FSGroup stays unset unless configured — a root executor must not +// the default identity is root: worlds are owned by the game uid (or root, for a +// world an older release wrote), and Paper writes mode-0600 files any other +// non-root reader cannot open. FSGroup stays unset unless configured — a root executor must not // needlessly chgrp the world volume. func backupPodSecurityContext(p JobParams) *corev1.PodSecurityContext { sc := &corev1.PodSecurityContext{ diff --git a/internal/fileedit/editor.go b/internal/fileedit/editor.go index d2bdb7d..41a8bd8 100644 --- a/internal/fileedit/editor.go +++ b/internal/fileedit/editor.go @@ -113,10 +113,10 @@ type Config struct { CPULimit string MemLimit string // RunAsUser / RunAsGroup / FSGroup are the Pod's runtime identity. They default - // to ROOT (0:0): the world volume is written by the game image's own UID (root - // for the images we ship), and Paper saves mode-0600 files a non-root editor - // can neither read nor rewrite (level.dat). DAC_OVERRIDE on the container - // covers images whose UID is neither root nor ours; FSGroup is omitted when + // to ROOT (0:0): the world volume belongs to the game uid (naming.GameUID), and + // Paper saves mode-0600 files a different non-root uid can neither read nor + // rewrite (level.dat). DAC_OVERRIDE on the container reaches them, CHOWN on a + // write hands the created file back to the game uid; FSGroup is omitted when // zero. RunAsUser int64 RunAsGroup int64 diff --git a/internal/fileedit/exec.go b/internal/fileedit/exec.go index 60b0339..aa0cf41 100644 --- a/internal/fileedit/exec.go +++ b/internal/fileedit/exec.go @@ -10,6 +10,8 @@ import ( "os" "path" "time" + + "felis.lolicon.best/internal/naming" ) // The three operations the editor supports. The set is deliberately closed and @@ -343,9 +345,22 @@ func write(r *os.Root, path string, content []byte) Result { if err := f.Close(); err != nil { return failure(err, path) } + // The Job runs as root, so a file it just created is root's. The server runs as + // the game uid and could read it (0644) but never rewrite it — a config the + // panel authored that Paper then fails to save. Best effort: the content has + // landed and reporting failure would lie, and the server's prepare-data + // initContainer re-owns anything left behind on its next start anyway. + _ = ownWritten(r, path) return Result{} } +// ownWritten hands a written file to the game uid. os.Root.Chown follows a symlink +// only within the root, so this can never re-own a file outside the mount. A var so +// tests, which cannot chown, can observe the call. +var ownWritten = func(r *os.Root, name string) error { + return r.Chown(name, int(naming.GameUID), int(naming.GameGID)) +} + // failure maps a filesystem error onto a caller-facing Result code. Anything that // is genuinely "nothing is there" becomes not_found; EVERYTHING else — including // every os.Root containment refusal — becomes bad_path. diff --git a/internal/fileedit/exec_test.go b/internal/fileedit/exec_test.go index 8a04d13..a810f24 100644 --- a/internal/fileedit/exec_test.go +++ b/internal/fileedit/exec_test.go @@ -194,9 +194,19 @@ func TestExecuteHappyPath(t *testing.T) { }) t.Run("write creates a new file but not parent directories", func(t *testing.T) { + var owned []string + prev := ownWritten + ownWritten = func(_ *os.Root, name string) error { + owned = append(owned, name) + return os.ErrPermission // a test runner cannot chown; the write must still succeed + } + defer func() { ownWritten = prev }() if res, err := Execute(root, OpWrite, "ops.json", []byte("[]")); err != nil || res.Code != "" { t.Fatalf("creating a new file should succeed: %v / %+v", err, res) } + if len(owned) != 1 || owned[0] != "ops.json" { + t.Errorf("written file handed to the game uid = %v, want [ops.json]", owned) + } res, err := Execute(root, OpWrite, "nope/deep.txt", []byte("x")) if err != nil { t.Fatalf("Execute: %v", err) diff --git a/internal/fileedit/jobspec.go b/internal/fileedit/jobspec.go index 21f10db..79cb557 100644 --- a/internal/fileedit/jobspec.go +++ b/internal/fileedit/jobspec.go @@ -169,12 +169,9 @@ func FilesJob(p JobParams) (*batchv1.Job, error) { Privileged: boolPtr(false), AllowPrivilegeEscalation: boolPtr(false), ReadOnlyRootFilesystem: boolPtr(true), - // Root + DAC_OVERRIDE (see Config.RunAsUser): the file the editor is - // asked to touch may be a mode-0600 file the game wrote as its own - // (image) UID — level.dat — which a fixed non-root uid cannot open. Capabilities: &corev1.Capabilities{ Drop: []corev1.Capability{"ALL"}, - Add: []corev1.Capability{"DAC_OVERRIDE"}, + Add: filesCapabilities(p.Op), }, }, } @@ -253,9 +250,21 @@ func boolPtr(b bool) *bool { return &b } func int32Ptr(i int32) *int32 { return &i } func int64Ptr(i int64) *int64 { return &i } -// filesPodSecurityContext pins the Pod identity. Root by default: the world -// volume belongs to the game image's UID (root for the images we ship) and its -// mode-0600 files (level.dat) are otherwise unreadable/unwritable. FSGroup is +// filesCapabilities is what the root executor keeps after dropping ALL (see +// Config.RunAsUser). DAC_OVERRIDE opens a mode-0600 file (level.dat) the game wrote +// as its own uid, which a fixed non-root uid could not. A write also keeps CHOWN so +// the file it creates can be handed to naming.GameUID (exec.go ownWritten); a list +// or read changes nothing and gets no more than it needs. +func filesCapabilities(op string) []corev1.Capability { + if op == OpWrite { + return []corev1.Capability{"CHOWN", "DAC_OVERRIDE"} + } + return []corev1.Capability{"DAC_OVERRIDE"} +} + +// filesPodSecurityContext pins the Pod identity. Root by default: the world volume +// belongs to the game uid (naming.GameUID), and root with DAC_OVERRIDE reaches its +// mode-0600 files as well as any a previous root-run release left behind. FSGroup is // only rendered when configured so a root executor never chgrps the volume. func filesPodSecurityContext(p JobParams) *corev1.PodSecurityContext { sc := &corev1.PodSecurityContext{ diff --git a/internal/fileedit/jobspec_test.go b/internal/fileedit/jobspec_test.go index f9dc9b0..144c506 100644 --- a/internal/fileedit/jobspec_test.go +++ b/internal/fileedit/jobspec_test.go @@ -69,8 +69,8 @@ func TestFilesJobIsolation(t *testing.T) { if sc == nil || sc.RunAsNonRoot == nil || *sc.RunAsNonRoot { t.Fatal("RunAsNonRoot must be false: root is the owner-matching default for game-image worlds") } - // Root because the world volume belongs to the game image's UID and Paper - // saves mode-0600 files a fixed non-root editor cannot open. + // Root because the world volume belongs to the game uid and Paper saves + // mode-0600 files a different non-root editor uid cannot open. if sc.RunAsUser == nil || *sc.RunAsUser != 0 || sc.RunAsGroup == nil || *sc.RunAsGroup != 0 { t.Fatalf("uid/gid must be 0:0 by default, got %+v", sc) @@ -105,6 +105,19 @@ func TestFilesJobIsolation(t *testing.T) { } }) + // Only a write creates a file it must hand back to the game uid, so only a + // write keeps CHOWN; a read stays at DAC_OVERRIDE alone (asserted above). + t.Run("a write also keeps CHOWN", func(t *testing.T) { + w, err := FilesJob(testParams(OpWrite)) + if err != nil { + t.Fatalf("FilesJob: %v", err) + } + add := w.Spec.Template.Spec.Containers[0].SecurityContext.Capabilities.Add + if len(add) != 2 || add[0] != "CHOWN" || add[1] != "DAC_OVERRIDE" { + t.Fatalf("write capabilities = %v, want [CHOWN DAC_OVERRIDE]", add) + } + }) + t.Run("is one-shot, deadlined, and self-collecting", func(t *testing.T) { if job.Spec.BackoffLimit == nil || *job.Spec.BackoffLimit != 0 { t.Fatal("BackoffLimit must be 0 — a retried write is a second write") diff --git a/internal/naming/naming.go b/internal/naming/naming.go index 9f91b66..13422e4 100644 --- a/internal/naming/naming.go +++ b/internal/naming/naming.go @@ -145,6 +145,18 @@ func ValidateSystemServerName(name string) error { // mounts it), so it lives here rather than being duplicated per subsystem. const worldVolumeName = "world" +// GameUID / GameGID are the identity every game server process runs as, and so the +// owner every file on a world volume must carry. The operator pins them in the +// server pod's securityContext (whatever USER the image declares), its prepare-data +// initContainer chowns a world that an older root-run release or a root restore +// left behind, and the file editor hands a file it creates to them. 1000 is the +// uid the eclipse-temurin (Ubuntu) images the Felis game images build on already +// reserve for their unprivileged user. +const ( + GameUID int64 = 1000 + GameGID int64 = 1000 +) + // WorldPVCName returns the world PersistentVolumeClaim name for a server, // matching the operator's StatefulSet volumeClaimTemplate naming // ("world--0" for the sole replica). diff --git a/internal/operator/builders.go b/internal/operator/builders.go index 345f1a6..a31354c 100644 --- a/internal/operator/builders.go +++ b/internal/operator/builders.go @@ -228,6 +228,10 @@ func buildStatefulSet(server *v1alpha1.MinecraftServer, replicas int32, felisIma // StartupSpec.HealthHTTPPort) that reports true readiness — used below when // set. ReadinessProbe: readinessProbe(server), + // The server runs untrusted plugins, so it keeps no capability and can never + // regain one. The root filesystem stays writable: an arbitrary Paper image + // may unpack its runtime or write temp files outside /data. + SecurityContext: hardenedContainerSecurityContext(false), } if hp := server.Spec.Startup.HealthHTTPPort; hp > 0 { container.Ports = append(container.Ports, corev1.ContainerPort{ @@ -252,13 +256,18 @@ func buildStatefulSet(server *v1alpha1.MinecraftServer, replicas int32, felisIma } } - // An arbitrary user Paper image does not consume FELIS_FORWARDING_SECRET, so the - // operator writes the forwarding config into the world volume for it via an - // initContainer. System servers (login/lobby) are Felis-built and handle it in - // their own entrypoints, and without a felis image name there is nothing to run. + // Every server first hands its world volume to the game uid (prepareDataInitContainer), + // since the pod runs as that uid and a world an older root-run release wrote would + // otherwise be read-only to it. An arbitrary user Paper image then gets the forwarding + // config written for it (it does not consume FELIS_FORWARDING_SECRET itself); system + // servers (login/lobby) are Felis-built and handle forwarding in their own + // entrypoints. Without a felis image name there is nothing to run either step with. var initContainers []corev1.Container - if felisImage != "" && server.Labels[v1alpha1.LabelSystemRole] == "" { - initContainers = append(initContainers, forwardingInitContainer(felisImage)) + if felisImage != "" { + initContainers = append(initContainers, prepareDataInitContainer(felisImage)) + if server.Labels[v1alpha1.LabelSystemRole] == "" { + initContainers = append(initContainers, forwardingInitContainer(felisImage)) + } } grace := graceSeconds(server) @@ -298,6 +307,7 @@ func buildStatefulSet(server *v1alpha1.MinecraftServer, replicas int32, felisIma // default to no SA-token mount). The pod keeps the default SA but // with automounting explicitly disabled. AutomountServiceAccountToken: boolPtr(false), + SecurityContext: gamePodSecurityContext(), }, }, VolumeClaimTemplates: []corev1.PersistentVolumeClaim{pvc}, @@ -395,11 +405,10 @@ func forwardingSecretEnvVar() corev1.EnvVar { // felis image's `init-forwarding` subcommand, which merges the proxies.velocity block // into config/paper-global.yml and forces online-mode=false in server.properties. // -// It runs as root: the world volume's ownership is set by the storage provisioner and -// the main container runs as the user image's own UID, so root is the only UID that -// can reliably write these files and leave them rewritable by that main container. -// This is a bounded privilege — the init exits before the server container starts, and -// the server container keeps whatever (non-root) UID its image declares. +// It runs as the game uid like the server container (the pod securityContext), after +// prepareDataInitContainer has handed the volume to that uid, so it needs no privilege +// at all: no capability, a read-only root filesystem, and the files it writes are +// owned by the very uid that rewrites them on boot. // // Only user servers get it: the Felis-built system images (login limbo, lobby) already // consume the secret in their own entrypoints, and the login limbo is not Paper at all. @@ -412,9 +421,89 @@ func forwardingInitContainer(felisImage string) corev1.Container { VolumeMounts: []corev1.VolumeMount{ {Name: dataVolumeName, MountPath: dataMountPath}, }, + Resources: initContainerResources(), + SecurityContext: hardenedContainerSecurityContext(true), + } +} + +// prepareDataInitContainer runs `felis init-volume`, which chowns every world-volume +// entry not already owned by naming.GameUID:GameGID. It is the one container in the +// pod that runs as root, and it holds only what a chown walk needs: CHOWN to change +// an owner and DAC_OVERRIDE to descend into a directory some other uid left at 0700. +// Both are inside the PodSecurity baseline profile; everything else is dropped, the +// root filesystem is read-only, and it exits before the server container starts. +// +// fsGroup (gamePodSecurityContext) alone would not do: kubelet skips it for hostPath +// volumes, which is what a k3s local-path PV is underneath, and it only fixes the +// group besides. +func prepareDataInitContainer(felisImage string) corev1.Container { + return corev1.Container{ + Name: "prepare-data", + Image: felisImage, + Command: []string{felisBinaryPath, "init-volume", "--data", dataMountPath}, + VolumeMounts: []corev1.VolumeMount{ + {Name: dataVolumeName, MountPath: dataMountPath}, + }, + Resources: initContainerResources(), SecurityContext: &corev1.SecurityContext{ - RunAsUser: int64Ptr(0), - RunAsNonRoot: boolPtr(false), + RunAsUser: int64Ptr(0), + RunAsGroup: int64Ptr(0), + RunAsNonRoot: boolPtr(false), + Privileged: boolPtr(false), + AllowPrivilegeEscalation: boolPtr(false), + ReadOnlyRootFilesystem: boolPtr(true), + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + Add: []corev1.Capability{"CHOWN", "DAC_OVERRIDE"}, + }, + }, + } +} + +// gamePodSecurityContext pins every container in a server pod to the game uid, +// whatever USER its image declares, and to the runtime's default seccomp filter. +// fsGroup makes a volume type that supports ownership management group-writable +// for that uid; OnRootMismatch keeps kubelet from re-walking a large world on every +// start once the volume root already carries the group. +func gamePodSecurityContext() *corev1.PodSecurityContext { + onRootMismatch := corev1.FSGroupChangeOnRootMismatch + return &corev1.PodSecurityContext{ + RunAsNonRoot: boolPtr(true), + RunAsUser: int64Ptr(naming.GameUID), + RunAsGroup: int64Ptr(naming.GameGID), + FSGroup: int64Ptr(naming.GameGID), + FSGroupChangePolicy: &onRootMismatch, + SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault}, + } +} + +// hardenedContainerSecurityContext drops every capability and forbids gaining one +// back through a setuid binary. readOnlyRoot is set for the felis-image containers, +// which write nothing outside the world volume. +func hardenedContainerSecurityContext(readOnlyRoot bool) *corev1.SecurityContext { + sc := &corev1.SecurityContext{ + Privileged: boolPtr(false), + AllowPrivilegeEscalation: boolPtr(false), + Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + } + if readOnlyRoot { + sc.ReadOnlyRootFilesystem = boolPtr(true) + } + return sc +} + +// initContainerResources bounds the two felis-image initContainers. Both are short +// file walks; the memory ceiling stops a pathological volume from taking the node's +// memory with it, and no CPU limit keeps a large world's chown from being throttled +// into the pod's start-up time. +func initContainerResources() corev1.ResourceRequirements { + return corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("10m"), + corev1.ResourceMemory: resource.MustParse("32Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("128Mi"), }, } } diff --git a/internal/operator/builders_internal_test.go b/internal/operator/builders_internal_test.go index 50e5c05..c260737 100644 --- a/internal/operator/builders_internal_test.go +++ b/internal/operator/builders_internal_test.go @@ -56,9 +56,9 @@ func TestReadinessProbeHTTPCustomPath(t *testing.T) { } } -// A user server (no system-role label) gets the forwarding-config initContainer, -// running the felis image as root and mounting the world volume. A system server -// and a build with no felis image name get none. +// A user server (no system-role label) gets the forwarding-config initContainer after +// prepare-data, running the felis image and mounting the world volume. A system +// server gets only prepare-data, and a build with no felis image name gets neither. func TestBuildStatefulSetForwardingInitContainer(t *testing.T) { user := &v1alpha1.MinecraftServer{} user.Spec.Storage.Size = "1Gi" @@ -68,15 +68,18 @@ func TestBuildStatefulSetForwardingInitContainer(t *testing.T) { t.Fatalf("buildStatefulSet: %v", err) } inits := sts.Spec.Template.Spec.InitContainers - if len(inits) != 1 { - t.Fatalf("want 1 initContainer, got %d", len(inits)) + if len(inits) != 2 || inits[0].Name != "prepare-data" || inits[1].Name != "init-forwarding" { + t.Fatalf("want [prepare-data init-forwarding], got %+v", inits) } - ic := inits[0] + ic := inits[1] if ic.Image != "felis:demo" { t.Errorf("init image = %q, want felis:demo", ic.Image) } - if ic.SecurityContext == nil || ic.SecurityContext.RunAsUser == nil || *ic.SecurityContext.RunAsUser != 0 { - t.Errorf("init must run as root, got %+v", ic.SecurityContext) + // It shares the server's uid (pod securityContext) and holds no privilege. + if sc := ic.SecurityContext; sc == nil || sc.RunAsUser != nil || + sc.Capabilities == nil || len(sc.Capabilities.Add) != 0 || !dropsAll(sc) || + sc.ReadOnlyRootFilesystem == nil || !*sc.ReadOnlyRootFilesystem { + t.Errorf("init-forwarding must run unprivileged as the pod uid, got %+v", sc) } mounted := false for _, vm := range ic.VolumeMounts { @@ -91,7 +94,7 @@ func TestBuildStatefulSetForwardingInitContainer(t *testing.T) { // cannot do without the secret: a missing Env here makes `init-forwarding` no-op and // the server Ready-but-unjoinable — the exact silent failure the feature removes. // Same secretKeyRef rule as the main container (optional so a non-modern proxy still - // schedules), so assert it, not just the image/root/mount above. + // schedules), so assert it, not just the image/identity/mount above. fwd := findEnv(ic.Env, envForwardingSecret) if fwd == nil { t.Fatalf("init must carry %s or it writes no forwarding config", envForwardingSecret) @@ -109,13 +112,63 @@ func TestBuildStatefulSetForwardingInitContainer(t *testing.T) { t.Error("no felis image must yield no initContainer") } - // System server handles forwarding in its own entrypoint. + // System server handles forwarding in its own entrypoint, but its world still + // needs handing to the game uid. sys := &v1alpha1.MinecraftServer{} sys.Spec.Storage.Size = "1Gi" sys.Labels = map[string]string{v1alpha1.LabelSystemRole: "lobby"} sysSts, _ := buildStatefulSet(sys, 1, "felis:demo") - if len(sysSts.Spec.Template.Spec.InitContainers) != 0 { - t.Error("system server must get no forwarding initContainer") + if got := sysSts.Spec.Template.Spec.InitContainers; len(got) != 1 || got[0].Name != "prepare-data" { + t.Errorf("system server must get only prepare-data, got %+v", got) + } +} + +func dropsAll(sc *corev1.SecurityContext) bool { + return sc.Capabilities != nil && len(sc.Capabilities.Drop) == 1 && sc.Capabilities.Drop[0] == "ALL" +} + +// The server pod runs as the game uid under the runtime's seccomp filter, and the +// game container holds no capability. Only prepare-data runs as root, with exactly +// the two capabilities a chown walk needs — both inside the PodSecurity baseline. +func TestBuildStatefulSetRunsGameAsNonRoot(t *testing.T) { + s := &v1alpha1.MinecraftServer{} + s.Spec.Storage.Size = "1Gi" + sts, err := buildStatefulSet(s, 1, "felis:demo") + if err != nil { + t.Fatalf("buildStatefulSet: %v", err) + } + pod := sts.Spec.Template.Spec.SecurityContext + if pod == nil || pod.RunAsNonRoot == nil || !*pod.RunAsNonRoot || + pod.RunAsUser == nil || *pod.RunAsUser != naming.GameUID || + pod.RunAsGroup == nil || *pod.RunAsGroup != naming.GameGID || + pod.FSGroup == nil || *pod.FSGroup != naming.GameGID || + pod.SeccompProfile == nil || pod.SeccompProfile.Type != corev1.SeccompProfileTypeRuntimeDefault { + t.Fatalf("pod securityContext = %+v, want non-root %d:%d with fsGroup and RuntimeDefault seccomp", + pod, naming.GameUID, naming.GameGID) + } + game := sts.Spec.Template.Spec.Containers[0].SecurityContext + if game == nil || game.AllowPrivilegeEscalation == nil || *game.AllowPrivilegeEscalation || + !dropsAll(game) || len(game.Capabilities.Add) != 0 || game.RunAsUser != nil { + t.Errorf("game container securityContext = %+v, want no escalation and drop ALL", game) + } + + prep := sts.Spec.Template.Spec.InitContainers[0] + sc := prep.SecurityContext + if sc == nil || sc.RunAsUser == nil || *sc.RunAsUser != 0 || sc.RunAsNonRoot == nil || *sc.RunAsNonRoot { + t.Fatalf("prepare-data must run as root, got %+v", sc) + } + if !dropsAll(sc) || len(sc.Capabilities.Add) != 2 || + sc.Capabilities.Add[0] != "CHOWN" || sc.Capabilities.Add[1] != "DAC_OVERRIDE" { + t.Errorf("prepare-data capabilities = %+v, want drop ALL + CHOWN, DAC_OVERRIDE", sc.Capabilities) + } + if sc.AllowPrivilegeEscalation == nil || *sc.AllowPrivilegeEscalation { + t.Error("prepare-data must forbid privilege escalation") + } + if len(prep.Command) < 2 || prep.Command[1] != "init-volume" { + t.Errorf("prepare-data command = %v, want felis init-volume", prep.Command) + } + if prep.Resources.Limits.Memory().IsZero() { + t.Error("prepare-data must carry a memory limit") } } diff --git a/internal/platform/workloads.go b/internal/platform/workloads.go index 5c5ad6c..2e3380e 100644 --- a/internal/platform/workloads.go +++ b/internal/platform/workloads.go @@ -978,11 +978,12 @@ func hardenedPodSecurityContext() *corev1.PodSecurityContext { // reaperPodSecurityContext is the reaper's Pod identity: ROOT, deliberately NOT // the control-plane's non-root uid. Its HostPath mount IS the live storage root, // and the world directories beneath it (and the files inside them) are written -// by the game image's own UID — root for every Paper image we ship — with -// Paper's mode-0600 saves (level.dat) included. Only an owner-matching uid (or -// DAC override, granted on the container below) can archive and delete those -// worlds; the uid-1000 convention failed them with `permission denied` -// (verified live). Same rationale as the operator's forwarding-init container. +// by the game uid (naming.GameUID) — or by root, in a world an older release +// wrote — with Paper's mode-0600 saves (level.dat) included, while the storage +// root that holds each world directory belongs to root. Only root with DAC +// override (granted on the container below) can both archive every world and +// remove its directory from that root; the uid-1000 control-plane convention +// failed them with `permission denied` (verified live). func reaperPodSecurityContext() *corev1.PodSecurityContext { return &corev1.PodSecurityContext{ RunAsNonRoot: boolPtr(false), diff --git a/internal/platform/workloads_test.go b/internal/platform/workloads_test.go index 2a202ea..6c87957 100644 --- a/internal/platform/workloads_test.go +++ b/internal/platform/workloads_test.go @@ -807,9 +807,9 @@ func TestReaperCronJob_Shape(t *testing.T) { } // The reaper is the one world-touching workload, so its identity is ROOT, not - // the control-plane's non-root uid: the worlds it archives and deletes are - // written by the game image's own UID (root for the images we ship), including - // Paper's mode-0600 files. DAC_OVERRIDE covers images with another UID. + // the control-plane's non-root uid: the worlds it archives and deletes sit in a + // root-owned storage root and hold Paper's mode-0600 files, whichever uid (the + // game uid, or root for a world an older release wrote) owns them. if ps.SecurityContext == nil || ps.SecurityContext.RunAsNonRoot == nil || *ps.SecurityContext.RunAsNonRoot { t.Error("reaper pod must NOT require non-root: root is the owner-matching identity for game-image worlds") } diff --git a/internal/restore/jobspec.go b/internal/restore/jobspec.go index 46eac61..4123de7 100644 --- a/internal/restore/jobspec.go +++ b/internal/restore/jobspec.go @@ -126,9 +126,11 @@ func RestoreJob(p JobParams) (*batchv1.Job, error) { AllowPrivilegeEscalation: boolPtr(false), ReadOnlyRootFilesystem: boolPtr(true), // Root + DAC_OVERRIDE (see restore.Config.RunAsUser): the world is - // owned by the game image's UID and Paper's files are mode 0600, so - // the restore must bypass file modes to overwrite what the server - // wrote — otherwise level.dat is un-restorable. + // owned by the game uid and Paper's files are mode 0600, so the + // restore must bypass file modes to overwrite what the server wrote — + // otherwise level.dat is un-restorable. What it extracts lands + // root-owned; the server's prepare-data initContainer hands it to the + // game uid before the server next starts. Capabilities: &corev1.Capabilities{ Drop: []corev1.Capability{"ALL"}, Add: []corev1.Capability{"DAC_OVERRIDE"}, @@ -226,8 +228,8 @@ func resourceLimits(cpu, mem string) (corev1.ResourceList, error) { func boolPtr(b bool) *bool { return &b } // restorePodSecurityContext pins the Pod identity. Root by default — the world -// volume is owned by the game image's UID and Paper writes mode-0600 files, so a -// fixed non-root executor could neither read nor replace them. FSGroup is only +// volume is owned by the game uid and Paper writes mode-0600 files, so a different +// non-root executor could neither read nor replace them. FSGroup is only // rendered when configured: a root executor must not chgrp the world volume. func restorePodSecurityContext(p JobParams) *corev1.PodSecurityContext { sc := &corev1.PodSecurityContext{ diff --git a/internal/restore/jobspec_test.go b/internal/restore/jobspec_test.go index 21fd34a..236d17a 100644 --- a/internal/restore/jobspec_test.go +++ b/internal/restore/jobspec_test.go @@ -161,7 +161,7 @@ func TestRestoreJobIsBoundedOneShotAndSelfCleaning(t *testing.T) { } } -// The Pod runs as root (the world volume belongs to the game image's UID — see +// The Pod runs as root (the world volume belongs to the game uid — see // restore.Config.RunAsUser), and the container stays non-privileged, // escalation-proof, read-only root, ALL caps dropped except DAC_OVERRIDE, with // resource limits. diff --git a/internal/restore/restore.go b/internal/restore/restore.go index 2bc0171..d18e2f9 100644 --- a/internal/restore/restore.go +++ b/internal/restore/restore.go @@ -87,11 +87,11 @@ type Config struct { CPULimit string MemLimit string // RunAsUser / RunAsGroup / FSGroup are the Pod's runtime identity. They default - // to ROOT (0:0) for the same reason the operator's forwarding-init runs as - // root: the world volume is written by the game image's own UID (root for the - // images we ship), and Paper saves mode-0600 files a non-root writer/reader - // cannot replace (a uid-1000 restore cannot overwrite level.dat). DAC_OVERRIDE - // on the container covers images whose UID is neither root nor ours; FSGroup + // to ROOT (0:0): the world volume is written by the game uid (naming.GameUID), + // or by root in a world an older release wrote, and Paper saves mode-0600 + // files only their owner can replace. DAC_OVERRIDE on the container overwrites + // them whichever uid owns them; the extracted files land root-owned and the + // server's prepare-data initContainer re-owns them on its next start. FSGroup // is omitted when zero. RunAsUser int64 RunAsGroup int64