Package: bubblewrap
Version: 0~git160513-2
Severity: normal
Tags: patch
Forwarded: https://github.com/projectatomic/bubblewrap/pull/71
"bwrap --ro-bind / / --unshare-user --uid 2 --gid 3 /bin/sh" runs the
wrapped shell in its own user namespace with uid 2 and gid 3, as expected.
Similarly, "bwrap --ro-bind / / --dev /dev /bin/sh" runs the wrapped
shell with a minimal /dev, as expected.
However, combining "--dev /dev" with "--unshare-user" fails on Debian
kernels with their default configuration (kernel.unprivileged_userns_clone=0);
in particular, this breaks flatpak-builder. Alex Larsson has proposed
patches upstream for this issue, which I have applied to the embedded
copy of bubblewrap in flatpak 0.6.0-2.
Patches are attached, and also available in git here:
ssh://git.debian.org/git/users/smcv/bubblewrap.git
among several others.
Ideally, I would like to copy that git repository into collab-maint,
and commit this sort of thing directly as a co-maintainer.
Regards,
S
>From d8cbc3d8e3f43e56f43285f6fe4cb2c923f794cc Mon Sep 17 00:00:00 2001
From: Simon McVittie <[email protected]>
Date: Sat, 21 May 2016 17:31:10 +0100
Subject: [PATCH 6/7] Add patches from upstream bug 71 to make --dev coexist
with --unshare-user
---
debian/changelog | 2 +
debian/patches/Add-unshare-user-try.patch | 57 ++++++
...tuid-no-unprivileged-user-namespaces-work.patch | 197 +++++++++++++++++++++
debian/patches/series | 2 +
4 files changed, 258 insertions(+)
create mode 100644 debian/patches/Add-unshare-user-try.patch
create mode 100644 debian/patches/Make-setuid-no-unprivileged-user-namespaces-work.patch
create mode 100644 debian/patches/series
diff --git a/debian/changelog b/debian/changelog
index 00aad10..77fece6 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -7,6 +7,8 @@ bubblewrap (0~git160513-3) UNRELEASED; urgency=medium
* Increase Priority to optional, because this tool is likely to be
depended on by gnome-software (via Flatpak) in future
* debian/gbp.conf: add DEP-14-style git-buildpackage configuration
+ * Add patches from upstream bug 71 to make --dev coexist with
+ --unshare-user
-- Simon McVittie <[email protected]> Sat, 21 May 2016 15:10:56 +0100
diff --git a/debian/patches/Add-unshare-user-try.patch b/debian/patches/Add-unshare-user-try.patch
new file mode 100644
index 0000000..1053ad7
--- /dev/null
+++ b/debian/patches/Add-unshare-user-try.patch
@@ -0,0 +1,57 @@
+From: Alexander Larsson <[email protected]>
+Date: Fri, 20 May 2016 15:13:57 +0200
+Subject: Add --unshare-user-try
+
+This optionally enables user namespaces, but ignores it if its
+not supported by the kernel.
+
+Note: For this to make any sense, bwrap has to be setuid,
+because unprivileged use requires user namespaces.
+
+Bug: https://github.com/projectatomic/bubblewrap/pull/71
+---
+ bubblewrap.c | 10 ++++++++++
+ 1 file changed, 10 insertions(+)
+
+diff --git a/bubblewrap.c b/bubblewrap.c
+index 59407a7..6c4664e 100644
+--- a/bubblewrap.c
++++ b/bubblewrap.c
+@@ -148,6 +148,7 @@ usage (int ecode, FILE *out)
+ " --version Print version\n"
+ " --args FD Parse nul-separated args from FD\n"
+ " --unshare-user Create new user namespace (may be automatically implied if not setuid)\n"
++ " --unshare-user-try Create new user namespace if possible else continue by skipping it\n"
+ " --unshare-ipc Create new ipc namespace\n"
+ " --unshare-pid Create new pid namespace\n"
+ " --unshare-net Create new network namespace\n"
+@@ -840,6 +841,7 @@ read_priv_sec_op (int read_socket,
+
+ char *opt_chdir_path = NULL;
+ bool opt_unshare_user = FALSE;
++bool opt_unshare_user_try = FALSE;
+ bool opt_unshare_pid = FALSE;
+ bool opt_unshare_ipc = FALSE;
+ bool opt_unshare_net = FALSE;
+@@ -955,6 +957,10 @@ parse_args_recurse (int *argcp,
+ {
+ opt_unshare_user = TRUE;
+ }
++ else if (strcmp (arg, "--unshare-user-try") == 0)
++ {
++ opt_unshare_user_try = TRUE;
++ }
+ else if (strcmp (arg, "--unshare-ipc") == 0)
+ {
+ opt_unshare_ipc = TRUE;
+@@ -1327,6 +1333,10 @@ main (int argc,
+ if (!is_privileged)
+ opt_unshare_user = TRUE;
+
++ if (opt_unshare_user_try &&
++ stat ("/proc/self/ns/user", &sbuf) == 0)
++ opt_unshare_user = TRUE;
++
+ if (argc == 0)
+ usage (EXIT_FAILURE, stderr);
+
diff --git a/debian/patches/Make-setuid-no-unprivileged-user-namespaces-work.patch b/debian/patches/Make-setuid-no-unprivileged-user-namespaces-work.patch
new file mode 100644
index 0000000..ec18f7e
--- /dev/null
+++ b/debian/patches/Make-setuid-no-unprivileged-user-namespaces-work.patch
@@ -0,0 +1,197 @@
+From: Alexander Larsson <[email protected]>
+Date: Fri, 20 May 2016 14:59:43 +0200
+Subject: Make setuid + no-unprivileged user namespaces work
+
+On e.g. debian by default unprivileged namespaces are not allowed.
+Typically the setuid mode is then used. However, if /dev is mounted
+(and thus devpts) then we need to do some workaround in how we
+create the uid/gid maps so uid 0 is mapped while we mount devpts.
+
+Unfortunately the way we were working around that is by using an
+unprivileged unshare(NEWUSER) in the sandbox, which doesn't work.
+See https://github.com/flatpak/flatpak/issues/2 for details.
+
+We work around this by mapping uid/gid 0 + the user. However, since
+this is a privileged operation we need to do that in the parent
+namespace, and we need setuid/setgid rights.
+
+Bug: https://github.com/projectatomic/bubblewrap/pull/71
+---
+ Makefile.am | 2 +-
+ bubblewrap.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------
+ 2 files changed, 70 insertions(+), 11 deletions(-)
+
+diff --git a/Makefile.am b/Makefile.am
+index e4d250b..6e47b5f 100644
+--- a/Makefile.am
++++ b/Makefile.am
+@@ -14,7 +14,7 @@ if PRIV_MODE_SETUID
+ $(SUDO_BIN) chmod u+s $(DESTDIR)$(bindir)/bwrap
+ else
+ if PRIV_MODE_FILECAPS
+- $(SUDO_BIN) setcap cap_sys_admin,cap_net_admin,cap_sys_chroot+ep $(DESTDIR)$(bindir)/bwrap
++ $(SUDO_BIN) setcap cap_sys_admin,cap_net_admin,cap_sys_chroot,cap_setuid,cap_setgid+ep $(DESTDIR)$(bindir)/bwrap
+ endif
+ endif
+
+diff --git a/bubblewrap.c b/bubblewrap.c
+index 1f9331a..59407a7 100644
+--- a/bubblewrap.c
++++ b/bubblewrap.c
+@@ -358,7 +358,7 @@ do_init (int event_fd, pid_t initial_pid)
+ }
+
+ /* low 32bit caps needed */
+-#define REQUIRED_CAPS_0 (CAP_TO_MASK (CAP_SYS_ADMIN) | CAP_TO_MASK (CAP_SYS_CHROOT) | CAP_TO_MASK (CAP_NET_ADMIN))
++#define REQUIRED_CAPS_0 (CAP_TO_MASK (CAP_SYS_ADMIN) | CAP_TO_MASK (CAP_SYS_CHROOT) | CAP_TO_MASK (CAP_SETUID) | CAP_TO_MASK (CAP_SETGID))
+ /* high 32bit caps needed */
+ #define REQUIRED_CAPS_1 0
+
+@@ -441,21 +441,44 @@ write_uid_gid_map (uid_t sandbox_uid,
+ uid_t parent_uid,
+ uid_t sandbox_gid,
+ uid_t parent_gid,
+- bool deny_groups)
++ pid_t pid,
++ bool deny_groups,
++ bool map_root)
+ {
+ cleanup_free char *uid_map = NULL;
+ cleanup_free char *gid_map = NULL;
++ cleanup_free char *dir = NULL;
++ cleanup_fd int dir_fd = -1;
+
+- uid_map = xasprintf ("%d %d 1\n", sandbox_uid, parent_uid);
+- if (write_file_at (proc_fd, "self/uid_map", uid_map) != 0)
++ if (pid == -1)
++ dir = xstrdup ("self");
++ else
++ dir = xasprintf ("%d", pid);
++
++ dir_fd = openat (proc_fd, dir, O_RDONLY | O_PATH);
++ if (dir_fd < 0)
++ die_with_error ("open /proc/%s failed", dir);
++
++ if (map_root && parent_uid != 0 && sandbox_uid != 0)
++ uid_map = xasprintf ("0 0 1\n"
++ "%d %d 1\n", sandbox_uid, parent_uid);
++ else
++ uid_map = xasprintf ("%d %d 1\n", sandbox_uid, parent_uid);
++
++ if (write_file_at (dir_fd, "uid_map", uid_map) != 0)
+ die_with_error ("setting up uid map");
+
+ if (deny_groups &&
+- write_file_at (proc_fd, "self/setgroups", "deny\n") != 0)
++ write_file_at (dir_fd, "setgroups", "deny\n") != 0)
+ die_with_error ("error writing to setgroups");
+
+- gid_map = xasprintf ("%d %d 1\n", sandbox_gid, parent_gid);
+- if (write_file_at (proc_fd, "self/gid_map", gid_map) != 0)
++ if (map_root && parent_gid != 0 && sandbox_gid != 0)
++ gid_map = xasprintf ("0 0 1\n"
++ "%d %d 1\n", sandbox_gid, parent_gid);
++ else
++ gid_map = xasprintf ("%d %d 1\n", sandbox_gid, parent_gid);
++
++ if (write_file_at (dir_fd, "gid_map", gid_map) != 0)
+ die_with_error ("setting up gid map");
+ }
+
+@@ -1269,10 +1292,13 @@ main (int argc,
+ char *old_cwd = NULL;
+ pid_t pid;
+ int event_fd = -1;
++ int child_wait_fd = -1;
+ const char *new_cwd;
+ uid_t ns_uid;
+ gid_t ns_gid;
+ struct stat sbuf;
++ uint64_t val;
++ int res UNUSED;
+
+ /* Get the (optional) capabilities we need, drop root */
+ acquire_caps ();
+@@ -1374,6 +1400,10 @@ main (int argc,
+ if (!stat ("/proc/self/ns/cgroup", &sbuf))
+ clone_flags |= CLONE_NEWCGROUP;
+
++ child_wait_fd = eventfd (0, EFD_CLOEXEC);
++ if (child_wait_fd == -1)
++ die_with_error ("eventfd()");
++
+ pid = raw_clone (clone_flags, NULL);
+ if (pid == -1)
+ {
+@@ -1388,23 +1418,52 @@ main (int argc,
+ die_with_error ("Creating new namespace failed");
+ }
+
++ ns_uid = opt_sandbox_uid;
++ ns_gid = opt_sandbox_gid;
++
+ if (pid != 0)
+ {
++ if (is_privileged && opt_unshare_user)
++ {
++ /* Map the uid/gid 0 if opt_needs_devpts, as otherwise
++ * mounting it will fail.
++ * Due to this non-direct mapping we need to have set[ug]id
++ * caps in the parent namespaces, and thus we need to write
++ * the map in the parent namespace, not the child. */
++ write_uid_gid_map (ns_uid, uid,
++ ns_gid, gid,
++ pid, TRUE, opt_needs_devpts);
++ }
++
+ /* Initial launched process, wait for exec:ed command to exit */
+
+ /* We don't need any caps in the launcher, drop them immediately. */
+ drop_caps ();
++
++ /* Let child run */
++ val = 1;
++ res = write (child_wait_fd, &val, 8);
++ /* Ignore res, if e.g. the child died and closed child_wait_fd we don't want to error out here */
++ close (child_wait_fd);
++
+ monitor_child (event_fd);
+ exit (0); /* Should not be reached, but better safe... */
+ }
+
++ /* Wait for the parent to init uid/gid maps and drop caps */
++ res = read (child_wait_fd, &val, 8);
++ close (child_wait_fd);
++
+ if (opt_unshare_net && loopback_setup () != 0)
+ die ("Can't create loopback device");
+
+ ns_uid = opt_sandbox_uid;
+ ns_gid = opt_sandbox_gid;
+- if (opt_unshare_user)
++ if (!is_privileged && opt_unshare_user)
+ {
++ /* In the unprivileged case we have to write the uid/gid maps in
++ * the child, because we have no caps in the parent */
++
+ if (opt_needs_devpts)
+ {
+ /* This is a bit hacky, but we need to first map the real uid/gid to
+@@ -1417,7 +1476,7 @@ main (int argc,
+
+ write_uid_gid_map (ns_uid, uid,
+ ns_gid, gid,
+- TRUE);
++ -1, TRUE, FALSE);
+ }
+
+ old_umask = umask (0);
+@@ -1523,7 +1582,7 @@ main (int argc,
+
+ write_uid_gid_map (opt_sandbox_uid, ns_uid,
+ opt_sandbox_gid, ns_gid,
+- FALSE);
++ -1, FALSE, FALSE);
+ }
+
+ /* Now make /newroot the real root */
diff --git a/debian/patches/series b/debian/patches/series
new file mode 100644
index 0000000..850bb4f
--- /dev/null
+++ b/debian/patches/series
@@ -0,0 +1,2 @@
+Make-setuid-no-unprivileged-user-namespaces-work.patch
+Add-unshare-user-try.patch
--
2.8.1
>From cb3042bd4ca549e9b6452bc9afdd65929f287f81 Mon Sep 17 00:00:00 2001
From: Simon McVittie <[email protected]>
Date: Sat, 21 May 2016 21:44:19 +0100
Subject: [PATCH 7/7] Add some simple autopkgtests
In particular, "userns" verifies that upstream bug 71 is fixed.
---
debian/changelog | 1 +
debian/tests/basic | 13 +++++++++++++
debian/tests/control | 5 +++++
debian/tests/dev | 36 ++++++++++++++++++++++++++++++++++++
debian/tests/testlib.sh | 40 ++++++++++++++++++++++++++++++++++++++++
debian/tests/userns | 35 +++++++++++++++++++++++++++++++++++
6 files changed, 130 insertions(+)
create mode 100755 debian/tests/basic
create mode 100644 debian/tests/control
create mode 100755 debian/tests/dev
create mode 100644 debian/tests/testlib.sh
create mode 100755 debian/tests/userns
diff --git a/debian/changelog b/debian/changelog
index 77fece6..7435098 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -9,6 +9,7 @@ bubblewrap (0~git160513-3) UNRELEASED; urgency=medium
* debian/gbp.conf: add DEP-14-style git-buildpackage configuration
* Add patches from upstream bug 71 to make --dev coexist with
--unshare-user
+ * Add some simple autopkgtests, including one for bug 71
-- Simon McVittie <[email protected]> Sat, 21 May 2016 15:10:56 +0100
diff --git a/debian/tests/basic b/debian/tests/basic
new file mode 100755
index 0000000..41d6597
--- /dev/null
+++ b/debian/tests/basic
@@ -0,0 +1,13 @@
+#!/usr/bin/bats
+# vim:set sw=4 sts=4 et ft=sh:
+
+set -e
+
+. debian/tests/testlib.sh
+
+@test "Basic usage" {
+ ret=0
+ run bwrap --ro-bind / / /usr/bin/id
+ is "$status" 0
+ is "$output" "$(id)"
+}
diff --git a/debian/tests/control b/debian/tests/control
new file mode 100644
index 0000000..9774a97
--- /dev/null
+++ b/debian/tests/control
@@ -0,0 +1,5 @@
+Tests:
+ userns
+Depends:
+ bats,
+ bubblewrap,
diff --git a/debian/tests/dev b/debian/tests/dev
new file mode 100755
index 0000000..f14b7e8
--- /dev/null
+++ b/debian/tests/dev
@@ -0,0 +1,36 @@
+#!/usr/bin/bats
+# vim:set sw=4 sts=4 et ft=sh:
+
+set -e
+
+. debian/tests/testlib.sh
+
+@test "Mount new minimal /dev" {
+ run bwrap --ro-bind / / --dev /dev /bin/sh -c 'echo /dev/*'
+
+ like " $output " " /dev/full "
+ like " $output " " /dev/null "
+ like " $output " " /dev/pts "
+ like " $output " " /dev/random "
+ like " $output " " /dev/shm "
+ like " $output " " /dev/stderr "
+ like " $output " " /dev/stdin "
+ like " $output " " /dev/stdout "
+ like " $output " " /dev/tty "
+ like " $output " " /dev/urandom "
+ like " $output " " /dev/zero "
+
+ # an arbitrary selection of devices not expected to be passed through
+ unlike " $output " " /dev/hda "
+ unlike " $output " " /dev/dsp "
+ unlike " $output " " /dev/fuse "
+ unlike " $output " " /dev/kmsg "
+ unlike " $output " " /dev/loop0 "
+ unlike " $output " " /dev/mem "
+ unlike " $output " " /dev/sda "
+ unlike " $output " " /dev/snd "
+ unlike " $output " " /dev/tty1 "
+ unlike " $output " " /dev/vda "
+
+ is "$status" 0
+}
diff --git a/debian/tests/testlib.sh b/debian/tests/testlib.sh
new file mode 100644
index 0000000..31946b0
--- /dev/null
+++ b/debian/tests/testlib.sh
@@ -0,0 +1,40 @@
+# vim:set sw=4 sts=4 et ft=sh:
+
+# is GOT EXPECTED
+# Assert that GOT == EXPECTED.
+# (Inspired by Perl's Test::More)
+is () {
+ if [ "x$1" = "x$2" ]; then
+ return 0
+ else
+ printf "# got: %q\n" "$1"
+ printf "# expected: %q\n" "$2"
+ return 1
+ fi
+}
+
+# like GOT EREGEX
+# Assert that GOT matches EREGEX.
+# (Inspired by Perl's Test::More)
+like () {
+ if [[ $1 =~ $2 ]]; then
+ return 0
+ else
+ printf "# got: %q\n" "$1"
+ printf "# should match ERE: %q\n" "$2"
+ return 1
+ fi
+}
+
+# unlike GOT EREGEX
+# Assert that GOT matches EREGEX.
+# (Inspired by Perl's Test::More)
+unlike () {
+ if [[ $1 =~ $2 ]]; then
+ printf "# got: %q\n" "$1"
+ printf "# should not match ERE: %q\n" "$2"
+ return 1
+ else
+ return 0
+ fi
+}
diff --git a/debian/tests/userns b/debian/tests/userns
new file mode 100755
index 0000000..c8862c6
--- /dev/null
+++ b/debian/tests/userns
@@ -0,0 +1,35 @@
+#!/usr/bin/bats
+# vim:set sw=4 sts=4 et ft=sh:
+
+set -e
+
+. debian/tests/testlib.sh
+
+@test "Unshare user ID" {
+ run bwrap --ro-bind / / --unshare-user --uid 2 --gid 3 /usr/bin/id -u
+ is "$status" 0
+ is "$output" 2
+ run bwrap --ro-bind / / --unshare-user --uid 2 --gid 3 /usr/bin/id -g
+ is "$status" 0
+ is "$output" 3
+ run bwrap --ro-bind / / --unshare-user --uid 2 --gid 3 /bin/sh -c 'ls -l /etc/passwd'
+ is "$status" 0
+ like "$output" " nobody nogroup "
+}
+
+@test "Combine new /dev with new user namespace (#71)" {
+ run bwrap --ro-bind / / --unshare-user --uid 2 --gid 3 --dev /dev /bin/sh -c 'echo /dev/*'
+
+ like " $output " " /dev/full "
+ unlike " $output " " /dev/tty1 "
+
+ is "$status" 0
+
+ run bwrap --ro-bind / / --unshare-user --uid 2 --gid 3 --dev /dev /usr/bin/id -u
+ is "$status" 0
+ is "$output" 2
+
+ run bwrap --ro-bind / / --unshare-user --uid 2 --gid 3 --dev /dev /usr/bin/id -g
+ is "$status" 0
+ is "$output" 3
+}
--
2.8.1
