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

RPs cannot show "You've Already Registered This Authenticator" Message #806

Closed
leshi opened this issue Feb 16, 2018 · 23 comments · Fixed by #809
Closed

RPs cannot show "You've Already Registered This Authenticator" Message #806

leshi opened this issue Feb 16, 2018 · 23 comments · Fixed by #809

Comments

@leshi
Copy link
Contributor

leshi commented Feb 16, 2018

Many RPs prevent users from registering the same Authenticator twice. RPs use specifying the already known credentials in the excludeCredentials during the create() call, which then gets passed down into authenticatorMakeCredential in the excludeCredentialDescriptorList. Authenticators "should not create a new credential" if they recognize a credential that they have previously minted.

In reading 6.2.2, step 3.1, when the authenticator discovers that it owns one of the excluded credentials, the authenticator replies with a NotAllowedError:

If looking up descriptor.id in this authenticator returns non-null, and the returned item's RP ID and type match rpEntity.id and excludeCredentialDescriptorList.type respectively, then return an error code equivalent to "NotAllowedError" and terminate the operation.

This error is then propagated to the RP by the client. In looking at step 5.1.3 step 21, we see:

Return a DOMException whose name is "NotAllowedError". In order to prevent information leak that could identify the user without consent, this step MUST NOT be executed before lifetimeTimer has expired. See §14.2 Registration and Authentication Ceremonies Privacy for details.

As written, the spec has two problems:

  1. The generic "NotAllowedError" is shared between lots of different error situations. This does not allow the RP to show a meaningful message to the user. Note that Google and Github (and probably many other RPs that use U2F) already give users meaningful messagers in these cases:
    github
    google

  2. The lifetimeTimer will prevent this error from being responsive. Users will touch their Security Key and then stare into blank space until the timer expires.

@leshi
Copy link
Contributor Author

leshi commented Feb 16, 2018

It looks like CTAP2 is already having authenticators return CTAP2_ERR_CREDENTIAL_EXCLUDED. I think we (a) should wrap this up in a new error message that webauthn can return to callers and (b) exclude this new error from the lifetimeTimer requirement as it's in response to explicit user action.

@emlun
Copy link
Member

emlun commented Feb 16, 2018

This limitation is by design, as discussed extensively in #184, #204, #764, #687 (comment) , #629 (comment) . In summary: providing this information would allow malicious RPs to reliably identify (i.e., track) the user without consent.

@kpaulh
Copy link
Contributor

kpaulh commented Feb 16, 2018

What about in the case where the user does give consent? I.e. they touch a device they want to register without realizing it has already been registered. They fully intended to give consent to its registration.

@leshi
Copy link
Contributor Author

leshi commented Feb 16, 2018

providing this information would allow malicious RPs to reliably identify (i.e., track) the user without consent.

No. The CTAP2 spec explicitly says that user consent must be gathered before an authenticator replies with CTAP2_ERR_CREDENTIAL_EXCLUDED

@emlun
Copy link
Member

emlun commented Feb 16, 2018

That is a good point... Unfortunately CTAP2 specifies that consent is collected after the CTAP2_ERR_CREDENTIAL_EXCLUDED error would be returned, so the two cases are not distinguishable to the client.

...Unless the client can ask the user up front if they intend to create a credential (i.e., something like Chrome's and Firefox's "share location?" popup: "foo.com would like to register your identity, proceed?")? In that case I suppose the client could return the EXCLUDED error early without any unintended information leak. Updating the spec to accommodate that shouldn't be too hard.

I don't immediately see any other ways to solve this without sacrificing the protection against the information leak.

@agl
Copy link
Contributor

agl commented Feb 16, 2018

I think the following would work?

  1. Have the browser send the exclude list down to the authenticators. For any which say CTAP2_ERR_CREDENTIAL_EXCLUDED, note them and reissue the command without an exclude list.
  2. If the user ends up tapping a different authenticator, great. No problem.
  3. If the user selects an excluded authenticator either a) tell the site via an error status or b) prompt the user "You have already registered this authenticator on this site. Really register again?"

Option a) lets the site probe for suspected identities, although it only gets a single bit per user action. Option b) solves this problem, but gives the user the power to register an authenticator twice if they wish, which might be crappy if sites are requiring multiple authenticators for a good reason.

@emlun
Copy link
Member

emlun commented Feb 16, 2018

A drawback of that strategy is that the authenticator will actually create the credential before the client knows to ask "are you sure?". If that was a resident credential, then some of the authenticator's (probably limited) internal storage has now been allocated to it and it may not be obvious to the user that they need to delete it (CTAP2 has no "delete single credential" command, so the client can't do this automatically either).

Aside from that, I think it sounds like a good idea. I wouldn't worry about the information leak in option (a) since, as @kpaulh notes, the user evidently does intend to share the information in this case.

@leshi
Copy link
Contributor Author

leshi commented Feb 16, 2018

CTAP2 authenticators must gather consent before replying with CTAP2_ERR_CREDENTIAL_EXCLUDED:

When an authenticatorMakeCredential request is received, the authenticator performs the following procedure:

If the excludeList parameter is present and contains a credential ID that is present on this authenticator and bound to the specified rpId, wait for user presence, then terminate this procedure and return error code CTAP2_ERR_CREDENTIAL_EXCLUDED. User presence check is required for CTAP2 authenticators before the RP gets told that the token is already registered to behave similarly to CTAP1/U2F authenticators.

The procedure @agl suggests is what we do for U2F (CTAP1) authenticators today -- they do not have resident keys.

@kpaulh
Copy link
Contributor

kpaulh commented Feb 16, 2018

Well, almost. The way this has been handled for U2F is that instead of issuing another create command to get user consent, a sign request with fake data is issued. This doesn't have the side effect of creating a credential.

@kpaulh
Copy link
Contributor

kpaulh commented Feb 16, 2018

I apologize, I was incorrect here. We do send another create, but with fake data. So, yes, that does have the side effect of creating a credential.

@leshi
Copy link
Contributor Author

leshi commented Feb 16, 2018

But that side effect doesn't matter as all U2F devices I know don't actually store anything.

@cpiper
Copy link

cpiper commented Feb 16, 2018

Would sending a getAssertion request suffice? So, first a query to determine that the credential was previously registered, and if so, just send a getAssertion request. If there are multiple authenticators, and a user affirmatively touches a previously registered one, a getAssertion response would be received. In that case, it can be definitively be determined from the response that this was a previously registered authenticator. If a makeCredential response is received, then the authenticator is new to the RP.

@agl
Copy link
Contributor

agl commented Feb 16, 2018

I think some of the confusion here might be from looking at the public draft of CTAP2. In the 2017-09-27 version it says:

If the excludeList parameter is present and contains a credential ID that is present on this authenticator, terminate this procedure and return error code CTAP2_ERR_CREDENTIAL_EXCLUDED.

I.e. don't wait for user presence. Sounds like that's been fixed since then and browsers could return the error directly.

@emlun
Copy link
Member

emlun commented Feb 16, 2018

@leshi Oh, my bad - I guess I was reading an outdated draft?

  1. If the excludeList parameter is present and contains a credential ID that is present on this authenticator, terminate this procedure and return error code CTAP2_ERR_CREDENTIAL_EXCLUDED.

@kpaulh Ah, a fake sign request is a clever way to work around it! That seems to me like a good solution: reply with some kind of AlreadyRegisteredError if the user does confirm the sign action, and the ambiguous NotAllowedError if the user does not confirm it.

I suppose that could be confusing if the authenticator has a rich interface and makes create and sign operations substantially different. I imagine that could perhaps become an issue with U2F on "Intel Online Connect", for example, but it seems minor compared to the other issues discussed here.

@emlun
Copy link
Member

emlun commented Feb 16, 2018

Oh, right, if the EXCLUDED response from the authenticator is now guaranteed to have collected consent first, then the information leak I was talking about shouldn't happen and I see no problem with returning a unique error before the timeout ends.

@leshi
Copy link
Contributor Author

leshi commented Feb 16, 2018

Great! What error should we return? :)

@emlun
Copy link
Member

emlun commented Feb 16, 2018

I don't know how these error types are typically selected, so I'll defer that question to someone who does (@equalsJeffH, @selfissued, @jcjones, @akshayku?) - but I'm guessing maybe InUseAttributeError, InvalidStateError or OperationError, maybe? You're all welcome to leave suggestions as well. I can draft a PR for this (sans a specific error type) on Monday.

@akshayku
Copy link
Contributor

AlreadyExistError ?

@leshi
Copy link
Contributor Author

leshi commented Feb 16, 2018

I think we need to choose from here: https://heycam.github.io/webidl/#idl-DOMException-error-names

@leshi
Copy link
Contributor Author

leshi commented Feb 16, 2018

Looking at https://heycam.github.io/webidl/#idl-DOMException, it appears that we might be able to use the message field? h/t @kpaulh

@arnar
Copy link
Contributor

arnar commented Feb 17, 2018

"ConstraintError" seems fitting, and matches other errors due to the client asking for sth that can't be done (e.g. 'requireResidentKey'). Or simply "NotAllwedError".

@idamlaj
Copy link

idamlaj commented Feb 17, 2018

I would like us to use a different error than "ConstraintError" and "NotAllowedError", so the relying party can display the appropriate error message for the user.

@emlun
Copy link
Member

emlun commented Feb 17, 2018

https://heycam.github.io/webidl/#idl-DOMException-error-names lists further descriptions of the error types. ConstraintError looks like it's specifically meant for STM errors, so probably not a good fit for this.

Edit: On the other hand, ConstraintError is already used in WebAuthn for other things, so perhaps we don't need to care too much about that particular aspect. I think we need a different one to separate it from unsatisfied UV/UP requirements, though.

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.

9 participants