Bug 203618 – It should be possible to create a mach sandbox extension for the WebContent process before the audit token is known

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

[Per Arne Vollan]

Currently, we are only able to create a mach sandbox extension for the WebContent process if we know its audit token. It should be possible to create a mach extension without the audit token, since this is needed when we want to create extensions before the PID or audit token is known. These extensions are typically sent in the WebProcess creation parameters.

[Radar WebKit Bug Importer]

<rdar://problem/56750384>

[Per Arne Vollan]

Created attachment 382332 [details]
Patch

[EWS Watchlist]

Comment on attachment 382332 [details]
Patch

Attachment 382332 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/13192015

New failing tests:
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-ftl-eager-no-cjit-validate-phases

[Brent Fulgham]

Comment on attachment 382332 [details]
Patch

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

> Source/WebKit/ChangeLog:11
> +        typically sent in the WebProcess creation parameters.

Is there a way to ensure we only call this no-audit-token code in cases where we don't have a connection yet? Maybe have the code take a ProcessProxy object and get its connection internally so a future developer can't accidentally misuse the API?

> Source/WebKit/UIProcess/ios/WebProcessProxyIOS.mm:58
> +    if (!SandboxExtension::createHandleForMachLookup("com.apple.iphone.axserver-systemwide", connection() ? connection()->getAuditToken() : WTF::nullopt, handle))

What if we passed the {Web|Network|ProcessProxy here (instead of the audit token pointer) so we could always confirm we were only passing a nullptr audit token when a connection was missing.

[Per Arne Vollan]

(In reply to Brent Fulgham from comment #4)
> Comment on attachment 382332 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=382332&action=review
> 
> > Source/WebKit/ChangeLog:11
> > +        typically sent in the WebProcess creation parameters.
> 
> Is there a way to ensure we only call this no-audit-token code in cases
> where we don't have a connection yet? Maybe have the code take a
> ProcessProxy object and get its connection internally so a future developer
> can't accidentally misuse the API?
> 
> > Source/WebKit/UIProcess/ios/WebProcessProxyIOS.mm:58
> > +    if (!SandboxExtension::createHandleForMachLookup("com.apple.iphone.axserver-systemwide", connection() ? connection()->getAuditToken() : WTF::nullopt, handle))
> 
> What if we passed the {Web|Network|ProcessProxy here (instead of the audit
> token pointer) so we could always confirm we were only passing a nullptr
> audit token when a connection was missing.

I think this is an excellent idea to enforce providing the audit token when available. However, I see that we normally don't include things from WebKit (e.g. #include <WebKit/WebProcessProxy.h>) from files in the WebKit Shared folder. Given this, should I still make the change?

Thanks for reviewing!

[Brent Fulgham]

Comment on attachment 382332 [details]
Patch

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

>>> Source/WebKit/ChangeLog:11
>>> +        typically sent in the WebProcess creation parameters.
>> 
>> Is there a way to ensure we only call this no-audit-token code in cases where we don't have a connection yet? Maybe have the code take a ProcessProxy object and get its connection internally so a future developer can't accidentally misuse the API?
> 
> I think this is an excellent idea to enforce providing the audit token when available. However, I see that we normally don't include things from WebKit (e.g. #include <WebKit/WebProcessProxy.h>) from files in the WebKit Shared folder. Given this, should I still make the change?
> 
> Thanks for reviewing!

I see -- since this is used in UIProcess and {Web|Network}Process, this would be a layering violation.

Okay -- let's leave it as-is.

[Per Arne Vollan]

Comment on attachment 382332 [details]
Patch

Thanks!

[WebKit Commit Bot]

Comment on attachment 382332 [details]
Patch

Clearing flags on attachment: 382332

Committed r251825: <https://trac.webkit.org/changeset/251825>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.