diff --git a/radicale/auth.py b/radicale/auth.py index 40bec60..af8b303 100644 --- a/radicale/auth.py +++ b/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) diff --git a/radicale/tests/test_auth.py b/radicale/tests/test_auth.py index 7e27d89..0dd6738 100644 --- a/radicale/tests/test_auth.py +++ b/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)