[MIR] bubblewrap
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
| bubblewrap (Ubuntu) |
High
|
Unassigned |
Bug Description
Availability
============
Built for all supported architectures.
In sync with Debian.
Rationale
=========
The gnome-desktop3 library 3.25.90+ requires bubblewrap. bubblewrap is most commonly used as part of Flatpak's security isolation feature. Here it's being used to sandbox the thumbnailers.
See https:/
The bubblewrap feature was disabled in Ubuntu 17.10's gnome-desktop3 package because this MIR was not processed.
Security
========
No known open security vulnerabilities in any Ubuntu releases.
https:/
I helped prepare a security update (LP: #1657357) (CVE-2017-5226) for bubblewrap/flatpak several months ago.
Security-sensitive package.
Quality assurance
=================
Bug subscriber: should be Ubuntu Desktop Bugs
https:/
https:/
https:/
dh_auto_test runs the build tests but they appear to be set as SKIP upstream. (See comment #4)
Multiple autopkgtests passing on all Ubuntu architectures. Because the tests require machine isolation, the autopkgtests don't run on Debian's infrastructure currently.
Dependencies
============
check-mir reports all other binary dependencies are in main
Standards compliance
=======
4.0.0
Maintenance
===========
- Actively developed upstream
https:/
- Maintained in Debian by the pkg-utopia team but more specifically, it is maintained by Simon McVittie (smcv) who also maintains Flatpak and ostree in Debian and Ubuntu.
short dh7 style rules, dh compat 10
Background information
=======
William Hua (attente) had been working last year on a snapcraft plugin that used bubblewrap.
So maybe more stuff will use bubblewrap in the future.
CVE References
amano (jyaku) wrote : | #2 |
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://
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,..."
amano (jyaku) wrote : | #3 |
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:/
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.
summary: |
- [MIR] bubblewrap + [FFe][MIR] bubblewrap |
summary: |
- [FFe][MIR] bubblewrap + FFe: [MIR] bubblewrap |
> 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.
summary: |
- FFe: [MIR] bubblewrap + [MIR] bubblewrap |
description: | updated |
description: | updated |
amano (jyaku) wrote : | #5 |
The current state on the corresponding Trello card (https:/
Didier Roche (didrocks) wrote : | #6 |
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/
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.
Changed in bubblewrap (Ubuntu): | |
assignee: | nobody → Canonical Security Team (canonical-security) |
Jeremy Bicha (jbicha) wrote : | #7 |
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:/
Those files are shipped in the bubblewrap binary package in
/usr/share/
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:/
Didier Roche (didrocks) wrote : | #8 |
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!
Simon McVittie (smcv) wrote : | #9 |
> 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/
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.
Iain Lane (laney) wrote : | #10 |
(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.
Changed in bubblewrap (Ubuntu): | |
assignee: | Canonical Security Team (canonical-security) → Ubuntu Security Team (ubuntu-security) |
Jeremy Bicha (jbicha) wrote : | #11 |
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.
Jamie Strandboge (jdstrand) wrote : | #12 |
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.
Changed in bubblewrap (Ubuntu): | |
assignee: | Ubuntu Security Team (ubuntu-security) → Seth Arnold (seth-arnold) |
status: | Confirmed → Triaged |
importance: | Undecided → High |
Jamie Strandboge (jdstrand) wrote : | #13 |
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.
Colin Walters (walters) wrote : | #14 |
> 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).
Colin Walters (walters) wrote : | #15 |
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.
Jed Davis (jld-moz) wrote : | #16 |
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:/
Changed in bubblewrap (Ubuntu): | |
assignee: | Seth Arnold (seth-arnold) → Alex Murray (alexmurray) |
Alex Murray (alexmurray) wrote : | #17 |
- 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
- 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...
Changed in bubblewrap (Ubuntu): | |
assignee: | Alex Murray (alexmurray) → nobody |
Jeremy Bicha (jbicha) wrote : | #18 |
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://
--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]
Alex Murray (alexmurray) wrote : | #20 |
Ah ok thanks - sorry I somehow missed those details in comment 4 - cheers.
Jeremy Bicha (jbicha) wrote : | #21 |
setuid has been dropped now:
https:/
Sebastien Bacher (seb128) wrote : | #22 |
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/
bubblewrap 0.3.1-1ubuntu2 in cosmic arm64: universe/
bubblewrap 0.3.1-1ubuntu2 in cosmic armhf: universe/
bubblewrap 0.3.1-1ubuntu2 in cosmic i386: universe/
bubblewrap 0.3.1-1ubuntu2 in cosmic ppc64el: universe/
bubblewrap 0.3.1-1ubuntu2 in cosmic s390x: universe/
Override [y|N]? y
7 publications overridden.
Changed in bubblewrap (Ubuntu): | |
status: | Triaged → Fix Released |
Jalon Funk (francescohickle15) wrote : | #23 |
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"
Status changed to 'Confirmed' because the bug affects multiple users.