From 2bd2a649ca5ec616be14047cd1b95a4e4a137e8f Mon Sep 17 00:00:00 2001 From: Johnny Date: Tue, 13 Oct 2020 20:27:05 +0000 Subject: [PATCH 1/7] Refactors throttle to use Django caching. --- evennia/accounts/accounts.py | 4 +- evennia/server/throttle.py | 97 +++++++++++++++++++++++++++++------- 2 files changed, 80 insertions(+), 21 deletions(-) diff --git a/evennia/accounts/accounts.py b/evennia/accounts/accounts.py index 8bf324c898..3861a118b4 100644 --- a/evennia/accounts/accounts.py +++ b/evennia/accounts/accounts.py @@ -52,10 +52,10 @@ _MUDINFO_CHANNEL = None # Create throttles for too many account-creations and login attempts CREATION_THROTTLE = Throttle( - limit=settings.CREATION_THROTTLE_LIMIT, timeout=settings.CREATION_THROTTLE_TIMEOUT + name='creation', limit=settings.CREATION_THROTTLE_LIMIT, timeout=settings.CREATION_THROTTLE_TIMEOUT ) LOGIN_THROTTLE = Throttle( - limit=settings.LOGIN_THROTTLE_LIMIT, timeout=settings.LOGIN_THROTTLE_TIMEOUT + name='authentication', limit=settings.LOGIN_THROTTLE_LIMIT, timeout=settings.LOGIN_THROTTLE_TIMEOUT ) diff --git a/evennia/server/throttle.py b/evennia/server/throttle.py index 4a22c03113..a38c7cedcc 100644 --- a/evennia/server/throttle.py +++ b/evennia/server/throttle.py @@ -1,4 +1,5 @@ -from collections import defaultdict, deque +from django.core.cache import caches +from collections import deque from evennia.utils import logger import time @@ -12,8 +13,8 @@ class Throttle(object): This version of the throttle is usable by both the terminal server as well as the web server, imposes limits on memory consumption by using deques - with length limits instead of open-ended lists, and removes sparse keys when - no recent failures have been recorded. + with length limits instead of open-ended lists, and uses native Django + caches for automatic key eviction and persistence configurability. """ error_msg = "Too many failed attempts; you must wait a few minutes before trying again." @@ -23,6 +24,7 @@ class Throttle(object): Allows setting of throttle parameters. Keyword Args: + name (str): Name of this throttle. limit (int): Max number of failures before imposing limiter timeout (int): number of timeout seconds after max number of tries has been reached. @@ -30,9 +32,37 @@ class Throttle(object): rolling window; this is NOT the same as the limit after which the throttle is imposed! """ - self.storage = defaultdict(deque) - self.cache_size = self.limit = kwargs.get("limit", 5) + try: + self.storage = caches['throttle'] + except Exception as e: + logger.log_err(f'Throttle: {e}') + self.storage = caches['default'] + + self.name = kwargs.get('name', 'ip-throttle') + self.limit = kwargs.get("limit", 5) + self.cache_size = kwargs.get('cache_size', self.limit) self.timeout = kwargs.get("timeout", 5 * 60) + + def get_cache_key(self, *args, **kwargs): + """ + Creates a 'prefixed' key containing arbitrary terms to prevent key + collisions in the same namespace. + + """ + return '-'.join((self.name, *args)) + + def touch(self, key, *args, **kwargs): + """ + Refreshes the timeout on a given key and ensures it is recorded in the + key register. + + Args: + key(str): Key of entry to renew. + + """ + cache_key = self.get_cache_key(key) + if self.storage.touch(cache_key, self.timeout): + self.record_key(key) def get(self, ip=None): """ @@ -50,9 +80,18 @@ class Throttle(object): """ if ip: - return self.storage.get(ip, deque(maxlen=self.cache_size)) + cache_key = self.get_cache_key(str(ip)) + return self.storage.get(cache_key, deque(maxlen=self.limit)) else: - return self.storage + keys_key = self.get_cache_key('keys') + keys = self.storage.get_or_set(keys_key, set(), self.timeout) + data = self.storage.get_many((self.get_cache_key(x) for x in keys)) + + found_keys = set(data.keys()) + if len(keys) != len(found_keys): + self.storage.set(keys_key, found_keys, self.timeout) + + return data def update(self, ip, failmsg="Exceeded threshold."): """ @@ -67,24 +106,41 @@ class Throttle(object): None """ + cache_key = self.get_cache_key(ip) + # Get current status previously_throttled = self.check(ip) - # Enforce length limits - if not self.storage[ip].maxlen: - self.storage[ip] = deque(maxlen=self.cache_size) - - self.storage[ip].append(time.time()) + # Get previous failures, if any + entries = self.storage.get(cache_key, []) + entries.append(time.time()) + + # Store updated record + self.storage.set(cache_key, deque(entries, maxlen=self.limit), self.timeout) # See if this update caused a change in status currently_throttled = self.check(ip) # If this makes it engage, log a single activation event if not previously_throttled and currently_throttled: - logger.log_sec( - "Throttle Activated: %s (IP: %s, %i hits in %i seconds.)" - % (failmsg, ip, self.limit, self.timeout) - ) + logger.log_sec(f"Throttle Activated: {failmsg} (IP: {ip}, {self.limit} hits in {self.timeout} seconds.)") + + self.record_key(ip) + + def record_key(self, key, *args, **kwargs): + """ + Tracks keys as they are added to the cache (since there is no way to + get a list of keys after-the-fact). + + Args: + key(str): Key being added to cache. This should be the original + key, not the cache-prefixed version. + + """ + keys_key = self.get_cache_key('keys') + keys = self.storage.get(keys_key, set()) + keys.add(key) + self.storage.set(keys_key, keys, self.timeout) def check(self, ip): """ @@ -102,17 +158,20 @@ class Throttle(object): """ now = time.time() ip = str(ip) + + cache_key = self.get_cache_key(ip) # checking mode - latest_fails = self.storage[ip] + latest_fails = self.storage.get(cache_key) if latest_fails and len(latest_fails) >= self.limit: # too many fails recently if now - latest_fails[-1] < self.timeout: # too soon - timeout in play + self.touch(cache_key) return True else: # timeout has passed. clear faillist - del self.storage[ip] + self.storage.delete(cache_key) return False else: - return False + return False \ No newline at end of file From fef356435bd95ada1367e6023c5b38b64b2a521f Mon Sep 17 00:00:00 2001 From: Johnny Date: Tue, 13 Oct 2020 20:57:16 +0000 Subject: [PATCH 2/7] Corrects variable used for setting cache size. --- evennia/server/throttle.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/evennia/server/throttle.py b/evennia/server/throttle.py index a38c7cedcc..5ac4f8517f 100644 --- a/evennia/server/throttle.py +++ b/evennia/server/throttle.py @@ -81,7 +81,7 @@ class Throttle(object): """ if ip: cache_key = self.get_cache_key(str(ip)) - return self.storage.get(cache_key, deque(maxlen=self.limit)) + return self.storage.get(cache_key, deque(maxlen=self.cache_size)) else: keys_key = self.get_cache_key('keys') keys = self.storage.get_or_set(keys_key, set(), self.timeout) @@ -116,7 +116,7 @@ class Throttle(object): entries.append(time.time()) # Store updated record - self.storage.set(cache_key, deque(entries, maxlen=self.limit), self.timeout) + self.storage.set(cache_key, deque(entries, maxlen=self.cache_size), self.timeout) # See if this update caused a change in status currently_throttled = self.check(ip) From 8219a1db284180b3f2edd17565b1b949a2e3a6cb Mon Sep 17 00:00:00 2001 From: Johnny Date: Thu, 15 Oct 2020 01:22:55 +0000 Subject: [PATCH 3/7] Renames authentication throttle back to login throttle. --- evennia/accounts/accounts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evennia/accounts/accounts.py b/evennia/accounts/accounts.py index 3861a118b4..f6bc0cb7d4 100644 --- a/evennia/accounts/accounts.py +++ b/evennia/accounts/accounts.py @@ -55,7 +55,7 @@ CREATION_THROTTLE = Throttle( name='creation', limit=settings.CREATION_THROTTLE_LIMIT, timeout=settings.CREATION_THROTTLE_TIMEOUT ) LOGIN_THROTTLE = Throttle( - name='authentication', limit=settings.LOGIN_THROTTLE_LIMIT, timeout=settings.LOGIN_THROTTLE_TIMEOUT + name='login', limit=settings.LOGIN_THROTTLE_LIMIT, timeout=settings.LOGIN_THROTTLE_TIMEOUT ) From 92c41a721c9da19fb56cb9940c49d42d548a4f65 Mon Sep 17 00:00:00 2001 From: Johnny Date: Thu, 15 Oct 2020 01:23:10 +0000 Subject: [PATCH 4/7] Adds cache definition to settings. --- evennia/settings_default.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/evennia/settings_default.py b/evennia/settings_default.py index ac00b18b29..d212e820fd 100644 --- a/evennia/settings_default.py +++ b/evennia/settings_default.py @@ -866,6 +866,21 @@ WEBCLIENT_OPTIONS = { # messages } +# Django cache settings +# https://docs.djangoproject.com/en/dev/topics/cache/#setting-up-the-cache +CACHES = { + 'default': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + }, + 'throttle': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'TIMEOUT': 60 * 5, + 'OPTIONS': { + 'MAX_ENTRIES': 2000 + } + } +} + # We setup the location of the website template as well as the admin site. TEMPLATES = [ { From ee95fca6cfc9a4531a7f40c1b0a00aebe8093bca Mon Sep 17 00:00:00 2001 From: Johnny Date: Tue, 20 Oct 2020 20:35:14 +0000 Subject: [PATCH 5/7] Adds more verbose error message and logs traceback when errors encountered at init. --- evennia/server/throttle.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/evennia/server/throttle.py b/evennia/server/throttle.py index 5ac4f8517f..eb94df5644 100644 --- a/evennia/server/throttle.py +++ b/evennia/server/throttle.py @@ -35,10 +35,10 @@ class Throttle(object): try: self.storage = caches['throttle'] except Exception as e: - logger.log_err(f'Throttle: {e}') + logger.log_trace("Throttle: Errors encountered; using default cache.") self.storage = caches['default'] - self.name = kwargs.get('name', 'ip-throttle') + self.name = kwargs.get('name', 'undefined-throttle') self.limit = kwargs.get("limit", 5) self.cache_size = kwargs.get('cache_size', self.limit) self.timeout = kwargs.get("timeout", 5 * 60) From 3444cce6e9c8a466f4c6562b2e4321ce676ea24d Mon Sep 17 00:00:00 2001 From: Johnny Date: Tue, 20 Oct 2020 21:00:03 +0000 Subject: [PATCH 6/7] Adds remove() method and renames record/unrecord methods to reflect expected input. --- evennia/server/throttle.py | 50 ++++++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 7 deletions(-) diff --git a/evennia/server/throttle.py b/evennia/server/throttle.py index eb94df5644..c8d8571122 100644 --- a/evennia/server/throttle.py +++ b/evennia/server/throttle.py @@ -125,22 +125,58 @@ class Throttle(object): if not previously_throttled and currently_throttled: logger.log_sec(f"Throttle Activated: {failmsg} (IP: {ip}, {self.limit} hits in {self.timeout} seconds.)") - self.record_key(ip) - - def record_key(self, key, *args, **kwargs): + self.record_ip(ip) + + def remove(self, ip, *args, **kwargs): + """ + Clears data stored for an IP from the throttle. + + Args: + ip(str): IP to clear. + + """ + exists = self.get(ip) + if not exists: return False + + cache_key = self.get_cache_key(ip) + self.storage.delete(cache_key) + self.unrecord_ip(ip) + + # Return True if NOT exists + return ~bool(self.get(ip)) + + def record_ip(self, ip, *args, **kwargs): """ Tracks keys as they are added to the cache (since there is no way to get a list of keys after-the-fact). Args: - key(str): Key being added to cache. This should be the original - key, not the cache-prefixed version. + ip(str): IP being added to cache. This should be the original + IP, not the cache-prefixed key. """ keys_key = self.get_cache_key('keys') keys = self.storage.get(keys_key, set()) - keys.add(key) + keys.add(ip) self.storage.set(keys_key, keys, self.timeout) + return True + + def unrecord_ip(self, ip, *args, **kwargs): + """ + Forces removal of a key from the key registry. + + Args: + ip(str): IP to remove from list of keys. + + """ + keys_key = self.get_cache_key('keys') + keys = self.storage.get(keys_key, set()) + try: + keys.remove(ip) + self.storage.set(keys_key, keys, self.timeout) + return True + except KeyError: + return False def check(self, ip): """ @@ -171,7 +207,7 @@ class Throttle(object): return True else: # timeout has passed. clear faillist - self.storage.delete(cache_key) + self.remove(ip) return False else: return False \ No newline at end of file From 05c452413c132b1094f4c27a9ab85b0185730900 Mon Sep 17 00:00:00 2001 From: Johnny Date: Tue, 20 Oct 2020 21:01:20 +0000 Subject: [PATCH 7/7] Changes test IPs to non-routable values, clears from throttle at end of test and adds checks to ensure manual key eviction works. --- evennia/server/tests/test_misc.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/evennia/server/tests/test_misc.py b/evennia/server/tests/test_misc.py index 3e0672ba9f..574c0cb6f1 100644 --- a/evennia/server/tests/test_misc.py +++ b/evennia/server/tests/test_misc.py @@ -90,8 +90,8 @@ class ThrottleTest(EvenniaTest): """ def test_throttle(self): - ips = ("94.100.176.153", "45.56.148.77", "5.196.1.129") - kwargs = {"limit": 5, "timeout": 15 * 60} + ips = ("256.256.256.257", "257.257.257.257", "258.258.258.258") + kwargs = {"name": "testing", "limit": 5, "timeout": 15 * 60} throttle = Throttle(**kwargs) @@ -124,3 +124,14 @@ class ThrottleTest(EvenniaTest): # There should only be (cache_size * num_ips) total in the Throttle cache self.assertEqual(sum([len(cache[x]) for x in cache.keys()]), throttle.cache_size * len(ips)) + + # Make sure the cache is populated + self.assertTrue(throttle.get()) + + # Remove the test IPs from the throttle cache + # (in case persistent storage was configured by the user) + for ip in ips: + self.assertTrue(throttle.remove(ip)) + + # Make sure the cache is empty + self.assertFalse(throttle.get()) \ No newline at end of file