only account attempts for distinct usernames in ratelimits

main
Florent Daigniere 2 years ago
parent 5b4f2fb075
commit e2a25c79fc

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

@ -52,10 +52,13 @@ class LimitWraperFactory(object):
app.logger.warn(f'Authentication attempt from {ip} has been rate-limited.') app.logger.warn(f'Authentication attempt from {ip} has been rate-limited.')
return is_rate_limited return is_rate_limited
def rate_limit_ip(self, ip): def rate_limit_ip(self, ip, username=None):
limiter = self.get_limiter(app.config["AUTH_RATELIMIT_IP"], 'auth-ip') limiter = self.get_limiter(app.config['AUTH_RATELIMIT_IP'], 'auth-ip')
client_network = utils.extract_network_from_ip(ip) client_network = utils.extract_network_from_ip(ip)
if self.is_subject_to_rate_limits(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) limiter.hit(client_network)
def should_rate_limit_user(self, username, ip, device_cookie=None, device_cookie_name=None): def should_rate_limit_user(self, username, ip, device_cookie=None, device_cookie_name=None):

@ -47,7 +47,7 @@ def login():
flask.flash(msg, "error") flask.flash(msg, "error")
return response return response
else: 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.current_app.logger.warn(f'Login failed for {username} from {client_ip}.')
flask.flash('Wrong e-mail or password', 'error') flask.flash('Wrong e-mail or password', 'error')
return flask.render_template('login.html', form=form, fields=fields) return flask.render_template('login.html', form=form, fields=fields)

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