diff options
| -rw-r--r-- | README.md | 39 | ||||
| -rw-r--r-- | SECURITY.md | 39 | ||||
| -rw-r--r-- | bind-mount.c | 2 | ||||
| -rw-r--r-- | bubblewrap.c | 84 | ||||
| -rw-r--r-- | bwrap.xml | 12 | ||||
| -rw-r--r-- | configure.ac | 2 | ||||
| -rw-r--r-- | meson.build | 7 | ||||
| -rw-r--r-- | network.c | 8 | ||||
| -rw-r--r-- | utils.c | 12 | ||||
| -rw-r--r-- | utils.h | 2 |
10 files changed, 136 insertions, 71 deletions
@@ -35,8 +35,8 @@ The original bubblewrap code existed before user namespaces - it inherits code f which in turn distantly derives from [linux-user-chroot](https://git.gnome.org/browse/linux-user-chroot). -Security --------- +System security +--------------- The maintainers of this tool believe that it does not, even when used in combination with typical software installed on that distribution, @@ -47,6 +47,27 @@ In particular, bubblewrap uses `PR_SET_NO_NEW_PRIVS` to turn off setuid binaries, which is the [traditional way](https://en.wikipedia.org/wiki/Chroot#Limitations) to get out of things like chroots. +Sandbox security +---------------- + +bubblewrap is a tool for constructing sandbox environments. +bubblewrap is not a complete, ready-made sandbox with a specific security +policy. + +Some of bubblewrap's use-cases want a security boundary between the sandbox +and the real system; other use-cases want the ability to change the layout of +the filesystem for processes inside the sandbox, but do not aim to be a +security boundary. +As a result, the level of protection between the sandboxed processes and +the host system is entirely determined by the arguments passed to +bubblewrap. + +Whatever program constructs the command-line arguments for bubblewrap +(often a larger framework like Flatpak, libgnome-desktop, sandwine +or an ad-hoc script) is responsible for defining its own security model, +and choosing appropriate bubblewrap command-line arguments to implement +that security model. + Users ----- @@ -101,7 +122,14 @@ source code, but here's a trimmed down version which runs a new shell reusing the host's `/usr`. ``` -bwrap --ro-bind /usr /usr --symlink usr/lib64 /lib64 --proc /proc --dev /dev --unshare-pid bash +bwrap \ + --ro-bind /usr /usr \ + --symlink usr/lib64 /lib64 \ + --proc /proc \ + --dev /dev \ + --unshare-pid \ + --new-session \ + bash ``` This is an incomplete example, but useful for purposes of @@ -138,6 +166,11 @@ UTS namespace ([CLONE_NEWUTS](http://linux.die.net/man/2/clone)): The sandbox wi Seccomp filters: You can pass in seccomp filters that limit which syscalls can be done in the sandbox. For more information, see [Seccomp](https://en.wikipedia.org/wiki/Seccomp). +If you are not filtering out `TIOCSTI` commands using seccomp filters, +argument `--new-session` is needed to protect against out-of-sandbox +command execution +(see [CVE-2017-5226](https://github.com/containers/bubblewrap/issues/142)). + Related project comparison: Firejail ------------------------------------ diff --git a/SECURITY.md b/SECURITY.md index d914d4a..0ddfc6c 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,3 +1,42 @@ ## Security and Disclosure Information Policy for the bubblewrap Project The bubblewrap Project follows the [Security and Disclosure Information Policy](https://github.com/containers/common/blob/HEAD/SECURITY.md) for the Containers Projects. + +### System security + +If bubblewrap is setuid root, then the goal is that it does not allow +a malicious local user to do anything that would not have been possible +on a kernel that allows unprivileged users to create new user namespaces. +For example, [CVE-2020-5291](https://github.com/containers/bubblewrap/security/advisories/GHSA-j2qp-rvxj-43vj) +was treated as a security vulnerability in bubblewrap. + +If bubblewrap is not setuid root, then it is not a security boundary +between the user and the OS, because anything bubblewrap could do, a +malicious user could equally well do by writing their own tool equivalent +to bubblewrap. + +### Sandbox security + +bubblewrap is a toolkit for constructing sandbox environments. +bubblewrap is not a complete, ready-made sandbox with a specific security +policy. + +Some of bubblewrap's use-cases want a security boundary between the sandbox +and the real system; other use-cases want the ability to change the layout of +the filesystem for processes inside the sandbox, but do not aim to be a +security boundary. +As a result, the level of protection between the sandboxed processes and +the host system is entirely determined by the arguments passed to +bubblewrap. + +Whatever program constructs the command-line arguments for bubblewrap +(often a larger framework like Flatpak, libgnome-desktop, sandwine +or an ad-hoc script) is responsible for defining its own security model, +and choosing appropriate bubblewrap command-line arguments to implement +that security model. + +For example, +[CVE-2017-5226](https://github.com/flatpak/flatpak/security/advisories/GHSA-7gfv-rvfx-h87x) +(in which a Flatpak app could send input to a parent terminal using the +`TIOCSTI` ioctl) is considered to be a Flatpak vulnerability, not a +bubblewrap vulnerability. diff --git a/bind-mount.c b/bind-mount.c index 488d85a..57b4236 100644 --- a/bind-mount.c +++ b/bind-mount.c @@ -237,7 +237,7 @@ parse_mountinfo (int proc_fd, MountInfo *end_tab; int n_mounts; char *line; - int i; + unsigned int i; int max_id; unsigned int n_lines; int root; diff --git a/bubblewrap.c b/bubblewrap.c index 8322ea0..de06305 100644 --- a/bubblewrap.c +++ b/bubblewrap.c @@ -72,35 +72,35 @@ static const char *opt_exec_label = NULL; static const char *opt_file_label = NULL; static bool opt_as_pid_1; -const char *opt_chdir_path = NULL; -bool opt_assert_userns_disabled = FALSE; -bool opt_disable_userns = FALSE; -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; -bool opt_unshare_uts = FALSE; -bool opt_unshare_cgroup = FALSE; -bool opt_unshare_cgroup_try = FALSE; -bool opt_needs_devpts = FALSE; -bool opt_new_session = FALSE; -bool opt_die_with_parent = FALSE; -uid_t opt_sandbox_uid = -1; -gid_t opt_sandbox_gid = -1; -int opt_sync_fd = -1; -int opt_block_fd = -1; -int opt_userns_block_fd = -1; -int opt_info_fd = -1; -int opt_json_status_fd = -1; -int opt_seccomp_fd = -1; -const char *opt_sandbox_hostname = NULL; -char *opt_args_data = NULL; /* owned */ -int opt_userns_fd = -1; -int opt_userns2_fd = -1; -int opt_pidns_fd = -1; -int next_perms = -1; -size_t next_size_arg = 0; +static const char *opt_chdir_path = NULL; +static bool opt_assert_userns_disabled = FALSE; +static bool opt_disable_userns = FALSE; +static bool opt_unshare_user = FALSE; +static bool opt_unshare_user_try = FALSE; +static bool opt_unshare_pid = FALSE; +static bool opt_unshare_ipc = FALSE; +static bool opt_unshare_net = FALSE; +static bool opt_unshare_uts = FALSE; +static bool opt_unshare_cgroup = FALSE; +static bool opt_unshare_cgroup_try = FALSE; +static bool opt_needs_devpts = FALSE; +static bool opt_new_session = FALSE; +static bool opt_die_with_parent = FALSE; +static uid_t opt_sandbox_uid = -1; +static gid_t opt_sandbox_gid = -1; +static int opt_sync_fd = -1; +static int opt_block_fd = -1; +static int opt_userns_block_fd = -1; +static int opt_info_fd = -1; +static int opt_json_status_fd = -1; +static int opt_seccomp_fd = -1; +static const char *opt_sandbox_hostname = NULL; +static char *opt_args_data = NULL; /* owned */ +static int opt_userns_fd = -1; +static int opt_userns2_fd = -1; +static int opt_pidns_fd = -1; +static int next_perms = -1; +static size_t next_size_arg = 0; #define CAP_TO_MASK_0(x) (1L << ((x) & 31)) #define CAP_TO_MASK_1(x) CAP_TO_MASK_0(x - 32) @@ -496,7 +496,7 @@ monitor_child (int event_fd, pid_t child_pid, int setup_finished_fd) int num_fds; struct signalfd_siginfo fdsi; int dont_close[] = {-1, -1, -1, -1}; - int j = 0; + unsigned int j = 0; int exitc; pid_t died_pid; int died_status; @@ -965,7 +965,7 @@ write_uid_gid_map (uid_t sandbox_uid, cleanup_free char *gid_map = NULL; cleanup_free char *dir = NULL; cleanup_fd int dir_fd = -1; - uid_t old_fsuid = -1; + uid_t old_fsuid = (uid_t)-1; if (pid == -1) dir = xstrdup ("self"); @@ -1014,7 +1014,7 @@ write_uid_gid_map (uid_t sandbox_uid, if (is_privileged) { setfsuid (old_fsuid); - if (setfsuid (-1) != real_uid) + if ((uid_t) setfsuid (-1) != real_uid) die ("Unable to re-set fsuid"); } } @@ -1065,7 +1065,7 @@ privileged_op (int privileged_op_socket, if (arg2 != NULL) strcpy ((char *) buffer + arg2_offset, arg2); - if (write (privileged_op_socket, buffer, buffer_size) != buffer_size) + if (write (privileged_op_socket, buffer, buffer_size) != (ssize_t)buffer_size) die ("Can't write to privileged_op_socket"); if (read (privileged_op_socket, buffer, 1) != 1) @@ -1182,7 +1182,7 @@ setup_newroot (bool unshare_pid, cleanup_free char *source = NULL; cleanup_free char *dest = NULL; int source_mode = 0; - int i; + unsigned int i; if (op->source && op->type != SETUP_MAKE_SYMLINK) @@ -1207,12 +1207,12 @@ setup_newroot (bool unshare_pid, * inaccessible by that group. */ if (op->perms >= 0 && (op->perms & 0070) == 0) - parent_mode &= ~0050; + parent_mode &= ~0050U; /* The same, but for users other than the owner and group. */ if (op->perms >= 0 && (op->perms & 0007) == 0) - parent_mode &= ~0005; + parent_mode &= ~0005U; dest = get_newroot_path (op->dest); if (mkdir_with_parents (dest, parent_mode, FALSE) != 0) @@ -1593,7 +1593,7 @@ read_priv_sec_op (int read_socket, if (rec_len == 0) exit (1); /* Privileged helper died and printed error, so exit silently */ - if (rec_len < sizeof (PrivSepOp)) + if ((size_t)rec_len < sizeof (PrivSepOp)) die ("Invalid size %zd from unprivileged helper", rec_len); /* Guarantee zero termination of any strings */ @@ -1647,7 +1647,7 @@ parse_args_recurse (int *argcp, * I picked 9000 because the Internet told me to and it was hard to * resist. */ - static const uint32_t MAX_ARGS = 9000; + static const int32_t MAX_ARGS = 9000; if (*total_parsed_argc_p > MAX_ARGS) die ("Exceeded maximum number of arguments %u", MAX_ARGS); @@ -2300,7 +2300,7 @@ parse_args_recurse (int *argcp, if (argc < 2) die ("--uid takes an argument"); - if (opt_sandbox_uid != -1) + if (opt_sandbox_uid != (uid_t)-1) warn_only_last_option ("--uid"); the_uid = strtol (argv[1], &endptr, 10); @@ -2320,7 +2320,7 @@ parse_args_recurse (int *argcp, if (argc < 2) die ("--gid takes an argument"); - if (opt_sandbox_gid != -1) + if (opt_sandbox_gid != (gid_t)-1) warn_only_last_option ("--gid"); the_gid = strtol (argv[1], &endptr, 10); @@ -2768,9 +2768,9 @@ main (int argc, __debug__ (("Creating root mount point\n")); - if (opt_sandbox_uid == -1) + if (opt_sandbox_uid == (uid_t)-1) opt_sandbox_uid = real_uid; - if (opt_sandbox_gid == -1) + if (opt_sandbox_gid == (gid_t)-1) opt_sandbox_gid = real_gid; if (!opt_unshare_user && opt_userns_fd == -1 && opt_sandbox_uid != real_uid) @@ -6,7 +6,7 @@ <refentryinfo> <title>bwrap</title> - <productname>Project Atomic</productname> + <productname>Containers</productname> <authorgroup> <author> <contrib>Developer</contrib> @@ -42,7 +42,8 @@ <refsect1><title>Description</title> <para> - <command>bwrap</command> is a privileged helper for container setup. You + <command>bwrap</command> is a unprivileged low-level sandboxing tool + (optionally setuid on older distributions). You are unlikely to use it directly from the commandline, although that is possible. </para> <para> @@ -463,7 +464,9 @@ </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. + the application can feed keyboard input to the terminal + which can e.g. lead to out-of-sandbox command execution + (see CVE-2017-5226). </para></listitem> </varlistentry> <varlistentry> @@ -484,7 +487,8 @@ <varlistentry> <term><option>--cap-add <arg choice="plain">CAP</arg></option></term> <listitem><para> - Add the specified capability when running as privileged user. It accepts + Add the specified capability <arg choice="plain">CAP</arg>, e.g. + CAP_DAC_READ_SEARCH, when running as privileged user. It accepts the special value ALL to add all the permitted caps. </para></listitem> </varlistentry> diff --git a/configure.ac b/configure.ac index 7934449..7059a56 100644 --- a/configure.ac +++ b/configure.ac @@ -1,5 +1,5 @@ AC_PREREQ([2.63]) -AC_INIT([bubblewrap], [0.7.0], [atomic-devel@projectatomic.io]) +AC_INIT([bubblewrap], [0.8.0], [atomic-devel@projectatomic.io]) AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/meson.build b/meson.build index 758509b..0c26aa5 100644 --- a/meson.build +++ b/meson.build @@ -1,7 +1,7 @@ project( 'bubblewrap', 'c', - version : '0.7.0', + version : '0.8.0', meson_version : '>=0.49.0', default_options : [ 'warning_level=2', @@ -37,11 +37,6 @@ add_project_arguments( '-Werror=switch-default', '-Wswitch-enum', - # Meson warning_level=2 would do this, but we are not fully - # signedness-safe yet - '-Wno-sign-compare', - '-Wno-error=sign-compare', - # Deliberately not warning about these, ability to zero-initialize # a struct is a feature '-Wno-missing-field-initializers', @@ -62,8 +62,8 @@ rtnl_send_request (int rtnl_fd, } static int -rtnl_read_reply (int rtnl_fd, - int seq_nr) +rtnl_read_reply (int rtnl_fd, + unsigned int seq_nr) { char buffer[1024]; ssize_t received; @@ -80,7 +80,7 @@ rtnl_read_reply (int rtnl_fd, { if (rheader->nlmsg_seq != seq_nr) return -1; - if (rheader->nlmsg_pid != getpid ()) + if ((pid_t)rheader->nlmsg_pid != getpid ()) return -1; if (rheader->nlmsg_type == NLMSG_ERROR) { @@ -130,7 +130,7 @@ rtnl_setup_request (char *buffer, header->nlmsg_seq = counter++; header->nlmsg_pid = getpid (); - return (struct nlmsghdr *) header; + return header; } void @@ -568,7 +568,6 @@ load_file_data (int fd, ssize_t data_read; ssize_t data_len; ssize_t res; - int errsv; data_read = 0; data_len = 4080; @@ -587,12 +586,7 @@ load_file_data (int fd, while (res < 0 && errno == EINTR); if (res < 0) - { - errsv = errno; - close (fd); - errno = errsv; - return NULL; - } + return NULL; data_read += res; } @@ -672,7 +666,7 @@ ensure_dir (const char *path, /* Sets errno on error (!= 0) */ int mkdir_with_parents (const char *pathname, - int mode, + mode_t mode, bool create_last) { cleanup_free char *fn = NULL; @@ -815,7 +809,7 @@ readlink_malloc (const char *pathname) if (n < 0) return NULL; } - while (size - 2 < n); + while (size - 2 < (size_t)n); value[n] = 0; return steal_pointer (&value); @@ -115,7 +115,7 @@ int ensure_dir (const char *path, mode_t mode); int get_file_mode (const char *pathname); int mkdir_with_parents (const char *pathname, - int mode, + mode_t mode, bool create_last); void create_pid_socketpair (int sockets[2]); void send_pid_on_socket (int socket); |