Security audit

[ChristophWurst]

Once this is in an usable state for end users, ask security experts to review/audit the code

[jas4711]

I consider myself a end user and I'm using your plugin on a live OC9.1 instance in daily use by ~5 people. So please go ahead and review the code, people! :)

Thanks Christoph for writing this plugin! The UI can be improved, but the functionality appears to work as far as I have been able to test at least.

[thomaskonrad]

One thing I just noticed which is very security relevant: After a certain amount of failed attempts to enter the OTP, the session must be killed and the user must be locked. Otherwise, the OTP is worthless, because a 6-digit number can be brute-forced in a reasonable amount of time (1,000,000 requests is doable).

It is quite hard to do this right, especially the user lockout, because if you lock users until they are unlocked by an administrator, the ownCloud instances out there will be susceptible to a denial-of-service attack. So a temporary lockout might be the solution. I'd suggest to look at how other services (Google, Dropbox) handle the brute-force case.

[ChristophWurst]

Good point, but shouldn't that be prevented by the core/server instead? Otherwise every 2FA provider app has to implement its own anti-bruteforce mechanism.

[jas4711]

There are several mitigators to this:

1. Don't permit more than one OTP attempt per 10 second period or similar.
2. After about 4 failed attempts, fail authentication so that a password input is necessary again.
3. After more than 5 failed attempts, suspend IP address for an hour.
4. Make admin/user aware of failed attempts.

Still, a 6-digit OTP can never be stronger than a, err well, 6-digit OTP. It is impossible to make a 6-digit OTP code fully secure against guessing attacks against someone who know the right password and has access to a large botnet and have patience. It is sufficient to make many attacks more costlier, and this is already provided (but can be improved). It provides better security than a password alone. If that isn't enough for someone, then U2F provide stronger guarantees.

Just my $.2 morning rant :)

Thomas Konrad [email protected] skrev: (1 augusti 2016 07:59:20 CEST)

One thing I just noticed which is very security relevant: After a
certain amount of failed attempts to enter the OTP, the session must be
killed and the user must be locked. Otherwise, the OTP is worthless,
because a 6-digit number can be brute-forced in a reasonable amount of
time (1,000,000 requests is doable).

It is quite hard to do this right, especially the user lockout, because
if you lock users until they are unlocked by an administrator, the
ownCloud instances out there will be susceptible to a denial-of-service
attack. So a temporary lockout might be the solution. I'd suggest to
look at how other services (Google, Dropbox) handle the brute-force
case.

---

You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#5 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

[ChristophWurst]

Nextcloud 11 has anti-bruteforce, maybe we can protect the second factor too. cc @LukasReschke

[tribut]

Also remember that the whole point of TOTP is that the codes are only valid for 30 seconds. Nothing even remotely close to 1e6 attempts is possible against a web server in that time frame.

[MariusBluem]

We have some brute-force protection on the login-page starting with Nextcloud 10:
nextcloud/server#479

[thomaskonrad]

@jas4711:

1. Don't permit more than one OTP attempt per 10 second period or similar.

Be aware that a simple sleep() doesn't do the trick here, as an attacker will open several HTTP(S) connections in parallel. This requires the web application to be aware of ongoing login requests. Only one per user should be allowed at a time. This would definitely help, but is hard to implement correctly.

2. After about 4 failed attempts, fail authentication so that a password input is necessary again.

This does not help, because what we want to protect against is that an attacker who knows the password will be able to log into the account. So the re-login is scriptable for an attacker and does not prevent them to log in.

3. After more than 5 failed attempts, suspend IP address for an hour.

Again: An attacker might use several different IPs. Moreover, imagine an attacker sitting behind the NAT of a huge organisation where many users use the same ownCloud instance. One simple brute-force attack would cause the whole organisation not to be able to log in any more. So this is not a mitigation, but opens a denial-of-service vulnerability.

4. Make admin/user aware of failed attempts.

Be aware here that such notification e-mails can't simply be sent each time a failed log-in attempt happens, because then a massive brute-force attack would fill mail boxes in no time.

Don't get me wrong - I'm just trying to say that these things are very hard to implement and must be done by someone who knows what they are doing, otherwise you add a ton of complexity without really mitigating the flaw whilst adding more vulnerabilities.

[thomaskonrad]

@tribut:

Also remember that the whole point of TOTP is that the codes are only valid for 30 seconds. Nothing even remotely close to 1e6 attempts is possible against a web server in that time frame.

That's right. But an attacker could just keep trying 6-digit numbers until one of them matches. This is of course much less likely than when the number never changes (such as with an SMS token), but no entirely unfeasible in practice I guess.

Is anyone able to calculate the average time this would take with a token that renews every 30 secs? I'm afraid I'm not :(

[thomaskonrad]

Good point, but shouldn't that be prevented by the core/server instead?

Yes, I think you are right - it should be prevented in the core (which, as far as I know, also misses a brute-force protection for the general log-in page).

[jas4711]

@thomaskonrad I believe we actually agree on principles: it is possible to do something better than what is done, but it is not possible to reach something that is foolproof against someone equipped with the right password, a botnet and patience. This is an inherent weakness of 6-digit OTP's.

I disagree that all of this belongs in core -- many of the mitigating factors are due to how this particular two-factor authentication mechanism (OATH) works. Many other second-factor mechanisms does not have the same concerns. Compare U2F for example. Many mitigating mechanisms that make sense for OATH does not make sense for U2F. Perhaps core can include a framework for helping implementing these mitigating mechanisms?

Just brainstorming...

[ChristophWurst]

@LukasReschke what do you think? :-) Can we close this one?

[ChristophWurst]

Yes, I think you are right - it should be prevented in the core (which, as far as I know, also misses a brute-force protection for the general log-in page).

Nextcloud has built-in brute-force protection since version 10.

Let's continue the discussion regarding second-factor brute-force protection in nextcloud/server#2626. Thanks everyone for your valuable input, this is highly appreciated!