1317735 - Consolidate env vars for logging

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[Jim Mathies [:jimm]]

https://wiki.mozilla.org/Security/Sandbox#Activity_Logging

We have a MOZ_WIN_SANDBOX_LOGGING (Windows) and a MOZ_SANDBOX_VERBOSE (Linux). OSX currently doesn't use env vars for this.

We should combine these two env vars.

Comment 1

[David Parks [:handyman]]

I like MOZ_SANDBOX_LOG.  Doing this in conjunction with bug 1306239.

Comment 2

[David Parks [:handyman]]

Attachment: Make Pref and Env var that control sandbox violation logging platform-agnostic

Details:

* The patch assigns the preference "security.sandbox.logging.enabled" and the environment variable MOZ_SANDBOX_LOGGING to control whether or not sandbox violations are logged.
* "security.sandbox.logging.enabled" defaults to true
* "security.sandbox.windows.log" was the old pref on Windows and it defaulted to false, so this is a change in behavior.
* In the patch, Linux doesn't check "security.sandbox.logging.enabled".  This is because SandboxInfo's static init process (and the fact that we are building a shared library that doesn't statically link to libXUL) means that checking the pref is hard.  My best idea was kind of hacky and required larger-than-is-warranted changes, like making the logging test an inline method in SandboxInfo and #ifdefing it to work only when the header is included in libXUL.  E.g.:

SandboxInfo.h
-------------

#ifdef (MOZILLA_INTERNAL_API)
#include "mozilla/Preferences.h"
#endif

class SandboxInfo {
	...
	inline bool ShouldLog() {
#ifdef (MOZILLA_INTERNAL_API)
		return Test(kVerbose) || Preferences::GetBool("security.sandbox.logging.enabled");
#else
		return Test(kVerbose);
#endif
	}
}

...and then changing all of the places that call Test(kVerbose) to call ShouldLog().  There are 10 such spots in the linux sandbox.  They are here:

https://dxr.mozilla.org/mozilla-central/search?q=SandboxInfo%3A%3AkVerbose&redirect=false

Of course, this would only apply the setting to places in libXUL that use it -- namely, the 4 SandboxBroker.cpp calls, but that might be better than nothing.

Comment 3

[David Parks [:handyman]]

Tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6903c31615fb4b44fd60e9e2039e1a2420d4422

Comment 4

[Jim Mathies [:jimm]]

Comment on attachment 8829579 [details] [diff] [review]
Make Pref and Env var that control sandbox violation logging platform-agnostic

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

So linux only looks at MOZ_SANDBOX_LOGGING, I guess we can live with that. You mention this in the standup this week so people know, also lets get the wiki page updated one this merges to mc.

Comment 5

[Jim Mathies [:jimm]]

(In reply to Jim Mathies [:jimm] from comment #4)
> Comment on attachment 8829579 [details] [diff] [review]
> Make Pref and Env var that control sandbox violation logging
> platform-agnostic
> 
> Review of attachment 8829579 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> So linux only looks at MOZ_SANDBOX_LOGGING, I guess we can live with that.
> You mention this in the standup this week so people know, also lets get the
> wiki page updated one this merges to mc.

We should consider uplifting this to 52 as well to cut down on differences between versions as this rolls out.

Comment 6

[David Parks [:handyman]]

Note that uplifting will probably also require uplifting Bug 1306239.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/72f272108f0a
Consolidate env vars for logging. r=jimm

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/72f272108f0a