Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix runc run "permission denied" when rootless #3753

Merged
merged 3 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -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

Expand Down
17 changes: 17 additions & 0 deletions libcontainer/eaccess_go119.go
Original file line number Diff line number Diff line change
@@ -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)
}
10 changes: 10 additions & 0 deletions libcontainer/eaccess_stub.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
//go:build go1.20

package libcontainer

func eaccess(path string) error {
kolyshkin marked this conversation as resolved.
Show resolved Hide resolved
// 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
}
11 changes: 6 additions & 5 deletions libcontainer/standard_init_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 0 additions & 19 deletions libcontainer/system/linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
27 changes: 27 additions & 0 deletions tests/integration/start_hello.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down