From 8e88f1b8c36d8cd194a50eea444c12ecb45b64cb Mon Sep 17 00:00:00 2001 From: kaiyou Date: Sun, 9 Feb 2020 17:38:18 +0100 Subject: [PATCH] Refactor the rate limiting code Rate limiting was already redesigned to use Python limits. This introduced some unexpected behavior, including the fact that only one criteria is supported per limiter. Docs and setup utility are updated with this in mind. Also, the code was made more generic, so limiters can be delivered for something else than authentication. Authentication-specific code was moved directly to the authentication routine. --- core/admin/mailu/internal/__init__.py | 16 --------- core/admin/mailu/internal/views/auth.py | 20 ++++++++--- core/admin/mailu/limiter.py | 48 ++++++++++++------------- core/admin/mailu/utils.py | 2 +- docs/compose/.env | 6 +++- docs/configuration.rst | 5 ++- setup/flavors/compose/mailu.env | 4 +-- setup/templates/steps/config.html | 7 ++-- 8 files changed, 53 insertions(+), 55 deletions(-) diff --git a/core/admin/mailu/internal/__init__.py b/core/admin/mailu/internal/__init__.py index 95f2f782..560f4d97 100644 --- a/core/admin/mailu/internal/__init__.py +++ b/core/admin/mailu/internal/__init__.py @@ -1,23 +1,7 @@ -from mailu.limiter import RateLimitExceeded - -from mailu import utils -from flask import current_app as app - -import socket import flask internal = flask.Blueprint('internal', __name__, template_folder='templates') -@internal.app_errorhandler(RateLimitExceeded) -def rate_limit_handler(e): - response = flask.Response() - response.headers['Auth-Status'] = 'Authentication rate limit from one source exceeded' - response.headers['Auth-Error-Code'] = '451 4.3.2' - if int(flask.request.headers['Auth-Login-Attempt']) < 10: - response.headers['Auth-Wait'] = '3' - return response - - from mailu.internal.views import * diff --git a/core/admin/mailu/internal/views/auth.py b/core/admin/mailu/internal/views/auth.py index b1f37d17..825dba56 100644 --- a/core/admin/mailu/internal/views/auth.py +++ b/core/admin/mailu/internal/views/auth.py @@ -5,21 +5,31 @@ from flask import current_app as app import flask import flask_login import base64 - +import ipaddress @internal.route("/auth/email") def nginx_authentication(): """ Main authentication endpoint for Nginx email server """ - utils.limiter.check(flask.request.headers["Client-Ip"]) + limiter = utils.limiter.get_limiter(app.config["AUTH_RATELIMIT"], "auth-ip") + client_ip = flask.request.headers["Client-Ip"] + if not limiter.test(client_ip): + response = flask.Response() + response.headers['Auth-Status'] = 'Authentication rate limit from one source exceeded' + response.headers['Auth-Error-Code'] = '451 4.3.2' + if int(flask.request.headers['Auth-Login-Attempt']) < 10: + response.headers['Auth-Wait'] = '3' + return response 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"]) - + if ("Auth-Status" not in headers) or (headers["Auth-Status"] != "OK"): + limit_subnet = str(app.config["AUTH_RATELIMIT_SUBNET"]) != 'False' + subnet = ipaddress.ip_network(app.config["SUBNET"]) + if limit_subnet or ipaddress.ip_address(client_ip) not in subnet: + limiter.hit(flask.request.headers["Client-Ip"]) return response diff --git a/core/admin/mailu/limiter.py b/core/admin/mailu/limiter.py index f7819c31..b5f99915 100644 --- a/core/admin/mailu/limiter.py +++ b/core/admin/mailu/limiter.py @@ -1,36 +1,34 @@ import limits import limits.storage import limits.strategies -import ipaddress -class RateLimitExceeded(Exception): - pass -class Limiter: +class LimitWrapper(object): + """ Wraps a limit by providing the storage, item and identifiers + """ - def __init__(self): - self.storage = None - self.limiter = None - self.rate = None - self.subnet = None - self.rate_limit_subnet = True + def __init__(self, limiter, limit, *identifiers): + self.limiter = limiter + self.limit = limit + self.base_identifiers = identifiers + + def test(self, *args): + return self.limiter.test(self.limit, *(self.base_identifiers + args)) + + def hit(self, *args): + return self.limiter.hit(self.limit, *(self.base_identifiers + args)) + + def get_window_stats(self, *args): + return self.limiter.get_window_stats(self.limit, *(self.base_identifiers + args)) + + +class LimitWraperFactory(object): + """ Global limiter, to be used as a factory + """ 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) + def get_limiter(self, limit, *args): + return LimitWrapper(self.limiter, limits.parse(limit), *args) \ No newline at end of file diff --git a/core/admin/mailu/utils.py b/core/admin/mailu/utils.py index df23b8e7..39181678 100644 --- a/core/admin/mailu/utils.py +++ b/core/admin/mailu/utils.py @@ -20,7 +20,7 @@ def handle_needs_login(): ) # Rate limiter -limiter = limiter.Limiter() +limiter = limiter.LimitWraperFactory() # Application translation babel = flask_babel.Babel() diff --git a/docs/compose/.env b/docs/compose/.env index 218b94d2..69c91d82 100644 --- a/docs/compose/.env +++ b/docs/compose/.env @@ -38,7 +38,7 @@ POSTMASTER=admin TLS_FLAVOR=cert # Authentication rate limit (per source IP address) -AUTH_RATELIMIT=10/minute;1000/hour +AUTH_RATELIMIT=10/minute # Opt-out of statistics, replace with "True" to opt out DISABLE_STATISTICS=False @@ -68,6 +68,10 @@ ANTIVIRUS=none # Max attachment size will be 33% smaller MESSAGE_SIZE_LIMIT=50000000 +# Message rate limit for outgoing messages +# This limit is per user +MESSAGE_RATELIMIT=100/day + # Networks granted relay permissions # Use this with care, all hosts in this networks will be able to send mail without authentication! RELAYNETS= diff --git a/docs/configuration.rst b/docs/configuration.rst index cda6becb..2b7227ba 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -46,7 +46,6 @@ rules does also apply to auth requests coming from ``SUBNET``, especially for th 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`. @@ -57,6 +56,10 @@ The ``MESSAGE_SIZE_LIMIT`` is the maximum size of a single email. It should not be too low to avoid dropping legitimate emails and should not be too high to avoid filling the disks with large junk emails. +The ``MESSAGE_RATELIMIT`` is the limit of messages a single user can send. This is +meant to fight outbound spam in case of compromised or malicious account on the +server. + The ``RELAYNETS`` are network addresses for which mail is relayed for free with no authentication required. This should be used with great care. If you want other Docker services' outbound mail to be relayed, you can set this to ``172.16.0.0/12`` diff --git a/setup/flavors/compose/mailu.env b/setup/flavors/compose/mailu.env index 180239c3..78ecce72 100644 --- a/setup/flavors/compose/mailu.env +++ b/setup/flavors/compose/mailu.env @@ -30,8 +30,8 @@ POSTMASTER={{ postmaster }} TLS_FLAVOR={{ tls_flavor }} # Authentication rate limit (per source IP address) -{% if auth_ratelimit_pm > '0' and auth_ratelimit_ph > '0' %} -AUTH_RATELIMIT={{ auth_ratelimit_pm }}/minute;{{ auth_ratelimit_ph }}/hour +{% if auth_ratelimit_pm > '0' %} +AUTH_RATELIMIT={{ auth_ratelimit_pm }}/minute {% endif %} # Opt-out of statistics, replace with "True" to opt out diff --git a/setup/templates/steps/config.html b/setup/templates/steps/config.html index 330e008f..b3e57e22 100644 --- a/setup/templates/steps/config.html +++ b/setup/templates/steps/config.html @@ -47,11 +47,10 @@ Or in plain english: if receivers start to classify your mail as spam, this post
- +

/minute; - /hour

+ value="10" required > / minute +