Bug 217945 - [WebAuthn] Determine an AAGUID for the platform authenticators
Summary: [WebAuthn] Determine an AAGUID for the platform authenticators
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jiewen Tan
URL:
Keywords: InRadar
Depends on:
Blocks: 181943
  Show dependency treegraph
 
Reported: 2020-10-20 01:31 PDT by Frederic Jahn
Modified: 2020-11-05 00:47 PST (History)
4 users (show)

See Also:


Attachments
Patch (5.71 KB, patch)
2020-11-04 01:50 PST, Jiewen Tan
bfulgham: review+
bfulgham: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (5.97 KB, patch)
2020-11-04 23:45 PST, Jiewen Tan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frederic Jahn 2020-10-20 01:31:57 PDT
Our implementation (and surely some others) to allow-list certain authenticators is based on them being present in the FIDO Metadata Service.
To add Apple Authenticators to the list we would have to hard code a separate code path.
If you would generate a uuid and serve them in your attestation plus adding the Apple Authenticators to
https://mymds2.fidoalliance.org/ would definitely ensure better interoperability and make the life of developers easier.
Comment 1 Smoley 2020-10-22 14:01:38 PDT
Thanks for filing, I'll Cc a few folks here for consideration.
Comment 2 Radar WebKit Bug Importer 2020-10-22 14:01:48 PDT Comment hidden (obsolete)
Comment 3 Jiewen Tan 2020-10-29 09:15:03 PDT
<rdar://problem/70811618>
Comment 4 Jiewen Tan 2020-11-04 01:50:35 PST
Created attachment 413150 [details]
Patch
Comment 5 Brent Fulgham 2020-11-04 13:52:53 PST
Comment on attachment 413150 [details]
Patch

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

This looks good to me, but I'd like you to double-check the none-attestation case before we land.

> Source/WebKit/ChangeLog:9
> +        The AAGUID is randomly generated by CCRandomGenerateBytes.

I think we should say:

"Relying parties use the AAGUID to recognize supported authenticators. Using a NULL AAGUID blocks them from recognizing Apple products as valid WebAuthentication targets. We need to assign ourselves a GUID representing Apple authenticators, then publish with our attestation certificate and with the FIDO Alliance."

It would also be good to reference the communication to the FIDO alliance documenting this GUID (not sure if this would be a pull request, or how that works).

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:373
> +        auto authData = buildAuthData(creationOptions.rp.id, makeCredentialFlags, counter, buildAttestedCredentialData(Vector<uint8_t>(aaguidLength, 0), credentialId, cosePublicKey));

Do we not want to indicate that we are an Apple authenticator for the none-attestation case? From Frederic Jahn's bug report, it sounds like this is needed to decide whether they allow our authenticator at all.

If you aren't sure, can you check with Frederic?
Comment 6 Jiewen Tan 2020-11-04 23:43:45 PST
Comment on attachment 413150 [details]
Patch

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

Thanks Brent for r+ this patch.

>> Source/WebKit/ChangeLog:9
>> +        The AAGUID is randomly generated by CCRandomGenerateBytes.
> 
> I think we should say:
> 
> "Relying parties use the AAGUID to recognize supported authenticators. Using a NULL AAGUID blocks them from recognizing Apple products as valid WebAuthentication targets. We need to assign ourselves a GUID representing Apple authenticators, then publish with our attestation certificate and with the FIDO Alliance."
> 
> It would also be good to reference the communication to the FIDO alliance documenting this GUID (not sure if this would be a pull request, or how that works).

Sure. Fixed.

>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:373
>> +        auto authData = buildAuthData(creationOptions.rp.id, makeCredentialFlags, counter, buildAttestedCredentialData(Vector<uint8_t>(aaguidLength, 0), credentialId, cosePublicKey));
> 
> Do we not want to indicate that we are an Apple authenticator for the none-attestation case? From Frederic Jahn's bug report, it sounds like this is needed to decide whether they allow our authenticator at all.
> 
> If you aren't sure, can you check with Frederic?

For none attestation, the AAGUID is required to be all zero.
Comment 7 Jiewen Tan 2020-11-04 23:45:15 PST
Created attachment 413262 [details]
Patch for landing
Comment 8 EWS 2020-11-05 00:47:02 PST
Committed r269420: <https://trac.webkit.org/changeset/269420>

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