Bug 207743 – Dynamically generate media-related mach connections when not using the GPU Process

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

[Brent Fulgham]

As a first step to moving media-related XPC services out of the WebContent process, take the following steps:

1. Remove the permanent XPC service permissions for media-related mach connections.
2. Dynamically create these connections when not using the GPU Process.
3. If the GPU Process is in use, do not open connections -- they should be provided by the GPU Process sandbox.

[Radar WebKit Bug Importer]

<rdar://problem/59449750>

[Brent Fulgham]

Created attachment 390728 [details]
Patch

[Brent Fulgham]

Comment on attachment 390728 [details]
Patch

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

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:377
> +        SandboxExtension::createHandleForMachLookup("com.apple.nesessionmanager", WTF::nullopt, managerHandle);

I think the multiple WTFMove(handle) calls might do something wrong.
@Chris: Does the right thing happen if we pass a moved object to a method taking a reference to be assigned to?

[Per Arne Vollan]

Comment on attachment 390728 [details]
Patch

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

> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:-126
> -           (xpc-service-name "com.apple.audio.toolbox.reporting.service")))

This service does not seem to appear elsewhere in the patch, or am I mistaken?

> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:-212
> -        (xpc-service-name "com.apple.MediaPlayer.RemotePlayerService"))

Ditto.

[Per Arne Vollan]

Comment on attachment 390728 [details]
Patch

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

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:226
> +        "com.apple.coremedia.volumecontroller.xpc", "com.apple.accessibility.mediaaccessibilityd",

The service "com.apple.accessibility.mediaaccessibilityd" does not seem to have been removed from the macOS sandbox.

[Brent Fulgham]

(In reply to Per Arne Vollan from comment #5)
> Comment on attachment 390728 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=390728&action=review
> 
> > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:226
> > +        "com.apple.coremedia.volumecontroller.xpc", "com.apple.accessibility.mediaaccessibilityd",
> 
> The service "com.apple.accessibility.mediaaccessibilityd" does not seem to
> have been removed from the macOS sandbox.

Whoops! I'll definitely fix that.

[Brent Fulgham]

Created attachment 390795 [details]
Patch

[Brent Fulgham]

Created attachment 390797 [details]
Patch

[Per Arne Vollan]

Comment on attachment 390797 [details]
Patch

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

> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:855
> +            "com.apple.audio.AudioComponentRegistrar" "com.apple.audio.AudioSession" "com.apple.MediaPlayer.RemotePlayerService"
> +            "com.apple.audio.toolbox.reporting.service" "com.apple.coremedia.admin" "com.apple.coremedia.asset.xpc"

I think "com.apple.audio.toolbox.reporting.service" and "com.apple.MediaPlayer.RemotePlayerService" needs to be in a separate extension rule covering xpc-service-name.

> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:852
> +            "com.apple.accessibility.mediaaccessibilityd"

I think this service should be in a separate extension rule covering xpc-service-name.

[Brent Fulgham]

Created attachment 390819 [details]
Patch

[Brent Fulgham]

Comment on attachment 390797 [details]
Patch

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

>> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:855
>> +            "com.apple.audio.toolbox.reporting.service" "com.apple.coremedia.admin" "com.apple.coremedia.asset.xpc"
> 
> I think "com.apple.audio.toolbox.reporting.service" and "com.apple.MediaPlayer.RemotePlayerService" needs to be in a separate extension rule covering xpc-service-name.

Done.

>> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:852
>> +            "com.apple.accessibility.mediaaccessibilityd"
> 
> I think this service should be in a separate extension rule covering xpc-service-name.

Done.

[Per Arne Vollan]

Comment on attachment 390819 [details]
Patch

Looks good! R=me.

[WebKit Commit Bot]

Comment on attachment 390819 [details]
Patch

Clearing flags on attachment: 390819

Committed r256660: <https://trac.webkit.org/changeset/256660>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.