Greater Customisation

[Magnitus-]

First of all, thanks for developing and maintaining this library. It is great.

Currently, there are 2 limits to the library for my use-case:

1) Fixation on IP

I would be nice if the function 'getIPFromRequest' was renamed 'getKeyFromRequest', defaulted to 'getIPFromRequest', but could be customisable via a constructor option.

For example, in a route that requires a user to be logged in, a requester's user ID, rather than IP might be a more reliable metric to identify him.

2) Separation Between Incrementer and Verifier

Would be great if, optionally, the routes to increment the counter and verify the counter for a brute force attack could be separated.

This would allow you to do something like this:

• Check for brute force on login attempt
• If login failed, then increment access count

[Magnitus-]

Btw, if this is something you'd consider desirable for the project, I could implement the customisable key feature pretty quickly and do a PR for it if needed.

[AdamPflug]

If you look at https://github.com/AdamPflug/express-brute#a-more-complex-example there is a way to specify a key in addition to IP - however it does keep a separate count for each IP that user is signed in from. My thought process here was I would use it to prevent many login requests for the same user account, without enabling a DOS attack where someone could try to log in multiple times for the same account with a bad password to lock someone else out of the system.

If that doesn't go far enough maybe it does make sense to include an option for getMiddleware that allows you to omit the user's IP from the mix. Thoughts?

Could you explain a little more about the use case for splitting incrementing and preventing? It's an interesting concept. Right now I'm relying just on .reset() to set the counter back to 0 after a successful login. I couldn't think of a reason you would want it either not to be reset or to be reset to a non-0 number, but that doesn't mean there isn't one

• Adam

[Magnitus-]

Thanks for a very enlightening reply. Yes, the reset method would do nicely for problem number 2.

For problem number 1, getMiddleware(...) is fine after some consideration.

I was worried about IP spoofing, but now that I think more about it, the lower level TCP/IP handshake beneat HTTP would counter that.

I was also worried about people sharing IPs (ie, same office) being blocked unnecessarily in certain situations where you could just target the user account, but the custom key in getMiddleware takes care of that problem.

EDIT: Some additional thought about the spoofing bit... What if someone is physically close to the server? Then couldn't they potentially intercept any packets and spoof whichever IP they want, thus alternating IPs and bypassing the brute force protection which in certain (admittedly not all) situations could actually be prevented by only using higher level user-account matching?

For example, assume paying users can perform an expensive operation, but using the normal flow of the application, do so 5 minutes on average. A nefarious user that can intercept the packets (and thus spoof IP) could overwhelm the server by making the operation 1000 times a second (using the same account and alternating IPs). If he was only matched by his user-account, then he could be stopped dead in his tracks after, say, 5 requests in a short timespan and would have to pay again (with a new account) to make 5 more requests, which would get expensive quickly.

[AdamPflug]

I think you're right - if an attacker were to infect a router/switch that was reliably between your server and any spoofed source IPs then theoretically they could bypass the IP-based protections. That's a pretty high bar for an attacker to meet, but I suppose a state actor could do it. In that instance ignoring IP would technically have a better chance of stopping the attack - although you also probably want special business logic to handle that case because it indicates a larger problem. A malicious man in the middle for all your traffic is a major problem for lots of other reasons :)

If you want to implement it though, I would love a PR to add the option to ignore IPs to getMiddleware when a custom key function is provided.

[Magnitus-]

Admittedly, I'm not sure how difficult performing a man-in-the-middle attack is in terms of logistics as I've never been remotely inclined to do something like this myself. We do seem to hear a lot about it in literature, but you are most likely right that getting attacked by someone this motivated would only be a worry for someone who has something extremely valuable to protect (beyond what small companies tend to have).

Still, ignoring the IP is a costless measure to take, so why not?

It would be my pleasure to do a PR for it in the upcoming days.

[AdamPflug]

Most of the man in the middle attacks you hear about happen very close to the end user (for example a malicious Wifi access point, ARP poisoning on open wifi, etc). It gets significantly harder to do the closer you get to the server because at that point one of the major ISPs (or your hosting provider) needs to be complicit in the attack (although not necessarily the attacker - for example if the NSA issued them a national security letter compelling them to do so). Unless they were directed to do so by the government you would probably have a legal case against them so it's not something they're likely to do.

Anyway, I'm going to close the issue for now - if you do end up creating a PR for this just reference this ticket in it so the conversation doesn't get lost.

[Magnitus-]

My apologies for the delay. I was working on something else and it is just taking longer than expected because of a little specification detail I originally missed.

I'm at a good point to take a break so I might as well take care of this before resuming what I was doing.

[Magnitus-]

It's done. I took the liberty of adding a mock callback in one of the tests to prevent it from crashing.