Bug 209933 – [iOS] Deny mach lookup access to the runningboard service in the WebContent process

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Per Arne Vollan]

On iOS, after <https://trac.webkit.org/changeset/258180/webkit>, mach lookup access to "com.apple.runningboard" can be denied in the WebContent process.

[Per Arne Vollan]

rdar://problem/56995639

[Per Arne Vollan]

Created attachment 395311 [details]
Patch

[Brent Fulgham]

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 on attachment 395311 [details]
Patch

Thanks for reviewing!

[EWS]

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

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

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

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

Re-opened since this is blocked by bug 210066

[Per Arne Vollan]

Created attachment 395739 [details]
Patch

[Chris Dumez]

This likely needs updating after http://trac.webkit.org/changeset/259674/webkit.

[Per Arne Vollan]

Created attachment 395752 [details]
Patch

[Per Arne Vollan]

(In reply to Chris Dumez from comment #12)
> This likely needs updating after
> http://trac.webkit.org/changeset/259674/webkit.

Thanks!

[Chris Dumez]

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]

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

Created attachment 395758 [details]
Patch

[EWS]

Committed r259700: <https://trac.webkit.org/changeset/259700>

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