Bug 189289 – [WebAuthN] Import CTAP HID message and packet structure from Chromium

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

[Jiewen Tan]

This task aims to import CTAP message coder from Chromium including but not limited to:
1. Commands: CtapMakeCredentialRequest, CtapGetAssertionRequest.
2. Responses: device_response_converter.
3. CTAP to U2F: u2f_command_constructor, AuthenticatorMakeCredentialResponse::CreateFromU2fRegisterResponse, AuthenticatorGetAssertionResponse::CreateFromU2fSignResponse.

[Radar WebKit Bug Importer]

<rdar://problem/44120310>

[Jiewen Tan]

Created attachment 351870 [details]
Patch

[EWS Watchlist]

Attachment 351870 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/webauthn/fido/FidoHidMessage.h:65:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/Modules/webauthn/fido/FidoHidMessage.h:66:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/Modules/webauthn/fido/FidoHidMessage.h:67:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/Modules/webauthn/fido/FidoHidPacket.h:53:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/Modules/webauthn/fido/FidoHidPacket.h:54:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/Modules/webauthn/fido/FidoHidPacket.h:80:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/Modules/webauthn/fido/FidoHidPacket.h:81:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/Modules/webauthn/fido/FidoHidPacket.h:104:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 8 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brent Fulgham]

Comment on attachment 351870 [details]
Patch

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

> Source/WebCore/Modules/webauthn/fido/FidoHidMessage.cpp:89
> +    if (initPacket == nullptr)

if (!initPacket)

> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:-194
> -

Please remove this whitespace change.

[Brent Fulgham]

Comment on attachment 351870 [details]
Patch

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

> Source/WebCore/Modules/webauthn/fido/FidoHidMessage.h:67
> +    const Deque<std::unique_ptr<FidoHidPacket>>& getPacketsForTesting() const { return m_packets; }

Since this is a WEBCORE_EXPORT class, you should move these inlines to the implementation file to avoid the compiler getting angry:

"Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]"

> Source/WebCore/Modules/webauthn/fido/FidoHidPacket.h:54
> +    uint32_t channelId() const { return m_channelId; }

Since this is a WEBCORE_EXPORT class, you should move these inlines to the implementation file to avoid the compiler getting angry:

"Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]"

> Source/WebCore/Modules/webauthn/fido/FidoHidPacket.h:81
> +    uint16_t payloadLength() const { return m_payloadLength; }

Since this is a WEBCORE_EXPORT class, you should move these inlines to the implementation file to avoid the compiler getting angry:

"Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]"

> Source/WebCore/Modules/webauthn/fido/FidoHidPacket.h:104
> +    uint8_t sequence() const { return m_sequence; }

Since this is a WEBCORE_EXPORT class, you should move these inlines to the implementation file to avoid the compiler getting angry:

"Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]"

[Jiewen Tan]

Comment on attachment 351870 [details]
Patch

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

Thanks Brent for r+ this patch.

>> Source/WebCore/Modules/webauthn/fido/FidoHidMessage.cpp:89
>> +    if (initPacket == nullptr)
> 
> if (!initPacket)

Fixed.

>> Source/WebCore/Modules/webauthn/fido/FidoHidMessage.h:67
>> +    const Deque<std::unique_ptr<FidoHidPacket>>& getPacketsForTesting() const { return m_packets; }
> 
> Since this is a WEBCORE_EXPORT class, you should move these inlines to the implementation file to avoid the compiler getting angry:
> 
> "Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]"

Fixed.

>> Source/WebCore/Modules/webauthn/fido/FidoHidPacket.h:54
>> +    uint32_t channelId() const { return m_channelId; }
> 
> Since this is a WEBCORE_EXPORT class, you should move these inlines to the implementation file to avoid the compiler getting angry:
> 
> "Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]"

Fixed.

>> Source/WebCore/Modules/webauthn/fido/FidoHidPacket.h:81
>> +    uint16_t payloadLength() const { return m_payloadLength; }
> 
> Since this is a WEBCORE_EXPORT class, you should move these inlines to the implementation file to avoid the compiler getting angry:
> 
> "Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]"

Fixed.

>> Source/WebCore/Modules/webauthn/fido/FidoHidPacket.h:104
>> +    uint8_t sequence() const { return m_sequence; }
> 
> Since this is a WEBCORE_EXPORT class, you should move these inlines to the implementation file to avoid the compiler getting angry:
> 
> "Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]"

Fixed.

>> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:-194
>> -
> 
> Please remove this whitespace change.

Fixed.

[Jiewen Tan]

Comment on attachment 351870 [details]
Patch

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

>>> Source/WebCore/Modules/webauthn/fido/FidoHidMessage.h:67
>>> +    const Deque<std::unique_ptr<FidoHidPacket>>& getPacketsForTesting() const { return m_packets; }
>> 
>> Since this is a WEBCORE_EXPORT class, you should move these inlines to the implementation file to avoid the compiler getting angry:
>> 
>> "Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]"
> 
> Fixed.

Just confirmed with Alex that inline methods are allowed in classes that are marked as WEBCORE_EXPORT and it is the style checker bug.

[Jiewen Tan]

Created attachment 351900 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 351900 [details]
Patch for landing

Clearing flags on attachment: 351900

Committed r236976: <https://trac.webkit.org/changeset/236976>