1332190 - [Mac] Enable level 3 Mac content sandbox, removing filesystem read access

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[Haik Aftandilian [:haik]]

This bug covers the changes to remove file system read access from web content processes on Mac. With bug 1147911 fixed in Nightly, file:// URI's are handled by a separate process and the content process shouldn't need read access to the home directory. This work should be limited to Nightly for now and be easily reverted with prefs.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Just to clarify, this means disallowing an open() correct?  If we already have a fd it will still be readable, right?

Comment 2

[Haik Aftandilian [:haik]]

(In reply to Ben Kelly [:bkelly] from comment #1)
> Just to clarify, this means disallowing an open() correct?

It means disallowing an open() call depending on the path. The open() syscall won't be blocked completely. We'll still need to allow open() for reading in some directories like $PROFILE/extensions for now. The main benefit will be blocking open(READ) calls for files in the home directory including sensitive parts of the profile. With 52+, we're already blocking open(WRITE) calls for most of the home directory.

> If we already have a fd it will still be readable, right?

Yes, that should continue to work, but if you have a test case, let me know.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Haik Aftandilian [:haik] from comment #2)
> > If we already have a fd it will still be readable, right?
> 
> Yes, that should continue to work, but if you have a test case, let me know.

IndexedDB and Cache API do this extensively.  You can run mochitests in dom/cache.

Comment 4

[Alex Gaynor [:Alex_Gaynor]]

:haik, FYI I think as a result of needing to really fix mochitests, we'll be able to switch to whitelisting entirely. I'll keep that as a separate review though!

Comment 5

[Haik Aftandilian [:haik]]

(In reply to Alex Gaynor [:Alex_Gaynor] from comment #4)
> :haik, FYI I think as a result of needing to really fix mochitests, we'll be
> able to switch to whitelisting entirely. I'll keep that as a separate review
> though!

That's great. I don't know what the patches look like, but I'm expecting it will require significant refactoring of the policies so I'd like to land level 3 and then move to the whitelist. Maybe it would make sense to do it in level 4.

Comment 6

[Haik Aftandilian [:haik]]

Attachment: Bug 1332190 - [Mac] Enable level 3 Mac content sandbox, removing filesystem read access.

Review commit: https://reviewboard.mozilla.org/r/153780/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/153780/

Comment 7

[Alex Gaynor [:Alex_Gaynor]]

Comment on attachment 8882699 [details]
Bug 1332190 - [Mac] Enable level 3 Mac content sandbox, removing filesystem read access.

https://reviewboard.mozilla.org/r/153780/#review158964

LGTM! Good to land once the blockers are landed!

Comment 8

[Haik Aftandilian [:haik]]

Comment on attachment 8882699 [details]
Bug 1332190 - [Mac] Enable level 3 Mac content sandbox, removing filesystem read access.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153780/diff/1-2/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b101438c684
[Mac] Enable level 3 Mac content sandbox, removing filesystem read access. r=Alex_Gaynor

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6b101438c684