From 74311560c9e8d62fcaefb7e6b8c7c03dc2958786 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Mon, 30 Dec 2024 08:16:45 +0100 Subject: [PATCH 01/36] add cache_logins* options --- DOCUMENTATION.md | 12 ++++++++++++ config | 6 ++++++ radicale/config.py | 8 ++++++++ 3 files changed, 26 insertions(+) diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index f590a294..640025fe 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -812,6 +812,18 @@ Available backends: Default: `none` +##### cache_logins + +Cache successful logins until expiration time + +Default: `false` + +##### cache_logins_expiry + +Expiration time of caching successful logins in seconds + +Default: `5` + ##### htpasswd_filename Path to the htpasswd file. diff --git a/config b/config index c34b9d28..429cfca1 100644 --- a/config +++ b/config @@ -62,6 +62,12 @@ # Value: none | htpasswd | remote_user | http_x_remote_user | ldap | denyall #type = none +# Cache successful logins for until expiration time +#cache_logins = false + +# Expiration time for caching successful logins in seconds +#cache_logins_expiry = 5 + # URI to the LDAP server #ldap_uri = ldap://localhost diff --git a/radicale/config.py b/radicale/config.py index 0ac5970c..486e9223 100644 --- a/radicale/config.py +++ b/radicale/config.py @@ -183,6 +183,14 @@ def json_str(value: Any) -> dict: "help": "authentication method", "type": str_or_callable, "internal": auth.INTERNAL_TYPES}), + ("cache_logins", { + "value": "false", + "help": "cache successful logins for until expiration time", + "type": bool}), + ("cache_logins_expiry", { + "value": "5", + "help": "expiration time for caching successful logins in seconds", + "type": int}), ("htpasswd_filename", { "value": "/etc/radicale/users", "help": "htpasswd filename", From 8e97b709bf5cb4fb09ba6fe42113ea965a07b57d Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Mon, 30 Dec 2024 08:17:15 +0100 Subject: [PATCH 02/36] implement cache_logins* option --- radicale/auth/__init__.py | 56 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/radicale/auth/__init__.py b/radicale/auth/__init__.py index 812649c5..9cb70bb8 100644 --- a/radicale/auth/__init__.py +++ b/radicale/auth/__init__.py @@ -29,6 +29,8 @@ """ +import hashlib +import time from typing import Sequence, Set, Tuple, Union, final from radicale import config, types, utils @@ -57,6 +59,10 @@ 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 def __init__(self, configuration: "config.Configuration") -> None: """Initialize BaseAuth. @@ -70,11 +76,27 @@ def __init__(self, configuration: "config.Configuration") -> None: self._lc_username = configuration.get("auth", "lc_username") 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") 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") + logger.info("auth.cache_logins: %s", self._cache_logins) + 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() + + def _cache_digest(self, login: str, password: str, salt: str) -> str: + h = hashlib.sha3_512() + h.update(salt.encode()) + h.update(login.encode()) + h.update(password.encode()) + return h.digest() def get_external_login(self, environ: types.WSGIEnviron) -> Union[ Tuple[()], Tuple[str, str]]: @@ -110,4 +132,36 @@ def login(self, login: str, password: str) -> str: login = login.upper() if self._strip_domain: login = login.split('@')[0] - return self._login(login, password) + if self._cache_logins is True: + # time_ns is also used as salt + 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 = 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) + digest = "" + else: + logger.debug("Login cache entry for user found: '%s'", login) + result = login + else: + logger.debug("Login cache entry for user not matching: '%s'", login) + else: + # entry not found in cache, caculate always to avoid timing attacks + digest = self._cache_digest(login, password, str(time_ns)) + if result == "": + result = self._login(login, password) + if result is not "": + if digest is "": + # 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) + return result + else: + return self._login(login, password) From ddd099accd311236c0c3b4be9fb32cb4c15a8c19 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Mon, 30 Dec 2024 08:17:44 +0100 Subject: [PATCH 03/36] debug log which password hash method was used --- radicale/auth/htpasswd.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py index 7422e16d..43fee1b9 100644 --- a/radicale/auth/htpasswd.py +++ b/radicale/auth/htpasswd.py @@ -96,19 +96,19 @@ def __init__(self, configuration: config.Configuration) -> None: def _plain(self, hash_value: str, password: str) -> bool: """Check if ``hash_value`` and ``password`` match, plain method.""" - return hmac.compare_digest(hash_value.encode(), password.encode()) + return ("PLAIN", hmac.compare_digest(hash_value.encode(), password.encode())) def _bcrypt(self, bcrypt: Any, hash_value: str, password: str) -> bool: - return bcrypt.checkpw(password=password.encode('utf-8'), hashed_password=hash_value.encode()) + return ("BCRYPT", bcrypt.checkpw(password=password.encode('utf-8'), hashed_password=hash_value.encode())) def _md5apr1(self, hash_value: str, password: str) -> bool: - return apr_md5_crypt.verify(password, hash_value.strip()) + return ("MD5-APR1", apr_md5_crypt.verify(password, hash_value.strip())) def _sha256(self, hash_value: str, password: str) -> bool: - return sha256_crypt.verify(password, hash_value.strip()) + return ("SHA-256", sha256_crypt.verify(password, hash_value.strip())) def _sha512(self, hash_value: str, password: str) -> bool: - return sha512_crypt.verify(password, hash_value.strip()) + return ("SHA-512", sha512_crypt.verify(password, hash_value.strip())) def _autodetect(self, hash_value: str, password: str) -> bool: if hash_value.startswith("$apr1$", 0, 6) and len(hash_value) == 37: @@ -151,8 +151,9 @@ def _login(self, login: str, password: str) -> str: # timing attacks, see #591. login_ok = hmac.compare_digest( hash_login.encode(), login.encode()) - password_ok = self._verify(hash_value, password) + (method, password_ok) = self._verify(hash_value, password) if login_ok and password_ok: + logger.debug("Password verification for user '%s' with method '%s': password_ok=%s", login, method, password_ok) return login except ValueError as e: raise RuntimeError("Invalid htpasswd file %r: %s" % From 30e2ab490ef1e896a3687ca49eaccc69b819cfb2 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Mon, 30 Dec 2024 08:19:20 +0100 Subject: [PATCH 04/36] cache_logins+htpasswd --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88cb2e1e..353a4e68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # Changelog ## 3.3.4.dev +* Add: option [auth] cache_logins/cache_logins_expiry for caching successful logins +* Improve: log used hash method on debug for htpasswd authentication ## 3.3.3 * Add: display mtime_ns precision of storage folder with condition warning if too less From 9af15e6656f3fb28916726e4be419a798d80d078 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Mon, 30 Dec 2024 05:25:10 +0100 Subject: [PATCH 05/36] fixes triggered by tox --- radicale/auth/__init__.py | 6 +++--- radicale/auth/htpasswd.py | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/radicale/auth/__init__.py b/radicale/auth/__init__.py index 9cb70bb8..39e07026 100644 --- a/radicale/auth/__init__.py +++ b/radicale/auth/__init__.py @@ -96,7 +96,7 @@ def _cache_digest(self, login: str, password: str, salt: str) -> str: h.update(salt.encode()) h.update(login.encode()) h.update(password.encode()) - return h.digest() + return str(h.digest()) def get_external_login(self, environ: types.WSGIEnviron) -> Union[ Tuple[()], Tuple[str, str]]: @@ -155,8 +155,8 @@ def login(self, login: str, password: str) -> str: digest = self._cache_digest(login, password, str(time_ns)) if result == "": result = self._login(login, password) - if result is not "": - if digest is "": + if result != "": + if digest == "": # successful login, but expired, digest must be recalculated digest = self._cache_digest(login, password, str(time_ns)) # store successful login in cache diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py index 43fee1b9..1767c9e1 100644 --- a/radicale/auth/htpasswd.py +++ b/radicale/auth/htpasswd.py @@ -94,23 +94,23 @@ def __init__(self, configuration: config.Configuration) -> None: raise RuntimeError("The htpasswd encryption method %r is not " "supported." % encryption) - def _plain(self, hash_value: str, password: str) -> bool: + def _plain(self, hash_value: str, password: str) -> tuple[str, bool]: """Check if ``hash_value`` and ``password`` match, plain method.""" return ("PLAIN", hmac.compare_digest(hash_value.encode(), password.encode())) - def _bcrypt(self, bcrypt: Any, hash_value: str, password: str) -> bool: + def _bcrypt(self, bcrypt: Any, hash_value: str, password: str) -> tuple[str, bool]: return ("BCRYPT", bcrypt.checkpw(password=password.encode('utf-8'), hashed_password=hash_value.encode())) - def _md5apr1(self, hash_value: str, password: str) -> bool: + def _md5apr1(self, hash_value: str, password: str) -> tuple[str, bool]: return ("MD5-APR1", apr_md5_crypt.verify(password, hash_value.strip())) - def _sha256(self, hash_value: str, password: str) -> bool: + def _sha256(self, hash_value: str, password: str) -> tuple[str, bool]: return ("SHA-256", sha256_crypt.verify(password, hash_value.strip())) - def _sha512(self, hash_value: str, password: str) -> bool: + def _sha512(self, hash_value: str, password: str) -> tuple[str, bool]: return ("SHA-512", sha512_crypt.verify(password, hash_value.strip())) - def _autodetect(self, hash_value: str, password: str) -> bool: + def _autodetect(self, hash_value: str, password: str) -> tuple[str, bool]: if hash_value.startswith("$apr1$", 0, 6) and len(hash_value) == 37: # MD5-APR1 return self._md5apr1(hash_value, password) From 4f2990342dc42f5b254b0cbec2c0d7f73d00b1b2 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Tue, 31 Dec 2024 07:57:13 +0100 Subject: [PATCH 06/36] add additional debug line --- radicale/auth/htpasswd.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py index 1767c9e1..f872eba5 100644 --- a/radicale/auth/htpasswd.py +++ b/radicale/auth/htpasswd.py @@ -155,6 +155,8 @@ def _login(self, login: str, password: str) -> str: if login_ok and password_ok: logger.debug("Password verification for user '%s' with method '%s': password_ok=%s", login, method, password_ok) return login + elif login_ok: + logger.debug("Password verification for user '%s' with method '%s': password_ok=%s", login, method, password_ok) except ValueError as e: raise RuntimeError("Invalid htpasswd file %r: %s" % (self._filename, e)) from e From a794a518854c4d5d358a0e0b533ec43f573301b0 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Tue, 31 Dec 2024 07:57:54 +0100 Subject: [PATCH 07/36] 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 @@ def __init__(self, configuration: "config.Configuration") -> None: 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 @@ def login(self, login: str, password: str) -> str: 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 @@ def json_str(value: Any) -> dict: "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", From b75e303556842347761840e7a746020998d46808 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Tue, 31 Dec 2024 08:11:19 +0100 Subject: [PATCH 08/36] reorg code, disable caching on not required types --- radicale/auth/__init__.py | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/radicale/auth/__init__.py b/radicale/auth/__init__.py index e9640f30..1e9d0f2f 100644 --- a/radicale/auth/__init__.py +++ b/radicale/auth/__init__.py @@ -59,6 +59,7 @@ class BaseAuth: _lc_username: bool _uc_username: bool _strip_domain: bool + _type: str _cache_logins: bool _cache_successful: dict # login -> (digest, time_ns) _cache_successful_logins_expiry: int @@ -78,26 +79,32 @@ def __init__(self, configuration: "config.Configuration") -> None: self._lc_username = configuration.get("auth", "lc_username") 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_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() + self._cache_logins = configuration.get("auth", "cache_logins") + self._type = configuration.get("auth", "type") + if (self._type in [ "dovecot", "ldap", "htpasswd" ]) or (self._cache_logins is False): + logger.info("auth.cache_logins: %s", self._cache_logins) + else: + logger.info("auth.cache_logins: %s (but not required for type '%s' and disabled therefore)", self._cache_logins, self._type) + self._cache_logins = False if self._cache_logins is True: + 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.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) + # cache init + self._cache_successful = dict() + self._cache_failed = dict() + self._cache_failed_logins_salt_ns = time.time_ns() def _cache_digest(self, login: str, password: str, salt: str) -> str: h = hashlib.sha3_512() From c0acbd4402b06269f89cbc8622e3ab38b128277f Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Tue, 31 Dec 2024 08:12:49 +0100 Subject: [PATCH 09/36] update changelog --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 353a4e68..e9836cd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,8 @@ # Changelog ## 3.3.4.dev -* Add: option [auth] cache_logins/cache_logins_expiry for caching successful logins -* Improve: log used hash method on debug for htpasswd authentication +* Add: option [auth] cache_logins/cache_successful_logins_expiry/cache_failed_logins for caching logins +* Improve: log used hash method and result on debug for htpasswd authentication ## 3.3.3 * Add: display mtime_ns precision of storage folder with condition warning if too less From 79ba07e16b1955f795f75c5e3d4a9f4a84f3bdb3 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Tue, 31 Dec 2024 16:13:05 +0100 Subject: [PATCH 10/36] change default cache times --- DOCUMENTATION.md | 7 ++++--- config | 7 +++++-- radicale/config.py | 4 ++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index dc31d9b1..a5238bc6 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -814,7 +814,8 @@ Default: `none` ##### cache_logins -Cache successful/failed logins until expiration time +Cache successful/failed logins until expiration time. Enable this to avoid +overload of authentication backends. Default: `false` @@ -822,13 +823,13 @@ Default: `false` Expiration time of caching successful logins in seconds -Default: `5` +Default: `15` ##### cache_failed_logins_expiry Expiration time of caching failed logins in seconds -Default: `60` +Default: `90` ##### htpasswd_filename diff --git a/config b/config index 429cfca1..9ac082cf 100644 --- a/config +++ b/config @@ -62,11 +62,14 @@ # Value: none | htpasswd | remote_user | http_x_remote_user | ldap | denyall #type = none -# Cache successful logins for until expiration time +# Cache logins for until expiration time #cache_logins = false # Expiration time for caching successful logins in seconds -#cache_logins_expiry = 5 +#cache_successful_logins_expiry = 15 + +## Expiration time of caching failed logins in seconds +#cache_failed_logins_expiry = 90 # URI to the LDAP server #ldap_uri = ldap://localhost diff --git a/radicale/config.py b/radicale/config.py index f71f312b..b165345f 100644 --- a/radicale/config.py +++ b/radicale/config.py @@ -188,11 +188,11 @@ def json_str(value: Any) -> dict: "help": "cache successful/failed logins for until expiration time", "type": bool}), ("cache_successful_logins_expiry", { - "value": "5", + "value": "15", "help": "expiration time for caching successful logins in seconds", "type": int}), ("cache_failed_logins_expiry", { - "value": "60", + "value": "90", "help": "expiration time for caching failed logins in seconds", "type": int}), ("htpasswd_filename", { From 5ce0cee8bfee63b05b318b90d2bd872abe9bce48 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Tue, 31 Dec 2024 16:13:52 +0100 Subject: [PATCH 11/36] add chache cleanup and locking --- radicale/auth/__init__.py | 46 +++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/radicale/auth/__init__.py b/radicale/auth/__init__.py index 1e9d0f2f..d8f35e83 100644 --- a/radicale/auth/__init__.py +++ b/radicale/auth/__init__.py @@ -31,6 +31,7 @@ import hashlib import time +import threading from typing import Sequence, Set, Tuple, Union, final from radicale import config, types, utils @@ -63,9 +64,10 @@ class BaseAuth: _cache_logins: bool _cache_successful: dict # login -> (digest, time_ns) _cache_successful_logins_expiry: int - _cache_failed: dict # digest_failed -> (time_ns) + _cache_failed: dict # digest_failed -> (time_ns, login) _cache_failed_logins_expiry: int _cache_failed_logins_salt_ns: int # persistent over runtime + _lock: threading.Lock def __init__(self, configuration: "config.Configuration") -> None: """Initialize BaseAuth. @@ -105,6 +107,7 @@ def __init__(self, configuration: "config.Configuration") -> None: self._cache_successful = dict() self._cache_failed = dict() self._cache_failed_logins_salt_ns = time.time_ns() + self._lock = threading.Lock() def _cache_digest(self, login: str, password: str, salt: str) -> str: h = hashlib.sha3_512() @@ -152,19 +155,34 @@ def login(self, login: str, password: str) -> str: result = "" digest = "" time_ns = time.time_ns() + # cleanup failed login cache to avoid out-of-memory + cache_failed_entries = len(self._cache_failed) + if cache_failed_entries > 0: + logger.debug("Login failed cache investigation start (entries: %d)", cache_failed_entries) + self._lock.acquire() + cache_failed_cleanup = dict() + for digest in self._cache_failed: + (time_ns_cache, login_cache) = self._cache_failed[digest] + age_failed = int((time_ns - time_ns_cache) / 1000 / 1000 / 1000) + if age_failed > self._cache_failed_logins_expiry: + cache_failed_cleanup[digest] = (login_cache, age_failed) + cache_failed_cleanup_entries = len(cache_failed_cleanup) + logger.debug("Login failed cache cleanup start (entries: %d)", cache_failed_cleanup_entries) + if cache_failed_cleanup_entries > 0: + for digest in cache_failed_cleanup: + (login, age_failed) = cache_failed_cleanup[digest] + logger.debug("Login failed cache entry for user+password expired: '%s' (age: %d > %d sec)", login_cache, age_failed, self._cache_failed_logins_expiry) + del self._cache_failed[digest] + self._lock.release() + logger.debug("Login failed cache investigation finished") + # check for cache failed 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] + # login+password found in cache "failed" -> shortcut return + (time_ns_cache, login_cache) = self._cache_failed[digest] 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 "" + logger.debug("Login failed cache entry for user+password found: '%s' (age: %d sec)", login_cache, age_failed) + return "" if self._cache_successful.get(login): # login found in cache "successful" (digest_cache, time_ns_cache) = self._cache_successful[login] @@ -194,14 +212,18 @@ def login(self, login: str, password: str) -> str: # successful login, but expired, digest must be recalculated digest = self._cache_digest(login, password, str(time_ns)) # store successful login in cache + self._lock.acquire() self._cache_successful[login] = (digest, time_ns) + self._lock.release() 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 + self._lock.acquire() + self._cache_failed[digest_failed] = (time_ns, login) + self._lock.release() logger.debug("Login failed cache for user set: '%s'", login) return result else: From 2489356dda05af19754702f58e0ddc600754dc1b Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Tue, 31 Dec 2024 16:14:38 +0100 Subject: [PATCH 12/36] implement htpasswd file caching --- radicale/auth/htpasswd.py | 126 ++++++++++++++++++++++++++++++-------- 1 file changed, 100 insertions(+), 26 deletions(-) diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py index f872eba5..24cf742f 100644 --- a/radicale/auth/htpasswd.py +++ b/radicale/auth/htpasswd.py @@ -48,8 +48,11 @@ """ +import os +import time import functools import hmac +import threading from typing import Any from passlib.hash import apr_md5_crypt, sha256_crypt, sha512_crypt @@ -61,15 +64,26 @@ class Auth(auth.BaseAuth): _filename: str _encoding: str + _htpasswd: dict # login -> digest + _htpasswd_mtime_ns: int + _htpasswd_size: bytes + _htpasswd_ok: bool + _htpasswd_not_ok_seconds: int + _htpasswd_not_ok_reminder_seconds: int + _lock: threading.Lock def __init__(self, configuration: config.Configuration) -> None: super().__init__(configuration) self._filename = configuration.get("auth", "htpasswd_filename") self._encoding = configuration.get("encoding", "stock") encryption: str = configuration.get("auth", "htpasswd_encryption") - logger.info("auth htpasswd encryption is 'radicale.auth.htpasswd_encryption.%s'", encryption) + self._htpasswd_ok = False + self._htpasswd_not_ok_reminder_seconds = 60 # currently hardcoded + self._htpasswd_read = self._read_htpasswd(True) + self._lock = threading.Lock() + if encryption == "plain": self._verify = self._plain elif encryption == "md5": @@ -127,40 +141,100 @@ def _autodetect(self, hash_value: str, password: str) -> tuple[str, bool]: # assumed plaintext return self._plain(hash_value, password) - def _login(self, login: str, password: str) -> str: - """Validate credentials. + def _read_htpasswd(self, init: bool) -> bool: + """Read htpasswd file - Iterate through htpasswd credential file until login matches, extract - hash (encrypted password) and check hash against password, - using the method specified in the Radicale config. - - The content of the file is not cached because reading is generally a - very cheap operation, and it's useful to get live updates of the - htpasswd file. + init == True: stop on error + init == False: warn/skip on error and set mark to log reminder every interval """ + htpasswd_ok = True + if init is True: + info = "Read" + else: + info = "Re-read" + logger.info("%s content of htpasswd file start: %r", info, self._filename) + htpasswd = dict() try: with open(self._filename, encoding=self._encoding) as f: + line_num = 0 + entries = 0 + duplicates = 0 for line in f: + line_num += 1 line = line.rstrip("\n") if line.lstrip() and not line.lstrip().startswith("#"): try: - hash_login, hash_value = line.split( - ":", maxsplit=1) - # Always compare both login and password to avoid - # timing attacks, see #591. - login_ok = hmac.compare_digest( - hash_login.encode(), login.encode()) - (method, password_ok) = self._verify(hash_value, password) - if login_ok and password_ok: - logger.debug("Password verification for user '%s' with method '%s': password_ok=%s", login, method, password_ok) - return login - elif login_ok: - logger.debug("Password verification for user '%s' with method '%s': password_ok=%s", login, method, password_ok) + login, digest = line.split( ":", maxsplit=1) + if login == "" or digest == "": + if init is True: + raise ValueError("htpasswd file contains problematic line not matching : in line: %d" % line_num) + else: + logger.warning("htpasswd file contains problematic line not matching : in line: %d (ignored)", line_num) + htpasswd_ok = False + else: + if htpasswd.get(login): + duplicates += 1 + if init is True: + raise ValueError("htpasswd file contains duplicate login: '%s'", login, line_num) + else: + logger.warning("htpasswd file contains duplicate login: '%s' (line: %d / ignored)", login, line_num) + htpasswd_ok = False + else: + htpasswd[login] = digest + entries += 1 except ValueError as e: - raise RuntimeError("Invalid htpasswd file %r: %s" % - (self._filename, e)) from e + if init is True: + raise RuntimeError("Invalid htpasswd file %r: %s" % (self._filename, e)) from e except OSError as e: - raise RuntimeError("Failed to load htpasswd file %r: %s" % - (self._filename, e)) from e + if init is True: + raise RuntimeError("Failed to load htpasswd file %r: %s" % (self._filename, e)) from e + else: + logger.warning("Failed to load htpasswd file on re-read: %r" % (self._filename, e)) + htpasswd_ok = False + else: + self._htpasswd_size = os.stat(self._filename).st_size + self._htpasswd_time_ns = os.stat(self._filename).st_mtime_ns + self._htpasswd = htpasswd + logger.info("%s content of htpasswd file done: %r (entries: %d, duplicates: %d)", info, self._filename, entries, duplicates) + if htpasswd_ok is True: + self._htpasswd_not_ok_time = 0 + else: + self._htpasswd_not_ok_time = time.time() + return htpasswd_ok + + def _login(self, login: str, password: str) -> str: + """Validate credentials. + + Iterate through htpasswd credential file until login matches, extract + hash (encrypted password) and check hash against password, + using the method specified in the Radicale config. + + The content of the file is cached and live updates will be detected by + comparing mtime_ns and size + + """ + # check and re-read file if required + htpasswd_size = os.stat(self._filename).st_size + htpasswd_time_ns = os.stat(self._filename).st_mtime_ns + if (htpasswd_size != self._htpasswd_size) or (htpasswd_time_ns != self._htpasswd_time_ns): + with self._lock: + self._htpasswd_ok = self._read_htpasswd(False) + else: + # log reminder of problemantic file every interval + if (self._htpasswd_ok is False) and (self._htpasswd_not_ok_time > 0): + current_time = time.time() + if (current_time - self._htpasswd_not_ok_time) > self._htpasswd_not_ok_reminder_seconds: + logger.warning("htpasswd file still contains issues (REMINDER, check warnings in the past): %r" % self._filename) + self._htpasswd_not_ok_time = current_time + if self._htpasswd.get(login): + digest = self._htpasswd[login] + (method, password_ok) = self._verify(digest, password) + logger.debug("Login verification successful for user: '%s' (method '%s')", login, method) + if password_ok: + return login + else: + logger.debug("Login verification failed for user: '%s' ( method '%s')", login, method) + else: + logger.debug("Login verification user not found: '%s'", login) return "" From 9cac3008b7170441924aa5ad583a89c5949cf99e Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Tue, 31 Dec 2024 16:15:51 +0100 Subject: [PATCH 13/36] extend changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9836cd1..505202ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## 3.3.4.dev * Add: option [auth] cache_logins/cache_successful_logins_expiry/cache_failed_logins for caching logins * Improve: log used hash method and result on debug for htpasswd authentication +* Improve: htpasswd file now read and verified on start, automatic re-read triggered on change (mtime or size) ## 3.3.3 * Add: display mtime_ns precision of storage folder with condition warning if too less From 5357e692d9af661b35f69c47cced0bb3f648a490 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Tue, 31 Dec 2024 17:09:21 +0100 Subject: [PATCH 14/36] [auth] htpasswd: module 'bcrypt' is no longer mandatory in case digest method not used in file --- radicale/auth/htpasswd.py | 44 +++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py index 24cf742f..a5f46f93 100644 --- a/radicale/auth/htpasswd.py +++ b/radicale/auth/htpasswd.py @@ -70,6 +70,8 @@ class Auth(auth.BaseAuth): _htpasswd_ok: bool _htpasswd_not_ok_seconds: int _htpasswd_not_ok_reminder_seconds: int + _htpasswd_bcrypt_use: int + _has_bcrypt: bool _lock: threading.Lock def __init__(self, configuration: config.Configuration) -> None: @@ -79,9 +81,10 @@ def __init__(self, configuration: config.Configuration) -> None: encryption: str = configuration.get("auth", "htpasswd_encryption") logger.info("auth htpasswd encryption is 'radicale.auth.htpasswd_encryption.%s'", encryption) + self._has_bcrypt = False self._htpasswd_ok = False self._htpasswd_not_ok_reminder_seconds = 60 # currently hardcoded - self._htpasswd_read = self._read_htpasswd(True) + (self._htpasswd_ok, self._htpasswd_bcrypt_use) = self._read_htpasswd(True) self._lock = threading.Lock() if encryption == "plain": @@ -96,14 +99,24 @@ def __init__(self, configuration: config.Configuration) -> None: try: import bcrypt except ImportError as e: - raise RuntimeError( - "The htpasswd encryption method 'bcrypt' or 'autodetect' requires " - "the bcrypt module.") from e + if (encryption == "autodetect") and (self._htpasswd_bcrypt_use == 0): + logger.warning("auth htpasswd encryption is 'radicale.auth.htpasswd_encryption.%s' which can require bycrypt module, but currently no entries found", encryption) + else: + raise RuntimeError( + "The htpasswd encryption method 'bcrypt' or 'autodetect' requires " + "the bcrypt module (entries found: %d)." % self._htpasswd_bcrypt_use) from e + else: + if encryption == "autodetect": + if self._htpasswd_bcrypt_use == 0: + logger.info("auth htpasswd encryption is 'radicale.auth.htpasswd_encryption.%s' and bycrypt module found, but currently not required", encryption) + else: + logger.info("auth htpasswd encryption is 'radicale.auth.htpasswd_encryption.%s' and bycrypt module found (entries found: %d)", encryption, self._htpasswd_bcrypt_use) if encryption == "bcrypt": self._verify = functools.partial(self._bcrypt, bcrypt) else: self._verify = self._autodetect self._verify_bcrypt = functools.partial(self._bcrypt, bcrypt) + self._has_bcrypt = True else: raise RuntimeError("The htpasswd encryption method %r is not " "supported." % encryption) @@ -141,7 +154,7 @@ def _autodetect(self, hash_value: str, password: str) -> tuple[str, bool]: # assumed plaintext return self._plain(hash_value, password) - def _read_htpasswd(self, init: bool) -> bool: + def _read_htpasswd(self, init: bool) -> (bool, int): """Read htpasswd file init == True: stop on error @@ -149,6 +162,7 @@ def _read_htpasswd(self, init: bool) -> bool: """ htpasswd_ok = True + bcrypt_use = 0 if init is True: info = "Read" else: @@ -166,12 +180,14 @@ def _read_htpasswd(self, init: bool) -> bool: if line.lstrip() and not line.lstrip().startswith("#"): try: login, digest = line.split( ":", maxsplit=1) + skip = False if login == "" or digest == "": if init is True: raise ValueError("htpasswd file contains problematic line not matching : in line: %d" % line_num) else: logger.warning("htpasswd file contains problematic line not matching : in line: %d (ignored)", line_num) htpasswd_ok = False + skip = True else: if htpasswd.get(login): duplicates += 1 @@ -180,9 +196,19 @@ def _read_htpasswd(self, init: bool) -> bool: else: logger.warning("htpasswd file contains duplicate login: '%s' (line: %d / ignored)", login, line_num) htpasswd_ok = False + skip = True else: - htpasswd[login] = digest - entries += 1 + if digest.startswith("$2y$", 0, 4) and len(digest) == 60: + if init is True: + bcrypt_use += 1 + else: + if self._has_bcrypt is False: + logger.warning("htpasswd file contains bcrypt digest login: '%s' (line: %d / ignored because module is not loaded)", login, line_num) + skip = True + htpasswd_ok = False + if skip is False: + htpasswd[login] = digest + entries += 1 except ValueError as e: if init is True: raise RuntimeError("Invalid htpasswd file %r: %s" % (self._filename, e)) from e @@ -201,7 +227,7 @@ def _read_htpasswd(self, init: bool) -> bool: self._htpasswd_not_ok_time = 0 else: self._htpasswd_not_ok_time = time.time() - return htpasswd_ok + return (htpasswd_ok, bcrypt_use) def _login(self, login: str, password: str) -> str: """Validate credentials. @@ -219,7 +245,7 @@ def _login(self, login: str, password: str) -> str: htpasswd_time_ns = os.stat(self._filename).st_mtime_ns if (htpasswd_size != self._htpasswd_size) or (htpasswd_time_ns != self._htpasswd_time_ns): with self._lock: - self._htpasswd_ok = self._read_htpasswd(False) + (self._htpasswd_ok, self._htpasswd_bcrypt_use) = self._read_htpasswd(False) else: # log reminder of problemantic file every interval if (self._htpasswd_ok is False) and (self._htpasswd_not_ok_time > 0): From c00ab76c831488850c48ed9cea3818b0a0fef7b6 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Tue, 31 Dec 2024 17:09:29 +0100 Subject: [PATCH 15/36] [auth] htpasswd: module 'bcrypt' is no longer mandatory in case digest method not used in file / changelog --- CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 505202ed..4d8bbaa1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,9 @@ ## 3.3.4.dev * Add: option [auth] cache_logins/cache_successful_logins_expiry/cache_failed_logins for caching logins -* Improve: log used hash method and result on debug for htpasswd authentication -* Improve: htpasswd file now read and verified on start, automatic re-read triggered on change (mtime or size) +* Improve: [auth] log used hash method and result on debug for htpasswd authentication +* Improve: [auth] htpasswd file now read and verified on start, automatic re-read triggered on change (mtime or size) +* Improve: [auth] htpasswd: module 'bcrypt' is no longer mandatory in case digest method not used in file ## 3.3.3 * Add: display mtime_ns precision of storage folder with condition warning if too less From c1be04abd1a3c23ffe8bac5982a04076e8dec5ff Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Tue, 31 Dec 2024 18:26:43 +0100 Subject: [PATCH 16/36] fixes suggested by tox --- radicale/auth/__init__.py | 4 ++-- radicale/auth/htpasswd.py | 17 +++++++++-------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/radicale/auth/__init__.py b/radicale/auth/__init__.py index d8f35e83..fc453a84 100644 --- a/radicale/auth/__init__.py +++ b/radicale/auth/__init__.py @@ -30,8 +30,8 @@ """ import hashlib -import time import threading +import time from typing import Sequence, Set, Tuple, Union, final from radicale import config, types, utils @@ -89,7 +89,7 @@ def __init__(self, configuration: "config.Configuration") -> None: # cache_successful_logins self._cache_logins = configuration.get("auth", "cache_logins") self._type = configuration.get("auth", "type") - if (self._type in [ "dovecot", "ldap", "htpasswd" ]) or (self._cache_logins is False): + if (self._type in ["dovecot", "ldap", "htpasswd"]) or (self._cache_logins is False): logger.info("auth.cache_logins: %s", self._cache_logins) else: logger.info("auth.cache_logins: %s (but not required for type '%s' and disabled therefore)", self._cache_logins, self._type) diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py index a5f46f93..e4c420cd 100644 --- a/radicale/auth/htpasswd.py +++ b/radicale/auth/htpasswd.py @@ -48,12 +48,12 @@ """ -import os -import time import functools import hmac +import os import threading -from typing import Any +import time +from typing import Any, Tuple from passlib.hash import apr_md5_crypt, sha256_crypt, sha512_crypt @@ -66,9 +66,9 @@ class Auth(auth.BaseAuth): _encoding: str _htpasswd: dict # login -> digest _htpasswd_mtime_ns: int - _htpasswd_size: bytes + _htpasswd_size: int _htpasswd_ok: bool - _htpasswd_not_ok_seconds: int + _htpasswd_not_ok_time: float _htpasswd_not_ok_reminder_seconds: int _htpasswd_bcrypt_use: int _has_bcrypt: bool @@ -154,7 +154,7 @@ def _autodetect(self, hash_value: str, password: str) -> tuple[str, bool]: # assumed plaintext return self._plain(hash_value, password) - def _read_htpasswd(self, init: bool) -> (bool, int): + def _read_htpasswd(self, init: bool) -> Tuple[bool, int]: """Read htpasswd file init == True: stop on error @@ -168,6 +168,7 @@ def _read_htpasswd(self, init: bool) -> (bool, int): else: info = "Re-read" logger.info("%s content of htpasswd file start: %r", info, self._filename) + htpasswd: dict[str, str] htpasswd = dict() try: with open(self._filename, encoding=self._encoding) as f: @@ -179,7 +180,7 @@ def _read_htpasswd(self, init: bool) -> (bool, int): line = line.rstrip("\n") if line.lstrip() and not line.lstrip().startswith("#"): try: - login, digest = line.split( ":", maxsplit=1) + login, digest = line.split(":", maxsplit=1) skip = False if login == "" or digest == "": if init is True: @@ -216,7 +217,7 @@ def _read_htpasswd(self, init: bool) -> (bool, int): if init is True: raise RuntimeError("Failed to load htpasswd file %r: %s" % (self._filename, e)) from e else: - logger.warning("Failed to load htpasswd file on re-read: %r" % (self._filename, e)) + logger.warning("Failed to load htpasswd file on re-read: %r" % self._filename) htpasswd_ok = False else: self._htpasswd_size = os.stat(self._filename).st_size From 6ebca084237fe315ab5fe78831709b80300aeb5c Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Wed, 1 Jan 2025 15:47:22 +0100 Subject: [PATCH 17/36] extend copyright --- radicale/app/__init__.py | 2 +- radicale/auth/__init__.py | 2 +- radicale/auth/htpasswd.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/radicale/app/__init__.py b/radicale/app/__init__.py index 4f11ad3f..28c98802 100644 --- a/radicale/app/__init__.py +++ b/radicale/app/__init__.py @@ -3,7 +3,7 @@ # Copyright © 2008 Pascal Halter # Copyright © 2008-2017 Guillaume Ayoub # Copyright © 2017-2019 Unrud -# Copyright © 2024-2024 Peter Bieringer +# Copyright © 2024-2025 Peter Bieringer # # This library is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/radicale/auth/__init__.py b/radicale/auth/__init__.py index fc453a84..679ecf9d 100644 --- a/radicale/auth/__init__.py +++ b/radicale/auth/__init__.py @@ -3,7 +3,7 @@ # Copyright © 2008 Pascal Halter # Copyright © 2008-2017 Guillaume Ayoub # Copyright © 2017-2022 Unrud -# Copyright © 2024-2024 Peter Bieringer +# Copyright © 2024-2025 Peter Bieringer # # This library is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py index e4c420cd..8ed1ad33 100644 --- a/radicale/auth/htpasswd.py +++ b/radicale/auth/htpasswd.py @@ -3,7 +3,7 @@ # Copyright © 2008 Pascal Halter # Copyright © 2008-2017 Guillaume Ayoub # Copyright © 2017-2019 Unrud -# Copyright © 2024 Peter Bieringer +# Copyright © 2024-2025 Peter Bieringer # # This library is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by From c10ce7ae4661f2be8e46fd74db009d88e31f4c25 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Wed, 1 Jan 2025 16:30:34 +0100 Subject: [PATCH 18/36] add support for login info log --- radicale/app/__init__.py | 10 +++++----- radicale/auth/__init__.py | 13 +++++++++---- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/radicale/app/__init__.py b/radicale/app/__init__.py index 28c98802..eabac455 100644 --- a/radicale/app/__init__.py +++ b/radicale/app/__init__.py @@ -252,7 +252,7 @@ def response(status: int, headers: types.WSGIResponseHeaders, self.configuration, environ, base64.b64decode( authorization.encode("ascii"))).split(":", 1) - user = self._auth.login(login, password) or "" if login else "" + (user, info) = self._auth.login(login, password) or ("", "") if login else ("", "") if self.configuration.get("auth", "type") == "ldap": try: logger.debug("Groups %r", ",".join(self._auth._ldap_groups)) @@ -260,12 +260,12 @@ def response(status: int, headers: types.WSGIResponseHeaders, except AttributeError: pass if user and login == user: - logger.info("Successful login: %r", user) + logger.info("Successful login: %r (%s)", user, info) elif user: - logger.info("Successful login: %r -> %r", login, user) + logger.info("Successful login: %r -> %r (%s)", login, user, info) elif login: - logger.warning("Failed login attempt from %s: %r", - remote_host, login) + logger.warning("Failed login attempt from %s: %r (%s)", + remote_host, login, info) # Random delay to avoid timing oracles and bruteforce attacks if self._auth_delay > 0: random_delay = self._auth_delay * (0.5 + random.random()) diff --git a/radicale/auth/__init__.py b/radicale/auth/__init__.py index 679ecf9d..c1c7e884 100644 --- a/radicale/auth/__init__.py +++ b/radicale/auth/__init__.py @@ -143,7 +143,8 @@ def _login(self, login: str, password: str) -> str: raise NotImplementedError @final - def login(self, login: str, password: str) -> str: + def login(self, login: str, password: str) -> Tuple[str, str]: + result_from_cache = False if self._lc_username: login = login.lower() if self._uc_username: @@ -182,7 +183,7 @@ def login(self, login: str, password: str) -> str: (time_ns_cache, login_cache) = self._cache_failed[digest] age_failed = int((time_ns - time_ns_cache) / 1000 / 1000 / 1000) logger.debug("Login failed cache entry for user+password found: '%s' (age: %d sec)", login_cache, age_failed) - return "" + return ("", self._type + " / cached") if self._cache_successful.get(login): # login found in cache "successful" (digest_cache, time_ns_cache) = self._cache_successful[login] @@ -197,6 +198,7 @@ def login(self, login: str, password: str) -> str: else: logger.debug("Login successful cache entry for user+password found: '%s' (age: %d sec)", login, age_success) result = login + result_from_cache = True else: logger.debug("Login successful cache entry for user+password not matching: '%s'", login) else: @@ -225,6 +227,9 @@ def login(self, login: str, password: str) -> str: self._cache_failed[digest_failed] = (time_ns, login) self._lock.release() logger.debug("Login failed cache for user set: '%s'", login) - return result + if result_from_cache is True: + return (result, self._type + " / cached") + else: + return (result, self._type) else: - return self._login(login, password) + return (self._login(login, password), self._type) From 46fe98f60b3cdc967144024b5d53727294d5dea0 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Wed, 1 Jan 2025 16:31:31 +0100 Subject: [PATCH 19/36] make htpasswd cache optional --- DOCUMENTATION.md | 6 ++++ config | 3 ++ radicale/auth/htpasswd.py | 61 +++++++++++++++++++++++++-------------- radicale/config.py | 4 +++ 4 files changed, 53 insertions(+), 21 deletions(-) diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index a5238bc6..8f166e6e 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -872,6 +872,12 @@ Available methods: Default: `autodetect` +##### htpasswd_cache + +Enable caching of htpasswd file based on size and mtime_ns + +Default: `False` + ##### delay Average delay after failed login attempts in seconds. diff --git a/config b/config index 9ac082cf..3b6108fe 100644 --- a/config +++ b/config @@ -109,6 +109,9 @@ # bcrypt requires the installation of 'bcrypt' module. #htpasswd_encryption = autodetect +# Enable caching of htpasswd file based on size and mtime_ns +#htpasswd_cache = False + # Incorrect authentication delay (seconds) #delay = 1 diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py index 8ed1ad33..ec5bd280 100644 --- a/radicale/auth/htpasswd.py +++ b/radicale/auth/htpasswd.py @@ -71,6 +71,7 @@ class Auth(auth.BaseAuth): _htpasswd_not_ok_time: float _htpasswd_not_ok_reminder_seconds: int _htpasswd_bcrypt_use: int + _htpasswd_cache: bool _has_bcrypt: bool _lock: threading.Lock @@ -78,13 +79,15 @@ def __init__(self, configuration: config.Configuration) -> None: super().__init__(configuration) self._filename = configuration.get("auth", "htpasswd_filename") self._encoding = configuration.get("encoding", "stock") + self._htpasswd_cache = configuration.get("auth", "htpasswd_cache") + logger.info("auth htpasswd cache: %s", self._htpasswd_cache) encryption: str = configuration.get("auth", "htpasswd_encryption") logger.info("auth htpasswd encryption is 'radicale.auth.htpasswd_encryption.%s'", encryption) self._has_bcrypt = False self._htpasswd_ok = False self._htpasswd_not_ok_reminder_seconds = 60 # currently hardcoded - (self._htpasswd_ok, self._htpasswd_bcrypt_use) = self._read_htpasswd(True) + (self._htpasswd_ok, self._htpasswd_bcrypt_use, self._htpasswd, self._htpasswd_size, self._htpasswd_mtime_ns) = self._read_htpasswd(True, False) self._lock = threading.Lock() if encryption == "plain": @@ -154,27 +157,32 @@ def _autodetect(self, hash_value: str, password: str) -> tuple[str, bool]: # assumed plaintext return self._plain(hash_value, password) - def _read_htpasswd(self, init: bool) -> Tuple[bool, int]: + def _read_htpasswd(self, init: bool, suppress: bool) -> Tuple[bool, int, dict]: """Read htpasswd file init == True: stop on error init == False: warn/skip on error and set mark to log reminder every interval + suppress == True: suppress warnings, change info to debug (used in non-caching mode) + suppress == False: do not suppress warnings (used in caching mode) """ htpasswd_ok = True bcrypt_use = 0 - if init is True: + if (init is True) or (suppress is True): info = "Read" else: info = "Re-read" - logger.info("%s content of htpasswd file start: %r", info, self._filename) - htpasswd: dict[str, str] - htpasswd = dict() + if suppress is False: + logger.info("%s content of htpasswd file start: %r", info, self._filename) + else: + logger.debug("%s content of htpasswd file start: %r", info, self._filename) + htpasswd: dict[str, str] = dict() + entries = 0 + duplicates = 0 + errors = 0 try: with open(self._filename, encoding=self._encoding) as f: line_num = 0 - entries = 0 - duplicates = 0 for line in f: line_num += 1 line = line.rstrip("\n") @@ -186,6 +194,7 @@ def _read_htpasswd(self, init: bool) -> Tuple[bool, int]: if init is True: raise ValueError("htpasswd file contains problematic line not matching : in line: %d" % line_num) else: + errors += 1 logger.warning("htpasswd file contains problematic line not matching : in line: %d (ignored)", line_num) htpasswd_ok = False skip = True @@ -219,16 +228,17 @@ def _read_htpasswd(self, init: bool) -> Tuple[bool, int]: else: logger.warning("Failed to load htpasswd file on re-read: %r" % self._filename) htpasswd_ok = False + htpasswd_size = os.stat(self._filename).st_size + htpasswd_mtime_ns = os.stat(self._filename).st_mtime_ns + if suppress is False: + logger.info("%s content of htpasswd file done: %r (entries: %d, duplicates: %d, errors: %d)", info, self._filename, entries, duplicates, errors) else: - self._htpasswd_size = os.stat(self._filename).st_size - self._htpasswd_time_ns = os.stat(self._filename).st_mtime_ns - self._htpasswd = htpasswd - logger.info("%s content of htpasswd file done: %r (entries: %d, duplicates: %d)", info, self._filename, entries, duplicates) + logger.debug("%s content of htpasswd file done: %r (entries: %d, duplicates: %d, errors: %d)", info, self._filename, entries, duplicates, errors) if htpasswd_ok is True: self._htpasswd_not_ok_time = 0 else: self._htpasswd_not_ok_time = time.time() - return (htpasswd_ok, bcrypt_use) + return (htpasswd_ok, bcrypt_use, htpasswd, htpasswd_size, htpasswd_mtime_ns) def _login(self, login: str, password: str) -> str: """Validate credentials. @@ -241,19 +251,28 @@ def _login(self, login: str, password: str) -> str: comparing mtime_ns and size """ - # check and re-read file if required - htpasswd_size = os.stat(self._filename).st_size - htpasswd_time_ns = os.stat(self._filename).st_mtime_ns - if (htpasswd_size != self._htpasswd_size) or (htpasswd_time_ns != self._htpasswd_time_ns): + if self._htpasswd_cache is True: + # check and re-read file if required with self._lock: - (self._htpasswd_ok, self._htpasswd_bcrypt_use) = self._read_htpasswd(False) + htpasswd_size = os.stat(self._filename).st_size + htpasswd_mtime_ns = os.stat(self._filename).st_mtime_ns + if (htpasswd_size != self._htpasswd_size) or (htpasswd_mtime_ns != self._htpasswd_mtime_ns): + (self._htpasswd_ok, self._htpasswd_bcrypt_use, self._htpasswd, self._htpasswd_size, self._htpasswd_mtime_ns) = self._read_htpasswd(False, False) + self._htpasswd_not_ok_time = 0 else: - # log reminder of problemantic file every interval - if (self._htpasswd_ok is False) and (self._htpasswd_not_ok_time > 0): - current_time = time.time() + # read file on every request + (self._htpasswd_ok, self._htpasswd_bcrypt_use, self._htpasswd, self._htpasswd_size, self._htpasswd_mtime_ns) = self._read_htpasswd(False, True) + + # log reminder of problemantic file every interval + current_time = time.time() + if (self._htpasswd_ok is False): + if (self._htpasswd_not_ok_time > 0): if (current_time - self._htpasswd_not_ok_time) > self._htpasswd_not_ok_reminder_seconds: logger.warning("htpasswd file still contains issues (REMINDER, check warnings in the past): %r" % self._filename) self._htpasswd_not_ok_time = current_time + else: + self._htpasswd_not_ok_time = current_time + if self._htpasswd.get(login): digest = self._htpasswd[login] (method, password_ok) = self._verify(digest, password) diff --git a/radicale/config.py b/radicale/config.py index b165345f..224f68d3 100644 --- a/radicale/config.py +++ b/radicale/config.py @@ -203,6 +203,10 @@ def json_str(value: Any) -> dict: "value": "autodetect", "help": "htpasswd encryption method", "type": str}), + ("htpasswd_cache", { + "value": "False", + "help": "enable caching of htpasswd file", + "type": bool}), ("dovecot_socket", { "value": "/var/run/dovecot/auth-client", "help": "dovecot auth socket", From 8fdbd0dbf6e69b35e8651988dfff58de69f8f84c Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Wed, 1 Jan 2025 16:31:47 +0100 Subject: [PATCH 20/36] log cosmetics --- radicale/auth/htpasswd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py index ec5bd280..1f6c3865 100644 --- a/radicale/auth/htpasswd.py +++ b/radicale/auth/htpasswd.py @@ -113,7 +113,7 @@ def __init__(self, configuration: config.Configuration) -> None: if self._htpasswd_bcrypt_use == 0: logger.info("auth htpasswd encryption is 'radicale.auth.htpasswd_encryption.%s' and bycrypt module found, but currently not required", encryption) else: - logger.info("auth htpasswd encryption is 'radicale.auth.htpasswd_encryption.%s' and bycrypt module found (entries found: %d)", encryption, self._htpasswd_bcrypt_use) + logger.info("auth htpasswd encryption is 'radicale.auth.htpasswd_encryption.%s' and bycrypt module found (bcrypt entries found: %d)", encryption, self._htpasswd_bcrypt_use) if encryption == "bcrypt": self._verify = functools.partial(self._bcrypt, bcrypt) else: From ca665c4849267a5be56f692558b1182b398a2e93 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Wed, 1 Jan 2025 16:32:07 +0100 Subject: [PATCH 21/36] add a dummy delay action --- radicale/auth/htpasswd.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py index 1f6c3865..57bf1a40 100644 --- a/radicale/auth/htpasswd.py +++ b/radicale/auth/htpasswd.py @@ -282,5 +282,7 @@ def _login(self, login: str, password: str) -> str: else: logger.debug("Login verification failed for user: '%s' ( method '%s')", login, method) else: + # dummy delay + (method, password_ok) = self._plain(str(htpasswd_mtime_ns), password) logger.debug("Login verification user not found: '%s'", login) return "" From 8604dacad07e0619e9eb44d793328193d3cfb0f5 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Wed, 1 Jan 2025 16:40:55 +0100 Subject: [PATCH 22/36] fix typing --- radicale/auth/htpasswd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py index 57bf1a40..f444fdc3 100644 --- a/radicale/auth/htpasswd.py +++ b/radicale/auth/htpasswd.py @@ -157,7 +157,7 @@ def _autodetect(self, hash_value: str, password: str) -> tuple[str, bool]: # assumed plaintext return self._plain(hash_value, password) - def _read_htpasswd(self, init: bool, suppress: bool) -> Tuple[bool, int, dict]: + def _read_htpasswd(self, init: bool, suppress: bool) -> Tuple[bool, int, dict, int, int]: """Read htpasswd file init == True: stop on error From 5a591b6471b676291cba07854fa2c9b13d1f2d62 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Wed, 1 Jan 2025 16:41:11 +0100 Subject: [PATCH 23/36] use different token --- radicale/auth/htpasswd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py index f444fdc3..22b1b1ba 100644 --- a/radicale/auth/htpasswd.py +++ b/radicale/auth/htpasswd.py @@ -283,6 +283,6 @@ def _login(self, login: str, password: str) -> str: logger.debug("Login verification failed for user: '%s' ( method '%s')", login, method) else: # dummy delay - (method, password_ok) = self._plain(str(htpasswd_mtime_ns), password) + (method, password_ok) = self._plain(str(time.time_ns()), password) logger.debug("Login verification user not found: '%s'", login) return "" From 5d48ba5d1ec4e28469a9553e7217a24284f40160 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Wed, 1 Jan 2025 17:28:09 +0100 Subject: [PATCH 24/36] add test cases --- radicale/tests/test_auth.py | 48 ++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/radicale/tests/test_auth.py b/radicale/tests/test_auth.py index 1142caf4..23042b20 100644 --- a/radicale/tests/test_auth.py +++ b/radicale/tests/test_auth.py @@ -2,7 +2,7 @@ # Copyright © 2012-2016 Jean-Marc Martins # Copyright © 2012-2017 Guillaume Ayoub # Copyright © 2017-2022 Unrud -# Copyright © 2024-2024 Peter Bieringer +# Copyright © 2024-2025 Peter Bieringer # # This library is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -28,6 +28,7 @@ from typing import Iterable, Tuple, Union import pytest +import logging from radicale import xmlutils from radicale.tests import BaseTest @@ -101,6 +102,51 @@ def test_htpasswd_bcrypt_unicode(self) -> None: def test_htpasswd_multi(self) -> None: self._test_htpasswd("plain", "ign:ign\ntmp:bepo") + # login cache successful + def test_htpasswd_login_cache_successful_plain(self, caplog) -> None: + caplog.set_level(logging.INFO) + self.configure({"auth": {"cache_logins": "True"}}) + self._test_htpasswd("plain", "tmp:bepo", (("tmp", "bepo", True), ("tmp", "bepo", True))) + htpasswd_found = False + htpasswd_cached_found = False + for line in caplog.messages: + if line == "Successful login: 'tmp' (htpasswd)": + htpasswd_found = True + elif line == "Successful login: 'tmp' (htpasswd / cached)": + htpasswd_cached_found = True + if (htpasswd_found is False) or (htpasswd_cached_found is False): + raise ValueError("Logging misses expected log lines") + + # login cache failed + def test_htpasswd_login_cache_failed_plain(self, caplog) -> None: + caplog.set_level(logging.INFO) + self.configure({"auth": {"cache_logins": "True"}}) + self._test_htpasswd("plain", "tmp:bepo", (("tmp", "bepo1", False), ("tmp", "bepo1", False))) + htpasswd_found = False + htpasswd_cached_found = False + for line in caplog.messages: + if line == "Failed login attempt from unknown: 'tmp' (htpasswd)": + htpasswd_found = True + elif line == "Failed login attempt from unknown: 'tmp' (htpasswd / cached)": + htpasswd_cached_found = True + if (htpasswd_found is False) or (htpasswd_cached_found is False): + raise ValueError("Logging misses expected log lines") + + # htpasswd file cache + def test_htpasswd_file_cache(self, caplog) -> None: + self.configure({"auth": {"htpasswd_cache": "True"}}) + self._test_htpasswd("plain", "tmp:bepo") + + # detection of broken htpasswd file entries + def test_htpasswd_broken(self) -> None: + for userpass in ["tmp:", ":tmp" ]: + try: + self._test_htpasswd("plain", userpass) + except RuntimeError: + pass + else: + raise + @pytest.mark.skipif(sys.platform == "win32", reason="leading and trailing " "whitespaces not allowed in file names") def test_htpasswd_whitespace_user(self) -> None: From 0a5ae5b0b4faa9690c48f909f30c39157ef847c9 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Wed, 1 Jan 2025 17:31:16 +0100 Subject: [PATCH 25/36] extend startup logging for htpasswd --- radicale/auth/htpasswd.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py index 22b1b1ba..cc94a8f9 100644 --- a/radicale/auth/htpasswd.py +++ b/radicale/auth/htpasswd.py @@ -78,7 +78,9 @@ class Auth(auth.BaseAuth): def __init__(self, configuration: config.Configuration) -> None: super().__init__(configuration) self._filename = configuration.get("auth", "htpasswd_filename") + logger.info("auth htpasswd file: %r", self._filename) self._encoding = configuration.get("encoding", "stock") + logger.info("auth htpasswd file encoding: %r", self._encoding) self._htpasswd_cache = configuration.get("auth", "htpasswd_cache") logger.info("auth htpasswd cache: %s", self._htpasswd_cache) encryption: str = configuration.get("auth", "htpasswd_encryption") From 3763f28ae4eab549a444a2921923138d1238a098 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Wed, 1 Jan 2025 17:36:15 +0100 Subject: [PATCH 26/36] tox fixes --- radicale/tests/test_auth.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/radicale/tests/test_auth.py b/radicale/tests/test_auth.py index 23042b20..f2ba577b 100644 --- a/radicale/tests/test_auth.py +++ b/radicale/tests/test_auth.py @@ -23,12 +23,12 @@ """ import base64 +import logging import os import sys from typing import Iterable, Tuple, Union import pytest -import logging from radicale import xmlutils from radicale.tests import BaseTest @@ -139,7 +139,7 @@ def test_htpasswd_file_cache(self, caplog) -> None: # detection of broken htpasswd file entries def test_htpasswd_broken(self) -> None: - for userpass in ["tmp:", ":tmp" ]: + for userpass in ["tmp:", ":tmp"]: try: self._test_htpasswd("plain", userpass) except RuntimeError: From 70c4a34eb8c6072cb3658f3fe7b3b44fbdb0616d Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Wed, 1 Jan 2025 17:36:33 +0100 Subject: [PATCH 27/36] fix/extend changelog --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d8bbaa1..bade4998 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,10 @@ ## 3.3.4.dev * Add: option [auth] cache_logins/cache_successful_logins_expiry/cache_failed_logins for caching logins * Improve: [auth] log used hash method and result on debug for htpasswd authentication -* Improve: [auth] htpasswd file now read and verified on start, automatic re-read triggered on change (mtime or size) +* Improve: [auth] htpasswd file now read and verified on start +* Add: option [auth] htpasswd_cache to automatic re-read triggered on change (mtime or size) instead reading on each request * Improve: [auth] htpasswd: module 'bcrypt' is no longer mandatory in case digest method not used in file +* Improve: [auth] successful/failed login logs now type and whether result was taken from cache ## 3.3.3 * Add: display mtime_ns precision of storage folder with condition warning if too less From 6f0ac545f0f35438c5105d874118abd381fd2603 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Thu, 2 Jan 2025 08:08:22 +0100 Subject: [PATCH 28/36] code fix --- radicale/auth/htpasswd.py | 43 ++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py index cc94a8f9..842481fd 100644 --- a/radicale/auth/htpasswd.py +++ b/radicale/auth/htpasswd.py @@ -249,10 +249,19 @@ def _login(self, login: str, password: str) -> str: hash (encrypted password) and check hash against password, using the method specified in the Radicale config. - The content of the file is cached and live updates will be detected by + Optional: the content of the file is cached and live updates will be detected by comparing mtime_ns and size + TODO: improve against timing attacks + see also issue 591 + but also do not delay that much + see also issue 1466 + + As several hash methods are supported which have different speed a time based gap would be required + """ + login_ok = False + digest: str if self._htpasswd_cache is True: # check and re-read file if required with self._lock: @@ -261,22 +270,28 @@ def _login(self, login: str, password: str) -> str: if (htpasswd_size != self._htpasswd_size) or (htpasswd_mtime_ns != self._htpasswd_mtime_ns): (self._htpasswd_ok, self._htpasswd_bcrypt_use, self._htpasswd, self._htpasswd_size, self._htpasswd_mtime_ns) = self._read_htpasswd(False, False) self._htpasswd_not_ok_time = 0 + + # log reminder of problemantic file every interval + current_time = time.time() + if (self._htpasswd_ok is False): + if (self._htpasswd_not_ok_time > 0): + if (current_time - self._htpasswd_not_ok_time) > self._htpasswd_not_ok_reminder_seconds: + logger.warning("htpasswd file still contains issues (REMINDER, check warnings in the past): %r" % self._filename) + self._htpasswd_not_ok_time = current_time + else: + self._htpasswd_not_ok_time = current_time + + if self._htpasswd.get(login): + digest = self._htpasswd[login] + login_ok = True else: # read file on every request - (self._htpasswd_ok, self._htpasswd_bcrypt_use, self._htpasswd, self._htpasswd_size, self._htpasswd_mtime_ns) = self._read_htpasswd(False, True) - - # log reminder of problemantic file every interval - current_time = time.time() - if (self._htpasswd_ok is False): - if (self._htpasswd_not_ok_time > 0): - if (current_time - self._htpasswd_not_ok_time) > self._htpasswd_not_ok_reminder_seconds: - logger.warning("htpasswd file still contains issues (REMINDER, check warnings in the past): %r" % self._filename) - self._htpasswd_not_ok_time = current_time - else: - self._htpasswd_not_ok_time = current_time + (htpasswd_ok, htpasswd_bcrypt_use, htpasswd, htpasswd_size, htpasswd_mtime_ns) = self._read_htpasswd(False, True) + if htpasswd.get(login): + digest = htpasswd[login] + login_ok = True - if self._htpasswd.get(login): - digest = self._htpasswd[login] + if login_ok is True: (method, password_ok) = self._verify(digest, password) logger.debug("Login verification successful for user: '%s' (method '%s')", login, method) if password_ok: From 0d43a49ffb0ee4a65e630ca8ad6a7a6b8fc7c1a7 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Thu, 2 Jan 2025 22:33:54 +0100 Subject: [PATCH 29/36] add variable sleep to have a constant execution time on failed login --- radicale/auth/__init__.py | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/radicale/auth/__init__.py b/radicale/auth/__init__.py index c1c7e884..eb620e29 100644 --- a/radicale/auth/__init__.py +++ b/radicale/auth/__init__.py @@ -60,6 +60,8 @@ class BaseAuth: _lc_username: bool _uc_username: bool _strip_domain: bool + _auth_delay: float + _failed_auth_delay: float _type: str _cache_logins: bool _cache_successful: dict # login -> (digest, time_ns) @@ -86,6 +88,9 @@ def __init__(self, configuration: "config.Configuration") -> None: 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") + self._auth_delay = configuration.get("auth", "delay") + logger.info("auth.delay: %f", self._auth_delay) + self._failed_auth_delay = self._auth_delay # cache_successful_logins self._cache_logins = configuration.get("auth", "cache_logins") self._type = configuration.get("auth", "type") @@ -142,8 +147,22 @@ def _login(self, login: str, password: str) -> str: raise NotImplementedError + def _sleep(self, time_ns_begin): + """Sleep some time to reach a constant execution time finally + Increase final execution time in case initial limit exceeded + """ + time_delta = (time.time_ns() - time_ns_begin) / 1000 / 1000 / 1000 + if time_delta > self._failed_auth_delay: + logger.debug("Increase failed auth_delay %.3f -> %.3f seconds", self._failed_auth_delay, time_delta) + with self._lock: + self._failed_auth_delay = time_delta + sleep = self._failed_auth_delay - time_delta + logger.debug("Sleeping %.3f seconds", sleep) + time.sleep(sleep) + @final def login(self, login: str, password: str) -> Tuple[str, str]: + time_ns_begin = time.time_ns() result_from_cache = False if self._lc_username: login = login.lower() @@ -183,6 +202,7 @@ def login(self, login: str, password: str) -> Tuple[str, str]: (time_ns_cache, login_cache) = self._cache_failed[digest] age_failed = int((time_ns - time_ns_cache) / 1000 / 1000 / 1000) logger.debug("Login failed cache entry for user+password found: '%s' (age: %d sec)", login_cache, age_failed) + self._sleep(time_ns_begin) return ("", self._type + " / cached") if self._cache_successful.get(login): # login found in cache "successful" @@ -228,8 +248,16 @@ def login(self, login: str, password: str) -> Tuple[str, str]: self._lock.release() logger.debug("Login failed cache for user set: '%s'", login) if result_from_cache is True: + if result == "": + self._sleep(time_ns_begin) return (result, self._type + " / cached") else: + if result == "": + self._sleep(time_ns_begin) return (result, self._type) else: - return (self._login(login, password), self._type) + # self._cache_logins is False + result = self._login(login, password) + if result == "": + self._sleep(time_ns_begin) + return (result, self._type) From cf914450ee08ddaf19005e22f62cc8f409526323 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Fri, 3 Jan 2025 07:02:29 +0100 Subject: [PATCH 30/36] remove obsolete code and comment as constant execution time is now done by __init__.py --- radicale/auth/htpasswd.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py index 842481fd..8d007cb8 100644 --- a/radicale/auth/htpasswd.py +++ b/radicale/auth/htpasswd.py @@ -252,13 +252,6 @@ def _login(self, login: str, password: str) -> str: Optional: the content of the file is cached and live updates will be detected by comparing mtime_ns and size - TODO: improve against timing attacks - see also issue 591 - but also do not delay that much - see also issue 1466 - - As several hash methods are supported which have different speed a time based gap would be required - """ login_ok = False digest: str @@ -299,7 +292,5 @@ def _login(self, login: str, password: str) -> str: else: logger.debug("Login verification failed for user: '%s' ( method '%s')", login, method) else: - # dummy delay - (method, password_ok) = self._plain(str(time.time_ns()), password) logger.debug("Login verification user not found: '%s'", login) return "" From 5a00baab3f17a4e5355a10061b00c1e5d07e6e6b Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Fri, 3 Jan 2025 07:11:51 +0100 Subject: [PATCH 31/36] cosmetics --- radicale/app/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radicale/app/__init__.py b/radicale/app/__init__.py index eabac455..7f8301f2 100644 --- a/radicale/app/__init__.py +++ b/radicale/app/__init__.py @@ -269,7 +269,7 @@ def response(status: int, headers: types.WSGIResponseHeaders, # Random delay to avoid timing oracles and bruteforce attacks if self._auth_delay > 0: random_delay = self._auth_delay * (0.5 + random.random()) - logger.debug("Sleeping %.3f seconds", random_delay) + logger.debug("Failed login, sleeping random: %.3f sec", random_delay) time.sleep(random_delay) if user and not pathutils.is_safe_path_component(user): From a9f2e6fe7b57ca51a1a48d4eb5775c56eedfe812 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Fri, 3 Jan 2025 07:14:32 +0100 Subject: [PATCH 32/36] improve code/adjustments --- radicale/auth/__init__.py | 43 ++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/radicale/auth/__init__.py b/radicale/auth/__init__.py index eb620e29..a0974296 100644 --- a/radicale/auth/__init__.py +++ b/radicale/auth/__init__.py @@ -90,7 +90,7 @@ def __init__(self, configuration: "config.Configuration") -> None: raise RuntimeError("auth.lc_username and auth.uc_username cannot be enabled together") self._auth_delay = configuration.get("auth", "delay") logger.info("auth.delay: %f", self._auth_delay) - self._failed_auth_delay = self._auth_delay + self._failed_auth_delay = 0 # cache_successful_logins self._cache_logins = configuration.get("auth", "cache_logins") self._type = configuration.get("auth", "type") @@ -147,18 +147,33 @@ def _login(self, login: str, password: str) -> str: raise NotImplementedError - def _sleep(self, time_ns_begin): - """Sleep some time to reach a constant execution time finally + def _sleep_for_constant_exec_time(self, time_ns_begin): + """Sleep some time to reach a constant execution time for failed logins + + Independent of time required by external backend or used digest methods + Increase final execution time in case initial limit exceeded + + See also issue 591 + """ time_delta = (time.time_ns() - time_ns_begin) / 1000 / 1000 / 1000 - if time_delta > self._failed_auth_delay: - logger.debug("Increase failed auth_delay %.3f -> %.3f seconds", self._failed_auth_delay, time_delta) - with self._lock: - self._failed_auth_delay = time_delta - sleep = self._failed_auth_delay - time_delta - logger.debug("Sleeping %.3f seconds", sleep) - time.sleep(sleep) + with self._lock: + # avoid that another thread is changing global value at the same time + failed_auth_delay = self._failed_auth_delay + failed_auth_delay_old = failed_auth_delay + if time_delta > failed_auth_delay: + # set new + failed_auth_delay = time_delta + # store globally + self._failed_auth_delay = failed_auth_delay + if (failed_auth_delay_old != failed_auth_delay): + logger.debug("Failed login constant execution time need increase of failed_auth_delay: %.9f -> %.9f sec", failed_auth_delay_old, failed_auth_delay) + # sleep == 0 + else: + sleep = failed_auth_delay - time_delta + logger.debug("Failed login constant exection time alignment, sleeping: %.9f sec", sleep) + time.sleep(sleep) @final def login(self, login: str, password: str) -> Tuple[str, str]: @@ -202,7 +217,7 @@ def login(self, login: str, password: str) -> Tuple[str, str]: (time_ns_cache, login_cache) = self._cache_failed[digest] age_failed = int((time_ns - time_ns_cache) / 1000 / 1000 / 1000) logger.debug("Login failed cache entry for user+password found: '%s' (age: %d sec)", login_cache, age_failed) - self._sleep(time_ns_begin) + self._sleep_for_constant_exec_time(time_ns_begin) return ("", self._type + " / cached") if self._cache_successful.get(login): # login found in cache "successful" @@ -249,15 +264,15 @@ def login(self, login: str, password: str) -> Tuple[str, str]: logger.debug("Login failed cache for user set: '%s'", login) if result_from_cache is True: if result == "": - self._sleep(time_ns_begin) + self._sleep_for_constant_exec_time(time_ns_begin) return (result, self._type + " / cached") else: if result == "": - self._sleep(time_ns_begin) + self._sleep_for_constant_exec_time(time_ns_begin) return (result, self._type) else: # self._cache_logins is False result = self._login(login, password) if result == "": - self._sleep(time_ns_begin) + self._sleep_for_constant_exec_time(time_ns_begin) return (result, self._type) From 2442a794ae8aebae10ae1e7d435dde58191e21e8 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Thu, 2 Jan 2025 23:17:34 +0100 Subject: [PATCH 33/36] tox fixes --- radicale/auth/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/radicale/auth/__init__.py b/radicale/auth/__init__.py index a0974296..b30a3c79 100644 --- a/radicale/auth/__init__.py +++ b/radicale/auth/__init__.py @@ -91,6 +91,7 @@ def __init__(self, configuration: "config.Configuration") -> None: self._auth_delay = configuration.get("auth", "delay") logger.info("auth.delay: %f", self._auth_delay) self._failed_auth_delay = 0 + self._lock = threading.Lock() # cache_successful_logins self._cache_logins = configuration.get("auth", "cache_logins") self._type = configuration.get("auth", "type") @@ -112,7 +113,6 @@ def __init__(self, configuration: "config.Configuration") -> None: self._cache_successful = dict() self._cache_failed = dict() self._cache_failed_logins_salt_ns = time.time_ns() - self._lock = threading.Lock() def _cache_digest(self, login: str, password: str, salt: str) -> str: h = hashlib.sha3_512() @@ -147,7 +147,7 @@ def _login(self, login: str, password: str) -> str: raise NotImplementedError - def _sleep_for_constant_exec_time(self, time_ns_begin): + def _sleep_for_constant_exec_time(self, time_ns_begin: int): """Sleep some time to reach a constant execution time for failed logins Independent of time required by external backend or used digest methods From ad94acddf1d7131f29d3fed02c50f006adff23f8 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Thu, 2 Jan 2025 23:19:58 +0100 Subject: [PATCH 34/36] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bade4998..53978fd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * Add: option [auth] htpasswd_cache to automatic re-read triggered on change (mtime or size) instead reading on each request * Improve: [auth] htpasswd: module 'bcrypt' is no longer mandatory in case digest method not used in file * Improve: [auth] successful/failed login logs now type and whether result was taken from cache +* Improve: [auth] constant execution time for failed logins independent of external backend or by htpasswd used digest method ## 3.3.3 * Add: display mtime_ns precision of storage folder with condition warning if too less From b1220020778d2f27df3554222345cac9e5cb2ccc Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Fri, 3 Jan 2025 00:41:26 +0100 Subject: [PATCH 35/36] drop support of python 3.8, fixes https://github.com/Kozea/Radicale/issues/1628 --- .github/workflows/test.yml | 4 +--- DOCUMENTATION.md | 2 +- pyproject.toml | 3 +-- setup.py.legacy | 3 +-- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8339a975..82ac574f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,10 +6,8 @@ jobs: strategy: matrix: os: [ubuntu-latest, macos-latest, windows-latest] - python-version: ['3.8', '3.9', '3.10', '3.11', '3.12.3', '3.13.0', pypy-3.9] + python-version: ['3.9', '3.10', '3.11', '3.12.3', '3.13.0', pypy-3.9] exclude: - - os: windows-latest - python-version: pypy-3.8 - os: windows-latest python-version: pypy-3.9 runs-on: ${{ matrix.os }} diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index 8f166e6e..1a3ba9b4 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -55,7 +55,7 @@ Follow one of the chapters below depending on your operating system. #### Linux / \*BSD -First, make sure that **python** 3.8 or later and **pip** are installed. On most distributions it should be +First, make sure that **python** 3.9 or later and **pip** are installed. On most distributions it should be enough to install the package ``python3-pip``. Then open a console and type: diff --git a/pyproject.toml b/pyproject.toml index d01e3967..15af7e1a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,7 +17,6 @@ classifiers = [ "License :: OSI Approved :: GNU General Public License (GPL)", "Operating System :: OS Independent", "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", @@ -28,7 +27,7 @@ classifiers = [ "Topic :: Office/Business :: Groupware", ] urls = {Homepage = "https://radicale.org/"} -requires-python = ">=3.8.0" +requires-python = ">=3.9.0" dependencies = [ "defusedxml", "passlib", diff --git a/setup.py.legacy b/setup.py.legacy index 52d74dda..c1cbe249 100644 --- a/setup.py.legacy +++ b/setup.py.legacy @@ -61,7 +61,7 @@ setup( install_requires=install_requires, extras_require={"test": test_requires, "bcrypt": bcrypt_requires, "ldap": ldap_requires}, keywords=["calendar", "addressbook", "CalDAV", "CardDAV"], - python_requires=">=3.8.0", + python_requires=">=3.9.0", classifiers=[ "Development Status :: 5 - Production/Stable", "Environment :: Console", @@ -71,7 +71,6 @@ setup( "License :: OSI Approved :: GNU General Public License (GPL)", "Operating System :: OS Independent", "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", From 976dfe4a3fcc71afb5de10cf50df5f89eb4a5837 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Fri, 3 Jan 2025 00:42:08 +0100 Subject: [PATCH 36/36] drop Python 3.8 changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53978fd0..bbed6051 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ * Improve: [auth] htpasswd: module 'bcrypt' is no longer mandatory in case digest method not used in file * Improve: [auth] successful/failed login logs now type and whether result was taken from cache * Improve: [auth] constant execution time for failed logins independent of external backend or by htpasswd used digest method +* Drop: support for Python 3.8 ## 3.3.3 * Add: display mtime_ns precision of storage folder with condition warning if too less