From 531ef794e4ecd628006a865ad334a048ee2b4b2e Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Thu, 15 May 2025 16:12:21 +1000 Subject: [PATCH] console: use TIOCGPTPEER when allocating peer PTY When opening the peer end of a pty, the old kernel API required us to open /dev/pts/$num inside the container (at least since we fixed console handling many years ago in commit 244c9fc426ae ("*: console rewrite")). The problem is that in a hostile container it is possible for /dev/pts/$num to be an attacker-controlled symlink that runc can be tricked into resolving when doing bind-mounts. This allows the attacker to (among other things) persist /proc/... entries that are later masked by runc, allowing an attacker to escape through the kernel.core_pattern sysctl (/proc/sys/kernel/core_pattern). This is the original issue reported by Lei Wang and Li Fu Bang in CVE-2025-52565. However, it should be noted that this is not entirely a newly-discovered problem. Way back in Linux 4.13 (2017), I added the TIOCGPTPEER ioctl, which allows us to get a pty peer without touching the /dev/pts inside the container. The original threat model was around an attacker replacing /dev/pts/$n or /dev/pts/ptmx with some malicious inode (a DoS inode, or possibly a PTY they wanted a confused deputy to operate on). Unfortunately, there was no practical way for runc to cache a safe O_PATH handle to /dev/pts/ptmx (unlike other runtimes like LXC, which switched to TIOCGPTPEER way back in 2017). Since it wasn't clear how we could protect against the main attack TIOCGPTPEER was meant to protect against, we never switched to it (even though I implemented it specifically to harden container runtimes). Unfortunately, It turns out that mount *sources* are a threat we didn't fully consider. Since TIOCGPTPEER already solves this problem entirely for us in a race free way, we should just use that. In a later patch, we will add some hardening for /dev/pts/$num opening to maintain support for very old kernels (Linux 4.13 is very old at this point, but RHEL 7 is still kicking and is stuck on Linux 3.10). Fixes: GHSA-qw9x-cqr3-wc7r CVE-2025-52565 Reported-by: Lei Wang (CVE-2025-52565) Reported-by: lfbzhm (CVE-2025-52565) Reported-by: Aleksa Sarai (TIOCGPTPEER) Signed-off-by: Aleksa Sarai --- internal/linux/linux.go | 20 ++++++++++++++ libcontainer/console_linux.go | 50 ++++++++++++++++++++++++++--------- libcontainer/init_linux.go | 7 ++--- 3 files changed, 61 insertions(+), 16 deletions(-) diff --git a/internal/linux/linux.go b/internal/linux/linux.go index 1cf479f41..ef230b133 100644 --- a/internal/linux/linux.go +++ b/internal/linux/linux.go @@ -85,3 +85,23 @@ func SetMempolicy(mode uint, mask *unix.CPUSet) error { }) return os.NewSyscallError("set_mempolicy", err) } + +// GetPtyPeer is a wrapper for ioctl(TIOCGPTPEER). +func GetPtyPeer(ptyFd uintptr, unsafePeerPath string, flags int) (*os.File, error) { + // Make sure O_NOCTTY is always set -- otherwise runc might accidentally + // gain it as a controlling terminal. O_CLOEXEC also needs to be set to + // make sure we don't leak the handle either. + flags |= unix.O_NOCTTY | unix.O_CLOEXEC + + // There is no nice wrapper for this kind of ioctl in unix. + peerFd, _, errno := unix.Syscall( + unix.SYS_IOCTL, + ptyFd, + uintptr(unix.TIOCGPTPEER), + uintptr(flags), + ) + if errno != 0 { + return nil, os.NewSyscallError("ioctl TIOCGPTPEER", errno) + } + return os.NewFile(peerFd, unsafePeerPath), nil +} diff --git a/libcontainer/console_linux.go b/libcontainer/console_linux.go index c42e60e3c..36db6d69b 100644 --- a/libcontainer/console_linux.go +++ b/libcontainer/console_linux.go @@ -1,15 +1,39 @@ package libcontainer import ( + "fmt" "os" + "runtime" + + "github.com/containerd/console" + "golang.org/x/sys/unix" "github.com/opencontainers/runc/internal/linux" - "golang.org/x/sys/unix" ) -// mount initializes the console inside the rootfs mounting with the specified mount label -// and applying the correct ownership of the console. -func mountConsole(slavePath string) error { +// safeAllocPty returns a new (ptmx, peer pty) allocation for use inside a +// container. +func safeAllocPty() (pty console.Console, peer *os.File, Err error) { + pty, unsafePeerPath, err := console.NewPty() + if err != nil { + return nil, nil, err + } + defer func() { + if Err != nil { + _ = pty.Close() + } + }() + + peer, err = linux.GetPtyPeer(pty.Fd(), unsafePeerPath, unix.O_RDWR|unix.O_NOCTTY) + if err != nil { + return nil, nil, fmt.Errorf("failed to get peer end of newly-allocated console: %w", err) + } + return pty, peer, nil +} + +// mountConsole bind-mounts the provided pty on top of /dev/console so programs +// that operate on /dev/console operate on the correct container pty. +func mountConsole(peerPty *os.File) error { f, err := os.Create("/dev/console") if err != nil && !os.IsExist(err) { return err @@ -21,20 +45,20 @@ func mountConsole(slavePath string) error { } f.Close() } - return mount(slavePath, "/dev/console", "bind", unix.MS_BIND, "") + mntSrc := &mountSource{ + Type: mountSourcePlain, + file: peerPty, + } + return mountViaFds(peerPty.Name(), mntSrc, "/dev/console", "", "bind", unix.MS_BIND, "") } -// dupStdio opens the slavePath for the console and dups the fds to the current -// processes stdio, fd 0,1,2. -func dupStdio(slavePath string) error { - fd, err := linux.Open(slavePath, unix.O_RDWR, 0) - if err != nil { - return err - } +// dupStdio replaces stdio with the given peerPty. +func dupStdio(peerPty *os.File) error { for _, i := range []int{0, 1, 2} { - if err := linux.Dup3(fd, i, 0); err != nil { + if err := linux.Dup3(int(peerPty.Fd()), i, 0); err != nil { return err } } + runtime.KeepAlive(peerPty) return nil } diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index 95a4d7f78..9672b037a 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -377,12 +377,13 @@ func setupConsole(socket *os.File, config *initConfig, mount bool) error { // the UID owner of the console to be the user the process will run as (so // they can actually control their console). - pty, slavePath, err := console.NewPty() + pty, peerPty, err := safeAllocPty() if err != nil { return err } // After we return from here, we don't need the console anymore. defer pty.Close() + defer peerPty.Close() if config.ConsoleHeight != 0 && config.ConsoleWidth != 0 { err = pty.Resize(console.WinSize{ @@ -396,7 +397,7 @@ func setupConsole(socket *os.File, config *initConfig, mount bool) error { // Mount the console inside our rootfs. if mount { - if err := mountConsole(slavePath); err != nil { + if err := mountConsole(peerPty); err != nil { return err } } @@ -407,7 +408,7 @@ func setupConsole(socket *os.File, config *initConfig, mount bool) error { runtime.KeepAlive(pty) // Now, dup over all the things. - return dupStdio(slavePath) + return dupStdio(peerPty) } // syncParentReady sends to the given pipe a JSON payload which indicates that