Two Factor local login proposal

Continuing the discussion from Two-factor local login option:

Hey everyone, I’m planning to look into tackling two factor authentication very soon. This would allow a user to secure a discourse account by requiring a one time password to be generated via Google Authenticator/Authy.

Here’s a proposal to how two factor authentication through google authenticator flow would work:

The user would log in as usual. After a user logs in, the system checks whether or not the user has two factor enabled. If two factor is enabled, a second login prompt would be shown, asking a user for their one time password.

A user will be able to enable two factor authentication in the user preferences.

Enabling would bring up a QR code, that would need to be confirmed with a successful one time password.

If a user is unable to scan the QR code, or prefers to type in the key manually, they are able to view the key via the “enter the key manually link”.

Once Discourse confirms that you can successfully generate codes, it will require future local logins to be completed with an authorization key.

Some implementation details:

If a user attempts to disable a 2fa key, it will ask for a token before the key is disabled.

If a user re-enabled 2-factor, the 2fa key will be re-generated. This allows a 2-factor “reset” in the event of a stolen key. However, any authenticator would have to be re-set up, which could be a pain if you have multiple devices/apps that you would like to set up with the authenticator.

I really like the idea of being able to generate some kind of backup key codes, in the even that you are parted with your authenticator app somehow, or perhaps gets disabled when a user resets a password. The latter would follow the general Discourse account handling that “if you have access to the email, you have access to Discourse”. (Backup codes tabled for v2.)

I also really like the idea of being able to set up two factor through texts later, possibly by using a twilio API.
Edit: not happening, as NIST recommends against this.

As a first step, I’m looking to tackle the basic flow described above to get Discourse working with Google Authenticator, and expand out from there. I’m looking at adding the gems rotp for the code generation + checking, and rqrcode for processing QR codes.

Admins and Moderators will be able to disable 2fa for other users.

Any feedback, requests, or security concerns about this proposal are welcome - Let me know what you think!

25 Likes

I don’t - and more important: NIST doesn’t like it anymore either:

11 Likes

That’s looking amazing, thank you for working on this! :smiley:

As an alternative second factor, support for U2F tokens might make more sense than SMS tokens – it’s very secure and is gaining traction (Firefox recently added native support, Chrome has had it for quite a while).

7 Likes

Thanks for the feedback, I’ll nix the idea of text then, and look into uf2.

2 Likes

I would say, if you enabled 2fa and then click “disable”, you must enter a token to actually disable it.

I would hold on this to v2, I like the idea of backup codes but would like to live with this first

Looks like a good plan, I would prefer only to add the GitHub - mdp/rotp: Ruby One Time Password library dependency cause I dont want some gem deciding how our models should look and how we implement stuff. The rotp gem is going to be a much safer and easier to audit dependency.


I would also ensure that admins/moderators can “disable” 2fa for other users (except for themselves) via the admin screens.

I do wonder if we should add a link of “I lost my phone / uninstalled app” on the "enter your otp page, but it can possibly wait for now.

7 Likes

What should that link do?
I’d suggest just referring the user to the contact email address – if the request looks legit, a staff member can then disable 2fa for that user :slight_smile:

2 Likes

Just to confirm, would admins/mods will have the ability to disable 2fa on anyone but themselves, or any non-admin users? (Should admins/mods be able to disable another admin’s 2fa?)

That makes sense, I’ll add that to the spec.

Thanks for the suggestion - I’ll add that to the dependencies instead of the active_model_otp one.

That’s why I was suggesting it to be disabled when a user verifies their email in a forgot password link. Personally, the best way to verify a legit request would be to check the user’s email origin, which could be done without staff intervention. This gives the 2fa to the local discourse account with no increased risk to user lockout while they have access to their email.

2 Likes

Updates - Making good progress so far.

One change I would like to get approved:

I’m adding a dialogue box to confirm your password before you are able to view the 2fa key. This prevents anyone from having access to your OTP secret token from your session (either as some sophisticated remote attack, or by simply being next to your computer that you walked away from). The request to view the token will require a password parameter.

Done:

  • Login via the 2fa authentication key. User option enables/disables otp checking.
  • User preferences enable - type in password to display QR code. Submit a valid token to enable.
  • User preferences disable - submit a valid token to disable.

Todo checklist:

  • Clean up the login prompt, and user preferences UI.
  • Clean up the functionality to enable/disable so that if a user somehow double-submits it’ll know how to set.
  • Better transitions/loading after enabling 2fa.
  • Better error handling/error messages.
  • Update admin login
  • Add admin options to disable 2fa for users.
  • Convert all messaging to l18n.

It is still alpha/debug quality, but I’d like to publish progress as a “working implementation” milestone has been reached.

10 Likes

Hmm here are some initial thoughts

Process for changing password should be:

  • send password reset
  • approve 2fa
  • we send email
  • user clicks on link in email
  • another 2fa
  • change password

To disable 2fa

  • approve 2fa (or you need an admin)
  • send email for confirmation
  • user clicks on link in email
  • approve 2fa
  • 2fa is disabled (so you had to 2 2fa confirmations to have it disabled)

To change email

  • approve 2fa
  • send email to be changed message to new and old emails (like it is today)
  • click on email
  • approve 2fa

Basically, changing email/password or disabling 2fa gets very annoying once you have 2fa enabled, by design.

6 Likes

I was initially erring on the “A” side of confidentiality, integrity and availability, for more user-friendlyness, and continuing the assumption that if a user has access to the email, then they should have access to the discourse account.

But if 2fa is being used as a failsafe in the event that a user’s email has been compromised, I’ll work in that direction.

I’m not sure what having two 2fa confirmations will add, other than being super annoying. If they have a valid 2fa key, shouldn’t that be sufficient to disable the 2fa on its own, without the extra email confirm?

I have a working version in which:

  • User clicks “disable”
  • User enters 2fa
  • 2fa gets disabled.

Similarly, while I agree that it’s probably a good idea to change email or password with 2fa confirmation, doing it twice doesn’t seem to add any security.

Can I suggest the following for the email/password workflow instead?

  • send email
  • user clicks link
  • 2fa challenge on link
  • once challenge is completed, email/password change accepted

Unless I’m missing something - what does having two 2fa challenges add to this workflow?

2 Likes

The main issue is timing…

If you confirmed the 2fa at 2pm and then go and finally change password/email at 7pm we need another 2fa.

Basically we need it so a 2fa “unlock” only unlocks change password/ change email for N minutes.

With 2fa, only if a user has access to both email and 2fa device are they granted access.

I feel requiring email to disable 2fa is reasonable cause you force a better audit trail as well.

3 Likes

For email/password changes - In my proposed workflow, the timing would be the same as your proposal – a single 2fa challenge at the time of the password change is the same timing as your “second” challenge. I was suggesting we remove the first 2fa challenge before the initial emailing.

As for disabling 2fa, we could add a confirmation email a user and inform them that 2fa has been disabled for auditing without the extra key input. Disabling 2fa in my proposal still means: They have logged into the account somehow, and they can supply a valid 2fa key, which to me would be sufficient to prove authenticity to disable it.

I think my main concern is that requiring two 2fa challenges rings of a little security theater to me.

2 Likes

I think that as long as we defer the 2fa to the latest point in time we can be fine.

So:

  1. click “send password reset email”
  2. User clicks on email
  3. User confirms 2fa
  4. User changes password

My issue is that I want the 2fa always performed just before the actual database write.

A change password link for example can be opened on any browser and from any IP. I think it is important to defer the security to the last moment to reduce surface area.

6 Likes

Remember to have an admin escape hatch to remove 2FA from the account, because you’re going to get people who didn’t set up enough backups. I suggest strong wording along the lines of “Please make extra sure this request is actually from the person who owns the account!”

You could even have an email notification “2FA Removal Requested” and mandatory time delay + cancel link, but that does feel like it might be overkill?

On a technical note, I think there’s value in having a single second_factor_data table containing all the methods registered to accounts, instead of spreading it out over multiple tables - that way, making the management UI for removing and auditing your 2FA methods only needs a single query.

user_id method created_at last_used data
208 totp 2018-01-10 … 2018-01-11 … 9bJGDVffCc6G58Epr1xJ
208 recovery 2018-01-10 … NULL JFMYBZCVZ8
208 recovery 2018-01-10 … NULL VJND3AECEH
208 recovery 2018-01-10 … NULL D4KL2WYZYL
208 recovery 2018-01-10 … NULL R6AYTHTIHO
208 u2f 2018-01-12 … 2018-01-12 … {“version”:“U2F-V2”,“keyHandle”:“d7AIn-S2cx7n2GgttorX_w”, “publicKey”:“MIIDnzCCAoegAwI…”}
136 webauthn-tpm 2018-01-07 … 2018-01-12 … {“publicKey”:“MIIDnzCCAoegAwI…”, …}
136 recovery 2018-01-07 … NULL VTXFPJJGWE

I had a little bit written about plugins adding extra methods, but there isn’t really anything mature other than U2F right now.

4 Likes

Can it be extended to use Authy as well?

They have some relatively advanced options compared to Google Authenticator and They have this really simple process of approving a sign in via a Notification.

Authy supports the same totp protocol as Google authenticator, so you would be able to use that app as a drop in replacement.

Any “extra” methods that are authy specific would need to be built separately though, I don’t plan on supporting anything outside of totp for the initial release of this.

8 Likes

Agreed - the Authy custom protocol needs to be in a plugin, and NEVER required/mandatory.

4 Likes

Weekly update:

Completed all of the entrypoints where a second factor is required.

Done:

  • Refactor second factor settings into its own table in anticipation for possible additional second factor options. :white_check_mark:
  • Login screen UI. :white_check_mark:
  • Second factor required on admin login (if enabled). :white_check_mark:
  • Second factor required on change password (if enabled). :white_check_mark:
  • Second factor required on change email (if enabled). :white_check_mark:

TODO:

  • Clean up user preferences UI.
  • Better error handling + error messages.
  • Skin change email screens a little better, add invalid token messaging.
  • Email notification when second factor is disabled.
  • Refactor and normalize names, variables, and parameters. The most current name stands as “second_factor_tokens” to encompass possible other future 2fa methods.
  • Convert messages to I18n.

Concerns:
It’s difficult to implement rate limit counting “correctly” here. Currently a valid login eats up two “attempts” at rate limiting for a single password + second factor token request: One for a password attempt, and another for a password + second factor attempt. I want to rate limit somehow - the easiest way would be to share the rate limits between the two as it doesn’t leak what failed when logging in, so we don’t leak which users do or do not have 2fa enabled by just a username. It’s the simplest method of rate limiting, but would be problematic for sites set to 1 max login per ip per minute/hour. Do we need a second rate limiter for 2fa attempts?

15 Likes

I would go with what is simplest and cleanest to implement, adding another rate limiter if needed is fine

6 Likes

OK, I’ve added a simple rate limiter, hard coded with a rate of 6/minute/ip for now.

Done:

  • Cleaned up UI, better look/feel across the board :white_check_mark:
  • Display error handling + messages :white_check_mark:
  • Send an email notification when second-factor is disabled :white_check_mark:
  • normalized names. Display as “2-Step Verification” to keep in line with other common 2factor names. Internal name as second_factor. :white_check_mark:
  • Converted all messages to I18n. :white_check_mark:

TODO:

  • Feedback, and cleanup?

https://github.com/discourse/discourse/pull/5521

Did I hear you say screenshots?

Setup phase:

After enabling, disable is similar:

Login prompt:

Password reset:

Change email:

Admin login:

2fa-admin-login-1

Admin panel to turn off other users’ 2-fa:

Disable email audit notification:

17 Likes