From b096459a070e27eeef46b4305933297c4a047421 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Mon, 30 Sep 2024 16:25:35 +0200 Subject: [PATCH] vendor: update github.com/cyphar/filepath-securejoin to v0.3.3 This fixes issues we had with spurious MkdirAll failures. Signed-off-by: Aleksa Sarai --- go.mod | 2 +- go.sum | 4 +- .../cyphar/filepath-securejoin/CHANGELOG.md | 15 +- .../cyphar/filepath-securejoin/VERSION | 2 +- .../cyphar/filepath-securejoin/join.go | 16 +-- .../cyphar/filepath-securejoin/mkdir_linux.go | 106 +++++---------- .../cyphar/filepath-securejoin/open_linux.go | 14 +- .../filepath-securejoin/openat2_linux.go | 33 ++--- .../filepath-securejoin/procfs_linux.go | 128 ++++++------------ .../cyphar/filepath-securejoin/vfs.go | 24 ++-- vendor/modules.txt | 4 +- 11 files changed, 128 insertions(+), 220 deletions(-) diff --git a/go.mod b/go.mod index 9c9dc9f237b..478c04dc7cd 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/cilium/ebpf v0.16.0 github.com/containerd/console v1.0.4 github.com/coreos/go-systemd/v22 v22.5.0 - github.com/cyphar/filepath-securejoin v0.3.2 + github.com/cyphar/filepath-securejoin v0.3.3 github.com/docker/go-units v0.5.0 github.com/godbus/dbus/v5 v5.1.0 github.com/moby/sys/mountinfo v0.7.1 diff --git a/go.sum b/go.sum index 02393213996..d515898bbfd 100644 --- a/go.sum +++ b/go.sum @@ -9,8 +9,8 @@ github.com/coreos/go-systemd/v22 v22.5.0 h1:RrqgGjYQKalulkV8NGVIfkXQf6YYmOyiJKk8 github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/cpuguy83/go-md2man/v2 v2.0.2 h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHHBfMQ6w= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= -github.com/cyphar/filepath-securejoin v0.3.2 h1:QhZu5AxQ+o1XZH0Ye05YzvJ0kAdK6VQc0z9NNMek7gc= -github.com/cyphar/filepath-securejoin v0.3.2/go.mod h1:F7i41x/9cBF7lzCrVsYs9fuzwRZm4NQsGTBdpp6mETc= +github.com/cyphar/filepath-securejoin v0.3.3 h1:lofZkCEVFIBe0KcdQOzFs8Soy9oaHOWl4gGtPI+gCFc= +github.com/cyphar/filepath-securejoin v0.3.3/go.mod h1:8s/MCNJREmFK0H02MF6Ihv1nakJe4L/w3WZLHNkvlYM= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/vendor/github.com/cyphar/filepath-securejoin/CHANGELOG.md b/vendor/github.com/cyphar/filepath-securejoin/CHANGELOG.md index 98172cedda7..23f7bc7f62a 100644 --- a/vendor/github.com/cyphar/filepath-securejoin/CHANGELOG.md +++ b/vendor/github.com/cyphar/filepath-securejoin/CHANGELOG.md @@ -6,6 +6,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] ## +## [0.3.3] - 2024-09-30 ## + +### Fixed ### +- The mode and owner verification logic in `MkdirAll` has been removed. This + was originally intended to protect against some theoretical attacks but upon + further consideration these protections don't actually buy us anything and + they were causing spurious errors with more complicated filesystem setups. +- The "is the created directory empty" logic in `MkdirAll` has also been + removed. This was not causing us issues yet, but some pseudofilesystems (such + as `cgroup`) create non-empty directories and so this logic would've been + wrong for such cases. + ## [0.3.2] - 2024-09-13 ## ### Changed ### @@ -145,7 +157,8 @@ This is our first release of `github.com/cyphar/filepath-securejoin`, containing a full implementation with a coverage of 93.5% (the only missing cases are the error cases, which are hard to mocktest at the moment). -[Unreleased]: https://github.com/cyphar/filepath-securejoin/compare/v0.3.2...HEAD +[Unreleased]: https://github.com/cyphar/filepath-securejoin/compare/v0.3.3...HEAD +[0.3.3]: https://github.com/cyphar/filepath-securejoin/compare/v0.3.2...v0.3.3 [0.3.2]: https://github.com/cyphar/filepath-securejoin/compare/v0.3.1...v0.3.2 [0.3.1]: https://github.com/cyphar/filepath-securejoin/compare/v0.3.0...v0.3.1 [0.3.0]: https://github.com/cyphar/filepath-securejoin/compare/v0.2.5...v0.3.0 diff --git a/vendor/github.com/cyphar/filepath-securejoin/VERSION b/vendor/github.com/cyphar/filepath-securejoin/VERSION index d15723fbe8d..1c09c74e221 100644 --- a/vendor/github.com/cyphar/filepath-securejoin/VERSION +++ b/vendor/github.com/cyphar/filepath-securejoin/VERSION @@ -1 +1 @@ -0.3.2 +0.3.3 diff --git a/vendor/github.com/cyphar/filepath-securejoin/join.go b/vendor/github.com/cyphar/filepath-securejoin/join.go index bd86a48b0cc..ca4ce1f29a6 100644 --- a/vendor/github.com/cyphar/filepath-securejoin/join.go +++ b/vendor/github.com/cyphar/filepath-securejoin/join.go @@ -22,27 +22,27 @@ const maxSymlinkLimit = 255 // IsNotExist tells you if err is an error that implies that either the path // accessed does not exist (or path components don't exist). This is -// effectively a more broad version of os.IsNotExist. +// effectively a more broad version of [os.IsNotExist]. func IsNotExist(err error) bool { // Check that it's not actually an ENOTDIR, which in some cases is a more // convoluted case of ENOENT (usually involving weird paths). return errors.Is(err, os.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) || errors.Is(err, syscall.ENOENT) } -// SecureJoinVFS joins the two given path components (similar to Join) except +// SecureJoinVFS joins the two given path components (similar to [filepath.Join]) except // that the returned path is guaranteed to be scoped inside the provided root // path (when evaluated). Any symbolic links in the path are evaluated with the // given root treated as the root of the filesystem, similar to a chroot. The -// filesystem state is evaluated through the given VFS interface (if nil, the -// standard os.* family of functions are used). +// filesystem state is evaluated through the given [VFS] interface (if nil, the +// standard [os].* family of functions are used). // // Note that the guarantees provided by this function only apply if the path // components in the returned string are not modified (in other words are not // replaced with symlinks on the filesystem) after this function has returned. -// Such a symlink race is necessarily out-of-scope of SecureJoin. +// Such a symlink race is necessarily out-of-scope of SecureJoinVFS. // // NOTE: Due to the above limitation, Linux users are strongly encouraged to -// use OpenInRoot instead, which does safely protect against these kinds of +// use [OpenInRoot] instead, which does safely protect against these kinds of // attacks. There is no way to solve this problem with SecureJoinVFS because // the API is fundamentally wrong (you cannot return a "safe" path string and // guarantee it won't be modified afterwards). @@ -123,8 +123,8 @@ func SecureJoinVFS(root, unsafePath string, vfs VFS) (string, error) { return filepath.Join(root, finalPath), nil } -// SecureJoin is a wrapper around SecureJoinVFS that just uses the os.* library -// of functions as the VFS. If in doubt, use this function over SecureJoinVFS. +// SecureJoin is a wrapper around [SecureJoinVFS] that just uses the [os].* library +// of functions as the [VFS]. If in doubt, use this function over [SecureJoinVFS]. func SecureJoin(root, unsafePath string) (string, error) { return SecureJoinVFS(root, unsafePath, nil) } diff --git a/vendor/github.com/cyphar/filepath-securejoin/mkdir_linux.go b/vendor/github.com/cyphar/filepath-securejoin/mkdir_linux.go index 49ffdbe02bd..b5f674524c8 100644 --- a/vendor/github.com/cyphar/filepath-securejoin/mkdir_linux.go +++ b/vendor/github.com/cyphar/filepath-securejoin/mkdir_linux.go @@ -9,7 +9,6 @@ package securejoin import ( "errors" "fmt" - "io" "os" "path/filepath" "slices" @@ -23,23 +22,23 @@ var ( errPossibleAttack = errors.New("possible attack detected") ) -// MkdirAllHandle is equivalent to MkdirAll, except that it is safer to use in -// two respects: +// MkdirAllHandle is equivalent to [MkdirAll], except that it is safer to use +// in two respects: // -// - The caller provides the root directory as an *os.File (preferably O_PATH) +// - The caller provides the root directory as an *[os.File] (preferably O_PATH) // handle. This means that the caller can be sure which root directory is // being used. Note that this can be emulated by using /proc/self/fd/... as -// the root path with MkdirAll. +// the root path with [os.MkdirAll]. // -// - Once all of the directories have been created, an *os.File (O_PATH) handle +// - Once all of the directories have been created, an *[os.File] O_PATH handle // to the directory at unsafePath is returned to the caller. This is done in // an effectively-race-free way (an attacker would only be able to swap the // final directory component), which is not possible to emulate with -// MkdirAll. +// [MkdirAll]. // // In addition, the returned handle is obtained far more efficiently than doing -// a brand new lookup of unsafePath (such as with SecureJoin or openat2) after -// doing MkdirAll. If you intend to open the directory after creating it, you +// a brand new lookup of unsafePath (such as with [SecureJoin] or openat2) after +// doing [MkdirAll]. If you intend to open the directory after creating it, you // should use MkdirAllHandle. func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err error) { // Make sure there are no os.FileMode bits set. @@ -108,35 +107,6 @@ func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err // Make sure the mode doesn't have any type bits. mode &^= unix.S_IFMT - // What properties do we expect any newly created directories to have? - var ( - // While umask(2) is a per-thread property, and thus this value could - // vary between threads, a functioning Go program would LockOSThread - // threads with different umasks and so we don't need to LockOSThread - // for this entire mkdirat loop (if we are in the locked thread with a - // different umask, we are already locked and there's nothing for us to - // do -- and if not then it doesn't matter which thread we run on and - // there's nothing for us to do). - expectedMode = uint32(unix.S_IFDIR | (mode &^ getUmask())) - - // We would want to get the fs[ug]id here, but we can't access those - // from userspace. In practice, nobody uses setfs[ug]id() anymore, so - // just use the effective [ug]id (which is equivalent to the fs[ug]id - // for programs that don't use setfs[ug]id). - expectedUid = uint32(unix.Geteuid()) - expectedGid = uint32(unix.Getegid()) - ) - - // The setgid bit (S_ISGID = 0o2000) is inherited to child directories and - // affects the group of any inodes created in said directory, so if the - // starting directory has it set we need to adjust our expected mode and - // owner to match. - if st, err := fstatFile(currentDir); err != nil { - return nil, fmt.Errorf("failed to stat starting path for mkdir %q: %w", currentDir.Name(), err) - } else if st.Mode&unix.S_ISGID == unix.S_ISGID { - expectedMode |= unix.S_ISGID - expectedGid = st.Gid - } // Create the remaining components. for _, part := range remainingParts { @@ -147,7 +117,7 @@ func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err } // NOTE: mkdir(2) will not follow trailing symlinks, so we can safely - // create the finaly component without worrying about symlink-exchange + // create the final component without worrying about symlink-exchange // attacks. if err := unix.Mkdirat(int(currentDir.Fd()), part, uint32(mode)); err != nil { err = &os.PathError{Op: "mkdirat", Path: currentDir.Name() + "/" + part, Err: err} @@ -175,40 +145,30 @@ func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err _ = currentDir.Close() currentDir = nextDir - // Make sure that the directory matches what we expect. An attacker - // could have swapped the directory between us making it and opening - // it. There's no way for us to be sure that the directory is - // _precisely_ the same as the directory we created, but if we are in - // an empty directory with the same owner and mode as the one we - // created then there is nothing the attacker could do with this new - // directory that they couldn't do with the old one. - if stat, err := fstat(currentDir); err != nil { - return nil, fmt.Errorf("check newly created directory: %w", err) - } else { - if stat.Mode != expectedMode { - return nil, fmt.Errorf("%w: newly created directory %q has incorrect mode 0o%.3o (expected 0o%.3o)", errPossibleAttack, currentDir.Name(), stat.Mode, expectedMode) - } - if stat.Uid != expectedUid || stat.Gid != expectedGid { - return nil, fmt.Errorf("%w: newly created directory %q has incorrect owner %d:%d (expected %d:%d)", errPossibleAttack, currentDir.Name(), stat.Uid, stat.Gid, expectedUid, expectedGid) - } - // Check that the directory is empty. We only need to check for - // a single entry, and we should get EOF if the directory is - // empty. - _, err := currentDir.Readdirnames(1) - if !errors.Is(err, io.EOF) { - if err == nil { - err = fmt.Errorf("%w: newly created directory %q is non-empty", errPossibleAttack, currentDir.Name()) - } - return nil, fmt.Errorf("check if newly created directory %q is empty: %w", currentDir.Name(), err) - } - // Reset the offset. - _, _ = currentDir.Seek(0, unix.SEEK_SET) - } + // It's possible that the directory we just opened was swapped by an + // attacker. Unfortunately there isn't much we can do to protect + // against this, and MkdirAll's behaviour is that we will reuse + // existing directories anyway so the need to protect against this is + // incredibly limited (and arguably doesn't even deserve mention here). + // + // Ideally we might want to check that the owner and mode match what we + // would've created -- unfortunately, it is non-trivial to verify that + // the owner and mode of the created directory match. While plain Unix + // DAC rules seem simple enough to emulate, there are a bunch of other + // factors that can change the mode or owner of created directories + // (default POSIX ACLs, mount options like uid=1,gid=2,umask=0 on + // filesystems like vfat, etc etc). We used to try to verify this but + // it just lead to a series of spurious errors. + // + // We could also check that the directory is non-empty, but + // unfortunately some pseduofilesystems (like cgroupfs) create + // non-empty directories, which would result in different spurious + // errors. } return currentDir, nil } -// MkdirAll is a race-safe alternative to the Go stdlib's os.MkdirAll function, +// MkdirAll is a race-safe alternative to the [os.MkdirAll] function, // where the new directory is guaranteed to be within the root directory (if an // attacker can move directories from inside the root to outside the root, the // created directory tree might be outside of the root but the key constraint @@ -221,16 +181,16 @@ func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err // err := os.MkdirAll(path, mode) // // But is much safer. The above implementation is unsafe because if an attacker -// can modify the filesystem tree between SecureJoin and MkdirAll, it is +// can modify the filesystem tree between [SecureJoin] and [os.MkdirAll], it is // possible for MkdirAll to resolve unsafe symlink components and create // directories outside of the root. // // If you plan to open the directory after you have created it or want to use -// an open directory handle as the root, you should use MkdirAllHandle instead. -// This function is a wrapper around MkdirAllHandle. +// an open directory handle as the root, you should use [MkdirAllHandle] instead. +// This function is a wrapper around [MkdirAllHandle]. // // NOTE: The mode argument must be set the unix mode bits (unix.S_I...), not -// the Go generic mode bits (os.Mode...). +// the Go generic mode bits ([os.FileMode]...). func MkdirAll(root, unsafePath string, mode int) error { rootDir, err := os.OpenFile(root, unix.O_PATH|unix.O_DIRECTORY|unix.O_CLOEXEC, 0) if err != nil { diff --git a/vendor/github.com/cyphar/filepath-securejoin/open_linux.go b/vendor/github.com/cyphar/filepath-securejoin/open_linux.go index 52dce76f3f4..230be73f0eb 100644 --- a/vendor/github.com/cyphar/filepath-securejoin/open_linux.go +++ b/vendor/github.com/cyphar/filepath-securejoin/open_linux.go @@ -14,8 +14,8 @@ import ( "golang.org/x/sys/unix" ) -// OpenatInRoot is equivalent to OpenInRoot, except that the root is provided -// using an *os.File handle, to ensure that the correct root directory is used. +// OpenatInRoot is equivalent to [OpenInRoot], except that the root is provided +// using an *[os.File] handle, to ensure that the correct root directory is used. func OpenatInRoot(root *os.File, unsafePath string) (*os.File, error) { handle, err := completeLookupInRoot(root, unsafePath) if err != nil { @@ -31,7 +31,7 @@ func OpenatInRoot(root *os.File, unsafePath string) (*os.File, error) { // handle, err := os.OpenFile(path, unix.O_PATH|unix.O_CLOEXEC) // // But is much safer. The above implementation is unsafe because if an attacker -// can modify the filesystem tree between SecureJoin and OpenFile, it is +// can modify the filesystem tree between [SecureJoin] and [os.OpenFile], it is // possible for the returned file to be outside of the root. // // Note that the returned handle is an O_PATH handle, meaning that only a very @@ -39,7 +39,7 @@ func OpenatInRoot(root *os.File, unsafePath string) (*os.File, error) { // accidentally opening an untrusted file that could cause issues (such as a // disconnected TTY that could cause a DoS, or some other issue). In order to // use the returned handle, you can "upgrade" it to a proper handle using -// Reopen. +// [Reopen]. func OpenInRoot(root, unsafePath string) (*os.File, error) { rootDir, err := os.OpenFile(root, unix.O_PATH|unix.O_DIRECTORY|unix.O_CLOEXEC, 0) if err != nil { @@ -49,7 +49,7 @@ func OpenInRoot(root, unsafePath string) (*os.File, error) { return OpenatInRoot(rootDir, unsafePath) } -// Reopen takes an *os.File handle and re-opens it through /proc/self/fd. +// Reopen takes an *[os.File] handle and re-opens it through /proc/self/fd. // Reopen(file, flags) is effectively equivalent to // // fdPath := fmt.Sprintf("/proc/self/fd/%d", file.Fd()) @@ -59,7 +59,9 @@ func OpenInRoot(root, unsafePath string) (*os.File, error) { // maliciously-configured /proc mount. While this attack scenario is not // common, in container runtimes it is possible for higher-level runtimes to be // tricked into configuring an unsafe /proc that can be used to attack file -// operations. See CVE-2019-19921 for more details. +// operations. See [CVE-2019-19921] for more details. +// +// [CVE-2019-19921]: https://github.com/advisories/GHSA-fh74-hm69-rqjw func Reopen(handle *os.File, flags int) (*os.File, error) { procRoot, err := getProcRoot() if err != nil { diff --git a/vendor/github.com/cyphar/filepath-securejoin/openat2_linux.go b/vendor/github.com/cyphar/filepath-securejoin/openat2_linux.go index 921b3e1d449..ae3b381efe6 100644 --- a/vendor/github.com/cyphar/filepath-securejoin/openat2_linux.go +++ b/vendor/github.com/cyphar/filepath-securejoin/openat2_linux.go @@ -13,34 +13,21 @@ import ( "path/filepath" "strings" "sync" - "testing" "golang.org/x/sys/unix" ) -var ( - hasOpenat2Bool bool - hasOpenat2Once sync.Once - - testingForceHasOpenat2 *bool -) - -func hasOpenat2() bool { - if testing.Testing() && testingForceHasOpenat2 != nil { - return *testingForceHasOpenat2 - } - hasOpenat2Once.Do(func() { - fd, err := unix.Openat2(unix.AT_FDCWD, ".", &unix.OpenHow{ - Flags: unix.O_PATH | unix.O_CLOEXEC, - Resolve: unix.RESOLVE_NO_SYMLINKS | unix.RESOLVE_IN_ROOT, - }) - if err == nil { - hasOpenat2Bool = true - _ = unix.Close(fd) - } +var hasOpenat2 = sync.OnceValue(func() bool { + fd, err := unix.Openat2(unix.AT_FDCWD, ".", &unix.OpenHow{ + Flags: unix.O_PATH | unix.O_CLOEXEC, + Resolve: unix.RESOLVE_NO_SYMLINKS | unix.RESOLVE_IN_ROOT, }) - return hasOpenat2Bool -} + if err != nil { + return false + } + _ = unix.Close(fd) + return true +}) func scopedLookupShouldRetry(how *unix.OpenHow, err error) bool { // RESOLVE_IN_ROOT (and RESOLVE_BENEATH) can return -EAGAIN if we resolve diff --git a/vendor/github.com/cyphar/filepath-securejoin/procfs_linux.go b/vendor/github.com/cyphar/filepath-securejoin/procfs_linux.go index adf0bd08f3b..fa7929a5298 100644 --- a/vendor/github.com/cyphar/filepath-securejoin/procfs_linux.go +++ b/vendor/github.com/cyphar/filepath-securejoin/procfs_linux.go @@ -54,33 +54,26 @@ func verifyProcRoot(procRoot *os.File) error { return nil } -var ( - hasNewMountApiBool bool - hasNewMountApiOnce sync.Once -) - -func hasNewMountApi() bool { - hasNewMountApiOnce.Do(func() { - // All of the pieces of the new mount API we use (fsopen, fsconfig, - // fsmount, open_tree) were added together in Linux 5.1[1,2], so we can - // just check for one of the syscalls and the others should also be - // available. - // - // Just try to use open_tree(2) to open a file without OPEN_TREE_CLONE. - // This is equivalent to openat(2), but tells us if open_tree is - // available (and thus all of the other basic new mount API syscalls). - // open_tree(2) is most light-weight syscall to test here. - // - // [1]: merge commit 400913252d09 - // [2]: - fd, err := unix.OpenTree(-int(unix.EBADF), "/", unix.OPEN_TREE_CLOEXEC) - if err == nil { - hasNewMountApiBool = true - _ = unix.Close(fd) - } - }) - return hasNewMountApiBool -} +var hasNewMountApi = sync.OnceValue(func() bool { + // All of the pieces of the new mount API we use (fsopen, fsconfig, + // fsmount, open_tree) were added together in Linux 5.1[1,2], so we can + // just check for one of the syscalls and the others should also be + // available. + // + // Just try to use open_tree(2) to open a file without OPEN_TREE_CLONE. + // This is equivalent to openat(2), but tells us if open_tree is + // available (and thus all of the other basic new mount API syscalls). + // open_tree(2) is most light-weight syscall to test here. + // + // [1]: merge commit 400913252d09 + // [2]: + fd, err := unix.OpenTree(-int(unix.EBADF), "/", unix.OPEN_TREE_CLOEXEC) + if err != nil { + return false + } + _ = unix.Close(fd) + return true +}) func fsopen(fsName string, flags int) (*os.File, error) { // Make sure we always set O_CLOEXEC. @@ -172,14 +165,6 @@ func privateProcRoot() (*os.File, error) { return procRoot, err } -var ( - procRootHandle *os.File - procRootError error - procRootOnce sync.Once - - errUnsafeProcfs = errors.New("unsafe procfs detected") -) - func unsafeHostProcRoot() (_ *os.File, Err error) { procRoot, err := os.OpenFile("/proc", unix.O_PATH|unix.O_NOFOLLOW|unix.O_DIRECTORY|unix.O_CLOEXEC, 0) if err != nil { @@ -207,17 +192,15 @@ func doGetProcRoot() (*os.File, error) { return procRoot, err } -func getProcRoot() (*os.File, error) { - procRootOnce.Do(func() { - procRootHandle, procRootError = doGetProcRoot() - }) - return procRootHandle, procRootError -} +var getProcRoot = sync.OnceValues(func() (*os.File, error) { + return doGetProcRoot() +}) -var ( - haveProcThreadSelf bool - haveProcThreadSelfOnce sync.Once -) +var hasProcThreadSelf = sync.OnceValue(func() bool { + return unix.Access("/proc/thread-self/", unix.F_OK) == nil +}) + +var errUnsafeProcfs = errors.New("unsafe procfs detected") type procThreadSelfCloser func() @@ -230,13 +213,6 @@ type procThreadSelfCloser func() // This is similar to ProcThreadSelf from runc, but with extra hardening // applied and using *os.File. func procThreadSelf(procRoot *os.File, subpath string) (_ *os.File, _ procThreadSelfCloser, Err error) { - haveProcThreadSelfOnce.Do(func() { - // If the kernel doesn't support thread-self, it doesn't matter which - // /proc handle we use. - _, err := fstatatFile(procRoot, "thread-self", unix.AT_SYMLINK_NOFOLLOW) - haveProcThreadSelf = (err == nil) - }) - // We need to lock our thread until the caller is done with the handle // because between getting the handle and using it we could get interrupted // by the Go runtime and hit the case where the underlying thread is @@ -251,7 +227,7 @@ func procThreadSelf(procRoot *os.File, subpath string) (_ *os.File, _ procThread // Figure out what prefix we want to use. threadSelf := "thread-self/" - if !haveProcThreadSelf || testingForceProcSelfTask() { + if !hasProcThreadSelf() || testingForceProcSelfTask() { /// Pre-3.17 kernels don't have /proc/thread-self, so do it manually. threadSelf = "self/task/" + strconv.Itoa(unix.Gettid()) + "/" if _, err := fstatatFile(procRoot, threadSelf, unix.AT_SYMLINK_NOFOLLOW); err != nil || testingForceProcSelf() { @@ -275,7 +251,7 @@ func procThreadSelf(procRoot *os.File, subpath string) (_ *os.File, _ procThread // absolutely sure we are operating on a clean /proc handle that // doesn't have any cheeky overmounts that could trick us (including // symlink mounts on top of /proc/thread-self). RESOLVE_BENEATH isn't - // stricly needed, but just use it since we have it. + // strictly needed, but just use it since we have it. // // NOTE: /proc/self is technically a magic-link (the contents of the // symlink are generated dynamically), but it doesn't use @@ -313,24 +289,16 @@ func procThreadSelf(procRoot *os.File, subpath string) (_ *os.File, _ procThread return handle, runtime.UnlockOSThread, nil } -var ( - hasStatxMountIdBool bool - hasStatxMountIdOnce sync.Once -) - -func hasStatxMountId() bool { - hasStatxMountIdOnce.Do(func() { - var ( - stx unix.Statx_t - // We don't care which mount ID we get. The kernel will give us the - // unique one if it is supported. - wantStxMask uint32 = unix.STATX_MNT_ID_UNIQUE | unix.STATX_MNT_ID - ) - err := unix.Statx(-int(unix.EBADF), "/", 0, int(wantStxMask), &stx) - hasStatxMountIdBool = (err == nil && (stx.Mask&wantStxMask != 0)) - }) - return hasStatxMountIdBool -} +var hasStatxMountId = sync.OnceValue(func() bool { + var ( + stx unix.Statx_t + // We don't care which mount ID we get. The kernel will give us the + // unique one if it is supported. + wantStxMask uint32 = unix.STATX_MNT_ID_UNIQUE | unix.STATX_MNT_ID + ) + err := unix.Statx(-int(unix.EBADF), "/", 0, int(wantStxMask), &stx) + return err == nil && stx.Mask&wantStxMask != 0 +}) func getMountId(dir *os.File, path string) (uint64, error) { // If we don't have statx(STATX_MNT_ID*) support, we can't do anything. @@ -443,22 +411,6 @@ func isDeadInode(file *os.File) error { return nil } -func getUmask() int { - // umask is a per-thread property, but it is inherited by children, so we - // need to lock our OS thread to make sure that no other goroutine runs in - // this thread and no goroutines are spawned from this thread until we - // revert to the old umask. - // - // We could parse /proc/self/status to avoid this get-set problem, but - // /proc/thread-self requires LockOSThread anyway, so there's no real - // benefit over just using umask(2). - runtime.LockOSThread() - umask := unix.Umask(0) - unix.Umask(umask) - runtime.UnlockOSThread() - return umask -} - func checkProcSelfFdPath(path string, file *os.File) error { if err := isDeadInode(file); err != nil { return err diff --git a/vendor/github.com/cyphar/filepath-securejoin/vfs.go b/vendor/github.com/cyphar/filepath-securejoin/vfs.go index 6e27c7dd8e1..36373f8c517 100644 --- a/vendor/github.com/cyphar/filepath-securejoin/vfs.go +++ b/vendor/github.com/cyphar/filepath-securejoin/vfs.go @@ -10,19 +10,19 @@ import "os" // are several projects (umoci and go-mtree) that are using this sort of // interface. -// VFS is the minimal interface necessary to use SecureJoinVFS. A nil VFS is -// equivalent to using the standard os.* family of functions. This is mainly +// VFS is the minimal interface necessary to use [SecureJoinVFS]. A nil VFS is +// equivalent to using the standard [os].* family of functions. This is mainly // used for the purposes of mock testing, but also can be used to otherwise use -// SecureJoin with VFS-like system. +// [SecureJoinVFS] with VFS-like system. type VFS interface { - // Lstat returns a FileInfo describing the named file. If the file is a - // symbolic link, the returned FileInfo describes the symbolic link. Lstat - // makes no attempt to follow the link. These semantics are identical to - // os.Lstat. + // Lstat returns an [os.FileInfo] describing the named file. If the + // file is a symbolic link, the returned [os.FileInfo] describes the + // symbolic link. Lstat makes no attempt to follow the link. + // The semantics are identical to [os.Lstat]. Lstat(name string) (os.FileInfo, error) - // Readlink returns the destination of the named symbolic link. These - // semantics are identical to os.Readlink. + // Readlink returns the destination of the named symbolic link. + // The semantics are identical to [os.Readlink]. Readlink(name string) (string, error) } @@ -30,12 +30,6 @@ type VFS interface { // module. type osVFS struct{} -// Lstat returns a FileInfo describing the named file. If the file is a -// symbolic link, the returned FileInfo describes the symbolic link. Lstat -// makes no attempt to follow the link. These semantics are identical to -// os.Lstat. func (o osVFS) Lstat(name string) (os.FileInfo, error) { return os.Lstat(name) } -// Readlink returns the destination of the named symbolic link. These -// semantics are identical to os.Readlink. func (o osVFS) Readlink(name string) (string, error) { return os.Readlink(name) } diff --git a/vendor/modules.txt b/vendor/modules.txt index 95fd1d36bbf..bd1228e32a3 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -25,8 +25,8 @@ github.com/coreos/go-systemd/v22/dbus # github.com/cpuguy83/go-md2man/v2 v2.0.2 ## explicit; go 1.11 github.com/cpuguy83/go-md2man/v2/md2man -# github.com/cyphar/filepath-securejoin v0.3.2 -## explicit; go 1.20 +# github.com/cyphar/filepath-securejoin v0.3.3 +## explicit; go 1.21 github.com/cyphar/filepath-securejoin # github.com/docker/go-units v0.5.0 ## explicit