diff options
author | OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com> | 2021-10-18 19:53:46 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-10-18 19:53:46 +0200 |
commit | e0ffc431fe7f016124fdcb36819698a90fe448a9 (patch) | |
tree | eadd9bb4bed1e6a9815f426e11df01afbcefc4aa | |
parent | abc6527c9659bc7153ce68794f2e90e14880391c (diff) | |
parent | 207abc4a9ada6dcc03420a02e243041d2f2f1c79 (diff) | |
download | podman-e0ffc431fe7f016124fdcb36819698a90fe448a9.tar.gz podman-e0ffc431fe7f016124fdcb36819698a90fe448a9.tar.bz2 podman-e0ffc431fe7f016124fdcb36819698a90fe448a9.zip |
Merge pull request #11959 from rhatdan/selinux
We should only be relabeling when on first run
-rw-r--r-- | libpod/container_internal_linux.go | 56 | ||||
-rw-r--r-- | test/system/160-volumes.bats | 30 | ||||
-rw-r--r-- | test/system/410-selinux.bats | 70 |
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" |