From e2a25c79fc0fecce6be4e095efb35610c3beeb8a Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Sat, 4 Feb 2023 16:32:51 +0100 Subject: [PATCH 01/10] only account attempts for distinct usernames in ratelimits --- core/admin/mailu/internal/views/auth.py | 6 +++--- core/admin/mailu/limiter.py | 7 +++++-- core/admin/mailu/sso/views/base.py | 2 +- towncrier/newsfragments/2644.misc | 1 + 4 files changed, 10 insertions(+), 6 deletions(-) create mode 100644 towncrier/newsfragments/2644.misc diff --git a/core/admin/mailu/internal/views/auth.py b/core/admin/mailu/internal/views/auth.py index 5f5f8821..435defb5 100644 --- a/core/admin/mailu/internal/views/auth.py +++ b/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 diff --git a/core/admin/mailu/limiter.py b/core/admin/mailu/limiter.py index be4199d2..31ed317a 100644 --- a/core/admin/mailu/limiter.py +++ b/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-{ip}-{username}') > 0): + return + self.storage.incr(f'dedup-{ip}-{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): diff --git a/core/admin/mailu/sso/views/base.py b/core/admin/mailu/sso/views/base.py index aa0e9ea9..f8fd5e10 100644 --- a/core/admin/mailu/sso/views/base.py +++ b/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) diff --git a/towncrier/newsfragments/2644.misc b/towncrier/newsfragments/2644.misc new file mode 100644 index 00000000..8a20b39b --- /dev/null +++ b/towncrier/newsfragments/2644.misc @@ -0,0 +1 @@ +Implement de-dupplication on rate limits. Now only attempts for distinct usernames will count as a hit. From a60159a0db89b56fc86e163a77a13e760a315237 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Sat, 4 Feb 2023 16:46:27 +0100 Subject: [PATCH 02/10] update defaults, rephrase doc --- core/admin/mailu/configuration.py | 2 +- docs/configuration.rst | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/core/admin/mailu/configuration.py b/core/admin/mailu/configuration.py index 11d79643..2f5f6b0c 100644 --- a/core/admin/mailu/configuration.py +++ b/core/admin/mailu/configuration.py @@ -40,7 +40,7 @@ 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_USER': '100/day', diff --git a/docs/configuration.rst b/docs/configuration.rst index 240870cf..c867d8dd 100644 --- a/docs/configuration.rst +++ b/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: /56). 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 From d30f71234dbade3a2c05bf1ed996cd2e4840032d Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Sat, 4 Feb 2023 16:50:43 +0100 Subject: [PATCH 03/10] Apply the mask on the IP too --- core/admin/mailu/limiter.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/admin/mailu/limiter.py b/core/admin/mailu/limiter.py index 31ed317a..85891ee3 100644 --- a/core/admin/mailu/limiter.py +++ b/core/admin/mailu/limiter.py @@ -56,9 +56,9 @@ class LimitWraperFactory(object): 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-{ip}-{username}') > 0): + if username and (self.storage.get(f'dedup-{client_network}-{username}') > 0): return - self.storage.incr(f'dedup-{ip}-{username}', limits.parse(app.config['AUTH_RATELIMIT_IP']).GRANULARITY.seconds ,True) + 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): From 35e9bfb8ab08c3ec8c43e3efdee3db72d1bcc00f Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Sat, 4 Feb 2023 16:54:25 +0100 Subject: [PATCH 04/10] Clarify --- core/admin/mailu/limiter.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/core/admin/mailu/limiter.py b/core/admin/mailu/limiter.py index 85891ee3..2d8380ad 100644 --- a/core/admin/mailu/limiter.py +++ b/core/admin/mailu/limiter.py @@ -56,10 +56,9 @@ class LimitWraperFactory(object): 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) + if username and (self.storage.get(f'dedup-{client_network}-{username}') < 1): + 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): limiter = self.get_limiter(app.config["AUTH_RATELIMIT_USER"], 'auth-user') From 294ac4adb2be1e5ffc4687e96142388cc4a28e93 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Sat, 4 Feb 2023 17:08:26 +0100 Subject: [PATCH 05/10] Revert "Clarify" This reverts commit 35e9bfb8ab08c3ec8c43e3efdee3db72d1bcc00f. --- core/admin/mailu/limiter.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/admin/mailu/limiter.py b/core/admin/mailu/limiter.py index 2d8380ad..85891ee3 100644 --- a/core/admin/mailu/limiter.py +++ b/core/admin/mailu/limiter.py @@ -56,9 +56,10 @@ class LimitWraperFactory(object): 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}') < 1): - self.storage.incr(f'dedup-{client_network}-{username}', limits.parse(app.config['AUTH_RATELIMIT_IP']).GRANULARITY.seconds ,True) - limiter.hit(client_network) + 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): limiter = self.get_limiter(app.config["AUTH_RATELIMIT_USER"], 'auth-user') From fa084d7b1ce842a138d60886797ccf0b32fcfe35 Mon Sep 17 00:00:00 2001 From: Alexander Graf Date: Tue, 7 Feb 2023 08:54:13 +0100 Subject: [PATCH 06/10] Styling only --- core/admin/mailu/limiter.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/admin/mailu/limiter.py b/core/admin/mailu/limiter.py index 85891ee3..d8b36111 100644 --- a/core/admin/mailu/limiter.py +++ b/core/admin/mailu/limiter.py @@ -56,9 +56,9 @@ class LimitWraperFactory(object): 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): + 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) + 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): From 085bac6e08fd6c5ef1155d7e4cc93a0482e45479 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Tue, 7 Feb 2023 09:54:50 +0100 Subject: [PATCH 07/10] Change AUTH_RATELIMIT_IP_V6_MASK from /56 to /48 --- core/admin/mailu/configuration.py | 2 +- docs/configuration.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/admin/mailu/configuration.py b/core/admin/mailu/configuration.py index 2f5f6b0c..b958537c 100644 --- a/core/admin/mailu/configuration.py +++ b/core/admin/mailu/configuration.py @@ -42,7 +42,7 @@ DEFAULT_CONFIG = { 'DEFER_ON_TLS_ERROR': True, '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, diff --git a/docs/configuration.rst b/docs/configuration.rst index c867d8dd..abb0860d 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -45,7 +45,7 @@ attackers that attempt a password spraying attack. The value defines the limit o 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: /56). +``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 From 83cc23a51a3f04a21f81b35e255d2e95b926eb35 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Thu, 9 Feb 2023 11:24:06 +0100 Subject: [PATCH 08/10] Update comment too --- setup/flavors/compose/mailu.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup/flavors/compose/mailu.env b/setup/flavors/compose/mailu.env index 975e8136..8fac8ad9 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 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 %} From d20c217ae68213aa0d5236e3189d758322f77c05 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Thu, 9 Feb 2023 11:29:55 +0100 Subject: [PATCH 09/10] Change the default in setup too --- setup/templates/steps/config.html | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/setup/templates/steps/config.html b/setup/templates/steps/config.html index fe8bc0cb..34f22ec1 100644 --- a/setup/templates/steps/config.html +++ b/setup/templates/steps/config.html @@ -39,8 +39,7 @@ Or in plain english: if receivers start to classify your mail as spam, this post
-

/ hour +

/ hour

From bb127d15ffccfbfeb3e9c61851105fdd07562465 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Thu, 9 Feb 2023 11:32:10 +0100 Subject: [PATCH 10/10] clarify --- setup/templates/steps/config.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup/templates/steps/config.html b/setup/templates/steps/config.html index 34f22ec1..b3b5fe87 100644 --- a/setup/templates/steps/config.html +++ b/setup/templates/steps/config.html @@ -37,7 +37,7 @@ Or in plain english: if receivers start to classify your mail as spam, this post
- +

/ hour