Alias blacklist to prevent PIN nbrute-forcing, fixes Issue #234

[fmarier]

I've reviewed the change and as far as I can tell, it does everything that was proposed in #234.

With respect to simplifying the memory management, have you thought of storing only the first timestamp as well as a counter?

e.g. { 'foo@example.com': [ 19826484210, 3 ] } where 19826484210 is the timestamp and 3 is the number of failed attempts

That would make _expireCounter a constant time function and would also limit the size of each map entry to a fixed size of 2 numbers.

[ozten]

I didn't consider this, as it cannot handle expiring a range of attempts.

Example: 5:20 - I do a bad pin attempt 4 times 5:29 - I do a bad pin attempt 5:32 - I make a bad pin attempt

As I interpreted the requirements, at 5:32 we should still count the attempts made at 5:29 against the user, but expire the first 4 attempts from 5:20.

[fmarier]

@ozten You're right, based on the requirements as they are written, your solution is better. I'd like to suggest that we relax the "5 attempts per 10 min" requirement a bit if it helps make the code simpler/faster.

This requirement takes advantage of the fact that after 10 minutes of coming into existence, a PIN will expire. Any previous attempts at cracking that PIN are irrelevant after 10 minutes given that the PIN no longer exists.

So in your example, when the 5:32 attempt comes in, we could assume that the 5:29 attempt was for a previous PIN and is no longer relevant. Now technically speaking, the PIN expiry and the blacklist may not overlap perfectly, for example:

1. 2:20 - create PIN 1 for victim@example.com
2. 2:22 - make 1 attempt at cracking PIN 1
3. 2:30 - PIN 1 expires and we create PIN 2 for victim@example.com
4. 2:31 - make 4 attempts at cracking PIN 2 (victim@example.com is locked out)
5. 2:32 - blacklist is cleared for victim@example.com
6. 2:33 - make 5 more attempts at cracking PIN 2, locking victim@example.com again

but that only doubles the number of attempts in the worst case, it doesn't significantly change the strength of the solution. We would still satisfy a "10 attempts per 10 min" requirement.

[ozten]

I'd like to suggest that we relax the "5 attempts per 10 min" requirement a bit if it helps make the code simpler/faster.

Okay, I'll update the code as you've requested.

@fmarier thanks again for all your help.

@lloyd - did you have any other feedback, that would block this PR? or the overall PIN fix?

[ozten]

This is ready to review. I've removed the list of expiration dates and replaced it with a single date.

Thanks!

[ozten]

@fmarier does this new commit address all open concerns?

[lloyd]

belatedly, I'm completely happy with this if @ozten and @fmarier are!

[fmarier]

r+ from me