[MIR] bubblewrap

Status changed to 'Confirmed' because the bug affects multiple users.

An additional sandbox is probably rather a security win than a security risk. It would be great if that could be MIRed before feature freeze.

GDK-pixbuf, Evince and other "thumbnailer users" seem to depend on that: http://www.hadess.net/2017/07/security-for-security-gods-sandboxing.html

To quote Bastien Nocera: " For GNOME 3.26 (and today in git master), the thumbnailer stall will be doubly bolted by a Bubblewrap sandbox and a seccomp blacklist.

This closes a whole vector of attack for the GNOME Desktop,..."

I am still feeling uncomfortable shipping some crucial GNOME components like Nautilus more insecure than upstream.

An it is not just a matter of having bubblewrap in main or not. Not a matter of the default and anybody who wishes the default upstream security level could rectify this by “sudo apt install bubblewrap“. Because now sandboxing has to be turned off at build time and installing bubblewrap afterwards will not help anything.

And it is not that the risks of shipping without sandbox are just theoretical: Ubuntu got some flak for this thumbnailing hole: https://csorianognome.wordpress.com/2017/07/20/clarification-on-a-security-flaw-on-a-thumbnailer/

Adding the Ubuntu release team a to get this in as a FFe as soon as possible. Disabling security features doesn't sound like worthwile Ubuntu modifications.

> dh_auto_test runs the build tests but they appear to be set as SKIP upstream.

They are automatically skipped if you are building in an environment where the simplest possible use of bwrap (bind-mounting / over / and running /bin/true) doesn't work, which unfortunately includes all official Debian and Ubuntu buildds.

On Debian, one factor is that Debian's kernel doesn't allow creation of unprivileged user namespaces by default, which means bwrap needs to be setuid to work, and in a buildd environment the just-built bwrap is not yet setuid. I'm not sure whether this applies on Ubuntu.

Another factor is that we can't typically run bwrap inside a container, and I'm not sure that running it inside a chroot works either.

In general the bubblewrap/flatpak stack will have to be tested via autopkgtest or equivalent - expecting container stuff to work correctly at build-time is not really very feasible.

The current state on the corresponding Trello card (https://trello.com/c/ZMkHCrQY/20-bubblewrap-mir) was that didrocks was going to do an initial security review in November. But then Didier took some time off (until end of year?) thus probably didn't have time to do the review yet.

The package looks good to me. I have some questions though:
There are some demos content in debian/dists.
Those are not shipped by any package and not used in autopkgtests (no reference found in debian/tests). So why those are shipped? I'm not very found of finding a binary as well in this one: debian/dist/demos/flatpak.bpf without any description of what it serves for and such.

I'm deferring the security review to the security team (unsure why amano thought I would do a security review, I'm a MIR team member, not a security expert) as obviously, this package needs a deep look into.

Please try to review the bionic version instead of the artful version ;)

Those files were moved to a patch and the patch should be able to be dropped in the next upstream release:
https://salsa.debian.org/debian/bubblewrap/raw/debian/master/debian/patches/dist/Add-missing-files-from-upstream-git.patch

Those files are shipped in the bubblewrap binary package in
/usr/share/doc/bubblewrap/examples/

Is that a problem? Do we need to split the examples into a separate package that we wouldn't need to be install by default? (They don't take up much space.)

Here's a bug for the binary BPF file:
https://github.com/projectatomic/bubblewrap/issues/250

ok, my deb-src were still on artful for some reason… I woudl split them in a separate package as they don't need to be installed by default, but it's up to you.
Thanks for filing the bpf big and explaining the changes that happened in bionic!

> I woudl split them in a separate package as they don't need to be installed by default, but it's up to you.

Sorry, I am not willing to put this package through the Debian NEW queue just to split out a few KB of examples into a separate binary package, and I suspect the ftp team would take a dim view of this: the size of the archive metadata required to describe that binary package would the same order of magnitude as the size of the package itself. If they are considered to be a serious problem for some reason, then I'll delete them altogether, and just patch in the README.

The demos are re-included via debian/dist/ (older versions) or debian/patches/dist/ (newer) because I was looking at packaging a git snapshot in experimental, and happened to notice that they are shipped upstream but were accidentally not included in tarballs. I also contributed a patch upstream to include them in `make dist`, and that patch has been merged.

I believe flatpak.bpf is a snapshot of the seccomp filter that was set up by some random older version of Flatpak, and accompanies flatpak-run.sh to make flatpak-run.sh more closely resemble what Flatpak actually does. bubblewrap takes seccomp filters as input in binary form rather than building them using libseccomp, because bubblewrap is (initially) highly privileged, so library dependencies are minimized to reduce attack surface; instead, the unprivileged Flatpak binary links libseccomp and constructs the filter itself.

(cleaning up ~ubuntu-release bugs)

I've seen other MIR bugs assigned to ~ubuntu-security instead of ~canonical-security - reassigning in case this helps move this MIR forward. Please review.

Otherwise, at this point there's nothing for the release team to review. If the MIR is approved with time to go before Bionic, we can look to re-enabling bubblewrap in gnome-desktop (and anything else?). Please re-subscribe in that event.

Nautilus 3.30 now requires bubblewrap for its thumbnail feature. I mean we could disable it if we had to, but that doesn't seem like a great idea.

Ubuntu 18.10 will still use Nautilus 3.26, but we intend to update Nautilus for Ubuntu 19.04.

I'm coming up to speed on this issue now and have discussed this with Jamie Bennett, the security team and various stakeholders to unblock this MIR. The security team will prioritize this MIR for 18.10. Assuming it passing review, I would encourage the Ubuntu Desktop team to SRU this back to at least 18.04 LTS so users can benefit from the sandboxing feature.

FYI, while this is currently assigned to Seth, I do want to note that bubblewrap is setuid so it is going to require extra scrutiny (incidentally this was not called out in this bug's description). Regardless of the outcome of the bubblewrap review, the sandboxing feature is highly desirable so we'll be sure to outline a path forward so these thumbnailers can run in a restricted environment.

> bubblewrap is setuid

Doesn't Ubuntu have unprivileged userns available, just like e.g. Fedora? If so, then bwrap isn't setuid, and offers no more attack surface than the kernel does to every process (that doesn't have access to CLONE_NEWUSER denied via e.g. seccomp, as e.g. Docker does by default for its containers).

To clarify I'm one of the upstream bubblewrap maintainers, if you have any concerns don't hesitate to file an issue upstream, but we can chat here too.

Ubuntu does enable unprivileged userns by default (at least on desktop installs?), but there's at least one exception to watch out for: the lightdm "guest session" option applies an AppArmor policy that allows CLONE_NEWUSER but denies any use of the resulting capabilities; see also https://bugzilla.mozilla.org/show_bug.cgi?id=1434528 where we ran into that with Firefox. There's an exception for Chromium's sandbox, so in principle that could also be done for bubblewrap.

- 1 closed CVE in our CVE database CVE-2017-5226 (LP #1657357)
  - Fixed in a timely fashion but by updating to a version which is not ideal
- Provides ability to launch other applications within a sandbox via (user)
  namespaces and bind mounts etc.
- Build-Depends: libcap-dev, libselinux1-dev
- Does not daemonize
- No use of udev
- No pre/post inst/rm scripts
- No initscripts / systemd unit files
- No DBus services
- 1 setuid file:
  rwsr-xr-x root/root 59496 2018-07-12 18:33 ./usr/bin/bwrap
  - Note: the Ubuntu kernel supports unprivileged user namespaces so there is
    no reason for bwrap to be setuid - I have tested this also without setuid
    and it works as expected. As such I strongly suggest we patch out the part
    of the debian packaging which makes this setuid.
- binaries added to the PATH
  rwsr-xr-x root/root 59496 2018-07-12 18:33 ./usr/bin/bwrap
- No sudo fragments
- No udev rules
- System tests exist but are not run as part of package build
  - no unit tests
- No cronjobs
- Clean build logs - no warnings during build

- Subprocesses are spawned as that is the core functionality of bubblewrap
- Memory management looks good, no obvious issues and all memory allocations
  are checked and appropriately handled with good defensive programming.
- Does not directly use any environment variables but does use setenv to allow
  caller to set vars in child process environment
- Uses privileged operations to setup namespace then drops privileges before
  executing child process. Correctly detects when running as setuid root and
  correctly drops down to the saved user-id before executing the resulting
  child process so should not be a problem when using new user namespace even
  though is setuid root.
- No cryptography
- No network connections
- Uses temporary files to be able to pass in files to the subprocesses
  namespace - uses umask(0) and mkstemp() so this looks to be secure
- No WebKit
- No JavaScript
- No PolicyKit
- Clean cppcheck
  - 1 false positive error for a memory leak - cppcheck is confused due to the
    use of gcc's cleanup attribute to automatically free the memory when it
    goes out of scope)
- scan-build (6.0):
  - 4 warnings:
    - 1 false positive memory leak (confusion due to gcc cleanup attribute)
    - 1 false positive dead assignment (confusion due to gcc cleanup attribute)
    - 2 warnings about passing possible NULL pointers to functions which expect
      non-NULL values
      - 1 for creat(), 1 for symlink()
      - both should be impossible since current call sites never provide the
        preconditions to allow these to occur
      - so both are false positives
        - would be useful to have some assertions to convey this impossible
          state

So the only real concerns I have is that the system-level tests do not seem to
be integrated with the package build process in any way, and that it is setuid
root which does not seem necessary as our kernels enable unprivileged user
namespaces. So I am happy to ACK this from the security team if both of these
can be investigated and resolved (ie. add the tests to the build process and
drop the adding of setuid permission during packaging).

It would...

Alex, the tests aren't run during the build because we can't test this kind of functionality in that environment. Please see comment 4.

The tests are run as autopkgtests with the isolation-machine configuration.

On Fri, Sep 14, 2018 at 01:46:05PM -0000, Jeremy Bicha wrote:
> Alex, the tests aren't run during the build because we can't test this
> kind of functionality in that environment. Please see comment 4.
>
> The tests are run as autopkgtests with the isolation-machine
> configuration.

Yes, that looks quite appropriate for this situation and should give us
quite good guarantees that bubblewrap continues to work after its
dependencies are updated - even if we could have build-time tests,
autopkgtests would be a great idea too.

See: http://autopkgtest.ubuntu.com/packages/bubblewrap

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

Ah ok thanks - sorry I somehow missed those details in comment 4 - cheers.

setuid has been dropped now:

https://launchpad.net/ubuntu/+source/bubblewrap/0.3.1-1ubuntu1

I promoted it but then noticed that the bug status was not "fix commited", security & MIR team seemed fine though so I guess it's only an admin change for the bug at this point?

Override component to main
bubblewrap 0.3.1-1ubuntu2 in cosmic: universe/misc -> main
bubblewrap 0.3.1-1ubuntu2 in cosmic amd64: universe/admin/optional/100% -> main
bubblewrap 0.3.1-1ubuntu2 in cosmic arm64: universe/admin/optional/100% -> main
bubblewrap 0.3.1-1ubuntu2 in cosmic armhf: universe/admin/optional/100% -> main
bubblewrap 0.3.1-1ubuntu2 in cosmic i386: universe/admin/optional/100% -> main
bubblewrap 0.3.1-1ubuntu2 in cosmic ppc64el: universe/admin/optional/100% -> main
bubblewrap 0.3.1-1ubuntu2 in cosmic s390x: universe/admin/optional/100% -> main
Override [y|N]? y
7 publications overridden.

I just wanted to point out that after dropping setuid bit the package description is now wrong.

"setuid wrapper for unprivileged chroot and namespace manipulation"