Bug 218870 – [macOS] Perform AX TCC check in the UI process

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Per Arne Vollan]

On behalf of the WebContent process, perform AX TCC check in the Networking process on macOS. This is in preparation of blocking tccd in the WebContent process.

[Radar WebKit Bug Importer]

<rdar://problem/71339830>

[Per Arne Vollan]

Created attachment 413973 [details]
Patch

[Per Arne Vollan]

Created attachment 413974 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 413974 [details]
Patch

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

> Source/WebCore/PAL/ChangeLog:3
> +        [macOS] Perform AX TCC check in the Networking process

I know that Networking process does more than networking, but making it responsible for accessibility in any way goes beyond reasonable in my opinion.

[Per Arne Vollan]

Created attachment 414045 [details]
Patch

[Per Arne Vollan]

(In reply to Alexey Proskuryakov from comment #4)
> Comment on attachment 413974 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=413974&action=review
> 
> > Source/WebCore/PAL/ChangeLog:3
> > +        [macOS] Perform AX TCC check in the Networking process
> 
> I know that Networking process does more than networking, but making it
> responsible for accessibility in any way goes beyond reasonable in my
> opinion.

Yes, that is a very good point. I have moved this to the UI process in the latest patch.

Thanks for reviewing!

[Brent Fulgham]

Comment on attachment 414045 [details]
Patch

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

> Source/WebCore/PAL/pal/spi/mac/HIServicesSPI.h:161
> +typedef Boolean (*AXAuditTokenIsAuthenticatedCallback)(audit_token_t auditToken);

We might not need the 'auditToken' name for the argument here.

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:184
> +static Boolean isAXAuthenticatedCallback(audit_token_t auditToken)

Do we know how frequently this is called? Is it just once at process launch, or does it happen frequently in UIKit/AppKit code?

Would it be better to perform the message send asynchronously as part of the launch process, and store the result so that this callback is very quick?

[Per Arne Vollan]

(In reply to Brent Fulgham from comment #7)
> Comment on attachment 414045 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=414045&action=review
> 
> > Source/WebCore/PAL/pal/spi/mac/HIServicesSPI.h:161
> > +typedef Boolean (*AXAuditTokenIsAuthenticatedCallback)(audit_token_t auditToken);
> 
> We might not need the 'auditToken' name for the argument here.
> 

Will fix!

> > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:184
> > +static Boolean isAXAuthenticatedCallback(audit_token_t auditToken)
> 
> Do we know how frequently this is called? Is it just once at process launch,
> or does it happen frequently in UIKit/AppKit code?
> 

This will only be called when an Accessibility client (e.g. VoiceOver) makes the first request to the WebContent process, so it is not called frequently.

> Would it be better to perform the message send asynchronously as part of the
> launch process, and store the result so that this callback is very quick?

That's a good point, although in this case I don't believe we can cache the result, since the result of this function call depends on the provided audit token, which is the audit token of the client making the accessibility request.

Thanks for reviewing!

[Brent Fulgham]

Comment on attachment 414045 [details]
Patch

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

r=me. Please monitor perf bots (as you always do!) :-)

> Source/WebCore/PAL/pal/spi/mac/HIServicesSPI.h:161
> +typedef Boolean (*AXAuditTokenIsAuthenticatedCallback)(audit_token_t auditToken);

We might not need the 'auditToken' name for the argument here.

[Per Arne Vollan]

Created attachment 414338 [details]
Patch

[Per Arne Vollan]

(In reply to Brent Fulgham from comment #9)
> Comment on attachment 414045 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=414045&action=review
> 
> r=me. Please monitor perf bots (as you always do!) :-)
> 

Will do!

> > Source/WebCore/PAL/pal/spi/mac/HIServicesSPI.h:161
> > +typedef Boolean (*AXAuditTokenIsAuthenticatedCallback)(audit_token_t auditToken);
> 
> We might not need the 'auditToken' name for the argument here.

Fixed in latest patch.

Thanks for reviewing!

[EWS]

Patch 414338 does not build

[Per Arne Vollan]

Created attachment 414344 [details]
Patch

[EWS]

Committed r269906: <https://trac.webkit.org/changeset/269906>

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