Skip to content

Commit 21962d1

Browse files
committed
fix: improve userns validation when joining pods
- remove old CLI validation that only checked --pod flag - add validation in namespaces.go to catch all paths (cli, quadlet, api) - block userns mixing for all pods with infra, not just ipc/net - update error message to be clearer - fix test cleanup to use PodmanExitCleanly() - use dynamic pod names in system tests to avoid conflicts fixes #26848 Signed-off-by: 0xdvc <neilohene@gmail.com>
1 parent 2b646e7 commit 21962d1

File tree

5 files changed

+36
-12
lines changed

5 files changed

+36
-12
lines changed

cmd/podman/containers/create.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -308,10 +308,6 @@ func CreateInit(c *cobra.Command, vals entities.ContainerCreateOptions, isInfra
308308
if c.Flag("cgroups").Changed && vals.CgroupsMode == "split" && registry.IsRemote() {
309309
return vals, fmt.Errorf("the option --cgroups=%q is not supported in remote mode", vals.CgroupsMode)
310310
}
311-
312-
if c.Flag("pod").Changed && !strings.HasPrefix(c.Flag("pod").Value.String(), "new:") && c.Flag("userns").Changed {
313-
return vals, errors.New("--userns and --pod cannot be set together")
314-
}
315311
}
316312
if c.Flag("shm-size").Changed {
317313
vals.ShmSize = c.Flag("shm-size").Value.String()

pkg/specgen/generate/namespaces.go

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,20 @@ import (
2121

2222
const host = "host"
2323

24+
// userNSConflictsWithPod returns an error if the user namespace mode
25+
// conflicts with pod namespace sharing requirements.
26+
// Containers in a pod must use the same user namespace to avoid ownership and
27+
// capability issues with shared resources.
28+
func userNSConflictsWithPod(pod *libpod.Pod, mode specgen.NamespaceMode) error {
29+
if pod != nil && pod.HasInfraContainer() {
30+
// FromPod mode is allowed - container inherits from pod
31+
if mode != specgen.FromPod {
32+
return fmt.Errorf("cannot set user namespace mode when joining pod with infra container: %w", define.ErrInvalidArg)
33+
}
34+
}
35+
return nil
36+
}
37+
2438
// Get the default namespace mode for any given namespace type.
2539
func GetDefaultNamespaceMode(nsType string, cfg *config.Config, pod *libpod.Pod) (specgen.Namespace, error) {
2640
// The default for most is private
@@ -211,7 +225,11 @@ func namespaceOptions(s *specgen.SpecGenerator, rt *libpod.Runtime, pod *libpod.
211225
}
212226
}
213227

214-
// User
228+
// Validate that user namespace mode is compatible with pod.
229+
if err := userNSConflictsWithPod(pod, s.UserNS.NSMode); err != nil {
230+
return nil, err
231+
}
232+
215233
switch s.UserNS.NSMode {
216234
case specgen.KeepID:
217235
opts, err := namespaces.UsernsMode(s.UserNS.String()).GetKeepIDOptions()
@@ -247,6 +265,10 @@ func namespaceOptions(s *specgen.SpecGenerator, rt *libpod.Runtime, pod *libpod.
247265
return nil, fmt.Errorf("looking up container to share user namespace with: %w", err)
248266
}
249267
toReturn = append(toReturn, libpod.WithUserNSFrom(userCtr))
268+
case specgen.Private:
269+
case specgen.Auto:
270+
case specgen.NoMap:
271+
case specgen.Path:
250272
}
251273

252274
// This wipes the UserNS settings that get set from the infra container
@@ -255,8 +277,6 @@ func namespaceOptions(s *specgen.SpecGenerator, rt *libpod.Runtime, pod *libpod.
255277
if s.IDMappings != nil {
256278
if pod == nil {
257279
toReturn = append(toReturn, libpod.WithIDMappings(*s.IDMappings))
258-
} else if pod.HasInfraContainer() && (len(s.IDMappings.UIDMap) > 0 || len(s.IDMappings.GIDMap) > 0) {
259-
return nil, fmt.Errorf("cannot specify a new uid/gid map when entering a pod with an infra container: %w", define.ErrInvalidArg)
260280
}
261281
}
262282
if s.User != "" {

test/e2e/create_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -681,13 +681,14 @@ var _ = Describe("Podman create", func() {
681681
create := podmanTest.Podman([]string{"create", "--uidmap", "0:1000:1000", "--pod", "new:testing123", ALPINE})
682682
create.WaitWithDefaultTimeout()
683683
Expect(create).ShouldNot(ExitCleanly())
684-
Expect(create.ErrorToString()).To(ContainSubstring("cannot specify a new uid/gid map when entering a pod with an infra container"))
684+
Expect(create.ErrorToString()).To(ContainSubstring("cannot set user namespace mode when joining pod with infra container"))
685+
686+
podmanTest.PodmanExitCleanly("pod", "rm", "-f", "testing123")
685687

686688
create = podmanTest.Podman([]string{"create", "--gidmap", "0:1000:1000", "--pod", "new:testing1234", ALPINE})
687689
create.WaitWithDefaultTimeout()
688690
Expect(create).ShouldNot(ExitCleanly())
689-
Expect(create.ErrorToString()).To(ContainSubstring("cannot specify a new uid/gid map when entering a pod with an infra container"))
690-
691+
Expect(create.ErrorToString()).To(ContainSubstring("cannot set user namespace mode when joining pod with infra container"))
691692
})
692693

693694
It("podman create --chrootdirs inspection test", func() {

test/e2e/pod_create_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -804,7 +804,7 @@ ENTRYPOINT ["sleep","99999"]
804804
// fail if --pod and --userns set together
805805
session = podmanTest.Podman([]string{"run", "--pod", podName, "--userns", "keep-id", ALPINE, "id", "-u"})
806806
session.WaitWithDefaultTimeout()
807-
Expect(session).Should(ExitWithError(125, "--userns and --pod cannot be set together"))
807+
Expect(session).Should(ExitWithError(125, "cannot set user namespace mode when joining pod with infra container"))
808808
})
809809

810810
It("podman pod create with --userns=keep-id can add users", func() {

test/system/620-option-conflicts.bats

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ load helpers
1414
create,run | --cpu-period=1 | --cpus=2 | $IMAGE
1515
create,run | --cpu-quota=1 | --cpus=2 | $IMAGE
1616
create,run | --no-hosts | --add-host=foo:1.1.1.1 | $IMAGE
17-
create,run | --userns=bar | --pod=foo | $IMAGE
1817
container cleanup | --all | --exec=foo
1918
container cleanup | --exec=foo | --rmi | foo
2019
"
@@ -48,6 +47,14 @@ container cleanup | --exec=foo | --rmi | foo
4847
"podman $cmd --platform + --$opt"
4948
done
5049
done
50+
51+
# --userns and --pod have a different error message format
52+
podname=p-$(safename)
53+
run_podman pod create --name $podname
54+
run_podman 125 run --uidmap=0:1000:1000 --pod=$podname $IMAGE true
55+
is "$output" "Error: cannot set user namespace mode when joining pod with infra container: invalid argument" \
56+
"podman run --uidmap + --pod"
57+
run_podman pod rm -f $podname
5158
}
5259

5360

0 commit comments

Comments
 (0)