1067301 - Add mach / mochitest argument to enable the Windows content sandbox

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Bob Owen (:bobowen)]

Add mach / mochitest argument to enable the Windows content sandbox similar to the --e10s argument to enable e10s.

Comment 1

[Bob Owen (:bobowen)]

Attachment: Add mach / mochitest option to run tests with Windows content sandbox.

This adds a new option of --content-sandbox (off,warn,on) to run mochitests via mach with the Windows content sandbox enabled.

It assumes --e10s as you can't have the content sandbox without it.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8489382 [details] [diff] [review]
Add mach / mochitest option to run tests with Windows content sandbox.

Review of attachment 8489382 [details] [diff] [review]:
-----------------------------------------------------------------

just one question, otherwise this looks good.

::: testing/mochitest/mach_commands.py
@@ +489,5 @@
>      this_chunk = CommandArgument('--e10s', action='store_true',
>          help='Run tests with electrolysis preferences and test filtering enabled.')
>      func = this_chunk(func)
>  
> +    this_chunk = CommandArgument('--content-sandbox', default='off', choices=['off', 'warn', 'on'],

this works consistently across windows,linux,osx,android,b2g?

Comment 3

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

(In reply to Joel Maher (:jmaher) from comment #2)
> > +    this_chunk = CommandArgument('--content-sandbox', default='off', choices=['off', 'warn', 'on'],
> 
> this works consistently across windows,linux,osx,android,b2g?

Assuming I'm following the patch correctly and it just sets a pref: it'll be a no-op on the Linux platforms.  Specifically: content process sandboxing is always enabled if supported on B2G (which includes TBPL emulator runs since February (bug 932098)), and is currently config'ed off on Linux desktop and Android (and would probably break lots of stuff if enabled).  Also, non-B2G Android devices typically don't have support for it.

Comment 4

[Bob Owen (:bobowen)]

(In reply to Jed Davis [:jld] from comment #3)
> (In reply to Joel Maher (:jmaher) from comment #2)
> > > +    this_chunk = CommandArgument('--content-sandbox', default='off', choices=['off', 'warn', 'on'],
> > 
> > this works consistently across windows,linux,osx,android,b2g?
> 
> Assuming I'm following the patch correctly and it just sets a pref: it'll be
> a no-op on the Linux platforms.  Specifically: content process sandboxing is
> always enabled if supported on B2G (which includes TBPL emulator runs since
> February (bug 932098)), and is currently config'ed off on Linux desktop and
> Android (and would probably break lots of stuff if enabled).  Also, non-B2G
> Android devices typically don't have support for it.

Yeah, I suspect the pref would get set, but only Windows uses it, so it would have no effect if someone used it on other platforms.

Comment 5

[Bob Owen (:bobowen)]

Attachment: Add mach / mochitest option to run tests with Windows content sandbox. v2

r=jmaher - from comment 2

(In reply to Joel Maher (:jmaher) from comment #2)

> > +    this_chunk = CommandArgument('--content-sandbox', default='off', choices=['off', 'warn', 'on'],
> 
> this works consistently across windows,linux,osx,android,b2g?

I'm glad you raised this as when I did some testing on this, I noticed that because I was defaulting the parameter in run_desktop_test to None this meant that if the new argument wasn't specified it was always turning on e10s.

This patch just defaults content_sandbox to 'off' instead.
The pref does always get set, but shouldn't affect other platforms.

Try push on most platforms with this change:
https://tbpl.mozilla.org/?tree=Try&rev=3f77ac7a5d49

Comment 6

[Bob Owen (:bobowen)]

Try push showing that tests are unaffected without the new option specifiedn:
https://tbpl.mozilla.org/?tree=Try&rev=3f77ac7a5d49

Thanks.

Comment 7

[Bob Owen (:bobowen)]

I'm on PTO for a week, so I'll re-request when I get back.

Comment 8

[Bob Owen (:bobowen)]

Just checked this still applies cleanly to m-c.

Try push showing that tests are unaffected without the new option specified:
https://tbpl.mozilla.org/?tree=Try&rev=3f77ac7a5d49

Thanks.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0fce2a1d8ab6

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0fce2a1d8ab6