Closed julien731 closed 8 years ago
Thanks Julien.
I just found this plugin: https://wordpress.org/plugins/google-authenticator-per-user-prompt/ Maybe it can provide an idea how to handle that?
Thanks for the link. I will give it a look.
+1
While I like the idea of having a separate form prompting for TOTP only for users actually having 2FA enabled, I'm afraid it is too insecure to do.
I've had a look at Google Authenticator - Per User Prompt as suggested by @solhuebner but the authentication mechanism seems to be weakened by this. While certain measures are taken to ensure that the authentication mechanism remain safe, I believe it's not safe enough.
The main problem here is that, to have a separate prompt for TOTP, the authentication process has to be broken down into two parts:
The problem is that those two steps can't communicate with each other because there is no persistance. This means that (and that's what is happening with Google Authenticator - Per User Prompt the process is as follows:
If step 1 is validated, no authentication is done because we still need to validate step 2.
Step 2 is a completely different step. We just take a user, check his or her TOTP, and if it's valid then we authenticate the user, assuming that step 1 was indeed validated.
Now, even though it is theoretically not possible to reach step 2 if step 1 is not validated, we can't just exclude the possibility that someone could bypass step 1 and only validates step 2.
A clever guy with bad intentions, by looking up the code and racking his brain, figures out a way to inject a correct nonce, which gives him access to step 2 (the nonce being passed as a $_GET
var it's not very difficult to inject it).
If by some clever tricks he is able to validate the TOTP, then the authentication is done, even though he never even input a username and password.
I've been looking at the authentication process, and more particularly wp_validate_auth_cookie()
hoping to find a filter that would allow me to add some logic:
Unfortunately there isn't a useful hook. All available hooks are wrapped in conditional logic making it unusable for our case.
I have pushed a prompt
branch where part of the work is done. I've stopped at the authentication process though because I couldn't find a solution that's satisfying from a security point of view.
If any of you guys have an idea on how to do this safely I'm all ears. If not, I'm afraid I won't be able to implement separate TOTP prompt.
@julien731 thank you for having a look at this and raising your concerns. I hope there is a solution but it looks like you already thought quite a bit about it.
Hi @julien731, thanks for the detailed reply. The idea is solid as it would increase usability, but it sounds like the implementation has some theoretical limitation.
The limitation seem to be how step 1 communicates to step 2 that the username and password is correct. A nonce passed via $_GET is obviously problematic and would weaken the authentication. (The Codex specifically says, "always assume Nonces can be compromised.")
I'm wondering if wrapping the function which displays the TOTP field in current_user_can() would protect against someone correctly guessing the nonces?
Unfortunately that wouldn't work because when we reach the TOTP prompt, the user isn't yet logged-in, making it impossible to use current_user_can()
. The only function that could be used is user_can()
but again, if we assume the login attempt was compromised, it wouldn't be much of a protection because user_can()
would always return true for the user that's being impersonated.
+1
Hey @julien731 I'm author of Google Authenticator - Per User Prompt (GAPUP). Kudos for digging into the code and being so careful about the security of your plugin.
I'm not an expert, so I could definitely be wrong here, but I've spent a lot of time reviewing this, and I believe the process GAPUP uses is fundamentally safe (explained in detail below). I did just release version 0.6 with a few changes to harden the process, though, just as a precaution. Thanks for pushing me to make those.
This is going to be a long comment, so if you're only interested in the bits that are relevant to WP Google Authenticator, you can skip to the An Alternative for WP Google Authenticator
section.
In the future, if you think you've discovered a vulnerability in somebody else's plugin, please practice responsible disclosure.
If you can put yourself in their shoes for a minute, how would you feel if someone found a security hole in one of your plugins and published the details without giving you a chance to fix it first, and didn't even let you know about it? That would give attackers a huge head start and put all your users at risk.
It's often pretty easy to find a developer's contact information, and if you can't, you can always report issues to the WordPress.org plugin team, and they'll work with the author to get it solved.
In my case, I even have a bug bounty setup, where I'll happily pay people who find vulnerabilities and disclose them privately.
I definitely agree that nonces aren't appropriate for authorization, but I think that, at least in this context, they are adequate for authentication. In this case, the nonce is essentially replacing the user's password, and in the vast majority of cases, it's going to be much stronger and much more difficult to crack than the user's.
Part of that is because GAPUP doesn't actually use Core's nonces. Core's nonces have a long expiration period and can be used multiple times. Both of those qualities make it easier to brute-force them. GAPUP creates nonces in a similar way to Core, and uses Core's underlying hashing function, but they have a very short expiration period, and can only be used a single time.
So, in order for an attacker to exploit this, they would have to monitor a user while they log in, determine that user's nonce, and then send the nonce before the user sends it, and before the nonce window expires.
Generating a valid nonce requires either: a) knowing all the components that were used to originally create it; b) brute forcing all possible values; or c) knowing some of the values and brute-forcing the remaining search space.
The values the attacker would need to know are:
1) The user's ID (easy to get) 2) A cryptographically-secure pseudo-random number (very difficult) 3) The exact microsecond that the nonce was generated (difficult) 4) The NONCE_SALT (difficult)
The attacker would have at most 60 seconds to generate the correct nonce, and at worse just a few seconds.
The nonce is transmitted as a GET parameter, but sending it via POST or a cookie wouldn't provide any meaningful protection, only a trivial amount of obscurity.
cc @soulseekah (who wrote that comment on the Codex), in case you see a problem that I missed.
Regardless of whether or not it's safe to use nonces for authentication, I think I've come up with a better approach.
Instead of splitting the screens into the username + password in the first step, and the OTP in the second, you could split the screens into only the username in the first step, and then the regular password + OTP in the second step. That way you make sure that the regular password and OTP are checked in the same step, avoiding the need for a nonce altogether.
The only downside I can think of there is that the users who don't have the plugin configured for their account would have 2 screens instead of 1. I haven't spent a lot of time thinking through the details, though, so there could be other problems.
I'm happy to be proven wrong on anything above, but if you think there's a practical vulnerability in GAPUP 0.6, please report it privately to my bug bounty or to ian@iandunn.name.
Hi @iandunn
First of all, sorry if you feel I didn't report this correctly. I didn't report it because I don't think this is a security hole with your plugin, but rather a design issue with WordPress itself for doing this kind of thing. It looks to me like WP just isn't designed to have the login process modified to that extent.
With that being said, I can see your nonces are custom and designed to be stronger.
I agree that the whole process it quite strong and that it is unlikely that someone will crack it. My decision for closing this issue was to not add the smallest chance of vulnerability in the login process. That might be over zealous but I prefer this approach for a security plugin.
Regarding your suggestion for breaking down the login process differently is very interesting. Sounds like a "Google-style" login process with login and password in different steps. Which makes me think that, even for users not using 2FA, it might not be such a big deal. I, at least, don't feel bothered by this approach when logging into Gmail.
Your suggestion definitely raised my interest in this issue again though and I'll be thinking about a new approach for a 2-parts process, maybe using Ajax for doing so.
Sounds good :)
You might be interested in following #15384-wordpress, which will make WP much more flexible for this kind of thing. There's also a feature-plugin to bring some basic 2FA support to Core, which plugins could build on top of.
As suggested by MJP, the auth code could possibly be moved to a second step. It would make things clearer for users who are not using the 2FA.