Bug 209933

Summary: [iOS] Deny mach lookup access to the runningboard service in the WebContent process
Product: WebKit Reporter: Per Arne Vollan <pvollan>
Component: WebKit Misc.Assignee: Per Arne Vollan <pvollan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cdumez, commit-queue, eric.carlson, ggaren, jer.noble, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 210066    
Bug Blocks: 226357    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
cdumez: review+
Patch none

Per Arne Vollan
Reported 2020-04-02 15:20:12 PDT
On iOS, after <https://trac.webkit.org/changeset/258180/webkit>, mach lookup access to "com.apple.runningboard" can be denied in the WebContent process.
Attachments
Patch (4.04 KB, patch)
2020-04-02 15:29 PDT, Per Arne Vollan 		no flags
DetailsFormatted DiffDiff
Patch (9.65 KB, patch)
2020-04-07 14:26 PDT, Per Arne Vollan 		no flags
DetailsFormatted DiffDiff
Patch (9.66 KB, patch)
2020-04-07 15:41 PDT, Per Arne Vollan 		cdumez: review+
DetailsFormatted DiffDiff
Patch (9.45 KB, patch)
2020-04-07 16:24 PDT, Per Arne Vollan 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Per Arne Vollan
Comment 1 2020-04-02 15:20:32 PDT
rdar://problem/56995639
Per Arne Vollan
Comment 2 2020-04-02 15:29:38 PDT
Created attachment 395311 [details] Patch
Brent Fulgham
Comment 3 2020-04-03 10:30:51 PDT
Comment on attachment 395311 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=395311&action=review r=me > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:561 > +(deny mach-lookup (with telemetry-backtrace) Hooray!
Per Arne Vollan
Comment 4 2020-04-03 10:31:28 PDT
Comment on attachment 395311 [details] Patch Thanks for reviewing!
EWS
Comment 5 2020-04-03 10:37:51 PDT
Committed r259469: <https://trac.webkit.org/changeset/259469> All reviewed patches have been landed. Closing bug and clearing flags on attachment 395311 [details].
Chris Dumez
Comment 6 2020-04-06 10:19:18 PDT
Comment on attachment 395311 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=395311&action=review > Source/WebKit/ChangeLog:9 > + On iOS, after <https://trac.webkit.org/changeset/258180/webkit>, mach lookup access to "com.apple.runningboard" How can this be true? We still take a RunningBoard DependencyAssertion in the WebProcess. I suspect this broke media (again).
Chris Dumez
Comment 7 2020-04-06 10:19:37 PDT
(In reply to Chris Dumez from comment #6) > Comment on attachment 395311 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=395311&action=review > > > Source/WebKit/ChangeLog:9 > > + On iOS, after <https://trac.webkit.org/changeset/258180/webkit>, mach lookup access to "com.apple.runningboard" > > How can this be true? We still take a RunningBoard DependencyAssertion in > the WebProcess. I suspect this broke media (again). + Jer/ Eric
Per Arne Vollan
Comment 8 2020-04-06 11:12:24 PDT
(In reply to Chris Dumez from comment #6) > Comment on attachment 395311 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=395311&action=review > > > Source/WebKit/ChangeLog:9 > > + On iOS, after <https://trac.webkit.org/changeset/258180/webkit>, mach lookup access to "com.apple.runningboard" > > How can this be true? We still take a RunningBoard DependencyAssertion in > the WebProcess. I suspect this broke media (again). I don't think calling 'm_uiProcessDependencyProcessAssertion = makeUnique<DependencyProcessAssertion>(remoteProcessID, "WebContent process dependency on UIProcess"_s)' in the WebContent process will need access to the runningboard service.
Chris Dumez
Comment 9 2020-04-06 11:23:15 PDT
(In reply to Per Arne Vollan from comment #8) > (In reply to Chris Dumez from comment #6) > > Comment on attachment 395311 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=395311&action=review > > > > > Source/WebKit/ChangeLog:9 > > > + On iOS, after <https://trac.webkit.org/changeset/258180/webkit>, mach lookup access to "com.apple.runningboard" > > > > How can this be true? We still take a RunningBoard DependencyAssertion in > > the WebProcess. I suspect this broke media (again). > > I don't think calling 'm_uiProcessDependencyProcessAssertion = > makeUnique<DependencyProcessAssertion>(remoteProcessID, "WebContent process > dependency on UIProcess"_s)' in the WebContent process will need access to > the runningboard service. That would be good news. As long as you have verified that then I guess we're good. Breakage would not be obvious but you would definitely see sandbox violations if things were not working I think.
WebKit Commit Bot
Comment 10 2020-04-06 12:58:16 PDT
Re-opened since this is blocked by bug 210066
Per Arne Vollan
Comment 11 2020-04-07 14:26:06 PDT
Created attachment 395739 [details] Patch
Chris Dumez
Comment 12 2020-04-07 15:28:50 PDT
This likely needs updating after http://trac.webkit.org/changeset/259674/webkit.
Per Arne Vollan
Comment 13 2020-04-07 15:41:26 PDT
Created attachment 395752 [details] Patch
Per Arne Vollan
Comment 14 2020-04-07 15:42:25 PDT
(In reply to Chris Dumez from comment #12) > This likely needs updating after > http://trac.webkit.org/changeset/259674/webkit. Thanks!
Chris Dumez
Comment 15 2020-04-07 15:44:33 PDT
Comment on attachment 395752 [details] Patch Ok, assuming you have verified that the assertion reliably gets taken at RunningBoard level and does not get invalidated.
Per Arne Vollan
Comment 16 2020-04-07 15:47:08 PDT
(In reply to Chris Dumez from comment #15) > Comment on attachment 395752 [details] > Patch > > Ok, assuming you have verified that the assertion reliably gets taken at > RunningBoard level and does not get invalidated. Yes, I have confirmed that the creation of the dependency assertion succeeds, and manually tested the audio playback scenario. Thanks for reviewing!
Per Arne Vollan
Comment 17 2020-04-07 16:24:46 PDT
Created attachment 395758 [details] Patch
EWS
Comment 18 2020-04-07 19:38:48 PDT
Committed r259700: <https://trac.webkit.org/changeset/259700> All reviewed patches have been landed. Closing bug and clearing flags on attachment 395758 [details].
Note You need to log in before you can comment on or make changes to this bug.