From eaddcdcf13048bbe4868ad84c2b4028db2083f2d Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 12 Jul 2022 10:42:57 -0400 Subject: [PATCH 1/3] Island: Unlock RepositoryEncryptor in AuthenticationService --- .../cc/services/authentication_service.py | 24 +++-- .../services/test_authentication_service.py | 89 ++++++++++++------- 2 files changed, 75 insertions(+), 38 deletions(-) diff --git a/monkey/monkey_island/cc/services/authentication_service.py b/monkey/monkey_island/cc/services/authentication_service.py index 045ef75b2..200038708 100644 --- a/monkey/monkey_island/cc/services/authentication_service.py +++ b/monkey/monkey_island/cc/services/authentication_service.py @@ -10,6 +10,7 @@ from common.utils.exceptions import ( from monkey_island.cc.models import UserCredentials from monkey_island.cc.repository import IUserRepository from monkey_island.cc.server_utils.encryption import ( + ILockableEncryptor, reset_datastore_encryptor, unlock_datastore_encryptor, ) @@ -17,9 +18,15 @@ from monkey_island.cc.setup.mongo.database_initializer import reset_database class AuthenticationService: - def __init__(self, data_dir: Path, user_datastore: IUserRepository): + def __init__( + self, + data_dir: Path, + user_datastore: IUserRepository, + repository_encryptor: ILockableEncryptor, + ): self._data_dir = data_dir self._user_datastore = user_datastore + self._repository_encryptor = repository_encryptor def needs_registration(self) -> bool: return not self._user_datastore.has_registered_users() @@ -30,7 +37,7 @@ class AuthenticationService: credentials = UserCredentials(username, _hash_password(password)) self._user_datastore.add_user(credentials) - self._reset_datastore_encryptor(username, password) + self._reset_repository_encryptor(username, password) reset_database() def authenticate(self, username: str, password: str): @@ -42,14 +49,21 @@ class AuthenticationService: if not _credentials_match_registered_user(username, password, registered_user): raise IncorrectCredentialsError() - self._unlock_datastore_encryptor(username, password) + self._unlock_repository_encryptor(username, password) - def _unlock_datastore_encryptor(self, username: str, password: str): + def _unlock_repository_encryptor(self, username: str, password: str): secret = _get_secret_from_credentials(username, password) + self._repository_encryptor.unlock(secret.encode()) + + # Legacy datastore encryptor will be removed soon unlock_datastore_encryptor(self._data_dir, secret) - def _reset_datastore_encryptor(self, username: str, password: str): + def _reset_repository_encryptor(self, username: str, password: str): secret = _get_secret_from_credentials(username, password) + self._repository_encryptor.reset_key() + self._repository_encryptor.unlock(secret.encode()) + + # Legacy datastore encryptor will be removed soon reset_datastore_encryptor(self._data_dir, secret) diff --git a/monkey/tests/unit_tests/monkey_island/cc/services/test_authentication_service.py b/monkey/tests/unit_tests/monkey_island/cc/services/test_authentication_service.py index 558d17a84..5f927a4f8 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/services/test_authentication_service.py +++ b/monkey/tests/unit_tests/monkey_island/cc/services/test_authentication_service.py @@ -10,6 +10,7 @@ from common.utils.exceptions import ( ) from monkey_island.cc.models import UserCredentials from monkey_island.cc.repository import IUserRepository +from monkey_island.cc.server_utils.encryption import ILockableEncryptor from monkey_island.cc.services import AuthenticationService, authentication_service USERNAME = "user1" @@ -48,6 +49,11 @@ def mock_unlock_datastore_encryptor(): return MagicMock() +@pytest.fixture +def mock_repository_encryptor(): + return MagicMock(spec=ILockableEncryptor) + + @pytest.fixture(autouse=True) def patch_datastore_utils( monkeypatch, @@ -64,20 +70,20 @@ def patch_datastore_utils( ) -def test_needs_registration__true(tmp_path): +def test_needs_registration__true(tmp_path, mock_repository_encryptor): has_registered_users = False mock_user_datastore = MockUserDatastore(lambda: has_registered_users, None, None) - a_s = AuthenticationService(tmp_path, mock_user_datastore) + a_s = AuthenticationService(tmp_path, mock_user_datastore, mock_repository_encryptor) assert a_s.needs_registration() -def test_needs_registration__false(tmp_path): +def test_needs_registration__false(tmp_path, mock_repository_encryptor): has_registered_users = True mock_user_datastore = MockUserDatastore(lambda: has_registered_users, None, None) - a_s = AuthenticationService(tmp_path, mock_user_datastore) + a_s = AuthenticationService(tmp_path, mock_user_datastore, mock_repository_encryptor) assert not a_s.needs_registration() @@ -85,39 +91,45 @@ def test_needs_registration__false(tmp_path): @pytest.mark.slow @pytest.mark.parametrize("error", [InvalidRegistrationCredentialsError, AlreadyRegisteredError]) def test_register_new_user__fails( - tmp_path, mock_reset_datastore_encryptor, mock_reset_database, error + tmp_path, mock_reset_datastore_encryptor, mock_reset_database, mock_repository_encryptor, error ): mock_user_datastore = MockUserDatastore(lambda: True, MagicMock(side_effect=error), None) - a_s = AuthenticationService(tmp_path, mock_user_datastore) + a_s = AuthenticationService(tmp_path, mock_user_datastore, mock_repository_encryptor) with pytest.raises(error): a_s.register_new_user(USERNAME, PASSWORD) mock_reset_datastore_encryptor.assert_not_called() + mock_repository_encryptor.reset_key().assert_not_called() + mock_repository_encryptor.unlock.assert_not_called() mock_reset_database.assert_not_called() def test_register_new_user__empty_password_fails( - tmp_path, mock_reset_datastore_encryptor, mock_reset_database + tmp_path, mock_reset_datastore_encryptor, mock_reset_database, mock_repository_encryptor ): mock_user_datastore = MockUserDatastore(lambda: False, None, None) - a_s = AuthenticationService(tmp_path, mock_user_datastore) + a_s = AuthenticationService(tmp_path, mock_user_datastore, mock_repository_encryptor) with pytest.raises(InvalidRegistrationCredentialsError): a_s.register_new_user(USERNAME, "") mock_reset_datastore_encryptor.assert_not_called() + mock_repository_encryptor.reset_key().assert_not_called() + mock_repository_encryptor.unlock.assert_not_called() mock_reset_database.assert_not_called() @pytest.mark.slow -def test_register_new_user(tmp_path, mock_reset_datastore_encryptor, mock_reset_database): +def test_register_new_user( + tmp_path, mock_reset_datastore_encryptor, mock_reset_database, mock_repository_encryptor +): mock_add_user = MagicMock() mock_user_datastore = MockUserDatastore(lambda: False, mock_add_user, None) - a_s = AuthenticationService(tmp_path, mock_user_datastore) + a_s = AuthenticationService(tmp_path, mock_user_datastore, mock_repository_encryptor) a_s.register_new_user(USERNAME, PASSWORD) @@ -127,30 +139,16 @@ def test_register_new_user(tmp_path, mock_reset_datastore_encryptor, mock_reset_ mock_reset_datastore_encryptor.assert_called_once() assert mock_reset_datastore_encryptor.call_args[0][1] != USERNAME + mock_repository_encryptor.reset_key.assert_called_once() + mock_repository_encryptor.unlock.assert_called_once() + assert mock_repository_encryptor.unlock.call_args[0][0] != USERNAME + mock_reset_database.assert_called_once() @pytest.mark.slow -def test_authenticate__success(tmp_path, mock_unlock_datastore_encryptor): - mock_user_datastore = MockUserDatastore( - lambda: True, - None, - lambda _: UserCredentials(USERNAME, PASSWORD_HASH), - ) - - a_s = AuthenticationService(tmp_path, mock_user_datastore) - - # If authentication fails, this function will raise an exception and the test will fail. - a_s.authenticate(USERNAME, PASSWORD) - mock_unlock_datastore_encryptor.assert_called_once() - - -@pytest.mark.slow -@pytest.mark.parametrize( - ("username", "password"), [("wrong_username", PASSWORD), (USERNAME, "wrong_password")] -) -def test_authenticate__failed_wrong_credentials( - tmp_path, mock_unlock_datastore_encryptor, username, password +def test_authenticate__success( + tmp_path, mock_unlock_datastore_encryptor, mock_repository_encryptor ): mock_user_datastore = MockUserDatastore( lambda: True, @@ -158,22 +156,47 @@ def test_authenticate__failed_wrong_credentials( lambda _: UserCredentials(USERNAME, PASSWORD_HASH), ) - a_s = AuthenticationService(tmp_path, mock_user_datastore) + a_s = AuthenticationService(tmp_path, mock_user_datastore, mock_repository_encryptor) + + # If authentication fails, this function will raise an exception and the test will fail. + a_s.authenticate(USERNAME, PASSWORD) + mock_unlock_datastore_encryptor.assert_called_once() + mock_repository_encryptor.unlock.assert_called_once() + + +@pytest.mark.slow +@pytest.mark.parametrize( + ("username", "password"), [("wrong_username", PASSWORD), (USERNAME, "wrong_password")] +) +def test_authenticate__failed_wrong_credentials( + tmp_path, mock_unlock_datastore_encryptor, username, password, mock_repository_encryptor +): + mock_user_datastore = MockUserDatastore( + lambda: True, + None, + lambda _: UserCredentials(USERNAME, PASSWORD_HASH), + ) + + a_s = AuthenticationService(tmp_path, mock_user_datastore, mock_repository_encryptor) with pytest.raises(IncorrectCredentialsError): a_s.authenticate(username, password) mock_unlock_datastore_encryptor.assert_not_called() + mock_repository_encryptor.unlock.assert_not_called() -def test_authenticate__failed_no_registered_user(tmp_path, mock_unlock_datastore_encryptor): +def test_authenticate__failed_no_registered_user( + tmp_path, mock_unlock_datastore_encryptor, mock_repository_encryptor +): mock_user_datastore = MockUserDatastore( lambda: True, None, MagicMock(side_effect=UnknownUserError) ) - a_s = AuthenticationService(tmp_path, mock_user_datastore) + a_s = AuthenticationService(tmp_path, mock_user_datastore, mock_repository_encryptor) with pytest.raises(IncorrectCredentialsError): a_s.authenticate(USERNAME, PASSWORD) mock_unlock_datastore_encryptor.assert_not_called() + mock_repository_encryptor.unlock.assert_not_called() From e8ebe845bfc574eef4a6e3ddfa87dad89235b4e3 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 12 Jul 2022 11:25:10 -0400 Subject: [PATCH 2/3] Island: Register RepositoryEncryptor with the DIContainer --- monkey/monkey_island/cc/services/initialize.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/monkey/monkey_island/cc/services/initialize.py b/monkey/monkey_island/cc/services/initialize.py index 4f326f58d..a622b091c 100644 --- a/monkey/monkey_island/cc/services/initialize.py +++ b/monkey/monkey_island/cc/services/initialize.py @@ -31,6 +31,7 @@ from monkey_island.cc.repository import ( RetrievalError, ) from monkey_island.cc.server_utils.consts import MONKEY_ISLAND_ABS_PATH +from monkey_island.cc.server_utils.encryption import ILockableEncryptor, RepositoryEncryptor from monkey_island.cc.services import AWSService, IslandModeService from monkey_island.cc.services.post_breach_files import PostBreachFilesService from monkey_island.cc.services.run_local_monkey import LocalMonkeyRunService @@ -48,6 +49,7 @@ from .reporting.report import ReportService logger = logging.getLogger(__name__) AGENT_BINARIES_PATH = Path(MONKEY_ISLAND_ABS_PATH) / "cc" / "binaries" +REPOSITORY_KEY_FILE_NAME = "repository_key.bin" def initialize_services(data_dir: Path) -> DIContainer: @@ -56,6 +58,9 @@ def initialize_services(data_dir: Path) -> DIContainer: container.register_instance(AWSInstance, AWSInstance()) container.register_instance(MongoClient, MongoClient(MONGO_URL, serverSelectionTimeoutMS=100)) + container.register_instance( + ILockableEncryptor, RepositoryEncryptor(data_dir / REPOSITORY_KEY_FILE_NAME) + ) _register_repositories(container, data_dir) _register_services(container) From eac318ada009c1cfb16cb44a1a9f8cf9301721f2 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 12 Jul 2022 11:26:48 -0400 Subject: [PATCH 3/3] Island: Rename _user_datastore -> user_repository --- .../cc/services/authentication_service.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/monkey/monkey_island/cc/services/authentication_service.py b/monkey/monkey_island/cc/services/authentication_service.py index 200038708..7b32e4f1c 100644 --- a/monkey/monkey_island/cc/services/authentication_service.py +++ b/monkey/monkey_island/cc/services/authentication_service.py @@ -21,28 +21,28 @@ class AuthenticationService: def __init__( self, data_dir: Path, - user_datastore: IUserRepository, + user_repository: IUserRepository, repository_encryptor: ILockableEncryptor, ): self._data_dir = data_dir - self._user_datastore = user_datastore + self._user_repository = user_repository self._repository_encryptor = repository_encryptor def needs_registration(self) -> bool: - return not self._user_datastore.has_registered_users() + return not self._user_repository.has_registered_users() def register_new_user(self, username: str, password: str): if not username or not password: raise InvalidRegistrationCredentialsError("Username or password can not be empty.") credentials = UserCredentials(username, _hash_password(password)) - self._user_datastore.add_user(credentials) + self._user_repository.add_user(credentials) self._reset_repository_encryptor(username, password) reset_database() def authenticate(self, username: str, password: str): try: - registered_user = self._user_datastore.get_user_credentials(username) + registered_user = self._user_repository.get_user_credentials(username) except UnknownUserError: raise IncorrectCredentialsError()