rootfs: avoid using os.Create for new device inodes

If an attacker were to make the target of a device inode creation be a
symlink to some host path, os.Create would happily truncate the target
which could lead to all sorts of issues. This exploit is probably not as
exploitable because device inodes are usually only bind-mounted for
rootless containers, which cannot overwrite important host files (though
user files would still be up for grabs).

The regular inode creation logic could also theoretically be tricked
into changing the access mode and ownership of host files if the
newly-created device inode was swapped with a symlink to a host path.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

internal/sys/opath_linux.go

@@ -0,0 +1,53 @@
+package sys
+
+import (
+	"fmt"
+	"os"
+	"runtime"
+	"strconv"
+
+	"golang.org/x/sys/unix"
+
+	"github.com/opencontainers/runc/internal/pathrs"
+)
+
+// FchmodFile is a wrapper around fchmodat2(AT_EMPTY_PATH) with fallbacks for
+// older kernels. This is distinct from [File.Chmod] and [unix.Fchmod] in that
+// it works on O_PATH file descriptors.
+func FchmodFile(f *os.File, mode uint32) error {
+	err := unix.Fchmodat(int(f.Fd()), "", mode, unix.AT_EMPTY_PATH)
+	// If fchmodat2(2) is not available at all, golang.org/x/unix (probably
+	// in order to mirror glibc) returns EOPNOTSUPP rather than EINVAL
+	// (what the kernel actually returns for invalid flags, which is being
+	// emulated) or ENOSYS (which is what glibc actually sees).
+	if err != unix.EINVAL && err != unix.EOPNOTSUPP { //nolint:errorlint // unix errors are bare
+		// err == nil is implicitly handled
+		return os.NewSyscallError("fchmodat2 AT_EMPTY_PATH", err)
+	}
+
+	// AT_EMPTY_PATH support was added to fchmodat2 in Linux 6.6
+	// (5daeb41a6fc9d0d81cb2291884b7410e062d8fa1). The alternative for
+	// older kernels is to go through /proc.
+	fdDir, closer, err2 := pathrs.ProcThreadSelfOpen("fd/", unix.O_DIRECTORY)
+	if err2 != nil {
+		return fmt.Errorf("fchmodat2 AT_EMPTY_PATH fallback: %w", err2)
+	}
+	defer closer()
+	defer fdDir.Close()
+
+	err = unix.Fchmodat(int(fdDir.Fd()), strconv.Itoa(int(f.Fd())), mode, 0)
+	if err != nil {
+		err = fmt.Errorf("fchmodat /proc/self/fd/%d: %w", f.Fd(), err)
+	}
+	runtime.KeepAlive(f)
+	return err
+}
+
+// FchownFile is a wrapper around fchownat(AT_EMPTY_PATH). This is distinct
+// from [File.Chown] and [unix.Fchown] in that it works on O_PATH file
+// descriptors.
+func FchownFile(f *os.File, uid, gid int) error {
+	err := unix.Fchownat(int(f.Fd()), "", uid, gid, unix.AT_EMPTY_PATH)
+	runtime.KeepAlive(f)
+	return os.NewSyscallError("fchownat AT_EMPTY_PATH", err)
+}

libcontainer/rootfs_linux.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"runtime"
 	"strconv"
 	"strings"
 	"syscall"
@@ -932,17 +933,18 @@ func createDevices(config *configs.Config) error {
 	return nil
 }
 
-func bindMountDeviceNode(rootfs, dest string, node *devices.Device) error {
-	f, err := os.Create(dest)
-	if err != nil && !os.IsExist(err) {
-		return err
+func bindMountDeviceNode(destDir *os.File, destName string, node *devices.Device) error {
+	dstFile, err := utils.Openat(destDir, destName, unix.O_CREAT|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0o000)
+	if err != nil {
+		return fmt.Errorf("create device inode %s: %w", node.Path, err)
 	}
-	if f != nil {
-		_ = f.Close()
-	}
-	return utils.WithProcfd(rootfs, dest, func(dstFd string) error {
-		return mountViaFds(node.Path, nil, dest, dstFd, "bind", unix.MS_BIND, "")
-	})
+	defer dstFile.Close()
+
+	dstFd, closer := utils.ProcThreadSelfFd(dstFile.Fd())
+	defer closer()
+
+	dstPath := filepath.Join(destDir.Name(), destName)
+	return mountViaFds(node.Path, nil, dstPath, dstFd, "bind", unix.MS_BIND, "")
 }
 
 // Creates the device node in the rootfs of the container.
@@ -951,31 +953,33 @@ func createDeviceNode(rootfs string, node *devices.Device, bind bool) error {
 		// The node only exists for cgroup reasons, ignore it here.
 		return nil
 	}
-	dest, err := securejoin.SecureJoin(rootfs, node.Path)
+	destPath, err := securejoin.SecureJoin(rootfs, node.Path)
 	if err != nil {
 		return err
 	}
-	if dest == rootfs {
+	if destPath == rootfs {
 		return fmt.Errorf("%w: mknod over rootfs", errRootfsToFile)
 	}
-	if err := pathrs.MkdirAllInRoot(rootfs, filepath.Dir(dest), 0o755); err != nil {
-		return err
+	destDirPath, destName := filepath.Split(destPath)
+	destDir, err := pathrs.MkdirAllInRootOpen(rootfs, destDirPath, 0o755)
+	if err != nil {
+		return fmt.Errorf("mkdir parent of device inode %q: %w", node.Path, err)
 	}
 	if bind {
-		return bindMountDeviceNode(rootfs, dest, node)
+		return bindMountDeviceNode(destDir, destName, node)
 	}
-	if err := mknodDevice(dest, node); err != nil {
+	if err := mknodDevice(destDir, destName, node); err != nil {
 		if errors.Is(err, os.ErrExist) {
 			return nil
 		} else if errors.Is(err, os.ErrPermission) {
-			return bindMountDeviceNode(rootfs, dest, node)
+			return bindMountDeviceNode(destDir, destName, node)
 		}
 		return err
 	}
 	return nil
 }
 
-func mknodDevice(dest string, node *devices.Device) error {
+func mknodDevice(destDir *os.File, destName string, node *devices.Device) error {
 	fileMode := node.FileMode
 	switch node.Type {
 	case devices.BlockDevice:
@@ -991,14 +995,44 @@ func mknodDevice(dest string, node *devices.Device) error {
 	if err != nil {
 		return err
 	}
-	if err := unix.Mknod(dest, uint32(fileMode), int(dev)); err != nil {
-		return &os.PathError{Op: "mknod", Path: dest, Err: err}
+	if err := unix.Mknodat(int(destDir.Fd()), destName, uint32(fileMode), int(dev)); err != nil {
+		return &os.PathError{Op: "mknodat", Path: filepath.Join(destDir.Name(), destName), Err: err}
 	}
-	// Ensure permission bits (can be different because of umask).
-	if err := os.Chmod(dest, fileMode); err != nil {
+
+	// Get a handle and verify that it matches the expected inode type and
+	// major:minor before we operate on it.
+	devFile, err := utils.Openat(destDir, destName, unix.O_NOFOLLOW|unix.O_PATH, 0)
+	if err != nil {
+		return fmt.Errorf("open new %c device inode %s: %w", node.Type, node.Path, err)
+	}
+	defer devFile.Close()
+
+	if err := sys.VerifyInode(devFile, func(stat *unix.Stat_t, _ *unix.Statfs_t) error {
+		if stat.Mode&unix.S_IFMT != uint32(fileMode)&unix.S_IFMT {
+			return fmt.Errorf("new %c device inode %s has incorrect ftype: %#x doesn't match expected %#v",
+				node.Type, node.Path,
+				stat.Mode&unix.S_IFMT, fileMode&unix.S_IFMT)
+		}
+		if stat.Rdev != dev {
+			return fmt.Errorf("new %c device inode %s has incorrect major:minor: %d:%d doesn't match expected %d:%d",
+				node.Type, node.Path,
+				unix.Major(stat.Rdev), unix.Minor(stat.Rdev),
+				unix.Major(dev), unix.Minor(dev))
+		}
+		return nil
+	}); err != nil {
 		return err
 	}
-	return os.Chown(dest, int(node.Uid), int(node.Gid))
+
+	// Ensure permission bits (can be different because of umask).
+	if err := sys.FchmodFile(devFile, uint32(fileMode)); err != nil {
+		return fmt.Errorf("update new %c device inode %s file mode: %w", node.Type, node.Path, err)
+	}
+	if err := sys.FchownFile(devFile, int(node.Uid), int(node.Gid)); err != nil {
+		return fmt.Errorf("update new %c device inode %s owner: %w", node.Type, node.Path, err)
+	}
+	runtime.KeepAlive(devFile)
+	return nil
 }
 
 // rootfsParentMountPrivate ensures rootfs parent mount is private.

libcontainer/system/linux.go

@@ -160,3 +160,23 @@ func SetLinuxPersonality(personality int) error {
 	}
 	return nil
 }
+
+// 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
+}