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

Prevent browsers from deleting credentials that the RP wanted to be server-side #1569

Open
lgarron opened this issue Feb 9, 2021 · 22 comments
Assignees
Labels
@Risk Items that are at risk for L3 type:technical
Milestone

Comments

@lgarron
Copy link
Contributor

lgarron commented Feb 9, 2021

At the moment, Safari (using Touch ID/Face ID) and Windows Hello create discoverable credentials even if the RP does not require/prefer "resident key". This somewhat makes sense, since there is no way for the RP disallow discoverable credentials — only "discourage".

However, this means that Safari and Windows Hello overwrite any existing credential for a given [RP, user handle] pair when a new one is created. This is arguably allowed, per:

Note that a discoverable credential capable authenticator MAY support both storage strategies. In this case, the authenticator MAY at its discretion use different storage strategies for different credentials, though subject to the residentKey or requireResidentKey options of create().
https://w3c.github.io/webauthn/#sctn-credential-storage-modality

However, this presents a problem for sites that want to implement passwordless/usernameless flows, if they already have existing "server-side" registrations for security keys. Many such sites will have users with "security key" registrations that are backed by platform authenticators through Windows Hello or Safari. In that case, the RP cannot create a new registration without the risk of silently invalidating old registration. This can potentially lock out users — especially security-conscious users who try to ensure they can second-factor auth only using security keys.

The RP can avoid this by including all security key registrations in excludeCredentials when calling create for a new passwordless/usernameless registration. But if the RP wants to detect an existing registration and let the user (re-)register it, here is the simplest "safe" way I can think of:

  • Call create for a new registration with security keys in the exclude list.
  • If there was an existing registration (which is not straightforward to tell; see Can RPs assume that InvalidStateError for create() means an excludeCredentials match? #1566), call get for the user to authenticate so the site can tell which security key.
  • Call create for a new registration with the given security key excluded, then delete the old security key registration and add the new registration.

This is... cumbersome. In addition:

  • If the new registration does not have a superset of the functionality of security keys, the user may need to make an uncomfortable choice and potentially put themselves at higher risk of lockout.
    • Safari and Google's contributions to HowToFIDO have advocated for cookie-based passwordless registrations that are not persistent. The user would not be able to use the platform authenticator after clearing their cookies (and note that that devs need to be especially careful in Safari to avoid cookies being cleared).
  • If the flow fails in the middle, it can be confusing to explain what happened, and the RP might need to make the user jump through even more hoops. I can certainly imagine a situation where an RP misses an edge case and locks out a user (e.g. by deleting the old registration before the new one is full registered).

A "create or get" operation could help, but it would get complicated if we want to handle the case where a new registration with more stringent authenticator selection needs to replace an existing security key registration
an existing security key registration needs to replaced with one that satisfies more stringent authenticator selection criteria than the RP can verify.


Windows Hello and Safari might change their implementations to avoid overwriting security keys. But there will be new implementations in the future, and this is a non-obvious trap for browsers and RPs that can lock out users — it's so non-obvious that we discovered it at GitHub only only after months of working on WebAuthn, while in contact with browser authors. I think it would be best for RPs and users if the spec could help make sure that old security key registrations (i.e. registrations where the RP did not set requireResidentKey: true) are never overwritten by accident.


Solutions

Going forward, one option would be to replace "MAY" with "MUST" in the section I quoted above and add a clarifying sentence or two.

But if Windows Hello/Safari can't tell which existing registrations were created with requireResidentKey: true (I don't know, but they might be able to?), it might need a more clever solution.

@lgarron lgarron changed the title Prevent browsers from deleting server-side credentials for the RP Prevent browsers from deleting "server-side credentials" for the RP Feb 9, 2021
@lgarron
Copy link
Contributor Author

lgarron commented Feb 9, 2021

One not-so-crazy option would for RPs with existing Webuthn user handles that are meant for security keys: generate second* user handle for each user,, to be used for passwordless registrations. (And maybe a third for usernameless registrations, if that's not covered by "passwordless"?). However, this seems like an unnecessary cost to heap on RPs when it could be fixed at the spec or browser level.

@lgarron lgarron changed the title Prevent browsers from deleting "server-side credentials" for the RP Prevent browsers from deleting credentials the RP meant to be server-side Feb 9, 2021
@lgarron lgarron changed the title Prevent browsers from deleting credentials the RP meant to be server-side Prevent browsers from deleting credentials that the RP meant to be server-side Feb 9, 2021
@lgarron lgarron changed the title Prevent browsers from deleting credentials that the RP meant to be server-side Prevent browsers from deleting credentials that the RP wanted to be server-side Feb 9, 2021
@emlun
Copy link
Member

emlun commented Feb 9, 2021

Even if Windows Hello and Safari were to change their implementations in the future, there will continue to exist roaming authenticators that will still overwrite an existing discoverable credential if it already has one with the same (rpId, userHandle). But it looks like this could be solved if we recommend that RPs, for example, generate a new unique user handle for each new credential, instead of using a single user handle per user.

On the other hand, maybe it's better if clients could ask the user to confirm an overwrite - after all, one of the reasons to have it behave that way was that it's probably rarely useful to have more than one credential for the same account on the same authenticator. I'm not completely certain CTAP2 gives the client access to all the information it would need to detect the situation, but maybe? It might require the credential management additions in CTAP2.1 though.

@limpkin
Copy link

limpkin commented Feb 9, 2021

our authenticator actually prevents overwriting credentials and does incorporate a way to manage credentials due to this very same reason...

@akshayku
Copy link
Contributor

akshayku commented Feb 9, 2021

In that case, the RP cannot create a new registration without the risk of silently invalidating old registration.

Don't understand. Why would existing registration will not suffice? And if a new credential is created somehow, then that credential will work.

The RP can avoid this by including all security key registrations in excludeCredentials when calling create for a new passwordless/usernameless registration.

Yes, that would be my recommendation.

But if the RP wants to detect an existing registration and let the user (re-)register it, here is the simplest "safe" way I can think of:

Sorry, don't understand the need for it. Once a credential is created, RP should check for uv bit to figure out whether that credential can be used for passwordless flows. And in .get() call you pass all the credentials which are uv capable to the authenticator if you are doing the with-username flows.

Windows Hello and Safari might change their implementations to avoid overwriting security keys.

Windows only supports resident keys for some releases and will follow the behavior of overwriting the existing credential if exclude list doesn't contain that credential.

But if Windows Hello/Safari can't tell which existing registrations were created with requireResidentKey: true (I don't know, but they might be able to?)

Windows don't store the request details, so we don't know.

@lgarron
Copy link
Contributor Author

lgarron commented Feb 10, 2021

In that case, the RP cannot create a new registration without the risk of silently invalidating old registration.

Don't understand. Why would existing registration will not suffice? And if a new credential is created somehow, then that credential will work.

Sorry, don't understand the need for it. Once a credential is created, RP should check for uv bit to figure out whether that credential can be used for passwordless flows. And in .get() call you pass all the credentials which are uv capable to the authenticator if you are doing the with-username flows.

Unfortunately, we don't know which existing registrations are a discoverable credentials and/or platform authenticators, because we don't get that information by default. More to the point, there is no way to tell from a successful get response whether the authenticator would satisfy isUserVerifyingPlatformAuthenticatorAvailable() in a fresh browser profile, right?

I own at least one user-verifying authenticator that is not a platform authenticator, and Yubico has already announced they will sell one.

Windows don't store the request details, so we don't know.

That's good to know!

@akshayku
Copy link
Contributor

More to the point, there is no way to tell from a successful get response whether the authenticator would satisfy isUserVerifyingPlatformAuthenticatorAvailable() in a fresh browser profile, right?

Yes, In fresh browser profile, you don't know which machine it is. And for privacy reasons, we cannot expose this information over the web.

I own at least one user-verifying authenticator that is not a platform authenticator, and Yubico has already announced they will sell one.

There are many user-verifying authenticators that are not a platform authenticators and Yubico already sells one. May be you are confusing fingerprint based authenticators with user-verifying based authenticators. user verifying authenticators also consists of authenticators which are local PIN based.

I have many user-verifying authenticators types. Some are local PIN based. Some are fingerprint based.

Overall for this issue, Windows has no plans to support non-discoverable credentials. And if RP does not want credentials to be overwritten, they should provide an exclude list with all the credentials.

@lgarron
Copy link
Contributor Author

lgarron commented Feb 10, 2021

There are many user-verifying authenticators that are not a platform authenticators and Yubico already sells one. May be you are confusing fingerprint based authenticators with user-verifying based authenticators. user verifying authenticators also consists of authenticators which are local PIN based.

I believe understand the UV/PA/RK properties well enough (although I admit I'm still learning new things about them constantly).

My point is more that the API does not allow us to distinguish PA/ or RK for existing registrations, especially if we did not save transport data. So if we wanted to enforce UV+PA for new registrations (as encouraged explicitly by the peerless existence of isUserVerifyingPlatformAuthenticatorAvailable()), we wouldn't know which old registrations satisfy it.

@nadalin nadalin assigned nadalin and akshayku and unassigned nadalin Feb 10, 2021
@nadalin nadalin added this to the L3-WD-01 milestone Feb 10, 2021
@equalsJeffH
Copy link
Contributor

on 2021-02-10 call:
@akshayku to followup.

@MasterKale
Copy link
Contributor

After discovering this issue last week I brought it up with a colleague. After some discussion we ended up agreeing that the ability to overwrite credentials (and the current abstraction of how a credential might be persisted internally within an authenticator) is actually a good idea. If there's a reason a user needs to re-enroll an authenticator then it's as simple as a second navigator.credentials.create() call without the existing credential's ID in excludeCredentials to get the authenticator to generate a fresh credential (and internally overwrite the existing discoverable credential that isn't wanted anymore anyway).

But if the RP wants to detect an existing registration and let the user (re-)register it, here is the simplest "safe" way I can think of...

Is there something wrong with allowing a user to log in with an existing-credential-that-should-be-re-registered, then forcing the user to re-register it during login? Here's a flow we imagined for re-registering a credential:

  1. RP requests assertion as usual w/existing Touch ID credential which is flagged for re-registration
  2. User successfully generates assertion response via Touch ID
  3. RP sees that returned credential ID corresponds to a credential flagged for re-registration
  4. RP requests attestation without the existing Touch ID credential ID in excludeCredentials
  5. User successfully generates attestation response via Touch ID
    1. Touch ID overwrites existing discoverable credential
  6. RP saves new Touch ID credential
  7. RP deletes old Touch ID credential that was flagged for re-registration

This seemed to us to be a straightforward re-registration flow that would work with the current release of the spec without the introduction of any new APIs. Is there something "unsafe" about this flow?

@lgarron
Copy link
Contributor Author

lgarron commented Apr 15, 2021

Is there something wrong with allowing a user to log in with an existing-credential-that-should-be-re-registered, then forcing the user to re-register it during login? Here's a flow we imagined for re-registering a credential:

Hmm, I don't completely follow your flow.

Also, it seems you're working off the assumption that we can naturally detect the "credential to re-register" during a normal sign-in flow. But if we could do that, we wouldn't need to re-register it. We could just note what credential was used and continue with Apple's/Google's advice from there.

But our problem is that this will not always happen during the natural flow. In particular, your suggested flow will ask the user to authenticate even if the authentication is guaranteed to fail — and it's not a good experience if the first thing on a new device is an error dialog that the RP can't even control (i.e. if the browser chooses an explanation for their WebAuthn dialog that is not relevant to this use case, the RP can't do anything about it). 😔

There's also the slight technical detail that your suggested flow doesn't guarantee that the new registration replaces the expected old registration on the appropriate client, or that it can be used where the old registration was.

@Kieun
Copy link
Member

Kieun commented Apr 15, 2021

I can't follow the issue. Is this about the platform authenticator with a discoverable feature or the security keys?
At least , RP should have a knowledge about the registered authenticator so that RP will refer that knowledge to decide user login flow.

For re-registration issue, the registration is performed after the user account is identified with an authenticated session so that the RP would have the list of registered credentials for that account. So, you can safely exclude the such credentials by populating those credential Ids in the excludeCredentials if you ask for the user to register an authenticator.

@lgarron
Copy link
Contributor Author

lgarron commented Apr 15, 2021

At least , RP should have a knowledge about the registered authenticator so that RP will refer that knowledge to decide user login flow.

How should this work if a user is logging in from a brand-new browser profile?

At GitHub, we want to make trusted device functionality available to as many people as possible, so it will not be tied directly to 2FA. Most users will still only need a password to log into a new browser profile, even if they have registered a trusted device. When such a user logs in, we don't learn which/if any of their registrations are available in that browser/device.

As a workaround, we're planning to use some UA sniffing heuristics to guide the user experience for registration (in the hope that #1568 will offer a better solution some day). Because of the main issue being discussed on this page, we also have to explain to some users that they must remove a security key registration in order to use their platform authenticator as a trusted device.

For re-registration issue, the registration is performed after the user account is identified with an authenticated session so that the RP would have the list of registered credentials for that account. So, you can safely exclude the such credentials by populating those credential Ids in the excludeCredentials if you ask for the user to register an authenticator.

I still have concerns about this:

  • The RP still has to know which credential is intended to be replaced (which e.g. requires an extra prompt in general).
  • The RP has no guarantee that the new registration is actually a "re-registration". It's likely to be so (especially for platform authenticators, which tend to be unique per device), but I can definitely think of edge cases.

@MasterKale
Copy link
Contributor

@lgarron What is the User Story you're trying to engineer a solution to?

From everything I read so far I'm understanding it's something related to handling user login from a "new computer" (new/different browser profile, etc...), but beyond that there're some generalizations that are making it hard to figure out what your exact problem is. Is it some kind of issue related to 2FA-oriented attestation (UP-only, "none" attestation) internally generating a discoverable credential that is at risk of being replaced when you try to "upgrade" that user to Passwordless or Usernameless via a "re-registration" (a second attestation requiring UV and direct attestation)?

@Kieun
Copy link
Member

Kieun commented Apr 15, 2021

How should this work if a user is logging in from a brand-new browser profile?

If you say about the case where is no client (browser) side information about the credential, why not just identifying the user first and then try to get associated credentials from the server?

At GitHub, we want to make trusted device functionality available to as many people as possible, so it will not be tied directly to 2FA. Most users will still only need a password to log into a new browser profile, even if they have registered a trusted device. When such a user logs in, we don't learn which/if any of their registrations are available in that browser/device.

First of all, when you introduce to allow for users to register the platform authenticator, you should store such information.
Without such information, RP would be hard to make a decision in an authentication flow.
While adopting WebAuthn, supporting different types of authenticators and polices are hard and even it would be disaster.

  • RK and/or NRK
  • Platform and/or cross-platform
  • Usernameless and/or passwordless

My idea about your situation is

  • When deploying trusted device feature, you only allow the trusted device feature for the user who start to register the platform authenticator (with attachment) and store that information in the server side.
  • If there is no ambient credential (maybe fresh browser or cookies are cleared), you just prompt normal authentication process with password. Then, RP can get associated credentials for that user and search credentials for platform authenticator and ask for the user authentication with those credential in the allowList.
  • If the user successfully is authenticated with platform authenticator credentials, you create ambient credential for this browser
  • If you get an error saying there is no such credential in this device (platform), ask for the user to register the platform authenticator.

@lgarron
Copy link
Contributor Author

lgarron commented Apr 15, 2021

@lgarron What is the User Story you're trying to engineer a solution to?

The ideal user story would be something similar to Touch ID (e.g. "use Touch ID to sign into this app on this device") as it works on iOS apps, which does not have the same issues because:

  • An app can reliably tell if the device has an available Touch ID registration it can prompt the user for.
  • A new registration does not break existing registrations — especially not in other apps.

If you replace "Touch ID" with "WebAuthn" and "app" with "browser", then:

  • The first bullet is strongly constrained by privacy goals of the WebAuthn spec authors.
  • The issue I filed here is about addressing the second bullet for some specific situations.

From everything I read so far I'm understanding it's something related to handling user login from a "new computer" (new/different browser profile, etc...), but beyond that there're some generalizations that are making it hard to figure out what your exact problem is. Is it some kind of issue related to 2FA-oriented attestation (UP-only, "none" attestation) internally generating a discoverable credential that is at risk of being replaced when you try to "upgrade" that user to Passwordless or Usernameless via a "re-registration" (a second attestation requiring UV and direct attestation)?

It's a combination of the following:

We are trying to implement trusted device functionality given these constraints.

@lgarron
Copy link
Contributor Author

lgarron commented Apr 15, 2021

  • If there is no ambient credential (maybe fresh browser or cookies are cleared), you just prompt normal authentication process with password. Then, RP can get associated credentials for that user and search credentials for platform authenticator and ask for the user authentication with those credential in the allowList.

I have the following concerns about this:

  • The user is already authenticated. We would have to explain that we are starting with a WebAuthn authentication prompt even though they're already logged in and the real purpose is registering a trusted device. That's pretty confusing for a general audience.
  • Any time the user logs into a new device, the expected flow for registration will start with a prompt that is guaranteed to end in an error — that's not a great user experience and we can't control how the browser explains the error.
  • If the existing registration is a security key, we may need to detect and delete it. This requires careful logic (so the user can't accidentally get stuck in a state with neither the old nor the new registration, or with a non-working registration), and requires extra UI that would otherwise be unnecessary.

Our trusted device implementation could be significantly simpler if we could assume that old security key registrations (which we did not ask the browser to make discoverable) will not be invalidated as a side effect of the new functionality.

@Kieun
Copy link
Member

Kieun commented Apr 15, 2021

Another thought for this is that adding attachment option for .get() call, so that RP would indicate their preference for the authentication.
You can refer the issue here #1267.
And Apple is trying to introduce attachment option for get call. https://bugs.webkit.org/show_bug.cgi?id=223547

@MasterKale
Copy link
Contributor

What is "trusted device functionality"? "Trusted device" is mentioned once in the spec and doesn't mention anything beyond offering the convenience of not needing to dig around for a roaming authenticator:

The primary use case for platform authenticators is to register a particular client device as a "trusted device", so the client device itself acts as a something you have authentication factor for future authentication. This gives the user the convenience benefit of not needing a roaming authenticator for future authentication ceremonies, e.g., the user will not have to dig around in their pocket for their key fob or phone.

@lgarron
Copy link
Contributor Author

lgarron commented Apr 20, 2021

What is "trusted device functionality"? "Trusted device" is mentioned once in the spec and doesn't mention anything beyond offering the convenience of not needing to dig around for a roaming authenticator:

The primary use case for platform authenticators is to register a particular client device as a "trusted device", so the client device itself acts as a something you have authentication factor for future authentication. This gives the user the convenience benefit of not needing a roaming authenticator for future authentication ceremonies, e.g., the user will not have to dig around in their pocket for their key fob or phone.

Correct. We are using the concept of "trusted device" as described in that paragraph of the spec.

@MasterKale
Copy link
Contributor

@lgarron Do you think this issue is still valid?

@lgarron
Copy link
Contributor Author

lgarron commented Sep 12, 2023

@lgarron Do you think this issue is still valid?

Until #1568 is resolved, yeah.

But I'm also not actively working on a WebAuthn RP implementation at the moment.

@plehegar plehegar modified the milestones: L3-WD-01, L3-WD-02 Oct 4, 2023
@arianvp
Copy link

arianvp commented Dec 29, 2023

I have a follow-up question on this that seems related:

I've noticed that under adverse network conditions (very common on mobile phones!) it can happen quite often that the navigator.credentials.create call succeeds but the credential never gets received by the RP and our 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 field. (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

Combined with this behaviour of replacing the existing server-side key this can also completely lock out a user due to a bad network! This sounds extremely fragile.

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.

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 prone to failure.

  1. 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@Risk Items that are at risk for L3 type:technical
Projects
None yet
Development

No branches or pull requests