1345046 - Create a low level API for logging that is sandbox friendly.

Status: NEW

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

Description

[Bob Owen (:bobowen)]

We have seem a number of logging bugs (see See Also), where the sandbox blocks the open/create of the log file.

It would be great if we had some sort of simple low level logging API/service that was sandbox friendly.

(Putting this in Security: Process Sandboxing, because this cuts across a few different areas and I can't find a general logging component.)

Just dumping some thoughts in here ...

Trying to create something that fits all the various logging would probably be a mistake, so I think this should just handle the opening of the file.

Opening the file in the parent and passing down the FD/HANDLE seems like the best solution (as opposed to streaming data over IPC) and should work in all OS sandboxes.

The parent must decide if the file is allowed to be opened and not just accept any requests from the child to open files.
This part is more problematic where currently the child is reading the pref (env var, etc.) and reacting to it directly. It would be better if the parent detected these changes/settings and sent a message along with the FD to turn on logging and not have the child requesting access at all.
This might not be straight forward in some cases (see bug 1269930 comment 8).
I guess the child logging code would need to register with this service, to receive notifications of when logging was turned on.

For things that require log cycling, we wouldn't want to open all the files up front, so maybe we could pass down a unique token with the first FD and the the child could request the next file with the token. If it knew how many files there are, it could possibly cache the FDs, so it didn't need to use IPC once they were all open.

If we find the child has to trigger the logging (and therefore file open) in some cases, then maybe some sort of fixed token could be used to request the file. So, the parent is still determining where the file is and what it is called. Even so, we should probably only allow this if some sort of switch in the parent has been flipped to turn this on.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

We have a pref for AEC logs that says what directory they go in; if there's a directory we're allowed to save in, that would work well.  There is a list of files it will save there (and if you record multiple times, it bumps a number in each filename).  Alternatively, we could remote over IPC the file open/write/flush/close calls (and restrict open to a specific directory).

For WebRTC trace logs, that writes logs similar to MOZ_LOG but it also normally wraps the log at around 10MB, but that's basically seeking the write location back to 0.  It will also cycle logs (close and reope) with a number appended if media.webrtc.debug.multi_log is set.  

Trace logs in webrtc are controlled by env var WEBRTC_TRACE_LOG *or* by someone turning them on in about:webrtc.  What is logged (if anything) is controlled by MOZ_LOG via webrtc_trace:N (1-5) or media.webrtc.debug.trace_mask (16-bit integer mask).  If WEBRTC_TRACE_FILE is 'nspr' or 'moz_log' then all internal webrtc.org logging will be redirected to normal mozilla LOG calls  (and it all gets mixed together).

All that said: much like with the 'nspr/moz_log' logging, we can insert ourselves and redirect logging as needed.  The AEC logs (with some work) could likewise be revectored through our IPC APIs if we have an appropriate mechanism for writing binary data to (restricted in name and location and perhaps size) files.

A solution that works for other potential users of this would be good - and maybe it would just be a temp directory we're allowed to create files in from Content processes.  I just want to see us move forward on something.

Comment 2

[Valentin Gosu [:valentin] (he/him)]

I think we now have a directory that content processes are allowed security.sandbox.content.tempDirSuffix.
We could default to writing to that directory?

Comment 3

[Gian-Carlo Pascutto [:gcp]]

I think we now have a directory that content processes are allowed security.sandbox.content.tempDirSuffix.
We could default to writing to that directory?

First of all, that's something we want to get rid of: https://bugzilla.mozilla.org/show_bug.cgi?id=1539841
Secondly, that won't work if you don't have (any) write permissions to begin with.