Merge #1278

1278: Limiter implementation r=kaiyou a=micw

## What type of PR?

(Feature, enhancement, bug-fix, documentation)

## What does this PR do?

Adds a custom limter based on the "limits" lirary that counts up on failed auths only

### Related issue(s)
- closes #1195
- closes #634

## Prerequistes

- [X] Unless it's docs or a minor change: add [changelog](https://mailu.io/master/contributors/guide.html#changelog) entry file.


Co-authored-by: Michael Wyraz <michael@wyraz.de>
Co-authored-by: micw <michael@wyraz.de>

core/admin/mailu/configuration.py

@@ -32,6 +32,7 @@ DEFAULT_CONFIG = {
     'POSTMASTER': 'postmaster',
     'TLS_FLAVOR': 'cert',
     'AUTH_RATELIMIT': '10/minute;1000/hour',
+    'AUTH_RATELIMIT_SUBNET': True,
     'DISABLE_STATISTICS': False,
     # Mail settings
     'DMARC_RUA': None,

core/admin/mailu/internal/__init__.py

@@ -1,4 +1,4 @@
-from flask_limiter import RateLimitExceeded
+from mailu.limiter import RateLimitExceeded
 
 from mailu import utils
 from flask import current_app as app
@@ -20,13 +20,4 @@ def rate_limit_handler(e):
     return response
 
 
-@utils.limiter.request_filter
-def whitelist_webmail():
-    try:
-        return flask.request.headers["Client-Ip"] ==\
-            app.config["HOST_WEBMAIL"]
-    except:
-        return False
-
-
 from mailu.internal.views import *

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

@@ -7,18 +7,19 @@ import flask_login
 import base64
 
 
+
 @internal.route("/auth/email")
-@utils.limiter.limit(
-    lambda: app.config["AUTH_RATELIMIT"],
-    lambda: flask.request.headers["Client-Ip"]
-)
 def nginx_authentication():
     """ Main authentication endpoint for Nginx email server
     """
+    utils.limiter.check(flask.request.headers["Client-Ip"])
     headers = nginx.handle_authentication(flask.request.headers)
     response = flask.Response()
     for key, value in headers.items():
         response.headers[key] = str(value)
+    if ("Auth-Status" not in headers) or (headers["Auth-Status"]!="OK"):
+        utils.limiter.hit(flask.request.headers["Client-Ip"])
+
     return response
 
 

core/admin/mailu/limiter.py

@@ -0,0 +1,36 @@
+import limits
+import limits.storage
+import limits.strategies
+import ipaddress
+
+class RateLimitExceeded(Exception):
+    pass
+
+class Limiter:
+
+    def __init__(self):
+        self.storage = None
+        self.limiter = None
+        self.rate = None
+        self.subnet = None
+        self.rate_limit_subnet = True
+
+    def init_app(self, app):
+        self.storage = limits.storage.storage_from_string(app.config["RATELIMIT_STORAGE_URL"])
+        self.limiter = limits.strategies.MovingWindowRateLimiter(self.storage)
+        self.rate = limits.parse(app.config["AUTH_RATELIMIT"])
+        self.rate_limit_subnet = str(app.config["AUTH_RATELIMIT_SUBNET"])!='False'
+        self.subnet = ipaddress.ip_network(app.config["SUBNET"])
+
+    def check(self,clientip):
+        # disable limits for internal requests (e.g. from webmail)?
+        if self.rate_limit_subnet==False and ipaddress.ip_address(clientip) in self.subnet:
+            return
+        if not self.limiter.test(self.rate,"client-ip",clientip):
+            raise RateLimitExceeded()
+
+    def hit(self,clientip):
+        # disable limits for internal requests (e.g. from webmail)?
+        if self.rate_limit_subnet==False and ipaddress.ip_address(clientip) in self.subnet:
+            return
+        self.limiter.hit(self.rate,"client-ip",clientip)

core/admin/mailu/utils.py

@@ -1,11 +1,10 @@
-from mailu import models
+from mailu import models, limiter
 
 import flask
 import flask_login
 import flask_script
 import flask_migrate
 import flask_babel
-import flask_limiter
 
 from werkzeug.contrib import fixers
 
@@ -20,10 +19,8 @@ def handle_needs_login():
         flask.url_for('ui.login', next=flask.request.endpoint)
     )
 
-
+# Rate limiter
-# Request rate limitation
+limiter = limiter.Limiter()
-limiter = flask_limiter.Limiter(key_func=lambda: current_user.username)
-
 
 # Application translation
 babel = flask_babel.Babel()

core/admin/requirements.txt

@@ -7,7 +7,7 @@ Flask-migrate
 Flask-script
 Flask-wtf
 Flask-debugtoolbar
-Flask-limiter
+limits
 redis
 WTForms-Components
 socrate

docs/configuration.rst

@@ -38,8 +38,14 @@ recommended to setup a generic value and later configure a mail alias for that
 address.
 
 The ``AUTH_RATELIMIT`` holds a security setting for fighting attackers that
-try to guess user passwords. The value is the limit of requests that a single
+try to guess user passwords. The value is the limit of failed authentication attempts
-IP address can perform against IMAP, POP and SMTP authentication endpoints.
+that a single IP address can perform against IMAP, POP and SMTP authentication endpoints.
+
+If ``AUTH_RATELIMIT_SUBNET`` is ``True`` (which is the default), the ``AUTH_RATELIMIT``
+rules does also apply to auth requests coming from ``SUBNET``, especially for the webmail.
+If you disable this, ensure that the rate limit on the webmail is enforced in a different
+way (e.g. roundcube plug-in), otherwise an attacker can simply bypass the limit using webmail.
+
 
 The ``TLS_FLAVOR`` sets how Mailu handles TLS connections. Setting this value to
 ``notls`` will cause Mailu not to server any web content! More on :ref:`tls_flavor`.

towncrier/newsfragments/1278.fix

@@ -0,0 +1,2 @@
+
+Ratelimit counts up on failed auth only now