From e450ed981223e12e09c33640f4fda0ac7f38033d Mon Sep 17 00:00:00 2001 From: Pushkar Joglekar Date: Tue, 4 Apr 2023 21:41:29 -0700 Subject: [PATCH] CVE-2023-27561: Bump runc go module v1.1.4 -> v1.1.5 --- go.mod | 2 +- go.sum | 3 +- .../libcontainer/cgroups/systemd/common.go | 14 ++- .../runc/libcontainer/init_linux.go | 5 +- .../runc/libcontainer/rootfs_linux.go | 100 +++++++++++------- vendor/modules.txt | 2 +- 6 files changed, 82 insertions(+), 44 deletions(-) diff --git a/go.mod b/go.mod index a697234490d..9c335b1df92 100644 --- a/go.mod +++ b/go.mod @@ -52,7 +52,7 @@ require ( github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 github.com/onsi/ginkgo/v2 v2.9.1 github.com/onsi/gomega v1.27.4 - github.com/opencontainers/runc v1.1.4 + github.com/opencontainers/runc v1.1.5 github.com/opencontainers/selinux v1.10.0 github.com/pkg/errors v0.9.1 github.com/pmezard/go-difflib v1.0.0 diff --git a/go.sum b/go.sum index 4737e3196c7..6db4bfb52f4 100644 --- a/go.sum +++ b/go.sum @@ -544,8 +544,9 @@ github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8 github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.0.2 h1:9yCKha/T5XdGtO0q9Q9a6T5NUCsTn/DrBg0D7ufOcFM= github.com/opencontainers/image-spec v1.0.2/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0= -github.com/opencontainers/runc v1.1.4 h1:nRCz/8sKg6K6jgYAFLDlXzPeITBZJyX28DBVhWD+5dg= github.com/opencontainers/runc v1.1.4/go.mod h1:1J5XiS+vdZ3wCyZybsuxXZWGrgSr8fFJHLXuG2PsnNg= +github.com/opencontainers/runc v1.1.5 h1:L44KXEpKmfWDcS02aeGm8QNTFXTo2D+8MYGDIJ/GDEs= +github.com/opencontainers/runc v1.1.5/go.mod h1:1J5XiS+vdZ3wCyZybsuxXZWGrgSr8fFJHLXuG2PsnNg= github.com/opencontainers/runtime-spec v1.0.2/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0= github.com/opencontainers/runtime-spec v1.0.3-0.20200929063507-e6143ca7d51d/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0= github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0= diff --git a/vendor/github.com/opencontainers/runc/libcontainer/cgroups/systemd/common.go b/vendor/github.com/opencontainers/runc/libcontainer/cgroups/systemd/common.go index 45744c15c0a..b6bfb080a33 100644 --- a/vendor/github.com/opencontainers/runc/libcontainer/cgroups/systemd/common.go +++ b/vendor/github.com/opencontainers/runc/libcontainer/cgroups/systemd/common.go @@ -293,8 +293,18 @@ func generateDeviceProperties(r *configs.Resources) ([]systemdDbus.Property, err // rules separately to systemd) we can safely skip entries that don't // have a corresponding path. if _, err := os.Stat(entry.Path); err != nil { - logrus.Debugf("skipping device %s for systemd: %s", entry.Path, err) - continue + // Also check /sys/dev so that we don't depend on /dev/{block,char} + // being populated. (/dev/{block,char} is populated by udev, which + // isn't strictly required for systemd). Ironically, this happens most + // easily when starting containerd within a runc created container + // itself. + + // We don't bother with securejoin here because we create entry.Path + // right above here, so we know it's safe. + if _, err := os.Stat("/sys" + entry.Path); err != nil { + logrus.Warnf("skipping device %s for systemd: %s", entry.Path, err) + continue + } } } deviceAllowList = append(deviceAllowList, entry) diff --git a/vendor/github.com/opencontainers/runc/libcontainer/init_linux.go b/vendor/github.com/opencontainers/runc/libcontainer/init_linux.go index 1e5c394c3e0..2e4c59353c8 100644 --- a/vendor/github.com/opencontainers/runc/libcontainer/init_linux.go +++ b/vendor/github.com/opencontainers/runc/libcontainer/init_linux.go @@ -411,8 +411,9 @@ func fixStdioPermissions(u *user.ExecUser) error { return &os.PathError{Op: "fstat", Path: file.Name(), Err: err} } - // Skip chown if uid is already the one we want. - if int(s.Uid) == u.Uid { + // 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 { continue } diff --git a/vendor/github.com/opencontainers/runc/libcontainer/rootfs_linux.go b/vendor/github.com/opencontainers/runc/libcontainer/rootfs_linux.go index ec7638e4d51..c3f88fc7038 100644 --- a/vendor/github.com/opencontainers/runc/libcontainer/rootfs_linux.go +++ b/vendor/github.com/opencontainers/runc/libcontainer/rootfs_linux.go @@ -329,26 +329,41 @@ func mountCgroupV2(m *configs.Mount, c *mountConfig) error { if err := os.MkdirAll(dest, 0o755); err != nil { return err } - return utils.WithProcfd(c.root, m.Destination, func(procfd string) error { - if err := mount(m.Source, m.Destination, procfd, "cgroup2", uintptr(m.Flags), m.Data); err != nil { - // when we are in UserNS but CgroupNS is not unshared, we cannot mount cgroup2 (#2158) - if errors.Is(err, unix.EPERM) || errors.Is(err, unix.EBUSY) { - src := fs2.UnifiedMountpoint - if c.cgroupns && c.cgroup2Path != "" { - // Emulate cgroupns by bind-mounting - // the container cgroup path rather than - // the whole /sys/fs/cgroup. - src = c.cgroup2Path - } - err = mount(src, m.Destination, procfd, "", uintptr(m.Flags)|unix.MS_BIND, "") - if c.rootlessCgroups && errors.Is(err, unix.ENOENT) { - err = nil - } - } - return err - } - return nil + err = utils.WithProcfd(c.root, m.Destination, func(procfd string) error { + return mount(m.Source, m.Destination, procfd, "cgroup2", uintptr(m.Flags), m.Data) }) + if err == nil || !(errors.Is(err, unix.EPERM) || errors.Is(err, unix.EBUSY)) { + return err + } + + // When we are in UserNS but CgroupNS is not unshared, we cannot mount + // cgroup2 (#2158), so fall back to bind mount. + bindM := &configs.Mount{ + Device: "bind", + Source: fs2.UnifiedMountpoint, + Destination: m.Destination, + Flags: unix.MS_BIND | m.Flags, + PropagationFlags: m.PropagationFlags, + } + if c.cgroupns && c.cgroup2Path != "" { + // Emulate cgroupns by bind-mounting the container cgroup path + // rather than the whole /sys/fs/cgroup. + bindM.Source = c.cgroup2Path + } + // mountToRootfs() handles remounting for MS_RDONLY. + // No need to set c.fd here, because mountToRootfs() calls utils.WithProcfd() by itself in mountPropagate(). + err = mountToRootfs(bindM, c) + if c.rootlessCgroups && errors.Is(err, unix.ENOENT) { + // ENOENT (for `src = c.cgroup2Path`) happens when rootless runc is being executed + // outside the userns+mountns. + // + // Mask `/sys/fs/cgroup` to ensure it is read-only, even when `/sys` is mounted + // with `rbind,ro` (`runc spec --rootless` produces `rbind,ro` for `/sys`). + err = utils.WithProcfd(c.root, m.Destination, func(procfd string) error { + return maskPath(procfd, c.label) + }) + } + return err } func doTmpfsCopyUp(m *configs.Mount, rootfs, mountLabel string) (Err error) { @@ -398,6 +413,35 @@ func doTmpfsCopyUp(m *configs.Mount, rootfs, mountLabel string) (Err error) { func mountToRootfs(m *configs.Mount, c *mountConfig) error { rootfs := c.root + + // procfs and sysfs are special because we need to ensure they are actually + // mounted on a specific path in a container without any funny business. + switch m.Device { + case "proc", "sysfs": + // If the destination already exists and is not a directory, we bail + // out. This is to avoid mounting through a symlink or similar -- which + // has been a "fun" attack scenario in the past. + // TODO: This won't be necessary once we switch to libpathrs and we can + // stop all of these symlink-exchange attacks. + dest := filepath.Clean(m.Destination) + if !strings.HasPrefix(dest, rootfs) { + // Do not use securejoin as it resolves symlinks. + dest = filepath.Join(rootfs, dest) + } + if fi, err := os.Lstat(dest); err != nil { + if !os.IsNotExist(err) { + return err + } + } else if !fi.IsDir() { + return fmt.Errorf("filesystem %q must be mounted on ordinary directory", m.Device) + } + if err := os.MkdirAll(dest, 0o755); err != nil { + return err + } + // Selinux kernels do not support labeling of /proc or /sys. + return mountPropagate(m, rootfs, "", nil) + } + mountLabel := c.label mountFd := c.fd dest, err := securejoin.SecureJoin(rootfs, m.Destination) @@ -406,24 +450,6 @@ func mountToRootfs(m *configs.Mount, c *mountConfig) error { } switch m.Device { - case "proc", "sysfs": - // If the destination already exists and is not a directory, we bail - // out This is to avoid mounting through a symlink or similar -- which - // has been a "fun" attack scenario in the past. - // TODO: This won't be necessary once we switch to libpathrs and we can - // stop all of these symlink-exchange attacks. - if fi, err := os.Lstat(dest); err != nil { - if !os.IsNotExist(err) { - return err - } - } else if fi.Mode()&os.ModeDir == 0 { - return fmt.Errorf("filesystem %q must be mounted on ordinary directory", m.Device) - } - if err := os.MkdirAll(dest, 0o755); err != nil { - return err - } - // Selinux kernels do not support labeling of /proc or /sys - return mountPropagate(m, rootfs, "", nil) case "mqueue": if err := os.MkdirAll(dest, 0o755); err != nil { return err diff --git a/vendor/modules.txt b/vendor/modules.txt index 3e9bf905103..59d750a6d83 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -552,7 +552,7 @@ github.com/onsi/gomega/types # github.com/opencontainers/go-digest v1.0.0 ## explicit; go 1.13 github.com/opencontainers/go-digest -# github.com/opencontainers/runc v1.1.4 +# github.com/opencontainers/runc v1.1.5 ## explicit; go 1.16 github.com/opencontainers/runc/libcontainer github.com/opencontainers/runc/libcontainer/apparmor