Bug 208676

Summary: [WebAuthn] Do not perform Attestation with type is 'none'
Product: WebKit Reporter: Jiewen Tan <jiewen_tan>
Component: WebKit Misc.Assignee: Jiewen Tan <jiewen_tan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, jiewen_tan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
bfulgham: review+
Patch for Landing none

Description Jiewen Tan 2020-03-05 17:00:19 PST 
Avoid Apple Attestation when attestation = "none".
Comment 1 Jiewen Tan 2020-03-05 17:00:32 PST 
<rdar://problem/59692104>
Comment 2 Jiewen Tan 2020-03-05 17:14:29 PST 
Created attachment 392653 [details]
Patch
Comment 3 Brent Fulgham 2020-03-06 12:30:55 PST 
Comment on attachment 392653 [details]
Patch

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

> Source/WebKit/ChangeLog:3
> +        [WebAuthn] Avoid Apple Attestation when attestation = "none"

Maybe call this "Do not perform Attestation with type is 'none'"?

> Source/WebKit/ChangeLog:10
> +        accesses to Apple Attestation for now. The whitelist includes file URL,

"... to restrict access until validation is complete. The whitelist allows file URLs and test-related domains."

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:101
> +// FIXME<rdar://problem/60108131>: Remove this whitelist before shipping.

I think its enough just say:

// FIXME(<rdar://problem/60108131>): Remove this whitelist once testing is complete.

> LayoutTests/ChangeLog:3
> +        [WebAuthn] Avoid Apple Attestation when attestation = "none"

Ditto (change title).
Comment 4 Jiewen Tan 2020-03-06 12:37:10 PST 
Comment on attachment 392653 [details]
Patch

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

Thanks Brent for r+ this patch.

>> Source/WebKit/ChangeLog:3
>> +        [WebAuthn] Avoid Apple Attestation when attestation = "none"
> 
> Maybe call this "Do not perform Attestation with type is 'none'"?

Fixed.

>> Source/WebKit/ChangeLog:10
>> +        accesses to Apple Attestation for now. The whitelist includes file URL,
> 
> "... to restrict access until validation is complete. The whitelist allows file URLs and test-related domains."

Fixed.

>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:101
>> +// FIXME<rdar://problem/60108131>: Remove this whitelist before shipping.
> 
> I think its enough just say:
> 
> // FIXME(<rdar://problem/60108131>): Remove this whitelist once testing is complete.

Fixed.

>> LayoutTests/ChangeLog:3
>> +        [WebAuthn] Avoid Apple Attestation when attestation = "none"
> 
> Ditto (change title).

Fixed.
Comment 5 Jiewen Tan 2020-03-06 12:41:37 PST 
Created attachment 392757 [details]
Patch for Landing
Comment 6 Jiewen Tan 2020-03-06 12:42:48 PST 
Committed r258020: <https://trac.webkit.org/changeset/258020>