On Fri, Jul 19, 2019 at 11:45:02AM +0000, gv@members.scinet.supercomputing.org wrote:
On Thu, Jul 18, 2019 at 05:53:54PM -0400, Greg Veldman wrote:
Would a default-off ifdef to activate that code block work for this? I did intentionally keep that part of the change self contained, so it wouldn't be hard to add that...
Actually, re-reading RFC 6238 this morning (section 5.2, especially paragraph 1), I believe this part of the proposed change is both compliant and in accordance with recommended practice in its current form.
Yes, it makes sense to check the previous interval in case you entered the token just at the end of its validity even if you don't choose to test them in a broader window for resynchronisation (section 6).
BTW, why did you choose to check that the user has already logged in rather than "(told == 0 || t - 1 > told)"?
It might be better to check both the token and password independently and then return success iff both of them succeeded to avoid timing leaks of OTP success/failure?
Sounds like a powerful oracle with PBKDF2 and similar hashes if you don't aggressively lock out users. Also probably needs to track the last successful OTP token somewhere else than the lastbind attribute even if the overall bind failed to mitigate the opposite attack.
Also maybe use op->o_time instead of time(0L).
Regards,