Merge #2646

2646: Smarter ratelimit r=mergify[bot] a=nextgens

## What type of PR?

enhancement

## What does this PR do?

Only account for **distinct** usernames in the IP rate-limiter.

This enables to have a much tighter default as a user with a misconfigured device will now only account for a single attempt.

The goal here is to make the rate-limiter more acceptable and to avoid people disabling it altogether.

### 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.

- [ ] 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: Alexander Graf <ghostwheel42@users.noreply.github.com>

core/admin/mailu/configuration.py

@@ -40,9 +40,9 @@ DEFAULT_CONFIG = {
     'TLS_FLAVOR': 'cert',
     'INBOUND_TLS_ENFORCE': False,
     'DEFER_ON_TLS_ERROR': True,
-    'AUTH_RATELIMIT_IP': '60/hour',
+    'AUTH_RATELIMIT_IP': '5/hour',
     'AUTH_RATELIMIT_IP_V4_MASK': 24,
-    'AUTH_RATELIMIT_IP_V6_MASK': 56,
+    'AUTH_RATELIMIT_IP_V6_MASK': 48,
     'AUTH_RATELIMIT_USER': '100/day',
     'AUTH_RATELIMIT_EXEMPTION': '',
     'AUTH_RATELIMIT_EXEMPTION_LENGTH': 86400,

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

@@ -33,8 +33,8 @@ def nginx_authentication():
     for key, value in headers.items():
         response.headers[key] = str(value)
     is_valid_user = False
+    username = response.headers['Auth-User']
     if response.headers.get("Auth-User-Exists") == "True":
-        username = response.headers["Auth-User"]
         if utils.limiter.should_rate_limit_user(username, client_ip):
             # FIXME could be done before handle_authentication()
             status, code = nginx.get_status(flask.request.headers['Auth-Protocol'], 'ratelimit')
@@ -50,7 +50,7 @@ def nginx_authentication():
     elif is_valid_user:
         utils.limiter.rate_limit_user(username, client_ip)
     elif not is_from_webmail:
-        utils.limiter.rate_limit_ip(client_ip)
+        utils.limiter.rate_limit_ip(client_ip, username)
     return response
 
 @internal.route("/auth/admin")
@@ -109,7 +109,7 @@ def basic_authentication():
                 utils.limiter.exempt_ip_from_ratelimits(client_ip)
                 return response
             # We failed check_credentials
-            utils.limiter.rate_limit_user(user_email, client_ip) if user else utils.limiter.rate_limit_ip(client_ip)
+            utils.limiter.rate_limit_user(user_email, client_ip) if user else utils.limiter.rate_limit_ip(client_ip, user_email)
     response = flask.Response(status=401)
     response.headers["WWW-Authenticate"] = 'Basic realm="Login Required"'
     return response

core/admin/mailu/limiter.py

@@ -52,10 +52,13 @@ class LimitWraperFactory(object):
             app.logger.warn(f'Authentication attempt from {ip} has been rate-limited.')
         return is_rate_limited
 
-    def rate_limit_ip(self, ip):
-        limiter = self.get_limiter(app.config["AUTH_RATELIMIT_IP"], 'auth-ip')
+    def rate_limit_ip(self, ip, username=None):
+        limiter = self.get_limiter(app.config['AUTH_RATELIMIT_IP'], 'auth-ip')
         client_network = utils.extract_network_from_ip(ip)
         if self.is_subject_to_rate_limits(ip):
+            if username and self.storage.get(f'dedup-{client_network}-{username}') > 0:
+                return
+            self.storage.incr(f'dedup-{client_network}-{username}', limits.parse(app.config['AUTH_RATELIMIT_IP']).GRANULARITY.seconds, True)
             limiter.hit(client_network)
 
     def should_rate_limit_user(self, username, ip, device_cookie=None, device_cookie_name=None):

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

@@ -47,7 +47,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)
+            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)
             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

@@ -40,11 +40,12 @@ address.
 The ``WILDCARD_SENDERS`` setting is a comma delimited list of user email addresses
 that are allowed to send emails from any existing address (spoofing the sender).
 
-The ``AUTH_RATELIMIT_IP`` (default: 60/hour) holds a security setting for fighting
-attackers that waste server resources by trying to guess user passwords (typically
-using a password spraying attack). The value defines the limit of authentication
-attempts that will be processed on non-existing accounts for a specific IP subnet
-(as defined in ``AUTH_RATELIMIT_IP_V4_MASK`` and ``AUTH_RATELIMIT_IP_V6_MASK`` below).
+The ``AUTH_RATELIMIT_IP`` (default: 5/hour) holds a security setting for fighting
+attackers that attempt a password spraying attack. The value defines the limit of
+authentication attempts that will be processed on **distinct** non-existing
+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
 attackers that attempt to guess a user's password (typically using a password

setup/flavors/compose/mailu.env

@@ -29,7 +29,7 @@ POSTMASTER={{ postmaster }}
 # Choose how secure connections will behave (value: letsencrypt, cert, notls, mail, mail-letsencrypt)
 TLS_FLAVOR={{ tls_flavor }}
 
-# Authentication rate limit per IP (per /24 on ipv4 and /56 on ipv6)
+# Authentication rate limit per IP (per /24 on ipv4 and /48 on ipv6)
 {% if auth_ratelimit_ip > '0' %}
 AUTH_RATELIMIT_IP={{ auth_ratelimit_ip }}/hour
 {% endif %}

setup/templates/steps/config.html

@@ -37,10 +37,9 @@ Or in plain english: if receivers start to classify your mail as spam, this post
 </div>
 
 <div class="form-group">
-  <label>Authentication rate limit per IP for failed login attempts or non-existing accounts</label>
+  <label>Authentication rate limit per IP for failed login attempts on unique non-existing accounts (password spraying counter-measure)</label>
   <!--   Validates number input only -->
-  <p><input class="form-control" style="width: 9%; display: inline;" type="number" name="auth_ratelimit_ip"
-  		value="60" required > / hour
+  <p><input class="form-control" style="width: 9%; display: inline;" type="number" name="auth_ratelimit_ip" value="5" required > / hour
   </p>
 </div>
 

towncrier/newsfragments/2644.misc

@@ -0,0 +1 @@
+Implement de-dupplication on rate limits. Now only attempts for distinct usernames will count as a hit.