From a794a518854c4d5d358a0e0b533ec43f573301b0 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Tue, 31 Dec 2024 07:57:54 +0100 Subject: [PATCH] fix failed_login cache, improve coding --- DOCUMENTATION.md | 10 ++++-- radicale/auth/__init__.py | 72 ++++++++++++++++++++++++++++----------- radicale/config.py | 8 +++-- 3 files changed, 67 insertions(+), 23 deletions(-) diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index 640025fe..dc31d9b1 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -814,16 +814,22 @@ Default: `none` ##### cache_logins -Cache successful logins until expiration time +Cache successful/failed logins until expiration time Default: `false` -##### cache_logins_expiry +##### cache_successful_logins_expiry Expiration time of caching successful logins in seconds Default: `5` +##### cache_failed_logins_expiry + +Expiration time of caching failed logins in seconds + +Default: `60` + ##### htpasswd_filename Path to the htpasswd file. diff --git a/radicale/auth/__init__.py b/radicale/auth/__init__.py index 39e07026..e9640f30 100644 --- a/radicale/auth/__init__.py +++ b/radicale/auth/__init__.py @@ -59,10 +59,12 @@ class BaseAuth: _lc_username: bool _uc_username: bool _strip_domain: bool - _cache: dict _cache_logins: bool - _cache_logins_expiry: int - _cache_logins_expiry_ns: int + _cache_successful: dict # login -> (digest, time_ns) + _cache_successful_logins_expiry: int + _cache_failed: dict # digest_failed -> (time_ns) + _cache_failed_logins_expiry: int + _cache_failed_logins_salt_ns: int # persistent over runtime def __init__(self, configuration: "config.Configuration") -> None: """Initialize BaseAuth. @@ -77,19 +79,25 @@ class BaseAuth: self._uc_username = configuration.get("auth", "uc_username") self._strip_domain = configuration.get("auth", "strip_domain") self._cache_logins = configuration.get("auth", "cache_logins") - self._cache_logins_expiry = configuration.get("auth", "cache_logins_expiry") - if self._cache_logins_expiry < 0: - raise RuntimeError("self._cache_logins_expiry cannot be < 0") + self._cache_successful_logins_expiry = configuration.get("auth", "cache_successful_logins_expiry") + if self._cache_successful_logins_expiry < 0: + raise RuntimeError("self._cache_successful_logins_expiry cannot be < 0") + self._cache_failed_logins_expiry = configuration.get("auth", "cache_failed_logins_expiry") + if self._cache_failed_logins_expiry < 0: + raise RuntimeError("self._cache_failed_logins_expiry cannot be < 0") logger.info("auth.strip_domain: %s", self._strip_domain) logger.info("auth.lc_username: %s", self._lc_username) logger.info("auth.uc_username: %s", self._uc_username) if self._lc_username is True and self._uc_username is True: raise RuntimeError("auth.lc_username and auth.uc_username cannot be enabled together") + # cache_successful_logins logger.info("auth.cache_logins: %s", self._cache_logins) + self._cache_successful = dict() + self._cache_failed = dict() + self._cache_failed_logins_salt_ns = time.time_ns() if self._cache_logins is True: - logger.info("auth.cache_logins_expiry: %s seconds", self._cache_logins_expiry) - self._cache_logins_expiry_ns = self._cache_logins_expiry * 1000 * 1000 * 1000 - self._cache = dict() + logger.info("auth.cache_successful_logins_expiry: %s seconds", self._cache_successful_logins_expiry) + logger.info("auth.cache_failed_logins_expiry: %s seconds", self._cache_failed_logins_expiry) def _cache_digest(self, login: str, password: str, salt: str) -> str: h = hashlib.sha3_512() @@ -137,31 +145,57 @@ class BaseAuth: result = "" digest = "" time_ns = time.time_ns() - if self._cache.get(login): - # entry found in cache - (digest_cache, time_ns_cache) = self._cache[login] + digest_failed = login + ":" + self._cache_digest(login, password, str(self._cache_failed_logins_salt_ns)) + if self._cache_failed.get(digest_failed): + # login+password found in cache "failed" + time_ns_cache = self._cache_failed[digest_failed] + age_failed = int((time_ns - time_ns_cache) / 1000 / 1000 / 1000) + if age_failed > self._cache_failed_logins_expiry: + logger.debug("Login failed cache entry for user+password found but expired: '%s' (age: %d > %d sec)", login, age_failed, self._cache_failed_logins_expiry) + # delete expired failed from cache + del self._cache_failed[digest_failed] + else: + # shortcut return + logger.debug("Login failed cache entry for user+password found: '%s' (age: %d sec)", login, age_failed) + return "" + if self._cache_successful.get(login): + # login found in cache "successful" + (digest_cache, time_ns_cache) = self._cache_successful[login] digest = self._cache_digest(login, password, str(time_ns_cache)) if digest == digest_cache: - if (time_ns - time_ns_cache) > self._cache_logins_expiry_ns: - logger.debug("Login cache entry for user found but expired: '%s'", login) + age_success = int((time_ns - time_ns_cache) / 1000 / 1000 / 1000) + if age_success > self._cache_successful_logins_expiry: + logger.debug("Login successful cache entry for user+password found but expired: '%s' (age: %d > %d sec)", login, age_success, self._cache_successful_logins_expiry) + # delete expired success from cache + del self._cache_successful[login] digest = "" else: - logger.debug("Login cache entry for user found: '%s'", login) + logger.debug("Login successful cache entry for user+password found: '%s' (age: %d sec)", login, age_success) result = login else: - logger.debug("Login cache entry for user not matching: '%s'", login) + logger.debug("Login successful cache entry for user+password not matching: '%s'", login) else: - # entry not found in cache, caculate always to avoid timing attacks + # login not found in cache, caculate always to avoid timing attacks digest = self._cache_digest(login, password, str(time_ns)) if result == "": + # verify login+password via configured backend + logger.debug("Login verification for user+password via backend: '%s'", login) result = self._login(login, password) if result != "": + logger.debug("Login successful for user+password via backend: '%s'", login) if digest == "": # successful login, but expired, digest must be recalculated digest = self._cache_digest(login, password, str(time_ns)) # store successful login in cache - self._cache[login] = (digest, time_ns) - logger.debug("Login cache for user set: '%s'", login) + self._cache_successful[login] = (digest, time_ns) + logger.debug("Login successful cache for user set: '%s'", login) + if self._cache_failed.get(digest_failed): + logger.debug("Login failed cache for user cleared: '%s'", login) + del self._cache_failed[digest_failed] + else: + logger.debug("Login failed for user+password via backend: '%s'", login) + self._cache_failed[digest_failed] = time_ns + logger.debug("Login failed cache for user set: '%s'", login) return result else: return self._login(login, password) diff --git a/radicale/config.py b/radicale/config.py index 486e9223..f71f312b 100644 --- a/radicale/config.py +++ b/radicale/config.py @@ -185,12 +185,16 @@ DEFAULT_CONFIG_SCHEMA: types.CONFIG_SCHEMA = OrderedDict([ "internal": auth.INTERNAL_TYPES}), ("cache_logins", { "value": "false", - "help": "cache successful logins for until expiration time", + "help": "cache successful/failed logins for until expiration time", "type": bool}), - ("cache_logins_expiry", { + ("cache_successful_logins_expiry", { "value": "5", "help": "expiration time for caching successful logins in seconds", "type": int}), + ("cache_failed_logins_expiry", { + "value": "60", + "help": "expiration time for caching failed logins in seconds", + "type": int}), ("htpasswd_filename", { "value": "/etc/radicale/users", "help": "htpasswd filename",