Skip to content

Commit

Permalink
After failed login wait a second to slow down brute-force attacks (#1…
Browse files Browse the repository at this point in the history 
…490549)
  • Loading branch information
@alecpl
alecpl committed Oct 17, 2015
1 parent fddfd8e commit c1bbf0d
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions program/include/rcmail.php
View file
Expand Up @@ -596,6 +596,8 @@ function login($username, $pass, $host = null, $cookiecheck = false)

// try to log in
if (!$storage->connect($host, $username, $pass, $port, $ssl)) {
// Wait a second to slow down brute-force attacks (#1490549)
sleep(1);
return false;
}

Expand Down

2 comments on commit c1bbf0d

@mogest
Copy link

@mogest mogest commented on c1bbf0d Oct 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really not a good idea! Your web server only has so many workers, and if you sleep, that web worker is tied up. It'd be trivial to DoS the site; make 20-30 requests a second with invalid authentication details and no-one can access your site.

What's more, this doesn't really stop parallel attacks. A malicious user can just hit your server as many times as they like (well, until web workers are exhausted!)

A more secure way to do this is to log that a login request was made, and give an error message if that IP address/username tries to make another request within a second. Yeah, it's a lot more code, but it's the "right" way of doing it.

@thomascube
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some IMAP servers already throttle the response on failed logins. Even without this sleep(), this ultimately has the same effect. You're certainly right about web server workers but that may not be true for nxginx + php-fpm environments. Also the goal here is to avoid brute-force attacks and to protect the backend (IMAP) server. I'd rather let a few webserver nodes go down than forwarding all the DoS requests to the IMAP server.

Please note that this is meant to be an intermediate solution working without a database schema update. The final goal is to remember failed login attempts and lock the account for a while, without adding any delays as described here: http://trac.roundcube.net/ticket/1490566

We could probably improve the current solution by measuring the IMAP response time and only do sleep() (or usleep) if necessary.

Please sign in to comment.