Merge pull request #3817 from kolyshkin/1.1-user-exec

[1.1] Fix runc run "permission denied" when rootless

LGTMs: @AkihiroSuda, @mrunalp.

libcontainer/eaccess_go119.go

@@ -0,0 +1,17 @@
+//go:build !go1.20
+// +build !go1.20
+
+package libcontainer
+
+import "golang.org/x/sys/unix"
+
+func eaccess(path string) error {
+	// This check is similar to access(2) with X_OK except for
+	// setuid/setgid binaries where it checks against the effective
+	// (rather than real) uid and gid. It is not needed in go 1.20
+	// and beyond and will be removed later.
+
+	// Relies on code added in https://go-review.googlesource.com/c/sys/+/468877
+	// and older CLs linked from there.
+	return unix.Faccessat(unix.AT_FDCWD, path, unix.X_OK, unix.AT_EACCESS)
+}

libcontainer/eaccess_stub.go

@@ -0,0 +1,10 @@
+//go:build go1.20
+
+package libcontainer
+
+func eaccess(path string) error {
+	// Not needed in Go 1.20+ as the functionality is already in there
+	// (added by https://go.dev/cl/416115, https://go.dev/cl/414824,
+	// and fixed in Go 1.20.2 by https://go.dev/cl/469956).
+	return nil
+}

libcontainer/standard_init_linux.go

@@ -198,11 +198,12 @@ func (l *linuxStandardInit) Init() error {
 	if err != nil {
 		return err
 	}
-	// exec.LookPath might return no error for an executable residing on a
-	// file system mounted with noexec flag, so perform this extra check
-	// now while we can still return a proper error.
-	if err := system.Eaccess(name); err != nil {
-		return &os.PathError{Op: "exec", Path: name, Err: err}
+	// exec.LookPath in Go < 1.20 might return no error for an executable
+	// residing on a file system mounted with noexec flag, so perform this
+	// extra check now while we can still return a proper error.
+	// TODO: remove this once go < 1.20 is not supported.
+	if err := eaccess(name); err != nil {
+		return &os.PathError{Op: "eaccess", Path: name, Err: err}
 	}
 
 	// Set seccomp as close to execve as possible, so as few syscalls take

libcontainer/system/linux.go

@@ -31,25 +31,6 @@ func (p ParentDeathSignal) Set() error {
 	return SetParentDeathSignal(uintptr(p))
 }
 
-// Eaccess is similar to unix.Access except for setuid/setgid binaries
-// it checks against the effective (rather than real) uid and gid.
-func Eaccess(path string) error {
-	err := unix.Faccessat2(unix.AT_FDCWD, path, unix.X_OK, unix.AT_EACCESS)
-	if err != unix.ENOSYS && err != unix.EPERM { //nolint:errorlint // unix errors are bare
-		return err
-	}
-
-	// Faccessat2() not available; check if we are a set[ug]id binary.
-	if os.Getuid() == os.Geteuid() && os.Getgid() == os.Getegid() {
-		// For a non-set[ug]id binary, use access(2).
-		return unix.Access(path, unix.X_OK)
-	}
-
-	// For a setuid/setgid binary, there is no fallback way
-	// so assume we can execute the binary.
-	return nil
-}
-
 func Execv(cmd string, args []string, env []string) error {
 	name, err := exec.LookPath(cmd)
 	if err != nil {

tests/integration/start_hello.bats

@@ -37,6 +37,33 @@ function teardown() {
 	[[ "${output}" == *"Hello"* ]]
 }
 
+# https://github.com/opencontainers/runc/issues/3715.
+#
+# Fails when using Go 1.20 < 1.20.2, the reasons is https://go.dev/issue/58552.
+@test "runc run as user with no exec bit but CAP_DAC_OVERRIDE set" {
+	requires root # Can't chown/chmod otherwise.
+
+	# Remove exec perm for everyone but owner (root).
+	chown 0 rootfs/bin/echo
+	chmod go-x rootfs/bin/echo
+
+	# Replace "uid": 0 with "uid": 1000 and do a similar thing for gid.
+	update_config '	  (.. | select(.uid? == 0)) .uid |= 1000
+			| (.. | select(.gid? == 0)) .gid |= 100'
+
+	# Sanity check: make sure we can't run the container w/o CAP_DAC_OVERRIDE.
+	runc run test_busybox
+	[ "$status" -ne 0 ]
+
+	# Enable CAP_DAC_OVERRIDE.
+	update_config '	  .process.capabilities.bounding += ["CAP_DAC_OVERRIDE"]
+			| .process.capabilities.effective += ["CAP_DAC_OVERRIDE"]
+			| .process.capabilities.permitted += ["CAP_DAC_OVERRIDE"]'
+
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+}
+
 @test "runc run with rootfs set to ." {
 	cp config.json rootfs/.
 	rm config.json