Skip to content

Commit

Permalink
Merge pull request containers#11959 from rhatdan/selinux
Browse files Browse the repository at this point in the history 
We should only be relabeling when on first run
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot authored Oct 18, 2021
2 parents abc6527 + 207abc4 commit e0ffc43
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 e0ffc43

Please sign in to comment.