We should only be relabeling when on first run

On the second runs, the labels should be the same so no
need to relabel.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2013548

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

3 files changed, 123 insertions, 33 deletions

diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index d8385961f..27cc318b4 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -50,6 +50,7 @@ import (
 	runcuser "github.com/opencontainers/runc/libcontainer/user"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/runtime-tools/generate"
+	"github.com/opencontainers/selinux/go-selinux"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -390,11 +391,11 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 			for _, o := range namedVol.Options {
 				switch o {
 				case "U":
-					if err := chown.ChangeHostPathOwnership(mountPoint, true, int(hostUID), int(hostGID)); err != nil {
+					if err := c.ChangeHostPathOwnership(mountPoint, true, int(hostUID), int(hostGID)); err != nil {
 						return nil, err
 					}
 
-					if err := chown.ChangeHostPathOwnership(contentDir, true, int(hostUID), int(hostGID)); err != nil {
+					if err := c.ChangeHostPathOwnership(contentDir, true, int(hostUID), int(hostGID)); err != nil {
 						return nil, err
 					}
 				}
@@ -423,14 +424,15 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 				if m.Type == "tmpfs" {
 					options = append(options, []string{fmt.Sprintf("uid=%d", execUser.Uid), fmt.Sprintf("gid=%d", execUser.Gid)}...)
 				} else {
-					if err := chown.ChangeHostPathOwnership(m.Source, true, int(hostUID), int(hostGID)); err != nil {
+					// only chown on initial creation of container
+					if err := c.ChangeHostPathOwnership(m.Source, true, int(hostUID), int(hostGID)); err != nil {
 						return nil, err
 					}
 				}
 			case "z":
 				fallthrough
 			case "Z":
-				if err := label.Relabel(m.Source, c.MountLabel(), label.IsShared(o)); err != nil {
+				if err := c.relabel(m.Source, c.MountLabel(), label.IsShared(o)); err != nil {
 					return nil, err
 				}
 
@@ -477,11 +479,11 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 		for _, o := range overlayVol.Options {
 			switch o {
 			case "U":
-				if err := chown.ChangeHostPathOwnership(overlayVol.Source, true, int(hostUID), int(hostGID)); err != nil {
+				if err := c.ChangeHostPathOwnership(overlayVol.Source, true, int(hostUID), int(hostGID)); err != nil {
 					return nil, err
 				}
 
-				if err := chown.ChangeHostPathOwnership(contentDir, true, int(hostUID), int(hostGID)); err != nil {
+				if err := c.ChangeHostPathOwnership(contentDir, true, int(hostUID), int(hostGID)); err != nil {
 					return nil, err
 				}
 			}
@@ -1709,13 +1711,13 @@ func (c *Container) makeBindMounts() error {
 		}
 
 		if c.state.BindMounts["/etc/hosts"] != "" {
-			if err := label.Relabel(c.state.BindMounts["/etc/hosts"], c.config.MountLabel, true); err != nil {
+			if err := c.relabel(c.state.BindMounts["/etc/hosts"], c.config.MountLabel, true); err != nil {
 				return err
 			}
 		}
 
 		if c.state.BindMounts["/etc/resolv.conf"] != "" {
-			if err := label.Relabel(c.state.BindMounts["/etc/resolv.conf"], c.config.MountLabel, true); err != nil {
+			if err := c.relabel(c.state.BindMounts["/etc/resolv.conf"], c.config.MountLabel, true); err != nil {
 				return err
 			}
 		}
@@ -1997,7 +1999,7 @@ func (c *Container) generateResolvConf() (string, error) {
 	}
 
 	// Relabel resolv.conf for the container
-	if err := label.Relabel(destPath, c.config.MountLabel, true); err != nil {
+	if err := c.relabel(destPath, c.config.MountLabel, true); err != nil {
 		return "", err
 	}
 
@@ -2614,7 +2616,7 @@ func (c *Container) copyTimezoneFile(zonePath string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	if err := label.Relabel(localtimeCopy, c.config.MountLabel, false); err != nil {
+	if err := c.relabel(localtimeCopy, c.config.MountLabel, false); err != nil {
 		return "", err
 	}
 	if err := dest.Chown(c.RootUID(), c.RootGID()); err != nil {
@@ -2749,3 +2751,37 @@ func (c *Container) fixVolumePermissions(v *ContainerNamedVolume) error {
 	}
 	return nil
 }
+
+func (c *Container) relabel(src, mountLabel string, recurse bool) error {
+	if !selinux.GetEnabled() || mountLabel == "" {
+		return nil
+	}
+	// only relabel on initial creation of container
+	if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateUnknown) {
+		label, err := label.FileLabel(src)
+		if err != nil {
+			return err
+		}
+		// If labels are different, might be on a tmpfs
+		if label == mountLabel {
+			return nil
+		}
+	}
+	return label.Relabel(src, mountLabel, recurse)
+}
+
+func (c *Container) ChangeHostPathOwnership(src string, recurse bool, uid, gid int) error {
+	// only chown on initial creation of container
+	if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateUnknown) {
+		st, err := os.Stat(src)
+		if err != nil {
+			return err
+		}
+
+		// If labels are different, might be on a tmpfs
+		if int(st.Sys().(*syscall.Stat_t).Uid) == uid && int(st.Sys().(*syscall.Stat_t).Gid) == gid {
+			return nil
+		}
+	}
+	return chown.ChangeHostPathOwnership(src, recurse, uid, gid)
+}
diff --git a/test/system/160-volumes.bats b/test/system/160-volumes.bats
index 7128f1b65..43462de36 100644
--- a/test/system/160-volumes.bats
+++ b/test/system/160-volumes.bats
@@ -210,6 +210,36 @@ EOF
     run_podman volume rm my_vol2
 }
 
+# Podman volume user test
+@test "podman volume user test" {
+    is_rootless || skip "only meaningful when run rootless"
+    user="1000:2000"
+    newuser="100:200"
+    tmpdir=${PODMAN_TMPDIR}/volume_$(random_string)
+    mkdir $tmpdir
+    touch $tmpdir/test1
+
+    run_podman run --name user --user $user -v $tmpdir:/data:U $IMAGE stat -c "%u:%g" /data
+    is "$output" "$user" "user should be changed"
+
+    # Now chown the source directory and make sure recursive chown happens
+    run_podman unshare chown -R $newuser $tmpdir
+    run_podman start --attach user
+    is "$output" "$user" "user should be the same"
+
+    # Now chown the file in source directory and make sure recursive chown
+    # doesn't happen
+    run_podman unshare chown -R $newuser $tmpdir/test1
+    run_podman start --attach user
+    is "$output" "$user" "user should be the same"
+    # test1 should still be chowned to $newuser
+    run_podman unshare stat -c "%u:%g" $tmpdir/test1
+    is "$output" "$newuser" "user should not be changed"
+
+    run_podman unshare rm $tmpdir/test1
+    run_podman rm user
+}
+
 
 # Confirm that container sees the correct id
 @test "podman volume with --userns=keep-id" {
diff --git a/test/system/410-selinux.bats b/test/system/410-selinux.bats
index ed9e73a3e..dbdfd4b9d 100644
--- a/test/system/410-selinux.bats
+++ b/test/system/410-selinux.bats
@@ -27,9 +27,9 @@ function check_label() {
     is "$type" "$1" "SELinux type"
 
     if [ -n "$2" ]; then
-        # e.g. from the above example -> "s0:c45,c745"
-        range=$(cut -d: -f4,5 <<<"$context")
-        is "$range" "$2^@" "SELinux range"
+	# e.g. from the above example -> "s0:c45,c745"
+	range=$(cut -d: -f4,5 <<<"$context")
+	is "$range" "$2^@" "SELinux range"
     fi
 }
 
@@ -66,9 +66,9 @@ function check_label() {
     # FIXME this test fails when run rootless with runc:
     #   Error: container_linux.go:367: starting container process caused: process_linux.go:495: container init caused: readonly path /proc/asound: operation not permitted: OCI permission denied
     if is_rootless; then
-        runtime=$(podman_runtime)
-        test "$runtime" == "crun" \
-            || skip "runtime is $runtime; this test requires crun"
+	runtime=$(podman_runtime)
+	test "$runtime" == "crun" \
+	    || skip "runtime is $runtime; this test requires crun"
     fi
 
     check_label "--pid=host" "spc_t"
@@ -96,10 +96,10 @@ function check_label() {
     skip_if_no_selinux
 
     run_podman run -d --name myc \
-               --security-opt seccomp=unconfined \
-               --security-opt label=type:spc_t \
-               --security-opt label=level:s0 \
-               $IMAGE sh -c 'while test ! -e /stop; do sleep 0.1; done'
+	       --security-opt seccomp=unconfined \
+	       --security-opt label=type:spc_t \
+	       --security-opt label=level:s0 \
+	       $IMAGE sh -c 'while test ! -e /stop; do sleep 0.1; done'
     run_podman inspect --format='{{ .HostConfig.SecurityOpt }}' myc
     is "$output" "[label=type:spc_t,label=level:s0 seccomp=unconfined]" \
       "'podman inspect' preserves all --security-opts"
@@ -118,7 +118,7 @@ function check_label() {
     skip_if_rootless_cgroupsv1
 
     if [[ $(podman_runtime) == "runc" ]]; then
-        skip "some sort of runc bug, not worth fixing (#11784)"
+	skip "some sort of runc bug, not worth fixing (#11784)"
     fi
 
     run_podman run -d --name myctr $IMAGE top
@@ -136,7 +136,7 @@ function check_label() {
     # net NS: do not share context
     run_podman run --rm --net container:myctr $IMAGE cat -v /proc/self/attr/current
     if [[ "$output" = "$context_c1" ]]; then
-        die "run --net : context ($output) is same as running container (it should not be)"
+	die "run --net : context ($output) is same as running container (it should not be)"
     fi
 
     # The 'myctr2' above was not run with --rm, so it still exists, and
@@ -158,8 +158,8 @@ function check_label() {
 
     # We don't need a fullblown pause container; avoid pulling the k8s one
     run_podman pod create --name myselinuxpod \
-               --infra-image $IMAGE \
-               --infra-command /home/podman/pause
+	       --infra-image $IMAGE \
+	       --infra-command /home/podman/pause
 
     # Get baseline
     run_podman run --rm --pod myselinuxpod $IMAGE cat -v /proc/self/attr/current
@@ -190,7 +190,7 @@ function check_label() {
     # Even after #7902, labels (':c123,c456') should be different
     run_podman run --rm --pod myselinuxpod $IMAGE cat -v /proc/self/attr/current
     if [[ "$output" = "$context_c1" ]]; then
-        die "context ($output) is the same on two separate containers, it should have been different"
+	die "context ($output) is the same on two separate containers, it should have been different"
     fi
 
     run_podman pod rm myselinuxpod
@@ -203,12 +203,12 @@ function check_label() {
     # runc and crun emit different diagnostics
     runtime=$(podman_runtime)
     case "$runtime" in
-        # crun 0.20.1 changes the error message
-        #   from /proc/thread-self/attr/exec`: .* unable to assign
-        #   to   /proc/self/attr/keycreate`: .* unable to process
-        crun) expect="\`/proc/.*\`: OCI runtime error: unable to \(assign\|process\) security attribute" ;;
-        runc) expect="OCI runtime error: .*: failed to set /proc/self/attr/keycreate on procfs" ;;
-        *)    skip "Unknown runtime '$runtime'";;
+	# crun 0.20.1 changes the error message
+	#   from /proc/thread-self/attr/exec`: .* unable to assign
+	#   to   /proc/self/attr/keycreate`: .* unable to process
+	crun) expect="\`/proc/.*\`: OCI runtime error: unable to \(assign\|process\) security attribute" ;;
+	runc) expect="OCI runtime error: .*: failed to set /proc/self/attr/keycreate on procfs" ;;
+	*)    skip "Unknown runtime '$runtime'";;
     esac
 
     # The '.*' in the error below is for dealing with podman-remote, which
@@ -223,7 +223,7 @@ function check_label() {
     LABEL="system_u:object_r:tmp_t:s0"
     RELABEL="system_u:object_r:container_file_t:s0"
     tmpdir=$PODMAN_TMPDIR/vol
-    touch $tmpdir
+    mkdir -p $tmpdir
     chcon -vR ${LABEL} $tmpdir
     ls -Z $tmpdir
 
@@ -239,12 +239,36 @@ function check_label() {
     run ls -dZ $tmpdir
     is "$output" "${RELABEL} $tmpdir" "Privileged Relabel Correctly"
 
-    run_podman run -v $tmpdir:/test:Z $IMAGE cat /proc/self/attr/current
+    run_podman run --name label -v $tmpdir:/test:Z $IMAGE cat /proc/self/attr/current
     level=$(secon -l $output)
     run ls -dZ $tmpdir
     is "$output" "system_u:object_r:container_file_t:$level $tmpdir" \
        "Confined Relabel Correctly"
 
+    if is_rootless; then
+       run_podman unshare touch $tmpdir/test1
+       # Relabel entire directory
+       run_podman unshare chcon system_u:object_r:usr_t:s0 $tmpdir
+       run_podman start --attach label
+       newlevel=$(secon -l $output)
+       is "$level" "$newlevel" "start should relabel with same SELinux labels"
+       run ls -dZ $tmpdir
+       is "$output" "system_u:object_r:container_file_t:$level $tmpdir" \
+	  "Confined Relabel Correctly"
+	run ls -dZ $tmpdir/test1
+	is "$output" "system_u:object_r:container_file_t:$level $tmpdir/test1" \
+	   "Start did not Relabel"
+
+	# Relabel only file in subdir
+	run_podman unshare chcon system_u:object_r:usr_t:s0 $tmpdir/test1
+	run_podman start --attach label
+	newlevel=$(secon -l $output)
+	is "$level" "$newlevel" "start should use same SELinux labels"
+
+	run ls -dZ $tmpdir/test1
+	is "$output" "system_u:object_r:usr_t:s0 $tmpdir/test1" \
+	   "Start did not Relabel"
+    fi
     run_podman run -v $tmpdir:/test:z $IMAGE cat /proc/self/attr/current
     run ls -dZ $tmpdir
     is "$output" "${RELABEL} $tmpdir" "Shared Relabel Correctly"