|Summary:||[WebAuthn] Cache the PIN to improve NFC user experience|
|Product:||WebKit||Reporter:||login Llama <loginllama>|
|Component:||WebCore Misc.||Assignee:||Jiewen Tan <jiewen_tan>|
|Severity:||Normal||CC:||bfulgham, jiewen_tan, loginllama, webkit-bug-importer|
|Version:||Safari Technology Preview|
|Hardware:||iPhone / iPad|
|Bug Depends on:|
Description login Llama 2020-07-02 14:38:44 PDT
In iOS 14 developer beta The authenticator has a pin set: The Authenticator is attached over USB/Lightning or NFC. Tf the external key is removed from the NFC field or USB/Lightning port after the initial CTAP2 getKeyAgrement command getPinToken will fail. The problem is that the getKeyAgrement happens before the user is prompted to enter pin. With NFC the natural thing is to remove the key form the NFC field to use your hand to type the pin then tap again. Unfortunately, since keys are powered via USB or NFC the key will generate a new EC key pair and that will not agree with the one the phone got prior to the power interuption. The way that Windows 10 implemented it is with two taps. The first tap the platform gets authenticatorGetInfo. If the authenticator receives the option uv = True then it proceeds directly to getAssertion with the option uv=1 to trigger internal UV if that fails the authenticator will return pin-required. If the authenticatorGetInfo has pinToken = true and (uv is not True or received a pin-required error from the previous step): Prompt user for pin Prompt user to tap again or re-insert. Perform authenticatorClientPIN (0x06) with subcommand getKeyAgreement (0x03) Perform authenticatorClientPIN (0x06) with subcommand getPINToken (0x05) Perform authenticatorGetAssertion (0x02) with pinAuth form the previous step (might require more than one call depending on allow list size.) Windows supports CCID NFC readers on the desktop. Android only supports CTAP1 over NFC so is not a good example. Brave uses the Yubico SDK to implement NFC support. You can see the two tap pattern there. Unfortunately, Brave seems to be having a NFC issue on iOS 14 but may work on iOS 13 or did not long ago.
Comment 3 Jiewen Tan 2020-07-15 02:03:24 PDT
Comment on attachment 404328 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404328&action=review > Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:292 > + m_pendingRequestData.cachedPin = pin; Oops, forgot to use the weak pointer.
Comment 5 Brent Fulgham 2020-07-17 14:50:40 PDT
Comment on attachment 404364 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404364&action=review r=me > Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:284 > + // issue. The probability of the issue, however, should be rare. I think this is pretty well documented in the Bugzilla. I would suggest a shorter note: // Cache the PIN to improve NFC user experience so that a momentary movement of the NFC key away from the scanner doesn't force the PIN entry to be re-entered. Question: Should this have some kind of timeout to clear it if some period of time passes? That might be a reasonable followup patch if you think it's needed. I don't think it's required here.
Comment 6 Jiewen Tan 2020-07-17 15:13:14 PDT
Comment on attachment 404364 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404364&action=review Thanks Brent for r+ this patch. >> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:284 >> + // issue. The probability of the issue, however, should be rare. > > I think this is pretty well documented in the Bugzilla. > > I would suggest a shorter note: > > // Cache the PIN to improve NFC user experience so that a momentary movement of the NFC key away from the scanner doesn't force the PIN entry to be re-entered. > > Question: Should this have some kind of timeout to clear it if some period of time passes? > > That might be a reasonable followup patch if you think it's needed. I don't think it's required here. Fixed. For the timer, I'm not sure. The API has a global timeout timer which has a maximum value of 120s. After, the whole request will be cancelled. I guess it's enough?
Comment 8 EWS 2020-07-17 15:58:55 PDT
Committed r264543: <https://trac.webkit.org/changeset/264543> All reviewed patches have been landed. Closing bug and clearing flags on attachment 404603 [details].