From b00c7f22ca4f5ff216157f81a8a74ae2f426e048 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 17 Sep 2024 12:08:48 +0200 Subject: [PATCH] libpod: convert owner IDs only with :idmap convert the owner UID and GID into the user namespace only when ":idmap" mount is used. This changes the behaviour of :idmap with an empty volume. Now the existing directory ownership is copied up as in the other case. Closes: https://github.com/containers/podman/issues/23347 Closes: https://issues.redhat.com/browse/RHEL-67842 Signed-off-by: Giuseppe Scrivano (cherry picked from commit 432325236b7d195045edb9a4cc47be00f71d407f) Signed-off-by: Giuseppe Scrivano --- libpod/container_internal_common.go | 7 +++++-- libpod/runtime_ctr.go | 11 +---------- test/system/030-run.bats | 15 ++++++++++----- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index 47f2401c4c..f95644a8a3 100644 --- a/libpod/container_internal_common.go +++ b/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, diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 8ba4a2fd26..3551613a1f 100644 --- a/libpod/runtime_ctr.go +++ b/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 { diff --git a/test/system/030-run.bats b/test/system/030-run.bats index 70731b4829..d6a2779836 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -1293,18 +1293,23 @@ EOF 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 chown 1000:1000 $romount/volume - run_podman run --security-opt label=disable --rm --uidmap=0:1000:10000 -v $myvolume:/volume:idmap --rootfs $romount stat -c %u:%g /volume - is "$output" "0:0" + 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 idmap volume maintains the original ownership + # verify that copyup with an empty idmap volume maintains the original ownership with different mappings myvolume=my-volume-$(safename) - run_podman run --rm --uidmap=0:1000:10000 -v $myvolume:/etc:idmap $IMAGE stat -c %u:%g /etc/passwd - is "$output" "0:0" + 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