go-gitea / gitea

Git with a cup of tea! Painless self-hosted all-in-one software development service, including Git hosting, code review, team collaboration, package registry and CI/CD
https://gitea.com
MIT License
45.02k stars 5.49k forks source link

Improving authentication with hardware keys / FIDO2 / webauthn + 2FA #21675

Open noerw opened 2 years ago

noerw commented 2 years ago

The Problem


Possbile Solutions

(1) The Very Simple Fix

Revert #11573, but otherwise leave the auth conditions as they are. Fixes the UI inconsistency and the no-backup code situation, implements github's authentication logic, but does not enable passwordless login and makes phishable credentials mandatory.

(2) The Somewhat Simple Fix

This mostly keeps the current auth logic, except for requiring a backup second factor to be set up:

username && pw && (totp || fidokey1 || fidokey2 || backupcodes)

This auth logic does not require phishable credentials.

(3) The Correct Solution

The most correct way imho would be to implement the auth logic that is outlined below as Nextcloud's implementation. Effectively the needed changes would be:

This would result in the following auth logic:

username && (pw || fidokey1 || fidokey2) && (totp || fidokey3 || backupcodes)

Of course not all of those credentials need to be used, all but one from each block could be unavailable/unknown. This option has the benefit to get along without mandatory phishable credentials (password, TOTP), as well as enabling passwordless auth.


Context

Gitea's current (as of 1.18.0+dev-558-g94d6d93cc, 94d6d93cc) authentication behaviour is as follows: context auth requirements
default username && pw
TOTP 2FA enabled username && pw && (totp \|\| backupkeys)
(multiple) fido2 keys registered username && pw && (fidokey1 \|\| fidokey2)
TOTP 2FA enabled & fido2 keys registered username && pw && (fidokey1 \|\| fidokey2 \|\| totp \|\| backupcodes)

For reference some other implementations of FIDO2:

implementation context auth requirements
github, mastodon default username && pw && emailed-token
TOTP 2FA enabled username && pw && (totp \|\| backupkeys)
(multiple) fido2 keys registered case not allowed, must enable 2FA
TOTP 2FA enabled & fido2 keys registered username && pw && (fidokey1 \|\| fidokey2 \|\| totp \|\| backupcodes)
nextcloud default username && pw
TOTP 2FA enabled username && pw && (totp \|\| backupkeys)
(multiple) fido2 keys registered username && (pw \|\| fidokey1 \|\| fidokey2)
TOTP 2FA enabled & fido2 keys registered username && (pw \|\| fidokey1 \|\| fidokey2) && (totp \|\| backupcodes)

As you can see, there is no clear correct de-facto implementation, I have yet to check fido2/webauthn spec if there is a correct approved way of implementing things. However there is a strong preference in the wider community on how to do things correctly with security and usability in mind:


some SEO optimization: webauthn u2f security key 2fa totp fido fido2
lunny commented 2 years ago

~I would like the first implementation(github, mastodon), nextcloud's looks like a 3FA when TOTP 2FA enabled & fido2 keys registered~

noerw commented 2 years ago

nextcloud's looks like a 3FA when TOTP 2FA enabled & fido2 keys registered

If you look closely, you see that it's still 2FA: username && (pw || fidokey1 || fidokey2) && (totp || backupcodes).

The decision here is how much effort the gitea project is willing to put into changing this auth logic, with the possible gain of having an unphishable authentication method (to be clear, Gitea currently has this property (since #11573), so we should think hard about dropping it). That said, I don't think touching this is urgent, as there is no acute security issue with the current way this auth is handled, the UI is just misleading currently. So it may be beneficial to wait and do this properly once the resources or priority for it are there, and refactor the UI in the meantime.

theAkito commented 1 year ago

Was about to create a dedicated issue, but found this one, when searching for existing ones, so I'll add another improvement. If you need a separate issue out of this, I'll make one.

Gitea's WebAuthn does not save the domain string.

Information about the topic follows.

lunny commented 1 year ago

Gitea's auth has a more complex situation. Some users want to login with a third auth server i.e. OAuth2 . I think we need to put all these situations together when refactoring/redesigning.

f0sh commented 1 year ago

I would like to catch up on the statement of the beginning of this issue:

It can be argued that webauthn is supposed to be passwordless auth, enabling 2FA should be a separate choice, and that mandatory TOTP 2FA is reducing security instead of increasing it due to phishing/MITM risk.

Webauthn was designed as a generic, phishing proof protocol and therefore can be used for passwordless authentication. It brings it's own function of a 2nd factor (where you can use a 2F to access your local public key) but is also downward compatible as a 2F with U2F. That's a bit confusing. Because the domain is (as the relying party RP) fundamental part of the authentication process in webauthn, there is a drastically reduced attack vector for phishing in webauthn compared to U2F, because you just cannot use your credentials on a different site, without tampering your local machine.

When it comes to the implementation, then you actually do not use (or strictly saying, you do not have to use) a username anymore, because you are working with a publickey authentication approach.

So I totally agree with what @lunny said, that

I think we need to put all these situations together when refactoring/redesigning.

because there would be actually a pool of equal authentication methods/backends, every with their own set of credentials:

This is indeed a lot of effort to implement. However, considering the move of bigtechs towards Passkeys and being already available on most platforms, this is worth the discussion and integration. Github just announced few days ago, that they now support passkey authentification as well. Paypal, Google, Apple are alreading doing it.

My point is, when tackling the described issues, I would suggest to refactor it in a way that the whole picture is considered as it is discussed in #24821. I think this will give more momentum. A first approach can be hanko.io.