Add 2 Factor Authentication to Admin CP #1623

Closed
PirataNervo opened this Issue Nov 17, 2014 · 37 comments

Projects

None yet

8 participants

@PirataNervo
Contributor

To enhance security on MyBB forums, we're adding 2-FA on the Admin CP.

A new table/container will be added to Home - Preferences in which admins can turn On 2-FA by doing the following:

  • Scan QR code (which changes on each page load)
  • After scanning, the user must enter the code provided by his QR code scanner (like Google Authenticator) in a textbox to validate the QR code. There should exist a 2 minute relaxation time (- and + 2 minutes) for time computation differences.

After it's enabled, the QR code displayed on that page does not change. Enabling or disabling gets logged in the Administrator Logs.

When logging in, the 2FA code should requested after the username/password combination is valid, if the specified user has 2FA enabled.

@PirataNervo PirataNervo self-assigned this Nov 17, 2014
@PirataNervo PirataNervo added this to the 1.8.3 milestone Nov 17, 2014
@JN-Jones
Contributor

If this is added to the core we also need to add recovery codes (or something like that) to avoid that admins are completly logged out.

@Cu8eeeR
Contributor
Cu8eeeR commented Nov 17, 2014

@JN-Jones @PirataNervo - you can add 2FA for users too and integrate this plugin into core (http://community.mybb.com/thread-162369.html)

EDIT: Recovery codes must be added

@PirataNervo
Contributor

We could add recovery codes yes but to disable 2FA for a certain admin he's just need to set the 2FA key field to empty in the database.

@JN-Jones
Contributor

We shouldn't force users to change something in the database. Even if it's only one field.

@PirataNervo
Contributor

I'm just saying, recovery codes were first implemented because users using 2FA would have no way of proving they're the rightful owners of their accounts. If you're the server manager, you know you're the owner. But I guess we could use recovery codes just in case.

@Sama34
Contributor
Sama34 commented Nov 17, 2014

I suppose they think about forum administrators that are not server administrators (DB).

Also, I think the request of the 2FA should be done after the login page (most sites do it like this).

@PirataNervo
Contributor

Why do you think it should be done like that? Just because most sites do it, doesn't mean it's the best way to do it. IMO, it could lead to potential session bugs as we'd need an extra validation page after a successful username/password combination test (which right now is a session creation, rather than a test). If you simply add the 2FA check in the same process, you reduce the chances of having sessions issues at this point. It's way less intrusive.

@PenguinPaul
Contributor

While @Sama34 is right that that's how most people do it, I think that it would be simpler for us just to use @PirataNervo's idea.

@JN-Jones
Contributor

The reason they do it, is to hide it for users who haven't enabled 2FA. It's easy in the ACP to add it as a new page for every module (add a new check in admin/index.php) without session bugs.

@Cu8eeeR
Contributor
Cu8eeeR commented Nov 18, 2014

I agree with @JN-Jones - why force users to use it...

@PirataNervo
Contributor

No one's forcing to use anything...you only tick the box if you have 2FA enabled on your account.

I'd like everyone to remember that 1.8.3 is to be released soon...we don't have time for complex features. The simpler the ideas, the less the bugs.

@JN-Jones
Contributor

IMHO it's a lot more intuitive if you enter your usual details and then either are forced to enter the 2FA key or logged in automatically than always remembering to tick a new box and enter the key. And both isn't complex, it's pretty easy to implement 2FA to the ACP.

@PirataNervo
Contributor

Alright updated first post with re-written info. I'm still not sure about recovery codes though...IIRC they're used to validate someone's authenticity. Therefore, they are requested by a human to validate the person on the other side. Do you think it's useful for an admin to verify the authenticy of another admin? Unless I misunderstood the concept of recovery codes here (might be different from what I'm used to).

@Cu8eeeR
Contributor
Cu8eeeR commented Nov 19, 2014

@PirataNervo - difficult to say what is better. There are two approaches. I understand you, but without recovery codes it would cause many problems (for example: lost phones with authentificators and many more).

For idea where to place 2FA - I would like to see @JN-Jones implementation - first you have to type you name + pw + pin (in ACP) and then you are redirected to 2FA page.

One more thing - you wanna make MyBB more secure, but there are some ideas in community board which should be considered too for 1.8.3 and they are easier to implement than 2FA.

Do not hurry with 1.8.3 release - it should contains as much as possible improvements in security

@JN-Jones
Contributor

What I understand as recovery code(s) is what github does when enabling 2FA: Showing a new page with 5 or 10 random codes which are reloaded every time that page is shown (which means that those codes aren't shown on the standard setting page, they're only shown on a special page). Those codes can be used once instead of a time based token if you've lost your phone or whatever.

@Cu8eeeR
Contributor
Cu8eeeR commented Nov 19, 2014

@JN-Jones @PirataNervo Same like in gmail when 2FA is activated. Try it if you dont know how it works. I like its system

@PirataNervo
Contributor

@JN-Jones I thought the codes will not show up anymore after 2FA is enabled? They are only shown once after enabling it . (i.e. you can disable and enable again to get new codes). So what you're suggesting is a "don't have 2fa code" feature and in that case the user enters one recovery code - which can be used once only.

@Sama34
Contributor
Sama34 commented Nov 20, 2014

Yes that is it.

Recovery codes are random generated codes to bypass your 2FA instance once for whatever reason (lost phone, you are up in an airplane, etc). "Security-alternative codes" could be more appropriate I think.

@PirataNervo PirataNervo removed their assignment Jan 12, 2015
@Sama34
Contributor
Sama34 commented Jan 17, 2015

@JN-Jones would you be willing to include MyBB-2FA?

@JN-Jones
Contributor

Sorry, was busy during the weekend. Yes I can work on it if we agree where and how to add it. As already said: IMHO we should add it to the front end too (similar to my 2FA plugin) and not only to the acp @Stefan-ST

@JN-Jones JN-Jones self-assigned this Jan 19, 2015
@Destroy666x
Member

If you want to add it to front-end too, I don't see why not.

@JN-Jones
Contributor

Any objections against the way my plugin works? The code will be optimized of course as I don't need to relay on hooks but the general way. (Recovery codes will be added too but they aren't included in the plugin atm)

@Stefan-ST
Member

@JN-Jones I think 2FA is not needed for the front-end because 99% of the forum users won't use it and we mainly want to secure the back-end.
However if you already have a solution that protects both, why not.

@JN-Jones
Contributor

I'd use the solution from my plugin: https://github.com/JN-Jones/MyBB-2FA
It displays the QR Code in the UCP and sets a new column to the secret. If the secret isn't empty the login page (frontend and acp) will redirect to a new page where the token should be entered. The "block" part of the plugin will be modified of course. The login part is nearly the same for both.
I need to look into an issue with Google Authenticator someone reported first but noone else had any problems with the implementation so far.

@Sama34
Contributor
Sama34 commented Jan 20, 2015

I agree it should be implemented in both areas considering you already have most of the code anyways.

@JN-Jones
Contributor

Ok, I've a made something for the frontend, however there's one issue: I need to save whether or not the 2FA challenge has been entered for the current session. But as MyBB only saves one session per user I'd either need to save it as a cookie (problem: user can change it) or the value may be screwed when the same user is logged in from two pc's (either the user needs to reenter a new code on every page visit or one of the two sessions is automatically validated though it shouldn't).

I can leave it as it is in my plugin (one session is probably validated) or only implement it to the acp where multiple sessions are saved. I'd go for the second as it doesn't make sense to implement it when it's easy to break.

@Sama34
Contributor
Sama34 commented Jan 21, 2015

So we are presented with session l;imitations in the front-end? We can leave it for the ACP at first then and then over time implemented in the front-end, which would mean to modify the sessions system on it too.

@JN-Jones
Contributor

Modifying wouldn't help, we'd need nearly a complete rewrite :P

@JN-Jones JN-Jones added a commit to JN-Jones/mybb that referenced this issue Jan 22, 2015
@JN-Jones JN-Jones WIP #1623 Added QR Code and checks for ACP login 7120ec1
@JN-Jones
Contributor

Todo:

  • Test with Google Authenticator
  • Add recovery codes
  • Log enabling/disabling (is that really necessary?)
  • Add new column to SQLite/PgSQL and upgrade script
@Sama34
Contributor
Sama34 commented Jan 23, 2015

Log enabling/disabling (is that really necessary?)

Since it would only apply to the ACP, yes it wouldn't hurt having it.

@JN-Jones JN-Jones added a commit to JN-Jones/mybb that referenced this issue Jan 24, 2015
@JN-Jones JN-Jones WIP #1623 Added enabling/disabling 2FA to the log 33f1490
@JN-Jones JN-Jones added a commit to JN-Jones/mybb that referenced this issue Jan 24, 2015
@JN-Jones JN-Jones WIP #1623 Add recovery codes e77f026
@JN-Jones
Contributor

The functionality has been added, only testing is needed. I've used Authy as app, however many people use Google Authenticator so that app should be tested too. Also the upgrade/installation process hasn't been tested.

@Sama34
Contributor
Sama34 commented Jan 24, 2015

I can't find your PR :P

@ATofighi ATofighi referenced this issue Jan 24, 2015
Merged

Fix 1623 #1777

@JN-Jones
Contributor

I didn't want to create a PR before testing it... Also a PR isn't needed at all for testing as you can simply go to my repo, select the branch and download it

@Sama34
Contributor
Sama34 commented Jan 25, 2015

Alright, I just noted some issues without testing.

@JN-Jones JN-Jones added s:fixed and removed s:in-progress labels Jan 26, 2015
@JN-Jones JN-Jones added a commit to JN-Jones/mybb that referenced this issue Jan 29, 2015
@JN-Jones JN-Jones WIP #1623 Rename directory 50fce08
@JN-Jones JN-Jones added a commit to JN-Jones/mybb that referenced this issue Jan 31, 2015
@JN-Jones JN-Jones WIP #1623 Avoid generating the same code twice and added a notice whe…
…n all recovery codes are used
464001d
@JN-Jones JN-Jones added a commit to JN-Jones/mybb that referenced this issue Jan 31, 2015
@JN-Jones JN-Jones WIP #1623 & should be & cda0d87
@JN-Jones JN-Jones added a commit to JN-Jones/mybb that referenced this issue Feb 4, 2015
@JN-Jones JN-Jones WIP #1623 Add lockout feature 6649821
@JN-Jones JN-Jones added a commit to JN-Jones/mybb that referenced this issue Feb 11, 2015
@JN-Jones JN-Jones WIP #1623 Add link to docs for 2FA apps b066c5b
@PaulBender
Member

Fixed in #1777

@PaulBender PaulBender closed this Feb 11, 2015
@Cu8eeeR
Contributor
Cu8eeeR commented Feb 16, 2015

@JN-Jones - what about "remember this PC" option - no codes are needed in future (for example for another 14 days)

@Destroy666x Destroy666x removed the p:high label Feb 16, 2015
@Sama34
Contributor
Sama34 commented Feb 17, 2015

@Cu8eR please open a new issue with your suggestion.

@Cu8eeeR Cu8eeeR referenced this issue Feb 17, 2015
Open

2FA - improvements #1843

0 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment