Skip to content

Commit

Permalink
We should only be relabeling when on first run
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@rhatdan
rhatdan committed Oct 15, 2021
1 parent 171f7b8 commit 207abc4
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 33 deletions.
56 changes: 46 additions & 10 deletions libpod/container_internal_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
30 changes: 30 additions & 0 deletions test/system/160-volumes.bats
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down
70 changes: 47 additions & 23 deletions test/system/410-selinux.bats
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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"
Expand Down

0 comments on commit 207abc4

Please sign in to comment.