diff --git a/CHANGELOG.md b/CHANGELOG.md index 41fdb69f2..9c0c2a833 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/monkey/monkey_island/Pipfile b/monkey/monkey_island/Pipfile index 70c0f304c..65eeaae4b 100644 --- a/monkey/monkey_island/Pipfile +++ b/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" diff --git a/monkey/monkey_island/Pipfile.lock b/monkey/monkey_island/Pipfile.lock index 55f5e4502..46479e643 100644 --- a/monkey/monkey_island/Pipfile.lock +++ b/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", diff --git a/monkey/monkey_island/cc/environment/environment_config.py b/monkey/monkey_island/cc/environment/environment_config.py index 70d27e546..1f9602d22 100644 --- a/monkey/monkey_island/cc/environment/environment_config.py +++ b/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) diff --git a/monkey/monkey_island/cc/environment/standard.py b/monkey/monkey_island/cc/environment/standard.py index 35ca84a34..2f337fbf0 100644 --- a/monkey/monkey_island/cc/environment/standard.py +++ b/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_CREDS = ( - "55e97c9dcfd22b8079189ddaeea9bce8125887e3237b800c6176c9afa80d2062" - "8d2c8d0b1538d2208c1444ac66535b764a3d902b35e751df3faec1e477ed3557" - ) + NO_AUTH_USER = "1234567890!@#$%^&*()_nothing_up_my_sleeve_1234567890!@#$%^&*()" + NO_AUTH_SECRET = "$2b$12$frH7uEwV3jkDNGgReW6j2udw8hy/Yw1SWAqytrcBYK48kn1V5lQIa" 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)] diff --git a/monkey/monkey_island/cc/environment/user_creds.py b/monkey/monkey_island/cc/environment/user_creds.py index 98a23a14a..aba349f2d 100644 --- a/monkey/monkey_island/cc/environment/user_creds.py +++ b/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) diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 29d2d9e89..064395eaf 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/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) - # Unpack auth info from request - username = credentials["username"] - secret = credentials["password"] - # 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]}" - ) + (username, password) = _get_credentials_from_request(request) + + if _credentials_match_registered_user(username, password): + access_token = _create_access_token(username) 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) diff --git a/monkey/monkey_island/cc/resources/auth/password_utils.py b/monkey/monkey_island/cc/resources/auth/password_utils.py new file mode 100644 index 000000000..f470fd882 --- /dev/null +++ b/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")) diff --git a/monkey/monkey_island/cc/resources/auth/registration.py b/monkey/monkey_island/cc/resources/auth/registration.py index e5ca99232..121b03d71 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/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) diff --git a/monkey/monkey_island/cc/ui/src/services/AuthService.js b/monkey/monkey_island/cc/ui/src/services/AuthService.js index 52658f5a9..d7d1b9c2f 100644 --- a/monkey/monkey_island/cc/ui/src/services/AuthService.js +++ b/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 = - '55e97c9dcfd22b8079189ddaeea9bce8125887e3237b800c6176c9afa80d2062' + - '8d2c8d0b1538d2208c1444ac66535b764a3d902b35e751df3faec1e477ed3557'; + NO_AUTH_CREDS = '1234567890!@#$%^&*()_nothing_up_my_sleeve_1234567890!@#$%^&*()'; 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, this.hashSha3(password)); - } else { - return this._register(username, password); - } + 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)), ''); - } } diff --git a/monkey/tests/monkey_island/cc/environment/test_environment.py b/monkey/tests/monkey_island/cc/environment/test_environment.py index 0c98d1ccf..6b648ddee 100644 --- a/monkey/tests/monkey_island/cc/environment/test_environment.py +++ b/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) diff --git a/monkey/tests/monkey_island/cc/environment/test_user_creds.py b/monkey/tests/monkey_island/cc/environment/test_user_creds.py index 93da16e24..802c13416 100644 --- a/monkey/tests/monkey_island/cc/environment/test_user_creds.py +++ b/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") - self.assertDictEqual(user_creds.to_dict(), {"user": "Test"}) +def test_bool_true(): + 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") - self.assertDictEqual(user_creds.to_dict(), {"user": "Test", "password_hash": "abc1231234"}) +def test_bool_false_empty_password_hash(): + 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") - auth_user = user_creds.to_auth_user() - self.assertEqual(auth_user.id, 1) - self.assertEqual(auth_user.username, "Test") - self.assertEqual(auth_user.secret, "") +def test_bool_false_empty_user(): + assert not UserCreds("", TEST_HASH) - user_creds = UserCreds(password_hash="abc1231234") - auth_user = user_creds.to_auth_user() - self.assertEqual(auth_user.id, 1) - self.assertEqual(auth_user.username, "") - self.assertEqual(auth_user.secret, "abc1231234") + +def test_bool_false_empty_user_and_password_hash(): + assert not UserCreds("", "") + + +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