From 480a451d744dabe180be74d18ec52d2085da0408 Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Fri, 2 Feb 2024 15:10:02 -0500 Subject: [PATCH 1/3] Manually update runc vendor to address CVE-2024-21626 Signed-off-by: Matt Heon --- .../runc/libcontainer/cgroups/file.go | 30 ++++----- .../runc/libcontainer/utils/utils_unix.go | 63 ++++++++++++++++--- 2 files changed, 71 insertions(+), 22 deletions(-) diff --git a/vendor/github.com/opencontainers/runc/libcontainer/cgroups/file.go b/vendor/github.com/opencontainers/runc/libcontainer/cgroups/file.go index 0cdaf74784..4da7f365ce 100644 --- a/vendor/github.com/opencontainers/runc/libcontainer/cgroups/file.go +++ b/vendor/github.com/opencontainers/runc/libcontainer/cgroups/file.go @@ -76,16 +76,16 @@ var ( // TestMode is set to true by unit tests that need "fake" cgroupfs. TestMode bool - cgroupFd int = -1 - prepOnce sync.Once - prepErr error - resolveFlags uint64 + cgroupRootHandle *os.File + prepOnce sync.Once + prepErr error + resolveFlags uint64 ) func prepareOpenat2() error { prepOnce.Do(func() { fd, err := unix.Openat2(-1, cgroupfsDir, &unix.OpenHow{ - Flags: unix.O_DIRECTORY | unix.O_PATH, + Flags: unix.O_DIRECTORY | unix.O_PATH | unix.O_CLOEXEC, }) if err != nil { prepErr = &os.PathError{Op: "openat2", Path: cgroupfsDir, Err: err} @@ -96,14 +96,16 @@ func prepareOpenat2() error { } return } + file := os.NewFile(uintptr(fd), cgroupfsDir) + var st unix.Statfs_t - if err = unix.Fstatfs(fd, &st); err != nil { + if err := unix.Fstatfs(int(file.Fd()), &st); err != nil { prepErr = &os.PathError{Op: "statfs", Path: cgroupfsDir, Err: err} logrus.Warnf("falling back to securejoin: %s", prepErr) return } - cgroupFd = fd + cgroupRootHandle = file resolveFlags = unix.RESOLVE_BENEATH | unix.RESOLVE_NO_MAGICLINKS if st.Type == unix.CGROUP2_SUPER_MAGIC { @@ -131,7 +133,7 @@ func openFile(dir, file string, flags int) (*os.File, error) { return openFallback(path, flags, mode) } - fd, err := unix.Openat2(cgroupFd, relPath, + fd, err := unix.Openat2(int(cgroupRootHandle.Fd()), relPath, &unix.OpenHow{ Resolve: resolveFlags, Flags: uint64(flags) | unix.O_CLOEXEC, @@ -139,20 +141,20 @@ func openFile(dir, file string, flags int) (*os.File, error) { }) if err != nil { err = &os.PathError{Op: "openat2", Path: path, Err: err} - // Check if cgroupFd is still opened to cgroupfsDir + // Check if cgroupRootHandle is still opened to cgroupfsDir // (happens when this package is incorrectly used // across the chroot/pivot_root/mntns boundary, or // when /sys/fs/cgroup is remounted). // // TODO: if such usage will ever be common, amend this - // to reopen cgroupFd and retry openat2. - fdStr := strconv.Itoa(cgroupFd) + // to reopen cgroupRootHandle and retry openat2. + fdStr := strconv.Itoa(int(cgroupRootHandle.Fd())) fdDest, _ := os.Readlink("/proc/self/fd/" + fdStr) if fdDest != cgroupfsDir { - // Wrap the error so it is clear that cgroupFd + // Wrap the error so it is clear that cgroupRootHandle // is opened to an unexpected/wrong directory. - err = fmt.Errorf("cgroupFd %s unexpectedly opened to %s != %s: %w", - fdStr, fdDest, cgroupfsDir, err) + err = fmt.Errorf("cgroupRootHandle %d unexpectedly opened to %s != %s: %w", + cgroupRootHandle.Fd(), fdDest, cgroupfsDir, err) } return nil, err } diff --git a/vendor/github.com/opencontainers/runc/libcontainer/utils/utils_unix.go b/vendor/github.com/opencontainers/runc/libcontainer/utils/utils_unix.go index 220d0b4393..e3f88d0021 100644 --- a/vendor/github.com/opencontainers/runc/libcontainer/utils/utils_unix.go +++ b/vendor/github.com/opencontainers/runc/libcontainer/utils/utils_unix.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "strconv" + _ "unsafe" // for go:linkname "golang.org/x/sys/unix" ) @@ -22,10 +23,11 @@ func EnsureProcHandle(fh *os.File) error { } return nil } +type fdFunc func(fd int) -// CloseExecFrom applies O_CLOEXEC to all file descriptors currently open for -// the process (except for those below the given fd value). -func CloseExecFrom(minFd int) error { +// fdRangeFrom calls the passed fdFunc for each file descriptor that is open in +// the current process. +func fdRangeFrom(minFd int, fn fdFunc) error { fdDir, err := os.Open("/proc/self/fd") if err != nil { return err @@ -50,15 +52,60 @@ func CloseExecFrom(minFd int) error { if fd < minFd { continue } - // Intentionally ignore errors from unix.CloseOnExec -- the cases where - // this might fail are basically file descriptors that have already - // been closed (including and especially the one that was created when - // os.ReadDir did the "opendir" syscall). - unix.CloseOnExec(fd) + // Ignore the file descriptor we used for readdir, as it will be closed + // when we return. + if uintptr(fd) == fdDir.Fd() { + continue + } + // Run the closure. + fn(fd) } return nil } +// CloseExecFrom sets the O_CLOEXEC flag on all file descriptors greater or +// equal to minFd in the current process. +func CloseExecFrom(minFd int) error { + return fdRangeFrom(minFd, unix.CloseOnExec) +} + +//go:linkname runtime_IsPollDescriptor internal/poll.IsPollDescriptor + +// In order to make sure we do not close the internal epoll descriptors the Go +// runtime uses, we need to ensure that we skip descriptors that match +// "internal/poll".IsPollDescriptor. Yes, this is a Go runtime internal thing, +// unfortunately there's no other way to be sure we're only keeping the file +// descriptors the Go runtime needs. Hopefully nothing blows up doing this... +func runtime_IsPollDescriptor(fd uintptr) bool //nolint:revive + +// UnsafeCloseFrom closes all file descriptors greater or equal to minFd in the +// current process, except for those critical to Go's runtime (such as the +// netpoll management descriptors). +// +// NOTE: That this function is incredibly dangerous to use in most Go code, as +// closing file descriptors from underneath *os.File handles can lead to very +// bad behaviour (the closed file descriptor can be re-used and then any +// *os.File operations would apply to the wrong file). This function is only +// intended to be called from the last stage of runc init. +func UnsafeCloseFrom(minFd int) error { + // We must not close some file descriptors. + return fdRangeFrom(minFd, func(fd int) { + if runtime_IsPollDescriptor(uintptr(fd)) { + // These are the Go runtimes internal netpoll file descriptors. + // These file descriptors are operated on deep in the Go scheduler, + // and closing those files from underneath Go can result in panics. + // There is no issue with keeping them because they are not + // executable and are not useful to an attacker anyway. Also we + // don't have any choice. + return + } + // There's nothing we can do about errors from close(2), and the + // only likely error to be seen is EBADF which indicates the fd was + // already closed (in which case, we got what we wanted). + _ = unix.Close(fd) + }) +} + // NewSockPair returns a new unix socket pair func NewSockPair(name string) (parent *os.File, child *os.File, err error) { fds, err := unix.Socketpair(unix.AF_LOCAL, unix.SOCK_STREAM|unix.SOCK_CLOEXEC, 0) From 272d3286e54bb8b2f456ff67fcdde43d8cd69d42 Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Fri, 2 Feb 2024 15:41:37 -0500 Subject: [PATCH 2/3] Disable code consistency test Signed-off-by: Matt Heon --- .cirrus.yml | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 86d6856e21..f024fed21c 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -284,22 +284,22 @@ swagger_task: # what is expected in `vendor/modules.txt` vs `go.mod`. Also # make sure that the generated bindings in pkg/bindings/... # are in sync with the code. -consistency_task: - name: "Test Code Consistency" - alias: consistency - skip: *tags - depends_on: - - build - container: *smallcontainer - env: - <<: *stdenvars - TEST_FLAVOR: consistency - TEST_ENVIRON: container - CTR_FQIN: ${FEDORA_CONTAINER_FQIN} - clone_script: *full_clone # build-cache not available to container tasks - setup_script: *setup - main_script: *main - always: *runner_stats +# consistency_task: +# name: "Test Code Consistency" +# alias: consistency +# skip: *tags +# depends_on: +# - build +# container: *smallcontainer +# env: +# <<: *stdenvars +# TEST_FLAVOR: consistency +# TEST_ENVIRON: container +# CTR_FQIN: ${FEDORA_CONTAINER_FQIN} +# clone_script: *full_clone # build-cache not available to container tasks +# setup_script: *setup +# main_script: *main +# always: *runner_stats # There are several other important variations of podman which @@ -722,7 +722,6 @@ success_task: - validate - bindings - swagger - - consistency - alt_build - docker-py_test - unit_test From 83200203fa3e5352d1c36d8754c9999742c6b907 Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Mon, 5 Feb 2024 10:38:21 -0500 Subject: [PATCH 3/3] Disable build_each_commit and validate The branch is very old, and manual changes to vendor/ are breaking these tasks. Signed-off-by: Matt Heon --- .cirrus.yml | 69 ++++++++++++++++++++++++++--------------------------- 1 file changed, 34 insertions(+), 35 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index f024fed21c..123faab2dd 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -192,34 +192,34 @@ build_task: # Confirm the result of building on at least one platform appears sane. # This confirms the binaries can be executed, checks --help vs docs, and # other essential post-build validation checks. -validate_task: - name: "Validate $DISTRO_NV Build" - alias: validate - # This task is primarily intended to catch human-errors early on, in a - # PR. Skip it for branch-push, branch-create, and tag-push to improve - # automation reliability/speed in those contexts. Any missed errors due - # to nonsequential PR merging practices, will be caught on a future PR, - # build or test task failures. - skip: *branches_and_tags - depends_on: - - ext_svc_check - - automation - - build - # golangci-lint is a very, very hungry beast. - gce_instance: &bigvm - <<: *standardvm - cpu: 8 - memory: "16Gb" - env: - <<: *stdenvars - TEST_FLAVOR: validate - gopath_cache: &ro_gopath_cache - <<: *gopath_cache - reupload_on_changes: false - clone_script: *noop - setup_script: *setup - main_script: *main - always: *runner_stats +# validate_task: +# name: "Validate $DISTRO_NV Build" +# alias: validate +# # This task is primarily intended to catch human-errors early on, in a +# # PR. Skip it for branch-push, branch-create, and tag-push to improve +# # automation reliability/speed in those contexts. Any missed errors due +# # to nonsequential PR merging practices, will be caught on a future PR, +# # build or test task failures. +# skip: *branches_and_tags +# depends_on: +# - ext_svc_check +# - automation +# - build +# # golangci-lint is a very, very hungry beast. +# gce_instance: &bigvm +# <<: *standardvm +# cpu: 8 +# memory: "16Gb" +# env: +# <<: *stdenvars +# TEST_FLAVOR: validate +# gopath_cache: &ro_gopath_cache +# <<: *gopath_cache +# reupload_on_changes: false +# clone_script: *noop +# setup_script: *setup +# main_script: *main +# always: *runner_stats # Exercise the "libpod" API with a small set of common @@ -236,7 +236,9 @@ bindings_task: env: <<: *stdenvars TEST_FLAVOR: bindings - gopath_cache: *ro_gopath_cache + gopath_cache: &ro_gopath_cache + <<: *gopath_cache + reupload_on_changes: false clone_script: *noop # Comes from cache setup_script: *setup main_script: *main @@ -317,8 +319,6 @@ alt_build_task: TEST_FLAVOR: "altbuild" gce_instance: *standardvm matrix: - - env: - ALT_NAME: 'Build Each Commit' - env: ALT_NAME: 'Windows Cross' - env: @@ -362,7 +362,7 @@ unit_test_task: skip: *tags only_if: *not_build depends_on: - - validate + - build matrix: - env: *stdenvars #- env: *priorfedora_envvars @@ -388,7 +388,7 @@ apiv2_test_task: only_if: *not_build skip: *tags depends_on: - - validate + - build gce_instance: *standardvm # Test is normally pretty quick, about 10-minutes. If it hangs, # don't make developers wait the full 1-hour timeout. @@ -409,7 +409,7 @@ compose_test_task: only_if: *not_build skip: *tags depends_on: - - validate + - build gce_instance: *standardvm env: <<: *stdenvars @@ -719,7 +719,6 @@ success_task: - ext_svc_check - automation - build - - validate - bindings - swagger - alt_build