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>
We recently switched VERSION to be read from env vars (#4270). This
broke several projects, as they were building runc and using a `VERSION`
env var for, e.g. the containerd version.
When fixing that in #4370, we discussed to consider doing the same for
these variables too
(https://github.com/opencontainers/runc/pull/4370#pullrequestreview-2240030944).
Let's stop reading them from env vars, as it is very easy to do it by
mistake (e.g. compile runc and define a COMMIT env var, not to override
the commit shown in `runc --version`) and users that want can still
override them if they want to. For example, with:
make EXTRA_BUILDTAGS=runc_nodmz
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
Add this new make variable so users can specify build information
without modifying the runc version nor the source code.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
The following commands are moved from `contrib/cmd` to `tests/cmd`:
- fs-idmap
- pidfd-kill
- recvtty
- remap-rootfs
- sd-helper
- seccompagent
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
This reverts commit 9d9273c926.
This commit broke the build for several other projects (see comments
here: https://github.com/opencontainers/runc/pull/4270, after the merge)
and we don't really need this to be able to set the version without
changing the file.
With this commit reverted, we can still run:
make VERSION="1.2.3"
and it just works. It doesn't take it from an env variable, but that is
what broke all the other projects (VERSION is just too generic as an env
var, especially for a project like runc that is embedded in many
others).
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
The warnings fixed were:
libcontainer/configs/config_test.go:205:12: printf: non-constant format string in call to (*testing.common).Errorf (govet)
t.Errorf(fmt.Sprintf("Expected error to not occur but it was %+v", err))
^
libcontainer/cgroups/fs/blkio_test.go:481:13: printf: non-constant format string in call to (*testing.common).Errorf (govet)
t.Errorf(fmt.Sprintf("test case '%s' failed unexpectedly: %s", testCase.desc, err))
^
libcontainer/cgroups/fs/blkio_test.go:595:13: printf: non-constant format string in call to (*testing.common).Errorf (govet)
t.Errorf(fmt.Sprintf("test case '%s' failed unexpectedly: %s", testCase.desc, err))
^
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
- drop Go 1.21;
- add Go 1.23;
- for a few jobs that were using Go 1.21, switch to 1.22;
Also, bump go to 1.22 in go.mod.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Rename current -> got, expected -> want.
2. check_cgroup_value: add file name to output.
3. Improve functions description.
This is mostly to simplify debugging test failures.
Example output before:
current 500000 !? 500
After:
cpu.max.burst: got 500000, want 500
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In all the three cases, we check that the program returned non-zero exit
code. This can be done in a much simpler manner.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The logic for how we create mountpoints is spread over each mountpoint
preparation function, when in reality the behaviour is pretty uniform
with only a handful of exceptions. So just move it all to one function
that is easier to understand.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Add a CI job to ensure go fix produces no result. Quoting
`go doc cmd/fix`:
> Fix finds Go programs that use old APIs and rewrites them to use newer
> ones. After you update to a new Go release, fix helps make the
> necessary changes to your programs.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Remove CentOS 7 as it is EOL.
Add back RHEL 8 clone (CentOS Stream 8 was removed by commit
40bb9c468e).
Switch from CentOS Stream 9 to Almalinux 9.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Remove changes that are already reflected in v1.1.13 changelog:
- rlimit_nofile fix;
- rt_period vs rt_runtime fix;
- gpg vs keyboxd fix;
- nsexec debug log fix;
- fips faking;
- vagrant Fedora 39 bump;
- golangci-lint bump;
- x/net bump;
- centos stream 8 removal;
- codespell ci fixes.
Compact some of the entries that are related (e.g. about actuated-ci).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Now that we dropped support for go < 1.21, we can use this; moving
the sync.once out of the runningInUserNS() implementation would also
allow for it to be more easily tested if we'd decide to.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The fuzzer for this only runs on Linux; rename the file to be Linux-only
so that we don't have to stub out the uidMapInUserNS function.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This was a poor decision on my side; 4316df8b53
moved this utility to a separate package, and split the exported function
from the implementation (and stubs). Out of convenience, I used an alias
for the latter part, but there's two downsides to that;
- `RunningInUserNS` being an exported var means that (technically) it can
be replaced by other code; perhaps that's a "feature", but not one we
intended it to be used for.
- `RunningInUserNS` being implemented through a var / alias means it's
also documented as such on [pkg.go.dev], which is confusing.
This patch changes it to a regular function, acting as a wrapper for
the underlying implementations. While at it, also slightly touching
up the GoDoc to describe its functionality / behavior.
[pkg.go.dev]: https://pkg.go.dev/github.com/opencontainers/runc@v1.1.13/libcontainer/userns#RunningInUserNS
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit 4316df8b53 isolated RunningInUserNS
to a separate package to make it easier to consume without bringing in
additional dependencies, and with the potential to move it separate in
a similar fashion as libcontainer/user was moved to a separate module
in commit ca32014adb. While RunningInUserNS
is fairly trivial to implement, it (or variants of this utility) is used
in many codebases, and moving to a separate module could consolidate
those implementations, as well as making it easier to consume without
large dependency trees (when being a package as part of a larger code
base).
Commit 1912d5988b and follow-ups introduced
cgo code into the userns package, and code introduced in those commits
are not intended for external use, therefore complicating the potential
of moving the userns package separate.
This commit moves the new code to a separate package; some of this code
was included in v1.1.11 and up, but I could not find external consumers
of `GetUserNamespaceMappings` and `IsSameMapping`. The `Mapping` and
`Handles` types (added in ba0b5e2698) only
exist in main and in non-stable releases (v1.2.0-rc.x), so don't need
an alias / deprecation.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
lifubang