Merge #2732

2732: Only account for distinct attempts in rate limits r=mergify[bot] a=nextgens

## What type of PR?

enhancement

## What does this PR do?

Only account for distinct attempts in rate limits. This is solving the problem related to users changing their passwords and having their client hammer the old credentials.

Reduce the default to 50 distinct passwords per day

### Related issue(s)

## Prerequisites
Before we can consider review and merge, please make sure the following list is done and checked.
If an entry in not applicable, you can check it or remove it from the list.

- [x] In case of feature or enhancement: documentation updated accordingly
- [x] Unless it's docs or a minor change: add [changelog](https://mailu.io/master/contributors/workflow.html#changelog) entry file.


Co-authored-by: Florent Daigniere <nextgens@freenetproject.org>
Co-authored-by: Florent Daigniere <nextgens@users.noreply.github.com>

core/admin/mailu/configuration.py

@@ -44,7 +44,7 @@ DEFAULT_CONFIG = {
     'AUTH_RATELIMIT_IP': '5/hour',
     'AUTH_RATELIMIT_IP_V4_MASK': 24,
     'AUTH_RATELIMIT_IP_V6_MASK': 48,
-    'AUTH_RATELIMIT_USER': '100/day',
+    'AUTH_RATELIMIT_USER': '50/day',
     'AUTH_RATELIMIT_EXEMPTION': '',
     'AUTH_RATELIMIT_EXEMPTION_LENGTH': 86400,
     'DISABLE_STATISTICS': False,

core/admin/mailu/internal/nginx.py

@@ -85,6 +85,7 @@ def handle_authentication(headers):
         raw_user_email = urllib.parse.unquote(headers["Auth-User"])
         raw_password = urllib.parse.unquote(headers["Auth-Pass"])
         user_email = 'invalid'
+        password = 'invalid'
         try:
             user_email = raw_user_email.encode("iso8859-1").decode("utf8")
             password = raw_password.encode("iso8859-1").decode("utf8")
@@ -107,6 +108,7 @@ def handle_authentication(headers):
                         "Auth-Server": server,
                         "Auth-User": user_email,
                         "Auth-User-Exists": is_valid_user,
+                        "Auth-Password": password,
                         "Auth-Port": port
                     }
         status, code = get_status(protocol, "authentication")
@@ -115,6 +117,7 @@ def handle_authentication(headers):
             "Auth-Error-Code": code,
             "Auth-User": user_email,
             "Auth-User-Exists": is_valid_user,
+            "Auth-Password": password,
             "Auth-Wait": 0
         }
     # Unexpected

core/admin/mailu/internal/views/auth.py

@@ -48,7 +48,7 @@ def nginx_authentication():
     if headers.get("Auth-Status") == "OK":
         utils.limiter.exempt_ip_from_ratelimits(client_ip)
     elif is_valid_user:
-        utils.limiter.rate_limit_user(username, client_ip)
+        utils.limiter.rate_limit_user(username, client_ip, password=response.headers.get('Auth-Password', None))
     elif not is_from_webmail:
         utils.limiter.rate_limit_ip(client_ip, username)
     return response

core/admin/mailu/limiter.py

@@ -68,9 +68,13 @@ class LimitWraperFactory(object):
             app.logger.warn(f'Authentication attempt from {ip} for {username} has been rate-limited.')
         return is_rate_limited
 
-    def rate_limit_user(self, username, ip, device_cookie=None, device_cookie_name=None):
+    def rate_limit_user(self, username, ip, device_cookie=None, device_cookie_name=None, password=''):
         limiter = self.get_limiter(app.config["AUTH_RATELIMIT_USER"], 'auth-user')
         if self.is_subject_to_rate_limits(ip):
+            truncated_password = hmac.new(bytearray(username, 'utf-8'), bytearray(password, 'utf-8'), 'sha256').hexdigest()[-6:]
+            if password and (self.storage.get(f'dedup2-{username}-{truncated_password}') > 0):
+                return
+            self.storage.incr(f'dedup2-{username}-{truncated_password}', limits.parse(app.config['AUTH_RATELIMIT_USER']).GRANULARITY.seconds, True)
             limiter.hit(device_cookie if device_cookie_name == username else username)
 
     """ Device cookies as described on:

core/admin/mailu/sso/views/base.py

@@ -59,7 +59,7 @@ def login():
                 flask.flash(msg, "error")
             return response
         else:
-            utils.limiter.rate_limit_user(username, client_ip, device_cookie, device_cookie_username) if models.User.get(username) else utils.limiter.rate_limit_ip(client_ip, username)
+            utils.limiter.rate_limit_user(username, client_ip, device_cookie, device_cookie_username, form.pw.data) if models.User.get(username) else utils.limiter.rate_limit_ip(client_ip, username)
             flask.current_app.logger.warn(f'Login failed for {username} from {client_ip}.')
             flask.flash('Wrong e-mail or password', 'error')
     return flask.render_template('login.html', form=form, fields=fields)

docs/configuration.rst

@@ -47,10 +47,11 @@ accounts for a specific IP subnet as defined in
 ``AUTH_RATELIMIT_IP_V4_MASK`` (default: /24) and
 ``AUTH_RATELIMIT_IP_V6_MASK`` (default: /48).
 
-The ``AUTH_RATELIMIT_USER`` (default: 100/day) holds a security setting for fighting
+The ``AUTH_RATELIMIT_USER`` (default: 50/day) holds a security setting for fighting
 attackers that attempt to guess a user's password (typically using a password
-bruteforce attack). The value defines the limit of authentication attempts allowed
-for any given account within a specific timeframe.
+bruteforce attack). The value defines the limit of distinct authentication attempts
+allowed for any given account within a specific timeframe. Multiple attempts for the
+same account with the same password only counts for one.
 
 The ``AUTH_RATELIMIT_EXEMPTION_LENGTH`` (default: 86400) is the number of seconds
 after a successful login for which a specific IP address is exempted from rate limits.

setup/templates/steps/config.html

@@ -47,7 +47,7 @@ Or in plain english: if receivers start to classify your mail as spam, this post
   <label>Authentication rate limit per user</label>
   <!--   Validates number input only -->
   <p><input class="form-control" style="width: 9%; display: inline;" type="number" name="auth_ratelimit_user"
-  		value="100" required > / day
+  		value="50" required > / day
   </p>
 </div>
 

towncrier/newsfragments/2726.misc

@@ -0,0 +1 @@
+Change the behaviour of AUTH_RATELIMIT_USER and only account for distinct attempts. Same username and same password is now a only accounted once per period.