diff --git a/Dockerfile b/Dockerfile index 1504b4b1ba3..9610f430645 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -ARG GO_VERSION=1.19 +ARG GO_VERSION=1.20 ARG BATS_VERSION=v1.3.0 ARG LIBSECCOMP_VERSION=2.5.4 diff --git a/libcontainer/eaccess_go119.go b/libcontainer/eaccess_go119.go new file mode 100644 index 00000000000..cc1e2079a79 --- /dev/null +++ b/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) +} diff --git a/libcontainer/eaccess_stub.go b/libcontainer/eaccess_stub.go new file mode 100644 index 00000000000..7c049fd7aa0 --- /dev/null +++ b/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 +} diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go index 6ad25c9a4df..32de78eb20d 100644 --- a/libcontainer/standard_init_linux.go +++ b/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 diff --git a/libcontainer/system/linux.go b/libcontainer/system/linux.go index 039059a444c..e1d6eb18034 100644 --- a/libcontainer/system/linux.go +++ b/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 { diff --git a/tests/integration/start_hello.bats b/tests/integration/start_hello.bats index 77398c951f2..87005484748 100644 --- a/tests/integration/start_hello.bats +++ b/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