Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to guarantee created resident key is actually received by RP in adverse networking conditions? #2038

Closed
arianvp opened this issue Mar 4, 2024 · 7 comments · Fixed by #2093

Comments

@arianvp
Copy link

arianvp commented Mar 4, 2024

I've noticed that under adverse network conditions (very common on mobile phones; especially in rural areas) it can happen quite often that the navigator.credentials.create call succeeds but the credential never gets received by the RP and users end up with many "junk" resident keys in their Apple Keychain because of it. There seems to be no way to avoid this happening. excludeCredentials is not an option as the RP isn't even aware that the credential is created. And the user is frustrated because they half of the time select the wrong passkey.

Note this is an issue I have actually observed in the wild. (in this image only one of the two passkeys work. One was created whilst artificially throttling the network in Dev Tools, causing the credential to never be received by the RP)
image

How are RPs supposed to make this robust? My only solution so far is to drop the whole idea of allowing people to create resident credentials. Instead always opt for using server-side credentials.

There is also no way for the device that stores the credentials to know which credentials are valid and which not; so it can also not be fixed on that side.

Hypothetically I think perhaps a hybrid flow would be nice where a server-side credential can be "upgraded" to a resident one during the first get() call. Anything else seems to be prone to failure.

  1. With the initial create() call; the credential is initially stored as a server side credential as credential id cid1
  2. The first get() MUST include allowCredentials: [ cid1 ]
  3. Only after the first get() call does the authenticator actually store the credential as resident key. And maybe we can signal in the response from get() that the key is now stored as resident
  4. subsequent calls to get() can be done without allowCredentials

Originally posted by @arianvp in #1569 (comment)

@arianvp arianvp changed the title How to guarantee resident key is actually received by RP in adverse networking conditions? How to guarantee created resident key is actually received by RP in adverse networking conditions? Mar 4, 2024
@timcappalli
Copy link
Member

timcappalli commented Mar 4, 2024

This should be addressed by https://github.com/w3c/webauthn/wiki/Explainer:-WebAuthn-Report-API-explainer

I'd also add that the RP should be using the same user handle which will prevent multiple credentials for the same account in the same authenticator.

@arianvp
Copy link
Author

arianvp commented Mar 4, 2024

Oh good point about making sure the same userHandle is reused! I implemented a user registration page rather naively. I guess it would make sense to write some more guidance for RPs here.

This is what I implemented:

  1. I have a registration page where people can register an account by creating a passkey
  2. On page load of /register I generated a fresh userHandle and challenge . People then register by picking a userName and creating a passkey.
  3. If then due to networking conditions this step fails the user tries again. they refresh the page. Which causes a new userHandle and challenge to be allocated.
  4. now the user has two passkeys in their keychain, with different userHandle's but same userNames. Only one of them work.

I guess I should fix this as an RP by:

  1. On the register page generate a userHandle just once and save it in the session cookie.
  2. If person aborts the registration (e.g. due to networking errors) and reloads the page they use the same userHandle
  3. If I understand correctly the new passkey then overrides the old passkey in the UI (Due to having the same userHandle ?)

@timcappalli
Copy link
Member

If I understand correctly the new passkey then overrides the old passkey in the UI (Due to having the same userHandle ?)

Yes, that's correct

@arianvp
Copy link
Author

arianvp commented Mar 26, 2024

Related to this problem -- at least Safari seems to completely ignore excludeCredentials. This means users of our RP end up with multiple iCloud passkeys in their account and there is nothing stopping them from registering with the same authenticator multiple times. They then don't know which one is the "Active" passkey and when they delete the wrong one they get locked out of their account forever. As Safari will delete the old passkey as soon as you register the new one.

Edit: Issue is tracked here: https://bugs.webkit.org/show_bug.cgi?id=270553

@arianvp
Copy link
Author

arianvp commented Mar 26, 2024

How can I protect myself against a misbehaving authenticator that ignores excludeCredentials like Safari?

Now the following scenario can happen which is even worse than the original issue. Namely account lockout for existing accounts:

  1. Register passkey
  2. Log in
  3. Click register passkey button again. Safari overrides the passkey in your keychain in-place.
  4. Network is lost and the response from navigator.credentials.create() doesn't arrive at the RP
  5. You're now completely locked out of your account as the first Passkey doesn't work anymore as it's not in your keychain anymore

@timcappalli
Copy link
Member

timcappalli commented Mar 26, 2024

Authenticators and clients are expected to be spec compliant. There is only so much that can be done at a spec level.

Please ask these authenticator-specific questions via a developer/deployment channel (FIDO-DEV, Passkeys Developer, etc).

@arianvp
Copy link
Author

arianvp commented Mar 27, 2024

To make a concrete proposal to The spec:

The spec currently reads

excludeCredentialDescriptorList
An OPTIONAL list of PublicKeyCredentialDescriptor objects provided by the Relying Party with the intention that, if any of these are known to the authenticator, it SHOULD NOT create a new credential. excludeCredentialDescriptorList contains a list of known credentials.

Which gives the impression that implementing excludeCredentials is optional for authenticators. But I'd say it should be mandatory for client side discoverable credentials.

Given that this overrides discoverable credentials that can lead to user lockout I think we should change the wording to SHALL NOT or MUST NOT.

We should also make it clear that calling create without excludeCredentials can lead to lockout. Calling this an OPTIONAL list is maybe a bit too weak too?

I think both the sections for RPs and for Authenticators could use a bit more guidance on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants