Commit 2bab4a5 resulted in a warning from gcc:
nsexec.c: In function ‘write_log’:
nsexec.c:171:2: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
171 | write(logfd, json, ret);
| ^~~~~~~~~~~~~~~~~~~~~~~
As there's nothing we can or want to do in case write fails,
let's just tell the compiler we're not going to use it.
Fixes: 2bab4a5
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit db8330c9e5)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As reported in issue 3119, there is a race in nsexec logging
that can lead to garbled json received by log forwarder, which
complains about it with a "failed to decode" error.
This happens because dprintf (used since the very beginning of nsexec
logging introduced in commit ba3cabf932) relies on multiple write(2)
calls, and with additional logging added by 64bb59f592 a race is
possible between runc init parent and its children.
The fix is to prepare a string and write it using a single call to
write(2).
[v2: NULLify json on error from asprintf]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 2bab4a56f1)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
We shouldn't refuse to build on architectures just because we don't know
what the syscall number of memfd_create(2) is. In addition, use the
correct defined(...) macros for ppc64 (these are the ones glibc uses).
Fixes: 3aead32ea2 ("nsenter: hard-code memfd_create(2) syscall numbers")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Since the previous commit, some strings logged by write_log() contain a
literal newline, which leads to errors like this one:
> # time="2020-06-07T15:41:37Z" level=error msg="failed to decode \"{\\\"level\\\":\\\"debug\\\", \\\"msg\\\": \\\"nsexec-0[2265]: update /proc/2266/uid_map to '0 1000 1\\n\" to json: invalid character '\\n' in string literal"
The fix is to escape such characters.
Add a simple (as much as it can be) routine which implements JSON string
escaping as required by RFC4627, section 2.5, plus escaping of DEL (0x7f)
character (not required, but allowed by the standard, and usually done
by tools such as jq).
As much as I hate to code something like this, I was not able to find
a ready to consume and decent C implementation (not using glib).
Added a test case (and some additional asserts in C code, conditionally
enabled by the test case) to make sure the implementation is correct.
The test case have to live in a separate directory so we can use
different C flags to compile the test, and use C from go test.
[v2: try to simplify the code, add more tests]
[v3: don't do exit(1), try returning an error instead]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In order to make 'runc --debug' actually useful for debugging nsexec
bugs, provide information about all the internal operations when in
debug mode.
[@kolyshkin: rebasing; fix formatting via indent for make validate to pass]
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The stars can be aligned in a way that results in runc to leave a stale
bind mount in container's state directory, which manifests itself later,
while trying to remove the container, in an error like this:
> remove /run/runc/test2: unlinkat /run/runc/test2/runc.W24K2t: device or resource busy
The stale mount happens because runc start/run/exec kills runc init
while it is inside ensure_cloned_binary(). One such scenario is when
a unified cgroup resource is specified for cgroup v1, a cgroup manager's
Apply returns an error (as of commit b006f4a180), and when
(*initProcess).start() kills runc init just after it was started.
One solution is NOT to kill runc init too early. To achieve that,
amend the libcontainer/nsenter code to send a \0 byte to signal
that it is past the initial setup, and make start() (for both
run/start and exec) wait for this byte before proceeding with
kill on an error path.
While at it, improve some error messages.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Apparently, scripts/validate-c is not working in CI (or maybe
maintainers ignored the failures from it) -- current C code
gets some changes if we run indent on it.
This commit fixes this, simplifying things along the way.
In particular:
1. Remove "validate" make target, add "cfmt" target that just runs
indent on all *.c files in the repository (NOTE that *.h files
are not included, as before).
This may help a contributor to fix their code -- they just need
to run "make cfmt" now instead of running "make validate" and
copy-pasting the indent command and options from the hint.
2. Split GHA validate/misc into validate/release and validate/cfmt.
The latter checks that the sources are not changed after "make cfmt".
3. Adds a few more options to indent. This was mostly motivated by
trying to save the existing formatting, minimizing the amount of
changes indent produces.
The new options are:
* -il0: sets the offset for goto labels to 0 (currently all labels
but one are not indented -- let's keep it that way);
* -ppi2: sets the indentation for nested preprocessor directives
to 2 spaces (same as it is done in "SYS_memfd_create" defines);
* -cp1: sets the indentation between #else / #endif and the
following comment to 1 space.
4. Reformat the code using the new indent options.
5. Remove the now-unused script/{.validate,validate-c}.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Remove comments with architectures when defining SYS_memfd_create,
as they are redundant, and indent has a funny way of indenting them.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This block apparently does nothing except for creating
a need for additional indentation. Remove it.
While at it, break a long line in this code.
No functional change.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Some libc versions still in use by distributions (such as SLE) do not
define SYS_memfd_create even though the kernel supports the feature.
Since the syscall numbers are fixed, we can just hard-code them if
__NR_memfd_create is not defined.
We only do this for a handful of architectures, since containers aren't
widely supported on every possible Linux architecture.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
It is obvious that the loop at the first place executes at least
twice, and the close() call after the first time always returns
an EBADF error, so move these operations outside the loop that
do not need to be repeated.
Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Refactor configuring logging into a reusable component
so that it can be nicely used in both main() and init process init()
Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
Co-authored-by: Giuseppe Capizzi <gcapizzi@pivotal.io>
Co-authored-by: Claudia Beresford <cberesford@pivotal.io>
Signed-off-by: Danail Branekov <danailster@gmail.com>
Add support for children processes logging (including nsexec).
A pipe is used to send logs from children to parent in JSON.
The JSON format used is the same used by logrus JSON formatted,
i.e. children process can use standard logrus APIs.
Signed-off-by: Marco Vedovati <mvedovati@suse.com>
secure_getenv is a Glibc extension and so this code does not compile
on Musl libc any more after this patch.
secure_getenv is only intended to be used in setuid binaries, in
order that they should not trust their environment. It simply returns
NULL if the binary is running setuid. If runc was installed setuid,
the user can already do anything as root, so it is game over, so this
check is not needed.
Signed-off-by: Justin Cormack <justin.cormack@docker.com>
There are some circumstances where sendfile(2) can fail (one example is
that AppArmor appears to block writing to deleted files with sendfile(2)
under some circumstances) and so we need to have a userspace fallback.
It's fairly trivial (and handles short-writes).
Signed-off-by: Aleksa Sarai <asarai@suse.de>
The usage of memfd_create(2) and other copying techniques is quite
wasteful, despite attempts to minimise it with _LIBCONTAINER_STATEDIR.
memfd_create(2) added ~10M of memory usage to the cgroup associated with
the container, which can result in some setups getting OOM'd (or just
hogging the hosts' memory when you have lots of created-but-not-started
containers sticking around).
The easiest way of solving this is by creating a read-only bind-mount of
the binary, opening that read-only bindmount, and then umounting it to
ensure that the host won't accidentally be re-mounted read-write. This
avoids all copying and cleans up naturally like the other techniques
used. Unfortunately, like the O_TMPFILE fallback, this requires being
able to create a file inside _LIBCONTAINER_STATEDIR (since bind-mounting
over the most obvious path -- /proc/self/exe -- is a *very bad idea*).
Unfortunately detecting this isn't fool-proof -- on a system with a
read-only root filesystem (that might become read-write during "runc
init" execution), we cannot tell whether we have already done an ro
remount. As a partial mitigation, we store a _LIBCONTAINER_CLONED_BINARY
environment variable which is checked *alongside* the protection being
present.
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Writing a file to tmpfs actually incurs a memcg penalty, and thus the
benefit of being able to disable memfd_create(2) with
_LIBCONTAINER_DISABLE_MEMFD_CLONE is fairly minimal -- though it should
be noted that quite a few distributions don't use tmpfs for /tmp (and
instead have it as a regular directory or subvolume of the host
filesystem).
Since runc must have write access to the state directory anyway (and the
state directory is usually not on a tmpfs) we can use that instead of
/tmp -- avoiding potential memcg costs with no real downside.
Signed-off-by: Aleksa Sarai <asarai@suse.de>
In order to get around the memfd_create(2) requirement, 0a8e4117e7
("nsenter: clone /proc/self/exe to avoid exposing host binary to
container") added an O_TMPFILE fallback. However, this fallback was
flawed in two ways:
* It required O_TMPFILE which is relatively new (having been added to
Linux 3.11).
* The fallback choice was made at compile-time, not runtime. This
results in several complications when it comes to running binaries
on different machines to the ones they were built on.
The easiest way to resolve these things is to have fallbacks work in a
more procedural way (though it does make the code unfortunately more
complicated) and to add a new fallback that uses mkotemp(3).
Signed-off-by: Aleksa Sarai <asarai@suse.de>
For a variety of reasons, sendfile(2) can end up doing a short-copy so
we need to just loop until we hit the binary size. Since /proc/self/exe
is tautologically our own binary, there's no chance someone is going to
modify it underneath us (or changing the size).
Signed-off-by: Aleksa Sarai <asarai@suse.de>
My first attempt to simplify this and make it less costly focussed on
the way constructors are called. I was under the impression that the ELF
specification mandated that arg, argv, and actually even envp need to be
passed to functions located in the .init_arry section (aka
"constructors"). Actually, the specifications is (cf. [2]):
SHT_INIT_ARRAY
This section contains an array of pointers to initialization functions,
as described in ``Initialization and Termination Functions'' in Chapter
5. Each pointer in the array is taken as a parameterless procedure with
a void return.
which means that this becomes a libc specific decision. Glibc passes
down those args, musl doesn't. So this approach can't work. However, we
can at least remove the environment parsing part based on POSIX since
[1] mandates that there should be an environ variable defined in
unistd.h which provides access to the environment. See also the relevant
Open Group specification [1].
[1]: http://pubs.opengroup.org/onlinepubs/9699919799/
[2]: http://www.sco.com/developers/gabi/latest/ch4.sheader.html#init_array
Fixes: CVE-2019-5736
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
There are quite a few circumstances where /proc/self/exe pointing to a
pretty important container binary is a _bad_ thing, so to avoid this we
have to make a copy (preferably doing self-clean-up and not being
writeable).
We require memfd_create(2) -- though there is an O_TMPFILE fallback --
but we can always extend this to use a scratch MNT_DETACH overlayfs or
tmpfs. The main downside to this approach is no page-cache sharing for
the runc binary (which overlayfs would give us) but this is far less
complicated.
This is only done during nsenter so that it happens transparently to the
Go code, and any libcontainer users benefit from it. This also makes
ExtraFiles and --preserve-fds handling trivial (because we don't need to
worry about it).
Fixes: CVE-2019-5736
Co-developed-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Cgroup namespace can be configured in `config.json` as other
namespaces. Here is an example:
```
"namespaces": [
{
"type": "pid"
},
{
"type": "network"
},
{
"type": "ipc"
},
{
"type": "uts"
},
{
"type": "mount"
},
{
"type": "cgroup"
}
],
```
Note that if you want to run a container which has shared cgroup ns with
another container, then it's strongly recommended that you set
proper `CgroupsPath` of both containers(the second container's cgroup
path must be the subdirectory of the first one). Or there might be
some unexpected results.
Signed-off-by: Yuanhong Peng <pengyuanhong@huawei.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
This PR decomposes `libcontainer/configs.Config.Rootless bool` into `RootlessEUID bool` and
`RootlessCgroups bool`, so as to make "runc-in-userns" to be more compatible with "rootful" runc.
`RootlessEUID` denotes that runc is being executed as a non-root user (euid != 0) in
the current user namespace. `RootlessEUID` is almost identical to the former `Rootless`
except cgroups stuff.
`RootlessCgroups` denotes that runc is unlikely to have the full access to cgroups.
`RootlessCgroups` is set to false if runc is executed as the root (euid == 0) in the initial namespace.
Otherwise `RootlessCgroups` is set to true.
(Hint: if `RootlessEUID` is true, `RootlessCgroups` becomes true as well)
When runc is executed as the root (euid == 0) in an user namespace (e.g. by Docker-in-LXD, Podman, Usernetes),
`RootlessEUID` is set to false but `RootlessCgroups` is set to true.
So, "runc-in-userns" behaves almost same as "rootful" runc except that cgroups errors are ignored.
This PR does not have any impact on CLI flags and `state.json`.
Note about CLI:
* Now `runc --rootless=(auto|true|false)` CLI flag is only used for setting `RootlessCgroups`.
* Now `runc spec --rootless` is only required when `RootlessEUID` is set to true.
For runc-in-userns, `runc spec` without `--rootless` should work, when sufficient numbers of
UID/GID are mapped.
Note about `$XDG_RUNTIME_DIR` (e.g. `/run/user/1000`):
* `$XDG_RUNTIME_DIR` is ignored if runc is being executed as the root (euid == 0) in the initial namespace, for backward compatibility.
(`/run/runc` is used)
* If runc is executed as the root (euid == 0) in an user namespace, `$XDG_RUNTIME_DIR` is honored if `$USER != "" && $USER != "root"`.
This allows unprivileged users to allow execute runc as the root in userns, without mounting writable `/run/runc`.
Note about `state.json`:
* `rootless` is set to true when `RootlessEUID == true && RootlessCgroups == true`.
Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
Technically, this change should not be necessary, as the kernel
documentation claims that if you call clone(flags|CLONE_NEWUSER), the
new user namespace will be the owner of all other namespaces created in
@flags. Unfortunately this isn't always the case, due to various
additional semantics and kernel bugs.
One particular instance is SELinux, which acts very strangely towards
the IPC namespace and mqueue. If you unshare the IPC namespace *before*
you map a user in the user namespace, the IPC namespace's internal
kern-mount for mqueue will be labelled incorrectly and the container
won't be able to access it. The only way of solving this is to unshare
IPC *after* the user has been mapped and we have changed to that user.
I've also heard of this happening to the NET namespace while talking to
some LXC folks, though I haven't personally seen that issue.
This change matches our handling of user namespaces to be the same as
how LXC handles these problems.
Signed-off-by: Aleksa Sarai <asarai@suse.de>
With the addition of our new{uid,gid}map support, we used to call
execvp(3) from inside nsexec. This would mean that the path resolution
for the binaries would happen in nsexec. Move the resolution to the
initial setup code, and pass the absolute path to nsexec.
Signed-off-by: Aleksa Sarai <asarai@suse.de>
After quite a bit of debugging, I found that previous versions of this
patchset did not include newgidmap in a rootless setting. Fix this by
passing it whenever group mappings are applied, and also providing some
better checking for try_mapping_tool. This commit also includes some
stylistic improvements.
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Take advantage of the newuidmap/newgidmap tools to allow multiple
users/groups to be mapped into the new user namespace in the rootless
case.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
[ rebased to handle intelrdt changes. ]
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Kir Kolyshkin