Closed
Bug 1332522
Opened 8 years ago
Closed 8 years ago
[Mac] remove ~/Library read restriction from file content process sandbox
Categories
(Core :: Security: Process Sandboxing, defect)
Tracking
()
VERIFIED
FIXED
mozilla54
People
(Reporter: haik, Assigned: haik)
References
Details
(Whiteboard: sbmc2)
Attachments
(2 files, 1 obsolete file)
On Nightly (because security.sandbox.content.level=2) the Mac content sandbox prevents read access to ~/Library excluding $PROFILE/{extensions,weave,chrome}. The file-content process uses the same sandbox rules, but doesn't need this restriction because it should be allowed to read anywhere on the filesystem the user has access to.
Assignee | ||
Updated•8 years ago
|
Assignee | ||
Comment 1•8 years ago
|
||
Requesting some general feedback on passing a new command line argument to content processes ("file" or "web") to indicate which type of content process it is. This allows the content process to use different sandbox settings for file and web content processes. For debugging, on Mac at least, I found it's really convenient to be able to determine what type of process you are looking at from /bin/ps output.
Attachment #8829770 -
Flags: feedback?(gpascutto)
Attachment #8829770 -
Flags: feedback?(bobowencode)
Comment 2•8 years ago
|
||
Comment on attachment 8829770 [details] [diff] [review] Passes "file" or "web" to the content process, depending on process type Review of attachment 8829770 [details] [diff] [review]: ----------------------------------------------------------------- The content process already knows what remote type it is, although it seems that I didn't do that early enough for it to be used in sandboxing. I didn't realise that the Mac sandbox policy was all determined by the child. On Windows it's set up front by the parent. I think we should move the call at [1] to [2] (with a comment saying this must happen before the SendSetProcessSandbox). It may well be useful to have things on the command line for debugging. I'm not sure what the general feeling/policy is over adding extra command line parameters. If we decide that it's a good enough reason to have it, we should probably pass the remote type, not just file and web and remove the SendRemoteType call. You can pass extra arguments to LaunchAndWaitForProcessHandle. [1] https://hg.mozilla.org/mozilla-central/file/8ff550409e1d/dom/ipc/ContentParent.cpp#l1095 [2] https://hg.mozilla.org/mozilla-central/file/8ff550409e1d/dom/ipc/ContentParent.cpp#l1861
Attachment #8829770 -
Flags: feedback?(bobowencode)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 5•8 years ago
|
||
mozreview-review |
Comment on attachment 8830968 [details] Bug 1332522 - Part 1 - Send the remote type to the child before enabling the content sandbox; https://reviewboard.mozilla.org/r/106216/#review108986
Attachment #8830968 -
Flags: review?(bobowencode) → review+
Assignee | ||
Updated•8 years ago
|
Attachment #8829770 -
Attachment is obsolete: true
Attachment #8829770 -
Flags: feedback?(gpascutto)
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to Bob Owen (:bobowen) from comment #2) > Comment on attachment 8829770 [details] [diff] [review] > Passes "file" or "web" to the content process, depending on process type > > Review of attachment 8829770 [details] [diff] [review]: > ----------------------------------------------------------------- > > The content process already knows what remote type it is, although it seems > that I didn't do that early enough for it to be used in sandboxing. I didn't > realise that the Mac sandbox policy was all determined by the child. On > Windows it's set up front by the parent. > > I think we should move the call at [1] to [2] (with a comment saying this > must happen before the SendSetProcessSandbox). > > It may well be useful to have things on the command line for debugging. > I'm not sure what the general feeling/policy is over adding extra command > line parameters. > If we decide that it's a good enough reason to have it, we should probably > pass the remote type, not just file and web and remove the SendRemoteType > call. > You can pass extra arguments to LaunchAndWaitForProcessHandle. > > > [1] > https://hg.mozilla.org/mozilla-central/file/8ff550409e1d/dom/ipc/ > ContentParent.cpp#l1095 > [2] > https://hg.mozilla.org/mozilla-central/file/8ff550409e1d/dom/ipc/ > ContentParent.cpp#l1861 (Meant to send this before posting the code for review.) Thanks for the feedback. I've changed the code to send the remote type earlier in the content startup flow so Mac can use it to setup the sandbox. I'll look into other ways of identifying the file content process for debugging and might file a follow up to add a flag identifying them by the command line arguments. Soon we'll also have extensions processes.
Comment 8•8 years ago
|
||
mozreview-review |
Comment on attachment 8830969 [details] Bug 1332522 - Part 2 - Remove read restrictions from level 2 policy for file content processes; https://reviewboard.mozilla.org/r/106224/#review109018
Attachment #8830969 -
Flags: review?(gpascutto) → review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 9•8 years ago
|
||
Autoland couldn't rebase this for landing.
Flags: needinfo?(haftandilian)
Keywords: checkin-needed
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•8 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #9) > Autoland couldn't rebase this for landing. Synced with central. Problem should be fixed now. Sorry about that.
Flags: needinfo?(haftandilian)
Keywords: checkin-needed
Comment 13•8 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/07b8ccb72585 Part 1 - Send the remote type to the child before enabling the content sandbox; r=bobowen https://hg.mozilla.org/integration/autoland/rev/a4117383c886 Part 2 - Remove read restrictions from level 2 policy for file content processes; r=gcp
Keywords: checkin-needed
Comment 14•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/07b8ccb72585 https://hg.mozilla.org/mozilla-central/rev/a4117383c886
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 15•8 years ago
|
||
Reproduced the ~/Library read restriction using Nightly 53.0a1 (Build ID: 20170115030210) on Mac OS X 10.12. Verified as fixed on the latest Nightly 55.0a1 (Build ID: 20170403030207) and Aurora 54.0a2 (Build ID: 20170403004002) on Mac OS X 10.12.
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•