console: don't chown(2) the slave PTY

Since the gid=X and mode=Y flags can be set inside config.json as mount
options, don't override them with our own defaults. This avoids
/dev/pts/* not being owned by tty in a regular container, as well as all
of the issues with us implementing grantpt(3) manually. This is the
least opinionated approach to take.

This patch is part of the console rewrite patchset.

Reported-by: Mrunal Patel <[email protected]>
Signed-off-by: Aleksa Sarai <[email protected]>

Author: cyphar

libcontainer/console_freebsd.go

@@ -8,6 +8,6 @@ import (
 
 // newConsole returns an initialized console that can be used within a container by copying bytes
 // from the master side to the slave that is attached as the tty for the container's init process.
-func newConsole(uid, gid int) (Console, error) {
+func newConsole() (Console, error) {
 	return nil, errors.New("libcontainer console is not supported on FreeBSD")
 }

libcontainer/console_linux.go

@@ -11,7 +11,7 @@ import (
 
 // newConsole returns an initialized console that can be used within a container by copying bytes
 // from the master side to the slave that is attached as the tty for the container's init process.
-func newConsole(uid, gid int) (Console, error) {
+func newConsole() (Console, error) {
 	master, err := os.OpenFile("/dev/ptmx", syscall.O_RDWR|syscall.O_NOCTTY|syscall.O_CLOEXEC, 0)
 	if err != nil {
 		return nil, err
@@ -26,12 +26,6 @@ func newConsole(uid, gid int) (Console, error) {
 	if err := unlockpt(master); err != nil {
 		return nil, err
 	}
-	if err := os.Chmod(console, 0600); err != nil {
-		return nil, err
-	}
-	if err := os.Chown(console, uid, gid); err != nil {
-		return nil, err
-	}
 	return &linuxConsole{
 		slavePath: console,
 		master:    master,

libcontainer/console_solaris.go

@@ -6,6 +6,6 @@ import (
 
 // newConsole returns an initialized console that can be used within a container by copying bytes
 // from the master side to the slave that is attached as the tty for the container's init process.
-func newConsole(uid, gid int) (Console, error) {
+func newConsole() (Console, error) {
 	return nil, errors.New("libcontainer console is not supported on Solaris")
 }

libcontainer/console_windows.go

@@ -1,7 +1,7 @@
 package libcontainer
 
 // newConsole returns an initialized console that can be used within a container
-func newConsole(uid, gid int) (Console, error) {
+func newConsole() (Console, error) {
 	return &windowsConsole{}, nil
 }
 

libcontainer/init_linux.go

@@ -157,8 +157,14 @@ func finalizeNamespace(config *initConfig) error {
 // issues related to that). This has to be run *after* we've pivoted to the new
 // rootfs (and the users' configuration is entirely set up).
 func setupConsole(pipe *os.File, config *initConfig, mount bool) error {
-	// At this point, /dev/ptmx points to something that we would expect.
-	console, err := newConsole(0, 0)
+	// At this point, /dev/ptmx points to something that we would expect. We
+	// 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
+	// the UID owner of the console to be the user the process will run as (so
+	// they can actually control their console).
+	console, err := newConsole()
 	if err != nil {
 		return err
 	}
@@ -309,11 +315,17 @@ func fixStdioPermissions(u *user.ExecUser) error {
 		if err := syscall.Fstat(int(fd), &s); err != nil {
 			return err
 		}
-		// skip chown of /dev/null if it was used as one of the STDIO fds.
+		// Skip chown of /dev/null if it was used as one of the STDIO fds.
 		if s.Rdev == null.Rdev {
 			continue
 		}
-		if err := syscall.Fchown(int(fd), u.Uid, u.Gid); err != nil {
+		// We only change the uid owner (as it is possible for the mount to
+		// prefer a different gid, and there's no reason for us to change it).
+		// The reason why we don't just leave the default uid=X mount setup is
+		// 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 := syscall.Fchown(int(fd), u.Uid, int(s.Gid)); err != nil {
 			return err
 		}
 	}