Merge pull request #1729 from pbiering/htpasswd-cosmetic-bcrypt-exten…

…sions Htpasswd cosmetic bcrypt extensions
Kozea · Mar 9, 2025 · 0f67336 · 0f67336
2 parents 4f0e607 + cf72710
commit 0f67336
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@
 * Adjust: [auth] imap: use AUTHENTICATE PLAIN instead of LOGIN towards remote IMAP server
 * Improve: log client IP on SSL error and SSL protocol+cipher if successful
 * Improve: catch htpasswd hash verification errors
+* Improve: add support for more bcrypt algos on autodetection, extend logging for autodetection fallback to PLAIN in case of hash length is not matching
 
 ## 3.4.1
 * Add: option [auth] dovecot_connection_type / dovecot_host / dovecot_port

diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py
@@ -51,6 +51,7 @@
 import functools
 import hmac
 import os
+import re
 import threading
 import time
 from typing import Any, Tuple
@@ -131,33 +132,49 @@ 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 _plain_fallback(self, method_orig, hash_value: str, password: str) -> tuple[str, bool]:
+        """Check if ``hash_value`` and ``password`` match, plain method / fallback in case of hash length is not matching on autodetection."""
+        info = "PLAIN/fallback as hash length not matching for " + method_orig + ": " + str(len(hash_value))
+        return (info, hmac.compare_digest(hash_value.encode(), password.encode()))
+
     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()))
+        if self._encryption == "autodetect" and len(hash_value) != 60:
+            return self._plain_fallback("BCRYPT", hash_value, password)
+        else:
+            return ("BCRYPT", bcrypt.checkpw(password=password.encode('utf-8'), hashed_password=hash_value.encode()))
 
     def _md5apr1(self, hash_value: str, password: str) -> tuple[str, bool]:
-        return ("MD5-APR1", apr_md5_crypt.verify(password, hash_value.strip()))
+        if self._encryption == "autodetect" and len(hash_value) != 37:
+            return self._plain_fallback("MD5-APR1", hash_value, password)
+        else:
+            return ("MD5-APR1", apr_md5_crypt.verify(password, hash_value.strip()))
 
     def _sha256(self, hash_value: str, password: str) -> tuple[str, bool]:
-        return ("SHA-256", sha256_crypt.verify(password, hash_value.strip()))
+        if self._encryption == "autodetect" and len(hash_value) != 63:
+            return self._plain_fallback("SHA-256", hash_value, password)
+        else:
+            return ("SHA-256", sha256_crypt.verify(password, hash_value.strip()))
 
     def _sha512(self, hash_value: str, password: str) -> tuple[str, bool]:
-        return ("SHA-512", sha512_crypt.verify(password, hash_value.strip()))
+        if self._encryption == "autodetect" and len(hash_value) != 106:
+            return self._plain_fallback("SHA-512", hash_value, password)
+        else:
+            return ("SHA-512", sha512_crypt.verify(password, hash_value.strip()))
 
     def _autodetect(self, hash_value: str, password: str) -> tuple[str, bool]:
-        if hash_value.startswith("$apr1$", 0, 6) and len(hash_value) == 37:
+        if hash_value.startswith("$apr1$", 0, 6):
             # MD5-APR1
             return self._md5apr1(hash_value, password)
-        elif hash_value.startswith("$2y$", 0, 4) and len(hash_value) == 60:
+        elif re.match(r"^\$2(a|b|x|y)?\$", hash_value):
             # BCRYPT
             return self._verify_bcrypt(hash_value, password)
-        elif hash_value.startswith("$5$", 0, 3) and len(hash_value) == 63:
+        elif hash_value.startswith("$5$", 0, 3):
             # SHA-256
             return self._sha256(hash_value, password)
-        elif hash_value.startswith("$6$", 0, 3) and len(hash_value) == 106:
+        elif hash_value.startswith("$6$", 0, 3):
             # SHA-512
             return self._sha512(hash_value, password)
         else:
-            # assumed plaintext
             return self._plain(hash_value, password)
 
     def _read_htpasswd(self, init: bool, suppress: bool) -> Tuple[bool, int, dict, int, int]:
@@ -289,13 +306,13 @@ def _login(self, login: str, password: str) -> str:
             try:
                 (method, password_ok) = self._verify(digest, password)
             except ValueError as e:
-                logger.warning("Login verification failed for user: '%s' (method '%s') '%s'", login, self._encryption, e)
+                logger.error("Login verification failed for user: '%s' (htpasswd/%s) with errror '%s'", login, self._encryption, e)
                 return ""
-            logger.debug("Login verification successful for user: '%s' (method '%s')", login, method)
             if password_ok:
+                logger.debug("Login verification successful for user: '%s' (htpasswd/%s/%s)", login, self._encryption, method)
                 return login
             else:
-                logger.warning("Login verification failed for user: '%s' (method '%s')", login, method)
+                logger.warning("Login verification failed for user: '%s' (htpasswd/%s/%s)", login, self._encryption, method)
         else:
-            logger.warning("Login verification user not found: '%s'", login)
+            logger.warning("Login verification user not found (htpasswd): '%s'", login)
         return ""
diff --git a/radicale/tests/test_auth.py b/radicale/tests/test_auth.py
@@ -79,6 +79,9 @@ def _test_htpasswd(self, htpasswd_encryption: str, htpasswd_content: str,
     def test_htpasswd_plain(self) -> None:
         self._test_htpasswd("plain", "tmp:bepo")
 
+    def test_htpasswd_plain_autodetect(self) -> None:
+        self._test_htpasswd("autodetect", "tmp:bepo")
+
     def test_htpasswd_plain_password_split(self) -> None:
         self._test_htpasswd("plain", "tmp:be:po", (
             ("tmp", "be:po", True), ("tmp", "bepo", False)))
@@ -89,25 +92,60 @@ def test_htpasswd_plain_unicode(self) -> None:
     def test_htpasswd_md5(self) -> None:
         self._test_htpasswd("md5", "tmp:$apr1$BI7VKCZh$GKW4vq2hqDINMr8uv7lDY/")
 
+    def test_htpasswd_md5_autodetect(self) -> None:
+        self._test_htpasswd("autodetect", "tmp:$apr1$BI7VKCZh$GKW4vq2hqDINMr8uv7lDY/")
+
     def test_htpasswd_md5_unicode(self):
         self._test_htpasswd(
             "md5", "😀:$apr1$w4ev89r1$29xO8EvJmS2HEAadQ5qy11", "unicode")
 
     def test_htpasswd_sha256(self) -> None:
         self._test_htpasswd("sha256", "tmp:$5$i4Ni4TQq6L5FKss5$ilpTjkmnxkwZeV35GB9cYSsDXTALBn6KtWRJAzNlCL/")
 
+    def test_htpasswd_sha256_autodetect(self) -> None:
+        self._test_htpasswd("autodetect", "tmp:$5$i4Ni4TQq6L5FKss5$ilpTjkmnxkwZeV35GB9cYSsDXTALBn6KtWRJAzNlCL/")
+
     def test_htpasswd_sha512(self) -> None:
         self._test_htpasswd("sha512", "tmp:$6$3Qhl8r6FLagYdHYa$UCH9yXCed4A.J9FQsFPYAOXImzZUMfvLa0lwcWOxWYLOF5sE/lF99auQ4jKvHY2vijxmefl7G6kMqZ8JPdhIJ/")
 
+    def test_htpasswd_sha512_autodetect(self) -> None:
+        self._test_htpasswd("autodetect", "tmp:$6$3Qhl8r6FLagYdHYa$UCH9yXCed4A.J9FQsFPYAOXImzZUMfvLa0lwcWOxWYLOF5sE/lF99auQ4jKvHY2vijxmefl7G6kMqZ8JPdhIJ/")
+
+    @pytest.mark.skipif(has_bcrypt == 0, reason="No bcrypt module installed")
+    def test_htpasswd_bcrypt_2a(self) -> None:
+        self._test_htpasswd("bcrypt", "tmp:$2a$10$Mj4A9vMecAp/K7.0fMKoVOk1SjgR.RBhl06a52nvzXhxlT3HB7Reu")
+
+    @pytest.mark.skipif(has_bcrypt == 0, reason="No bcrypt module installed")
+    def test_htpasswd_bcrypt_2a_autodetect(self) -> None:
+        self._test_htpasswd("autodetect", "tmp:$2a$10$Mj4A9vMecAp/K7.0fMKoVOk1SjgR.RBhl06a52nvzXhxlT3HB7Reu")
+
+    @pytest.mark.skipif(has_bcrypt == 0, reason="No bcrypt module installed")
+    def test_htpasswd_bcrypt_2b(self) -> None:
+        self._test_htpasswd("bcrypt", "tmp:$2b$12$7a4z/fdmXlBIfkz0smvzW.1Nds8wpgC/bo2DVOb4OSQKWCDL1A1wu")
+
+    @pytest.mark.skipif(has_bcrypt == 0, reason="No bcrypt module installed")
+    def test_htpasswd_bcrypt_2b_autodetect(self) -> None:
+        self._test_htpasswd("autodetect", "tmp:$2b$12$7a4z/fdmXlBIfkz0smvzW.1Nds8wpgC/bo2DVOb4OSQKWCDL1A1wu")
+
+    @pytest.mark.skipif(has_bcrypt == 0, reason="No bcrypt module installed")
+    def test_htpasswd_bcrypt_2y(self) -> None:
+        self._test_htpasswd("bcrypt", "tmp:$2y$05$oD7hbiQFQlvCM7zoalo/T.MssV3VNTRI3w5KDnj8NTUKJNWfVpvRq")
+
+    @pytest.mark.skipif(has_bcrypt == 0, reason="No bcrypt module installed")
+    def test_htpasswd_bcrypt_2y_autodetect(self) -> None:
+        self._test_htpasswd("autodetect", "tmp:$2y$05$oD7hbiQFQlvCM7zoalo/T.MssV3VNTRI3w5KDnj8NTUKJNWfVpvRq")
+
+    @pytest.mark.skipif(has_bcrypt == 0, reason="No bcrypt module installed")
+    def test_htpasswd_bcrypt_C10(self) -> None:
+        self._test_htpasswd("bcrypt", "tmp:$2y$10$bZsWq06ECzxqi7RmulQvC.T1YHUnLW2E3jn.MU2pvVTGn1dfORt2a")
+
     @pytest.mark.skipif(has_bcrypt == 0, reason="No bcrypt module installed")
-    def test_htpasswd_bcrypt(self) -> None:
-        self._test_htpasswd("bcrypt", "tmp:$2y$05$oD7hbiQFQlvCM7zoalo/T.MssV3V"
-                            "NTRI3w5KDnj8NTUKJNWfVpvRq")
+    def test_htpasswd_bcrypt_C10_autodetect(self) -> None:
+        self._test_htpasswd("bcrypt", "tmp:$2y$10$bZsWq06ECzxqi7RmulQvC.T1YHUnLW2E3jn.MU2pvVTGn1dfORt2a")
 
     @pytest.mark.skipif(has_bcrypt == 0, reason="No bcrypt module installed")
     def test_htpasswd_bcrypt_unicode(self) -> None:
-        self._test_htpasswd("bcrypt", "😀:$2y$10$Oyz5aHV4MD9eQJbk6GPemOs4T6edK"
-                            "6U9Sqlzr.W1mMVCS8wJUftnW", "unicode")
+        self._test_htpasswd("bcrypt", "😀:$2y$10$Oyz5aHV4MD9eQJbk6GPemOs4T6edK6U9Sqlzr.W1mMVCS8wJUftnW", "unicode")
 
     def test_htpasswd_multi(self) -> None:
         self._test_htpasswd("plain", "ign:ign\ntmp:bepo")