Commit Graph

17 Commits

Author SHA1 Message Date
Sebastiaan van Stijn 9b60a93cf3 libcontainer/userns: migrate to github.com/moby/sys/userns
The userns package was moved to the moby/sys/userns module
at commit 3778ae603c.

This patch deprecates the old location, and adds it as an alias
for the moby/sys/userns package.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-09 22:20:25 +08:00
Aleksa Sarai 81d63f265a merge #4331 into opencontainers/runc:main
Sebastiaan van Stijn (1):
  libct/userns: split userns detection from internal userns code

LGTMs: kolyshkin cyphar
2024-07-03 14:24:05 +10:00
Sebastiaan van Stijn 6980adb67c libct/userns: implement RunningInUserNS with sync.OnceValue
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>
2024-07-01 19:20:58 +02:00
Sebastiaan van Stijn b3b31ff27c libct/userns: make fuzzer Linux-only, and remove stub for uidMapInUserNS
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>
2024-07-01 19:20:58 +02:00
Sebastiaan van Stijn 5b09a71228 libct/userns: change RunningInUserNS to a wrapper instead of an alias
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>
2024-07-01 19:20:35 +02:00
Sebastiaan van Stijn 30b530ca94 libct/userns: split userns detection from internal userns code
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>
2024-06-30 20:06:30 +02:00
Sebastiaan van Stijn c14213399a remove pre-go1.17 build-tags
Removed pre-go1.17 build-tags with go fix;

    go fix -mod=readonly ./...

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-06-29 15:45:25 +02:00
lfbzhm d08ba9ca89 fix a (u|g)IDMappings type value convertion error
Signed-off-by: lfbzhm <lifubang@acmcoder.com>
2023-12-18 05:36:10 +00:00
lfbzhm 371ff9c5e7 Merge pull request #3985 from cyphar/idmap-generic
libcontainer: remove all mount logic from nsexec
2023-12-18 13:10:45 +08:00
Aleksa Sarai 8e8b136c49 tree-wide: use /proc/thread-self for thread-local state
With the idmap work, we will have a tainted Go thread in our
thread-group that has a different mount namespace to the other threads.
It seems that (due to some bad luck) the Go scheduler tends to make this
thread the thread-group leader in our tests, which results in very
baffling failures where /proc/self/mountinfo produces gibberish results.

In order to avoid this, switch to using /proc/thread-self for everything
that is thread-local. This primarily includes switching all file
descriptor paths (CLONE_FS), all of the places that check the current
cgroup (technically we never will run a single runc thread in a separate
cgroup, but better to be safe than sorry), and the aforementioned
mountinfo code. We don't need to do anything for the following because
the results we need aren't thread-local:

 * Checks that certain namespaces are supported by stat(2)ing
   /proc/self/ns/...

 * /proc/self/exe and /proc/self/cmdline are not thread-local.

 * While threads can be in different cgroups, we do not do this for the
   runc binary (or libcontainer) and thus we do not need to switch to
   the thread-local version of /proc/self/cgroups.

 * All of the CLONE_NEWUSER files are not thread-local because you
   cannot set the usernamespace of a single thread (setns(CLONE_NEWUSER)
   is blocked for multi-threaded programs).

Note that we have to use runtime.LockOSThread when we have an open
handle to a tid-specific procfs file that we are operating on multiple
times. Go can reschedule us such that we are running on a different
thread and then kill the original thread (causing -ENOENT or similarly
confusing errors). This is not strictly necessary for most usages of
/proc/thread-self (such as using /proc/thread-self/fd/$n directly) since
only operating on the actual inodes associated with the tid requires
this locking, but because of the pre-3.17 fallback for CentOS, we have
to do this in most cases.

In addition, CentOS's kernel is too old for /proc/thread-self, which
requires us to emulate it -- however in rootfs_linux.go, we are in the
container pid namespace but /proc is the host's procfs. This leads to
the incredibly frustrating situation where there is no way (on pre-4.1
Linux) to figure out which /proc/self/task/... entry refers to the
current tid. We can just use /proc/self in this case.

Yes this is all pretty ugly. I also wish it wasn't necessary.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2023-12-14 11:36:41 +11:00
Aleksa Sarai ba0b5e2698 libcontainer: remove all mount logic from nsexec
With open_tree(OPEN_TREE_CLONE), it is possible to implement both the
id-mapped mounts and bind-mount source file descriptor logic entirely in
Go without requiring any complicated handling from nsexec.

However, implementing it the naive way (do the OPEN_TREE_CLONE in the
host namespace before the rootfs is set up -- which is what the existing
implementation did) exposes issues in how mount ordering (in particular
when handling mount sources from inside the container rootfs, but also
in relation to mount propagation) was handled for idmapped mounts and
bind-mount sources. In order to solve this problem completely, it is
necessary to spawn a thread which joins the container mount namespace
and provides mountfds when requested by the rootfs setup code (ensuring
that the mount order and mount propagation of the source of the
bind-mount are handled correctly). While the need to join the mount
namespace leads to other complicated (such as with the usage of
/proc/self -- fixed in a later patch) the resulting code is still
reasonable and is the only real way to solve the issue.

This allows us to reduce the amount of C code we have in nsexec, as well
as simplifying a whole host of places that were made more complicated
with the addition of id-mapped mounts and the bind sourcefd logic.
Because we join the container namespace, we can continue to use regular
O_PATH file descriptors for non-id-mapped bind-mount sources (which
means we don't have to raise the kernel requirement for that case).

In addition, we can easily add support for id-mappings that don't match
the container's user namespace. The approach taken here is to use Go's
officially supported mechanism for spawning a process in a user
namespace, but (ab)use PTRACE_TRACEME to avoid actually having to exec a
different process. The most efficient way to implement this would be to
do clone() in cgo directly to run a function that just does
kill(getpid(), SIGSTOP) -- we can always switch to that if it turns out
this approach is too slow. It should be noted that the included
micro-benchmark seems to indicate this is Fast Enough(TM):

  goos: linux
  goarch: amd64
  pkg: github.com/opencontainers/runc/libcontainer/userns
  cpu: Intel(R) Core(TM) i5-10210U CPU @ 1.60GHz
  BenchmarkSpawnProc
  BenchmarkSpawnProc-8        1670            770065 ns/op

Fixes: fda12ab101 ("Support idmap mounts on volumes")
Fixes: 9c444070ec ("Open bind mount sources from the host userns")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2023-12-14 11:36:40 +11:00
Aleksa Sarai ebcef3e651 specconv: temporarily allow userns path and mapping if they match
It turns out that the error added in commit 09822c3da8 ("configs:
disallow ambiguous userns and timens configurations") causes issues with
containerd and CRIO because they pass both userns mappings and a userns
path.

These configurations are broken, but to avoid the regression in this one
case, output a warning to tell the user that the configuration is
incorrect but we will continue to use it if and only if the configured
mappings are identical to the mappings of the provided namespace.

Fixes: 09822c3da8 ("configs: disallow ambiguous userns and timens configurations")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2023-12-10 20:49:43 +11:00
Aleksa Sarai 1912d5988b *: actually support joining a userns with a new container
Our handling for name space paths with user namespaces has been broken
for a long time. In particular, the need to parse /proc/self/*id_map in
quite a few places meant that we would treat userns configurations that
had a namespace path as if they were a userns configuration without
mappings, resulting in errors.

The primary issue was down to the id translation helper functions, which
could only handle configurations that had explicit mappings. Obviously,
when joining a user namespace we need to map the ids but figuring out
the correct mapping is non-trivial in comparison.

In order to get the mapping, you need to read /proc/<pid>/*id_map of a
process inside the userns -- while most userns paths will be of the form
/proc/<pid>/ns/user (and we have a fast-path for this case), this is not
guaranteed and thus it is necessary to spawn a process inside the
container and read its /proc/<pid>/*id_map files in the general case.

As Go does not allow us spawn a subprocess into a target userns,
we have to use CGo to fork a sub-process which does the setns(2). To be
honest, this is a little dodgy in regards to POSIX signal-safety(7) but
since we do no allocations and we are executing in the forked context
from a Go program (not a C program), it should be okay. The other
alternative would be to do an expensive re-exec (a-la nsexec which would
make several other bits of runc more complicated), or to use nsenter(1)
which might not exist on the system and is less than ideal.

Because we need to logically remap users quite a few times in runc
(including in "runc init", where joining the namespace is not feasable),
we cache the mapping inside the libcontainer config struct. A future
patch will make sure that we stop allow invalid user configurations
where a mapping is specified as well as a userns path to join.

Finally, add an integration test to make sure we don't regress this again.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2023-12-05 17:46:08 +11:00
Sebastiaan van Stijn d8e9ed3e08 libcontainer/userns: simplify, and separate from "user" package.
This makes libcontainer/userns self-dependent, largely returning to
the original implementation from lxc. The `uiMapInUserNS` is kept as
a separate function for unit-testing and fuzzing.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-09-04 10:57:48 +02:00
Kir Kolyshkin d8da00355e *: add go-1.17+ go:build tags
Go 1.17 introduce this new (and better) way to specify build tags.
For more info, see https://golang.org/design/draft-gobuild.

As a way to seamlessly switch from old to new build tags, gofmt (and
gopls) from go 1.17 adds the new tags along with the old ones.

Later, when go < 1.17 is no longer supported, the old build tags
can be removed.

Now, as I started to use latest gopls (v0.7.1), it adds these tags
while I edit. Rather than to randomly add new build tags, I guess
it is better to do it once for all files.

Mind that previous commits removed some tags that were useless,
so this one only touches packages that can at least be built
on non-linux.

Brought to you by

        go1.17 fmt ./...

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-08-30 20:58:22 -07:00
Kir Kolyshkin 9ff64c3d97 *: rm redundant linux build tag
For files that end with _linux.go or _linux_test.go, there is no need to
specify linux build tag, as it is assumed from the file name.

In addition, rename libcontainer/notify_linux_v2.go -> libcontainer/notify_v2_linux.go
for the file name to make sense.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-08-30 20:15:00 -07:00
Sebastiaan van Stijn 4316df8b53 libcontainer/system: move userns utilities to separate package
Moving these utilities to a separate package, so that consumers of this
package don't have to pull in the whole "system" package.

Looking at uses of these utilities (outside of runc itself);

`RunningInUserNS()` is used by [various external consumers][1],
so adding a "Deprecated" alias for this.

[1]: https://grep.app/search?current=2&q=.RunningInUserNS

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2021-04-04 22:42:03 +02:00