diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index 1eb0279d9e0..ea6009d5814 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -374,7 +374,7 @@ func setupConsole(socket *os.File, config *initConfig, mount bool) error { // used to change the owner of the slave path, but since the /dev/pts mount // can have gid=X set (at the users' option). So touching the owner of the // slave PTY is not necessary, as the kernel will handle that for us. Note - // however, that setupUser (specifically fixStdioPermissions) *will* change + // however, that setupUser (specifically FixStdioPermissions) *will* change // the UID owner of the console to be the user the process will run as (so // they can actually control their console). @@ -503,7 +503,7 @@ func setupUser(config *initConfig) error { // Before we change to the container's user make sure that the processes // STDIO is correctly owned by the user that we are switching to. - if err := fixStdioPermissions(execUser); err != nil { + if err := FixStdioPermissions(execUser.Uid); err != nil { return err } @@ -550,10 +550,10 @@ func setupUser(config *initConfig) error { return nil } -// fixStdioPermissions fixes the permissions of PID 1's STDIO within the container to the specified user. +// FixStdioPermissions fixes the permissions of STDIO within the container to the specified user. // The ownership needs to match because it is created outside of the container and needs to be // localized. -func fixStdioPermissions(u *user.ExecUser) error { +func FixStdioPermissions(uid int) error { var null unix.Stat_t if err := unix.Stat("/dev/null", &null); err != nil { return &os.PathError{Op: "stat", Path: "/dev/null", Err: err} @@ -566,7 +566,7 @@ func fixStdioPermissions(u *user.ExecUser) error { // Skip chown if uid is already the one we want or any of the STDIO descriptors // were redirected to /dev/null. - if int(s.Uid) == u.Uid || s.Rdev == null.Rdev { + if int(s.Uid) == uid || s.Rdev == null.Rdev { continue } @@ -576,7 +576,7 @@ func fixStdioPermissions(u *user.ExecUser) error { // that users expect to be able to actually use their console. Without // this code, you couldn't effectively run as a non-root user inside a // container and also have a console set up. - if err := file.Chown(u.Uid, int(s.Gid)); err != nil { + if err := file.Chown(uid, int(s.Gid)); err != nil { // If we've hit an EINVAL then s.Gid isn't mapped in the user // namespace. If we've hit an EPERM then the inode's current owner // is not mapped in our user namespace (in particular, diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index fcbb54a3e41..12c3011d27e 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -917,7 +917,7 @@ func getPipeFds(pid int) ([]string, error) { // opposite side for each. Do not use this if you want to have a pseudoterminal // set up for you by libcontainer (TODO: fix that too). // TODO: This is mostly unnecessary, and should be handled by clients. -func (p *Process) InitializeIO(rootuid, rootgid int) (i *IO, err error) { +func (p *Process) InitializeIO(containerUID, containerGID int) (i *IO, err error) { var fds []uintptr i = &IO{} // cleanup in case of an error @@ -949,7 +949,11 @@ func (p *Process) InitializeIO(rootuid, rootgid int) (i *IO, err error) { p.Stderr, i.Stderr = w, r // change ownership of the pipes in case we are in a user namespace for _, fd := range fds { - if err := unix.Fchown(int(fd), rootuid, rootgid); err != nil { + if err := unix.Fchown(int(fd), containerUID, containerGID); err != nil { + if errors.Is(err, unix.EINVAL) || errors.Is(err, unix.EPERM) || errors.Is(err, unix.EROFS) { + // Let's wait to do chown in the container init + continue + } return nil, &os.PathError{Op: "fchown", Path: "fd " + strconv.Itoa(int(fd)), Err: err} } } diff --git a/tests/integration/userns.bats b/tests/integration/userns.bats index 78583ba4d45..d962de8b56e 100644 --- a/tests/integration/userns.bats +++ b/tests/integration/userns.bats @@ -36,6 +36,70 @@ function teardown() { fi } +@test "check stdio permission for root in userns [terminal=false && detached]" { + update_config ' .process.terminal = false + | .process.args = ["sh", "-c", "echo errormsg > /dev/stderr"]' + + touch log + __runc create test_busybox >log 2>&1 + + runc start test_busybox + [ "$status" -eq 0 ] + + wait_for_container 10 1 test_busybox stopped + + out=$(cat log) + # Keep this to debug is useful once we have a regression about this. + echo "$out" >&2 + + # We should let stdio could be accessed in userns container. + # Please see https://github.com/opencontainers/runc/issues/4475 + [[ "$out" = "errormsg" ]] +} + +@test "check stdio permission for root in userns [terminal=false && !detached]" { + update_config ' .process.terminal = false + | .process.args = ["sh", "-c", "echo errormsg > /dev/stderr"]' + + runc run test_busybox + [ "$status" -eq 0 ] + [[ "$output" = "errormsg" ]] +} + +@test "check stdio permission for non-root user in userns [terminal=false && detached]" { + requires root + update_config ' .process.terminal = false + | .process.user.uid = 1 + | .process.user.gid = 1 + | .process.args = ["sh", "-c", "echo errormsg > /dev/stderr"]' + + touch log + __runc create test_busybox >log 2>&1 + + runc start test_busybox + [ "$status" -eq 0 ] + + wait_for_container 10 1 test_busybox stopped + + out=$(cat log) + # Keep this to debug is useful once we have a regression about this. + echo "$out" >&2 + + [[ "$out" = "errormsg" ]] +} + +@test "check stdio permission for non-root user in userns [terminal=false && !detached]" { + requires root + update_config ' .process.terminal = false + | .process.user.uid = 1 + | .process.user.gid = 1 + | .process.args = ["sh", "-c", "echo errormsg > /dev/stderr"]' + + runc run test_busybox + [ "$status" -eq 0 ] + [[ "$output" = "errormsg" ]] +} + @test "userns with simple mount" { update_config ' .process.args += ["-c", "stat /tmp/mount-1/foo.txt"] | .mounts += [{"source": "source-accessible/dir", "destination": "/tmp/mount-1", "options": ["bind"]}] ' diff --git a/tty.go b/tty.go index c101aacb78b..538ae7ddce2 100644 --- a/tty.go +++ b/tty.go @@ -31,8 +31,8 @@ func (t *tty) copyIO(w io.Writer, r io.ReadCloser) { // setup pipes for the process so that advanced features like c/r are able to easily checkpoint // and restore the process's IO without depending on a host specific path or device -func setupProcessPipes(p *libcontainer.Process, rootuid, rootgid int) (*tty, error) { - i, err := p.InitializeIO(rootuid, rootgid) +func setupProcessPipes(p *libcontainer.Process, containerUID, containerGID int) (*tty, error) { + i, err := p.InitializeIO(containerUID, containerGID) if err != nil { return nil, err } @@ -63,10 +63,16 @@ func setupProcessPipes(p *libcontainer.Process, rootuid, rootgid int) (*tty, err return t, nil } -func inheritStdio(process *libcontainer.Process) { +func inheritStdio(process *libcontainer.Process, containerUID int) error { + if containerUID != os.Getuid() { + if err := libcontainer.FixStdioPermissions(containerUID); err != nil { + return err + } + } process.Stdin = os.Stdin process.Stdout = os.Stdout process.Stderr = os.Stderr + return nil } func (t *tty) initHostConsole() error { diff --git a/utils_linux.go b/utils_linux.go index feb6ef80c4a..23b6925176f 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -94,7 +94,7 @@ func newProcess(p specs.Process) (*libcontainer.Process, error) { } // setupIO modifies the given process config according to the options. -func setupIO(process *libcontainer.Process, rootuid, rootgid int, createTTY, detach bool, sockpath string) (*tty, error) { +func setupIO(process *libcontainer.Process, containerUID, containerGID int, createTTY, detach bool, sockpath string) (*tty, error) { if createTTY { process.Stdin = nil process.Stdout = nil @@ -137,10 +137,9 @@ func setupIO(process *libcontainer.Process, rootuid, rootgid int, createTTY, det // when runc will detach the caller provides the stdio to runc via runc's 0,1,2 // and the container's process inherits runc's stdio. if detach { - inheritStdio(process) - return &tty{}, nil + return &tty{}, inheritStdio(process, containerUID) } - return setupProcessPipes(process, rootuid, rootgid) + return setupProcessPipes(process, containerUID, containerGID) } // createPidFile creates a file containing the PID, @@ -237,11 +236,11 @@ func (r *runner) run(config *specs.Process) (int, error) { } process.ExtraFiles = append(process.ExtraFiles, os.NewFile(uintptr(i), "PreserveFD:"+strconv.Itoa(i))) } - rootuid, err := r.container.Config().HostRootUID() + containerUID, err := r.container.Config().HostUID(int(config.User.UID)) if err != nil { return -1, err } - rootgid, err := r.container.Config().HostRootGID() + containerGID, err := r.container.Config().HostGID(int(config.User.GID)) if err != nil { return -1, err } @@ -250,7 +249,7 @@ func (r *runner) run(config *specs.Process) (int, error) { // with detaching containers, and then we get a tty after the container has // started. handler := newSignalHandler(r.enableSubreaper, r.notifySocket) - tty, err := setupIO(process, rootuid, rootgid, config.Terminal, detach, r.consoleSocket) + tty, err := setupIO(process, containerUID, containerGID, config.Terminal, detach, r.consoleSocket) if err != nil { return -1, err }