Fix stdio permission error in userns container

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,

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}
 		}
 	}

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"]}] '

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 {

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
 	}