Bug 213357 – [iOS, macOS] Allow access to the container manager to support Mail InjectedBundle

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

[Brent Fulgham]

The Mail Injected Bundle requires access to the container manager to support certain OS operations. We do not need this access for web browsing, and should limit this access to this one case.

This patch creates a dynamic mach extension to the container manager for this single use case.

[Brent Fulgham]

<rdar://problem/63837247>

[Brent Fulgham]

Created attachment 402240 [details]
Patch

[Brent Fulgham]

Created attachment 402242 [details]
Patch

[Darin Adler]

Comment on attachment 402242 [details]
Patch

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

Is there any more scalable way to do this? Seems unfortunate to hardcode a client app bundle ID again.

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:299
> +#else

How about this instead?

#elif PLATFORM(IOS)
    return WebCore::IOSApplication::isMobileMail()
#else
    return false;
#endif

[David Kilzer (:ddkilzer)]

Comment on attachment 402242 [details]
Patch

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

>> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:299
>> +#else
> 
> How about this instead?
> 
> #elif PLATFORM(IOS)
>     return WebCore::IOSApplication::isMobileMail()
> #else
>     return false;
> #endif

The problem is that it’s not just MobileMail that needs access.  We see other apps that use the WebContent process and talk to containermanagerd as well.

This only fixes the majority of cases, but not all of them.

[Brent Fulgham]

Comment on attachment 402242 [details]
Patch

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

>>> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:299
>>> +#else
>> 
>> How about this instead?
>> 
>> #elif PLATFORM(IOS)
>>     return WebCore::IOSApplication::isMobileMail()
>> #else
>>     return false;
>> #endif
> 
> The problem is that it’s not just MobileMail that needs access.  We see other apps that use the WebContent process and talk to containermanagerd as well.
> 
> This only fixes the majority of cases, but not all of them.

Darin: Will do!

David: I'll add telemetry-backtrace so we get a backtrace if this is hit by anyone else. I'm very interested to see how that's possible.

[Brent Fulgham]

Created attachment 402319 [details]
Patch for landing

[EWS]

Committed r263287: <https://trac.webkit.org/changeset/263287>

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

[Ryan Haddad]

This change caused testing to exit early on Catalina due to crashes:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebKit              	0x0000000104e58d95 WebKit::AuxiliaryProcess::initializeSandbox(WebKit::AuxiliaryProcessInitializationParameters const&, WebKit::SandboxInitializationParameters&) + 6783 (AuxiliaryProcessMac.mm:693)
1   com.apple.WebKit              	0x00000001050fcdb0 WebKit::WebProcess::initializeSandbox(WebKit::AuxiliaryProcessInitializationParameters const&, WebKit::SandboxInitializationParameters&) + 196 (WebProcessCocoa.mm:589)
2   com.apple.WebKit              	0x0000000104c8e1ff WebKit::AuxiliaryProcess::initialize(WebKit::AuxiliaryProcessInitializationParameters const&) + 103
3   com.apple.WebKit              	0x000000010506cab9 void WebKit::XPCServiceInitializer<WebKit::WebProcess, WebKit::XPCServiceInitializerDelegate>(WTF::OSObjectPtr<NSObject<OS_xpc_object>*>, NSObject<OS_xpc_object>*, NSObject<OS_xpc_object>*) + 645 (XPCServiceEntryPoint.h:135)
4   com.apple.WebKit              	0x000000010506c7fe WebContentServiceInitializer + 61 (WebContentServiceEntryPoint.mm:53)


https://build.webkit.org/builders/Apple-Catalina-Release-WK2-Tests/builds/6485

[Ryan Haddad]

Created attachment 402363 [details]
Catalina crash log

[Ryan Haddad]

Reverted in https://trac.webkit.org/changeset/263307/webkit

[Brent Fulgham]

Created attachment 402397 [details]
Patch

[EWS]

Committed r263320: <https://trac.webkit.org/changeset/263320>

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