Skip to content

Commit

Permalink
merge #15 into cyphar/filepath-securejoin:main
Browse files Browse the repository at this point in the history
Aleksa Sarai (6):
  procfs: make procSelfFdReadlink more generic with generics
  open: add Open(at)InRoot and Reopen
  open: add OpenInRoot and Reopen tests
  mkdirall: switch away from O_PATH for mkdir loop
  README: update to describe and strongly recommend new APIs

LGTMs: cyphar
  • Loading branch information
cyphar committed Jul 11, 2024
2 parents a91c705 + 0a923e5 commit 6ae6d58
Show file tree
Hide file tree
Showing 6 changed files with 627 additions and 57 deletions.
139 changes: 114 additions & 25 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,24 @@

[![Build Status](https://github.com/cyphar/filepath-securejoin/actions/workflows/ci.yml/badge.svg)](https://github.com/cyphar/filepath-securejoin/actions/workflows/ci.yml)

An implementation of `SecureJoin`, a [candidate for inclusion in the Go
standard library][go#20126]. The purpose of this function is to be a "secure"
alternative to `filepath.Join`, and in particular it provides certain
guarantees that are not provided by `filepath.Join`.

> **NOTE**: This code is *only* safe if you are not at risk of other processes
> modifying path components after you've used `SecureJoin`. If it is possible
> for a malicious process to modify path components of the resolved path, then
> you will be vulnerable to some fairly trivial TOCTOU race conditions. [There
> are some Linux kernel patches I'm working on which might allow for a better
> solution.][lwn-obeneath]
>
> In addition, with a slightly modified API it might be possible to use
> `O_PATH` and verify that the opened path is actually the resolved one -- but
> I have not done that yet. I might add it in the future as a helper function
> to help users verify the path (we can't just return `/proc/self/fd/<foo>`
> because that doesn't always work transparently for all users).
This is the function prototype:
### Old API ###

```go
func SecureJoin(root, unsafePath string) (string, error)
```
This library was originally just an implementation of `SecureJoin` which was
[intended to be included in the Go standard library][go#20126] as a safer
`filepath.Join` that would restrict the path lookup to be inside a root
directory.

The implementation was based on code that existed in several container
runtimes. Unfortunately, this API is **fundamentally unsafe** against attackers
that can modify path components after `SecureJoin` returns and before the
caller uses the path, allowing for some fairly trivial TOCTOU attacks.

`SecureJoin` (and `SecureJoinVFS`) are still provided by this library to
support legacy users, but new users are strongly suggested to avoid using
`SecureJoin` and instead use the [new api](#new-api) or switch to
[libpathrs][libpathrs].

This library **guarantees** the following:
With the above limitations in mind, this library guarantees the following:

* If no error is set, the resulting string **must** be a child path of
`root` and will not contain any symlink path components (they will all be
Expand All @@ -47,7 +40,7 @@ This library **guarantees** the following:
A (trivial) implementation of this function on GNU/Linux systems could be done
with the following (note that this requires root privileges and is far more
opaque than the implementation in this library, and also requires that
`readlink` is inside the `root` path):
`readlink` is inside the `root` path and is trustworthy):

```go
package securejoin
Expand All @@ -70,9 +63,105 @@ func SecureJoin(root, unsafePath string) (string, error) {
}
```

[lwn-obeneath]: https://lwn.net/Articles/767547/
[libpathrs]: https://github.com/openSUSE/libpathrs
[go#20126]: https://github.com/golang/go/issues/20126

### New API ###

While we recommend users switch to [libpathrs][libpathrs] as soon as it has a
stable release, some methods implemented by libpathrs have been ported to this
library to ease the transition. These APIs are only supported on Linux.

These APIs are implemented such that `filepath-securejoin` will
opportunistically use certain newer kernel APIs that make these operations far
more secure. In particular:

* All of the lookup operations will use [`openat2`][openat2.2] on new enough
kernels (Linux 5.6 or later) to restrict lookups through magic-links and
bind-mounts (for certain operations) and to make use of `RESOLVE_IN_ROOT` to
efficiently resolve symlinks within a rootfs.

* The APIs provide hardening against a malicious `/proc` mount to either detect
or avoid being tricked by a `/proc` that is not legitimate. This is done
using [`openat2`][openat2.2] for all users, and privileged users will also be
further protected by using [`fsopen`][fsopen.2] and [`open_tree`][open_tree.2]
(Linux 4.18 or later).

[openat2.2]: https://www.man7.org/linux/man-pages/man2/openat2.2.html
[fsopen.2]: https://github.com/brauner/man-pages-md/blob/main/fsopen.md
[open_tree.2]: https://github.com/brauner/man-pages-md/blob/main/open_tree.md

#### `OpenInRoot` ####

```go
func OpenInRoot(root, unsafePath string) (*os.File, error)
func OpenatInRoot(root *os.File, unsafePath string) (*os.File, error)
func Reopen(handle *os.File, flags int) (*os.File, error)
```

`OpenInRoot` is a much safer version of

```go
path, err := securejoin.SecureJoin(root, unsafePath)
file, err := os.OpenFile(path, unix.O_PATH|unix.O_CLOEXEC)
```

that protects against various race attacks that could lead to serious security
issues, depending on the application. Note that the returned `*os.File` is an
`O_PATH` file descriptor, which is quite restricted. Callers will probably need
to use `Reopen` to get a more usable handle (this split is done to provide
useful features like PTY spawning and to avoid users accidentally opening bad
inodes that could cause a DoS).

Callers need to be careful in how they use the returned `*os.File`. Usually it
is only safe to operate on the handle directly, and it is very easy to create a
security issue. [libpathrs][libpathrs] provides far more helpers to make using
these handles safer -- there is currently no plan to port them to
`filepath-securejoin`.

`OpenatInRoot` is like `OpenInRoot` except that the root is provided using an
`*os.File`. This allows you to ensure that multiple `OpenatInRoot` (or
`MkdirAllHandle`) calls are operating on the same rootfs.

> **NOTE**: Unlike `SecureJoin`, `OpenInRoot` will error out as soon as it hits
> a dangling symlink or non-existent path. This is in contrast to `SecureJoin`
> which treated non-existent components as though they were real directories,
> and would allow for partial resolution of dangling symlinks. These behaviours
> are at odds with how Linux treats non-existent paths and dangling symlinks,
> and so these are no longer allowed.

#### `MkdirAll` ####

```go
func MkdirAll(root, unsafePath string, mode int) error
func MkdirAllHandle(root *os.File, unsafePath string, mode int) (*os.File, error)
```

`MkdirAll` is a much safer version of

```go
path, err := securejoin.SecureJoin(root, unsafePath)
err = os.MkdirAll(path, mode)
```

that protects against the same kinds of races that `OpenInRoot` protects
against.

`MkdirAllHandle` is like `MkdirAll` except that the root is provided using an
`*os.File` (the reason for this is the same as with `OpenatInRoot`) and an
`*os.File` of the final created directory is returned (this directory is
guaranteed to be effectively identical to the directory created by
`MkdirAllHandle`, which is not possible to ensure by just using `OpenatInRoot`
after `MkdirAll`).

> **NOTE**: Unlike `SecureJoin`, `MkdirAll` will error out as soon as it hits
> a dangling symlink or non-existent path. This is in contrast to `SecureJoin`
> which treated non-existent components as though they were real directories,
> and would allow for partial resolution of dangling symlinks. These behaviours
> are at odds with how Linux treats non-existent paths and dangling symlinks,
> and so these are no longer allowed. This means that `MkdirAll` will not
> create non-existent directories referenced by a dangling symlink.

### License ###

The license of this project is the same as Go, which is a BSD 3-clause license
Expand Down
6 changes: 6 additions & 0 deletions join.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ func IsNotExist(err error) bool {
// replaced with symlinks on the filesystem) after this function has returned.
// Such a symlink race is necessarily out-of-scope of SecureJoin.
//
// NOTE: Due to the above limitation, Linux users are strongly encouraged to
// 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).
//
// Volume names in unsafePath are always discarded, regardless if they are
// provided via direct input or when evaluating symlinks. Therefore:
//
Expand Down
45 changes: 22 additions & 23 deletions mkdir_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,15 @@ func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err
return nil, fmt.Errorf("finding existing subpath of %q: %w", unsafePath, err)
}

// Check that we actually got a directory.
if st, err := currentDir.Stat(); err != nil {
return nil, fmt.Errorf("stat existing subpath handle %q: %w", currentDir.Name(), err)
} else if !st.IsDir() {
// Re-open the path to match the O_DIRECTORY reopen loop later (so that we
// always return a non-O_PATH handle). We also check that we actually got a
// directory.
if reopenDir, err := Reopen(currentDir, unix.O_DIRECTORY|unix.O_CLOEXEC); errors.Is(err, unix.ENOTDIR) {
return nil, fmt.Errorf("cannot create subdirectories in %q: %w", currentDir.Name(), unix.ENOTDIR)
} else if err != nil {
return nil, fmt.Errorf("re-opening handle to %q: %w", currentDir.Name(), err)
} else {
currentDir = reopenDir
}

remainingParts := strings.Split(remainingPath, string(filepath.Separator))
Expand Down Expand Up @@ -135,15 +139,16 @@ func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err
return nil, err
}

// Get a handle to the next component.
// Get a handle to the next component. O_DIRECTORY means we don't need
// to use O_PATH.
var nextDir *os.File
if hasOpenat2() {
nextDir, err = openat2File(currentDir, part, &unix.OpenHow{
Flags: unix.O_PATH | unix.O_NOFOLLOW | unix.O_DIRECTORY | unix.O_CLOEXEC,
Flags: unix.O_NOFOLLOW | unix.O_DIRECTORY | unix.O_CLOEXEC,
Resolve: unix.RESOLVE_BENEATH | unix.RESOLVE_NO_SYMLINKS | unix.RESOLVE_NO_XDEV,
})
} else {
nextDir, err = openatFile(currentDir, part, unix.O_PATH|unix.O_NOFOLLOW|unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
nextDir, err = openatFile(currentDir, part, unix.O_NOFOLLOW|unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
}
if err != nil {
return nil, err
Expand All @@ -167,24 +172,18 @@ func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err
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)
}
// Re-open our O_PATH handle for the directory with O_RDONLY so we
// can check if it the directory is empty. This is safe to do with
// open(2) because there is no way for "." to be replaced or
// mounted over.
if dir, err := openatFile(currentDir, ".", unix.O_RDONLY|unix.O_NOFOLLOW|unix.O_DIRECTORY|unix.O_CLOEXEC, 0); err != nil {
return nil, fmt.Errorf("%w: re-open newly created directory %q: %w", errPossibleAttack, currentDir.Name(), err)
} else {
// We only need to check for a single entry, and we should get EOF
// if the directory is empty.
_, err := dir.Readdirnames(1)
_ = dir.Close()
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)
// 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)
}
}
return currentDir, nil
Expand Down
83 changes: 83 additions & 0 deletions open_linux.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
//go:build linux

// Copyright (C) 2024 SUSE LLC. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package securejoin

import (
"fmt"
"os"

"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.
func OpenatInRoot(root *os.File, unsafePath string) (*os.File, error) {
handle, remainingPath, err := partialLookupInRoot(root, unsafePath)
if err != nil {
return nil, err
}
if remainingPath != "" {
_ = handle.Close()
return nil, &os.PathError{Op: "securejoin.OpenInRoot", Path: unsafePath, Err: unix.ENOENT}
}
return handle, nil
}

// OpenInRoot safely opens the provided unsafePath within the root.
// Effectively, OpenInRoot(root, unsafePath) is equivalent to
//
// path, _ := securejoin.SecureJoin(root, unsafePath)
// 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
// 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
// limited set of operations will work on the handle. This is done to avoid
// 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.
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 {
return nil, err
}
defer rootDir.Close()
return OpenatInRoot(rootDir, unsafePath)
}

// 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())
// os.OpenFile(fdPath, flags|unix.O_CLOEXEC)
//
// But with some extra hardenings to ensure that we are not tricked by a
// 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.
func Reopen(handle *os.File, flags int) (*os.File, error) {
procRoot, err := getProcRoot()
if err != nil {
return nil, err
}

flags |= unix.O_CLOEXEC
fdPath := fmt.Sprintf("fd/%d", handle.Fd())
return doProcSelfMagiclink(procRoot, fdPath, func(procDirHandle *os.File, base string) (*os.File, error) {
// Rather than just wrapping openatFile, open-code it so we can copy
// handle.Name().
reopenFd, err := unix.Openat(int(procDirHandle.Fd()), base, flags, 0)
if err != nil {
return nil, fmt.Errorf("reopen fd %d: %w", handle.Fd(), err)
}
return os.NewFile(uintptr(reopenFd), handle.Name()), nil
})
}
Loading

0 comments on commit 6ae6d58

Please sign in to comment.