Bug#852857: unblock: flatpak/0.8.2-1
Control: reopen 852857
On Sun, 29 Jan 2017 at 11:24:40 +0000, Jonathan Wiltshire wrote:
> On Fri, Jan 27, 2017 at 10:35:58PM +0000, Simon McVittie wrote:
> > Please reduce flatpak's time delay, or unblock it:
> >
> > unblock flatpak/0.8.2-1
>
> Unblocked.
Was this unblocked? I don't see a hint mentioning flatpak in any of the
hints files at <https://release.debian.org/britney/hints/>?
If flatpak's migration delay remains at 10 days (as it is now), then
getting it in will also require bubblewrap 0.1.7-1 (currently at 13/10
days) to be unblocked so it can still migrate after the hard freeze. The
bubblewrap currently in testing has an overly strict solution to
CVE-2017-5226 that makes interactive use (a shell inside the sandbox)
rather difficult by breaking job control; the revised solution to
CVE-2017-5226 moves responsibility for it from bubblewrap to flatpak,
resulting in adding some Breaks.
The 0.1.7 release also has other upstream fixes that improve security
hardening for the sandbox.
If you would prefer, I could enable flatpak's embedded code copy of
bubblewrap 0.1.7 and ask for the system bubblewrap to be removed from
testing, since these packages are somewhat tightly coupled - but I'd
rather not do that, because bubblewrap is useful in its own right.
I attach a debdiff for bubblewrap, in case that's useful information.
Thanks,
S
diffstat for bubblewrap-0.1.5 bubblewrap-0.1.7
.redhat-ci.yml | 5
bubblewrap.c | 133 ++++++++--
bwrap.xml | 19 +
configure.ac | 2
debian/bubblewrap.examples | 3
debian/changelog | 43 +++
debian/control | 2
debian/patches/Call-setsid-before-executing-sandboxed-code-CVE-2017-5226.patch | 29 --
debian/patches/series | 1
demos/bubblewrap-shell.sh | 16 +
10 files changed, 190 insertions(+), 63 deletions(-)
diff -Nru bubblewrap-0.1.5/bubblewrap.c bubblewrap-0.1.7/bubblewrap.c
--- bubblewrap-0.1.5/bubblewrap.c 2016-12-19 09:19:53.000000000 +0000
+++ bubblewrap-0.1.7/bubblewrap.c 2017-01-18 14:49:50.000000000 +0000
@@ -64,6 +64,7 @@
bool opt_unshare_cgroup = FALSE;
bool opt_unshare_cgroup_try = FALSE;
bool opt_needs_devpts = FALSE;
+bool opt_new_session = FALSE;
uid_t opt_sandbox_uid = -1;
gid_t opt_sandbox_gid = -1;
int opt_sync_fd = -1;
@@ -179,6 +180,8 @@
" --help Print this help\n"
" --version Print version\n"
" --args FD Parse nul-separated args from FD\n"
+ " --unshare-all Unshare every namespace we support by default\n"
+ " --share-net Retain the network namespace (can only combine with --unshare-all)\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"
@@ -213,6 +216,7 @@
" --seccomp FD Load and use seccomp rules from FD\n"
" --block-fd FD Block on FD until some data to read is available\n"
" --info-fd FD Write information about the running container to FD\n"
+ " --new-session Create a new terminal session\n"
);
exit (ecode);
}
@@ -221,12 +225,17 @@
block_sigchild (void)
{
sigset_t mask;
+ int status;
sigemptyset (&mask);
sigaddset (&mask, SIGCHLD);
if (sigprocmask (SIG_BLOCK, &mask, NULL) == -1)
die_with_error ("sigprocmask");
+
+ /* Reap any outstanding zombies that we may have inherited */
+ while (waitpid (-1, &status, WNOHANG) > 0)
+ ;
}
static void
@@ -259,6 +268,25 @@
return 0;
}
+static int
+propagate_exit_status (int status)
+{
+ if (WIFEXITED (status))
+ return WEXITSTATUS (status);
+
+ /* The process died of a signal, we can't really report that, but we
+ * can at least be bash-compatible. The bash manpage says:
+ * The return value of a simple command is its
+ * exit status, or 128+n if the command is
+ * terminated by signal n.
+ */
+ if (WIFSIGNALED (status))
+ return 128 + WTERMSIG (status);
+
+ /* Weird? */
+ return 255;
+}
+
/* This stays around for as long as the initial process in the app does
* and when that exits it exits, propagating the exit status. We do this
* by having pid 1 in the sandbox detect this exit and tell the monitor
@@ -266,7 +294,7 @@
* pid 1 via a signalfd for SIGCHLD, and exit with an error in this case.
* This is to catch e.g. problems during setup. */
static void
-monitor_child (int event_fd)
+monitor_child (int event_fd, pid_t child_pid)
{
int res;
uint64_t val;
@@ -277,6 +305,8 @@
int num_fds;
struct signalfd_siginfo fdsi;
int dont_close[] = { event_fd, -1 };
+ pid_t died_pid;
+ int died_status;
/* Close all extra fds in the monitoring process.
Any passed in fds have been passed on to the child anyway. */
@@ -318,16 +348,21 @@
exit ((int) val - 1);
}
+ /* We need to read the signal_fd, or it will keep polling as read,
+ * however we ignore the details as we get them from waitpid
+ * below anway */
s = read (signal_fd, &fdsi, sizeof (struct signalfd_siginfo));
if (s == -1 && errno != EINTR && errno != EAGAIN)
- {
- die_with_error ("read signalfd");
- }
- else if (s == sizeof (struct signalfd_siginfo))
- {
- if (fdsi.ssi_signo != SIGCHLD)
- die ("Read unexpected signal\n");
- exit (fdsi.ssi_status);
+ die_with_error ("read signalfd");
+
+ /* We may actually get several sigchld compressed into one
+ SIGCHLD, so we have to handle all of them. */
+ while ((died_pid = waitpid (-1, &died_status, WNOHANG)) > 0)
+ {
+ /* We may be getting sigchild from other children too. For instance if
+ someone created a child process, and then exec:ed bubblewrap. Ignore them */
+ if (died_pid == child_pid)
+ exit (propagate_exit_status (died_status));
}
}
}
@@ -340,7 +375,7 @@
* When there are no other processes in the sandbox the wait will return
* ECHILD, and we then exit pid 1 to clean up the sandbox. */
static int
-do_init (int event_fd, pid_t initial_pid)
+do_init (int event_fd, pid_t initial_pid, struct sock_fprog *seccomp_prog)
{
int initial_exit_status = 1;
LockFile *lock;
@@ -364,6 +399,10 @@
/* Keep fd open to hang on to lock */
}
+ if (seccomp_prog != NULL &&
+ prctl (PR_SET_SECCOMP, SECCOMP_MODE_FILTER, seccomp_prog) != 0)
+ die_with_error ("prctl(PR_SET_SECCOMP)");
+
while (TRUE)
{
pid_t child;
@@ -375,8 +414,7 @@
uint64_t val;
int res UNUSED;
- if (WIFEXITED (status))
- initial_exit_status = WEXITSTATUS (status);
+ initial_exit_status = propagate_exit_status (status);
val = initial_exit_status + 1;
res = write (event_fd, &val, 8);
@@ -439,6 +477,19 @@
return data[0].permitted != 0 || data[1].permitted != 0;
}
+static void
+drop_cap_bounding_set (void)
+{
+ unsigned long cap;
+
+ for (cap = 0; cap <= 63; cap++)
+ {
+ int res = prctl (PR_CAPBSET_DROP, cap, 0, 0, 0);
+ if (res == -1 && errno != EINVAL)
+ die_with_error ("Dropping capability %ld from bounds", cap);
+ }
+}
+
/* This acquires the privileges that the bwrap will need it to work.
* If bwrap is not setuid, then this does nothing, and it relies on
* unprivileged user namespaces to be used. This case is
@@ -487,6 +538,9 @@
if (new_fsuid != real_uid)
die ("Unable to set fsuid (was %d)", (int)new_fsuid);
+ /* We never need capabilies after execve(), so lets drop everything from the bounding set */
+ drop_cap_bounding_set ();
+
/* Keep only the required capabilities for setup */
set_required_caps ();
}
@@ -505,6 +559,10 @@
static void
switch_to_user_with_privs (void)
{
+ /* If we're in a new user namespace, we got back the bounding set, clear it again */
+ if (opt_unshare_user)
+ drop_cap_bounding_set ();
+
if (!is_privileged)
return;
@@ -1158,6 +1216,17 @@
argv += 1;
argc -= 1;
}
+ else if (strcmp (arg, "--unshare-all") == 0)
+ {
+ /* Keep this in order with the older (legacy) --unshare arguments,
+ * we use the --try variants of user and cgroup, since we want
+ * to support systems/kernels without support for those.
+ */
+ opt_unshare_user_try = opt_unshare_ipc = opt_unshare_pid =
+ opt_unshare_uts = opt_unshare_cgroup_try =
+ opt_unshare_net = TRUE;
+ }
+ /* Begin here the older individual --unshare variants */
else if (strcmp (arg, "--unshare-user") == 0)
{
opt_unshare_user = TRUE;
@@ -1190,6 +1259,12 @@
{
opt_unshare_cgroup_try = TRUE;
}
+ /* Begin here the newer --share variants */
+ else if (strcmp (arg, "--share-net") == 0)
+ {
+ opt_unshare_net = FALSE;
+ }
+ /* End --share variants, other arguments begin */
else if (strcmp (arg, "--chdir") == 0)
{
if (argc < 2)
@@ -1536,6 +1611,10 @@
argv += 1;
argc -= 1;
}
+ else if (strcmp (arg, "--new-session") == 0)
+ {
+ opt_new_session = TRUE;
+ }
else if (*arg == '-')
{
die ("Unknown option %s", arg);
@@ -1602,6 +1681,9 @@
struct stat sbuf;
uint64_t val;
int res UNUSED;
+ cleanup_free char *seccomp_data = NULL;
+ size_t seccomp_len;
+ struct sock_fprog seccomp_prog;
real_uid = getuid ();
real_gid = getgid ();
@@ -1792,7 +1874,7 @@
close (opt_info_fd);
}
- monitor_child (event_fd);
+ monitor_child (event_fd, pid);
exit (0); /* Should not be reached, but better safe... */
}
@@ -1979,10 +2061,6 @@
if (opt_seccomp_fd != -1)
{
- cleanup_free char *seccomp_data = NULL;
- size_t seccomp_len;
- struct sock_fprog prog;
-
seccomp_data = load_file_data (opt_seccomp_fd, &seccomp_len);
if (seccomp_data == NULL)
die_with_error ("Can't read seccomp data");
@@ -1990,13 +2068,10 @@
if (seccomp_len % 8 != 0)
die ("Invalid seccomp data, must be multiple of 8");
- prog.len = seccomp_len / 8;
- prog.filter = (struct sock_filter *) seccomp_data;
+ seccomp_prog.len = seccomp_len / 8;
+ seccomp_prog.filter = (struct sock_filter *) seccomp_data;
close (opt_seccomp_fd);
-
- if (prctl (PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog) != 0)
- die_with_error ("prctl(PR_SET_SECCOMP)");
}
umask (old_umask);
@@ -2024,6 +2099,13 @@
xsetenv ("PWD", new_cwd, 1);
free (old_cwd);
+ if (opt_new_session &&
+ setsid () == (pid_t) -1)
+ die_with_error ("setsid");
+
+ if (label_exec (opt_exec_label) == -1)
+ die_with_error ("label_exec %s", argv[0]);
+
__debug__ (("forking for child\n"));
if (opt_unshare_pid || lock_files != NULL || opt_sync_fd != -1)
@@ -2056,7 +2138,7 @@
fdwalk (proc_fd, close_extra_fds, dont_close);
}
- return do_init (event_fd, pid);
+ return do_init (event_fd, pid, seccomp_data != NULL ? &seccomp_prog : NULL);
}
}
@@ -2071,8 +2153,9 @@
/* We want sigchild in the child */
unblock_sigchild ();
- if (label_exec (opt_exec_label) == -1)
- die_with_error ("label_exec %s", argv[0]);
+ if (seccomp_data != NULL &&
+ prctl (PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &seccomp_prog) != 0)
+ die_with_error ("prctl(PR_SET_SECCOMP)");
if (execvp (argv[0], argv) == -1)
die_with_error ("execvp %s", argv[0]);
diff -Nru bubblewrap-0.1.5/bwrap.xml bubblewrap-0.1.7/bwrap.xml
--- bubblewrap-0.1.5/bwrap.xml 2016-12-19 09:19:53.000000000 +0000
+++ bubblewrap-0.1.7/bwrap.xml 2017-01-18 14:49:50.000000000 +0000
@@ -53,9 +53,10 @@
the namespace.
</para>
<para>
- By default, <command>bwrap</command> creates a new user namespace for the sandbox.
- Optionally it also sets up new ipc, pid, network and uts namespaces. The application
- in the sandbox can be made to run with a different UID and GID.
+ By default, <command>bwrap</command> creates a new mount namespace for the sandbox.
+ Optionally it also sets up new user, ipc, pid, network and uts namespaces (but note the
+ user namespace is required if bwrap is not installed setuid root).
+ The application in the sandbox can be made to run with a different UID and GID.
</para>
<para>
If needed (e.g. when using a PID namespace) <command>bwrap</command>
@@ -263,6 +264,18 @@
Write information in JSON format about the sandbox to FD.
</para></listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--new-session</option></term>
+ <listitem><para>
+ Create a new terminal session for the sandbox (calls setsid()). This
+ disconnects the sandbox from the controlling terminal which means
+ the sandbox can't for instance inject input into the terminal.
+ </para><para>
+ Note: In a general sandbox, if you don't use --new-session, it is
+ recommended to use seccomp to disallow the TIOCSTI ioctl, otherwise
+ the application can feed keyboard input to the terminal.
+ </para></listitem>
+ </varlistentry>
</variablelist>
</refsect1>
diff -Nru bubblewrap-0.1.5/configure.ac bubblewrap-0.1.7/configure.ac
--- bubblewrap-0.1.5/configure.ac 2016-12-19 09:19:53.000000000 +0000
+++ bubblewrap-0.1.7/configure.ac 2017-01-18 14:49:50.000000000 +0000
@@ -1,5 +1,5 @@
AC_PREREQ([2.63])
-AC_INIT([bubblewrap], [0.1.5], [atomic-devel@projectatomic.io])
+AC_INIT([bubblewrap], [0.1.7], [atomic-devel@projectatomic.io])
AC_CONFIG_HEADER([config.h])
AC_CONFIG_MACRO_DIR([m4])
AC_CONFIG_AUX_DIR([build-aux])
diff -Nru bubblewrap-0.1.5/debian/bubblewrap.examples bubblewrap-0.1.7/debian/bubblewrap.examples
--- bubblewrap-0.1.5/debian/bubblewrap.examples 1970-01-01 01:00:00.000000000 +0100
+++ bubblewrap-0.1.7/debian/bubblewrap.examples 2017-01-19 14:33:46.000000000 +0000
@@ -0,0 +1,3 @@
+demos/bubblewrap-shell.sh
+demos/flatpak-run.sh
+demos/flatpak.bpf
diff -Nru bubblewrap-0.1.5/debian/changelog bubblewrap-0.1.7/debian/changelog
--- bubblewrap-0.1.5/debian/changelog 2017-01-09 18:09:54.000000000 +0000
+++ bubblewrap-0.1.7/debian/changelog 2017-01-19 14:33:46.000000000 +0000
@@ -1,3 +1,46 @@
+bubblewrap (0.1.7-1) unstable; urgency=medium
+
+ * New upstream release
+ - effectively the same as 0.1.6-2
+ - drop all patches
+
+ -- Simon McVittie <smcv@debian.org> Thu, 19 Jan 2017 14:33:46 +0000
+
+bubblewrap (0.1.6-2) unstable; urgency=medium
+
+ * d/p/Make-the-call-to-setsid-optional-with-new-session.patch:
+ Add patch from upstream to make the setsid() that addresses
+ CVE-2017-5226 optional, because it breaks interactive shells.
+ Users of bubblewrap to confine untrusted programs should either
+ add --new-session to the bwrap command line, or prevent the
+ TIOCSTI ioctl with a seccomp filter instead (as Flatpak does).
+ - d/control: add Breaks on versions of Flatpak that did not
+ load the necessary seccomp filter to prevent CVE-2017-5226
+ * d/p/demos-bubblewrap-shell.sh-Unshare-all-namespaces.patch:
+ Add patch from upstream to improve example code
+ * d/p/Call-setsid-and-setexeccon-befor-forking-the-init-monitor.patch,
+ d/p/Install-seccomp-filter-at-the-very-end.patch:
+ Add patches from upstream to re-order initialization. This means
+ the seccomp filter is no longer required to account for syscalls that
+ are made by bwrap itself.
+ * d/p/Add-unshare-all-and-share-net.patch:
+ Add patch from upstream introducing new command line options
+ --unshare-all and --share-net, for a more whitelist-based approach
+ to sharing namespaces with the parent.
+
+ -- Simon McVittie <smcv@debian.org> Wed, 18 Jan 2017 00:56:19 +0000
+
+bubblewrap (0.1.6-1) unstable; urgency=medium
+
+ * New upstream release
+ - drop the only patch, applied upstream
+ * debian/patches: update to upstream master for additional fixes
+ to SIGCHLD handling and documentation, and improved hardening
+ against being able to obtain capabilities
+ * debian/bubblewrap.examples: install upstream examples
+
+ -- Simon McVittie <smcv@debian.org> Sat, 14 Jan 2017 22:18:09 +0000
+
bubblewrap (0.1.5-2) unstable; urgency=high
* d/p/Call-setsid-before-executing-sandboxed-code-CVE-2017-5226.patch:
diff -Nru bubblewrap-0.1.5/debian/control bubblewrap-0.1.7/debian/control
--- bubblewrap-0.1.5/debian/control 2017-01-09 18:09:54.000000000 +0000
+++ bubblewrap-0.1.7/debian/control 2017-01-19 14:33:46.000000000 +0000
@@ -25,6 +25,8 @@
Depends:
${misc:Depends},
${shlibs:Depends},
+Breaks:
+ flatpak (<< 0.8.0-2~),
Description: setuid wrapper for unprivileged chroot and namespace manipulation
Core execution engine for unprivileged containers that works as a setuid
binary on kernels without user namespaces.
diff -Nru bubblewrap-0.1.5/debian/patches/Call-setsid-before-executing-sandboxed-code-CVE-2017-5226.patch bubblewrap-0.1.7/debian/patches/Call-setsid-before-executing-sandboxed-code-CVE-2017-5226.patch
--- bubblewrap-0.1.5/debian/patches/Call-setsid-before-executing-sandboxed-code-CVE-2017-5226.patch 2017-01-09 18:09:54.000000000 +0000
+++ bubblewrap-0.1.7/debian/patches/Call-setsid-before-executing-sandboxed-code-CVE-2017-5226.patch 1970-01-01 01:00:00.000000000 +0100
@@ -1,29 +0,0 @@
-From: Simon McVittie <smcv@debian.org>
-Date: Mon, 9 Jan 2017 17:46:07 +0000
-Subject: Call setsid() before executing sandboxed code (CVE-2017-5226)
-
-This prevents the sandboxed code from getting a controlling tty,
-which in turn prevents it from accessing the TIOCSTI ioctl and hence
-faking terminal input.
-
-Bug: https://github.com/projectatomic/bubblewrap/issues/142
-Bug-Debian: https://bugs.debian.org/850702
-Forwarded: https://github.com/projectatomic/bubblewrap/issues/143
----
- bubblewrap.c | 3 +++
- 1 file changed, 3 insertions(+)
-
-diff --git a/bubblewrap.c b/bubblewrap.c
-index 6e04459..4b5b8e6 100644
---- a/bubblewrap.c
-+++ b/bubblewrap.c
-@@ -2071,6 +2071,9 @@ main (int argc,
- /* We want sigchild in the child */
- unblock_sigchild ();
-
-+ if (setsid () == (pid_t) -1)
-+ die_with_error ("setsid");
-+
- if (label_exec (opt_exec_label) == -1)
- die_with_error ("label_exec %s", argv[0]);
-
diff -Nru bubblewrap-0.1.5/debian/patches/series bubblewrap-0.1.7/debian/patches/series
--- bubblewrap-0.1.5/debian/patches/series 2017-01-09 18:09:54.000000000 +0000
+++ bubblewrap-0.1.7/debian/patches/series 1970-01-01 01:00:00.000000000 +0100
@@ -1 +0,0 @@
-Call-setsid-before-executing-sandboxed-code-CVE-2017-5226.patch
diff -Nru bubblewrap-0.1.5/demos/bubblewrap-shell.sh bubblewrap-0.1.7/demos/bubblewrap-shell.sh
--- bubblewrap-0.1.5/demos/bubblewrap-shell.sh 2016-12-19 09:19:53.000000000 +0000
+++ bubblewrap-0.1.7/demos/bubblewrap-shell.sh 2017-01-18 14:49:50.000000000 +0000
@@ -1,8 +1,18 @@
#!/usr/bin/env bash
-# Use bubblewrap to run /bin/sh in the host's rootfs.
+# Use bubblewrap to run /bin/sh reusing the host OS binaries (/usr), but with
+# separate /tmp, /home, /var, /run, and /etc. For /etc we just inherit the
+# host's resolv.conf, and set up "stub" passwd/group files. Not sharing
+# /home for example is intentional. If you wanted to, you could design
+# a bwrap-using program that shared individual parts of /home, perhaps
+# public content.
+#
+# Another way to build on this example is to remove --share-net to disable
+# networking.
set -euo pipefail
(exec bwrap --ro-bind /usr /usr \
--dir /tmp \
+ --dir /var \
+ --symlink ../tmp var/tmp \
--proc /proc \
--dev /dev \
--ro-bind /etc/resolv.conf /etc/resolv.conf \
@@ -11,9 +21,11 @@
--symlink usr/bin /bin \
--symlink usr/sbin /sbin \
--chdir / \
- --unshare-pid \
+ --unshare-all \
+ --share-net \
--dir /run/user/$(id -u) \
--setenv XDG_RUNTIME_DIR "/run/user/`id -u`" \
+ --setenv PS1 "bwrap-demo$ " \
--file 11 /etc/passwd \
--file 12 /etc/group \
/bin/sh) \
diff -Nru bubblewrap-0.1.5/.redhat-ci.yml bubblewrap-0.1.7/.redhat-ci.yml
--- bubblewrap-0.1.5/.redhat-ci.yml 2016-12-19 09:19:53.000000000 +0000
+++ bubblewrap-0.1.7/.redhat-ci.yml 2017-01-18 14:49:50.000000000 +0000
@@ -52,6 +52,7 @@
- docbook-style-xsl
- clang
- libubsan
+ - libasan
env:
CC: 'clang'
@@ -61,8 +62,8 @@
inherit: true
-context: ubsan-f25
+context: f25-asan-ubsan
env:
CC: 'gcc'
- CFLAGS: '-g -Og -fsanitize=undefined'
+ CFLAGS: '-g -Og -fsanitize=undefined -fsanitize=address'
Reply to: