web-auth / webauthn-framework

FIDO-U2F / FIDO2 / Webauthn Framework
MIT License
401 stars 52 forks source link

Login with inexistent username leads to login with user associated to key-pair #215

Closed BenjaminHae closed 1 year ago

BenjaminHae commented 3 years ago

I've registered one user test on the demo server. When trying to login with the user test2, the response to /api/login/options sends back an empty array for the key allowCredentials. However, the browser (chrome) still starts the authentication. The response to the challenge is then calculated using the key-pair for test. As a result I'm logged in as test, although no request to the server occurred that contains this username.

I there should be a validation as to whether the response fits to the user specified - not only, whether there is a public-key in the server database. Additionally as a result, I believe the allowCredential key should be omitted in the options, as it's content might allow an unauthenticated user to find out which users exist on the server.

Spomky commented 3 years ago

Hi @BenjaminHae, Thank you for your patience.

I've registered one user test on the demo server. When trying to login with the user test2, the response to /api/login/options sends back an empty array for the key allowCredentials. However, the browser (chrome) still starts the authentication. The response to the challenge is then calculated using the key-pair for test. As a result I'm logged in as test, although no request to the server occurred that contains this username.

That's correct. When the username does not exist, the application generates a challenge and an empty list of credential. If you get authenticated with another username, this is because the browser has found credentials (resident key) and you are fully identified by the authenticator (PIN, fingerprint, facial recognation...). With simple touch-like authenticator, this behaviour is not possible. See this page for details.

I there should be a validation as to whether the response fits to the user specified - not only, whether there is a public-key in the server database.

On server side, the username is only used to retreive a list of credentials:

Additionally as a result, I believe the allowCredential key should be omitted in the options, as it's content might allow an unauthenticated user to find out which users exist on the server.

Yes and No. Yes because I am conviced that authentication without username is the most efficient and secured way to authenticate users. Nowaday, most authenticators supports at least PINs. With Windows Hello, Android devices and recently all Apple devices, you can easily use fingerprints or faces to enforce the authentication. No because if this list is empty, all other authenticators (old U2F devices, basic Yubico tokens and so on) won't be able to be used.

It is right to say that a non-empty list allows malicious application to establish a list of usernames. This is why it is not recommended to use e-mails as username (see https://webauthn-doc.spomky-labs.com/pre-requisites/user-entity-repository and https://www.w3.org/TR/webauthn-2/#sctn-username-enumeration).

With that being said, it seems like it is a good idea to add an option (firewall maybe) requiring an empty credential list. Another way I tried to implement was the use of fake users so that the list is always populated. I abandonned this way as it is not so easy to implement (requires cache to random credential IDs to be detected)

Spomky commented 1 year ago

Almost 2 years later, I must admit there is no way to address it. The fact is that for preventing user data leak, it is better to continue the authentication flow. This avoid username enumeration (https://www.w3.org/TR/webauthn-3/#sctn-username-enumeration). When a user already has an account, it will be prompted to sign-in. If username-less capabilities are enabled, the user verification is required. With this requirement, the user shall interact with the authenticator and input a PIN, fingerprint, or any other mean. To be noted that the allowedCredentials is now missing when empty.

github-actions[bot] commented 1 year ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.