diff --git a/core/admin/mailu/__init__.py b/core/admin/mailu/__init__.py index 9b712512..43e58171 100644 --- a/core/admin/mailu/__init__.py +++ b/core/admin/mailu/__init__.py @@ -28,6 +28,7 @@ def create_app_from_config(config): utils.proxy.init_app(app) utils.migrate.init_app(app, models.db) + app.device_cookie_key = hmac.new(bytearray(app.secret_key, 'utf-8'), bytearray('DEVICE_COOKIE_KEY', 'utf-8'), 'sha256').digest() app.temp_token_key = hmac.new(bytearray(app.secret_key, 'utf-8'), bytearray('WEBMAIL_TEMP_TOKEN_KEY', 'utf-8'), 'sha256').digest() # Initialize list of translations diff --git a/core/admin/mailu/configuration.py b/core/admin/mailu/configuration.py index 4401888a..b0a1bf7b 100644 --- a/core/admin/mailu/configuration.py +++ b/core/admin/mailu/configuration.py @@ -36,8 +36,11 @@ DEFAULT_CONFIG = { 'TLS_FLAVOR': 'cert', 'INBOUND_TLS_ENFORCE': False, 'DEFER_ON_TLS_ERROR': True, - 'AUTH_RATELIMIT': '1000/minute;10000/hour', - 'AUTH_RATELIMIT_SUBNET': False, + 'AUTH_RATELIMIT_IP': '10/hour', + 'AUTH_RATELIMIT_IP_V4_MASK': 24, + 'AUTH_RATELIMIT_IP_V6_MASK': 56, + 'AUTH_RATELIMIT_USER': '100/day', + 'AUTH_RATELIMIT_EXEMPTION_LENGTH': 86400, 'DISABLE_STATISTICS': False, # Mail settings 'DMARC_RUA': None, diff --git a/core/admin/mailu/internal/nginx.py b/core/admin/mailu/internal/nginx.py index 167341e2..78f83e99 100644 --- a/core/admin/mailu/internal/nginx.py +++ b/core/admin/mailu/internal/nginx.py @@ -19,6 +19,11 @@ STATUSES = { "encryption": ("Must issue a STARTTLS command first", { "smtp": "530 5.7.0" }), + "ratelimit": ("Temporary authentication failure (rate-limit)", { + "imap": "LIMIT", + "smtp": "451 4.3.2", + "pop3": "-ERR [LOGIN-DELAY] Retry later" + }), } def check_credentials(user, password, ip, protocol=None): @@ -71,8 +76,9 @@ def handle_authentication(headers): } # Authenticated user elif method == "plain": + is_valid_user = False service_port = int(urllib.parse.unquote(headers["Auth-Port"])) - if service_port == 25: + if 'Auth-Port' in headers and service_port == 25: return { "Auth-Status": "AUTH not supported", "Auth-Error-Code": "502 5.5.1", @@ -91,18 +97,23 @@ def handle_authentication(headers): app.logger.warn(f'Received undecodable user/password from nginx: {raw_user_email!r}/{raw_password!r}') else: user = models.User.query.get(user_email) + is_valid_user = True ip = urllib.parse.unquote(headers["Client-Ip"]) if check_credentials(user, password, ip, protocol): server, port = get_server(headers["Auth-Protocol"], True) return { "Auth-Status": "OK", "Auth-Server": server, + "Auth-User": user_email, + "Auth-User-Exists": is_valid_user, "Auth-Port": port } status, code = get_status(protocol, "authentication") return { "Auth-Status": status, "Auth-Error-Code": code, + "Auth-User": user_email, + "Auth-User-Exists": is_valid_user, "Auth-Wait": 0 } # Unexpected diff --git a/core/admin/mailu/internal/views/auth.py b/core/admin/mailu/internal/views/auth.py index 1686e1cb..457efd4b 100644 --- a/core/admin/mailu/internal/views/auth.py +++ b/core/admin/mailu/internal/views/auth.py @@ -5,19 +5,17 @@ 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 """ - limiter = utils.limiter.get_limiter(app.config["AUTH_RATELIMIT"], "auth-ip") client_ip = flask.request.headers["Client-Ip"] - if not limiter.test(client_ip): + if utils.limiter.should_rate_limit_ip(client_ip): + status, code = nginx.get_status(flask.request.headers['Auth-Protocol'], 'ratelimit') response = flask.Response() - response.headers['Auth-Status'] = 'Authentication rate limit from one source exceeded' - response.headers['Auth-Error-Code'] = '451 4.3.2' + response.headers['Auth-Status'] = status + response.headers['Auth-Error-Code'] = code if int(flask.request.headers['Auth-Login-Attempt']) < 10: response.headers['Auth-Wait'] = '3' return response @@ -25,14 +23,25 @@ def nginx_authentication(): response = flask.Response() for key, value in headers.items(): response.headers[key] = str(value) + is_valid_user = False + if "Auth-User-Exists" in response.headers and response.headers["Auth-User-Exists"]: + 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') + response = flask.Response() + response.headers['Auth-Status'] = status + response.headers['Auth-Error-Code'] = code + if int(flask.request.headers['Auth-Login-Attempt']) < 10: + response.headers['Auth-Wait'] = '3' + return response + is_valid_user = True 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"]) + utils.limiter.rate_limit_user(username, client_ip) if is_valid_user else rate_limit_ip(client_ip) + elif ("Auth-Status" in headers) and (headers["Auth-Status"] == "OK"): + utils.limiter.exempt_ip_from_ratelimits(client_ip) return response - @internal.route("/auth/admin") def admin_authentication(): """ Fails if the user is not an authenticated admin. @@ -60,15 +69,29 @@ def user_authentication(): def basic_authentication(): """ Tries to authenticate using the Authorization header. """ + client_ip = flask.request.headers["X-Real-IP"] if 'X-Real-IP' in flask.request.headers else flask.request.remote_addr + if utils.limiter.should_rate_limit_ip(client_ip): + response = flask.Response(status=401) + response.headers["WWW-Authenticate"] = 'Basic realm="Authentication rate limit from one source exceeded"' + response.headers['Retry-After'] = '60' + return response authorization = flask.request.headers.get("Authorization") if authorization and authorization.startswith("Basic "): encoded = authorization.replace("Basic ", "") user_email, password = base64.b64decode(encoded).split(b":", 1) - user = models.User.query.get(user_email.decode("utf8")) - if nginx.check_credentials(user, password.decode('utf-8'), flask.request.remote_addr, "web"): + user_email = user_email.decode("utf8") + if utils.limiter.should_rate_limit_user(user_email, client_ip): + response = flask.Response(status=401) + response.headers["WWW-Authenticate"] = 'Basic realm="Authentication rate limit for this username exceeded"' + response.headers['Retry-After'] = '60' + return response + user = models.User.query.get(user_email) + if user and nginx.check_credentials(user, password.decode('utf-8'), flask.request.remote_addr, "web"): response = flask.Response() response.headers["X-User"] = models.IdnaEmail.process_bind_param(flask_login, user.email, "") + utils.limiter.exempt_ip_from_ratelimits(client_ip) return response + utils.limiter.rate_limit_user(user_email, client_ip) if user else utils.limiter.rate_limit_ip(client_ip) response = flask.Response(status=401) response.headers["WWW-Authenticate"] = 'Basic realm="Login Required"' return response diff --git a/core/admin/mailu/limiter.py b/core/admin/mailu/limiter.py index b5f99915..70b09f21 100644 --- a/core/admin/mailu/limiter.py +++ b/core/admin/mailu/limiter.py @@ -1,7 +1,12 @@ +from mailu import utils +from flask import current_app as app +import base64 import limits import limits.storage import limits.strategies +import hmac +import secrets class LimitWrapper(object): """ Wraps a limit by providing the storage, item and identifiers @@ -31,4 +36,53 @@ class LimitWraperFactory(object): self.limiter = limits.strategies.MovingWindowRateLimiter(self.storage) def get_limiter(self, limit, *args): - return LimitWrapper(self.limiter, limits.parse(limit), *args) \ No newline at end of file + return LimitWrapper(self.limiter, limits.parse(limit), *args) + + def is_subject_to_rate_limits(self, ip): + return not (self.storage.get(f'exempt-{ip}') > 0) + + def exempt_ip_from_ratelimits(self, ip): + self.storage.incr(f'exempt-{ip}', app.config["AUTH_RATELIMIT_EXEMPTION_LENGTH"], True) + + def should_rate_limit_ip(self, ip): + limiter = self.get_limiter(app.config["AUTH_RATELIMIT_IP"], 'auth-ip') + client_network = utils.extract_network_from_ip(ip) + return self.is_subject_to_rate_limits(ip) and not limiter.test(client_network) + + def rate_limit_ip(self, ip): + if ip != app.config['WEBMAIL_ADDRESS']: + 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): + limiter.hit(client_network) + + def should_rate_limit_user(self, username, ip, device_cookie=None, device_cookie_name=None): + limiter = self.get_limiter(app.config["AUTH_RATELIMIT_USER"], 'auth-user') + return self.is_subject_to_rate_limits(ip) and not limiter.test(device_cookie if device_cookie_name == username else username) + + def rate_limit_user(self, username, ip, device_cookie=None, device_cookie_name=None): + limiter = self.get_limiter(app.config["AUTH_RATELIMIT_USER"], 'auth-user') + if self.is_subject_to_rate_limits(ip): + limiter.hit(device_cookie if device_cookie_name == username else username) + + """ Device cookies as described on: + https://owasp.org/www-community/Slow_Down_Online_Guessing_Attacks_with_Device_Cookies + """ + def parse_device_cookie(self, cookie): + try: + login, nonce, _ = cookie.split('$') + if hmac.compare_digest(cookie, self.device_cookie(login, nonce)): + return nonce, login + except: + pass + return None, None + + """ Device cookies don't require strong crypto: + 72bits of nonce, 96bits of signature is more than enough + and these values avoid padding in most cases + """ + def device_cookie(self, username, nonce=None): + if not nonce: + nonce = secrets.token_urlsafe(9) + sig = str(base64.urlsafe_b64encode(hmac.new(app.device_cookie_key, bytearray(f'device_cookie|{username}|{nonce}', 'utf-8'), 'sha256').digest()[20:]), 'utf-8') + return f'{username}${nonce}${sig}' diff --git a/core/admin/mailu/ui/views/base.py b/core/admin/mailu/ui/views/base.py index ecd3089a..30173acf 100644 --- a/core/admin/mailu/ui/views/base.py +++ b/core/admin/mailu/ui/views/base.py @@ -1,4 +1,4 @@ -from mailu import models +from mailu import models, utils from mailu.ui import ui, forms, access from flask import current_app as app @@ -14,16 +14,28 @@ def index(): @ui.route('/login', methods=['GET', 'POST']) def login(): + client_ip = flask.request.headers["X-Real-IP"] if 'X-Real-IP' in flask.request.headers else flask.request.remote_addr form = forms.LoginForm() if form.validate_on_submit(): - user = models.User.login(form.email.data, form.pw.data) + device_cookie, device_cookie_username = utils.limiter.parse_device_cookie(flask.request.cookies.get('rate_limit')) + username = form.email.data + if username != device_cookie_username and utils.limiter.should_rate_limit_ip(client_ip): + flask.flash('Too many attempts from your IP (rate-limit)', 'error') + return flask.render_template('login.html', form=form) + if utils.limiter.should_rate_limit_user(username, client_ip, device_cookie, device_cookie_username): + flask.flash('Too many attempts for this user (rate-limit)', 'error') + return flask.render_template('login.html', form=form) + user = models.User.login(username, form.pw.data) if user: flask.session.regenerate() flask_login.login_user(user) endpoint = flask.request.args.get('next', '.index') - return flask.redirect(flask.url_for(endpoint) + response = flask.redirect(flask.url_for(endpoint) or flask.url_for('.index')) + response.set_cookie('rate_limit', utils.limiter.device_cookie(username), max_age=31536000, path=flask.url_for('ui.login')) + 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) flask.flash('Wrong e-mail or password', 'error') return flask.render_template('login.html', form=form) diff --git a/core/admin/mailu/utils.py b/core/admin/mailu/utils.py index 34f52d8c..6fce17bc 100644 --- a/core/admin/mailu/utils.py +++ b/core/admin/mailu/utils.py @@ -17,10 +17,12 @@ from multiprocessing import Value from mailu import limiter +from flask import current_app as app import flask import flask_login import flask_migrate import flask_babel +import ipaddress import redis from flask.sessions import SessionMixin, SessionInterface @@ -70,6 +72,15 @@ def has_dane_record(domain, timeout=10): # Rate limiter limiter = limiter.LimitWraperFactory() +def extract_network_from_ip(ip): + n = ipaddress.ip_network(ip) + if isinstance(n, ipaddress.IPv4Network): + return str(n.supernet(prefixlen_diff=(32-int(app.config["AUTH_RATELIMIT_IP_V4_MASK"]))).network_address) + elif isinstance(n, ipaddress.IPv6Network): + return str(n.supernet(prefixlen_diff=(128-int(app.config["AUTH_RATELIMIT_IP_V6_MASK"]))).network_address) + else: # not sure what to do with it + return ip + # Application translation babel = flask_babel.Babel() diff --git a/core/nginx/conf/nginx.conf b/core/nginx/conf/nginx.conf index bfd664de..f013176d 100644 --- a/core/nginx/conf/nginx.conf +++ b/core/nginx/conf/nginx.conf @@ -217,6 +217,7 @@ http { location /internal { internal; + proxy_set_header X-Real-IP $remote_addr; proxy_set_header Authorization $http_authorization; proxy_pass_header Authorization; proxy_pass http://$admin; diff --git a/docs/configuration.rst b/docs/configuration.rst index 5f17b57e..73b56204 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -39,14 +39,21 @@ 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`` holds a security setting for fighting attackers that -try to guess user passwords. The value is the limit of failed authentication attempts -that a single IP address can perform against IMAP, POP and SMTP authentication endpoints. +The ``AUTH_RATELIMIT_IP`` (default: 10/hour) holds a security setting for fighting +attackers that waste server ressources 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). -If ``AUTH_RATELIMIT_SUBNET`` is ``True`` (default: False), 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 ``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 +bruteforce attack). The value defines the limit of authentication attempts allowed +for any given account within a specific timeframe. + +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. +This ensures that users behind a NAT don't get locked out when a single client is +misconfigured... but also potentially allow for users to attack each-other. 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`. diff --git a/docs/swarm/master/README.md b/docs/swarm/master/README.md index 42e742da..0e933308 100644 --- a/docs/swarm/master/README.md +++ b/docs/swarm/master/README.md @@ -100,6 +100,9 @@ https://github.com/moby/moby/issues/25526#issuecomment-336363408 ### Don't create an open relay ! As a side effect of this ingress mode "feature", make sure that the ingress subnet is not in your RELAYHOST, otherwise you would create an smtp open relay :-( +### Ratelimits + +When using ingress mode you probably want to disable rate limits, because all requests originate from the same ip address. Otherwise automatic login attempts can easily DoS the legitimate users. ## Scalability - smtp and imap are scalable diff --git a/setup/flavors/compose/mailu.env b/setup/flavors/compose/mailu.env index 52f4ee04..0ba39019 100644 --- a/setup/flavors/compose/mailu.env +++ b/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 source IP address) +# Authentication rate limit (per /24 on ipv4 and /56 on ipv6) {% if auth_ratelimit_pm > '0' %} AUTH_RATELIMIT={{ auth_ratelimit_pm }}/minute {% endif %} diff --git a/towncrier/newsfragments/116.feature b/towncrier/newsfragments/116.feature new file mode 100644 index 00000000..4f73e7a8 --- /dev/null +++ b/towncrier/newsfragments/116.feature @@ -0,0 +1 @@ + Make the rate limit apply to a subnet rather than a specific IP (/24 for v4 and /56 for v6) diff --git a/towncrier/newsfragments/1194.bugfix b/towncrier/newsfragments/1194.bugfix new file mode 100644 index 00000000..866c6c3b --- /dev/null +++ b/towncrier/newsfragments/1194.bugfix @@ -0,0 +1 @@ +Fix rate-limiting on /webdav/ diff --git a/towncrier/newsfragments/1612.feature b/towncrier/newsfragments/1612.feature new file mode 100644 index 00000000..04d8d508 --- /dev/null +++ b/towncrier/newsfragments/1612.feature @@ -0,0 +1 @@ +Refactor the rate limiter to ensure that it performs as intented.