TOTP code can be brute-forced #911

Closed
opened 2021-07-07 09:49:10 +00:00 by andreymal · 9 comments
Contributor

Let's assume the following ratelimit options:

ratelimit:
  enabled: true
  kind: ip
  period: 60
  limit: 100

This means ~1.6 requests per second.


By default the totp library uses Skew: 1, which means it checks 3 codes per request.

The chance that the code will be guessed is 3 / 1000000 = 0.0003% per request.

After 300000 requests the chance of failure is (100% - 0.0003%)300000 = 40.6%


This means that an attacker (who knows the user's password from somewhere) has ~60% chance to "hack" the account in just 2 days.

In case if ratelimit is disabled (I got ~8 requests per second) the required time is reduced to 8 hours.

In practice, I successfully "hacked" my test account after 401842 requests in 13.8 hours (with ratelimit disabled) using a simple single-threaded Python script.

(Update 12:14 UTC: I also tried again with ratelimit enabled — it took 17062 requests in 3 hours. Hail random! \o/)

If the user/admin is not monitoring CPU usage or Vikunja logs by some reason, they may not see this attack.


For comparison, I tried to repeat the same on GitHub:

  • it blocks IP after 5 failures for 10 minutes
  • it also blocks the ability to log into the attacked account, so using many IPs is useless for the attacker
  • it sends an email notifying about failed login attempts so that the user can change their password

I suggest to implement similar things in Vikunja regardless of whether ratelimit is enabled or disabled.


See also: https://lukeplant.me.uk/blog/posts/6-digit-otp-for-two-factor-auth-is-brute-forceable-in-3-days/

Let's assume the following ratelimit options: ```yaml ratelimit: enabled: true kind: ip period: 60 limit: 100 ``` This means ~1.6 requests per second. --- By default the totp library uses [Skew: 1](https://github.com/pquerna/otp/blob/766957efc75a746be2bff8ef6afe7ea1169d92fd/totp/totp.go#L43), which means it checks [3 codes](https://github.com/pquerna/otp/blob/766957efc75a746be2bff8ef6afe7ea1169d92fd/totp/totp.go#L105-L109) per request. The chance that the code will be guessed is 3 / 1000000 = 0.0003% per request. After 300000 requests the chance of failure is (100% - 0.0003%)<sup>300000</sup> = 40.6% --- This means that an attacker (who knows the user's password from somewhere) has ~60% chance to "hack" the account in just **2 days**. In case if ratelimit is disabled (I got ~8 requests per second) the required time is reduced to **8 hours**. In practice, I successfully "hacked" my test account after 401842 requests in 13.8 hours (with ratelimit disabled) using a simple single-threaded Python script. (**Update 12:14 UTC**: I also tried again with ratelimit enabled — it took 17062 requests in 3 hours. Hail random! \o/) If the user/admin is not monitoring CPU usage or Vikunja logs by some reason, they may not see this attack. --- For comparison, I tried to repeat the same on GitHub: - it blocks IP after 5 failures for 10 minutes - it also blocks the ability to log into the attacked account, so using many IPs is useless for the attacker - it sends an email notifying about failed login attempts so that the user can change their password I suggest to implement similar things in Vikunja regardless of whether ratelimit is enabled or disabled. --- See also: https://lukeplant.me.uk/blog/posts/6-digit-otp-for-two-factor-auth-is-brute-forceable-in-3-days/
konrad added the
security
label 2021-07-07 11:55:21 +00:00
Owner

Thanks for the analysis!

I think there's a few ways to fix this:

  • Log failed totp attempts so that something like fail2ban can pick it up and block the ip after too many attempts. Fail2ban is far more advanced and can be fine-tuned etc. No need to reinvent the wheel here.
  • Email the user when someone logs in using a correct password but entered an incorrect totp code.
  • Maybe after 10 failed attempts disable the account, reset their password and send them a mail to notify them so they set a new password.
Thanks for the analysis! I think there's a few ways to fix this: * Log failed totp attempts so that something like fail2ban can pick it up and block the ip after too many attempts. Fail2ban is far more advanced and can be fine-tuned etc. No need to reinvent the wheel here. * Email the user when someone logs in using a correct password but entered an incorrect totp code. * Maybe after 10 failed attempts disable the account, reset their password and send them a mail to notify them so they set a new password.
Contributor

Maybe a general cooldown for TOTP requests makes sense.

Maybe a general cooldown for TOTP requests makes sense.
Author
Contributor

something like fail2ban can pick it up and block the ip

Note that GitHub shows an informative error page and doesn't block other sessions that are already logged in (this is important for users behind NAT) — I'm not sure if this is possible with fail2ban.
That's why I prefer to reinvent wheels sometimes 🙃

>something like fail2ban can pick it up and block the ip Note that GitHub [shows an informative error page](https://i.imgur.com/7LteBxr.png) and doesn't block other sessions that are already logged in (this is important for users behind NAT) — I'm not sure if this is possible with fail2ban. That's why I prefer to reinvent wheels sometimes 🙃
Owner

I've implemented sending a notification for a failed TOTP attempt in 3572ac4b82.

As a next step I would disable the account after 10 failed attempts and send the user an email to reset their password.

I've implemented sending a notification for a failed TOTP attempt in 3572ac4b8232ddcb5d8a6a34f82211f5f388fedf. As a next step I would disable the account after 10 failed attempts and send the user an email to reset their password.
Owner

Implemented disabling the account after 10 failed TOTP attempts in 5cfc9bf2f9 - sending a bunch of emails in the process.

I'd consider this done now, feel free to reopen or ping if you find any other issues with it.

Implemented disabling the account after 10 failed TOTP attempts in https://drone.kolaente.de/link/vikunja/api/commit/5cfc9bf2f95effac4f837fbdfd3ec598ad05f02c - sending a bunch of emails in the process. I'd consider this done now, feel free to reopen or ping if you find any other issues with it.
Author
Contributor

I think there are too many emails :)

I think there are too many emails :) ![](https://i.imgur.com/1KFISrn.png)
Author
Contributor

I also noticed that keyvalue storage has no timeout and Vikunja remembers all login attempts, so if I make a typo in the TOTP code, my account is immediately disabled after first try. I guess it's safe to call something like keyvalue.Del(user.GetFailedTOTPAttemptsKey()) after resetting password or after successful login

I also noticed that keyvalue storage has no timeout and Vikunja remembers all login attempts, so if I make a typo in the TOTP code, my account is immediately disabled after first try. I guess it's safe to call something like `keyvalue.Del(user.GetFailedTOTPAttemptsKey())` after resetting password or after successful login
Owner

I think there are too many emails :)

I agree :) I think a solution to this may be to send only one of the "Someone just tried to login to you Vikunja account" email after the third failed attempt. If someone is really trying to brute-force, the account will be blocked very soon after that with the other email being sent in that case.

I've added sending the emails after each failed attempt before implementing the account disabling so I did not think about that.

I guess it's safe to call something like keyvalue.Del(user.GetFailedTOTPAttemptsKey()) after resetting password or after successful login

Definitely, looks like I overlooked that. Only though about unlocking the account when resetting the password.

> I think there are too many emails :) I agree :) I think a solution to this may be to send only one of the "Someone just tried to login to you Vikunja account" email after the third failed attempt. If someone is really trying to brute-force, the account will be blocked very soon after that with the other email being sent in that case. I've added sending the emails after each failed attempt before implementing the account disabling so I did not think about that. > I guess it's safe to call something like keyvalue.Del(user.GetFailedTOTPAttemptsKey()) after resetting password or after successful login Definitely, looks like I overlooked that. Only though about unlocking the account when resetting the password.
Owner

Now sending less emails in 24f7d9b4f7 and resetting the attempts after a successful login in 220f43331f

Now sending less emails in 24f7d9b4f7e653b7abec9e2f1ba8e27deb857e7e and resetting the attempts after a successful login in 220f43331f1c6dba351cd8d82400685d2989e656
Sign in to join this conversation.
No Milestone
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vikunja/vikunja#911
No description provided.