From 2010961d32c87b6e42b349c44750441ae2f6bf57 Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Wed, 23 Sep 2026 06:20:07 +0800 Subject: [PATCH] fix(workloads): world executors run as root so game-image worlds are readable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A live backup drill on test-one failed: 'tar walk: open /world/world/level.dat: permission denied'. The world volume belongs to the game image's own UID (root for every Paper image we ship), and Paper saves level.dat mode 0600 — a fixed uid-1000 executor can neither read it (backup/reaper archive) nor overwrite it (restore). The same identity silently broke on-demand backups, restores, and the reaper for every server that had saved once. Run the backup Job, restore Job, file Job, and the reaper pod as root with DAC_OVERRIDE on top of drop-ALL — the same owner-matching precedent as the operator's forwarding-init container; DAC_OVERRIDE extends it to game images whose UID is neither root nor ours. FSGroup is omitted when zero so a root executor never chgrps the world volume. Shape tests updated for the new identity. --- docs/troubleshooting.md | 16 +++++++---- internal/backupjob/backup.go | 21 ++++++--------- internal/backupjob/jobspec.go | 39 +++++++++++++++++++++------ internal/backupjob/jobspec_test.go | 30 +++++++++++++++++---- internal/fileedit/editor.go | 19 +++++-------- internal/fileedit/jobspec.go | 33 +++++++++++++++++------ internal/fileedit/jobspec_test.go | 29 +++++++++++--------- internal/platform/workloads.go | 42 +++++++++++++++++++++++++---- internal/platform/workloads_test.go | 15 ++++++++--- internal/restore/jobspec.go | 36 ++++++++++++++++++------- internal/restore/jobspec_test.go | 26 +++++++++++------- internal/restore/restore.go | 22 +++++---------- 12 files changed, 223 insertions(+), 105 deletions(-) diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index b27badd..008e120 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -557,11 +557,17 @@ the flag at k3s's storage root (`/var/lib/rancher/k3s/storage`) is therefore the supported way to enable retention on a stock install. Two deployment facts the resolver cannot fix: -- **Permissions.** The reaper pod runs as uid 1000, while k3s creates its - storage root `0700 root:root`. Without traverse (`setfacl -m u:1000:x`, or - `chmod o+x`; the installer applies this when `FELIS_WORLDS_HOST_PATH` is set) - every walk fails `permission denied` / `lstat …: permission denied` and the - world is **preserved**, never reaped — a silent no-op with ERROR logs. +- **Permissions.** The reaper Pod runs as **root** and carries `DAC_OVERRIDE`: + worlds are written by the game image's own UID (root for every Paper image we + ship), and Paper saves `level.dat` mode-0600, so any fixed non-root identity + (the previous uid-1000 convention, and the ACL setup that went with it) could + neither walk the tree nor read the files — every archive failed + `open …/level.dat: permission denied` and the same defect failed on-demand + backups/restores. Root is the same identity the game container itself runs as + (see the operator's forwarding-init note); `DAC_OVERRIDE` extends the archive + to game images with a different UID. If a world is still **preserved** while a + reap was expected, it is now a different cause: check the run's ERROR logs for + the resolver's `lstat` messages before suspecting permissions. - **Node placement.** Multi-node clusters: the world's directory exists only on the node holding its volume, and the CronJob sets no `nodeSelector`, so add one (single-node starters are pinned implicitly). diff --git a/internal/backupjob/backup.go b/internal/backupjob/backup.go index ca06992..cd2d82e 100644 --- a/internal/backupjob/backup.go +++ b/internal/backupjob/backup.go @@ -87,9 +87,14 @@ type Config struct { // CPULimit / MemLimit cap the backup container. CPULimit string MemLimit string - // RunAsUser / RunAsGroup / FSGroup are the Pod's runtime identity. FSGroup MUST - // match the operator StatefulSet's runtime group so the read-only world mount is - // readable by this Pod's uid. + // 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. RunAsUser int64 RunAsGroup int64 FSGroup int64 @@ -111,7 +116,6 @@ const ( defaultDeadline = 30 * time.Minute defaultCPULimit = "1" defaultMemLimit = "1Gi" - defaultRunAsID = int64(1000) defaultTTL = 10 * time.Minute ) @@ -145,15 +149,6 @@ func (c Config) withDefaults() Config { if c.MemLimit == "" { c.MemLimit = defaultMemLimit } - if c.RunAsUser == 0 { - c.RunAsUser = defaultRunAsID - } - if c.RunAsGroup == 0 { - c.RunAsGroup = defaultRunAsID - } - if c.FSGroup == 0 { - c.FSGroup = defaultRunAsID - } if c.TTLAfterFinished <= 0 { c.TTLAfterFinished = defaultTTL } diff --git a/internal/backupjob/jobspec.go b/internal/backupjob/jobspec.go index 22c3de0..307bceb 100644 --- a/internal/backupjob/jobspec.go +++ b/internal/backupjob/jobspec.go @@ -148,7 +148,14 @@ func BackupJob(p JobParams) (*batchv1.Job, error) { Privileged: boolPtr(false), AllowPrivilegeEscalation: boolPtr(false), ReadOnlyRootFilesystem: boolPtr(true), - Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + // DAC_OVERRIDE is granted on top of dropping ALL: the pod runs as root, + // but the world may have been written by a game image whose UID is + // neither root nor ours, and Paper's own files are mode 0600. It is the + // minimal extra power that makes the archive read every world shape. + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + Add: []corev1.Capability{"DAC_OVERRIDE"}, + }, }, } @@ -175,13 +182,12 @@ func BackupJob(p JobParams) (*batchv1.Job, error) { RestartPolicy: corev1.RestartPolicyNever, ServiceAccountName: p.ServiceAccount, AutomountServiceAccountToken: boolPtr(false), - SecurityContext: &corev1.PodSecurityContext{ - RunAsNonRoot: boolPtr(true), - RunAsUser: int64Ptr(p.RunAsUser), - RunAsGroup: int64Ptr(p.RunAsGroup), - FSGroup: int64Ptr(p.FSGroup), - }, - Containers: []corev1.Container{container}, + // Root by default (see Config.RunAsUser): the world volume's + // owner is the game image's 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), + Containers: []corev1.Container{container}, Volumes: []corev1.Volume{ { Name: worldVolume, @@ -240,3 +246,20 @@ func resourceLimits(cpu, mem string) (corev1.ResourceList, error) { func boolPtr(b bool) *bool { return &b } 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 +// needlessly chgrp the world volume. +func backupPodSecurityContext(p JobParams) *corev1.PodSecurityContext { + sc := &corev1.PodSecurityContext{ + RunAsNonRoot: boolPtr(false), + RunAsUser: int64Ptr(p.RunAsUser), + RunAsGroup: int64Ptr(p.RunAsGroup), + } + if p.FSGroup > 0 { + sc.FSGroup = int64Ptr(p.FSGroup) + } + return sc +} diff --git a/internal/backupjob/jobspec_test.go b/internal/backupjob/jobspec_test.go index da220f5..8c6a1d8 100644 --- a/internal/backupjob/jobspec_test.go +++ b/internal/backupjob/jobspec_test.go @@ -23,9 +23,9 @@ func sampleJobParams() JobParams { Deadline: 30 * time.Minute, CPULimit: "1", MemLimit: "1Gi", - RunAsUser: 1000, - RunAsGroup: 1000, - FSGroup: 1000, + RunAsUser: 0, + RunAsGroup: 0, + FSGroup: 0, TTLAfterFinished: 10 * time.Minute, } } @@ -109,13 +109,30 @@ func TestBackupJobMountsTwoPVCsPlusConfigSecretOnly(t *testing.T) { } } -// The backup container is hardened exactly like the restore/build Job containers: -// no privilege, no escalation, read-only root fs, drop ALL capabilities. +// The backup container is hardened like the restore/build Job containers: no +// privilege, no escalation, read-only root fs, ALL capabilities dropped — plus +// DAC_OVERRIDE, because the Pod runs as root and the world may have been written +// by a game image with a different UID (verified live: a uid-1000 executor cannot +// read Paper's mode-0600 level.dat). func TestBackupJobContainerIsHardened(t *testing.T) { job, err := BackupJob(sampleJobParams()) if err != nil { t.Fatalf("BackupJob: %v", err) } + pod := job.Spec.Template.Spec + if pod.SecurityContext == nil { + t.Fatal("pod SecurityContext is nil") + } + if pod.SecurityContext.RunAsNonRoot == nil || *pod.SecurityContext.RunAsNonRoot { + t.Error("pod must NOT require non-root: root is the owner-matching default for game-image worlds") + } + if pod.SecurityContext.RunAsUser == nil || *pod.SecurityContext.RunAsUser != 0 || + pod.SecurityContext.RunAsGroup == nil || *pod.SecurityContext.RunAsGroup != 0 { + t.Errorf("pod must run as 0:0 by default, got %+v", pod.SecurityContext) + } + if pod.SecurityContext.FSGroup != nil { + t.Error("fsGroup must stay unset when zero (a root executor must not chgrp the world volume)") + } sc := job.Spec.Template.Spec.Containers[0].SecurityContext if sc == nil { t.Fatal("container SecurityContext is nil") @@ -132,6 +149,9 @@ func TestBackupJobContainerIsHardened(t *testing.T) { if sc.Capabilities == nil || len(sc.Capabilities.Drop) != 1 || sc.Capabilities.Drop[0] != "ALL" { t.Error("capabilities must drop ALL") } + if len(sc.Capabilities.Add) != 1 || sc.Capabilities.Add[0] != "DAC_OVERRIDE" { + t.Errorf("capabilities must add exactly DAC_OVERRIDE, got %v", sc.Capabilities.Add) + } } // One-shot: a wedged archive must not loop, and a deadline caps it. diff --git a/internal/fileedit/editor.go b/internal/fileedit/editor.go index d9061f5..d2bdb7d 100644 --- a/internal/fileedit/editor.go +++ b/internal/fileedit/editor.go @@ -112,9 +112,12 @@ type Config struct { // CPULimit / MemLimit cap the container. CPULimit string MemLimit string - // RunAsUser / RunAsGroup / FSGroup are the Pod's runtime identity. FSGroup MUST - // match the operator StatefulSet's runtime group, or a file this Pod writes - // would be unreadable by the minecraft server that later mounts the same PVC. + // 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 + // zero. RunAsUser int64 RunAsGroup int64 FSGroup int64 @@ -136,7 +139,6 @@ const ( defaultTimeout = 90 * time.Second defaultCPULimit = "500m" defaultMemLimit = "256Mi" - defaultRunAsID = int64(1000) defaultTTL = 2 * time.Minute ) @@ -164,15 +166,6 @@ func (c Config) withDefaults() Config { if c.MemLimit == "" { c.MemLimit = defaultMemLimit } - if c.RunAsUser == 0 { - c.RunAsUser = defaultRunAsID - } - if c.RunAsGroup == 0 { - c.RunAsGroup = defaultRunAsID - } - if c.FSGroup == 0 { - c.FSGroup = defaultRunAsID - } if c.TTLAfterFinished <= 0 { c.TTLAfterFinished = defaultTTL } diff --git a/internal/fileedit/jobspec.go b/internal/fileedit/jobspec.go index 2fb7102..07aade8 100644 --- a/internal/fileedit/jobspec.go +++ b/internal/fileedit/jobspec.go @@ -165,7 +165,13 @@ func FilesJob(p JobParams) (*batchv1.Job, error) { Privileged: boolPtr(false), AllowPrivilegeEscalation: boolPtr(false), ReadOnlyRootFilesystem: boolPtr(true), - Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + // 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"}, + }, }, } @@ -199,13 +205,8 @@ func FilesJob(p JobParams) (*batchv1.Job, error) { RestartPolicy: corev1.RestartPolicyNever, ServiceAccountName: p.ServiceAccount, AutomountServiceAccountToken: boolPtr(false), - SecurityContext: &corev1.PodSecurityContext{ - RunAsNonRoot: boolPtr(true), - RunAsUser: int64Ptr(p.RunAsUser), - RunAsGroup: int64Ptr(p.RunAsGroup), - FSGroup: int64Ptr(p.FSGroup), - }, - Containers: []corev1.Container{container}, + SecurityContext: filesPodSecurityContext(p), + Containers: []corev1.Container{container}, Volumes: []corev1.Volume{{ Name: worldVolume, VolumeSource: corev1.VolumeSource{ @@ -247,3 +248,19 @@ func resourceLimits(cpu, mem string) (corev1.ResourceList, error) { 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 +// only rendered when configured so a root executor never chgrps the volume. +func filesPodSecurityContext(p JobParams) *corev1.PodSecurityContext { + sc := &corev1.PodSecurityContext{ + RunAsNonRoot: boolPtr(false), + RunAsUser: int64Ptr(p.RunAsUser), + RunAsGroup: int64Ptr(p.RunAsGroup), + } + if p.FSGroup > 0 { + sc.FSGroup = int64Ptr(p.FSGroup) + } + return sc +} diff --git a/internal/fileedit/jobspec_test.go b/internal/fileedit/jobspec_test.go index 59943fc..f9dc9b0 100644 --- a/internal/fileedit/jobspec_test.go +++ b/internal/fileedit/jobspec_test.go @@ -23,9 +23,9 @@ func testParams(op string) JobParams { Deadline: 2 * time.Minute, CPULimit: "500m", MemLimit: "256Mi", - RunAsUser: 1000, - RunAsGroup: 1000, - FSGroup: 1000, + RunAsUser: 0, + RunAsGroup: 0, + FSGroup: 0, TTLAfterFinished: 2 * time.Minute, } } @@ -64,17 +64,19 @@ func TestFilesJobIsolation(t *testing.T) { } }) - t.Run("runs non-root with the operator's runtime identity", func(t *testing.T) { + t.Run("runs as root, the owner-matching identity for game-image worlds", func(t *testing.T) { sc := spec.SecurityContext - if sc == nil || sc.RunAsNonRoot == nil || !*sc.RunAsNonRoot { - t.Fatal("RunAsNonRoot must be true") + if sc == nil || sc.RunAsNonRoot == nil || *sc.RunAsNonRoot { + t.Fatal("RunAsNonRoot must be false: root is the owner-matching default for game-image worlds") } - // FSGroup must match the minecraft server's group or a file this Pod writes - // would be unreadable by the server that later mounts the same volume. - if sc.RunAsUser == nil || *sc.RunAsUser != 1000 || - sc.RunAsGroup == nil || *sc.RunAsGroup != 1000 || - sc.FSGroup == nil || *sc.FSGroup != 1000 { - t.Fatalf("uid/gid/fsGroup must all be 1000, got %+v", sc) + // 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. + 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) + } + if sc.FSGroup != nil { + t.Fatalf("fsGroup must stay unset when zero, got %+v", sc.FSGroup) } }) @@ -98,6 +100,9 @@ func TestFilesJobIsolation(t *testing.T) { if sc.Capabilities == nil || len(sc.Capabilities.Drop) != 1 || sc.Capabilities.Drop[0] != "ALL" { t.Fatalf("capabilities must drop ALL, got %+v", sc.Capabilities) } + if len(sc.Capabilities.Add) != 1 || sc.Capabilities.Add[0] != "DAC_OVERRIDE" { + t.Fatalf("capabilities must add exactly DAC_OVERRIDE, got %+v", sc.Capabilities.Add) + } }) t.Run("is one-shot, deadlined, and self-collecting", func(t *testing.T) { diff --git a/internal/platform/workloads.go b/internal/platform/workloads.go index 1459e29..6be1975 100644 --- a/internal/platform/workloads.go +++ b/internal/platform/workloads.go @@ -557,7 +557,7 @@ func reaperCronJob(p Params) *batchv1.CronJob { {Name: tmpVolume, MountPath: "/tmp"}, }, Resources: controlPlaneResources(), - SecurityContext: hardenedContainerSecurityContext(), + SecurityContext: reaperContainerSecurityContext(), } volumes := []corev1.Volume{ @@ -609,7 +609,7 @@ func reaperCronJob(p Params) *batchv1.CronJob { ServiceAccountName: SAReaper, PriorityClassName: controlPlanePriorityName, RestartPolicy: corev1.RestartPolicyNever, - SecurityContext: hardenedPodSecurityContext(), + SecurityContext: reaperPodSecurityContext(), Containers: []corev1.Container{container}, Volumes: volumes, }, @@ -842,9 +842,11 @@ func controlPlaneResources() corev1.ResourceRequirements { } } -// hardenedPodSecurityContext is the pod-level hardening shared by every workload -// here: run as a fixed non-root uid/gid with a matching fsGroup (so the registry -// can write its group-owned PVC) and the RuntimeDefault seccomp profile. +// hardenedPodSecurityContext is the pod-level hardening shared by the +// control-plane workloads (api, operator, registry — the world-touching reaper +// uses reaperPodSecurityContext instead): run as a fixed non-root uid/gid with a +// matching fsGroup (so the registry can write its group-owned PVC) and the +// RuntimeDefault seccomp profile. // // SHAPE-ASSERTED, runtime-unverified: this asserts the images can run as // nonRootUID. The felis image is built to; registry:2 (CNCF Distribution) can, @@ -860,6 +862,36 @@ 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. +func reaperPodSecurityContext() *corev1.PodSecurityContext { + return &corev1.PodSecurityContext{ + RunAsNonRoot: boolPtr(false), + RunAsUser: int64Ptr(0), + RunAsGroup: int64Ptr(0), + SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault}, + } +} + +// reaperContainerSecurityContext is hardenedContainerSecurityContext plus +// DAC_OVERRIDE: with root's caps dropped, root can read only files it owns, and +// a world may have been written by a game image whose UID is neither root nor +// ours. DAC_OVERRIDE restores exactly the file-mode bypass the archive needs. +func reaperContainerSecurityContext() *corev1.SecurityContext { + sc := hardenedContainerSecurityContext() + sc.Capabilities = &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + Add: []corev1.Capability{"DAC_OVERRIDE"}, + } + return sc +} + // hardenedContainerSecurityContext mirrors the build/restore Job containers: no // privilege, no escalation, read-only root filesystem (all writes go to the // mounted volumes — the config/data mounts and the /tmp emptyDir), drop ALL diff --git a/internal/platform/workloads_test.go b/internal/platform/workloads_test.go index b4f1f92..9c87db4 100644 --- a/internal/platform/workloads_test.go +++ b/internal/platform/workloads_test.go @@ -704,9 +704,15 @@ func TestReaperCronJob_Shape(t *testing.T) { t.Errorf("reaper pod must auto-mount its SA token (got AutomountServiceAccountToken=%v); it needs the API", *ps.AutomountServiceAccountToken) } - // Hardening mirrors the other control-plane pods. - if ps.SecurityContext == nil || ps.SecurityContext.RunAsNonRoot == nil || !*ps.SecurityContext.RunAsNonRoot { - t.Error("reaper pod must set runAsNonRoot=true") + // 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. + 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") + } + if ps.SecurityContext == nil || ps.SecurityContext.RunAsUser == nil || *ps.SecurityContext.RunAsUser != 0 { + t.Error("reaper pod must run as uid 0") } if c.SecurityContext == nil || c.SecurityContext.ReadOnlyRootFilesystem == nil || !*c.SecurityContext.ReadOnlyRootFilesystem { t.Error("reaper container must set readOnlyRootFilesystem=true") @@ -714,6 +720,9 @@ func TestReaperCronJob_Shape(t *testing.T) { if c.SecurityContext == nil || c.SecurityContext.Capabilities == nil || len(c.SecurityContext.Capabilities.Drop) == 0 || c.SecurityContext.Capabilities.Drop[0] != "ALL" { t.Error("reaper container must drop ALL capabilities") } + if c.SecurityContext == nil || c.SecurityContext.Capabilities == nil || len(c.SecurityContext.Capabilities.Add) != 1 || c.SecurityContext.Capabilities.Add[0] != "DAC_OVERRIDE" { + t.Error("reaper container must add exactly DAC_OVERRIDE") + } // Entrypoint: `/usr/local/bin/felis reaper --config --worlds-root /worlds`. if got := append(append([]string{}, c.Command...), c.Args...); !containsSeq(got, []string{felisBinaryPath, "reaper"}) { diff --git a/internal/restore/jobspec.go b/internal/restore/jobspec.go index 886b257..46eac61 100644 --- a/internal/restore/jobspec.go +++ b/internal/restore/jobspec.go @@ -125,7 +125,14 @@ func RestoreJob(p JobParams) (*batchv1.Job, error) { Privileged: boolPtr(false), AllowPrivilegeEscalation: boolPtr(false), ReadOnlyRootFilesystem: boolPtr(true), - Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + // 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. + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + Add: []corev1.Capability{"DAC_OVERRIDE"}, + }, }, } @@ -148,13 +155,8 @@ func RestoreJob(p JobParams) (*batchv1.Job, error) { RestartPolicy: corev1.RestartPolicyNever, ServiceAccountName: p.ServiceAccount, AutomountServiceAccountToken: boolPtr(false), - SecurityContext: &corev1.PodSecurityContext{ - RunAsNonRoot: boolPtr(true), - RunAsUser: int64Ptr(p.RunAsUser), - RunAsGroup: int64Ptr(p.RunAsGroup), - FSGroup: int64Ptr(p.FSGroup), - }, - Containers: []corev1.Container{container}, + SecurityContext: restorePodSecurityContext(p), + Containers: []corev1.Container{container}, Volumes: []corev1.Volume{ { Name: worldVolume, @@ -221,6 +223,22 @@ func resourceLimits(cpu, mem string) (corev1.ResourceList, error) { }, nil } -func boolPtr(b bool) *bool { return &b } +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 +// rendered when configured: a root executor must not chgrp the world volume. +func restorePodSecurityContext(p JobParams) *corev1.PodSecurityContext { + sc := &corev1.PodSecurityContext{ + RunAsNonRoot: boolPtr(false), + RunAsUser: int64Ptr(p.RunAsUser), + RunAsGroup: int64Ptr(p.RunAsGroup), + } + if p.FSGroup > 0 { + sc.FSGroup = int64Ptr(p.FSGroup) + } + return sc +} func int32Ptr(i int32) *int32 { return &i } func int64Ptr(i int64) *int64 { return &i } diff --git a/internal/restore/jobspec_test.go b/internal/restore/jobspec_test.go index af79a04..21fd34a 100644 --- a/internal/restore/jobspec_test.go +++ b/internal/restore/jobspec_test.go @@ -23,9 +23,9 @@ func sampleJobParams() JobParams { Deadline: 30 * time.Minute, CPULimit: "1", MemLimit: "1Gi", - RunAsUser: 1000, - RunAsGroup: 1000, - FSGroup: 1000, + RunAsUser: 0, + RunAsGroup: 0, + FSGroup: 0, TTLAfterFinished: 10 * time.Minute, } } @@ -161,19 +161,24 @@ func TestRestoreJobIsBoundedOneShotAndSelfCleaning(t *testing.T) { } } -// The container must be non-root, non-privileged, escalation-proof, read-only -// root, drop ALL caps, and carry resource limits. +// The Pod runs as root (the world volume belongs to the game image's 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. func TestRestoreJobContainerIsHardened(t *testing.T) { job, err := RestoreJob(sampleJobParams()) if err != nil { t.Fatalf("RestoreJob: %v", err) } pod := job.Spec.Template.Spec - if pod.SecurityContext == nil || pod.SecurityContext.RunAsNonRoot == nil || !*pod.SecurityContext.RunAsNonRoot { - t.Error("pod must set runAsNonRoot=true") + if pod.SecurityContext == nil || pod.SecurityContext.RunAsNonRoot == nil || *pod.SecurityContext.RunAsNonRoot { + t.Error("pod must NOT require non-root: root is the owner-matching default for game-image worlds") } - if pod.SecurityContext == nil || pod.SecurityContext.FSGroup == nil || *pod.SecurityContext.FSGroup != 1000 { - t.Error("pod must set an fsGroup so restored files are group-owned by the server identity") + if pod.SecurityContext == nil || pod.SecurityContext.RunAsUser == nil || *pod.SecurityContext.RunAsUser != 0 { + t.Error("pod must run as uid 0 by default") + } + if pod.SecurityContext == nil || pod.SecurityContext.FSGroup != nil { + t.Error("fsGroup must stay unset when zero (a root executor must not chgrp the world volume)") } c := singleContainer(t, job) sc := c.SecurityContext @@ -192,6 +197,9 @@ func TestRestoreJobContainerIsHardened(t *testing.T) { if sc.Capabilities == nil || len(sc.Capabilities.Drop) == 0 || string(sc.Capabilities.Drop[0]) != "ALL" { t.Errorf("container must drop ALL capabilities, got %v", sc.Capabilities) } + if len(sc.Capabilities.Add) != 1 || sc.Capabilities.Add[0] != "DAC_OVERRIDE" { + t.Errorf("container must add exactly DAC_OVERRIDE, got %v", sc.Capabilities.Add) + } if c.Resources.Limits.Cpu().IsZero() || c.Resources.Limits.Memory().IsZero() { t.Error("container must carry CPU+memory limits") } diff --git a/internal/restore/restore.go b/internal/restore/restore.go index 63ee897..2bc0171 100644 --- a/internal/restore/restore.go +++ b/internal/restore/restore.go @@ -86,11 +86,13 @@ type Config struct { // CPULimit / MemLimit cap the restore container. CPULimit string MemLimit string - // RunAsUser / RunAsGroup / FSGroup are the Pod's runtime identity. FSGroup in - // particular MUST match the operator StatefulSet's runtime group so the files - // the restore Pod writes are readable by the minecraft server that later - // mounts the same world PVC. The default matches the conventional minecraft - // container uid; a deployment that runs minecraft as another id overrides it. + // 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 + // is omitted when zero. RunAsUser int64 RunAsGroup int64 FSGroup int64 @@ -112,7 +114,6 @@ const ( defaultDeadline = 30 * time.Minute defaultCPULimit = "1" defaultMemLimit = "1Gi" - defaultRunAsID = int64(1000) defaultTTL = 10 * time.Minute ) @@ -143,15 +144,6 @@ func (c Config) withDefaults() Config { if c.MemLimit == "" { c.MemLimit = defaultMemLimit } - if c.RunAsUser == 0 { - c.RunAsUser = defaultRunAsID - } - if c.RunAsGroup == 0 { - c.RunAsGroup = defaultRunAsID - } - if c.FSGroup == 0 { - c.FSGroup = defaultRunAsID - } if c.TTLAfterFinished <= 0 { c.TTLAfterFinished = defaultTTL }