Skip to content

Commit 490b715

Browse files
committed
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.
1 parent 398cba8 commit 490b715

File tree

5 files changed

+198
-1
lines changed

5 files changed

+198
-1
lines changed

CHANGES.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ Features:
7878
nosymessage before it gets sent. See issue:
7979
https://issues.roundup-tracker.org/issue2551018 for example
8080
nosyreaction and generated email. (Tom Ekberg (tekberg))
81+
- issue2550949: Rate limit password guesses/login attempts. Rate
82+
limit mechanism added for web page logins. Default is 3 login
83+
attempts/minute for a user. After which one login attempt every 20
84+
seconds can be done. (John Rouillard)
8185

8286
Fixed:
8387

roundup/cgi/actions.py

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,15 @@
55
from roundup.i18n import _
66
from roundup.cgi import exceptions, templating
77
from roundup.mailgw import uidFromAddress
8+
from roundup.rate_limit import Store, RateLimit
89
from roundup.exceptions import Reject, RejectRaw
910
from roundup.anypy import urllib_
1011
from roundup.anypy.strings import StringIO
1112
import roundup.anypy.random_ as random_
1213

14+
import time
15+
from datetime import timedelta
16+
1317
# Also add action to client.py::Client.actions property
1418
__all__ = ['Action', 'ShowAction', 'RetireAction', 'RestoreAction', 'SearchAction',
1519
'EditCSVAction', 'EditItemAction', 'PassResetAction',
@@ -31,6 +35,7 @@ def __init__(self, client):
3135
self.base = client.base
3236
self.user = client.user
3337
self.context = templating.context(client)
38+
self.loginLimit = RateLimit(self.db.config['WEB_LOGIN_ATTEMPTS_MIN'], timedelta(seconds=60))
3439

3540
def handle(self):
3641
"""Action handler procedure"""
@@ -1226,7 +1231,34 @@ def handle(self):
12261231
)
12271232

12281233
try:
1229-
self.verifyLogin(self.client.user, password)
1234+
# Implement rate limiting of logins by login name.
1235+
# Use prefix to prevent key collisions maybe??
1236+
rlkey="LOGIN-" + self.client.user
1237+
limit=self.loginLimit
1238+
s=Store()
1239+
otk=self.client.db.Otk
1240+
try:
1241+
val=otk.getall(rlkey)
1242+
s.set_tat_as_string(rlkey, val['tat'])
1243+
except KeyError:
1244+
# ignore if tat not set, it's 1970-1-1 by default.
1245+
pass
1246+
# see if rate limit exceeded and we need to reject the attempt
1247+
reject=s.update(rlkey, limit)
1248+
1249+
# Calculate a timestamp that will make OTK expire the
1250+
# unused entry 1 hour in the future
1251+
ts = time.time() - (60 * 60 * 24 * 7) + 3600
1252+
otk.set(rlkey, tat=s.get_tat_as_string(rlkey),
1253+
__timestamp=ts)
1254+
otk.commit()
1255+
1256+
if reject:
1257+
# User exceeded limits: find out how long to wait
1258+
status=s.status(rlkey, limit)
1259+
raise Reject(_("Logins occurring too fast. Please wait: %d seconds.")%status['Retry-After'])
1260+
else:
1261+
self.verifyLogin(self.client.user, password)
12301262
except exceptions.LoginError as err:
12311263
self.client.make_user_anonymous()
12321264
for arg in err.args:

roundup/configuration.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -718,6 +718,12 @@ def str2value(self, value):
718718
"variables supplied by your web server (in that order).\n"
719719
"Set this option to 'no' if you do not wish to use HTTP Basic\n"
720720
"Authentication in your web interface."),
721+
(IntegerNumberOption, 'login_attempts_min', "3",
722+
"Limit login attempts per user per minute to this number.\n"
723+
"By default the 4th login attempt in a minute will notify\n"
724+
"the user that they need to wait 20 seconds before trying to\n"
725+
"log in again. This limits password guessing attacks and\n"
726+
"shouldn't need to be changed.\n"),
721727
(SameSiteSettingOption, 'samesite_cookie_setting', "Lax",
722728
"""Set the mode of the SameSite cookie option for
723729
the session cookie. Choices are 'Lax' or

roundup/rate_limit.py

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
# Originaly from
2+
# https://smarketshq.com/implementing-gcra-in-python-5df1f11aaa96?gi=4b9725f99bfa
3+
# with imports, modifications for python 2, implementation of
4+
# set/get_tat and marshaling as string, support for testonly
5+
# and status method.
6+
7+
from datetime import timedelta, datetime
8+
9+
class RateLimit:
10+
def __init__(self, count, period):
11+
self.count = count
12+
self.period = period
13+
14+
@property
15+
def inverse(self):
16+
return self.period.total_seconds() / self.count
17+
18+
19+
class Store:
20+
21+
memory = {}
22+
23+
def get_tat(self, key):
24+
# This should return a previous tat for the key or the current time.
25+
if key in self.memory:
26+
return self.memory[key]
27+
else:
28+
return datetime.min
29+
30+
def set_tat(self, key, tat):
31+
self.memory[key] = tat
32+
33+
def get_tat_as_string(self, key):
34+
# get value as string:
35+
# YYYY-MM-DDTHH:MM:SS.mmmmmm
36+
# to allow it to be marshalled/unmarshaled
37+
if key in self.memory:
38+
return self.memory[key].isoformat()
39+
else:
40+
return datetime.min.isoformat()
41+
42+
43+
def set_tat_as_string(self, key, tat):
44+
# Take value as string and unmarshall:
45+
# YYYY-MM-DDTHH:MM:SS.mmmmmm
46+
# to datetime
47+
self.memory[key] = datetime.strptime(tat,"%Y-%m-%dT%H:%M:%S.%f")
48+
49+
def update(self, key, limit, testonly=False):
50+
'''Determine if the item assocaited with the key should be
51+
rejected given the RateLimit limit.
52+
'''
53+
now = datetime.utcnow()
54+
tat = max(self.get_tat(key), now)
55+
separation = (tat - now).total_seconds()
56+
max_interval = limit.period.total_seconds() - limit.inverse
57+
if separation > max_interval:
58+
reject = True
59+
else:
60+
reject = False
61+
if not testonly:
62+
new_tat = max(tat, now) + timedelta(seconds=limit.inverse)
63+
self.set_tat(key, new_tat)
64+
return reject
65+
66+
def status(self, key, limit):
67+
'''Return status suitable for displaying as headers:
68+
X-RateLimit-Limit: calls allowed per period. Period/window
69+
is not specified in any api I found.
70+
X-RateLimit-Limit-Period: Non standard. Defines period in
71+
seconds for RateLimit-Limit.
72+
X-RateLimit-Remaining: How many calls are left in this window.
73+
X-RateLimit-Reset: window ends in this many seconds (not an
74+
epoch timestamp) and all RateLimit-Limit calls are
75+
available again.
76+
Retry-After: if user's request fails, this is the next time there
77+
will be at least 1 available call to be consumed.
78+
'''
79+
80+
ret = {}
81+
tat = self.get_tat(key)
82+
83+
# static defined headers according to limit
84+
ret['X-RateLimit-Limit'] = limit.count
85+
ret['X-RateLimit-Limit-Period'] = limit.period.total_seconds()
86+
87+
# status of current limit as of now
88+
now = datetime.utcnow()
89+
90+
ret['X-RateLimit-Remaining'] = min(int(
91+
(limit.period - (tat - now)).total_seconds() \
92+
/limit.inverse),ret['X-RateLimit-Limit'])
93+
94+
tat_epochsec = (tat - datetime(1970, 1, 1)).total_seconds()
95+
seconds_to_tat = (tat - now).total_seconds()
96+
ret['X-RateLimit-Reset'] = max(seconds_to_tat, 0)
97+
ret['X-RateLimit-Reset-date'] = "%s"%tat
98+
ret['Now'] = (now - datetime(1970,1,1)).total_seconds()
99+
ret['Now-date'] = "%s"%now
100+
101+
if self.update(key, limit, testonly=True):
102+
# A new request would be rejected if it was processes.
103+
# The user has to wait until an item is dequeued.
104+
# One item is dequeued every limit.inverse seconds.
105+
ret['Retry-After'] = limit.inverse
106+
ret['Retry-After-Timestamp'] = "%s"%(now + timedelta(seconds=limit.inverse))
107+
else:
108+
# if we are not rejected, the user can post another
109+
# attempt immediately.
110+
# Do we even need this header if not rejected?
111+
# RFC implies this is used with a 503 (or presumably
112+
# 429 which may postdate the rfc). So if no error, no header?
113+
# ret['Retry-After'] = 0
114+
# ret['Retry-After-Timestamp'] = ret['Now-date']
115+
pass
116+
117+
return ret

test/test_actions.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@
77
from roundup.cgi.actions import *
88
from roundup.cgi.client import add_message
99
from roundup.cgi.exceptions import Redirect, Unauthorised, SeriousError, FormError
10+
from roundup.exceptions import Reject
1011

1112
from roundup.anypy.cmp_ import NoneAndDictComparable
13+
from time import sleep
14+
from datetime import datetime
1215

1316
from .mocknull import MockNull
1417

@@ -19,6 +22,11 @@ class ActionTestCase(unittest.TestCase):
1922
def setUp(self):
2023
self.form = FieldStorage(environ={'QUERY_STRING': ''})
2124
self.client = MockNull()
25+
self.client.db.Otk = MockNull()
26+
self.client.db.Otk.data = {}
27+
self.client.db.Otk.getall = self.data_get
28+
self.client.db.Otk.set = self.data_set
29+
self.client.db.config = {'WEB_LOGIN_ATTEMPTS_MIN': 20}
2230
self.client._ok_message = []
2331
self.client._error_message = []
2432
self.client.add_error_message = lambda x : add_message(
@@ -31,6 +39,12 @@ class TemplatingUtils:
3139
pass
3240
self.client.instance.interfaces.TemplatingUtils = TemplatingUtils
3341

42+
def data_get(self, key):
43+
return self.client.db.Otk.data[key]
44+
45+
def data_set(self, key, **value):
46+
self.client.db.Otk.data[key] = value
47+
3448
class ShowActionTestCase(ActionTestCase):
3549
def assertRaisesMessage(self, exception, callable, message, *args,
3650
**kwargs):
@@ -357,6 +371,30 @@ def opendb(username):
357371
self.assertLoginRaisesRedirect("http://whoami.com/path/issue255?%40error_message=Invalid+login",
358372
'foo', 'wrong', "http://whoami.com/path/issue255")
359373

374+
def testLoginRateLimit(self):
375+
''' Set number of logins in setup to 20 per minute. Three second
376+
delay between login attempts doesn't trip rate limit.
377+
Default limit is 3/min, but that means we sleep for 20
378+
seconds so I override the default limit to speed this up.
379+
'''
380+
# Do the first login setting an invalid login name
381+
self.assertLoginLeavesMessages(['Invalid login'], 'nouser')
382+
# use up the rest of the 20 login attempts
383+
for i in range(19):
384+
self.client._error_message = []
385+
self.assertLoginLeavesMessages(['Invalid login'])
386+
387+
self.assertRaisesMessage(Reject, LoginAction(self.client).handle,
388+
'Logins occurring too fast. Please wait: 3 seconds.')
389+
390+
sleep(3) # sleep as requested so we can do another login
391+
self.client._error_message = []
392+
self.assertLoginLeavesMessages(['Invalid login']) # this is expected
393+
394+
# and make sure we need to wait another three seconds
395+
self.assertRaisesMessage(Reject, LoginAction(self.client).handle,
396+
'Logins occurring too fast. Please wait: 3 seconds.')
397+
360398
class EditItemActionTestCase(ActionTestCase):
361399
def setUp(self):
362400
ActionTestCase.setUp(self)

0 commit comments

Comments
 (0)