Skip to content

Commit

Permalink
bwrap: Use user namespaces and avoid overmounting
Browse files Browse the repository at this point in the history
Now that we've split out the "base" constructor for the initramfs
special case, the "mutable" disposition tells us how to handle
binding `/etc` and `/var` in addition to `/usr`.  Drop the
`RPMOSTREE_BWRAP_IMMUTABLE` case as it is now only used in the
script case where we handle `/var`.

Next, user namespaces denies overmounting existing targets as it
could reveal things intended to be hidden.  In the recursive
container case, we need to just use e.g. `--bind /dev /dev`
rather than trying to create a new instances.

Further, in the selftest case we can't overmount what already
exists, so skip doing that entirely.

Closes: coreos#1329
  • Loading branch information
cgwalters committed Apr 11, 2018
1 parent 1f63e09 commit fe57432
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 38 deletions.
9 changes: 9 additions & 0 deletions .papr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,15 @@ tests:
-v $(pwd):/srv/code -w /srv/code
registry.fedoraproject.org/fedora:27 /bin/sh -c
"./ci/build.sh && make install && ./tests/compose"
# And now one that isn't --privileged https://github.com/projectatomic/rpm-ostree/issues/1329
- docker run --rm --security-opt seccomp=unconfined
--security-opt label=type:spc_t \
-e RPMOSTREE_COMPOSE_TEST_USE_REPOS=/etc/yum.repos.d.host
-v /etc/yum.repos.d:/etc/yum.repos.d.host:ro
-v $(pwd):/srv/code -w /srv/code
registry.fedoraproject.org/fedora:27 /bin/sh -c
"./ci/build.sh && make install && env TESTS=basic ./tests/compose"


artifacts:
- test-compose-logs
Expand Down
110 changes: 87 additions & 23 deletions src/libpriv/rpmostree-bwrap.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <err.h>
#include <stdio.h>
#include <systemd/sd-journal.h>
#include <sys/capability.h>

void
rpmostree_ptrarray_append_strdup (GPtrArray *argv_array, ...)
Expand Down Expand Up @@ -174,8 +175,7 @@ setup_rofiles_usr (RpmOstreeBwrap *bwrap,
return FALSE;

rpmostree_bwrap_append_bwrap_argv (bwrap, "--bind", rofiles_mntpath, "/usr", NULL);

/* also mount /etc from the rofiles mount to allow RPM scripts to change defaults, while
/* also mount /etc to the rofiles /usr/etc to allow RPM scripts to change defaults, while
* still being protected; note we use bind to ensure symlinks work, see:
* https://github.com/projectatomic/rpm-ostree/pull/640 */
const char *rofiles_etc_mntpath = glnx_strjoina (rofiles_mntpath, "/etc");
Expand All @@ -200,8 +200,21 @@ running_in_nspawn (void)
return g_strcmp0 (getenv ("container"), "systemd-nspawn") == 0;
}

RpmOstreeBwrap *
rpmostree_bwrap_new_base (int rootfs_fd, GError **error)
/* We want to support running rpm-ostree in non-privileged Kubernetes/OpenShift
* pods. This tests effectively if we running under default docker capabilities?
* Also equivalent to OpenShift scc `anyuid`.
*
* https://github.com/projectatomic/rpm-ostree/issues/1329
*/
static gboolean
running_as_uid0_container (void)
{
return getuid () == 0 &&
cap_get_bound (CAP_SYS_ADMIN) == 0;
}

static RpmOstreeBwrap *
bwrap_new_internal (int rootfs_fd, gboolean for_selftest, GError **error)
{
g_autoptr(RpmOstreeBwrap) ret = g_new0 (RpmOstreeBwrap, 1);
static const char *usr_links[] = {"lib", "lib32", "lib64", "bin", "sbin"};
Expand All @@ -228,20 +241,11 @@ rpmostree_bwrap_new_base (int rootfs_fd, GError **error)
/* ⚠⚠⚠ If you change this, also update scripts/bwrap-script-shell.sh ⚠⚠⚠ */
rpmostree_bwrap_append_bwrap_argv (ret,
WITH_BUBBLEWRAP_PATH,
"--dev", "/dev",
"--proc", "/proc",
"--dir", "/tmp",
"--chdir", "/",
"--ro-bind", "/sys/block", "/sys/block",
"--ro-bind", "/sys/bus", "/sys/bus",
"--ro-bind", "/sys/class", "/sys/class",
"--ro-bind", "/sys/dev", "/sys/dev",
"--ro-bind", "/sys/devices", "/sys/devices",
"--die-with-parent", /* Since 0.1.8 */
/* Here we do all namespaces except the user one.
* Down the line we want to do a userns too I think,
* but it may need some mapping work.
*/
/* See below regarding net and user namespaces */
"--dev", "/dev",
"--unshare-pid",
"--unshare-uts",
"--unshare-ipc",
Expand All @@ -251,6 +255,51 @@ rpmostree_bwrap_new_base (int rootfs_fd, GError **error)
if (!running_in_nspawn ())
rpmostree_bwrap_append_bwrap_argv (ret, "--unshare-net", NULL);

/* Down the line we might want to enable userns unconditionally everywhere,
* I'm currently just trying to be conservative and not affect the pkglayering
* case. I also worry about userns enablement on RHEL/CentOS, so it'd
* have to be at least --unshare-user-try there.
*
* Further, in the recursive case, we need to just bind mount the API mounts
* like /proc,/sys rather than try to creat new instances, as trying to
* create new ones would potentially undo the read-only mounts that the "outer
* container" has made.
*/
if (running_as_uid0_container ())
{
/* User namespace */
rpmostree_bwrap_append_bwrap_argv (ret, "--unshare-user", NULL);

/* In the selftest case, there's no reason to overmount. So don't.
*
* See above for rationale for the non-selftest container case.
*/
if (!for_selftest)
{
rpmostree_bwrap_append_bwrap_argv (ret,
"--bind", "/proc", "/proc",
"--bind", "/sys", "/sys",
NULL);
}
}
else
{
/* We're running on a raw host (e.g. package layering), or in a privileged
* container. Let's set up strict mounts, similar to what runc (I think)
* is doing. Note to self: bwrap covers less of this than runc does, e.g.
* I see a tmpfs mount over `/proc/scsi`. That said if we enable userns
* (see above) that should be unnecessary.
*/
rpmostree_bwrap_append_bwrap_argv (ret,
"--proc", "/proc",
"--ro-bind", "/sys/block", "/sys/block",
"--ro-bind", "/sys/bus", "/sys/bus",
"--ro-bind", "/sys/class", "/sys/class",
"--ro-bind", "/sys/dev", "/sys/dev",
"--ro-bind", "/sys/devices", "/sys/devices",
NULL);
}

/* Capabilities; this is a subset of the Docker (1.13 at least) default.
* Specifically we strip out in addition to that:
*
Expand Down Expand Up @@ -300,6 +349,12 @@ rpmostree_bwrap_new_base (int rootfs_fd, GError **error)
return g_steal_pointer (&ret);
}

RpmOstreeBwrap *
rpmostree_bwrap_new_base (int rootfs_fd, GError **error)
{
return bwrap_new_internal (rootfs_fd, FALSE, error);
}

RpmOstreeBwrap *
rpmostree_bwrap_new (int rootfs_fd,
RpmOstreeBwrapMutability mutable,
Expand All @@ -313,18 +368,27 @@ rpmostree_bwrap_new (int rootfs_fd,
va_list args;
switch (mutable)
{
case RPMOSTREE_BWRAP_IMMUTABLE:
rpmostree_bwrap_append_bwrap_argv (ret, "--ro-bind", "usr", "/usr", NULL);
break;
case RPMOSTREE_BWRAP_MUTATE_ROFILES:
if (!setup_rofiles_usr (ret, error))
return NULL;
{
if (!setup_rofiles_usr (ret, error))
return NULL;
rpmostree_bwrap_append_bwrap_argv (ret, "--ro-bind", "./var", "/var", NULL);
}
break;
case RPMOSTREE_BWRAP_MUTATE_FREELY:
rpmostree_bwrap_append_bwrap_argv (ret, "--bind", "usr", "/usr", NULL);
rpmostree_bwrap_append_bwrap_argv (ret, "--bind", "usr", "/usr",
"--bind", "usr/etc", "/etc",
"--bind", "var", "/var",
NULL);
break;
}

/* We may have a ro bind mount over /var above. However we want a writable
* var/tmp, so we need to tmpfs mount on top of it. See also
* https://github.com/projectatomic/bubblewrap/issues/182
*/
rpmostree_bwrap_append_bwrap_argv (ret, "--tmpfs", "/var/tmp", NULL);

const char *arg;
va_start (args, error);
while ((arg = va_arg (args, char *)))
Expand Down Expand Up @@ -421,10 +485,10 @@ rpmostree_bwrap_selftest (GError **error)
if (!glnx_opendirat (AT_FDCWD, "/", TRUE, &host_root_dfd, error))
return FALSE;

bwrap = rpmostree_bwrap_new (host_root_dfd, RPMOSTREE_BWRAP_IMMUTABLE, error, NULL);
bwrap = bwrap_new_internal (host_root_dfd, TRUE, error);
if (!bwrap)
return FALSE;

rpmostree_bwrap_append_bwrap_argv (bwrap, "--ro-bind", "usr", "/usr", NULL);
rpmostree_bwrap_append_child_argv (bwrap, "true", NULL);

if (!rpmostree_bwrap_run (bwrap, NULL, error))
Expand Down
1 change: 0 additions & 1 deletion src/libpriv/rpmostree-bwrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
#include "libglnx.h"

typedef enum {
RPMOSTREE_BWRAP_IMMUTABLE = 0,
RPMOSTREE_BWRAP_MUTATE_ROFILES,
RPMOSTREE_BWRAP_MUTATE_FREELY
} RpmOstreeBwrapMutability;
Expand Down
3 changes: 0 additions & 3 deletions src/libpriv/rpmostree-postprocess.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,11 @@ run_bwrap_mutably (int rootfs_fd,
bwrap = rpmostree_bwrap_new (rootfs_fd,
RPMOSTREE_BWRAP_MUTATE_ROFILES,
error,
"--ro-bind", "./var", "/var",
NULL);
else
bwrap = rpmostree_bwrap_new (rootfs_fd,
RPMOSTREE_BWRAP_MUTATE_FREELY,
error,
"--bind", "var", "/var",
"--bind", "usr/etc", "/etc",
NULL);

if (!bwrap)
Expand Down
20 changes: 9 additions & 11 deletions src/libpriv/rpmostree-scripts.c
Original file line number Diff line number Diff line change
Expand Up @@ -325,12 +325,7 @@ run_script_in_bwrap_container (int rootfs_fd,
}

/* ⚠⚠⚠ If you change this, also update scripts/bwrap-script-shell.sh ⚠⚠⚠ */

/* We just did a ro bind mount over /var above. However we want a writable
* var/tmp, so we need to tmpfs mount on top of it. See also
* https://github.com/projectatomic/bubblewrap/issues/182
* Similarly for /var/lib/rpm-state.
*
/*
* See above for why we special case glibc.
*/
const gboolean is_glibc_locales = strcmp (pkg_script, "glibc-all-langpacks.posttrans") == 0;
Expand All @@ -339,9 +334,6 @@ run_script_in_bwrap_container (int rootfs_fd,
bwrap = rpmostree_bwrap_new (rootfs_fd,
mutability,
error,
/* Scripts can see a /var with compat links like alternatives */
"--ro-bind", "./var", "/var",
"--tmpfs", "/var/tmp",
NULL);
if (!bwrap)
goto out;
Expand Down Expand Up @@ -927,10 +919,16 @@ rpmostree_deployment_sanitycheck (int rootfs_fd,

GLNX_AUTO_PREFIX_ERROR ("sanitycheck", error);
g_autoptr(RpmOstreeBwrap) bwrap =
rpmostree_bwrap_new (rootfs_fd, RPMOSTREE_BWRAP_IMMUTABLE, error,
"--ro-bind", "./usr/etc", "/etc", NULL);
rpmostree_bwrap_new_base (rootfs_fd, error);
if (!bwrap)
return FALSE;
/* The deployment /var will be empty, we could use the host's but eh, why not
* just do an empty one.
*/
rpmostree_bwrap_append_bwrap_argv (bwrap, "--ro-bind", "usr", "/usr",
"--ro-bind", "usr/etc", "/etc",
"--dir", "/var/tmp",
NULL);
rpmostree_bwrap_append_child_argv (bwrap, "/usr/bin/true", NULL);
if (!rpmostree_bwrap_run (bwrap, cancellable, error))
return FALSE;
Expand Down

0 comments on commit fe57432

Please sign in to comment.