htpasswd: don't strip whitespaces and allow ':' in plain password

radicale/auth.py

@@ -170,16 +170,18 @@ class Auth(BaseAuth):
 
     def _crypt(self, crypt, hash_value, password):
         """Check if ``hash_value`` and ``password`` match, crypt method."""
+        hash_value = hash_value.strip()
         return hmac.compare_digest(crypt.crypt(password, hash_value),
                                    hash_value)
 
     def _sha1(self, hash_value, password):
         """Check if ``hash_value`` and ``password`` match, sha1 method."""
-        hash_value = hash_value.replace("{SHA}", "").encode("ascii")
+        hash_value = base64.b64decode(hash_value.strip().replace(
+            "{SHA}", "").encode("ascii"))
         password = password.encode(self.configuration.get("encoding", "stock"))
         sha1 = hashlib.sha1()
         sha1.update(password)
-        return hmac.compare_digest(sha1.digest(), base64.b64decode(hash_value))
+        return hmac.compare_digest(sha1.digest(), hash_value)
 
     def _ssha(self, hash_value, password):
         """Check if ``hash_value`` and ``password`` match, salted sha1 method.
@@ -188,7 +190,7 @@ class Auth(BaseAuth):
         written with e.g. openssl, and nginx can parse it.
 
         """
-        hash_value = base64.b64decode(hash_value.replace(
+        hash_value = base64.b64decode(hash_value.strip().replace(
             "{SSHA}", "").encode("ascii"))
         password = password.encode(self.configuration.get("encoding", "stock"))
         salt_value = hash_value[20:]
@@ -199,9 +201,11 @@ class Auth(BaseAuth):
         return hmac.compare_digest(sha1.digest(), hash_value)
 
     def _bcrypt(self, bcrypt, hash_value, password):
+        hash_value = hash_value.strip()
         return bcrypt.verify(password, hash_value)
 
     def _md5apr1(self, md5_apr1, hash_value, password):
+        hash_value = hash_value.strip()
         return md5_apr1.verify(password, hash_value)
 
     def is_authenticated(self, user, password):
@@ -209,12 +213,12 @@ class Auth(BaseAuth):
         # very cheap operation, and it's useful to get live updates of the
         # htpasswd file.
         try:
-            with open(self.filename) as fd:
-                for line in fd:
-                    line = line.strip()
-                    if line:
+            with open(self.filename) as f:
+                for line in f:
+                    line = line.rstrip("\n")
+                    if line.lstrip():
                         try:
-                            login, hash_value = line.split(":")
+                            login, hash_value = line.split(":", maxsplit=1)
                             # Always compare both login and password to avoid
                             # timing attacks, see #591.
                             login_ok = hmac.compare_digest(login, user)

radicale/tests/test_auth.py

@@ -52,7 +52,8 @@ class TestBaseAuthRequests(BaseTest):
     def teardown(self):
         shutil.rmtree(self.colpath)
 
-    def _test_htpasswd(self, htpasswd_encryption, htpasswd_content):
+    def _test_htpasswd(self, htpasswd_encryption, htpasswd_content,
+                       test_matrix=None):
         """Test htpasswd authentication with user "tmp" and password "bepo"."""
         htpasswd_file_path = os.path.join(self.colpath, ".htpasswd")
         with open(htpasswd_file_path, "w") as f:
@@ -61,9 +62,11 @@ class TestBaseAuthRequests(BaseTest):
         self.configuration["auth"]["htpasswd_filename"] = htpasswd_file_path
         self.configuration["auth"]["htpasswd_encryption"] = htpasswd_encryption
         self.application = Application(self.configuration, self.logger)
-        for user, password, expected_status in (
+        if test_matrix is None:
+            test_matrix = (
                 ("tmp", "bepo", 207), ("tmp", "tmp", 401), ("tmp", "", 401),
-                ("unk", "unk", 401), ("unk", "", 401), ("", "", 401)):
+                ("unk", "unk", 401), ("unk", "", 401), ("", "", 401))
+        for user, password, expected_status in test_matrix:
             status, _, answer = self.request(
                 "PROPFIND", "/",
                 HTTP_AUTHORIZATION="Basic %s" % base64.b64encode(
@@ -73,6 +76,10 @@ class TestBaseAuthRequests(BaseTest):
     def test_htpasswd_plain(self):
         self._test_htpasswd("plain", "tmp:bepo")
 
+    def test_htpasswd_plain_password_split(self):
+        self._test_htpasswd("plain", "tmp:be:po", (
+            ("tmp", "be:po", 207), ("tmp", "bepo", 401)))
+
     def test_htpasswd_sha1(self):
         self._test_htpasswd("sha1", "tmp:{SHA}UWRS3uSJJq2itZQEUyIH8rRajCM=")
 
@@ -107,6 +114,13 @@ class TestBaseAuthRequests(BaseTest):
             "bcrypt",
             "tmp:$2y$05$oD7hbiQFQlvCM7zoalo/T.MssV3VNTRI3w5KDnj8NTUKJNWfVpvRq")
 
+    def test_htpasswd_multi(self):
+        self._test_htpasswd("plain", "ign:ign\ntmp:bepo")
+
+    def test_htpasswd_whitespace(self):
+        self._test_htpasswd("plain", " tmp : bepo ", (
+            (" tmp ", " bepo ", 207), ("tmp", "bepo", 401)))
+
     def test_remote_user(self):
         self.configuration["auth"]["type"] = "remote_user"
         self.application = Application(self.configuration, self.logger)