Bug 165824 – [Mac][WK2] Stop using file* rules in WebProcess sandbox profiles

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brent Fulgham]

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.

[Brent Fulgham]

<rdar://problem/14024823>

[Brent Fulgham]

Created attachment 297046 [details]
Patch

[Alex Christensen]

(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 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]

This seems worth trying, but I wouldn't be surprised if this broke something.

[Brent Fulgham]

(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]

> 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]

Created attachment 297099 [details]
Patch

[Brent Fulgham]

Created attachment 297103 [details]
Patch

[Brent Fulgham]

Comment on attachment 297103 [details]
Patch

Rebaselined patch.

[Brent Fulgham]

We should probably wait to land this until next week.

[Brent Fulgham]

Created attachment 297150 [details]
Patch

[Brent Fulgham]

Updated patch to support "read" and "read-write" extensions.

[Alexey Proskuryakov]

Comment on attachment 297150 [details]
Patch

Scary, but worth trying.

[Brent Fulgham]

We can land this now! cq+.

[WebKit Commit Bot]

Comment on attachment 297150 [details]
Patch

Clearing flags on attachment: 297150

Committed r210076: <http://trac.webkit.org/changeset/210076>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.