desec-stack icon indicating copy to clipboard operation
desec-stack copied to clipboard

add 2FA support

Open peterthomassen opened this issue 4 years ago • 19 comments

peterthomassen avatar Mar 23 '20 16:03 peterthomassen

coming from: https://talk.desec.io/t/plans-to-support-2-factor-authentication/33

Great to see it is already a ticket.

If you haven't decided on the specifics yet, here is some food for thought when you design your 2FA:

  • protocol: WebAuthn (instead of commonly seen TOTP which is vulnerable to phishing)
  • allow for multiple FIDO tokens to be registered for the same account (so people don't loose access to their account if one hw token breaks/gets lost)
  • would be nice to have the option to make accounts 2FA only (opt-in)
    • such accounts would required 2FA tokens
    • such accounts would no longer allow using the current password reset via email
    • require a temporary time-limited access token that has been requested using a FIDO token based authentication request

appliedprivacy avatar Apr 18 '20 12:04 appliedprivacy

We should require an email confirmation link for enabling 2FA so that someone guessing a login password cannot persist their access by enabling 2FA. (see https://krebsonsecurity.com/2020/06/turn-on-mfa-before-crooks-do-it-for-you/)

peterthomassen avatar Jul 03 '20 15:07 peterthomassen

TOTP, WebAuthn and others are supported by https://pypi.org/project/django-mfa2/. It seems to be rather simple to integrate, but we should do at least a cursory review of their code.

peterthomassen avatar Aug 03 '20 18:08 peterthomassen

See also: https://medium.com/@ksarthak4ever/django-two-factor-authentication-2ece42748610 https://django-otp-official.readthedocs.io/en/stable/overview.html https://django-two-factor-auth.readthedocs.io/en/1.12.1/class-reference.html

peterthomassen avatar Oct 12 '20 13:10 peterthomassen

  • require a temporary time-limited access token that has been requested using a FIDO token based authentication request

this may be an issue for DynDNS tho, maybe having scoped tokens for that would be cool so the dyndns can be managed without the time limitation but not the rest

  • protocol: WebAuthn (instead of commonly seen TOTP which is vulnerable to phishing)

which style would use use tho?

  • second factor only (User/Password+U2F)
  • passwordless FIDO2 (username + FIDO2 device + UV)
  • Usernameless FIDO2 (no username but resident keys instead, the annoying part is that most devices are on CTAP2.0 and therefore no way to remove individual keys which is not overly nice)

I personally would prefer the passwordless method especially as you could go not even storing a password. also depending on how crazy you go you could allow Password+U2F as an alternative for those not having a FIDO2 device, similar to nextcloud was trying to do.

notably tho is that if you go password- or usernameless that you actually check the UV flag SERVER-SIDE

My1 avatar Aug 02 '21 11:08 My1

  • second factor only (User/Password+U2F)

Sometimes I may have multiple accounts but want to share U2F tokens between them. All of the methods @My1 listed above sound like they could achieve that, but this first one is the only way I've seen it done in practice.

1MachineElf avatar Oct 04 '21 01:10 1MachineElf

just wanna say usernameless has no issues with multiple accounts, you can see it for example on Microsoft accounts. If you have multiple accounts you just get a list after entering your PINs. but I'd personally prefer passwordless fido2 and second factor only is good for many (especially people who still use old U2F keys). While I like the Passwordless option it hasnt been used widely so far (outside special solutions likely no one knows anyway)

My1 avatar Oct 04 '21 16:10 My1

https://github.com/xi/django-mfa3

peterthomassen avatar Jan 27 '22 12:01 peterthomassen

The pain with passwordless WebAuthn is that it invokes the complexity of the whole attestation certificate checking PKI/machinery. I'm aware that it is under debate whether attestation of the authenticator's provenance should be a requirement or not. We are postponing all of that discussion to a later time.

For now, we'll start with TOTP (initially) and "regular" WebAuthn (hopefully soon after).

peterthomassen avatar Aug 20 '22 00:08 peterthomassen

We have deployed TOTP support just now. You can find it in the GUI at the top right under "More". Let us know how you like it!

peterthomassen avatar Aug 31 '22 18:08 peterthomassen

Thanks a lot! minor nit: what do you think of autosubmitting the login form after the 6 digits got entered (like github does)?

appliedprivacy avatar Sep 05 '22 10:09 appliedprivacy

Unless it's from something like a clipboard event I'd say no because i wouldn't wanna autosubmit a possible typo.

My1 avatar Sep 05 '22 10:09 My1

There's no harm in submitting a typo. I took at stab at it in https://github.com/desec-io/desec-stack/pull/637/commits/57e27018be221f6e2da851f3da44fbb38d64f8ab.

peterthomassen avatar Sep 06 '22 19:09 peterthomassen

I just tried setting up 2FA for my account and may have noticed a problem with auto submit. When I copy & paste the 6 digit code from my Authenticator for first verification during setup, the token will be listed under TOTP Tokens, but 2FA will not be activated, I can log in without. I tried again, this time putting in the code during setup manually, this time 2FA was activated and at the next login I can copy & paste again without any issue.

FloSchwalm avatar Sep 10 '22 11:09 FloSchwalm

Thanks for implementing the auto-submit functionality!

Currently, disabling 2FA on an account with 2FA enabled requires no additional steps that you usually see in other places, like having to enter the account password or a TOTP token for that action again, maybe you want to add that?

Another question I was wondering: What recovery mechanisms are there if you lose your second factor? (same as with no 2FA?)

appliedprivacy avatar Sep 10 '22 20:09 appliedprivacy

I just tried setting up 2FA for my account and may have noticed a problem with auto submit.

@FloSchwalm Can you please open a separate issue where we can do the debugging? Thanks!

peterthomassen avatar Sep 12 '22 16:09 peterthomassen

Currently, disabling 2FA on an account with 2FA enabled requires no additional steps that you usually see in other places, like having to enter the account password or a TOTP token for that action again, maybe you want to add that?

When 2FA is activated for an account, then removing a 2FA token only works if the session that is used for doing so is authenticated via 2FA. In other words, if you have just a login or API token, but not submitted a TOTP code, you can't delete a TOTP token.

However, when you have not yet activated at least one TOTP token for the account, then no extra steps are needed for deleting them, because the account is not yet 2FA-enabled.

Does that address your concern / are you observing different behavior?

Another question I was wondering: What recovery mechanisms are there if you lose your second factor? (same as with no 2FA?)

Recovery codes are on the agenda, but have not yet been implemented. I created an issue to track this (https://github.com/desec-io/desec-stack/issues/639).

peterthomassen avatar Sep 12 '22 16:09 peterthomassen

When 2FA is activated for an account, then removing a 2FA token only works if the session that is used for doing so is authenticated via 2FA. [...] Does that address your concern [...]?

I was trying to get another point across, maybe that is better explained with the typical password change forms:

Usually changing a password requires the old and new password although you had to authenticate with the old password in the first place to create the current session, nevertheless requiring the password again makes sense.

So my suggestion was to require a TOTP on the request to delete it from the account (if it is the last). I don't feel like it is super important but just wanted to suggest it. Webauthn support is certainly more relevant :)

appliedprivacy avatar Sep 14 '22 21:09 appliedprivacy