Iulia Cristescu, QA [:JuliaC] (away, please needinfo? cornel.ionce@softvision.ro) Reporter
	Description • 6 years ago

[Affected versions]:
- 60.0a1

[Affected platforms]:
- Ubuntu 16.04 x32
- Ubuntu 16.04 x64

[Unaffected platforms]:
- macOS 10.13
- Windows 7 x32

[Steps to reproduce]:
1. Start a Guest session
2. Open a recent Nightly version (eg. 60.0a1 – 2018-01-30) with a clean profile

[Expected result]:
- The browser is successfully opened

[Actual result]:
- The tab that firstly gets in focus crashes (any attempt to open a new tab also triggers this behavior)

[Regression range]:
- I was able to find a recent regression: 
  * last good revision: 
  * first bad revision: 
  * pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=8ad8d2f1649a76d6f62d823a81b4c6a7a08ca9eb&tochange=bd7ff5744eb29e105b7b3c37cb5f46164fa11ef4

- Note this issue also occurred starting with 2017-07-20, all the way back to 2017-07-26 Nightly build

[Additional notes]:
- On Ubuntu 14.04 x32, the browser is launched, no tab crash is occurred, but no page/tab can be loaded inside it

	Marcia Knous [:marcia]
	Comment 1 • 6 years ago

Can you please add the crash report to this bug? Thanks.

	Iulia Cristescu, QA [:JuliaC] (away, please needinfo? cornel.ionce@softvision.ro) Reporter
	Comment 2 • 6 years ago

(In reply to Marcia Knous [:marcia - use ni] from comment #1)
> Can you please add the crash report to this bug? Thanks.

There are no crash reports generated, maybe because it's about tab crashes, not about browser crashes. Is there any solution to generate them even in this situation?

	Panos Astithas (he/him) [:past] (please ni?)
	Comment 3 • 6 years ago

Hey Ted, are crashes that our code doesn't catch stored by Linux somewhere?

	(not currently active) Ted Mielczarek
	Comment 4 • 6 years ago

Not by default, no. You can enable core dumps by running `ulimit -c unlimited` in a shell and then running Firefox from that shell. Note that it'll get written wherever `/proc/sys/kernel/core_pattern` says. By default on my Ubuntu 17.10 system it's set to pass them to apport, but I don't know if that will give you anything useful.

If you set the core_pattern to just a directory you should be able to get a core dump and then run gdb on it to get a stack. For Nightly or Release builds you can load this Python script into GDB to fetch debug symbols for Firefox:
https://gist.github.com/luser/193572147c401c8a965c

	Selena Deckelmann :selenamarie :selena
	Comment 5 • 6 years ago

Can you try enabling core dumps as ted describes in comment 4? Thanks!

	Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ Assignee
	Comment 6 • 6 years ago

Bug 1401062 is in the regression range (and later sandbox changes like bug 1430949 and bug 1213998 aren't), which suggests some experiments:
1. Lower security.sandbox.content.level from 4 to 3
2. With the sandbox level at 3, create the boolean pref security.sandbox.content.force-namespace set to true.

I'm going to investigate this, because I'm very likely going to be the assignee.

	Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ Assignee
	Comment 7 • 6 years ago

Firefox definitely doesn't work in an Ubuntu guest account for me… but it wouldn't have created a core dump, because the content process didn't crash, because it wasn't created in the first place:

[Parent 3068, Gecko_IOThread] WARNING: Failed to launch tab subprocess: file /builds/worker/workspace/build/src/ipc/glue/GeckoChildProcessHost.cpp, line 546

Why would it fail to launch the subprocess?  Probably this, from dmesg:

[  104.996591] audit: type=1400 audit(1518109950.046:108): apparmor="DENIED" operation="capable" profile="/usr/lib/lightdm/lightdm-guest-session" pid=3068 comm="Gecko_IOThread" capability=21  capname="sys_admin"

	Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ Assignee
	Comment 8 • 6 years ago

…but with the two pref changes I mentioned in comment #6 — which approximates how things were after bug 1401062 and before bug 1430949 and bug 1213998 and similar, it *does* properly crash:

[ 1003.731590] audit: type=1400 audit(1518110848.905:124): apparmor="DENIED" operation="open" profile="/usr/lib/lightdm/lightdm-guest-session" name="/proc/4718/uid_map" pid=4718 comm="Gecko_IOThread" requested_mask="w" denied_mask="w" fsuid=999 ouid=999
[ 1003.731884] Gecko_IOThread[4718]: segfault at 0 ip 00007fd31cf9abb6 sp 00007fd317645140 error 6 in libxul.so[7fd31cf8a000+47e5000]

That crash is probably https://searchfox.org/mozilla-central/rev/a539f8f7fe288d0b4d8d6a32510be20f52bb7a0f/security/sandbox/linux/launch/SandboxLaunch.cpp#428

What's going on here is that creating a new user namespace is allowed, but using any of the namespace-scoped capabilities is denied by Ubuntu's AppArmor policy for guest accounts, as is touching any of the user namespace controls.

So the feature detection in SandboxInfo sees unprivileged user namespaces as working: clone with just CLONE_NEWUSER is allowed, and the only thing that new process does is exit.  But adding other flags (e.g., CLONE_NEWUSER|CLONE_NEWNET) immediately fails, because it first handles CLONE_NEWUSER by creating a new user namespace and granting all cabilities in it, and then separately handles CLONE_NEWNET by checking for CAP_SYS_ADMIN; normally that check always succeeds, but it's denied by AppArmor.  And there's another rule denying write access to uid_map (and, I assume, the other id maps) so we can't even set up the user namespace.

	Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ Assignee
	Comment 9 • 6 years ago

Also, GeckoMediaPlugins (EME DRM and H.264 WebRTC) have likely been broken in this environment since Firefox 40 (bug 1151607), and I've confirmed that they're broken with the Firefox 58 currently shipping in Ubuntu 16.04.

Workarounds, if anyone needs them: for content only, lowering security.sandbox.content.level to 3; for all process types, setting MOZ_ASSUME_USER_NS=0 in the environment.

	Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ Assignee
	Comment 11 • 6 years ago

I did some unscientific performance measurements with `perf stat` and unshare(1) to decide which extra namespace to use: CLONE_NEWNET has a small but noticeable cost, and CLONE_NEWPID seemed to be slightly faster than CLONE_NEWIPC but they're both fast enough that it probably doesn't matter either way.

I'm not testing id_map writeability here, under the assumption that anything that would restrict it would also restrict capabilities (which are much more significant in terms of how much kernel attack surface they expose), and it's not worth the extra code (and, maybe, extra startup latency).

	Gian-Carlo Pascutto [:gcp]
	Comment 12 • 6 years ago
mozreview-review

Comment on attachment 8949641 [details]
Bug 1434528 - Adjust sandbox feature detection to deal with Ubuntu guest accounts.

https://reviewboard.mozilla.org/r/219002/#review224734

	Gian-Carlo Pascutto [:gcp]
	Comment 13 • 6 years ago

Should we file an upstream bug for this?

They're effectively causing browser sandboxes to be weakened because they have a nonsensical security policy for guest accounts. Seems rather...sub-optimal and counterproductive?

I presume Chrome has exactly the same problem even?

	Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ Assignee
	Comment 14 • 6 years ago

(In reply to Gian-Carlo Pascutto [:gcp] from comment #13)
> Should we file an upstream bug for this?
> 
> They're effectively causing browser sandboxes to be weakened because they
> have a nonsensical security policy for guest accounts. Seems
> rather...sub-optimal and counterproductive?
> 
> I presume Chrome has exactly the same problem even?

Thank you for asking that.  It turns out that Chromium has special treatment in the policy:

https://github.com/CanonicalLtd/lightdm/blob/master/data/apparmor/abstractions/lightdm_chromium-browser

	Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ Assignee
	Comment 15 • 6 years ago

So, I could write a policy (or, really, edit down the Chromium policy) and send them a PR, and that would cover Ubuntu's own packages, but without the patch in this bug it still wouldn't work to download a Firefox as a guest user and run it.  That may not be something users do often, but as we see here it's useful for testing prerelease builds for compatibility with the guest session environment if nothing else.  It's also true that this wouldn't work for Chrome/Chromium either, but they spent a few years not supporting the download-and-run use case at all (setuid sandbox vs. namespace sandbox).

If I land the SandboxInfo change *and* send a policy change upstream, then that fixes this, but we may not find out if anyone else is (or later starts) doing something similar.  On the other hand, we have Telemetry, and that's generally a nicer way to gather data than refusing to start the browser.  With just this patch we'll see this kind of restriction and `sysctl kernel.unprivileged_userns_clone` as the same thing, but in principle we could do additional telemetry to distinguish them if we really wanted to.

	Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ Assignee
	Comment 16 • 6 years ago

There's also a policy for Firefox, and it's nontrivial:

https://bazaar.launchpad.net/~mozillateam/firefox/firefox-trunk.head/view/head:/debian/usr.bin.firefox.apparmor.14.10

This looks complicated enough that I think the better approach is to file a bug report upstream instead of trying write the policy myself.

	Pulsebot
	Comment 17 • 6 years ago

Pushed by jedavis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9dcf26ff6a6e
Adjust sandbox feature detection to deal with Ubuntu guest accounts. r=gcp

	Narcis Beleuzu [:NarcisB]
	Comment 18 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/9dcf26ff6a6e