1375863 - Merge MOZ_SANDBOX and MOZ_CONTENT_SANDBOX

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, enhancement, P1)

Description

[Jim Mathies [:jimm]]

<jimm> what's the difference between MOZ_CONTENT_SANDBOX and MOZ_SANDBOX?
<Alex_Gaynor> jimm: I think MOZ_SANDBOX is if we sandbox _anything_ (GMP, etc.) CONTENT_SANDBOX is for content processes specifically
<jimm> huh, thx
<jimm> I wonder if we can remove the content one
<jimm> bobowen: ^?
<Alex_Gaynor> Just fold it into MOZ_SANDBOX? Seems prima facie reasonable to me.
<Alex_Gaynor> haik: sounds good; thanks
<jimm> probably has something to do with the funky linking we have to do for content processes
<jimm> (funky linking of the sandbox lib)
<bobowen> I agree, now that we sandboxing content on all platforms we can merge them

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Yes, MOZ_SANDBOX == MOZ_CONTENT_SANDBOX || MOZ_GMP_SANDBOX.

If we're going to make this change, is there any reason not to just merge all three flags?  I can see use cases for desktop-but-not-GMP sandboxing, but not really for the reverse.  (If GMP itself is disabled, the linker should be able to GC most of the GMP-only sandboxing code, and I don't think there's much of that to begin with.)

Also, if we're going to force --disable-gmp-sandbox on anyone with a reason to use --disable-content-sandbox, we should make a decision on whether that means running GMP unsandboxed (the current behavior) or disabling GMP (as proposed in bug 1141825).

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Bug 1402133 is making me think we'd be better off merging these: the Linux build with --enable-content-sandbox --disable-gmp-sandbox has apparently been broken for as long as the code has existed.  Also, the configure logic has some weird corner cases that I don't enjoy explaining, like --enable-sandbox not actually enabling anything if it's an unsupported platform.

There's still the question of --disable-content-sandbox --enable-gmp-sandbox, but we're not enforcing a minimum value for security.sandbox.content.level on Linux (yet), so there's still the alternative of setting the pref.

Also, --{en,dis}able-gmp-sandbox don't exist and never have; MOZ_GMP_SANDBOX is defined iff --enable-sandbox and x86, unless you edit the configure script.  So that's another oddity we could get rid of by flattening everything into MOZ_SANDBOX.

Comment 3

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

This should be doable by:

1. ripping out the MOZ_(CONTENT|GMP)_SANDBOX parts of old-configure.in, except that the architecture restrictions for Linux should now apply to MOZ_SANDBOX instead

2. globally replacing MOZ_(CONTENT|GMP)_SANDBOX with MOZ_SANDBOX

3. going through security/sandbox to remove the redundant MOZ_SANDBOX ifdefs left by step 2 (but not security/sandbox/chromium, which has some deliberate use of MOZ_SANDBOX patched in and I assume it's like that for a reason); there are about 30 of them, so it'll be mildly annoying but not awful

(As a more ambitious alternative to step 1, replace the remaining autoconf with moz.configure scripting.)

Comment 4

[Alex Gaynor [:Alex_Gaynor]]

Attachment: Bug 1375863 - fold MOZ_CONTENT_SANDBOX and MOZ_GMP_SANDBOX into MOZ_SANDBOX; r?jld

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc8935d7c0b1
fold MOZ_CONTENT_SANDBOX and MOZ_GMP_SANDBOX into MOZ_SANDBOX; r=jld,firefox-build-system-reviewers

Comment 6

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/dc8935d7c0b1