Mercurial > p > roundup > code
changeset 5717:cad18de2b988
issue2550949: Rate limit password guesses/login attempts.
Generic rate limit mechanism added. Deployed for web page
logins. Default is 3 login attempts/minute for a user. After which one
login attempt every 20 seconds can be done.
Uses gcra algorithm so all I need to store is a username and timestamp
in the one time key database. This does mean I don't have a list of
all failed login attempts as part of the rate limiter.
Set up config setting as well so admin can tune the rate. Maybe 1
every 10 seconds is ok at a site with poor typists who need 6 attempts
to get the password right 8-).
The gcra method can also be used to limit the rest and xmlrpc
interfaces if needed. The mechanism I added also supplies a status
method that calculates the expected values for http headers returned
as part of rate limiting.
Also tests added to test all code paths I hope.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Sat, 11 May 2019 17:24:58 -0400 |
| parents | 42a713e36def |
| children | 842252c3ee22 |
| files | CHANGES.txt roundup/cgi/actions.py roundup/configuration.py roundup/rate_limit.py test/test_actions.py |
| diffstat | 5 files changed, 198 insertions(+), 1 deletions(-) [+] |
line wrap: on
line diff
--- a/CHANGES.txt Sun Apr 28 18:44:05 2019 -0400 +++ b/CHANGES.txt Sat May 11 17:24:58 2019 -0400 @@ -78,6 +78,10 @@ nosymessage before it gets sent. See issue: https://issues.roundup-tracker.org/issue2551018 for example nosyreaction and generated email. (Tom Ekberg (tekberg)) +- issue2550949: Rate limit password guesses/login attempts. Rate + limit mechanism added for web page logins. Default is 3 login + attempts/minute for a user. After which one login attempt every 20 + seconds can be done. (John Rouillard) Fixed:
--- a/roundup/cgi/actions.py Sun Apr 28 18:44:05 2019 -0400 +++ b/roundup/cgi/actions.py Sat May 11 17:24:58 2019 -0400 @@ -5,11 +5,15 @@ from roundup.i18n import _ from roundup.cgi import exceptions, templating from roundup.mailgw import uidFromAddress +from roundup.rate_limit import Store, RateLimit from roundup.exceptions import Reject, RejectRaw from roundup.anypy import urllib_ from roundup.anypy.strings import StringIO import roundup.anypy.random_ as random_ +import time +from datetime import timedelta + # Also add action to client.py::Client.actions property __all__ = ['Action', 'ShowAction', 'RetireAction', 'RestoreAction', 'SearchAction', 'EditCSVAction', 'EditItemAction', 'PassResetAction', @@ -31,6 +35,7 @@ self.base = client.base self.user = client.user self.context = templating.context(client) + self.loginLimit = RateLimit(self.db.config['WEB_LOGIN_ATTEMPTS_MIN'], timedelta(seconds=60)) def handle(self): """Action handler procedure""" @@ -1226,7 +1231,34 @@ ) try: - self.verifyLogin(self.client.user, password) + # Implement rate limiting of logins by login name. + # Use prefix to prevent key collisions maybe?? + rlkey="LOGIN-" + self.client.user + limit=self.loginLimit + s=Store() + otk=self.client.db.Otk + try: + val=otk.getall(rlkey) + s.set_tat_as_string(rlkey, val['tat']) + except KeyError: + # ignore if tat not set, it's 1970-1-1 by default. + pass + # see if rate limit exceeded and we need to reject the attempt + reject=s.update(rlkey, limit) + + # Calculate a timestamp that will make OTK expire the + # unused entry 1 hour in the future + ts = time.time() - (60 * 60 * 24 * 7) + 3600 + otk.set(rlkey, tat=s.get_tat_as_string(rlkey), + __timestamp=ts) + otk.commit() + + if reject: + # User exceeded limits: find out how long to wait + status=s.status(rlkey, limit) + raise Reject(_("Logins occurring too fast. Please wait: %d seconds.")%status['Retry-After']) + else: + self.verifyLogin(self.client.user, password) except exceptions.LoginError as err: self.client.make_user_anonymous() for arg in err.args:
--- a/roundup/configuration.py Sun Apr 28 18:44:05 2019 -0400 +++ b/roundup/configuration.py Sat May 11 17:24:58 2019 -0400 @@ -718,6 +718,12 @@ "variables supplied by your web server (in that order).\n" "Set this option to 'no' if you do not wish to use HTTP Basic\n" "Authentication in your web interface."), + (IntegerNumberOption, 'login_attempts_min', "3", + "Limit login attempts per user per minute to this number.\n" + "By default the 4th login attempt in a minute will notify\n" + "the user that they need to wait 20 seconds before trying to\n" + "log in again. This limits password guessing attacks and\n" + "shouldn't need to be changed.\n"), (SameSiteSettingOption, 'samesite_cookie_setting', "Lax", """Set the mode of the SameSite cookie option for the session cookie. Choices are 'Lax' or
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/roundup/rate_limit.py Sat May 11 17:24:58 2019 -0400 @@ -0,0 +1,117 @@ +# Originaly from +# https://smarketshq.com/implementing-gcra-in-python-5df1f11aaa96?gi=4b9725f99bfa +# with imports, modifications for python 2, implementation of +# set/get_tat and marshaling as string, support for testonly +# and status method. + +from datetime import timedelta, datetime + +class RateLimit: + def __init__(self, count, period): + self.count = count + self.period = period + + @property + def inverse(self): + return self.period.total_seconds() / self.count + + +class Store: + + memory = {} + + def get_tat(self, key): + # This should return a previous tat for the key or the current time. + if key in self.memory: + return self.memory[key] + else: + return datetime.min + + def set_tat(self, key, tat): + self.memory[key] = tat + + def get_tat_as_string(self, key): + # get value as string: + # YYYY-MM-DDTHH:MM:SS.mmmmmm + # to allow it to be marshalled/unmarshaled + if key in self.memory: + return self.memory[key].isoformat() + else: + return datetime.min.isoformat() + + + def set_tat_as_string(self, key, tat): + # Take value as string and unmarshall: + # YYYY-MM-DDTHH:MM:SS.mmmmmm + # to datetime + self.memory[key] = datetime.strptime(tat,"%Y-%m-%dT%H:%M:%S.%f") + + def update(self, key, limit, testonly=False): + '''Determine if the item assocaited with the key should be + rejected given the RateLimit limit. + ''' + now = datetime.utcnow() + tat = max(self.get_tat(key), now) + separation = (tat - now).total_seconds() + max_interval = limit.period.total_seconds() - limit.inverse + if separation > max_interval: + reject = True + else: + reject = False + if not testonly: + new_tat = max(tat, now) + timedelta(seconds=limit.inverse) + self.set_tat(key, new_tat) + return reject + + def status(self, key, limit): + '''Return status suitable for displaying as headers: + X-RateLimit-Limit: calls allowed per period. Period/window + is not specified in any api I found. + X-RateLimit-Limit-Period: Non standard. Defines period in + seconds for RateLimit-Limit. + X-RateLimit-Remaining: How many calls are left in this window. + X-RateLimit-Reset: window ends in this many seconds (not an + epoch timestamp) and all RateLimit-Limit calls are + available again. + Retry-After: if user's request fails, this is the next time there + will be at least 1 available call to be consumed. + ''' + + ret = {} + tat = self.get_tat(key) + + # static defined headers according to limit + ret['X-RateLimit-Limit'] = limit.count + ret['X-RateLimit-Limit-Period'] = limit.period.total_seconds() + + # status of current limit as of now + now = datetime.utcnow() + + ret['X-RateLimit-Remaining'] = min(int( + (limit.period - (tat - now)).total_seconds() \ + /limit.inverse),ret['X-RateLimit-Limit']) + + tat_epochsec = (tat - datetime(1970, 1, 1)).total_seconds() + seconds_to_tat = (tat - now).total_seconds() + ret['X-RateLimit-Reset'] = max(seconds_to_tat, 0) + ret['X-RateLimit-Reset-date'] = "%s"%tat + ret['Now'] = (now - datetime(1970,1,1)).total_seconds() + ret['Now-date'] = "%s"%now + + if self.update(key, limit, testonly=True): + # A new request would be rejected if it was processes. + # The user has to wait until an item is dequeued. + # One item is dequeued every limit.inverse seconds. + ret['Retry-After'] = limit.inverse + ret['Retry-After-Timestamp'] = "%s"%(now + timedelta(seconds=limit.inverse)) + else: + # if we are not rejected, the user can post another + # attempt immediately. + # Do we even need this header if not rejected? + # RFC implies this is used with a 503 (or presumably + # 429 which may postdate the rfc). So if no error, no header? + # ret['Retry-After'] = 0 + # ret['Retry-After-Timestamp'] = ret['Now-date'] + pass + + return ret
--- a/test/test_actions.py Sun Apr 28 18:44:05 2019 -0400 +++ b/test/test_actions.py Sat May 11 17:24:58 2019 -0400 @@ -7,8 +7,11 @@ from roundup.cgi.actions import * from roundup.cgi.client import add_message from roundup.cgi.exceptions import Redirect, Unauthorised, SeriousError, FormError +from roundup.exceptions import Reject from roundup.anypy.cmp_ import NoneAndDictComparable +from time import sleep +from datetime import datetime from .mocknull import MockNull @@ -19,6 +22,11 @@ def setUp(self): self.form = FieldStorage(environ={'QUERY_STRING': ''}) self.client = MockNull() + self.client.db.Otk = MockNull() + self.client.db.Otk.data = {} + self.client.db.Otk.getall = self.data_get + self.client.db.Otk.set = self.data_set + self.client.db.config = {'WEB_LOGIN_ATTEMPTS_MIN': 20} self.client._ok_message = [] self.client._error_message = [] self.client.add_error_message = lambda x : add_message( @@ -31,6 +39,12 @@ pass self.client.instance.interfaces.TemplatingUtils = TemplatingUtils + def data_get(self, key): + return self.client.db.Otk.data[key] + + def data_set(self, key, **value): + self.client.db.Otk.data[key] = value + class ShowActionTestCase(ActionTestCase): def assertRaisesMessage(self, exception, callable, message, *args, **kwargs): @@ -357,6 +371,30 @@ self.assertLoginRaisesRedirect("http://whoami.com/path/issue255?%40error_message=Invalid+login", 'foo', 'wrong', "http://whoami.com/path/issue255") + def testLoginRateLimit(self): + ''' Set number of logins in setup to 20 per minute. Three second + delay between login attempts doesn't trip rate limit. + Default limit is 3/min, but that means we sleep for 20 + seconds so I override the default limit to speed this up. + ''' + # Do the first login setting an invalid login name + self.assertLoginLeavesMessages(['Invalid login'], 'nouser') + # use up the rest of the 20 login attempts + for i in range(19): + self.client._error_message = [] + self.assertLoginLeavesMessages(['Invalid login']) + + self.assertRaisesMessage(Reject, LoginAction(self.client).handle, + 'Logins occurring too fast. Please wait: 3 seconds.') + + sleep(3) # sleep as requested so we can do another login + self.client._error_message = [] + self.assertLoginLeavesMessages(['Invalid login']) # this is expected + + # and make sure we need to wait another three seconds + self.assertRaisesMessage(Reject, LoginAction(self.client).handle, + 'Logins occurring too fast. Please wait: 3 seconds.') + class EditItemActionTestCase(ActionTestCase): def setUp(self): ActionTestCase.setUp(self)
