Repository: opencontainers/runc
Pull request #2712 (2020-12-18 21:42:13)

libct/init_linux: reorder chdir to fix EPERM
_This is a carry of https://github.com/opencontainers/runc/pull/2685, adding test cases for it, as well for https://github.com/opencontainers/runc/pull/2086 to make sure there is no regression. Most work was done by @haircommander, I just tinkered with the tests. Original description from Peter follows._

----

commit 5e0e67d moved the chdir to be one of the
first steps of finalizing the namespace of the container.

However, this causes issues when the cwd is not accessible by the user running runc, but rather
as the container user.

Thus, setupUser has to happen before we call chdir. setupUser still happens before setting the caps,
so the user should be privileged enough to mitigate the issues fixed in 5e0e67d (I've tested it on my machine, so I believe it does not regress)

----

Closes: #2685


Patch:
diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go
index 575dd461..bb8ff0b5 100644
--- a/libcontainer/init_linux.go
+++ b/libcontainer/init_linux.go
@@ -129,12 +129,6 @@ func finalizeNamespace(config *initConfig) error {
 		return errors.Wrap(err, "close exec fds")
 	}
 
-	if config.Cwd != "" {
-		if err := unix.Chdir(config.Cwd); err != nil {
-			return fmt.Errorf("chdir to cwd (%q) set in config.json failed: %v", config.Cwd, err)
-		}
-	}
-
 	capabilities := &configs.Capabilities{}
 	if config.Capabilities != nil {
 		capabilities = config.Capabilities
@@ -156,6 +150,14 @@ func finalizeNamespace(config *initConfig) error {
 	if err := setupUser(config); err != nil {
 		return errors.Wrap(err, "setup user")
 	}
+	// Change working directory AFTER the user has been set up.
+	// Otherwise, if the cwd is also a volume that's been chowned to the container user (and not the user running runc),
+	// this command will EPERM.
+	if config.Cwd != "" {
+		if err := unix.Chdir(config.Cwd); err != nil {
+			return fmt.Errorf("chdir to cwd (%q) set in config.json failed: %v", config.Cwd, err)
+		}
+	}
 	if err := system.ClearKeepCaps(); err != nil {
 		return errors.Wrap(err, "clear keep caps")
 	}


Test patch:
diff --git a/tests/integration/cwd.bats b/tests/integration/cwd.bats
new file mode 100644
index 00000000..6fe533a9
--- /dev/null
+++ b/tests/integration/cwd.bats
@@ -0,0 +1,59 @@
+#!/usr/bin/env bats
+
+load helpers
+
+function setup() {
+	teardown_busybox
+	setup_busybox
+}
+
+function teardown() {
+	teardown_busybox
+}
+
+# Test case for https://github.com/opencontainers/runc/pull/2086
+@test "runc exec --user with no access to cwd" {
+	requires root
+
+	chown 42 rootfs/root
+	chmod 700 rootfs/root
+
+	update_config '	  .process.cwd = "/root"
+			| .process.user.uid = 42
+			| .process.args |= ["sleep", "1h"]'
+
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox
+	[ "$status" -eq 0 ]
+
+	runc exec --user 0 test_busybox true
+	[ "$status" -eq 0 ]
+}
+
+# Verify a cwd owned by the container user can be chdir'd to,
+# even if runc doesn't have the privilege to do so.
+@test "runc create sets up user before chdir to cwd" {
+	requires rootless rootless_idmap
+
+	# Some setup for this test (AUX_DIR and AUX_UID) is done
+	# by rootless.sh. Check that setup is done...
+	if [[ ! -d "$AUX_DIR" || -z "$AUX_UID" ]]; then
+		skip "bad/unset AUX_DIR/AUX_UID"
+	fi
+	# ... and is correct, i.e. the current user
+	# does not have permission to access AUX_DIR.
+	if ls -l "$AUX_DIR" 2>/dev/null; then
+		skip "bad AUX_DIR permissions"
+	fi
+
+	update_config '   .mounts += [{
+				source: "'"$AUX_DIR"'",
+				destination: "'"$AUX_DIR"'",
+				options: ["bind"]
+			    }]
+			| .process.user.uid = '"$AUX_UID"'
+			| .process.cwd = "'"$AUX_DIR"'"
+			| .process.args |= ["ls", "'"$AUX_DIR"'"]'
+
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+}
diff --git a/tests/rootless.sh b/tests/rootless.sh
index e298cc61..929fec2c 100755
--- a/tests/rootless.sh
+++ b/tests/rootless.sh
@@ -50,6 +50,18 @@ function enable_idmap() {
 	# Reactivate new{uid,gid}map helpers if applicable.
 	[ -e /usr/bin/unused-newuidmap ] && mv /usr/bin/{unused-,}newuidmap
 	[ -e /usr/bin/unused-newgidmap ] && mv /usr/bin/{unused-,}newgidmap
+
+	# Create a directory owned by $AUX_UID inside container, to be used
+	# by a test case in cwd.bats. This setup can't be done by the test itself,
+	# as it needs root for chown.
+	set -e
+	export AUX_UID=1024
+	AUX_DIR="$(mktemp -d)"
+	# 1000 is linux.uidMappings.containerID value,
+	# as set by runc_rootless_idmap
+	chown "$((ROOTLESS_UIDMAP_START - 1000 + AUX_UID))" "$AUX_DIR"
+	export AUX_DIR
+	set +e
 }
 
 function disable_idmap() {
@@ -65,6 +77,13 @@ function disable_idmap() {
 	[ -e /usr/bin/newgidmap ] && mv /usr/bin/{,unused-}newgidmap
 }
 
+function cleanup() {
+	if [ -n "$AUX_DIR" ]; then
+		rmdir "$AUX_DIR"
+		unset AUX_DIX
+	fi
+}
+
 # FEATURE: Opportunistic cgroups support, allowing a rootless container to set
 #          resource limits on condition that cgroupsPath is set to a path the
 #          rootless user has permissions on.
@@ -151,4 +170,5 @@ for enabled_features in $features_powerset; do
 		sudo -HE -u rootless PATH="$PATH" $(which bats) -t "$ROOT/tests/integration$ROOTLESS_TESTPATH"
 	fi
 	set +e
+	cleanup
 done