Commit Graph

13 Commits

Author SHA1 Message Date
Aleksa Sarai 864db8042d rootfs: make /dev initialisation code fd-based
These codepaths are very old and operate on pure paths but before
pivot_root(2), meaning that a bad image with a malicious /dev symlink
could cause us to operate on host paths instead.

In practice this means that we could be tricked into removing a file
called "ptmx" (note that /dev/pts/ptmx and /dev/ptmx are both immune for
different reasons) or creating a very restricted set of symlinks (with
fixed targets and names). The scope of these bugs is thus quite limited,
but we definitely need to harden against it.

These codepaths were unfortunately missed during the fd-based rework in
commit d40b3439a9 ("rootfs: switch to fd-based handling of mountpoint
targets") -- I must've assumed they were called after pivot_root(2)...

Fixes: GHSA-xjvp-4fhw-gc47
Fixes: CVE-2026-41579
Fixes: d40b3439a9 ("rootfs: switch to fd-based handling of mountpoint targets")
Signed-off-by: Aleksa Sarai <aleksa@amutable.com>
2026-06-12 18:12:37 +02:00
Kir Kolyshkin 28cb321887 Pre-open container root directory
A lot of filesystem-related stuff happens inside the container root
directory, and we have used its name before. It makes sense to pre-open
it and use a *os.File handle instead.

Function names in internal/pathrs are kept as is for simplicity (and it
is an internal package), but they now accept root as *os.File.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2026-03-29 12:02:36 -07:00
Aleksa Sarai 195e9551e4 pathrs: add MkdirAllParentInRoot helper
While CreateInRoot supports hallucinating the target path, we do not use
it directly when constructing device inode targets because we need to
have different handling for mknod and bind-mounts.

The solution is to simply have a more generic MkdirAllParentInRoot
helper that MkdirAll's the parent directory of the target path and then
allows the caller to create the trailing component however they like.
(This can be used by CreateInRoot internally as well!)

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2025-11-26 21:04:05 +11:00
Aleksa Sarai cfb74326be pathrs: add "hallucination" helpers for SecureJoin magic
In order to maintain compatibility with previous releases of runc (which
permitted dangling symlinks as path components by permitting
non-existent path components to be treated like real directories) we
have to first do SecureJoin to construct a target path that is
compatible with the old behaviour but has all dangling symlinks (or
other invalid paths like ".." components after non-existent directories)
removed.

This is effectively a more generic verison of commit 3f925525b4
("rootfs: re-allow dangling symlinks in mount targets") and will let us
remove the need for open-coding SecureJoin workarounds.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2025-11-26 21:04:05 +11:00
Aleksa Sarai 20c5a8ec4a pathrs: rename MkdirAllInRootOpen -> MkdirAllInRoot
Now that MkdirAllInRoot has been removed, we can make MkdirAllInRootOpen
less wordy by renaming it to MkdirAllInRoot. This is a non-functional
change.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2025-11-26 21:04:04 +11:00
Aleksa Sarai 9dbd37e06f libct: switch final WithProcfd users to WithProcfdFile
This probably should've been done as part of commit d40b3439a9
("rootfs: switch to fd-based handling of mountpoint targets") but it
seems I missed them when doing the rest of the conversions.

This also lets us remove utils.WithProcfd entirely, as well as
pathrs.MkdirAllInRoot. Unfortunately, WithProcfd was exposed in the
externally-importable "libcontainer/utils" package and so we need to
have a deprecation notice to remove it in runc 1.5.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2025-11-26 21:03:30 +11:00
Aleksa Sarai 42a1e19d67 libcontainer: move CleanPath and StripRoot to internal/pathrs
These helpers will be needed for the compatibility code added in future
patches in this series, but because "internal/pathrs" is imported by
"libcontainer/utils" we need to move them so that we can avoid circular
dependencies.

Because the old functions were in a non-internal package it is possible
some downstreams use them, so add some wrappers but mark them as
deprecated.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2025-11-26 21:03:29 +11:00
Aleksa Sarai a41366e740 openat2: improve resilience on busy systems
Previously, we would see a ~3% failure rate when starting containers
with mounts that contain ".." (which can trigger -EAGAIN). To counteract
this, filepath-securejoin v0.5.1 includes a bump of the internal retry
limit from 32 to 128, which lowers the failure rate to 0.12%.

However, there is still a risk of spurious failure on regular systems.
In order to try to provide more resilience (while avoiding DoS attacks),
this patch also includes an additional retry loop that terminates based
on a deadline rather than retry count. The deadline is 2ms, as my
testing found that ~800us for a single pathrs operation was the longest
latency due to -EAGAIN retries, and that was an outlier compared to the
more common ~400us latencies -- so 2ms should be more than enough for
any real system.

The failure rates above were based on more 50k runs of runc with an
attack script (from libpathrs) running a rename attack on all cores of a
16-core system, which is arguably a worst-case but heavily utilised
servers could likely approach similar results.

Tested-by: Phil Estes <estesp@gmail.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2025-11-05 18:57:51 +11:00
Aleksa Sarai d40b3439a9 rootfs: switch to fd-based handling of mountpoint targets
An attacker could race with us during mount configuration in order to
trick us into mounting over an unexpected path. This would bypass
checkProcMount() and would allow for security profiles to be left
unapplied by mounting over /proc/self/attr/... (or even more serious
outcomes such as killing the entire system by tricking runc into writing
strings to /proc/sysrq-trigger).

This is a larger issue with our current mount infrastructure, and the
ideal solution would be to rewrite it all to be fd-based (which would
also allow us to support the "new" mount API, which also avoids a bunch
of other issues with mount(8)). However, such a rewrite is not really
workable as a security fix, so this patch is a bit of a compromise
approach to fix the issue while also moving us a bit towards that
eventual end-goal.

The core issue in CVE-2025-52881 is that we currently use the (insecure)
SecureJoin to re-resolve mountpoint target paths multiple times during
mounting. Rather than generating a string from createMountpoint(), we
instead open an *os.File handle to the target mountpoint directly and
then operate on that handle. This will make it easier to remove
utils.WithProcfd() and rework mountViaFds() in the future.

The only real issue we need to work around is that we need to re-open
the mount target after doing the mount in order to get a handle to the
mountpoint -- pathrs.Reopen() doesn't work in this case (it just
re-opens the inode under the mountpoint) so we need to do a naive
re-open using the full path. Note that if we used move_mount(2) this
wouldn't be a problem because we would have a handle to the mountpoint
itself.

Note that this is still somewhat of a temporary solution -- ideally
mountViaFds would use *os.File directly to let us avoid some other
issues with using bare /proc/... paths, as well as also letting us more
easily use the new mount API on modern kernels.

Fixes: GHSA-cgrx-mc8f-2prm CVE-2025-52881
Co-developed-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2025-11-01 21:24:06 +11:00
Aleksa Sarai 77889b56db internal: add wrappers for securejoin.Proc*
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2025-11-01 21:24:04 +11:00
Aleksa Sarai 44a0fcf685 go.mod: update to github.com/cyphar/filepath-securejoin@v0.5.0
In order to avoid lint errors due to the deprecation of the top-level
securejoin methods ported from libpathrs, we need to adjust
internal/pathrs to use the new pathrs-lite subpackage instead.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2025-11-01 21:24:03 +11:00
Aleksa Sarai ff94f9991b *: switch to safer securejoin.Reopen
filepath-securejoin v0.3 gave us a much safer re-open primitive, we
should use it to avoid any theoretical attacks. Rather than using it
direcly, add a small pathrs wrapper to make libpathrs migrations in the
future easier...

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2025-11-01 21:24:02 +11:00
Aleksa Sarai 6fc1914491 internal: move utils.MkdirAllInRoot to internal/pathrs
We will have more wrappers around filepath-securejoin, and so move them
to their own specific package so that we can eventually use libpathrs
fairly cleanly (by swapping out the implementation).

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2025-11-01 21:24:02 +11:00