[webkit-reviews] review granted: [Bug 226772] [Cocoa] Harden WebAuthn process by restricting to browser-entitled processes : [Attachment 431223] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Jun 13 12:00:46 PDT 2021

Darin Adler <darin at apple.com> has granted Brent Fulgham <bfulgham at webkit.org>'s
request for review:
Bug 226772: [Cocoa] Harden WebAuthn process by restricting to browser-entitled

Attachment 431223: Patch


--- Comment #7 from Darin Adler <darin at apple.com> ---
Comment on attachment 431223
  --> https://bugs.webkit.org/attachment.cgi?id=431223

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

> Source/WTF/wtf/cocoa/Entitlements.h:39
> +WTF_EXPORT_PRIVATE String processEntitlementValue(audit_token_t, const char*

Given this is used only in Cocoa-specific code, should it return
RetainPtr<CFStringRef> instead?

Or since it’s only used for equality checks, maybe it should be:

    WTF_EXPORT_PRIVATE bool hasEntitlementValue(audit_token_t, const char*
entitlement, const char* value);

Then the client could just write:

    if (!WTF:: hasEntitlementValue(auditToken.value(),
"com.apple.pac.shared_region_id", "WebContent") {

> Source/WTF/wtf/cocoa/Entitlements.mm:72
> +    if (!value)
> +	   return { };
> +
> +    if (CFGetTypeID(value.get()) != CFStringGetTypeID())

Better as a single if, I think.

Might even want to put a helper somewhere because it’s *so* common for us to
want to check both nullity and if a CFTypeRef is a CFStringRef at the same

Someone motivated might even find a way to make it work with the syntax
is<CFStringRef> and downcast<CFStringRef> the way we do with checked casts in
our DOM classes, which would be neat! Obviously not for this patch. I’d just do
it here in this file probably for now and refactor later.

More information about the webkit-reviews mailing list