Merge pull request #4641 from dr-antimonious/master

[feat] Store hashed pwds in server config

Author: bruntib

docs/web/authentication.md

@@ -102,11 +102,27 @@ option of `CodeChecker server` command.
 
 ## <i>Dictionary</i> authentication <a name="dictionary-authentication"></a>
 
-> *Note*: Storing passwords in a plain text file is *strongly discouraged* due to security risks. If no other option is available, ensure that the file permissions are restricted to 0600 to limit access *only* to the file owner.
-
-The `authentication.method_dictionary` contains a plaintext `username:password`
-credentials for authentication. If the user's login matches any of the
-credentials listed, the user will be authenticated.
+> *Note*: Storing passwords in a plain text file is *strongly discouraged* due to security risks.
+If no other option is available, ensure that the file permissions are restricted to 0600
+to limit access *only* to the file owner.
+
+The `authentication.method_dictionary` may contain several formats of user credentials:
+
+- `username:password`
+  - legacy user credential format, **not recommended**
+- `username:password_hash:hash_algorithm`
+  - recommended user credential format
+- `username:password_hash:hash_algorithm:salt`
+  - highest security user credential format, `password_hash`
+    is calculated by appending `salt` to the provided password
+
+Supported `hash_algorithm` string values depend on hash algorithms supported by
+the [hashlib](https://docs.python.org/3/library/hashlib.html#hash-algorithms)
+Python module. Examples of supported `hash_algorithm` values:
+- `sha224`, `sha256`, `sha384`, `sha512`
+- added in Python 3.6: `sha3_224`, `sha3_256`, `sha3_384`, `sha3_512`
+
+If the user's login matches any of the credentials listed, the user will be authenticated.
 
 Groups are configured in a map which maps to each username the list of groups
 the user belongs to.

web/server/codechecker_server/session_manager.py

@@ -15,6 +15,7 @@
 import uuid
 
 from datetime import datetime
+import hashlib
 from typing import Optional
 
 from codechecker_common.compatibility.multiprocessing import cpu_count
@@ -606,12 +607,41 @@ def __try_auth_dictionary(self, auth_string):
         if not method_config:
             return False
 
-        valid = self.__is_method_enabled('dictionary') and \
-            auth_string in method_config.get('auths')
-        if not valid:
+        if not self.__is_method_enabled('dictionary'):
             return False
 
-        username = SessionManager.get_user_name(auth_string)
+        auth_string_split = auth_string.split(':', 1)
+        username = auth_string_split[0]
+
+        saved_auth_string = [auth for auth in method_config.get('auths')
+                             if auth.split(':', 1)[0] == username]
+        if len(saved_auth_string) == 0:
+            return False
+
+        saved_auth_string = saved_auth_string[0]
+        if saved_auth_string != auth_string:
+            saved_auth_string_split = saved_auth_string.split(':', 3)
+            try:
+                hash_algorithm = saved_auth_string_split[2]
+            except IndexError:
+                return False
+
+            if not hasattr(hashlib, hash_algorithm):
+                return False
+
+            password = auth_string_split[1]
+            try:
+                salt = saved_auth_string_split[3]
+                password += salt
+            except IndexError:
+                pass
+
+            password = password.encode("utf-8")
+            saved_password_hash = saved_auth_string_split[1]
+            if not saved_password_hash == \
+                    getattr(hashlib, hash_algorithm)(password).hexdigest():
+                return False
+
         group_list = method_config['groups'][username] if \
             'groups' in method_config and \
             username in method_config['groups'] else []

web/tests/functional/authentication/test_authentication.py

@@ -76,6 +76,26 @@ def test_privileged_access(self):
             auth_client.performLogin("Username:Password", None)
             print("Empty credentials gave us a token!")
 
+        with self.assertRaises(RequestFailed):
+            auth_client.performLogin("Username:Password", "colon:my:pass:word")
+            print("Incorrect password gave us a token!")
+
+        with self.assertRaises(RequestFailed):
+            auth_client.performLogin("Username:Password", "colon:mypassword")
+            print("Incorrect password gave us a token!")
+
+        with self.assertRaises(RequestFailed):
+            auth_client.performLogin("Username:Password",
+                                     "hashtest1:hashtest1")
+            print(("Pre-saved credentials with invalid "
+                   "hash algorithm gave us a token!"))
+
+        with self.assertRaises(RequestFailed):
+            auth_client.performLogin("Username:Password",
+                                     "hashtest2:hashtest2")
+            print("Pre-saved credentials with invalid "
+                  "hash value gave us a token!")
+
         # A non-authenticated session should return an empty user.
         user = auth_client.getLoggedInUser()
         self.assertEqual(user, "")
@@ -136,14 +156,22 @@ def test_privileged_access(self):
 
         self.assertTrue(result, "Server did not allow us to destroy session.")
 
-        self.session_token = auth_client.performLogin(
-            "Username:Password", "colon:my:password")
-        self.assertIsNotNone(self.session_token,
-                             "Valid credentials didn't give us a token!")
+        valid_credentials = ["colon:my:password",
+                             "colon123:my:password",
+                             "hashtest3:hashtest3",
+                             "hashtest4:hashtest4",
+                             "hashtest5:hashtest5"]
 
-        result = auth_client.destroySession()
+        for credential in valid_credentials:
+            self.session_token = auth_client.performLogin(
+                "Username:Password", credential)
+            self.assertIsNotNone(self.session_token,
+                                 "Valid credentials didn't give us a token!")
 
-        self.assertTrue(result, "Server did not allow us to destroy session.")
+            result = auth_client.destroySession()
+
+            self.assertTrue(result,
+                            "Server did not allow us to destroy session.")
 
         # Kill the session token that was created by login() too.
         codechecker.logout(self._test_cfg['codechecker_cfg'],

web/tests/libtest/env.py

@@ -362,9 +362,20 @@ def enable_auth(workspace):
     scfg_dict["authentication"]["super_user"] = "root"
     scfg_dict["authentication"]["method_dictionary"]["enabled"] = True
     scfg_dict["authentication"]["method_dictionary"]["auths"] = \
-        ["cc:test", "john:doe", "admin:admin123", "colon:my:password",
-         "admin_group_user:admin123", "regex_admin:blah",
-         "permission_view_user:pvu", "root:root"]
+        ["cc:test", "john:doe", "admin:admin123", "colon123:my:password",
+         "colon:my:password", "admin_group_user:admin123",
+         "regex_admin:blah", "permission_view_user:pvu", "root:root",
+         "hashtest1:hashtest1:this_will_fail",
+         "hashtest2:this_will_fail_too:sha512",
+         ("hashtest3:9d49be0aa9430dc908e6f6ecd1eff1c253e3aefd6df7ea"
+          "daeb2a66b797d9bba842f16963d4cc7a8dbb1b61c0f75cabb52f48a9"
+          "0d6b57b453ae4f85c4352e269f:sha512"),
+         ("hashtest4:8b440a15aba9665761a279b7cd12659bf1b6527bdbe6e4"
+          "3c2ef97026a05d1efe9321b6aa6fec32c2f00aaebc2baa6aab5dc54b"
+          "bd4c9f9adc0d7d3744f5b7f3df:sha3_512"),
+         ("hashtest5:33a3060019fb2bb16b4eb9eb9ec59bee4ccc658a9e3186"
+          "68e6ff0b142d523a0de571adf979428872eb2eb3fd34821687e09b92"
+          "f765ebc5ddbf9ea3cae76d292f:sha3_512:with:salt")]
     scfg_dict["authentication"]["method_dictionary"]["groups"] = \
         {"admin_group_user": ["admin_GROUP"]}
     scfg_dict["authentication"]["regex_groups"]["enabled"] = True