merge #4053 into opencontainers/runc:main

Kir Kolyshkin (3):
  Add dmz-vs-selinux kludge and a way to disable it
  README: fix reference to memfd-bind
  tests/int: add selinux test case

LGTMs: AkihiroSuda cyphar

Author: cyphar

.cirrus.yml

@@ -57,7 +57,7 @@ task:
     mkdir -p -m 0700 /root/.ssh
     vagrant ssh-config >> /root/.ssh/config
   guest_info_script: |
-    ssh default 'sh -exc "uname -a && systemctl --version && df -T && cat /etc/os-release && go version"'
+    ssh default 'sh -exc "uname -a && systemctl --version && df -T && cat /etc/os-release && go version && sestatus && rpm -q container-selinux"'
   check_config_script: |
     ssh default /vagrant/script/check-config.sh
   unit_tests_script: |
@@ -79,7 +79,7 @@ task:
     CIRRUS_WORKING_DIR: /home/runc
     GO_VERSION: "1.20"
     BATS_VERSION: "v1.9.0"
-    RPMS: gcc git iptables jq glibc-static libseccomp-devel make criu fuse-sshfs
+    RPMS: gcc git iptables jq glibc-static libseccomp-devel make criu fuse-sshfs container-selinux
     # yamllint disable rule:key-duplicates
     matrix:
       DISTRO: centos-7
@@ -159,6 +159,17 @@ task:
     echo -e "Host localhost\n\tStrictHostKeyChecking no\t\nIdentityFile /root/.ssh/id_ed25519\n" >> /root/.ssh/config
     sed -e "s,PermitRootLogin.*,PermitRootLogin prohibit-password,g" -i /etc/ssh/sshd_config
     systemctl restart sshd
+
+    # Disable the dmz-vs-selinux workaround for distros that have container-selinux >= 2.224.0.
+    case $DISTRO in
+    # TODO: remove centos-stream-8.
+    centos-7|centos-stream-8)
+      # Do nothing.
+      ;;
+    *)
+      echo 'export EXTRA_BUILDTAGS=runc_dmz_selinux_nocompat' >> /root/.bashrc
+      ;;
+    esac
   host_info_script: |
     uname -a
     # -----
@@ -170,6 +181,8 @@ task:
     # -----
     df -T
     # -----
+    sestatus
+    # -----
     cat /proc/cpuinfo
   check_config_script: |
     /home/runc/script/check-config.sh

README.md

@@ -69,13 +69,15 @@ make BUILDTAGS=""
 |---------------|---------------------------------------|--------------------|---------------------|
 | `seccomp`     | Syscall filtering using `libseccomp`. | yes                | `libseccomp`        |
 | `!runc_nodmz` | Reduce memory usage for CVE-2019-5736 protection by using a small C binary, [see `memfd-bind` for more details][contrib-memfd-bind]. `runc_nodmz` disables this feature and causes runc to use a different protection mechanism which will further increases memory usage temporarily during container startup. This feature can also be disabled at runtime by setting the `RUNC_DMZ=legacy` environment variable. | yes ||
+| `runc_dmz_selinux_nocompat` | Disables a SELinux DMZ workaround (new distros should set this). See [dmz README] for details. | no ||
 
 The following build tags were used earlier, but are now obsoleted:
  - **nokmem** (since runc v1.0.0-rc94 kernel memory settings are ignored)
  - **apparmor** (since runc v1.0.0-rc93 the feature is always enabled)
  - **selinux**  (since runc v1.0.0-rc93 the feature is always enabled)
 
- [contrib-memfd-bind]: /contrib/memfd-bind/README.md
+ [contrib-memfd-bind]: /contrib/cmd/memfd-bind/README.md
+ [dmz README]: /libcontainer/dmz/README.md
 
 ### Running the test suite
 

Vagrantfile.fedora

@@ -23,12 +23,18 @@ Vagrant.configure("2") do |config|
       cat << EOF | dnf -y --exclude=kernel,kernel-core shell && break
 config install_weak_deps false
 update
-install iptables gcc golang-go make glibc-static libseccomp-devel bats jq git-core criu fuse-sshfs
+install iptables gcc golang-go make glibc-static libseccomp-devel bats jq git-core criu fuse-sshfs container-selinux
 ts run
 EOF
     done
     dnf clean all
 
+    # To avoid "avc: denied { nosuid_transition }" from SELinux as we run tests on /tmp.
+    mount -o remount,suid /tmp
+
+    # Disable selinux-vs-dmz workaround as Fedora doesn't need it.
+    echo 'export EXTRA_BUILDTAGS=runc_dmz_selinux_nocompat' >> /root/.bashrc
+
     # Prevent the "fatal: unsafe repository" git complain during build.
     git config --global --add safe.directory /vagrant
 

libcontainer/container_linux.go

@@ -457,6 +457,10 @@ func slicesContains[S ~[]E, E comparable](slice S, needle E) bool {
 }
 
 func isDmzBinarySafe(c *configs.Config) bool {
+	if !dmz.WorksWithSELinux(c) {
+		return false
+	}
+
 	// Because we set the dumpable flag in nsexec, the only time when it is
 	// unsafe to use runc-dmz is when the container process would be able to
 	// race against "runc init" and bypass the ptrace_may_access() checks.

libcontainer/dmz/README.md

@@ -15,4 +15,16 @@ It also support all the architectures we support in runc.
 
 If the GOARCH we use for compiling doesn't support nolibc, it fallbacks to using the C stdlib.
 
+## SELinux compatibility issue and a workaround
+
+Older SELinux policy can prevent runc to execute the dmz binary. The issue is
+fixed in [container-selinux v2.224.0]. Yet, some older distributions may not
+have the fix, so runc has a runtime workaround of disabling dmz if it finds
+that SELinux is in enforced mode and the container SELinux label is set.
+
+Distributions that have a sufficiently new container-selinux can disable the
+workaround by building runc with the `runc_dmz_selinux_nocompat` build flag,
+essentially allowing dmz to be used together with SELinux.
+
 [nolibc-upstream]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/include/nolibc?h=v6.6-rc3
+[container-selinux v2.224.0]: https://github.com/containers/container-selinux/releases/tag/v2.224.0

libcontainer/dmz/selinux.go

@@ -0,0 +1,10 @@
+//go:build runc_dmz_selinux_nocompat || !linux
+
+package dmz
+
+import "github.com/opencontainers/runc/libcontainer/configs"
+
+// WorksWithSELinux tells whether runc-dmz can work with SELinux.
+func WorksWithSELinux(*configs.Config) bool {
+	return true
+}

libcontainer/dmz/selinux_compat.go

@@ -0,0 +1,28 @@
+//go:build linux && !runc_dmz_selinux_nocompat
+
+package dmz
+
+import (
+	"github.com/opencontainers/runc/libcontainer/configs"
+	"github.com/opencontainers/selinux/go-selinux"
+)
+
+// WorksWithSELinux tells whether runc-dmz can work with SELinux.
+//
+// Older SELinux policy can prevent runc to execute the dmz binary. The issue is
+// fixed in container-selinux >= 2.224.0:
+//
+//   - https://github.com/containers/container-selinux/issues/274
+//   - https://github.com/containers/container-selinux/pull/280
+//
+// Alas, there is is no easy way to do a runtime check if dmz works with
+// SELinux, so the below workaround is enabled by default. It results in
+// disabling dmz in case container SELinux label is set and the selinux is in
+// enforced mode.
+//
+// Newer distributions that have the sufficiently new container-selinux version
+// can build runc with runc_dmz_selinux_nocompat build flag to disable this
+// workaround (essentially allowing dmz to be used together with SELinux).
+func WorksWithSELinux(c *configs.Config) bool {
+	return c.ProcessLabel == "" || selinux.EnforceMode() != selinux.Enforcing
+}

tests/integration/selinux.bats

@@ -0,0 +1,55 @@
+#!/usr/bin/env bats
+
+load helpers
+
+function setup() {
+	requires root # for chcon
+	if ! selinuxenabled; then
+		skip "requires SELinux enabled and in enforcing mode"
+	fi
+
+	setup_busybox
+
+	# Use a copy of runc binary with proper selinux label set.
+	cp "$RUNC" .
+	export RUNC="$PWD/runc"
+	chcon -u system_u -r object_r -t container_runtime_exec_t "$RUNC"
+
+	# Label container fs.
+	chcon -u system_u -r object_r -t container_file_t -R rootfs
+
+	# Save the start date and time for ausearch.
+	AU_DD="$(date +%x)"
+	AU_TT="$(date +%H:%M:%S)"
+}
+
+function teardown() {
+	teardown_bundle
+	# Show any avc denials.
+	if [[ -v AU_DD && -v AU_TT ]] && command -v ausearch &>/dev/null; then
+		ausearch -ts "$AU_DD" "$AU_TT" -i -m avc || true
+	fi
+}
+
+# Baseline test, to check that runc works with selinux enabled.
+@test "runc run (no selinux label)" {
+	update_config '	  .process.args = ["/bin/true"]'
+	runc run tst
+	[ "$status" -eq 0 ]
+}
+
+# https://github.com/opencontainers/runc/issues/4057
+@test "runc run (custom selinux label)" {
+	update_config '	  .process.selinuxLabel |= "system_u:system_r:container_t:s0:c4,c5"
+			| .process.args = ["/bin/true"]'
+	runc run tst
+	[ "$status" -eq 0 ]
+}
+
+@test "runc run (custom selinux label, RUNC_DMZ=legacy)" {
+	export RUNC_DMZ=legacy
+	update_config '	  .process.selinuxLabel |= "system_u:system_r:container_t:s0:c4,c5"
+			| .process.args = ["/bin/true"]'
+	runc run tst
+	[ "$status" -eq 0 ]
+}