Merge branch 'pwd-hash' into develop

CHANGELOG.md

@@ -26,6 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
  performance tests are skipped.
 - Zerologon exploiter writes runtime artifacts to a secure temporary directory
   instead of $HOME. #1143
+- Authentication mechanism to use bcrypt on server side. #1139
 
 ### Fixed
 - Attempted to delete a directory when monkey config reset was called. #1054

monkey/monkey_island/Pipfile

@@ -6,6 +6,7 @@ name = "pypi"
 [packages]
 pyinstaller = "==3.6"
 awscli = "==1.18.131"
+bcrypt = "==3.2.0"
 boto3 = "==1.14.54"
 botocore = "==1.17.54"
 cffi = ">=1.8,!=1.11.3"

monkey/monkey_island/Pipfile.lock

@@ -1,7 +1,7 @@
 {
     "_meta": {
         "hash": {
-            "sha256": "4fdfe90af14139cf855a0363ad0acbe7fb307b35b038e2c099c4d1227322a13b"
+            "sha256": "3057235b1e85593ee307d9e5a2e0d15e26f13437bb709529303c7c900d3c7b41"
         },
         "pipfile-spec": 6,
         "requires": {
@@ -60,6 +60,19 @@
             "index": "pypi",
             "version": "==1.18.131"
         },
+        "bcrypt": {
+            "hashes": [
+                "sha256:5b93c1726e50a93a033c36e5ca7fdcd29a5c7395af50a6892f5d9e7c6cfbfb29",
+                "sha256:63d4e3ff96188e5898779b6057878fecf3f11cfe6ec3b313ea09955d587ec7a7",
+                "sha256:81fec756feff5b6818ea7ab031205e1d323d8943d237303baca2c5f9c7846f34",
+                "sha256:a67fb841b35c28a59cebed05fbd3e80eea26e6d75851f0574a9273c80f3e9b55",
+                "sha256:c95d4cbebffafcdd28bd28bb4e25b31c50f6da605c81ffd9ad8a3d1b2ab7b1b6",
+                "sha256:cd1ea2ff3038509ea95f687256c46b79f5fc382ad0aa3664d200047546d511d1",
+                "sha256:cdcdcb3972027f83fe24a48b1e90ea4b584d35f1cc279d76de6fc4b13376239d"
+            ],
+            "index": "pypi",
+            "version": "==3.2.0"
+        },
         "boto3": {
             "hashes": [
                 "sha256:4196b418598851ffd10cf9d1606694673cbfeca4ddf8b25d4e50addbd2fc60bf",

monkey/monkey_island/cc/environment/environment_config.py

@@ -38,13 +38,12 @@ class EnvironmentConfig:
         self._load_from_dict(data)
 
     def _load_from_dict(self, dict_data: Dict):
-        user_creds = UserCreds.get_from_dict(dict_data)
         aws = dict_data["aws"] if "aws" in dict_data else None
         data_dir = dict_data["data_dir"] if "data_dir" in dict_data else DEFAULT_DATA_DIR
 
         self.server_config = dict_data["server_config"]
         self.deployment = dict_data["deployment"]
-        self.user_creds = user_creds
+        self.user_creds = _get_user_credentials_from_config(dict_data)
         self.aws = aws
         self.data_dir = data_dir
 
@@ -75,3 +74,10 @@ class EnvironmentConfig:
     def get_users(self) -> List[User]:
         auth_user = self.user_creds.to_auth_user()
         return [auth_user] if auth_user else []
+
+
+def _get_user_credentials_from_config(dict_data: Dict):
+    username = dict_data.get("user", "")
+    password_hash = dict_data.get("password_hash", "")
+
+    return UserCreds(username, password_hash)

monkey/monkey_island/cc/environment/standard.py

@@ -7,11 +7,8 @@ __author__ = "itay.mizeretz"
 class StandardEnvironment(Environment):
     _credentials_required = False
 
-    # SHA3-512 of '1234567890!@#$%^&*()_nothing_up_my_sleeve_1234567890!@#$%^&*()'
+    NO_AUTH_USER = "1234567890!@#$%^&*()_nothing_up_my_sleeve_1234567890!@#$%^&*()"
-    NO_AUTH_CREDS = (
+    NO_AUTH_SECRET = "$2b$12$frH7uEwV3jkDNGgReW6j2udw8hy/Yw1SWAqytrcBYK48kn1V5lQIa"
-        "55e97c9dcfd22b8079189ddaeea9bce8125887e3237b800c6176c9afa80d2062"
-        "8d2c8d0b1538d2208c1444ac66535b764a3d902b35e751df3faec1e477ed3557"
-    )
 
     def get_auth_users(self):
-        return [User(1, StandardEnvironment.NO_AUTH_CREDS, StandardEnvironment.NO_AUTH_CREDS)]
+        return [User(1, StandardEnvironment.NO_AUTH_USER, StandardEnvironment.NO_AUTH_SECRET)]

monkey/monkey_island/cc/environment/user_creds.py

@@ -1,13 +1,12 @@
 from __future__ import annotations
 
-import json
 from typing import Dict
 
 from monkey_island.cc.resources.auth.auth_user import User
 
 
 class UserCreds:
-    def __init__(self, username="", password_hash=""):
+    def __init__(self, username, password_hash):
         self.username = username
         self.password_hash = password_hash
 
@@ -24,17 +23,3 @@ class UserCreds:
 
     def to_auth_user(self) -> User:
         return User(1, self.username, self.password_hash)
-
-    @staticmethod
-    def get_from_dict(data_dict: Dict) -> UserCreds:
-        creds = UserCreds()
-        if "user" in data_dict:
-            creds.username = data_dict["user"]
-        if "password_hash" in data_dict:
-            creds.password_hash = data_dict["password_hash"]
-        return creds
-
-    @staticmethod
-    def get_from_json(json_data: bytes) -> UserCreds:
-        cred_dict = json.loads(json_data)
-        return UserCreds.get_from_dict(cred_dict)

monkey/monkey_island/cc/resources/auth/auth.py

@@ -7,9 +7,9 @@ import flask_restful
 from flask import make_response, request
 from flask_jwt_extended.exceptions import JWTExtendedException
 from jwt import PyJWTError
-from werkzeug.security import safe_str_cmp
 
 import monkey_island.cc.environment.environment_singleton as env_singleton
+import monkey_island.cc.resources.auth.password_utils as password_utils
 import monkey_island.cc.resources.auth.user_store as user_store
 
 logger = logging.getLogger(__name__)
@@ -25,42 +25,55 @@ def init_jwt(app):
 
 class Authenticate(flask_restful.Resource):
     """
-    Resource for user authentication. The user provides the username and hashed password and we
+    Resource for user authentication. The user provides the username and password and we
     give them a JWT.
     See `AuthService.js` file for the frontend counterpart for this code.
     """
 
-    @staticmethod
-    def _authenticate(username, secret):
-        user = user_store.UserStore.username_table.get(username, None)
-        if user and safe_str_cmp(user.secret.encode("utf-8"), secret.encode("utf-8")):
-            return user
-
     def post(self):
         """
         Example request:
         {
             "username": "my_user",
-            "password": "343bb87e553b05430e5c44baf99569d4b66..."
+            "password": "my_password"
         }
         """
-        credentials = json.loads(request.data)
+        (username, password) = _get_credentials_from_request(request)
-        # Unpack auth info from request
+
-        username = credentials["username"]
+        if _credentials_match_registered_user(username, password):
-        secret = credentials["password"]
+            access_token = _create_access_token(username)
-        # If the user and password have been previously registered
-        if self._authenticate(username, secret):
-            access_token = flask_jwt_extended.create_access_token(
-                identity=user_store.UserStore.username_table[username].id
-            )
-            logger.debug(
-                f"Created access token for user {username} that begins with {access_token[:4]}"
-            )
             return make_response({"access_token": access_token, "error": ""}, 200)
         else:
             return make_response({"error": "Invalid credentials"}, 401)
 
 
+def _get_credentials_from_request(request):
+    credentials = json.loads(request.data)
+
+    username = credentials["username"]
+    password = credentials["password"]
+
+    return (username, password)
+
+
+def _credentials_match_registered_user(username, password):
+    user = user_store.UserStore.username_table.get(username, None)
+
+    if user and password_utils.password_matches_hash(password, user.secret):
+        return True
+
+    return False
+
+
+def _create_access_token(username):
+    access_token = flask_jwt_extended.create_access_token(
+        identity=user_store.UserStore.username_table[username].id
+    )
+    logger.debug(f"Created access token for user {username} that begins with {access_token[:4]}")
+
+    return access_token
+
+
 # See https://flask-jwt-extended.readthedocs.io/en/stable/custom_decorators/
 def jwt_required(fn):
     @wraps(fn)

monkey/monkey_island/cc/resources/auth/password_utils.py

@@ -0,0 +1,12 @@
+import bcrypt
+
+
+def hash_password(plaintext_password):
+    salt = bcrypt.gensalt()
+    password_hash = bcrypt.hashpw(plaintext_password.encode("utf-8"), salt)
+
+    return password_hash.decode()
+
+
+def password_matches_hash(plaintext_password, password_hash):
+    return bcrypt.checkpw(plaintext_password.encode("utf-8"), password_hash.encode("utf-8"))

monkey/monkey_island/cc/resources/auth/registration.py

@@ -1,7 +1,10 @@
+import json
+
 import flask_restful
 from flask import make_response, request
 
 import monkey_island.cc.environment.environment_singleton as env_singleton
+import monkey_island.cc.resources.auth.password_utils as password_utils
 from common.utils.exceptions import InvalidRegistrationCredentialsError, RegistrationNotNeededError
 from monkey_island.cc.environment.user_creds import UserCreds
 
@@ -11,9 +14,20 @@ class Registration(flask_restful.Resource):
         return {"needs_registration": env_singleton.env.needs_registration()}
 
     def post(self):
-        credentials = UserCreds.get_from_json(request.data)
+        credentials = _get_user_credentials_from_request(request)
+
         try:
             env_singleton.env.try_add_user(credentials)
             return make_response({"error": ""}, 200)
         except (InvalidRegistrationCredentialsError, RegistrationNotNeededError) as e:
             return make_response({"error": str(e)}, 400)
+
+
+def _get_user_credentials_from_request(request):
+    cred_dict = json.loads(request.data)
+
+    username = cred_dict.get("user", "")
+    password = cred_dict.get("password", "")
+    password_hash = password_utils.hash_password(password)
+
+    return UserCreds(username, password_hash)

monkey/monkey_island/cc/ui/src/services/AuthService.js

@@ -1,18 +1,14 @@
-import {SHA3} from 'sha3';
 import decode from 'jwt-decode';
 
 export default class AuthService {
-  // SHA3-512 of '1234567890!@#$%^&*()_nothing_up_my_sleeve_1234567890!@#$%^&*()'
+  NO_AUTH_CREDS = '1234567890!@#$%^&*()_nothing_up_my_sleeve_1234567890!@#$%^&*()';
-  NO_AUTH_CREDS =
-    '55e97c9dcfd22b8079189ddaeea9bce8125887e3237b800c6176c9afa80d2062' +
-    '8d2c8d0b1538d2208c1444ac66535b764a3d902b35e751df3faec1e477ed3557';
 
   SECONDS_BEFORE_JWT_EXPIRES = 20;
   AUTHENTICATION_API_ENDPOINT = '/api/auth';
   REGISTRATION_API_ENDPOINT = '/api/registration';
 
   login = (username, password) => {
-    return this._login(username, this.hashSha3(password));
+    return this._login(username, password);
   };
 
   authFetch = (url, options) => {
@@ -25,12 +21,6 @@ export default class AuthService {
     }
   };
 
-  hashSha3(text) {
-    let hash = new SHA3(512);
-    hash.update(text);
-    return this._toHexStr(hash.digest());
-  }
-
   _login = (username, password) => {
     return this._authFetch(this.AUTHENTICATION_API_ENDPOINT, {
       method: 'POST',
@@ -51,11 +41,7 @@ export default class AuthService {
   };
 
   register = (username, password) => {
-    if (password !== '') {
+    return this._register(username, password);
-      return this._register(username, this.hashSha3(password));
-    } else {
-      return this._register(username, password);
-    }
   };
 
   _register = (username, password) => {
@@ -63,7 +49,7 @@ export default class AuthService {
       method: 'POST',
       body: JSON.stringify({
         'user': username,
-        'password_hash': password
+        'password': password
       })
     }).then(res => {
       if (res.status === 200) {
@@ -156,7 +142,4 @@ export default class AuthService {
     return localStorage.getItem('jwt')
   }
 
-  _toHexStr(byteArr) {
-    return byteArr.reduce((acc, x) => (acc + ('0' + x.toString(0x10)).slice(-2)), '');
-  }
 }

monkey/tests/monkey_island/cc/environment/test_environment.py

@@ -20,6 +20,9 @@ PARTIAL_CREDENTIALS = None
 STANDARD_WITH_CREDENTIALS = None
 STANDARD_ENV = None
 
+EMPTY_USER_CREDENTIALS = UserCreds("", "")
+FULL_USER_CREDENTIALS = UserCreds(username="test", password_hash="1231234")
+
 
 # This fixture is a dirty hack that can be removed once these tests are converted from
 # unittest to pytest. Instead, the appropriate fixtures from conftest.py can be used.
@@ -67,7 +70,7 @@ def get_server_config_file_path_test_version():
 class TestEnvironment(TestCase):
     class EnvironmentCredentialsNotRequired(Environment):
         def __init__(self):
-            config = StubEnvironmentConfig("test", "test", UserCreds())
+            config = StubEnvironmentConfig("test", "test", EMPTY_USER_CREDENTIALS)
             super().__init__(config)
 
         _credentials_required = False
@@ -77,7 +80,7 @@ class TestEnvironment(TestCase):
 
     class EnvironmentCredentialsRequired(Environment):
         def __init__(self):
-            config = StubEnvironmentConfig("test", "test", UserCreds())
+            config = StubEnvironmentConfig("test", "test", EMPTY_USER_CREDENTIALS)
             super().__init__(config)
 
         _credentials_required = True
@@ -98,15 +101,15 @@ class TestEnvironment(TestCase):
     @patch.object(target=EnvironmentConfig, attribute="save_to_file", new=MagicMock())
     def test_try_add_user(self):
         env = TestEnvironment.EnvironmentCredentialsRequired()
-        credentials = UserCreds(username="test", password_hash="1231234")
+        credentials = FULL_USER_CREDENTIALS
         env.try_add_user(credentials)
 
-        credentials = UserCreds(username="test")
+        credentials = UserCreds(username="test", password_hash="")
         with self.assertRaises(InvalidRegistrationCredentialsError):
             env.try_add_user(credentials)
 
         env = TestEnvironment.EnvironmentCredentialsNotRequired()
-        credentials = UserCreds(username="test", password_hash="1231234")
+        credentials = FULL_USER_CREDENTIALS
         with self.assertRaises(RegistrationNotNeededError):
             env.try_add_user(credentials)
 

monkey/tests/monkey_island/cc/environment/test_user_creds.py

@@ -1,37 +1,45 @@
-from unittest import TestCase
-
 from monkey_island.cc.environment.user_creds import UserCreds
 
+TEST_USER = "Test"
+TEST_HASH = "abc1231234"
+TEST_SALT = b"$2b$12$JA7GdT1iyfIsquF2cTZv2."
 
-class TestUserCreds(TestCase):
-    def test_to_dict(self):
-        user_creds = UserCreds()
-        self.assertDictEqual(user_creds.to_dict(), {})
 
-        user_creds = UserCreds(username="Test")
+def test_bool_true():
-        self.assertDictEqual(user_creds.to_dict(), {"user": "Test"})
+    assert UserCreds(TEST_USER, TEST_HASH)
 
-        user_creds = UserCreds(password_hash="abc1231234")
-        self.assertDictEqual(user_creds.to_dict(), {"password_hash": "abc1231234"})
 
-        user_creds = UserCreds(username="Test", password_hash="abc1231234")
+def test_bool_false_empty_password_hash():
-        self.assertDictEqual(user_creds.to_dict(), {"user": "Test", "password_hash": "abc1231234"})
+    assert not UserCreds(TEST_USER, "")
 
-    def test_to_auth_user(self):
-        user_creds = UserCreds(username="Test", password_hash="abc1231234")
-        auth_user = user_creds.to_auth_user()
-        self.assertEqual(auth_user.id, 1)
-        self.assertEqual(auth_user.username, "Test")
-        self.assertEqual(auth_user.secret, "abc1231234")
 
-        user_creds = UserCreds(username="Test")
+def test_bool_false_empty_user():
-        auth_user = user_creds.to_auth_user()
+    assert not UserCreds("", TEST_HASH)
-        self.assertEqual(auth_user.id, 1)
-        self.assertEqual(auth_user.username, "Test")
-        self.assertEqual(auth_user.secret, "")
 
-        user_creds = UserCreds(password_hash="abc1231234")
+
-        auth_user = user_creds.to_auth_user()
+def test_bool_false_empty_user_and_password_hash():
-        self.assertEqual(auth_user.id, 1)
+    assert not UserCreds("", "")
-        self.assertEqual(auth_user.username, "")
+
-        self.assertEqual(auth_user.secret, "abc1231234")
+
+def test_to_dict_empty_creds():
+    user_creds = UserCreds("", "")
+    assert user_creds.to_dict() == {}
+
+
+def test_to_dict_full_creds():
+    user_creds = UserCreds(TEST_USER, TEST_HASH)
+    assert user_creds.to_dict() == {"user": TEST_USER, "password_hash": TEST_HASH}
+
+
+def test_to_auth_user_full_credentials():
+    user_creds = UserCreds(TEST_USER, TEST_HASH)
+    auth_user = user_creds.to_auth_user()
+    assert auth_user.id == 1
+    assert auth_user.username == TEST_USER
+    assert auth_user.secret == TEST_HASH
+
+
+def test_member_values(monkeypatch):
+    creds = UserCreds(TEST_USER, TEST_HASH)
+    assert creds.username == TEST_USER
+    assert creds.password_hash == TEST_HASH