Bug 165824

Summary: [Mac][WK2] Stop using file* rules in WebProcess sandbox profiles
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit2Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, aestes, andersca, ap, bfulgham, commit-queue
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Mac   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Brent Fulgham
Reported 2016-12-13 15:24:40 PST
Make the WebProcess sandbox profile more like the NetworkProcess by getting rid of the global "file*" rules, switching to more finely-focused versions. Similar changes should be made in the Databases and Plugin sandboxes.
Attachments
Patch (3.80 KB, patch)
2016-12-13 15:27 PST, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Patch (4.89 KB, patch)
2016-12-14 10:24 PST, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Patch (4.32 KB, patch)
2016-12-14 11:17 PST, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Patch (5.83 KB, patch)
2016-12-14 17:44 PST, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Brent Fulgham
Comment 1 2016-12-13 15:25:03 PST
<rdar://problem/14024823>
Brent Fulgham
Comment 2 2016-12-13 15:27:07 PST
Created attachment 297046 [details] Patch
Alex Christensen
Comment 3 2016-12-13 16:03:07 PST
(In reply to comment #0) > Make the WebProcess sandbox profile more like the NetworkProcess by getting > rid of the global "file*" rules, switching to more finely-focused versions. > > Similar changes should be made in the Databases and Plugin sandboxes. Why not make them all at once?
Alexey Proskuryakov
Comment 4 2016-12-13 18:14:26 PST
Comment on attachment 297046 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=297046&action=review > Source/WebKit2/PluginProcess/mac/com.apple.WebKit.plugin-common.sb.in:262 > +(allow file-read* file-write* (subpath (param "NSURL_CACHE_DIR"))) I think that CFNetwork may need to issue sandbox extensions for cache content. > Source/WebKit2/WebProcess/com.apple.WebProcess.sb.in:173 > + (allow file-issue-extension (require-all (extension-class "com.apple.app-sandbox.read") (subpath (param "DARWIN_USER_TEMP_DIR")))))) Did you test uploading packages? I'm not entirely sure if read extensions are all we use.
Alexey Proskuryakov
Comment 5 2016-12-13 18:15:12 PST
This seems worth trying, but I wouldn't be surprised if this broke something.
Brent Fulgham
Comment 6 2016-12-14 09:39:36 PST
(In reply to comment #4) > Comment on attachment 297046 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=297046&action=review > > > Source/WebKit2/PluginProcess/mac/com.apple.WebKit.plugin-common.sb.in:262 > > +(allow file-read* file-write* (subpath (param "NSURL_CACHE_DIR"))) > > I think that CFNetwork may need to issue sandbox extensions for cache > content. OK. I'll adjust the rules for that. > > Source/WebKit2/WebProcess/com.apple.WebProcess.sb.in:173 > > + (allow file-issue-extension (require-all (extension-class "com.apple.app-sandbox.read") (subpath (param "DARWIN_USER_TEMP_DIR")))))) > > Did you test uploading packages? I'm not entirely sure if read extensions > are all we use. I did find that I needed to use "com.apple.app-sandbox.read-write" during my local testing (after uploading this patch). I'll use the read-write extension-class when landing this change.
Alexey Proskuryakov
Comment 7 2016-12-14 09:43:50 PST
> I did find that I needed to use "com.apple.app-sandbox.read-write" during my local testing (after uploading this patch). Do we need com.apple.app-sandbox.read too?
Brent Fulgham
Comment 8 2016-12-14 10:24:11 PST
Created attachment 297099 [details] Patch
Brent Fulgham
Comment 9 2016-12-14 11:17:51 PST
Created attachment 297103 [details] Patch
Brent Fulgham
Comment 10 2016-12-14 11:18:10 PST
Comment on attachment 297103 [details] Patch Rebaselined patch.
Brent Fulgham
Comment 11 2016-12-14 11:20:53 PST
We should probably wait to land this until next week.
Brent Fulgham
Comment 12 2016-12-14 17:44:06 PST
Created attachment 297150 [details] Patch
Brent Fulgham
Comment 13 2016-12-14 17:44:34 PST
Updated patch to support "read" and "read-write" extensions.
Alexey Proskuryakov
Comment 14 2016-12-15 09:49:47 PST
Comment on attachment 297150 [details] Patch Scary, but worth trying.
Brent Fulgham
Comment 15 2016-12-21 13:39:09 PST
We can land this now! cq+.
WebKit Commit Bot
Comment 16 2016-12-21 14:03:31 PST
Comment on attachment 297150 [details] Patch Clearing flags on attachment: 297150 Committed r210076: <http://trac.webkit.org/changeset/210076>
WebKit Commit Bot
Comment 17 2016-12-21 14:03:35 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.