Update module github.com/cyphar/filepath-securejoin to v0.3.1

Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This commit is contained in:
renovate[bot] 2024-07-24 14:40:02 +00:00 committed by GitHub
parent 714f7bacb2
commit eadfbbc809
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 351 additions and 179 deletions

2
go.mod
View File

@ -26,7 +26,7 @@ require (
github.com/coreos/stream-metadata-go v0.4.4 github.com/coreos/stream-metadata-go v0.4.4
github.com/crc-org/crc/v2 v2.38.0 github.com/crc-org/crc/v2 v2.38.0
github.com/crc-org/vfkit v0.5.1 github.com/crc-org/vfkit v0.5.1
github.com/cyphar/filepath-securejoin v0.3.0 github.com/cyphar/filepath-securejoin v0.3.1
github.com/digitalocean/go-qemu v0.0.0-20230711162256-2e3d0186973e github.com/digitalocean/go-qemu v0.0.0-20230711162256-2e3d0186973e
github.com/docker/distribution v2.8.3+incompatible github.com/docker/distribution v2.8.3+incompatible
github.com/docker/docker v27.0.3+incompatible github.com/docker/docker v27.0.3+incompatible

4
go.sum
View File

@ -118,8 +118,8 @@ github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY=
github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
github.com/cyberphone/json-canonicalization v0.0.0-20231217050601-ba74d44ecf5f h1:eHnXnuK47UlSTOQexbzxAZfekVz6i+LKRdj1CU5DPaM= github.com/cyberphone/json-canonicalization v0.0.0-20231217050601-ba74d44ecf5f h1:eHnXnuK47UlSTOQexbzxAZfekVz6i+LKRdj1CU5DPaM=
github.com/cyberphone/json-canonicalization v0.0.0-20231217050601-ba74d44ecf5f/go.mod h1:uzvlm1mxhHkdfqitSA92i7Se+S9ksOn3a3qmv/kyOCw= github.com/cyberphone/json-canonicalization v0.0.0-20231217050601-ba74d44ecf5f/go.mod h1:uzvlm1mxhHkdfqitSA92i7Se+S9ksOn3a3qmv/kyOCw=
github.com/cyphar/filepath-securejoin v0.3.0 h1:tXpmbiaeBrS/K2US8nhgwdKYnfAOnVfkcLPKFgFHeA0= github.com/cyphar/filepath-securejoin v0.3.1 h1:1V7cHiaW+C+39wEfpH6XlLBQo3j/PciWFrgfCLS8XrE=
github.com/cyphar/filepath-securejoin v0.3.0/go.mod h1:F7i41x/9cBF7lzCrVsYs9fuzwRZm4NQsGTBdpp6mETc= github.com/cyphar/filepath-securejoin v0.3.1/go.mod h1:F7i41x/9cBF7lzCrVsYs9fuzwRZm4NQsGTBdpp6mETc=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=

View File

@ -0,0 +1,138 @@
# Changelog #
All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/)
and this project adheres to [Semantic Versioning](http://semver.org/).
## [Unreleased] ##
## [0.3.1] - 2024-07-23 ##
### Changed ###
- By allowing `Open(at)InRoot` to opt-out of the extra work done by `MkdirAll`
to do the necessary "partial lookups", `Open(at)InRoot` now does less work
for both implementations (resulting in a many-fold decrease in the number of
operations for `openat2`, and a modest improvement for non-`openat2`) and is
far more guaranteed to match the correct `openat2(RESOLVE_IN_ROOT)`
behaviour.
- We now use `readlinkat(fd, "")` where possible. For `Open(at)InRoot` this
effectively just means that we no longer risk getting spurious errors during
rename races. However, for our hardened procfs handler, this in theory should
prevent mount attacks from tricking us when doing magic-link readlinks (even
when using the unsafe host `/proc` handle). Unfortunately `Reopen` is still
potentially vulnerable to those kinds of somewhat-esoteric attacks.
Technically this [will only work on post-2.6.39 kernels][linux-readlinkat-emptypath]
but it seems incredibly unlikely anyone is using `filepath-securejoin` on a
pre-2011 kernel.
### Fixed ###
- Several improvements were made to the errors returned by `Open(at)InRoot` and
`MkdirAll` when dealing with invalid paths under the emulated (ie.
non-`openat2`) implementation. Previously, some paths would return the wrong
error (`ENOENT` when the last component was a non-directory), and other paths
would be returned as though they were acceptable (trailing-slash components
after a non-directory would be ignored by `Open(at)InRoot`).
These changes were done to match `openat2`'s behaviour and purely is a
consistency fix (most users are going to be using `openat2` anyway).
[linux-readlinkat-emptypath]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=65cfc6722361570bfe255698d9cd4dccaf47570d
## [0.3.0] - 2024-07-11 ##
### Added ###
- A new set of `*os.File`-based APIs have been added. These are adapted from
[libpathrs][] and we strongly suggest using them if possible (as they provide
far more protection against attacks than `SecureJoin`):
- `Open(at)InRoot` resolves a path inside a rootfs and returns an `*os.File`
handle to the path. Note that the handle returned is an `O_PATH` handle,
which cannot be used for reading or writing (as well as some other
operations -- [see open(2) for more details][open.2])
- `Reopen` takes an `O_PATH` file handle and safely re-opens it to upgrade
it to a regular handle. This can also be used with non-`O_PATH` handles,
but `O_PATH` is the most obvious application.
- `MkdirAll` is an implementation of `os.MkdirAll` that is safe to use to
create a directory tree within a rootfs.
As these are new APIs, they may change in the future. However, they should be
safe to start migrating to as we have extensive tests ensuring they behave
correctly and are safe against various races and other attacks.
[libpathrs]: https://github.com/openSUSE/libpathrs
[open.2]: https://www.man7.org/linux/man-pages/man2/open.2.html
## [0.2.5] - 2024-05-03 ##
### Changed ###
- Some minor changes were made to how lexical components (like `..` and `.`)
are handled during path generation in `SecureJoin`. There is no behaviour
change as a result of this fix (the resulting paths are the same).
### Fixed ###
- The error returned when we hit a symlink loop now references the correct
path. (#10)
## [0.2.4] - 2023-09-06 ##
### Security ###
- This release fixes a potential security issue in filepath-securejoin when
used on Windows ([GHSA-6xv5-86q9-7xr8][], which could be used to generate
paths outside of the provided rootfs in certain cases), as well as improving
the overall behaviour of filepath-securejoin when dealing with Windows paths
that contain volume names. Thanks to Paulo Gomes for discovering and fixing
these issues.
### Fixed ###
- Switch to GitHub Actions for CI so we can test on Windows as well as Linux
and MacOS.
[GHSA-6xv5-86q9-7xr8]: https://github.com/advisories/GHSA-6xv5-86q9-7xr8
## [0.2.3] - 2021-06-04 ##
### Changed ###
- Switch to Go 1.13-style `%w` error wrapping, letting us drop the dependency
on `github.com/pkg/errors`.
## [0.2.2] - 2018-09-05 ##
### Changed ###
- Use `syscall.ELOOP` as the base error for symlink loops, rather than our own
(internal) error. This allows callers to more easily use `errors.Is` to check
for this case.
## [0.2.1] - 2018-09-05 ##
### Fixed ###
- Use our own `IsNotExist` implementation, which lets us handle `ENOTDIR`
properly within `SecureJoin`.
## [0.2.0] - 2017-07-19 ##
We now have 100% test coverage!
### Added ###
- Add a `SecureJoinVFS` API that can be used for mocking (as we do in our new
tests) or for implementing custom handling of lookup operations (such as for
rootless containers, where work is necessary to access directories with weird
modes because we don't have `CAP_DAC_READ_SEARCH` or `CAP_DAC_OVERRIDE`).
## 0.1.0 - 2017-07-19
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.1...HEAD
[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
[0.2.5]: https://github.com/cyphar/filepath-securejoin/compare/v0.2.4...v0.2.5
[0.2.4]: https://github.com/cyphar/filepath-securejoin/compare/v0.2.3...v0.2.4
[0.2.3]: https://github.com/cyphar/filepath-securejoin/compare/v0.2.2...v0.2.3
[0.2.2]: https://github.com/cyphar/filepath-securejoin/compare/v0.2.1...v0.2.2
[0.2.1]: https://github.com/cyphar/filepath-securejoin/compare/v0.2.0...v0.2.1
[0.2.0]: https://github.com/cyphar/filepath-securejoin/compare/v0.1.0...v0.2.0

View File

@ -1 +1 @@
0.3.0 0.3.1

View File

@ -40,16 +40,18 @@ func (se symlinkStackEntry) Close() {
type symlinkStack []*symlinkStackEntry type symlinkStack []*symlinkStackEntry
func (s symlinkStack) IsEmpty() bool { func (s *symlinkStack) IsEmpty() bool {
return len(s) == 0 return s == nil || len(*s) == 0
} }
func (s *symlinkStack) Close() { func (s *symlinkStack) Close() {
for _, link := range *s { if s != nil {
link.Close() for _, link := range *s {
link.Close()
}
// TODO: Switch to clear once we switch to Go 1.21.
*s = nil
} }
// TODO: Switch to clear once we switch to Go 1.21.
*s = nil
} }
var ( var (
@ -58,11 +60,16 @@ var (
) )
func (s *symlinkStack) popPart(part string) error { func (s *symlinkStack) popPart(part string) error {
if s.IsEmpty() { if s == nil || s.IsEmpty() {
// If there is nothing in the symlink stack, then the part was from the // If there is nothing in the symlink stack, then the part was from the
// real path provided by the user, and this is a no-op. // real path provided by the user, and this is a no-op.
return errEmptyStack return errEmptyStack
} }
if part == "." {
// "." components are no-ops -- we drop them when doing SwapLink.
return nil
}
tailEntry := (*s)[len(*s)-1] tailEntry := (*s)[len(*s)-1]
// Double-check that we are popping the component we expect. // Double-check that we are popping the component we expect.
@ -102,17 +109,13 @@ func (s *symlinkStack) PopPart(part string) error {
} }
func (s *symlinkStack) push(dir *os.File, remainingPath, linkTarget string) error { func (s *symlinkStack) push(dir *os.File, remainingPath, linkTarget string) error {
if s == nil {
return nil
}
// Split the link target and clean up any "" parts. // Split the link target and clean up any "" parts.
linkTargetParts := slices.DeleteFunc( linkTargetParts := slices.DeleteFunc(
strings.Split(linkTarget, "/"), strings.Split(linkTarget, "/"),
func(part string) bool { return part == "" }) func(part string) bool { return part == "" || part == "." })
// Don't add a no-op link to the stack. You can't create a no-op link
// symlink, but if the symlink is /, partialLookupInRoot has already jumped to the
// root and so there's nothing more to do.
if len(linkTargetParts) == 0 {
return nil
}
// Copy the directory so the caller doesn't close our copy. // Copy the directory so the caller doesn't close our copy.
dirCopy, err := dupFile(dir) dirCopy, err := dupFile(dir)
@ -145,7 +148,7 @@ func (s *symlinkStack) SwapLink(linkPart string, dir *os.File, remainingPath, li
} }
func (s *symlinkStack) PopTopSymlink() (*os.File, string, bool) { func (s *symlinkStack) PopTopSymlink() (*os.File, string, bool) {
if s.IsEmpty() { if s == nil || s.IsEmpty() {
return nil, "", false return nil, "", false
} }
tailEntry := (*s)[0] tailEntry := (*s)[0]
@ -157,7 +160,22 @@ func (s *symlinkStack) PopTopSymlink() (*os.File, string, bool) {
// within the provided root (a-la RESOLVE_IN_ROOT) and opens the final existing // within the provided root (a-la RESOLVE_IN_ROOT) and opens the final existing
// component of the requested path, returning a file handle to the final // component of the requested path, returning a file handle to the final
// existing component and a string containing the remaining path components. // existing component and a string containing the remaining path components.
func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string, Err error) { func partialLookupInRoot(root *os.File, unsafePath string) (*os.File, string, error) {
return lookupInRoot(root, unsafePath, true)
}
func completeLookupInRoot(root *os.File, unsafePath string) (*os.File, error) {
handle, remainingPath, err := lookupInRoot(root, unsafePath, false)
if remainingPath != "" && err == nil {
// should never happen
err = fmt.Errorf("[bug] non-empty remaining path when doing a non-partial lookup: %q", remainingPath)
}
// lookupInRoot(partial=false) will always close the handle if an error is
// returned, so no need to double-check here.
return handle, err
}
func lookupInRoot(root *os.File, unsafePath string, partial bool) (Handle *os.File, _ string, _ error) {
unsafePath = filepath.ToSlash(unsafePath) // noop unsafePath = filepath.ToSlash(unsafePath) // noop
// This is very similar to SecureJoin, except that we operate on the // This is very similar to SecureJoin, except that we operate on the
@ -166,7 +184,7 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
// Try to use openat2 if possible. // Try to use openat2 if possible.
if hasOpenat2() { if hasOpenat2() {
return partialLookupOpenat2(root, unsafePath) return lookupOpenat2(root, unsafePath, partial)
} }
// Get the "actual" root path from /proc/self/fd. This is necessary if the // Get the "actual" root path from /proc/self/fd. This is necessary if the
@ -183,7 +201,8 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
return nil, "", fmt.Errorf("clone root fd: %w", err) return nil, "", fmt.Errorf("clone root fd: %w", err)
} }
defer func() { defer func() {
if Err != nil { // If a handle is not returned, close the internal handle.
if Handle == nil {
_ = currentDir.Close() _ = currentDir.Close()
} }
}() }()
@ -200,8 +219,11 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
// Note that the stack is ONLY used for book-keeping. All of the actual // Note that the stack is ONLY used for book-keeping. All of the actual
// path walking logic is still based on currentPath/remainingPath and // path walking logic is still based on currentPath/remainingPath and
// currentDir (as in SecureJoin). // currentDir (as in SecureJoin).
var symlinkStack symlinkStack var symStack *symlinkStack
defer symlinkStack.Close() if partial {
symStack = new(symlinkStack)
defer symStack.Close()
}
var ( var (
linksWalked int linksWalked int
@ -220,9 +242,11 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
} else { } else {
part, remainingPath = remainingPath[:i], remainingPath[i+1:] part, remainingPath = remainingPath[:i], remainingPath[i+1:]
} }
// Skip any "//" components. // If we hit an empty component, we need to treat it as though it is
// "." so that trailing "/" and "//" components on a non-directory
// correctly return the right error code.
if part == "" { if part == "" {
continue part = "."
} }
// Apply the component lexically to the path we are building. // Apply the component lexically to the path we are building.
@ -233,7 +257,7 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
// If we logically hit the root, just clone the root rather than // If we logically hit the root, just clone the root rather than
// opening the part and doing all of the other checks. // opening the part and doing all of the other checks.
if nextPath == "/" { if nextPath == "/" {
if err := symlinkStack.PopPart(part); err != nil { if err := symStack.PopPart(part); err != nil {
return nil, "", fmt.Errorf("walking into root with part %q failed: %w", part, err) return nil, "", fmt.Errorf("walking into root with part %q failed: %w", part, err)
} }
// Jump to root. // Jump to root.
@ -258,14 +282,49 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
} }
switch st.Mode() & os.ModeType { switch st.Mode() & os.ModeType {
case os.ModeDir: case os.ModeSymlink:
// readlinkat implies AT_EMPTY_PATH since Linux 2.6.39. See
// Linux commit 65cfc6722361 ("readlinkat(), fchownat() and
// fstatat() with empty relative pathnames").
linkDest, err := readlinkatFile(nextDir, "")
// We don't need the handle anymore.
_ = nextDir.Close()
if err != nil {
return nil, "", err
}
linksWalked++
if linksWalked > maxSymlinkLimit {
return nil, "", &os.PathError{Op: "securejoin.lookupInRoot", Path: logicalRootPath + "/" + unsafePath, Err: unix.ELOOP}
}
// Swap out the symlink's component for the link entry itself.
if err := symStack.SwapLink(part, currentDir, oldRemainingPath, linkDest); err != nil {
return nil, "", fmt.Errorf("walking into symlink %q failed: push symlink: %w", part, err)
}
// Update our logical remaining path.
remainingPath = linkDest + "/" + remainingPath
// Absolute symlinks reset any work we've already done.
if path.IsAbs(linkDest) {
// Jump to root.
rootClone, err := dupFile(root)
if err != nil {
return nil, "", fmt.Errorf("clone root fd: %w", err)
}
_ = currentDir.Close()
currentDir = rootClone
currentPath = "/"
}
default:
// If we are dealing with a directory, simply walk into it. // If we are dealing with a directory, simply walk into it.
_ = currentDir.Close() _ = currentDir.Close()
currentDir = nextDir currentDir = nextDir
currentPath = nextPath currentPath = nextPath
// The part was real, so drop it from the symlink stack. // The part was real, so drop it from the symlink stack.
if err := symlinkStack.PopPart(part); err != nil { if err := symStack.PopPart(part); err != nil {
return nil, "", fmt.Errorf("walking into directory %q failed: %w", part, err) return nil, "", fmt.Errorf("walking into directory %q failed: %w", part, err)
} }
@ -286,95 +345,45 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
return nil, "", fmt.Errorf("walking into %q had unexpected result: %w", part, err) return nil, "", fmt.Errorf("walking into %q had unexpected result: %w", part, err)
} }
} }
case os.ModeSymlink:
// We don't need the handle anymore.
_ = nextDir.Close()
// Unfortunately, we cannot readlink through our handle and so
// we need to do a separate readlinkat (which could race to
// give us an error if the attacker swapped the symlink with a
// non-symlink).
linkDest, err := readlinkatFile(currentDir, part)
if err != nil {
if errors.Is(err, unix.EINVAL) {
// The part was not a symlink, so assume that it's a
// regular file. It is possible for it to be a
// directory (if an attacker is swapping a directory
// and non-directory at this subpath) but erroring out
// here is better anyway.
err = fmt.Errorf("%w: path component %q is invalid: %w", errPossibleAttack, part, unix.ENOTDIR)
}
return nil, "", err
}
linksWalked++
if linksWalked > maxSymlinkLimit {
return nil, "", &os.PathError{Op: "partialLookupInRoot", Path: logicalRootPath + "/" + unsafePath, Err: unix.ELOOP}
}
// Swap out the symlink's component for the link entry itself.
if err := symlinkStack.SwapLink(part, currentDir, oldRemainingPath, linkDest); err != nil {
return nil, "", fmt.Errorf("walking into symlink %q failed: push symlink: %w", part, err)
}
// Update our logical remaining path.
remainingPath = linkDest + "/" + remainingPath
// Absolute symlinks reset any work we've already done.
if path.IsAbs(linkDest) {
// Jump to root.
rootClone, err := dupFile(root)
if err != nil {
return nil, "", fmt.Errorf("clone root fd: %w", err)
}
_ = currentDir.Close()
currentDir = rootClone
currentPath = "/"
}
default:
// For any other file type, we can't walk further and so we've
// hit the end of the lookup. The handling is very similar to
// ENOENT from openat(2), except that we return a handle to the
// component we just walked into (and we drop the component
// from the symlink stack).
_ = currentDir.Close()
// The part existed, so drop it from the symlink stack.
if err := symlinkStack.PopPart(part); err != nil {
return nil, "", fmt.Errorf("walking into non-directory %q failed: %w", part, err)
}
// If there are any remaining components in the symlink stack,
// we are still within a symlink resolution and thus we hit a
// dangling symlink. So pretend that the first symlink in the
// stack we hit was an ENOENT (to match openat2).
if oldDir, remainingPath, ok := symlinkStack.PopTopSymlink(); ok {
_ = nextDir.Close()
return oldDir, remainingPath, nil
}
// The current component exists, so return it.
return nextDir, remainingPath, nil
} }
case errors.Is(err, os.ErrNotExist): default:
if !partial {
return nil, "", err
}
// If there are any remaining components in the symlink stack, we // If there are any remaining components in the symlink stack, we
// are still within a symlink resolution and thus we hit a dangling // are still within a symlink resolution and thus we hit a dangling
// symlink. So pretend that the first symlink in the stack we hit // symlink. So pretend that the first symlink in the stack we hit
// was an ENOENT (to match openat2). // was an ENOENT (to match openat2).
if oldDir, remainingPath, ok := symlinkStack.PopTopSymlink(); ok { if oldDir, remainingPath, ok := symStack.PopTopSymlink(); ok {
_ = currentDir.Close() _ = currentDir.Close()
return oldDir, remainingPath, nil return oldDir, remainingPath, err
} }
// We have hit a final component that doesn't exist, so we have our // We have hit a final component that doesn't exist, so we have our
// partial open result. Note that we have to use the OLD remaining // partial open result. Note that we have to use the OLD remaining
// path, since the lookup failed. // path, since the lookup failed.
return currentDir, oldRemainingPath, nil return currentDir, oldRemainingPath, err
default:
return nil, "", err
} }
} }
// If the unsafePath had a trailing slash, we need to make sure we try to
// do a relative "." open so that we will correctly return an error when
// the final component is a non-directory (to match openat2). In the
// context of openat2, a trailing slash and a trailing "/." are completely
// equivalent.
if strings.HasSuffix(unsafePath, "/") {
nextDir, err := openatFile(currentDir, ".", unix.O_PATH|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0)
if err != nil {
if !partial {
_ = currentDir.Close()
currentDir = nil
}
return currentDir, "", err
}
_ = currentDir.Close()
currentDir = nextDir
}
// All of the components existed! // All of the components existed!
return currentDir, "", nil return currentDir, "", nil
} }

View File

@ -49,14 +49,14 @@ func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err
// Try to open as much of the path as possible. // Try to open as much of the path as possible.
currentDir, remainingPath, err := partialLookupInRoot(root, unsafePath) currentDir, remainingPath, err := partialLookupInRoot(root, unsafePath)
if err != nil {
return nil, fmt.Errorf("find existing subpath of %q: %w", unsafePath, err)
}
defer func() { defer func() {
if Err != nil { if Err != nil {
_ = currentDir.Close() _ = currentDir.Close()
} }
}() }()
if err != nil && !errors.Is(err, unix.ENOENT) {
return nil, fmt.Errorf("find existing subpath of %q: %w", unsafePath, err)
}
// If there is an attacker deleting directories as we walk into them, // If there is an attacker deleting directories as we walk into them,
// detect this proactively. Note this is guaranteed to detect if the // detect this proactively. Note this is guaranteed to detect if the
@ -82,6 +82,7 @@ func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err
} else if err != nil { } else if err != nil {
return nil, fmt.Errorf("re-opening handle to %q: %w", currentDir.Name(), err) return nil, fmt.Errorf("re-opening handle to %q: %w", currentDir.Name(), err)
} else { } else {
_ = currentDir.Close()
currentDir = reopenDir currentDir = reopenDir
} }

View File

@ -9,6 +9,7 @@ package securejoin
import ( import (
"fmt" "fmt"
"os" "os"
"strconv"
"golang.org/x/sys/unix" "golang.org/x/sys/unix"
) )
@ -16,13 +17,9 @@ import (
// OpenatInRoot is equivalent to OpenInRoot, except that the root is provided // 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. // using an *os.File handle, to ensure that the correct root directory is used.
func OpenatInRoot(root *os.File, unsafePath string) (*os.File, error) { func OpenatInRoot(root *os.File, unsafePath string) (*os.File, error) {
handle, remainingPath, err := partialLookupInRoot(root, unsafePath) handle, err := completeLookupInRoot(root, unsafePath)
if err != nil { if err != nil {
return nil, err return nil, &os.PathError{Op: "securejoin.OpenInRoot", Path: unsafePath, Err: err}
}
if remainingPath != "" {
_ = handle.Close()
return nil, &os.PathError{Op: "securejoin.OpenInRoot", Path: unsafePath, Err: unix.ENOENT}
} }
return handle, nil return handle, nil
} }
@ -69,15 +66,36 @@ func Reopen(handle *os.File, flags int) (*os.File, error) {
return nil, err return nil, err
} }
// We can't operate on /proc/thread-self/fd/$n directly when doing a
// re-open, so we need to open /proc/thread-self/fd and then open a single
// final component.
procFdDir, closer, err := procThreadSelf(procRoot, "fd/")
if err != nil {
return nil, fmt.Errorf("get safe /proc/thread-self/fd handle: %w", err)
}
defer procFdDir.Close()
defer closer()
// Try to detect if there is a mount on top of the magic-link we are about
// to open. If we are using unsafeHostProcRoot(), this could change after
// we check it (and there's nothing we can do about that) but for
// privateProcRoot() this should be guaranteed to be safe (at least since
// Linux 5.12[1], when anonymous mount namespaces were completely isolated
// from external mounts including mount propagation events).
//
// [1]: Linux commit ee2e3f50629f ("mount: fix mounting of detached mounts
// onto targets that reside on shared mounts").
fdStr := strconv.Itoa(int(handle.Fd()))
if err := checkSymlinkOvermount(procRoot, procFdDir, fdStr); err != nil {
return nil, fmt.Errorf("check safety of /proc/thread-self/fd/%s magiclink: %w", fdStr, err)
}
flags |= unix.O_CLOEXEC flags |= unix.O_CLOEXEC
fdPath := fmt.Sprintf("fd/%d", handle.Fd()) // Rather than just wrapping openatFile, open-code it so we can copy
return doProcSelfMagiclink(procRoot, fdPath, func(procDirHandle *os.File, base string) (*os.File, error) { // handle.Name().
// Rather than just wrapping openatFile, open-code it so we can copy reopenFd, err := unix.Openat(int(procFdDir.Fd()), fdStr, flags, 0)
// handle.Name(). if err != nil {
reopenFd, err := unix.Openat(int(procDirHandle.Fd()), base, flags, 0) return nil, fmt.Errorf("reopen fd %d: %w", handle.Fd(), err)
if err != nil { }
return nil, fmt.Errorf("reopen fd %d: %w", handle.Fd(), err) return os.NewFile(uintptr(reopenFd), handle.Name()), nil
}
return os.NewFile(uintptr(reopenFd), handle.Name()), nil
})
} }

View File

@ -87,6 +87,17 @@ func openat2File(dir *os.File, path string, how *unix.OpenHow) (*os.File, error)
return nil, &os.PathError{Op: "openat2", Path: fullPath, Err: errPossibleAttack} return nil, &os.PathError{Op: "openat2", Path: fullPath, Err: errPossibleAttack}
} }
func lookupOpenat2(root *os.File, unsafePath string, partial bool) (*os.File, string, error) {
if !partial {
file, err := openat2File(root, unsafePath, &unix.OpenHow{
Flags: unix.O_PATH | unix.O_CLOEXEC,
Resolve: unix.RESOLVE_IN_ROOT | unix.RESOLVE_NO_MAGICLINKS,
})
return file, "", err
}
return partialLookupOpenat2(root, unsafePath)
}
// partialLookupOpenat2 is an alternative implementation of // partialLookupOpenat2 is an alternative implementation of
// partialLookupInRoot, using openat2(RESOLVE_IN_ROOT) to more safely get a // partialLookupInRoot, using openat2(RESOLVE_IN_ROOT) to more safely get a
// handle to the deepest existing child of the requested path within the root. // handle to the deepest existing child of the requested path within the root.
@ -95,6 +106,7 @@ func partialLookupOpenat2(root *os.File, unsafePath string) (*os.File, string, e
unsafePath = filepath.ToSlash(unsafePath) // noop unsafePath = filepath.ToSlash(unsafePath) // noop
endIdx := len(unsafePath) endIdx := len(unsafePath)
var lastError error
for endIdx > 0 { for endIdx > 0 {
subpath := unsafePath[:endIdx] subpath := unsafePath[:endIdx]
@ -108,11 +120,12 @@ func partialLookupOpenat2(root *os.File, unsafePath string) (*os.File, string, e
endIdx += 1 endIdx += 1
} }
// We found a subpath! // We found a subpath!
return handle, unsafePath[endIdx:], nil return handle, unsafePath[endIdx:], lastError
} }
if errors.Is(err, unix.ENOENT) || errors.Is(err, unix.ENOTDIR) { if errors.Is(err, unix.ENOENT) || errors.Is(err, unix.ENOTDIR) {
// That path doesn't exist, let's try the next directory up. // That path doesn't exist, let's try the next directory up.
endIdx = strings.LastIndexByte(subpath, '/') endIdx = strings.LastIndexByte(subpath, '/')
lastError = err
continue continue
} }
return nil, "", fmt.Errorf("open subpath: %w", err) return nil, "", fmt.Errorf("open subpath: %w", err)
@ -124,5 +137,5 @@ func partialLookupOpenat2(root *os.File, unsafePath string) (*os.File, string, e
if err != nil { if err != nil {
return nil, "", err return nil, "", err
} }
return rootClone, unsafePath, nil return rootClone, unsafePath, lastError
} }

View File

@ -10,7 +10,6 @@ import (
"errors" "errors"
"fmt" "fmt"
"os" "os"
"path/filepath"
"runtime" "runtime"
"strconv" "strconv"
"sync" "sync"
@ -160,7 +159,7 @@ func clonePrivateProcMount() (_ *os.File, Err error) {
} }
func privateProcRoot() (*os.File, error) { func privateProcRoot() (*os.File, error) {
if !hasNewMountApi() { if !hasNewMountApi() || testingForceGetProcRootUnsafe() {
return nil, fmt.Errorf("new mount api: %w", unix.ENOTSUP) return nil, fmt.Errorf("new mount api: %w", unix.ENOTSUP)
} }
// Try to create a new procfs mount from scratch if we can. This ensures we // Try to create a new procfs mount from scratch if we can. This ensures we
@ -199,7 +198,7 @@ func unsafeHostProcRoot() (_ *os.File, Err error) {
func doGetProcRoot() (*os.File, error) { func doGetProcRoot() (*os.File, error) {
procRoot, err := privateProcRoot() procRoot, err := privateProcRoot()
if err != nil || testingForceGetProcRootUnsafe(procRoot) { if err != nil {
// Fall back to using a /proc handle if making a private mount failed. // Fall back to using a /proc handle if making a private mount failed.
// If we have openat2, at least we can avoid some kinds of over-mount // If we have openat2, at least we can avoid some kinds of over-mount
// attacks, but without openat2 there's not much we can do. // attacks, but without openat2 there's not much we can do.
@ -286,14 +285,14 @@ func procThreadSelf(procRoot *os.File, subpath string) (_ *os.File, _ procThread
// procSelfFdReadlink to clean up the returned f.Name() if we use // procSelfFdReadlink to clean up the returned f.Name() if we use
// RESOLVE_IN_ROOT (which would lead to an infinite recursion). // RESOLVE_IN_ROOT (which would lead to an infinite recursion).
handle, err = openat2File(procRoot, threadSelf+subpath, &unix.OpenHow{ handle, err = openat2File(procRoot, threadSelf+subpath, &unix.OpenHow{
Flags: unix.O_PATH | unix.O_CLOEXEC, Flags: unix.O_PATH | unix.O_NOFOLLOW | unix.O_CLOEXEC,
Resolve: unix.RESOLVE_BENEATH | unix.RESOLVE_NO_XDEV | unix.RESOLVE_NO_MAGICLINKS, Resolve: unix.RESOLVE_BENEATH | unix.RESOLVE_NO_XDEV | unix.RESOLVE_NO_MAGICLINKS,
}) })
if err != nil { if err != nil {
return nil, nil, fmt.Errorf("%w: %w", errUnsafeProcfs, err) return nil, nil, fmt.Errorf("%w: %w", errUnsafeProcfs, err)
} }
} else { } else {
handle, err = openatFile(procRoot, threadSelf+subpath, unix.O_PATH|unix.O_CLOEXEC, 0) handle, err = openatFile(procRoot, threadSelf+subpath, unix.O_PATH|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0)
if err != nil { if err != nil {
return nil, nil, fmt.Errorf("%w: %w", errUnsafeProcfs, err) return nil, nil, fmt.Errorf("%w: %w", errUnsafeProcfs, err)
} }
@ -333,10 +332,10 @@ func hasStatxMountId() bool {
return hasStatxMountIdBool return hasStatxMountIdBool
} }
func checkSymlinkOvermount(dir *os.File, path string) error { func getMountId(dir *os.File, path string) (uint64, error) {
// If we don't have statx(STATX_MNT_ID*) support, we can't do anything. // If we don't have statx(STATX_MNT_ID*) support, we can't do anything.
if !hasStatxMountId() { if !hasStatxMountId() {
return nil return 0, nil
} }
var ( var (
@ -346,31 +345,29 @@ func checkSymlinkOvermount(dir *os.File, path string) error {
wantStxMask uint32 = unix.STATX_MNT_ID_UNIQUE | unix.STATX_MNT_ID wantStxMask uint32 = unix.STATX_MNT_ID_UNIQUE | unix.STATX_MNT_ID
) )
err := unix.Statx(int(dir.Fd()), path, unix.AT_EMPTY_PATH|unix.AT_SYMLINK_NOFOLLOW, int(wantStxMask), &stx)
if stx.Mask&wantStxMask == 0 {
// It's not a kernel limitation, for some reason we couldn't get a
// mount ID. Assume it's some kind of attack.
err = fmt.Errorf("%w: could not get mount id", errUnsafeProcfs)
}
if err != nil {
return 0, &os.PathError{Op: "statx(STATX_MNT_ID_...)", Path: dir.Name() + "/" + path, Err: err}
}
return stx.Mnt_id, nil
}
func checkSymlinkOvermount(procRoot *os.File, dir *os.File, path string) error {
// Get the mntId of our procfs handle. // Get the mntId of our procfs handle.
err := unix.Statx(int(dir.Fd()), "", unix.AT_EMPTY_PATH, int(wantStxMask), &stx) expectedMountId, err := getMountId(procRoot, "")
if err != nil { if err != nil {
return &os.PathError{Op: "statx", Path: dir.Name(), Err: err} return err
} }
if stx.Mask&wantStxMask == 0 { // Get the mntId of the target magic-link.
// It's not a kernel limitation, for some reason we couldn't get a gotMountId, err := getMountId(dir, path)
// mount ID. Assume it's some kind of attack.
return fmt.Errorf("%w: could not get mnt id of dir %s", errUnsafeProcfs, dir.Name())
}
expectedMountId := stx.Mnt_id
// Get the mntId of the target symlink.
stx = unix.Statx_t{}
err = unix.Statx(int(dir.Fd()), path, unix.AT_SYMLINK_NOFOLLOW, int(wantStxMask), &stx)
if err != nil { if err != nil {
return &os.PathError{Op: "statx", Path: dir.Name() + "/" + path, Err: err} return err
} }
if stx.Mask&wantStxMask == 0 {
// It's not a kernel limitation, for some reason we couldn't get a
// mount ID. Assume it's some kind of attack.
return fmt.Errorf("%w: could not get mnt id of symlink %s", errUnsafeProcfs, path)
}
gotMountId := stx.Mnt_id
// As long as the directory mount is alive, even with wrapping mount IDs, // As long as the directory mount is alive, even with wrapping mount IDs,
// we would expect to see a different mount ID here. (Of course, if we're // we would expect to see a different mount ID here. (Of course, if we're
// using unsafeHostProcRoot() then an attaker could change this after we // using unsafeHostProcRoot() then an attaker could change this after we
@ -381,37 +378,33 @@ func checkSymlinkOvermount(dir *os.File, path string) error {
return nil return nil
} }
func doProcSelfMagiclink[T any](procRoot *os.File, subPath string, fn func(procDirHandle *os.File, base string) (T, error)) (T, error) { func doRawProcSelfFdReadlink(procRoot *os.File, fd int) (string, error) {
// We cannot operate on the magic-link directly with a handle, we need to fdPath := fmt.Sprintf("fd/%d", fd)
// create a handle to the parent of the magic-link and then do procFdLink, closer, err := procThreadSelf(procRoot, fdPath)
// single-component operations on it.
dir, base := filepath.Dir(subPath), filepath.Base(subPath)
procDirHandle, closer, err := procThreadSelf(procRoot, dir)
if err != nil { if err != nil {
return *new(T), fmt.Errorf("get safe /proc/thread-self/%s handle: %w", dir, err) return "", fmt.Errorf("get safe /proc/thread-self/%s handle: %w", fdPath, err)
} }
defer procDirHandle.Close() defer procFdLink.Close()
defer closer() defer closer()
// Try to detect if there is a mount on top of the symlink we are about to // Try to detect if there is a mount on top of the magic-link. Since we use the handle directly
// read. If we are using unsafeHostProcRoot(), this could change after we // provide to the closure. If the closure uses the handle directly, this
// check it (and there's nothing we can do about that) but for // should be safe in general (a mount on top of the path afterwards would
// privateProcRoot() this should be guaranteed to be safe (at least since // not affect the handle itself) and will definitely be safe if we are
// Linux 5.12[1], when anonymous mount namespaces were completely isolated // using privateProcRoot() (at least since Linux 5.12[1], when anonymous
// from external mounts including mount propagation events). // mount namespaces were completely isolated from external mounts including
// mount propagation events).
// //
// [1]: Linux commit ee2e3f50629f ("mount: fix mounting of detached mounts // [1]: Linux commit ee2e3f50629f ("mount: fix mounting of detached mounts
// onto targets that reside on shared mounts"). // onto targets that reside on shared mounts").
if err := checkSymlinkOvermount(procDirHandle, base); err != nil { if err := checkSymlinkOvermount(procRoot, procFdLink, ""); err != nil {
return *new(T), fmt.Errorf("check safety of %s proc magiclink: %w", subPath, err) return "", fmt.Errorf("check safety of /proc/thread-self/fd/%d magiclink: %w", fd, err)
} }
return fn(procDirHandle, base)
}
func doRawProcSelfFdReadlink(procRoot *os.File, fd int) (string, error) { // readlinkat implies AT_EMPTY_PATH since Linux 2.6.39. See Linux commit
fdPath := fmt.Sprintf("fd/%d", fd) // 65cfc6722361 ("readlinkat(), fchownat() and fstatat() with empty
return doProcSelfMagiclink(procRoot, fdPath, readlinkatFile) // relative pathnames").
return readlinkatFile(procFdLink, "")
} }
func rawProcSelfFdReadlink(fd int) (string, error) { func rawProcSelfFdReadlink(fd int) (string, error) {

View File

@ -42,9 +42,9 @@ func testingForcePrivateProcRootOpenTreeAtRecursive(f *os.File) bool {
testingCheckClose(*testingForceGetProcRoot >= forceGetProcRootOpenTreeAtRecursive, f) testingCheckClose(*testingForceGetProcRoot >= forceGetProcRootOpenTreeAtRecursive, f)
} }
func testingForceGetProcRootUnsafe(f *os.File) bool { func testingForceGetProcRootUnsafe() bool {
return testing.Testing() && testingForceGetProcRoot != nil && return testing.Testing() && testingForceGetProcRoot != nil &&
testingCheckClose(*testingForceGetProcRoot >= forceGetProcRootUnsafe, f) *testingForceGetProcRoot >= forceGetProcRootUnsafe
} }
type forceProcThreadSelfLevel int type forceProcThreadSelfLevel int

2
vendor/modules.txt vendored
View File

@ -439,7 +439,7 @@ github.com/crc-org/vfkit/pkg/util
# github.com/cyberphone/json-canonicalization v0.0.0-20231217050601-ba74d44ecf5f # github.com/cyberphone/json-canonicalization v0.0.0-20231217050601-ba74d44ecf5f
## explicit ## explicit
github.com/cyberphone/json-canonicalization/go/src/webpki.org/jsoncanonicalizer github.com/cyberphone/json-canonicalization/go/src/webpki.org/jsoncanonicalizer
# github.com/cyphar/filepath-securejoin v0.3.0 # github.com/cyphar/filepath-securejoin v0.3.1
## explicit; go 1.20 ## explicit; go 1.20
github.com/cyphar/filepath-securejoin github.com/cyphar/filepath-securejoin
# github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc # github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc