[webkit-reviews] review granted: [Bug 191533] [WebAuthN] Use a real nonce for CTAPHID_INIT : [Attachment 354782] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Jiewen Tan
<jiewen_tan at apple.com>'s request for review:
Bug 191533: [WebAuthN] Use a real nonce for CTAPHID_INIT
https://bugs.webkit.org/show_bug.cgi?id=191533

Attachment 354782: Patch

https://bugs.webkit.org/attachment.cgi?id=354782&action=review




--- Comment #2 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 354782
  --> https://bugs.webkit.org/attachment.cgi?id=354782
Patch

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

Looks good. r=me

> Source/WebKit/ChangeLog:4
> +	   https://bugs.webkit.org/show_bug.cgi?id=191533

Please always include a radar for these changes.

> Source/WebKit/ChangeLog:14
> +	   sufficient as the arrived init response will piped in
HidConnection::m_inputReports, which is designed in

"will BE piped THROUGH HidConnection::m_inputReports..."

> Source/WebKit/ChangeLog:15
> +	   purpose to store any data packets within (initialized, terminated)
time interval to prevent data loss in

"... which is designed ON purpose to ..."

> Source/WebKit/UIProcess/WebAuthentication/fido/CtapHidDriver.cpp:59
> +    // HidConnection could hold data from other applications, and thereofore
invalidate it before each transaction.

It seems like this could be the source of difficult-to-debug issues in the
future, though I guess not in WebKit after this change. I wonder how other
groups deal with HID connections that contain existing data.