Closed Bug 1332190 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: Security: Process Sandboxing, defect)

53 Branch
Unspecified
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox53 --- affected
firefox56 --- fixed

People

(Reporter: haik, Assigned: haik)

References

(Depends on 2 open bugs, Blocks 1 open bug)

Details

(Whiteboard: sbmc2)

Attachments

(1 file)

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.
Assignee: nobody → haftandilian
Depends on: 1147911
Whiteboard: sbmc2
Just to clarify, this means disallowing an open() correct?  If we already have a fd it will still be readable, right?
(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.
(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.
Depends on: 1334550
Depends on: 1340351
Depends on: 1294641
Depends on: 1356324
Depends on: 1357846
Depends on: 1360223
Depends on: 1363179
Depends on: 1374557
Summary: [Mac] Remove file system read access from content sandbox when separate file process in use → [Mac] Enable level 3 Mac content sandbox, removing read access to /Users, /Volumes, and more
: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!
(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.
Depends on: 1377355
Depends on: 1377128
No longer depends on: 1377128
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!
Attachment #8882699 - Flags: review?(agaynor) → review+
Summary: [Mac] Enable level 3 Mac content sandbox, removing read access to /Users, /Volumes, and more → [Mac] Enable level 3 Mac content sandbox, removing filesystem read access
Pushed by haftandilian@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6b101438c684
[Mac] Enable level 3 Mac content sandbox, removing filesystem read access. r=Alex_Gaynor
https://hg.mozilla.org/mozilla-central/rev/6b101438c684
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Depends on: 1379784
Depends on: 1380127
Depends on: 1380132
No longer depends on: 1380148
Depends on: 1382260
Depends on: 1391186
Depends on: 1393805
Depends on: 1421262
Depends on: 1404298
Depends on: 1437281
Depends on: 1448374
Depends on: 1452278
Depends on: 1446549
Depends on: 1469657
Depends on: 1542385
No longer depends on: 1542385
Depends on: 1594679
You need to log in before you can comment on or make changes to this bug.