Why does lock(this) thread.sleep not work with ASP.NET threading?

Question

I have a password page and when someone enters an incorrect password I want to simply foil a brute force attack by having

bool isGoodPassword = (password == expected_password);

lock (this)
{
     if (!isGoodPassword)
           Thread.Sleep(2000);
}

I would expect that this would allow all correct passwords without stalling, but if one user enters a bad password another successful password from a different user would also be blocked. However, the lock doesn't seem to lock across ASP.NET threads. Doesn't make sense.

Answer 1

Well, you haven't shown what "this" is, but if you're in the context of a page... each request is going to get its own instance of the page, isn't it? Otherwise how would they have different passwords in the first place? You'll have several threads, each locking against a separate object.

In many ways this is a good thing: you don't want genuine users to be affeted by an attacker. On the other hand, it means that the attacker just needs to make several attempts in parallel in order to effectively ignore your attempt to foil him. As other answers have stated, you could get round this by using a single object - but please don't. Don't forget that new threads won't created ad infinitum by IIS: this approach would allow a single attacker to make your entire app unusable for all users, not just for authentication but throughout the app... and they wouldn't even need to have a valid password.

Instead, you may wish to consider recording IP addresses which have failed authentication, and throttle the number of requests you are willing to process that way. (Admittedly that may have issues for some users if they're behind the same proxy as an attacker, but it's less likely.) This won't stop a distributed attack, but it's a good start.

Answer 2

If you really, really want to block ALL users from accessing the page if one of the messes up his password, you could always do

bool isGoodPassword = (password == expected_password);

lock (this.GetType())
{
     if (!isGoodPassword)
          Thread.Sleep(2000);
}

as you have written it, this would just slow down the refresh of the current request, it will not foil a multiple connections attack.

Also, comparing the passwords implies that you know the users password, ant that is always a bad practice. A better approach is to keep a (salted) hash of the user's pass, and compare that to a hash of the input. Also, you might want to employ progressive delays (1st mistake - 1 sec wait time, 2nd mistake - 2s, 3rd - 4, and so on)