Skip to content

Commit

Permalink
Merge pull request #3753 from kolyshkin/user-exec
Browse files Browse the repository at this point in the history
Fix runc run "permission denied" when rootless
  • Loading branch information
AkihiroSuda authored Mar 28, 2023
2 parents 2b221a6 + 8293ef2 commit 7f3f4be
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 25 deletions.
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 {
// 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

0 comments on commit 7f3f4be

Please sign in to comment.