minijail0: Remount mounts as MS_SLAVE by default.
When executing the sandboxed program in a new mount namespace the
Minijail library will by default remount all mounts with the MS_PRIVATE
flag. While this is an appropriate, safe default for the library,
MS_PRIVATE can be problematic: unmount events will not propagate into
mountpoints marked as MS_PRIVATE. This can cause bugs (see code
comments for details.)
Fix by keeping MS_PRIVATE as default in libminijail.c but switch the
default in minijail0_cli.c to MS_SLAVE.
In order to avoid command-lines like "-Kprivate -v" overriding the -K
option, call set_remount_mode() after all command-line options have
been processed.
Bug: 147609789
Test: Unit tests pass.
Test: See repro steps in bug.
Test: "minijail0 -Kprivate -v" remounts with MS_PRIVATE.
Change-Id: I4788ffb92309d9b6e02a38ef2998bcecf5314b50
diff --git a/minijail0_cli.c b/minijail0_cli.c
index 940bd3c..94d8578 100644
--- a/minijail0_cli.c
+++ b/minijail0_cli.c
@@ -645,6 +645,7 @@
int binding = 0;
int chroot = 0, pivot_root = 0;
int mount_ns = 0, change_remount = 0;
+ const char *remount_mode = NULL;
int inherit_suppl_gids = 0, keep_suppl_gids = 0;
int caps = 0, ambient_caps = 0;
int seccomp = -1;
@@ -746,11 +747,7 @@
add_mount(j, optarg);
break;
case 'K':
- if (optarg) {
- set_remount_mode(j, optarg);
- } else {
- minijail_skip_remount_private(j);
- }
+ remount_mode = optarg;
change_remount = 1;
break;
case 'P':
@@ -780,6 +777,37 @@
break;
case 'v':
minijail_namespace_vfs(j);
+ /*
+ * Set the default mount propagation in the command-line
+ * tool to MS_SLAVE.
+ *
+ * When executing the sandboxed program in a new mount
+ * namespace the Minijail library will by default
+ * remount all mounts with the MS_PRIVATE flag. While
+ * this is an appropriate, safe default for the library,
+ * MS_PRIVATE can be problematic: unmount events will
+ * not propagate into mountpoints marked as MS_PRIVATE.
+ * This means that if a mount is unmounted in the root
+ * mount namespace, it will not be unmounted in the
+ * non-root mount namespace.
+ * This in turn can be problematic because activity in
+ * the non-root mount namespace can now directly
+ * influence the root mount namespace (e.g. preventing
+ * re-mounts of said mount), which would be a privilege
+ * inversion.
+ *
+ * Setting the default in the command-line to MS_SLAVE
+ * will still prevent mounts from leaking out of the
+ * non-root mount namespace but avoid these
+ * privilege-inversion issues.
+ * For cases where mounts should not flow *into* the
+ * namespace either, the user can pass -Kprivate.
+ * Note that mounts are marked as MS_PRIVATE by default
+ * by the kernel, so unless the init process (like
+ * systemd) or something else marks them as shared, this
+ * won't do anything.
+ */
+ minijail_remount_mode(j, MS_SLAVE);
mount_ns = 1;
break;
case 'V':
@@ -990,6 +1018,15 @@
exit(1);
}
+ /* Configure the remount flag here to avoid having -v override it. */
+ if (change_remount) {
+ if (remount_mode != NULL) {
+ set_remount_mode(j, remount_mode);
+ } else {
+ minijail_skip_remount_private(j);
+ }
+ }
+
/*
* Proceed in setting the supplementary gids specified on the
* cmdline options.
