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

Get Credential List from the server during authentication. #28

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

akshayku
Copy link

@akshayku akshayku commented Jan 13, 2021

This came when an RP was adopting this document and users are getting to cross-browser situations where platform authentication is failing.

Issue is that resident credentials don't work the same across platforms as of now. As we are recommending the websites on how to develop FIDO solution, they are getting into issues in cross-browser scenarios.

With "requireResidentKey=false" during create, on Windows (I think also in Apple ecosystem), platform creates a resident/discoverable credential which is overwritten if RP asks for same user from different browser and without exclude list. On Android, platform creates a non-discoverable credential which is not overwritten.

Here is the flow:

  1. In Edge, login to RP with existing mechanisms. I use password + security key as an example.
  2. RP checks for IsUserVerifyingPlatformAuthenticatorAvailable API which returned true. RP then offered to create a credential with requireResidentKey=false and UserVerificationRequirement=required without any excludelist for User1. Windows created a resident credential and returned Cred1. RP stores that Cred1 into Edge cookie store.
  3. User logs out and tries to login again. RP sees the cookies and offers user to login using platform authenticator. User clicks on the link which start webauthn .get() call with only Cred1 in allowlist. Platform finds Cred1 and user logs in. User then logs out.
  4. User switches browser and uses chrome and tries to login to RP again.
  5. Although user already has registered this device, browser cookies don’t have any information about it and user is not offered to login with Windows Hello.
  6. User logs in with password + external security key.
  7. User offered again to re-register Windows Hello which they have already done earlier. RP offered again to create another credential for user1 without any excludelist. Windows again created a resident credential and has overwritten Cred1 with Cred2 for same user. RP stores Cred2 into Chrome cookie store.
  8. User switches browser again and uses edge again after sometime. RP cookie had Cred1 value and user is offered to login with Windows Hello with Cred1 in allowlist. Authentication failed as Cred1 does not exist anymore.

RP could have used exclude list while creating the platform credential. However, it results in an error condition in multi-browser scenario and I can understand the recommendation of no-exclude list for platform authenticator registration.

This document however recommends that RP stores the credentialID be stored locally. I don't see a reason why. Effect will be the same if RP gets this info from the server at the time of authentication. The issue of whether RP should even show the option of platform authenticator authentication can be done with a Boolean flag stored locally.

Proposal is to store a flag locally to denote that platform credentials exist on this device and always fetch credentials from the server from the server.

@equalsJeffH
Copy link
Contributor

question (thinking out loud here): If the RP first calls nav.creds.get() with an empty allowList, will that automatically utilize any existing Win Hello discoverable cred for that RP such that the RP would know a cred already exists (and not offer to create a new one), and if so, ought that work on either Edge or Chrome?

I.e., even though the RP did not explicitly request creation of a discoverable cred in the first case, the platform created one. Given such (fairly common) platform behavior, perhaps RPs ought to be guided to detect such in any case?

@sbweeden
Copy link
Contributor

question (thinking out loud here): If the RP first calls nav.creds.get() with an empty allowList, will that automatically utilize any existing Win Hello discoverable cred for that RP such that the RP would know a cred already exists (and not offer to create a new one), and if so, ought that work on either Edge or Chrome?

I.e., even though the RP did not explicitly request creation of a discoverable cred in the first case, the platform created one. Given such (fairly common) platform behavior, perhaps RPs ought to be guided to detect such in any case?

I think the issue with that idea is that it cannot be done silently, and therefore the user experience is impacted.

@akshayku
Copy link
Author

If the RP first calls nav.creds.get() with an empty allowList, will that automatically utilize any existing Win Hello discoverable cred for that RP such that the RP would know a cred already exists (and not offer to create a new one), and if so, ought that work on either Edge or Chrome?

If an RP calls .get() call with an empty allowlist, Windows Hello will automatically use previously created credentials. However, that is not a silent call as Shane noted above.

@equalsJeffH
Copy link
Contributor

that is not a silent call as Shane noted above.

yeah, sorry, I've been mostly thinking/working down at the platform & CTAP layers lately. :-/

@ve7jtb
Copy link

ve7jtb commented Jan 13, 2021

@akshay I don't really follow how storing the credentialID on the server really helps.
The RP can't really tell that the second browser is on the same computer as the first browser.

I suspect the real solution is resolving problems with exclude lists.

Perhaps at the WebAuthn level we need a single command that is:
1 If no credentialID is found in the exclude list then make credential and return
2 do getAssertion with exclude list as allow list.

That would work with existing authenticators.

@akshayku
Copy link
Author

I don't really follow how storing the credentialID on the server really helps.
The RP can't really tell that the second browser is on the same computer as the first browser.

On a new browser, first time login is not with FIDO credential. That is using traditional methods like username+password+whatever. I am not changing that. I am only talking about reauthentication as defined in this document in step 8. Credential ID are already stored at the server for this user. All I am asking is to fetch it because local data is not valid anymore.

Perhaps at the WebAuthn level we need a single command that is:
1 If no credentialID is found in the exclude list then make credential and return
2 do getAssertion with exclude list as allow list

I am solving the issue with current APIs, current Platform behavior and this document's theme around reauthentication vs bootstraping. Did not understood your proposal fully, is that for registration where platform just don't fail when encountered with older credential and instead return the signature. That probably can work.

@sbweeden
Copy link
Contributor

I am generally in favour of the approach mentioned by @akshayku to recommend to retrieve platform credential id's from the server. On first use of a browser, even if a platform credential is already registered, the user will re-register and overwrite, just as @akshayku has described. This is currently what happens on both Apple/Safari, and Windows implementations.

My howtofido.securitypoc.com implementation has browser detection code in it at the moment to do precisely this if the platform is detected to be windows, or the Safari browser is detected.

The only advantage that I can recall of having the credentialId local to the client is that I can do a silent pre-validation at the server to ensure the credentialId is still valid and associated with the user account before actually invoking navigator.credentials.get. This is a minor optimisation for a rare edge case where the user has disabled/deleted that credential in their server-side user profile.

@akshayku
Copy link
Author

The only advantage that I can recall of having the credentialId local to the client is that I can do a silent pre-validation at the server to ensure the credentialId is still valid and associated with the user account before actually invoking navigator.credentials.get. This is a minor optimisation for a rare edge case where the user has disabled/deleted that credential in their server-side user profile.

If you always get credentialID from the server, you don't need to do the validation in the first place?

@akshayku
Copy link
Author

My howtofido.securitypoc.com implementation has browser detection code in it at the moment to do precisely this if the platform is detected to be windows, or the Safari browser is detected.

Do you see any issue if you do this everywhere?

@sbweeden
Copy link
Contributor

My howtofido.securitypoc.com implementation has browser detection code in it at the moment to do precisely this if the platform is detected to be windows, or the Safari browser is detected.

Do you see any issue if you do this everywhere?

I am going to try turning this on by default on the howtofido.securitypoc.com site today and forcing everyone to that model, even if you already have a registered credential with credentialId in local storage to see what the impact is. Stay tuned for another reply after I've implemented it, and I'll ask everyone here (and perhaps via the CDWG mailing list) to put it through the paces.

I believe the main reason I did not do it everywhere before now was to try and stay true to the HowToFIDO document as much as possible, and because of the fact that until recently I did not have a secure ambient credential that I considered strong enough to retrieve an allowCredentials list for a userid (i.e. I did not want to leak credentialIds just knowing the username). That has changed in my implementation, and I now do have this type of encrypted long-lived ambient credential that can be used for that type of low-assurance request. So give me today to put together a change for this, then I'll come back to y'all for further testing and experimentation.

@equalsJeffH
Copy link
Contributor

That has changed in my implementation, and I now do have this type of encrypted long-lived ambient credential that can be used for that type of low-assurance request

am curious re what this is and how it works?

@sbweeden
Copy link
Contributor

sbweeden commented Jan 14, 2021

That has changed in my implementation, and I now do have this type of encrypted long-lived ambient credential that can be used for that type of low-assurance request

am curious re what this is and how it works?

It's essentially an encrypted copy of username+essential attributes, and came out as a new feature in our IAM offering end of 2020 called Remember Session. In the past you could do this with custom code extensions but now it's built-in.

I wrote about the feature in our product in a blog article, but what I didn't mention there is that the reference use case that led to the feature and the article was actually this particular HowToFIDO scenario.

@sbweeden
Copy link
Contributor

Ok - I have change the https://howtofido.securitypoc.com site to no longer have a dependency or require (for any platform) the credentialId to be stored client side. There is only one small edge case that I am aware of that behaves differently - it's the case where the user has deleted a platform credentialId from the "Security Settings" page on the server. Previously if the credentialId was stored client side (e.g. Chrome on Mac, or Android), a pre-validation of the client-side-known credentialId would have been performed before prompting for WebAuthn, and the user would have been taken immediately to a login experience which did not try to use the platform authenticator. Now that won't happen, and the user would see an error experience when UVPA authentication is initiated. This is an edge case though, and the user can always fallback to "Try another way", so it's not a dead end.

PLEASE test the site, and let me know if you encounter any issues (or even if you find it to be successful). This will help guide my feedback on this PR, and the recommendations of the document in general.

@sbweeden
Copy link
Contributor

I want to re-visit the importance of having (for the reauthentication use case) a secure encrypted long-lived credential that is part of localStorage or an equivalent persistent cookie that can be used to fetch the allowCredentials list. This is important to prevent information leakage about accounts and credentials. My first iteration of the site did not have such a thing, which is why I would have been previously uncomfortable implementing this approach. Now it does, and this seems like a reasonable thing to do.

If we are going to make this a permanent recommendation for the document (which I am generally in favour of), then the document should also reflect the need to have a secure long-lived session or other credential that can be used for low-assurance requests such as fetching the allowCredentials list.

@akshayku I think that before I would be comfortable approving the PR, something to describe this requirement (and why it's recommended) should be included.

@akshayku
Copy link
Author

@sbweeden:

There is only one small edge case that I am aware of that behaves differently - it's the case where the user has deleted a platform credentialId from the "Security Settings" page on the server. Previously if the credentialId was stored client side (e.g. Chrome on Mac, or Android), a pre-validation of the client-side-known credentialId would have been performed before prompting for WebAuthn, and the user would have been taken immediately to a login experience which did not try to use the platform authenticator. Now that won't happen, and the user would see an error experience when UVPA authentication is initiated. This is an edge case though, and the user can always fallback to "Try another way", so it's not a dead end.

True. If user has explicitly deleted a credential from the server which was the correct one (in case of multiple browsers), allowlist may not contain credential which is actually present on the platform. This should be too rare. One could argue that since user explicitly deleted the credential, user don't expect platform authenticator to work anymore. I think we are OK here.

I want to re-visit the importance of having (for the reauthentication use case) a secure encrypted long-lived credential that is part of localStorage or an equivalent persistent cookie that can be used to fetch the allowCredentials list. This is important to prevent information leakage about accounts and credentials.
If we are going to make this a permanent recommendation for the document (which I am generally in favour of), then the document should also reflect the need to have a secure long-lived session or other credential that can be used for low-assurance requests such as fetching the allowCredentials list.

Seems Ok to me. I can add something along those lines to this PR and you can review. But that should be applicable on general case also where user is entering a username and then RP is fetching the allowed credential list. Shouldn't such a recommendation be in Webauthn spec?

@akshayku
Copy link
Author

akshayku commented Jan 14, 2021

@ve7jtb :

I suspect the real solution is resolving problems with exclude lists.
Perhaps at the WebAuthn level we need a single command that is:
1 If no credentialID is found in the exclude list then make credential and return
2 do getAssertion with exclude list as allow list.

Windows when finds that existing credential exists in the allowlist, creates a dummy credential (different dummy RP) and goes through the motion of authenticating the user. API returns an error and RP gets InvalidStateError. But from the user's perspective, they have authenticated. And they don't have to cancel the operation.

When InvalidStateError happens, RP can choose to store local variable (2FACapableFIDOCredsAvailable as suggested in this PR) which says that FIDO credentials exists on the platform. So having an exclude list during bootstraping should not be an issue on Windows as user experience is good. Just tested on Android and there also user authenticates and specific InvalidStateError happens. Not sure about iOS experience.

So I don't see why we cannot include "excludelist" into this document as a recommendation so that we don't end up in overwrite situation in the first place. I have opened #29 w.r.t this.

@sbweeden , Can you also test above theory? While bootstraping, get the excludelist from the server and call .create(). If it succeeds, then store corresponding credentialID on the server and store 2FACapableFIDOCredsAvailable in local cache. If it fails with InvalidStateError, then don't error out and store 2FACapableFIDOCredsAvailable variable in local cache and offer FIDO authentication similar to if .create() had succeeded. This will prevent having multiple entries on the server, where actually there is only one credential on the platform.

@akshayku
Copy link
Author

@sbweeden , I have included session key recommendation in the text. Can you review and give feedback?

Also, anyone else has any feedback?

@akshayku
Copy link
Author

@sbweeden, Can you review?

@sbweeden
Copy link
Contributor

sbweeden commented Jan 24, 2021 via email

device** the user just authenticated from. For example, store the
credential id in a cookie (or associate it with a cookie), or store the
credential id in local storage.
account on the server. Also, make sure you associate this credential id as `2FACapableFidoCredential` and store all the transports applicable for this credential on the server. Associate a `2FACapableFIDOCredsAvailable` flag **with the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you suggesting the tag/terminology 2FACapableFidoCredential in the UVPA case? If there is an intent to introduce this type of terminology then I think it needs a very clear definition in the Let's Define Some Terms section. For example::

  • 2FACapableFIDOCredsAvailable - a flag associated with an account at the browser (cookie or local storage) signifying that the user has (or had) at least one tagged 2FACapableFidoCredential credential registered at the RP that may be used from this device.
  • 2FACapableFidoCredential - a flag associated with a credential registered for the user at the RP signifying that the credential is capable of being used in an assertion flow with userVerification="required".

Not sure I've got that correct - but you get the idea - clarity in the meaning and intent of the term is important. If indeed the descriptions I've taken a stab at above are accurate, then I would consider changing 2FACapable to UVCapable instead. Perhaps go even further and use terminology UVPACredsAvailable and UVPACredential respectively. I don't see any updates to other sections of the document outside the UVPA use case (platform credentials) where this terminology comes into play.

Either way, be consistent with Fido vs FIDO in those labels. Right now they are not consistent. Based on other elements in the document, suggest all capitals FIDO.

I agree it's a good idea to store all the transports, however these are not guaranteed to be available from all user agents (yet).

account on the server. Also, make sure you associate this credential id as `2FACapableFidoCredential` and store all the transports applicable for this credential on the server. Associate a `2FACapableFIDOCredsAvailable` flag **with the
user account** locally. For example, store the
`2FACapableFIDOCredsAvailable` flag in a cookie (or associate it with a cookie), or store the
`2FACapableFIDOCredsAvailable` flag in local storage. **Don't store the actual credential id locally**. Use `2FACapableFIDOCredsAvailable` flag instead of actual credential id which helps in cross-browser scenarios.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just delete the last two sentences here. Given there is no instruction to store the credential id locally at the user agent, there is also no need to explicitly say not to. I appreciate that you are changing the document from it's original guidance but when read standalone with the changes then it is not needed.


If *no credential id is available*, serve a traditional login challenge
a sensitive action, check whether you have a `2FACapableFIDOCredsAvailable` flag for this user
*for the purpose of reauthentication*.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the conditional requirement around the words: for the purpose of reauthentication? Either the 2FACapableFIDOCredsAvailable flag is sufficient to pivot to what happens next, or it's not. I think these words should be deleted.

then you can use FIDO-based reauthentication.
Fetch all `2FACapableFidoCredential` credentials from the server for this user.
Don't fetch non `2FACapableFidoCredential` credentials like U2F credentials.
Prefer using long lived local session key while fetching `2FACapableFidoCredential` credentials from the server.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be stronger that Prefer. I don't think it should be permitted to fetch credentials from the server for a user without some long-lived session key as otherwise there is unauthenticated credential leakage.

Fetch all `2FACapableFidoCredential` credentials from the server for this user.
Don't fetch non `2FACapableFidoCredential` credentials like U2F credentials.
Prefer using long lived local session key while fetching `2FACapableFidoCredential` credentials from the server.
Also order the credentials in the reverse order of when credentials are created (latest created credential is first in the list):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it would be better to order them based on "most recently used" rather than "most recently created"? Either way it would be useful to explain why this is being suggested.

Comment on lines +357 to +358
Prefer using long lived local session key while fetching `2FACapableFidoCredential` credentials from the server.
Also order the credentials in the reverse order of when credentials are created (latest created credential is first in the list):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments as above: I think this needs to be stronger that Prefer, and consider the ordering algorithm.

},
}, {
type: "public-key",
alg: -257
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are other places besides here (and the initial UVPA scenario) where pubKeyCredParams is incorrectly represented as an object and you've corrected to an array (with additional alg). May I suggest that if you're going to fix it in two places, how about fixing it in all 5 places :)

@lgarron
Copy link

lgarron commented Jan 29, 2021

I suspect the real solution is resolving problems with exclude lists.

Perhaps at the WebAuthn level we need a single command that is:
1 If no credentialID is found in the exclude list then make credential and return
2 do getAssertion with exclude list as allow list.

That would work with existing authenticators.

Indeed, I was hoping for that kind of solution when I filed w3c/webauthn#1533 . But it seems this is might be too much to ask of the spec, and could still be confusing for users.

I think that the non-modal UI idea (allow the RP to ask for the browser to auth the user only if they have a credential, but don't respond the RP if there is no credential) solves this far better than any cookie-based workarounds: w3c/webauthn#1545

(By far the simplest would be to give the RP a way to query if e.g. a platform authenticator is registered already. But it sounds like the spec purposely stays far away from this possibility.)

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

Successfully merging this pull request may close these issues.

5 participants