When preparing to mount container root, we need to make its parent mount
private (i.e. disable propagation), otherwise the new in-container
mounts are leaked to the host.
To find a parent mount, we use to read mountinfo and find the longest
entry which can be a parent of the container root directory.
Unfortunately, due to kernel bug in all Linux kernels older than v5.8
(see [1], [2]), sometimes mountinfo can't be read in its entirety. In
this case, getParentMount may occasionally return a wrong parent mount.
As a result, we do not change the mount propagation to private, and
container mounts are leaked.
Alas, we can not fix the kernel, and reading mountinfo a few times to
ensure its consistency (like it's done in, say, Kubernetes) does not
look like a good solution for performance reasons.
Fortunately, we don't need mountinfo. Let's just traverse the directory
tree, trying to remount it private until we find a mount point (any
error other than EINVAL means we just found it).
Fixes issue 2404.
[1]: https://github.com/kolyshkin/procfs-test
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9f6c61f96f2d97cbb5f
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Initially, this was a commit to switch from strings.Fields to
strings.SplitN in getCpuUsageBreakdown, since strings.Fields
was probably slower than strings.SplitN in some old Go versions.
Afterwards, strings.Cut was also considered for potential
speed improvements.
After writing a benchmark test, we learned that:
- strings.Fields performance is now adequate;
- strings.SplitN is slower than strings.Fields;
- strings.Cut had <5% performance gain from strings.Fields;
So, remove the TODO and keep the benchmark test.
Signed-off-by: Stavros Panakakis <stavrospanakakis@gmail.com>
The example is too long since it lists too many capabilities.
Simplify it, leaving only two capabilities.
Also, remove ambient capabilities from the set. Inheritable capabilities
were removed earlier by commit 98fe566c, but ambient capabilities can't
be raised without inheritable ones.
Fixes: 98fe566c
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 98fe566c removed inheritable capabilities from the example spec
(used by runc spec) and from the libcontainer/integration test config,
but neglected to also remove ambient capabilities.
An ambient capability could only be set if the same inheritable
capability is set, so as a result of the above change ambient
capabilities were not set (but due to a bug in gocapability package,
those errors are never reported).
Once we start using a library with the fix [1], that bug will become
apparent (both bats-based and libct/int tests will fail).
[1]: https://github.com/kolyshkin/capability/pull/3
Fixes: 98fe566c ("runc: do not set inheritable capabilities")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 98fe566c removed setting inheritable capabilities from runc exec
--cap, but neglected to also remove ambient capabilities.
An ambient capability could only be set if the same inheritable
capability is set, so as a result of the above change ambient
capabilities were not set (but due to a bug in gocapability package,
those errors are never reported).
Once we start using a library with the fix [1], that bug will become
apparent. Alas, we do not have any tests for runc exec --cap, so add
one.
Yet, if some inheritable bits are already set from spec, let's set
ambient to avoid a possible regression. Add a test case for that, too.
[1]: https://github.com/kolyshkin/capability/pull/3
Fixes: 98fe566c ("runc: do not set inheritable capabilities")
Co-authored-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
bats-core/bats-action installs a few support libraries by default which are not used by
runc. Disable the installation, which will remove /usr/bin/tar: Permission denied errors.
Signed-off-by: Akhil Mohan <akhilerm@gmail.com>
For some rootless container, runc has no access to cgroup,
But the container is still running. So we should return the
`ErrNotRunning` and `ErrCgroupNotExist` error seperatlly.
Signed-off-by: lifubang <lifubang@acmcoder.com>
Go since 1.21 allows to set a "toolchain" that specifies the minimum Go
toolchain to use when working in runc. In contrast to the go line,
toolchain does not impose a requirement on other modules[1][2].
As documented in the 1.2.0-rc.1 release notes, 1.22.4 is needed for the
nsenter package. Let's suggest this with the toolchain version.
[1]: https://go.dev/doc/toolchain
[2]: https://go.dev/blog/toolchain
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
mig4/setup-bats is now unmaintained(last commit in Sep 2021).
bats-core/bats-action can be used as a replacement maintained
by the bats-core team.
Signed-off-by: Akhil Mohan <akhilerm@gmail.com>
Shared pid namespace means `runc kill` (or `runc delete -f`) have to
kill all container processes, not just init. To do so, it needs a cgroup
to read the PIDs from.
If there is no cgroup, processes will be leaked, and so such
configuration is bad and should not be allowed. To keep backward
compatibility, though, let's merely warn about this for now.
Alas, the only way to know if cgroup access is available is by returning
an error from Manager.Apply. Amend fs cgroup managers to do so (systemd
doesn't need it, since v1 can't work with rootless, and cgroup v2 does
not have a special rootless case).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This aids in failed test analysis by allowing to distinguish the output
of various commands being run as part of the test case from the output
of teardown command like runc delete.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In these cases, this is exactly what we want to find out.
Slightly improves performance and readability.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It turns out that the suid and sgid mode bits are silently ignored by
Linux (though the sticky bit is honoured), and some users are requesting
mode bits that are ignored. While returning an error (as securejoin
does) makes some sense, this is a regression.
Ref: https://github.com/cyphar/filepath-securejoin/issues/23
Fixes: dd827f7b71 ("utils: switch to securejoin.MkdirAllHandle")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
We reintroduced this once already because it is quite easy to miss this
subtle aspect of proc mounting. The recent migration to
securejoin.MkdirAllInRoot could have also inadvertently reintroduced
this (though it didn't).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
It is available since Go 1.21 and is defined during compile time
(i.e. based on GOARCH during build).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
`signalAllProcesses()` depends on the cgroup and is expected to fail
when runc is running in rootless without an access to the cgroup.
When `RootlessCgroups` is set to `true`, runc just ignores the error
from `signalAllProcesses` and may leak some processes running.
(See the comments in PR 4395)
In the future, runc should walk the process tree to avoid such a leak.
Note that `RootlessCgroups` is a misnomer; it is set to `false` despite
the name when cgroup v2 delegation is configured.
This is expected to be renamed in a separate commit.
Fix issue 4394
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
filepath-securejoin has a bunch of extra hardening features and is very
well-tested, so we should use it instead of our own homebrew solution.
A lot of rootfs_linux.go callers pass a SecureJoin'd path, which means
we need to keep the wrapper helpers in utils, but at least the core
logic is no longer in runc. In future we will want to remove this dodgy
logic and just use file handles for everything (using libpathrs,
ideally).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
While we use SecureJoin to try to make all of our target paths inside
the container safe, SecureJoin is not safe against an attacker than can
change the path after we "resolve" it.
os.MkdirAll can inadvertently follow symlinks and thus an attacker could
end up tricking runc into creating empty directories on the host (note
that the container doesn't get access to these directories, and the host
just sees empty directories). However, this could potentially cause DoS
issues by (for instance) creating a directory in a conf.d directory for
a daemon that doesn't handle subdirectories properly.
In addition, the handling for creating file bind-mounts did a plain
open(O_CREAT) on the SecureJoin'd path, which is even more obviously
unsafe (luckily we didn't use O_TRUNC, or this bug could've allowed an
attacker to cause data loss...). Regardless of the symlink issue,
opening an untrusted file could result in a DoS if the file is a hung
tty or some other "nasty" file. We can use mknodat to safely create a
regular file without opening anything anyway (O_CREAT|O_EXCL would also
work but it makes the logic a bit more complicated, and we don't want to
open the file for any particular reason anyway).
libpathrs[1] is the long-term solution for these kinds of problems, but
for now we can patch this particular issue by creating a more restricted
MkdirAll that refuses to resolve symlinks and does the creation using
file descriptors. This is loosely based on a more secure version that
filepath-securejoin now has[2] and will be added to libpathrs soon[3].
[1]: https://github.com/openSUSE/libpathrs
[2]: https://github.com/cyphar/filepath-securejoin/releases/tag/v0.3.0
[3]: https://github.com/openSUSE/libpathrs/issues/10
Fixes: CVE-2024-45310
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Kir Kolyshkin