Merge pull request #24591 from giuseppe/backport-5.2-idmap

[5.2-rhel] libpod: fix volume copyup with idmap | convert owner IDs only with :idmap

Author: openshift-merge-bot[bot]

contrib/cirrus/setup_environment.sh

@@ -285,6 +285,13 @@ case "$PRIV_NAME" in
     *) die_unknown PRIV_NAME
 esac
 
+# Root user namespace
+for which in uid gid;do
+    if ! grep -qE '^containers:' /etc/sub$which; then
+        echo 'containers:10000000:1048576' >>/etc/sub$which
+    fi
+done
+
 # FIXME! experimental workaround for #16973, the "lookup cdn03.quay.io" flake.
 #
 # If you are reading this on or after April 2023:

libpod/container_internal.go

@@ -1924,6 +1924,11 @@ func (c *Container) mountNamedVolume(v *ContainerNamedVolume, mountpoint string)
 			getOptions := copier.GetOptions{
 				KeepDirectoryNames: false,
 			}
+			// If the volume is idmapped, we need to "undo" the idmapping
+			if slices.Contains(v.Options, "idmap") {
+				getOptions.UIDMap = c.config.IDMappings.UIDMap
+				getOptions.GIDMap = c.config.IDMappings.GIDMap
+			}
 			errChan <- copier.Get(srcDir, "", getOptions, []string{"/."}, writer)
 		}()
 

libpod/container_internal_common.go

@@ -2900,8 +2900,10 @@ func (c *Container) fixVolumePermissions(v *ContainerNamedVolume) error {
 		uid := int(c.config.Spec.Process.User.UID)
 		gid := int(c.config.Spec.Process.User.GID)
 
+		idmapped := hasIdmapOption(v.Options)
+
 		// if the volume is mounted with "idmap", leave the IDs in from the current environment.
-		if c.config.IDMappings.UIDMap != nil && !hasIdmapOption(v.Options) {
+		if c.config.IDMappings.UIDMap != nil && !idmapped {
 			p := idtools.IDPair{
 				UID: uid,
 				GID: gid,
@@ -2947,7 +2949,8 @@ func (c *Container) fixVolumePermissions(v *ContainerNamedVolume) error {
 			if stat, ok := st.Sys().(*syscall.Stat_t); ok {
 				uid, gid := int(stat.Uid), int(stat.Gid)
 
-				if c.config.IDMappings.UIDMap != nil {
+				// If the volume is idmapped then undo the conversion to obtain the desired UID/GID in the container
+				if c.config.IDMappings.UIDMap != nil && idmapped {
 					p := idtools.IDPair{
 						UID: uid,
 						GID: gid,

libpod/runtime_ctr.go

@@ -513,16 +513,11 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
 			volOptions = append(volOptions, withSetAnon())
 		}
 
-		needsChown := true
-
 		// If volume-opts are set, parse and add driver opts.
 		if len(vol.Options) > 0 {
 			isDriverOpts := false
 			driverOpts := make(map[string]string)
 			for _, opts := range vol.Options {
-				if opts == "idmap" {
-					needsChown = false
-				}
 				if strings.HasPrefix(opts, "volume-opt") {
 					isDriverOpts = true
 					driverOptKey, driverOptValue, err := util.ParseDriverOpts(opts)
@@ -538,11 +533,7 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
 			}
 		}
 
-		if needsChown {
-			volOptions = append(volOptions, WithVolumeUID(ctr.RootUID()), WithVolumeGID(ctr.RootGID()))
-		} else {
-			volOptions = append(volOptions, WithVolumeNoChown())
-		}
+		volOptions = append(volOptions, WithVolumeUID(ctr.RootUID()), WithVolumeGID(ctr.RootGID()))
 
 		_, err = r.newVolume(ctx, false, volOptions...)
 		if err != nil {

test/e2e/pod_create_test.go

@@ -799,7 +799,7 @@ ENTRYPOINT ["sleep","99999"]
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(ExitCleanly())
 		output := session.OutputToString()
-		Expect(output).To(MatchRegexp("\\s0\\s0\\s1"))
+		Expect(output).To(MatchRegexp(`(^|\s)0\s+0\s+1(\s|$)`))
 
 		podName = "testPod-1"
 		podCreate = podmanTest.Podman([]string{"pod", "create", "--userns=auto:size=8192,uidmapping=0:0:1", "--name", podName})
@@ -836,7 +836,7 @@ ENTRYPOINT ["sleep","99999"]
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(ExitCleanly())
 		output := session.OutputToString()
-		Expect(output).To(MatchRegexp("\\s0\\s0\\s1"))
+		Expect(output).To(MatchRegexp(`(^|\s)0\s+0\s+1(\s|$)`))
 
 		podName = "testPod-1"
 		podCreate = podmanTest.Podman([]string{"pod", "create", "--userns=auto:size=8192,gidmapping=0:0:1", "--name", podName})

test/e2e/run_userns_test.go

@@ -284,7 +284,7 @@ var _ = Describe("Podman UserNS support", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(ExitCleanly())
 		output := session.OutputToString()
-		Expect(output).To(MatchRegexp("\\s0\\s0\\s1"))
+		Expect(output).To(MatchRegexp(`(^|\s)0\s+0\s+1(\s|$)`))
 
 		session = podmanTest.Podman([]string{"run", "--userns=auto:size=8192,uidmapping=0:0:1", "alpine", "cat", "/proc/self/uid_map"})
 		session.WaitWithDefaultTimeout()
@@ -313,7 +313,7 @@ var _ = Describe("Podman UserNS support", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(ExitCleanly())
 		output := session.OutputToString()
-		Expect(output).To(MatchRegexp("\\s0\\s0\\s1"))
+		Expect(output).To(MatchRegexp(`(^|\s)0\s+0\s+1(\s|$)`))
 
 		session = podmanTest.Podman([]string{"run", "--userns=auto:size=8192,gidmapping=0:0:1", "alpine", "cat", "/proc/self/gid_map"})
 		session.WaitWithDefaultTimeout()

test/system/030-run.bats

@@ -1253,46 +1253,63 @@ EOF
     fi
 }
 
-@test "podman run - rootfs with idmapped mounts" {
+@test "podman run - idmapped mounts" {
     skip_if_rootless "idmapped mounts work only with root for now"
 
     skip_if_remote "userns=auto is set on the server"
 
     grep -E -q "^containers:" /etc/subuid || skip "no IDs allocated for user 'containers'"
 
-    # check if the underlying file system supports idmapped mounts
-    check_dir=$PODMAN_TMPDIR/idmap-check
-    mkdir $check_dir
-    run_podman '?' run --rm --uidmap=0:1000:10000 --rootfs $check_dir:idmap true
-    if [[ "$output" == *"failed to create idmapped mount: invalid argument"* ]]; then
-        skip "idmapped mounts not supported"
-    fi
+    # the TMPDIR must be accessible by different users as the following tests use different mappings
+    chmod 755 $PODMAN_TMPDIR
 
     run_podman image mount $IMAGE
     src="$output"
 
     # we cannot use idmap on top of overlay, so we need a copy
     romount=$PODMAN_TMPDIR/rootfs
-    cp -ar "$src" "$romount"
+    cp -a "$src" "$romount"
 
     run_podman image unmount $IMAGE
 
-    run_podman run --rm --uidmap=0:1000:10000 --rootfs $romount:idmap stat -c %u:%g /bin
+    # check if the underlying file system supports idmapped mounts
+    run_podman '?' run --security-opt label=disable --rm --uidmap=0:1000:10000 --rootfs $romount:idmap true
+    if [[ $status -ne 0 ]]; then
+        if [[ "$output" =~ "failed to create idmapped mount: invalid argument" ]]; then
+            skip "idmapped mounts not supported"
+        fi
+        # Any other error is fatal
+        die "Cannot create idmap mount: $output"
+    fi
+
+    run_podman run --security-opt label=disable --rm --uidmap=0:1000:10000 --rootfs $romount:idmap stat -c %u:%g /bin
     is "$output" "0:0"
 
-    run_podman run --uidmap=0:1000:10000 --rm --rootfs "$romount:idmap=uids=0-1001-10000;gids=0-1002-10000" stat -c %u:%g /bin
+    run_podman run --security-opt label=disable --uidmap=0:1000:10000 --rm --rootfs "$romount:idmap=uids=0-1001-10000;gids=0-1002-10000" stat -c %u:%g /bin
     is "$output" "1:2"
 
     touch $romount/testfile
     chown 2000:2000 $romount/testfile
-    run_podman run --uidmap=0:1000:200 --rm --rootfs "$romount:idmap=uids=@2000-1-1;gids=@2000-1-1" stat -c %u:%g /testfile
+    run_podman run --security-opt label=disable --uidmap=0:1000:200 --rm --rootfs "$romount:idmap=uids=@2000-1-1;gids=@2000-1-1" stat -c %u:%g /testfile
     is "$output" "1:1"
 
+    # verify that copyup with an empty idmap volume maintains the original ownership with different mappings and --rootfs
     myvolume=my-volume-$(safename)
     run_podman volume create $myvolume
     mkdir $romount/volume
-    run_podman run --rm --uidmap=0:1000:10000 -v volume:/volume:idmap --rootfs $romount stat -c %u:%g /volume
-    is "$output" "0:0"
+    chown 1000:1000 $romount/volume
+    for FROM in 1000 2000; do
+        run_podman run --security-opt label=disable --rm --uidmap=0:$FROM:10000 -v $myvolume:/volume:idmap --rootfs $romount stat -c %u:%g /volume
+        is "$output" "0:0"
+    done
+    run_podman volume rm $myvolume
+
+    # verify that copyup with an empty idmap volume maintains the original ownership with different mappings
+    myvolume=my-volume-$(safename)
+    for FROM in 1000 2000; do
+        run_podman run --rm --uidmap=0:$FROM:10000 -v $myvolume:/etc:idmap $IMAGE stat -c %u:%g /etc/passwd
+        is "$output" "0:0"
+    done
     run_podman volume rm $myvolume
 
     rm -rf $romount