*: update and improve godocs

Signed-off-by: Aleksa Sarai <[email protected]>

Author: cyphar

doc.go

@@ -21,9 +21,8 @@
 // protections that are not provided by the legacy API. There are many more
 // operations that most programs expect to be able to do safely, but we do not
 // provide explicit support for them because we want to encourage users to
-// switch to [libpathrs](https://github.com/openSUSE/libpathrs) which is a
-// cross-language next-generation library that is entirely designed around
-// operating on paths safely.
+// switch to [libpathrs] which is a cross-language next-generation library that
+// is entirely designed around operating on paths safely.
 //
 // securejoin has been used by several container runtimes (Docker, runc,
 // Kubernetes, etc) for quite a few years as a de-facto standard for operating
@@ -33,9 +32,14 @@
 // API as soon as possible (or even better, switch to libpathrs).
 //
 // This project was initially intended to be included in the Go standard
-// library, but [it was rejected](https://go.dev/issue/20126). There is now a
-// [new Go proposal](https://go.dev/issue/67002) for a safe path resolution API
-// that shares some of the goals of filepath-securejoin. However, that design
-// is intended to work like `openat2(RESOLVE_BENEATH)` which does not fit the
-// usecase of container runtimes and most system tools.
+// library, but it was rejected (see https://go.dev/issue/20126). Much later,
+// [os.Root] was added to the Go stdlib that shares some of the goals of
+// filepath-securejoin. However, its design is intended to work like
+// openat2(RESOLVE_BENEATH) which does not fit the usecase of container
+// runtimes and most system tools.
+//
+// [libpathrs]: https://github.com/openSUSE/libpathrs
+// [OpenInRoot]: https://pkg.go.dev/github.com/cyphar/filepath-securejoin/pathrs-lite#OpenInRoot
+// [MkdirAll]: https://pkg.go.dev/github.com/cyphar/filepath-securejoin/pathrs-lite#MkdirAll
+// [os.Root]; https:///pkg.go.dev/os#Root
 package securejoin

join.go

@@ -51,12 +51,13 @@ func hasDotDot(path string) bool {
 	return strings.Contains("/"+path+"/", "/../")
 }
 
-// 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).
+// 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).
 //
 // 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

pathrs-lite/internal/fd/at_linux.go

@@ -40,7 +40,7 @@ func prepareAt(dir Fd, path string) (dirFd int, unsafeUnmaskedPath string) {
 	// NOTE: If path is "." or "", the returned path won't be filepath.Clean,
 	// but that's okay since this path is either used for errors (in which case
 	// a trailing "/" or "/." is important information) or will be
-	// filepath.Clean'd later (in the case of openatFile).
+	// filepath.Clean'd later (in the case of fd.Openat).
 	return dirFd, path
 }
 

pathrs-lite/internal/procfs/procfs_linux.go

@@ -213,18 +213,7 @@ var getCachedProcRoot = gocompat.SyncOnceValue(func() *Handle {
 	return procRoot
 })
 
-// OpenProcRoot tries to open a "safer" handle to "/proc" (i.e., one with the
-// "subset=pid" mount option applied, available from Linux 5.8). Unless you
-// plan to do many operations with [ProcRoot], users should prefer to use this
-// over [OpenUnsafeProcRoot] which is far more dangerous to keep open.
-//
-// If a safe handle cannot be opened, OpenProcRoot will fall back to opening a
-// regular "/proc" handle.
-//
-// Note that using [ProcRoot] will still work with handles returned by this
-// function. If a [ProcRoot] subpath cannot be operated on with a safe "/proc"
-// handle, then [OpenUnsafeProcRoot] will be called internally and a temporary
-// unsafe handle will be used.
+// OpenProcRoot tries to open a "safer" handle to "/proc".
 func OpenProcRoot() (*Handle, error) {
 	if proc := getCachedProcRoot(); proc != nil {
 		return proc, nil
@@ -233,16 +222,7 @@ func OpenProcRoot() (*Handle, error) {
 }
 
 // OpenUnsafeProcRoot opens a handle to "/proc" without any overmounts or
-// masked paths. You must be extremely careful to make sure this handle is
-// never leaked to a container and that you program cannot be tricked into
-// writing to arbitrary paths within it.
-//
-// This is not necessary if you just wish to use [ProcRoot], as handles
-// returned by [OpenProcRoot] will fall back to using a *temporary* unsafe
-// handle in that case. You should only really use this if you need to do many
-// operations on [ProcRoot] and the performance overhead of making many procfs
-// handles is an issue, and you should make sure to close the handle as soon as
-// possible to avoid known-fd-number attacks.
+// masked paths (but also without "subset=pid").
 func OpenUnsafeProcRoot() (*Handle, error) { return getProcRoot(false) }
 
 func getProcRoot(subset bool) (*Handle, error) {
@@ -328,7 +308,7 @@ func (base procfsBase) prefix(proc *Handle) (string, error) {
 // [os.File]: https://pkg.go.dev/os#File
 type ProcThreadSelfCloser func()
 
-// Open is the core lookup operation for [Handle]. It returns a handle to
+// open is the core lookup operation for [Handle]. It returns a handle to
 // "/proc/<base>/<subpath>". If the returned [ProcThreadSelfCloser] is non-nil,
 // you should call it after you are done interacting with the returned handle.
 //
@@ -395,25 +375,13 @@ func (proc *Handle) OpenThreadSelf(subpath string) (_ *os.File, _ ProcThreadSelf
 }
 
 // OpenSelf returns a handle to /proc/self/<subpath>.
-//
-// Note that in Go programs with non-homogenous threads, this may result in
-// spurious errors. If you are monkeying around with APIs that are
-// thread-specific, you probably want to use [ProcThreadSelf] instead which
-// will guarantee that the handle refers to the same thread as the caller is
-// executing on.
 func (proc *Handle) OpenSelf(subpath string) (*os.File, error) {
 	file, closer, err := proc.open(ProcSelf, subpath)
 	assert.Assert(closer == nil, "closer for ProcSelf must be nil")
 	return file, err
 }
 
 // OpenRoot returns a handle to /proc/<subpath>.
-//
-// You should only use this when you need to operate on global procfs files
-// (such as sysctls in /proc/sys). Unlike [OpenThreadSelf], [OpenSelf], and
-// [ProcPid], the procfs handle used internally for this operation will never
-// use subset=pids, which makes it a more juicy target for CVE-2024-21626-style
-// attacks.
 func (proc *Handle) OpenRoot(subpath string) (*os.File, error) {
 	file, closer, err := proc.open(ProcRoot, subpath)
 	assert.Assert(closer == nil, "closer for ProcRoot must be nil")
@@ -422,16 +390,6 @@ func (proc *Handle) OpenRoot(subpath string) (*os.File, error) {
 
 // OpenPid returns a handle to /proc/$pid/<subpath> (pid can be a pid or tid).
 // This is mainly intended for usage when operating on other processes.
-//
-// You should not use this for the current thread, as special handling is
-// needed for /proc/thread-self (or /proc/self/task/<tid>) when dealing with
-// goroutine scheduling -- use [OpenThreadSelf] instead.
-//
-// To refer to the current thread-group, you should use prefer [OpenSelf] to
-// passing os.Getpid as the pid argument.
-//
-// If you want to operate on the top-level /proc filesystem, you should use
-// [OpenRoot] instead.
 func (proc *Handle) OpenPid(pid int, subpath string) (*os.File, error) {
 	return proc.OpenRoot(strconv.Itoa(pid) + "/" + subpath)
 }

pathrs-lite/mkdir_linux.go

@@ -72,6 +72,8 @@ func toUnixMode(mode os.FileMode) (uint32, error) {
 // 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.
+//
+// [SecureJoin]: https://pkg.go.dev/github.com/cyphar/filepath-securejoin#SecureJoin
 func MkdirAllHandle(root *os.File, unsafePath string, mode os.FileMode) (_ *os.File, Err error) {
 	unixMode, err := toUnixMode(mode)
 	if err != nil {
@@ -226,6 +228,8 @@ func MkdirAllHandle(root *os.File, unsafePath string, mode os.FileMode) (_ *os.F
 // 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].
+//
+// [SecureJoin]: https://pkg.go.dev/github.com/cyphar/filepath-securejoin#SecureJoin
 func MkdirAll(root, unsafePath string, mode os.FileMode) error {
 	rootDir, err := os.OpenFile(root, unix.O_PATH|unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
 	if err != nil {

pathrs-lite/open_linux.go

@@ -47,6 +47,8 @@ func OpenatInRoot(root *os.File, unsafePath string) (*os.File, error) {
 // 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].
+//
+// [SecureJoin]: https://pkg.go.dev/github.com/cyphar/filepath-securejoin#SecureJoin
 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 {
@@ -101,7 +103,7 @@ func Reopen(handle *os.File, flags int) (*os.File, error) {
 	}
 
 	flags |= unix.O_CLOEXEC
-	// Rather than just wrapping openatFile, open-code it so we can copy
+	// Rather than just wrapping fd.Openat, open-code it so we can copy
 	// handle.Name().
 	reopenFd, err := unix.Openat(int(procFdDir.Fd()), fdStr, flags, 0)
 	if err != nil {

pathrs-lite/procfs/procfs_linux.go

@@ -37,14 +37,14 @@ type Handle struct {
 
 // OpenProcRoot tries to open a "safer" handle to "/proc" (i.e., one with the
 // "subset=pid" mount option applied, available from Linux 5.8). Unless you
-// plan to do many operations with [ProcRoot], users should prefer to use this
-// over [OpenUnsafeProcRoot] which is far more dangerous to keep open.
+// plan to do many [Handle.OpenRoot] operations, users should prefer to use
+// this over [OpenUnsafeProcRoot] which is far more dangerous to keep open.
 //
 // If a safe handle cannot be opened, OpenProcRoot will fall back to opening a
 // regular "/proc" handle.
 //
-// Note that using [ProcRoot] will still work with handles returned by this
-// function. If a [ProcRoot] subpath cannot be operated on with a safe "/proc"
+// Note that using [Handle.OpenRoot] will still work with handles returned by
+// this function. If a subpath cannot be operated on with a safe "/proc"
 // handle, then [OpenUnsafeProcRoot] will be called internally and a temporary
 // unsafe handle will be used.
 func OpenProcRoot() (*Handle, error) {
@@ -60,12 +60,13 @@ func OpenProcRoot() (*Handle, error) {
 // never leaked to a container and that you program cannot be tricked into
 // writing to arbitrary paths within it.
 //
-// This is not necessary if you just wish to use [ProcRoot], as handles
+// This is not necessary if you just wish to use [Handle.OpenRoot], as handles
 // returned by [OpenProcRoot] will fall back to using a *temporary* unsafe
 // handle in that case. You should only really use this if you need to do many
-// operations on [ProcRoot] and the performance overhead of making many procfs
-// handles is an issue, and you should make sure to close the handle as soon as
-// possible to avoid known-fd-number attacks.
+// operations with [Handle.OpenRoot] and the performance overhead of making
+// many procfs handles is an issue. If you do use OpenUnsafeProcRoot, you
+// should make sure to close the handle as soon as possible to avoid
+// known-fd-number attacks.
 func OpenUnsafeProcRoot() (*Handle, error) {
 	proc, err := procfs.OpenUnsafeProcRoot()
 	if err != nil {
@@ -77,8 +78,10 @@ func OpenUnsafeProcRoot() (*Handle, error) {
 // OpenThreadSelf returns a handle to "/proc/thread-self/<subpath>" (or an
 // equivalent handle on older kernels where "/proc/thread-self" doesn't exist).
 // Once finished with the handle, you must call the returned closer function
-// (runtime.UnlockOSThread). You must not pass the returned *os.File to other
+// ([runtime.UnlockOSThread]). You must not pass the returned *os.File to other
 // Go threads or use the handle after calling the closer.
+//
+// [runtime.UnlockOSThread]: https://pkg.go.dev/runtime#UnlockOSThread
 func (proc *Handle) OpenThreadSelf(subpath string) (*os.File, ProcThreadSelfCloser, error) {
 	return proc.inner.OpenThreadSelf(subpath)
 }
@@ -87,20 +90,24 @@ func (proc *Handle) OpenThreadSelf(subpath string) (*os.File, ProcThreadSelfClos
 //
 // Note that in Go programs with non-homogenous threads, this may result in
 // spurious errors. If you are monkeying around with APIs that are
-// thread-specific, you probably want to use [ProcThreadSelf] instead which
-// will guarantee that the handle refers to the same thread as the caller is
-// executing on.
+// thread-specific, you probably want to use [Handle.OpenThreadSelf] instead
+// which will guarantee that the handle refers to the same thread as the caller
+// is executing on.
 func (proc *Handle) OpenSelf(subpath string) (*os.File, error) {
 	return proc.inner.OpenSelf(subpath)
 }
 
 // OpenRoot returns a handle to /proc/<subpath>.
 //
 // You should only use this when you need to operate on global procfs files
-// (such as sysctls in /proc/sys). Unlike [OpenThreadSelf], [OpenSelf], and
-// [ProcPid], the procfs handle used internally for this operation will never
-// use subset=pids, which makes it a more juicy target for CVE-2024-21626-style
-// attacks.
+// (such as sysctls in /proc/sys). Unlike [Handle.OpenThreadSelf],
+// [Handle.OpenSelf], and [Handle.OpenPid], the procfs handle used internally
+// for this operation will never use "subset=pid", which makes it a more juicy
+// target for [CVE-2024-21626]-style attacks (and doing something like opening
+// a directory with OpenRoot effectively leaks [OpenUnsafeProcRoot] as long as
+// the file descriptor is open).
+//
+// [CVE-2024-21626]: https://github.com/opencontainers/runc/security/advisories/GHSA-xr7r-f8xq-vfvv
 func (proc *Handle) OpenRoot(subpath string) (*os.File, error) {
 	return proc.inner.OpenRoot(subpath)
 }
@@ -110,19 +117,35 @@ func (proc *Handle) OpenRoot(subpath string) (*os.File, error) {
 //
 // You should not use this for the current thread, as special handling is
 // needed for /proc/thread-self (or /proc/self/task/<tid>) when dealing with
-// goroutine scheduling -- use [OpenThreadSelf] instead.
-//
-// To refer to the current thread-group, you should use prefer [OpenSelf] to
-// passing os.Getpid as the pid argument.
+// goroutine scheduling -- use [Handle.OpenThreadSelf] instead.
 //
-// If you want to operate on the top-level /proc filesystem, you should use
-// [OpenRoot] instead.
+// To refer to the current thread-group, you should use prefer
+// [Handle.OpenSelf] to passing os.Getpid as the pid argument.
 func (proc *Handle) OpenPid(pid int, subpath string) (*os.File, error) {
 	return proc.inner.OpenPid(pid, subpath)
 }
 
 // ProcSelfFdReadlink gets the real path of the given file by looking at
-// readlink(/proc/thread-self/fd/$n).
+// /proc/self/fd/<fd> with [readlink]. It is effectively just shorthand for
+// something along the lines of:
+//
+//	proc, err := procfs.OpenProcRoot()
+//	if err != nil {
+//		return err
+//	}
+//	link, err := proc.OpenThreadSelf(fmt.Sprintf("fd/%d", f.Fd()))
+//	if err != nil {
+//		return err
+//	}
+//	defer link.Close()
+//	var buf [4096]byte
+//	n, err := unix.Readlinkat(int(link.Fd()), "", buf[:])
+//	if err != nil {
+//		return err
+//	}
+//	pathname := buf[:n]
+//
+// [readlink]: https://pkg.go.dev/golang.org/x/sys/unix#Readlinkat
 func ProcSelfFdReadlink(f *os.File) (string, error) {
 	return procfs.ProcSelfFdReadlink(f)
 }