Some security issues

[vilda]

There's couple of things to consider before using this library in production. Since their implementation take some time, I suggest to update documentation.

Probably the most serious is missing rate limitations. If the account's password is compromised then its security depends on the strength of the authentication code. Authentication code is small however, susceptible to brute force attacks. Should the password be compromised, then multiple failed attempts to authenticate should lest to account lockout. This is recommended by RFC 4226.

A good practice is to store security keys in DB encrypted. Using one-way hash like scrypt is not an option for TOTP, since we need it actual value, but simple encryption with a key not stored in DB would suffice. This is recommended by RFC 6238.

Lastly a minor enhancement - verified code should not be accepted twice. This prevents over the shoulder attacks. I don't see it absolutely critical, but RFC 6238 declares it as MUST.

[jlhonora]

Thanks for your insights, they make a lot of sense.

Just a small comment: the auto_increment option should prevent using the same code twice, unless I'm missing something.

[andresriancho]

Probably the most serious is missing rate limitations

I agree, this library should have throttling embedded. Without that security measure the whole thing gives a false sense of security (unless the developer is skilled, understands the risks and implements throttling himself).

[23ranjan]

@jlhonora For some reasons my auto_increment is not working.

Tried generating the migration for otp_counter

This is my user model has_one_time_password counter_based: true, column_name: :otp_secret_key, length: 4

Any ideas ?