[ macOS ] Update sandbox rules for storage

• Component: WebKit2
• Status: RESOLVED FIXED

[Sihui Liu]

<rdar://problem/60972224>

[Sihui Liu]

Created attachment 395692 [details]
Patch

[Sihui Liu]

Created attachment 395695 [details]
Patch

[Geoffrey Garen]

Comment on attachment 395695 [details]
Patch

r=me

[Alexey Proskuryakov]

Comment on attachment 395695 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=395695&action=review

> Source/WebKit/NetworkProcess/mac/com.apple.WebKit.NetworkProcess.sb.in:451
> +(allow file-read* file-write*
> +    (home-subpath "/Library/HTTPStorages"))

This is not great, because all NetworkProcesses will have access to each other's cookies, right? So any WebKit2 client can have access to Safari cookies, for example.

[Sihui Liu]

(In reply to Alexey Proskuryakov from comment #4)
> Comment on attachment 395695 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=395695&action=review
> 
> > Source/WebKit/NetworkProcess/mac/com.apple.WebKit.NetworkProcess.sb.in:451
> > +(allow file-read* file-write*
> > +    (home-subpath "/Library/HTTPStorages"))
> 
> This is not great, because all NetworkProcesses will have access to each
> other's cookies, right? So any WebKit2 client can have access to Safari
> cookies, for example.

Safari's cookie is not under this path, only (In reply to Alexey Proskuryakov from comment #4)
> Comment on attachment 395695 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=395695&action=review
> 
> > Source/WebKit/NetworkProcess/mac/com.apple.WebKit.NetworkProcess.sb.in:451
> > +(allow file-read* file-write*
> > +    (home-subpath "/Library/HTTPStorages"))
> 
> This is not great, because all NetworkProcesses will have access to each
> other's cookies, right? So any WebKit2 client can have access to Safari
> cookies, for example.

Safari's cookies is not in this directory. I just updated the radar with more background.

[Geoffrey Garen]

Hmmm... if we can derive the expected path of the cookie storage for a given app, an alternative approach is to issue a sandbox extension for just that path at runtime. That would prevent a compromised Networking process from accessing cookies from other apps.

(Is any other data stored in HTTPStorages, like HSTS data or HTTP2 or HTTP3 data? If so, I guess we would need a list of sandbox extensions, one for each storage technology.)

That said, the networking process always had blanket access to nsurlstoraged / cookied, so I don't think this approach is a regression. I think we can check this in to resolve our high priority test failure regression, and consider more secure designs for the future.

[Sihui Liu]

(In reply to Geoffrey Garen from comment #6)
> Hmmm... if we can derive the expected path of the cookie storage for a given
> app, an alternative approach is to issue a sandbox extension for just that
> path at runtime. That would prevent a compromised Networking process from
> accessing cookies from other apps.
> 
This is an option if we can extract the path.

> (Is any other data stored in HTTPStorages, like HSTS data or HTTP2 or HTTP3
> data? If so, I guess we would need a list of sandbox extensions, one for
> each storage technology.)
> 
I don't know that. Will check with network people.

> That said, the networking process always had blanket access to nsurlstoraged
> / cookied, so I don't think this approach is a regression. I think we can
> check this in to resolve our high priority test failure regression, and
> consider more secure designs for the future.

Sure. And this is not a test-only regression. Apps like TestWebKitAPI may not be able to store cookies persistently.

[EWS]

Committed r259679: <https://trac.webkit.org/changeset/259679>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395695 [details].