TOTP code can be brute-forced #911
Loading…
x
Reference in New Issue
Block a user
No description provided.
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Let's assume the following ratelimit options:
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:
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/
Thanks for the analysis!
I think there's a few ways to fix this:
Maybe a general cooldown for TOTP requests makes sense.
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 🙃
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.
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.
I think there are too many emails :)
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 loginI 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.
Definitely, looks like I overlooked that. Only though about unlocking the account when resetting the password.
Now sending less emails in
24f7d9b4f7
and resetting the attempts after a successful login in220f43331f