Merge pull request #2712 from kolyshkin/fix-chdir-II

libct/init_linux: reorder chdir to fix EPERM

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")
 	}

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 ]
+}

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