w3c / webauthn

Web Authentication: An API for accessing Public Key Credentials
https://w3c.github.io/webauthn/
Other
1.17k stars 169 forks source link

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

Open lgarron opened 3 years ago

lgarron commented 3 years ago

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:

This is... cumbersome. In addition:

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 commented 3 years ago

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.

emlun commented 3 years ago

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 commented 3 years ago

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

akshayku commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.

equalsJeffH commented 3 years ago

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

MasterKale commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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:

MasterKale commented 3 years ago

@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 commented 3 years ago

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.

My idea about your situation is

lgarron commented 3 years ago

@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:

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

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 commented 3 years ago
  • 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:

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 1 year ago

@lgarron Do you think this issue is still valid?

lgarron commented 1 year ago

@lgarron Do you think this issue is still valid?

Until https://github.com/w3c/webauthn/issues/1568 is resolved, yeah.

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

arianvp commented 9 months ago

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