Bug 225171

Summary: [Cocoa] Always extend access to local process HTTP/3 cache directory
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit Misc.Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, achristensen, bfulgham, koivisto, mjs, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=225239
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Brent Fulgham
Reported 2021-04-28 16:26:26 PDT
I've seen a number of sandbox violations triggered by the UIProcess not extending access to the cache path used for HTTP/3 negotiations. This seems to happen when the OS overrides the user Safari user setting, or toggles it externally without restarting the process. Rather than trigger failures, lets just extend the path for all users (even if HTTP/3 is off), since it will eventually be on for everyone, and it doesn't expand the sandbox anywhere interesting.
Attachments
Patch (2.60 KB, patch)
2021-04-28 16:32 PDT, Brent Fulgham
no flags
Patch (3.17 KB, patch)
2021-04-29 16:18 PDT, Brent Fulgham
no flags
Patch (4.69 KB, patch)
2021-04-30 14:22 PDT, Brent Fulgham
no flags
Brent Fulgham
Comment 1 2021-04-28 16:28:07 PDT
Brent Fulgham
Comment 2 2021-04-28 16:32:57 PDT
Alex Christensen
Comment 3 2021-04-29 15:13:58 PDT
At the same time we should probably make defaultAlternativeServicesDirectory return the same directory as something else, like defaultNetworkCacheDirectory. Otherwise, we will have a startup performance regression from additional directory operations.
Brent Fulgham
Comment 4 2021-04-29 16:18:09 PDT
Alex Christensen
Comment 5 2021-04-29 17:19:50 PDT
Comment on attachment 427386 [details] Patch I think we should just return defaultNetworkCacheDirectory() instead of this.
Antti Koivisto
Comment 6 2021-04-30 06:30:48 PDT
I don't think we should have anything under NetworkCache directory that is not under control of the NetworkCache code. NetworkCache has habit of wiping out stuff that it doesn't recognize so it is pretty risky.
EWS
Comment 7 2021-04-30 08:15:47 PDT
Committed r276838 (237189@main): <https://commits.webkit.org/237189@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 427306 [details].
Brent Fulgham
Comment 8 2021-04-30 09:36:37 PDT
Wow -- how did this get landed?
Alex Christensen
Comment 9 2021-04-30 09:41:35 PDT
I r+ cq+ your original patch.
Brent Fulgham
Comment 10 2021-04-30 09:52:24 PDT
(In reply to Alex Christensen from comment #9) > I r+ cq+ your original patch. Oh! Good. I was terrified I had somehow clobbered your and Antti's review stuff. Thanks.
Aakash Jain
Comment 11 2021-04-30 11:27:05 PDT
(In reply to EWS from comment #7) > Committed r276838 (237189@main): <https://commits.webkit.org/237189@main> This broke api test on iOS: TestWebKitAPI.WebKit.AlternativeServicesDefaultDirectoryCreation History: https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.WebKit.AlternativeServicesDefaultDirectoryCreation
Ryan Haddad
Comment 12 2021-04-30 11:28:22 PDT
(In reply to Aakash Jain from comment #11) > (In reply to EWS from comment #7) > > Committed r276838 (237189@main): <https://commits.webkit.org/237189@main> > This broke api test on iOS: > TestWebKitAPI.WebKit.AlternativeServicesDefaultDirectoryCreation > > History: > https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.WebKit. > AlternativeServicesDefaultDirectoryCreation I just filed https://bugs.webkit.org/show_bug.cgi?id=225239 about this
Ryan Haddad
Comment 13 2021-04-30 12:51:47 PDT
Reverted r276838 for reason: Caused TestWebKitAPI.WebKit.AlternativeServicesDefaultDirectoryCreation to fail Committed r276850 (237201@main): <https://commits.webkit.org/237201@main>
Brent Fulgham
Comment 14 2021-04-30 14:22:16 PDT
EWS
Comment 15 2021-04-30 16:33:16 PDT
Committed r276862 (237210@main): <https://commits.webkit.org/237210@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 427446 [details].
Note You need to log in before you can comment on or make changes to this bug.