From 4944947b100feec60a5021c9d069da5259c7fc9d Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 4 Oct 2021 13:29:13 -0400 Subject: [PATCH 01/18] Island: Rename password_based_bytes_encrypt{ion,or}.py --- monkey/monkey_island/cc/server_utils/encryption/__init__.py | 2 +- ...ed_bytes_encryption.py => password_based_bytes_encryptor.py} | 0 .../encryption/encryptors/password_based_string_encryptior.py | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename monkey/monkey_island/cc/server_utils/encryption/encryptors/{password_based_bytes_encryption.py => password_based_bytes_encryptor.py} (100%) diff --git a/monkey/monkey_island/cc/server_utils/encryption/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/__init__.py index 109423634..54a549181 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/__init__.py +++ b/monkey/monkey_island/cc/server_utils/encryption/__init__.py @@ -6,7 +6,7 @@ from monkey_island.cc.server_utils.encryption.encryptors.password_based_string_e PasswordBasedStringEncryptor, is_encrypted, ) -from monkey_island.cc.server_utils.encryption.encryptors.password_based_bytes_encryption import ( +from monkey_island.cc.server_utils.encryption.encryptors.password_based_bytes_encryptor import ( PasswordBasedBytesEncryptor, InvalidCredentialsError, InvalidCiphertextError, diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryption.py b/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryptor.py similarity index 100% rename from monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryption.py rename to monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryptor.py diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptior.py b/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptior.py index 9f99f735b..be8b6d6d6 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptior.py +++ b/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptior.py @@ -4,7 +4,7 @@ import logging import pyAesCrypt from monkey_island.cc.server_utils.encryption import IEncryptor -from monkey_island.cc.server_utils.encryption.encryptors.password_based_bytes_encryption import ( +from monkey_island.cc.server_utils.encryption.encryptors.password_based_bytes_encryptor import ( PasswordBasedBytesEncryptor, ) From f65251dddee82b3280a4e8b616ab2f2a2439d45a Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 4 Oct 2021 13:32:28 -0400 Subject: [PATCH 02/18] Island: Rename password_based_string_encrypt{i,}or.py --- monkey/monkey_island/cc/server_utils/encryption/__init__.py | 2 +- ..._string_encryptior.py => password_based_string_encryptor.py} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename monkey/monkey_island/cc/server_utils/encryption/encryptors/{password_based_string_encryptior.py => password_based_string_encryptor.py} (100%) diff --git a/monkey/monkey_island/cc/server_utils/encryption/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/__init__.py index 54a549181..907ffbde6 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/__init__.py +++ b/monkey/monkey_island/cc/server_utils/encryption/__init__.py @@ -2,7 +2,7 @@ from monkey_island.cc.server_utils.encryption.encryptors.i_encryptor import IEnc from monkey_island.cc.server_utils.encryption.encryptors.key_based_encryptor import ( KeyBasedEncryptor, ) -from monkey_island.cc.server_utils.encryption.encryptors.password_based_string_encryptior import ( +from monkey_island.cc.server_utils.encryption.encryptors.password_based_string_encryptor import ( PasswordBasedStringEncryptor, is_encrypted, ) diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptior.py b/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptor.py similarity index 100% rename from monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptior.py rename to monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptor.py From 5aa0506ce1954c1077ce7a15244469aaf0cc3771 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 4 Oct 2021 13:59:06 -0400 Subject: [PATCH 03/18] Island: Use relative imports inside encryption package --- .../cc/server_utils/encryption/data_store_encryptor.py | 7 ++----- .../encryption/dict_encryption/dict_encryptor.py | 4 +--- .../field_encryptors/mimikatz_results_encryptor.py | 6 ++---- .../field_encryptors/string_list_encryptor.py | 6 ++---- .../cc/server_utils/encryption/encryptors/__init__.py | 4 ++++ .../encryption/encryptors/key_based_encryptor.py | 2 +- .../encryptors/password_based_bytes_encryptor.py | 2 +- .../encryptors/password_based_string_encryptor.py | 6 ++---- 8 files changed, 15 insertions(+), 22 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py index f7add80f3..3e8028944 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py @@ -3,13 +3,10 @@ from typing import Union from Crypto import Random # noqa: DUO133 # nosec: B413 -from monkey_island.cc.server_utils.encryption import ( - IEncryptor, - KeyBasedEncryptor, - PasswordBasedBytesEncryptor, -) from monkey_island.cc.server_utils.file_utils import open_new_securely_permissioned_file +from .encryptors import IEncryptor, KeyBasedEncryptor, PasswordBasedBytesEncryptor + _KEY_FILENAME = "mongo_key.bin" _BLOCK_SIZE = 32 diff --git a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/dict_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/dict_encryptor.py index a95a761e0..4dc4662d6 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/dict_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/dict_encryptor.py @@ -3,9 +3,7 @@ from typing import Callable, List, Type import dpath.util -from monkey_island.cc.server_utils.encryption.dict_encryption.field_encryptors import ( - IFieldEncryptor, -) +from .field_encryptors import IFieldEncryptor class FieldNotFoundError(Exception): diff --git a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/mimikatz_results_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/mimikatz_results_encryptor.py index ff2ee314e..163bee8fd 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/mimikatz_results_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/mimikatz_results_encryptor.py @@ -1,9 +1,7 @@ import logging -from monkey_island.cc.server_utils.encryption import get_datastore_encryptor -from monkey_island.cc.server_utils.encryption.dict_encryption.field_encryptors import ( - IFieldEncryptor, -) +from ... import get_datastore_encryptor +from . import IFieldEncryptor logger = logging.getLogger(__name__) diff --git a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/string_list_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/string_list_encryptor.py index 04374c462..6a0dd58d2 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/string_list_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/string_list_encryptor.py @@ -1,9 +1,7 @@ from typing import List -from monkey_island.cc.server_utils.encryption import get_datastore_encryptor -from monkey_island.cc.server_utils.encryption.dict_encryption.field_encryptors import ( - IFieldEncryptor, -) +from ... import get_datastore_encryptor +from . import IFieldEncryptor class StringListEncryptor(IFieldEncryptor): diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptors/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/encryptors/__init__.py index e69de29bb..11386f798 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/encryptors/__init__.py +++ b/monkey/monkey_island/cc/server_utils/encryption/encryptors/__init__.py @@ -0,0 +1,4 @@ +from .i_encryptor import IEncryptor +from .key_based_encryptor import KeyBasedEncryptor +from .password_based_string_encryptor import PasswordBasedStringEncryptor +from .password_based_bytes_encryptor import PasswordBasedBytesEncryptor diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptors/key_based_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/encryptors/key_based_encryptor.py index 551014be1..41c8b0db2 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/encryptors/key_based_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/encryptors/key_based_encryptor.py @@ -7,7 +7,7 @@ from Crypto import Random # noqa: DUO133 # nosec: B413 from Crypto.Cipher import AES # noqa: DUO133 # nosec: B413 from Crypto.Util import Padding # noqa: DUO133 -from monkey_island.cc.server_utils.encryption import IEncryptor +from .i_encryptor import IEncryptor logger = logging.getLogger(__name__) diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryptor.py index 2e7c05819..ca46f5646 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryptor.py @@ -3,7 +3,7 @@ import logging import pyAesCrypt -from monkey_island.cc.server_utils.encryption import IEncryptor +from .i_encryptor import IEncryptor logger = logging.getLogger(__name__) diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptor.py index be8b6d6d6..dac7276e9 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptor.py @@ -3,10 +3,8 @@ import logging import pyAesCrypt -from monkey_island.cc.server_utils.encryption import IEncryptor -from monkey_island.cc.server_utils.encryption.encryptors.password_based_bytes_encryptor import ( - PasswordBasedBytesEncryptor, -) +from .i_encryptor import IEncryptor +from .password_based_bytes_encryptor import PasswordBasedBytesEncryptor logger = logging.getLogger(__name__) From a24979155fdc04f39c79ecf98f8db170c56d5776 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 4 Oct 2021 17:21:08 -0400 Subject: [PATCH 04/18] Island: Improve logging in PasswordBasedBytesEncryptor --- .../encryption/encryptors/password_based_bytes_encryptor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryptor.py index ca46f5646..b50e77e87 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryptor.py @@ -47,10 +47,10 @@ class PasswordBasedBytesEncryptor(IEncryptor): ) except ValueError as ex: if str(ex).startswith("Wrong password"): - logger.info("Wrong password provided for decryption.") + logger.error("Wrong password provided for decryption.") raise InvalidCredentialsError else: - logger.info("The corrupt ciphertext provided.") + logger.error("The provided ciphertext was corrupt.") raise InvalidCiphertextError return plaintext_stream.getvalue() From 8f9289517f57233842385e691d7c19e3d2635c04 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 4 Oct 2021 17:22:52 -0400 Subject: [PATCH 05/18] Tests: Decouple uses_encryptor() fixture from AuthenticationService --- monkey/tests/unit_tests/monkey_island/cc/conftest.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/conftest.py b/monkey/tests/unit_tests/monkey_island/cc/conftest.py index 7d9642201..22390dea7 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/conftest.py +++ b/monkey/tests/unit_tests/monkey_island/cc/conftest.py @@ -11,7 +11,6 @@ from tests.unit_tests.monkey_island.cc.server_utils.encryption.test_password_bas ) from monkey_island.cc.server_utils.encryption import initialize_datastore_encryptor -from monkey_island.cc.services.authentication import AuthenticationService @pytest.fixture @@ -28,11 +27,7 @@ def monkey_config_json(monkey_config): return json.dumps(monkey_config) -MOCK_USERNAME = "m0nk3y_u53r" -MOCK_PASSWORD = "3cr3t_p455w0rd" - - @pytest.fixture def uses_encryptor(data_for_tests_dir): - secret = AuthenticationService._get_secret_from_credentials(MOCK_USERNAME, MOCK_PASSWORD) + secret = "m0nk3y_u53r:3cr3t_p455w0rd" initialize_datastore_encryptor(data_for_tests_dir, secret) From 849ced23342d8654ba92528484f6bf789466ab1c Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 5 Oct 2021 12:10:46 -0400 Subject: [PATCH 06/18] Tests: Improve telemetry_dal tests * Reduce unnecessary mocking * Remove defunct "mimikatz" field from mock telemetry * Test encryption/decryption of all secret types for all users --- .../models/telemetries/test_telemetry_dal.py | 55 +++++++------------ 1 file changed, 20 insertions(+), 35 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/models/telemetries/test_telemetry_dal.py b/monkey/tests/unit_tests/monkey_island/cc/models/telemetries/test_telemetry_dal.py index d6a35760a..f2b81eb24 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/models/telemetries/test_telemetry_dal.py +++ b/monkey/tests/unit_tests/monkey_island/cc/models/telemetries/test_telemetry_dal.py @@ -6,13 +6,9 @@ import pytest from monkey_island.cc.models.telemetries import get_telemetry_by_query, save_telemetry from monkey_island.cc.models.telemetries.telemetry import Telemetry -from monkey_island.cc.server_utils.encryption import SensitiveField -from monkey_island.cc.server_utils.encryption.dict_encryption.field_encryptors import ( - MimikatzResultsEncryptor, -) MOCK_CREDENTIALS = { - "Vakaris": { + "M0nk3y": { "username": "M0nk3y", "password": "", "ntlm_hash": "e87f2f73e353f1d95e42ce618601b61f", @@ -24,7 +20,6 @@ MOCK_CREDENTIALS = { MOCK_DATA_DICT = { "network_info": {}, "credentials": deepcopy(MOCK_CREDENTIALS), - "mimikatz": deepcopy(MOCK_CREDENTIALS), } MOCK_TELEMETRY = { @@ -49,19 +44,6 @@ MOCK_NO_ENCRYPTION_NEEDED_TELEMETRY = { "data": {"done": False}, } -MOCK_SENSITIVE_FIELDS = [ - SensitiveField("data.credentials", MimikatzResultsEncryptor), - SensitiveField("data.mimikatz", MimikatzResultsEncryptor), -] - - -@pytest.fixture(autouse=True) -def patch_sensitive_fields(monkeypatch): - monkeypatch.setattr( - "monkey_island.cc.models.telemetries.telemetry_dal.sensitive_fields", - MOCK_SENSITIVE_FIELDS, - ) - @pytest.fixture(autouse=True) def fake_mongo(monkeypatch): @@ -71,24 +53,27 @@ def fake_mongo(monkeypatch): @pytest.mark.usefixtures("uses_database", "uses_encryptor") def test_telemetry_encryption(): + secret_keys = ["password", "lm_hash", "ntlm_hash"] save_telemetry(MOCK_TELEMETRY) - assert ( - not Telemetry.objects.first()["data"]["credentials"]["user"]["password"] - == MOCK_CREDENTIALS["user"]["password"] - ) - assert ( - not Telemetry.objects.first()["data"]["mimikatz"]["Vakaris"]["ntlm_hash"] - == MOCK_CREDENTIALS["Vakaris"]["ntlm_hash"] - ) - assert ( - get_telemetry_by_query({})[0]["data"]["credentials"]["user"]["password"] - == MOCK_CREDENTIALS["user"]["password"] - ) - assert ( - get_telemetry_by_query({})[0]["data"]["mimikatz"]["Vakaris"]["ntlm_hash"] - == MOCK_CREDENTIALS["Vakaris"]["ntlm_hash"] - ) + + encrypted_telemetry = Telemetry.objects.first() + for user in MOCK_CREDENTIALS.keys(): + assert encrypted_telemetry["data"]["credentials"][user]["username"] == user + + for s in secret_keys: + assert ( + encrypted_telemetry["data"]["credentials"][user][s] != MOCK_CREDENTIALS[user][s] + ) + + decrypted_telemetry = get_telemetry_by_query({})[0] + for user in MOCK_CREDENTIALS.keys(): + assert decrypted_telemetry["data"]["credentials"][user]["username"] == user + + for s in secret_keys: + assert ( + decrypted_telemetry["data"]["credentials"][user][s] == MOCK_CREDENTIALS[user][s] + ) @pytest.mark.usefixtures("uses_database", "uses_encryptor") From e7fcf933b7912f039c3bd271516d5d3d7889f6ae Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 5 Oct 2021 12:12:38 -0400 Subject: [PATCH 07/18] Island: Remove try/except from MimikatzResultsEncryptor.encrypt() Catching this exception was a workaround for an issue that was resolved in PR #1508. --- .../mimikatz_cred_collector.py | 2 ++ .../field_encryptors/mimikatz_results_encryptor.py | 13 +++---------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/monkey/infection_monkey/system_info/windows_cred_collector/mimikatz_cred_collector.py b/monkey/infection_monkey/system_info/windows_cred_collector/mimikatz_cred_collector.py index 6a4ef7799..ab44d85ea 100644 --- a/monkey/infection_monkey/system_info/windows_cred_collector/mimikatz_cred_collector.py +++ b/monkey/infection_monkey/system_info/windows_cred_collector/mimikatz_cred_collector.py @@ -16,6 +16,8 @@ class MimikatzCredentialCollector(object): def cred_list_to_cred_dict(creds: List[WindowsCredentials]): cred_dict = {} for cred in creds: + # TODO: This should be handled by the island, not the agent. There is already similar + # code in monkey_island/cc/models/report/report_dal.py. # Lets not use "." and "$" in keys, because it will confuse mongo. # Ideally we should refactor island not to use a dict and simply parse credential list. key = cred.username.replace(".", ",").replace("$", "") diff --git a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/mimikatz_results_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/mimikatz_results_encryptor.py index 163bee8fd..696a9187b 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/mimikatz_results_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/mimikatz_results_encryptor.py @@ -14,16 +14,9 @@ class MimikatzResultsEncryptor(IFieldEncryptor): def encrypt(results: dict) -> dict: for _, credentials in results.items(): for secret_type in MimikatzResultsEncryptor.secret_types: - try: - credentials[secret_type] = get_datastore_encryptor().encrypt( - credentials[secret_type] - ) - except ValueError as e: - logger.error( - f"Failed encrypting sensitive field for " - f"user {credentials['username']}! Error: {e}" - ) - credentials[secret_type] = get_datastore_encryptor().encrypt("") + credentials[secret_type] = get_datastore_encryptor().encrypt( + credentials[secret_type] + ) return results @staticmethod From bf082d36ef81528ae9df2736fed529aae1a593db Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 5 Oct 2021 12:14:10 -0400 Subject: [PATCH 08/18] Tests: Mark encryption tests as slow --- .../cc/models/telemetries/test_telemetry_dal.py | 10 ++++------ .../field_encryptors/test_string_list_encryptor.py | 4 ++++ .../encryption/test_data_store_encryptor.py | 4 ++++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/models/telemetries/test_telemetry_dal.py b/monkey/tests/unit_tests/monkey_island/cc/models/telemetries/test_telemetry_dal.py index f2b81eb24..77003ec8f 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/models/telemetries/test_telemetry_dal.py +++ b/monkey/tests/unit_tests/monkey_island/cc/models/telemetries/test_telemetry_dal.py @@ -51,6 +51,7 @@ def fake_mongo(monkeypatch): monkeypatch.setattr("monkey_island.cc.models.telemetries.telemetry_dal.mongo", mongo) +@pytest.mark.slow @pytest.mark.usefixtures("uses_database", "uses_encryptor") def test_telemetry_encryption(): secret_keys = ["password", "lm_hash", "ntlm_hash"] @@ -62,20 +63,17 @@ def test_telemetry_encryption(): assert encrypted_telemetry["data"]["credentials"][user]["username"] == user for s in secret_keys: - assert ( - encrypted_telemetry["data"]["credentials"][user][s] != MOCK_CREDENTIALS[user][s] - ) + assert encrypted_telemetry["data"]["credentials"][user][s] != MOCK_CREDENTIALS[user][s] decrypted_telemetry = get_telemetry_by_query({})[0] for user in MOCK_CREDENTIALS.keys(): assert decrypted_telemetry["data"]["credentials"][user]["username"] == user for s in secret_keys: - assert ( - decrypted_telemetry["data"]["credentials"][user][s] == MOCK_CREDENTIALS[user][s] - ) + assert decrypted_telemetry["data"]["credentials"][user][s] == MOCK_CREDENTIALS[user][s] +@pytest.mark.slow @pytest.mark.usefixtures("uses_database", "uses_encryptor") def test_no_encryption_needed(): # Make sure telemetry save doesn't break when telemetry doesn't need encryption diff --git a/monkey/tests/unit_tests/monkey_island/cc/models/utils/field_encryptors/test_string_list_encryptor.py b/monkey/tests/unit_tests/monkey_island/cc/models/utils/field_encryptors/test_string_list_encryptor.py index 7ea21849b..e39dffd94 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/models/utils/field_encryptors/test_string_list_encryptor.py +++ b/monkey/tests/unit_tests/monkey_island/cc/models/utils/field_encryptors/test_string_list_encryptor.py @@ -1,3 +1,5 @@ +import pytest + from monkey_island.cc.server_utils.encryption.dict_encryption.field_encryptors import ( StringListEncryptor, ) @@ -6,6 +8,7 @@ MOCK_STRING_LIST = ["test_1", "test_2"] EMPTY_LIST = [] +@pytest.mark.slow def test_encryption_and_decryption(uses_encryptor): encrypted_list = StringListEncryptor.encrypt(MOCK_STRING_LIST) assert not encrypted_list == MOCK_STRING_LIST @@ -13,6 +16,7 @@ def test_encryption_and_decryption(uses_encryptor): assert decrypted_list == MOCK_STRING_LIST +@pytest.mark.slow def test_empty_list(uses_encryptor): # Tests that no errors are raised encrypted_list = StringListEncryptor.encrypt(EMPTY_LIST) diff --git a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py index 7c379af1c..f24040c22 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py @@ -11,6 +11,7 @@ PLAINTEXT = "Hello, Monkey!" MOCK_SECRET = "53CR31" +@pytest.mark.slow @pytest.mark.usefixtures("uses_encryptor") def test_encryption(data_for_tests_dir): encrypted_data = get_datastore_encryptor().encrypt(PLAINTEXT) @@ -33,10 +34,12 @@ def initialized_encryptor_dir(tmpdir): return tmpdir +@pytest.mark.slow def test_key_creation(initialized_encryptor_dir): assert (initialized_encryptor_dir / data_store_encryptor._KEY_FILENAME).isfile() +@pytest.mark.slow def test_key_removal(initialized_encryptor_dir): remove_old_datastore_key(initialized_encryptor_dir) assert not (initialized_encryptor_dir / data_store_encryptor._KEY_FILENAME).isfile() @@ -49,6 +52,7 @@ def test_key_removal__no_key(tmpdir): data_store_encryptor._factory = None +@pytest.mark.slow @pytest.mark.usefixtures("cleanup_encryptor") def test_key_file_encryption(tmpdir, monkeypatch): monkeypatch.setattr(data_store_encryptor, "_get_random_bytes", lambda: PLAINTEXT.encode()) From 0eafc6613a14908bab3c0952d3f6ad30d571dabb Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 5 Oct 2021 12:37:05 -0400 Subject: [PATCH 09/18] Island: Flatten directory structure for "encryption" package --- .../cc/server_utils/encryption/__init__.py | 15 ++++++++------- .../encryption/data_store_encryptor.py | 4 +++- .../encryption/dict_encryption/__init__.py | 0 .../{dict_encryption => }/dict_encryptor.py | 0 .../encryption/encryptors/__init__.py | 4 ---- .../field_encryptors/__init__.py | 0 .../field_encryptors/i_field_encryptor.py | 0 .../mimikatz_results_encryptor.py | 2 +- .../field_encryptors/string_list_encryptor.py | 2 +- .../encryption/{encryptors => }/i_encryptor.py | 0 .../{encryptors => }/key_based_encryptor.py | 0 .../password_based_bytes_encryptor.py | 0 .../password_based_string_encryptor.py | 0 .../monkey_island/cc/models/test_report_dal.py | 5 +---- .../encryption}/test_string_list_encryptor.py | 4 +--- 15 files changed, 15 insertions(+), 21 deletions(-) delete mode 100644 monkey/monkey_island/cc/server_utils/encryption/dict_encryption/__init__.py rename monkey/monkey_island/cc/server_utils/encryption/{dict_encryption => }/dict_encryptor.py (100%) delete mode 100644 monkey/monkey_island/cc/server_utils/encryption/encryptors/__init__.py rename monkey/monkey_island/cc/server_utils/encryption/{dict_encryption => }/field_encryptors/__init__.py (100%) rename monkey/monkey_island/cc/server_utils/encryption/{dict_encryption => }/field_encryptors/i_field_encryptor.py (100%) rename monkey/monkey_island/cc/server_utils/encryption/{dict_encryption => }/field_encryptors/mimikatz_results_encryptor.py (93%) rename monkey/monkey_island/cc/server_utils/encryption/{dict_encryption => }/field_encryptors/string_list_encryptor.py (86%) rename monkey/monkey_island/cc/server_utils/encryption/{encryptors => }/i_encryptor.py (100%) rename monkey/monkey_island/cc/server_utils/encryption/{encryptors => }/key_based_encryptor.py (100%) rename monkey/monkey_island/cc/server_utils/encryption/{encryptors => }/password_based_bytes_encryptor.py (100%) rename monkey/monkey_island/cc/server_utils/encryption/{encryptors => }/password_based_string_encryptor.py (100%) rename monkey/tests/unit_tests/monkey_island/cc/{models/utils/field_encryptors => server_utils/encryption}/test_string_list_encryptor.py (83%) diff --git a/monkey/monkey_island/cc/server_utils/encryption/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/__init__.py index 907ffbde6..1b302a6fc 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/__init__.py +++ b/monkey/monkey_island/cc/server_utils/encryption/__init__.py @@ -1,12 +1,12 @@ -from monkey_island.cc.server_utils.encryption.encryptors.i_encryptor import IEncryptor -from monkey_island.cc.server_utils.encryption.encryptors.key_based_encryptor import ( +from .i_encryptor import IEncryptor +from .key_based_encryptor import ( KeyBasedEncryptor, ) -from monkey_island.cc.server_utils.encryption.encryptors.password_based_string_encryptor import ( +from .password_based_string_encryptor import ( PasswordBasedStringEncryptor, is_encrypted, ) -from monkey_island.cc.server_utils.encryption.encryptors.password_based_bytes_encryptor import ( +from .password_based_bytes_encryptor import ( PasswordBasedBytesEncryptor, InvalidCredentialsError, InvalidCiphertextError, @@ -16,11 +16,12 @@ from .data_store_encryptor import ( get_datastore_encryptor, remove_old_datastore_key, ) -from .dict_encryption.dict_encryptor import ( +from .dict_encryptor import ( SensitiveField, encrypt_dict, decrypt_dict, FieldNotFoundError, ) -from .dict_encryption.field_encryptors.mimikatz_results_encryptor import MimikatzResultsEncryptor -from .dict_encryption.field_encryptors.string_list_encryptor import StringListEncryptor +from .field_encryptors.i_field_encryptor import IFieldEncryptor +from .field_encryptors.mimikatz_results_encryptor import MimikatzResultsEncryptor +from .field_encryptors.string_list_encryptor import StringListEncryptor diff --git a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py index 3e8028944..424a511f7 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py @@ -5,7 +5,9 @@ from Crypto import Random # noqa: DUO133 # nosec: B413 from monkey_island.cc.server_utils.file_utils import open_new_securely_permissioned_file -from .encryptors import IEncryptor, KeyBasedEncryptor, PasswordBasedBytesEncryptor +from .i_encryptor import IEncryptor +from .key_based_encryptor import KeyBasedEncryptor +from .password_based_bytes_encryptor import PasswordBasedBytesEncryptor _KEY_FILENAME = "mongo_key.bin" _BLOCK_SIZE = 32 diff --git a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/dict_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/dict_encryptor.py similarity index 100% rename from monkey/monkey_island/cc/server_utils/encryption/dict_encryption/dict_encryptor.py rename to monkey/monkey_island/cc/server_utils/encryption/dict_encryptor.py diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptors/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/encryptors/__init__.py deleted file mode 100644 index 11386f798..000000000 --- a/monkey/monkey_island/cc/server_utils/encryption/encryptors/__init__.py +++ /dev/null @@ -1,4 +0,0 @@ -from .i_encryptor import IEncryptor -from .key_based_encryptor import KeyBasedEncryptor -from .password_based_string_encryptor import PasswordBasedStringEncryptor -from .password_based_bytes_encryptor import PasswordBasedBytesEncryptor diff --git a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/field_encryptors/__init__.py similarity index 100% rename from monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/__init__.py rename to monkey/monkey_island/cc/server_utils/encryption/field_encryptors/__init__.py diff --git a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/i_field_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/field_encryptors/i_field_encryptor.py similarity index 100% rename from monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/i_field_encryptor.py rename to monkey/monkey_island/cc/server_utils/encryption/field_encryptors/i_field_encryptor.py diff --git a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/mimikatz_results_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/field_encryptors/mimikatz_results_encryptor.py similarity index 93% rename from monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/mimikatz_results_encryptor.py rename to monkey/monkey_island/cc/server_utils/encryption/field_encryptors/mimikatz_results_encryptor.py index 696a9187b..31f597e60 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/mimikatz_results_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/field_encryptors/mimikatz_results_encryptor.py @@ -1,6 +1,6 @@ import logging -from ... import get_datastore_encryptor +from ..data_store_encryptor import get_datastore_encryptor from . import IFieldEncryptor logger = logging.getLogger(__name__) diff --git a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/string_list_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/field_encryptors/string_list_encryptor.py similarity index 86% rename from monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/string_list_encryptor.py rename to monkey/monkey_island/cc/server_utils/encryption/field_encryptors/string_list_encryptor.py index 6a0dd58d2..ce0ceb8dd 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/dict_encryption/field_encryptors/string_list_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/field_encryptors/string_list_encryptor.py @@ -1,6 +1,6 @@ from typing import List -from ... import get_datastore_encryptor +from ..data_store_encryptor import get_datastore_encryptor from . import IFieldEncryptor diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptors/i_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/i_encryptor.py similarity index 100% rename from monkey/monkey_island/cc/server_utils/encryption/encryptors/i_encryptor.py rename to monkey/monkey_island/cc/server_utils/encryption/i_encryptor.py diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptors/key_based_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/key_based_encryptor.py similarity index 100% rename from monkey/monkey_island/cc/server_utils/encryption/encryptors/key_based_encryptor.py rename to monkey/monkey_island/cc/server_utils/encryption/key_based_encryptor.py diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/password_based_bytes_encryptor.py similarity index 100% rename from monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryptor.py rename to monkey/monkey_island/cc/server_utils/encryption/password_based_bytes_encryptor.py diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/password_based_string_encryptor.py similarity index 100% rename from monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptor.py rename to monkey/monkey_island/cc/server_utils/encryption/password_based_string_encryptor.py diff --git a/monkey/tests/unit_tests/monkey_island/cc/models/test_report_dal.py b/monkey/tests/unit_tests/monkey_island/cc/models/test_report_dal.py index 5d3d5a49a..67ac8355e 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/models/test_report_dal.py +++ b/monkey/tests/unit_tests/monkey_island/cc/models/test_report_dal.py @@ -5,10 +5,7 @@ import pytest from monkey_island.cc.models import Report from monkey_island.cc.models.report import get_report, save_report -from monkey_island.cc.server_utils.encryption import SensitiveField -from monkey_island.cc.server_utils.encryption.dict_encryption.field_encryptors import ( - IFieldEncryptor, -) +from monkey_island.cc.server_utils.encryption import IFieldEncryptor, SensitiveField MOCK_SENSITIVE_FIELD_CONTENTS = ["the_string", "the_string2"] MOCK_REPORT_DICT = { diff --git a/monkey/tests/unit_tests/monkey_island/cc/models/utils/field_encryptors/test_string_list_encryptor.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_string_list_encryptor.py similarity index 83% rename from monkey/tests/unit_tests/monkey_island/cc/models/utils/field_encryptors/test_string_list_encryptor.py rename to monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_string_list_encryptor.py index e39dffd94..b78cd6ec0 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/models/utils/field_encryptors/test_string_list_encryptor.py +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_string_list_encryptor.py @@ -1,8 +1,6 @@ import pytest -from monkey_island.cc.server_utils.encryption.dict_encryption.field_encryptors import ( - StringListEncryptor, -) +from monkey_island.cc.server_utils.encryption import StringListEncryptor MOCK_STRING_LIST = ["test_1", "test_2"] EMPTY_LIST = [] From c0b257127a738b5621fc3f2b3b7142488e12bc16 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 5 Oct 2021 15:59:39 -0400 Subject: [PATCH 10/18] Island: Implement DataStoreEncryptor as a class This allows us to begin decoupling some implementation details from the AuthenticationService. --- .../encryption/data_store_encryptor.py | 82 +++++++++++-------- .../cc/services/authentication.py | 2 +- .../encryption/test_data_store_encryptor.py | 74 +++++++++-------- 3 files changed, 88 insertions(+), 70 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py index 424a511f7..6a44b5cff 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py @@ -1,4 +1,5 @@ import os +from pathlib import Path from typing import Union from Crypto import Random # noqa: DUO133 # nosec: B413 @@ -9,49 +10,62 @@ from .i_encryptor import IEncryptor from .key_based_encryptor import KeyBasedEncryptor from .password_based_bytes_encryptor import PasswordBasedBytesEncryptor -_KEY_FILENAME = "mongo_key.bin" -_BLOCK_SIZE = 32 - _encryptor: Union[None, IEncryptor] = None -def _load_existing_key(key_file_path: str, secret: str) -> KeyBasedEncryptor: - with open(key_file_path, "rb") as f: - encrypted_key = f.read() - cipher_key = PasswordBasedBytesEncryptor(secret).decrypt(encrypted_key) - return KeyBasedEncryptor(cipher_key) +class DataStoreEncryptor(IEncryptor): + _KEY_LENGTH_BYTES = 32 + + def __init__(self, secret: str, key_file_path: Path): + self._key_file_path = key_file_path + self._password_based_encryptor = PasswordBasedBytesEncryptor(secret) + self._key_based_encryptor = self._initialize_key_based_encryptor() + + def _initialize_key_based_encryptor(self): + if os.path.exists(self._key_file_path): + return self._load_existing_key() + + return self._create_new_key() + + def _load_existing_key(self) -> KeyBasedEncryptor: + with open(self._key_file_path, "rb") as f: + encrypted_key = f.read() + + plaintext_key = self._password_based_encryptor.decrypt(encrypted_key) + return KeyBasedEncryptor(plaintext_key) + + def _create_new_key(self) -> KeyBasedEncryptor: + plaintext_key = Random.new().read(DataStoreEncryptor._KEY_LENGTH_BYTES) + + encrypted_key = self._password_based_encryptor.encrypt(plaintext_key) + with open_new_securely_permissioned_file(self._key_file_path, "wb") as f: + f.write(encrypted_key) + + return KeyBasedEncryptor(plaintext_key) + + def encrypt(self, plaintext: str) -> str: + return self._key_based_encryptor.encrypt(plaintext) + + def decrypt(self, ciphertext: str): + return self._key_based_encryptor.decrypt(ciphertext) + + def erase_key(self): + if self._key_file_path.is_file(): + self._key_file_path.unlink() -def _create_new_key(key_file_path: str, secret: str) -> KeyBasedEncryptor: - cipher_key = _get_random_bytes() - encrypted_key = PasswordBasedBytesEncryptor(secret).encrypt(cipher_key) - with open_new_securely_permissioned_file(key_file_path, "wb") as f: - f.write(encrypted_key) - return KeyBasedEncryptor(cipher_key) +def remove_old_datastore_key(): + if _encryptor: + _encryptor.erase_key() -def _get_random_bytes() -> bytes: - return Random.new().read(_BLOCK_SIZE) - - -def remove_old_datastore_key(key_file_dir: str): - key_file_path = _get_key_file_path(key_file_dir) - if os.path.isfile(key_file_path): - os.remove(key_file_path) - - -def initialize_datastore_encryptor(key_file_dir: str, secret: str): +def initialize_datastore_encryptor( + key_file_dir: str, secret: str, key_file_name: str = "mongo_key.bin" +): global _encryptor - key_file_path = _get_key_file_path(key_file_dir) - if os.path.exists(key_file_path): - _encryptor = _load_existing_key(key_file_path, secret) - else: - _encryptor = _create_new_key(key_file_path, secret) - - -def _get_key_file_path(key_file_dir: str) -> str: - return os.path.join(key_file_dir, _KEY_FILENAME) + key_file_path = Path(key_file_dir) / key_file_name + _encryptor = DataStoreEncryptor(secret, key_file_path) def get_datastore_encryptor() -> IEncryptor: diff --git a/monkey/monkey_island/cc/services/authentication.py b/monkey/monkey_island/cc/services/authentication.py index 9d3d3baa7..2d7940055 100644 --- a/monkey/monkey_island/cc/services/authentication.py +++ b/monkey/monkey_island/cc/services/authentication.py @@ -22,7 +22,7 @@ class AuthenticationService: @staticmethod def reset_datastore_encryptor(username: str, password: str): - remove_old_datastore_key(AuthenticationService.KEY_FILE_DIRECTORY) + remove_old_datastore_key() AuthenticationService._init_encryptor_from_credentials(username, password) @staticmethod diff --git a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py index f24040c22..7d5616185 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py @@ -10,10 +10,24 @@ from monkey_island.cc.server_utils.encryption import ( PLAINTEXT = "Hello, Monkey!" MOCK_SECRET = "53CR31" +KEY_FILENAME = "test_key.bin" + + +@pytest.fixture(autouse=True) +def cleanup_encryptor(): + yield + data_store_encryptor._encryptor = None + + +@pytest.fixture +def key_file(tmp_path): + return tmp_path / KEY_FILENAME + @pytest.mark.slow -@pytest.mark.usefixtures("uses_encryptor") -def test_encryption(data_for_tests_dir): +def test_encryption(tmp_path): + initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) + encrypted_data = get_datastore_encryptor().encrypt(PLAINTEXT) assert encrypted_data != PLAINTEXT @@ -21,45 +35,35 @@ def test_encryption(data_for_tests_dir): assert decrypted_data == PLAINTEXT -@pytest.fixture -def cleanup_encryptor(): - yield - data_store_encryptor._encryptor = None - - -@pytest.mark.usefixtures("cleanup_encryptor") -@pytest.fixture -def initialized_encryptor_dir(tmpdir): - initialize_datastore_encryptor(tmpdir, MOCK_SECRET) - return tmpdir +@pytest.mark.slow +def test_key_creation(key_file, tmp_path): + assert not key_file.is_file() + initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) + assert key_file.is_file() @pytest.mark.slow -def test_key_creation(initialized_encryptor_dir): - assert (initialized_encryptor_dir / data_store_encryptor._KEY_FILENAME).isfile() +def test_key_removal(key_file, tmp_path): + initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) + assert key_file.is_file() + + remove_old_datastore_key() + assert not key_file.is_file() -@pytest.mark.slow -def test_key_removal(initialized_encryptor_dir): - remove_old_datastore_key(initialized_encryptor_dir) - assert not (initialized_encryptor_dir / data_store_encryptor._KEY_FILENAME).isfile() - - -def test_key_removal__no_key(tmpdir): - assert not (tmpdir / data_store_encryptor._KEY_FILENAME).isfile() +def test_key_removal__no_key(key_file): + assert not key_file.is_file() # Make sure no error thrown when we try to remove an non-existing key - remove_old_datastore_key(tmpdir) - data_store_encryptor._factory = None + remove_old_datastore_key() -@pytest.mark.slow -@pytest.mark.usefixtures("cleanup_encryptor") -def test_key_file_encryption(tmpdir, monkeypatch): - monkeypatch.setattr(data_store_encryptor, "_get_random_bytes", lambda: PLAINTEXT.encode()) - initialize_datastore_encryptor(tmpdir, MOCK_SECRET) - key_file_path = data_store_encryptor._get_key_file_path(tmpdir) - key_file_contents = open(key_file_path, "rb").read() - assert not key_file_contents == PLAINTEXT.encode() +def test_key_removal__no_key_2(key_file, tmp_path): + assert not key_file.is_file() + initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) + assert key_file.is_file() - key_based_encryptor = data_store_encryptor._load_existing_key(key_file_path, MOCK_SECRET) - assert key_based_encryptor._key == PLAINTEXT.encode() + key_file.unlink() + assert not key_file.is_file() + + # Make sure no error thrown when we try to remove an non-existing key + get_datastore_encryptor().erase_key() From 95221ef53aa896f02e0388e430191a9dd1c3b55d Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 5 Oct 2021 16:42:49 -0400 Subject: [PATCH 11/18] Island: Add reinitialize_datastore_encryptor() --- .../cc/server_utils/encryption/__init__.py | 4 +- .../encryption/data_store_encryptor.py | 15 ++++++- .../cc/services/authentication.py | 6 +-- .../encryption/test_data_store_encryptor.py | 40 ++++++++++++++----- 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/encryption/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/__init__.py index 1b302a6fc..7fbc882cb 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/__init__.py +++ b/monkey/monkey_island/cc/server_utils/encryption/__init__.py @@ -12,9 +12,9 @@ from .password_based_bytes_encryptor import ( InvalidCiphertextError, ) from .data_store_encryptor import ( - initialize_datastore_encryptor, get_datastore_encryptor, - remove_old_datastore_key, + initialize_datastore_encryptor, + reinitialize_datastore_encryptor, ) from .dict_encryptor import ( SensitiveField, diff --git a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py index 6a44b5cff..7e9a279d4 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py @@ -53,11 +53,24 @@ class DataStoreEncryptor(IEncryptor): if self._key_file_path.is_file(): self._key_file_path.unlink() + self._key_based_encryptor = None + + +def reinitialize_datastore_encryptor( + key_file_dir: str, secret: str, key_file_name: str = "mongo_key.bin" +): + _delete_encryptor() + initialize_datastore_encryptor(key_file_dir, secret, key_file_name) + + +def _delete_encryptor(): + global _encryptor -def remove_old_datastore_key(): if _encryptor: _encryptor.erase_key() + _encryptor = None + def initialize_datastore_encryptor( key_file_dir: str, secret: str, key_file_name: str = "mongo_key.bin" diff --git a/monkey/monkey_island/cc/services/authentication.py b/monkey/monkey_island/cc/services/authentication.py index 2d7940055..9cb0121c5 100644 --- a/monkey/monkey_island/cc/services/authentication.py +++ b/monkey/monkey_island/cc/services/authentication.py @@ -1,7 +1,7 @@ from monkey_island.cc.server_utils.encryption import ( get_datastore_encryptor, initialize_datastore_encryptor, - remove_old_datastore_key, + reinitialize_datastore_encryptor, ) @@ -22,8 +22,8 @@ class AuthenticationService: @staticmethod def reset_datastore_encryptor(username: str, password: str): - remove_old_datastore_key() - AuthenticationService._init_encryptor_from_credentials(username, password) + secret = AuthenticationService._get_secret_from_credentials(username, password) + reinitialize_datastore_encryptor(AuthenticationService.KEY_FILE_DIRECTORY, secret) @staticmethod def _init_encryptor_from_credentials(username: str, password: str): diff --git a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py index 7d5616185..4ae6b15e1 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py @@ -1,10 +1,11 @@ import pytest +from common.utils.file_utils import get_file_sha256_hash from monkey_island.cc.server_utils.encryption import ( data_store_encryptor, get_datastore_encryptor, initialize_datastore_encryptor, - remove_old_datastore_key, + reinitialize_datastore_encryptor, ) PLAINTEXT = "Hello, Monkey!" @@ -42,28 +43,47 @@ def test_key_creation(key_file, tmp_path): assert key_file.is_file() +@pytest.mark.slow +def test_existing_key_reused(key_file, tmp_path): + assert not key_file.is_file() + + initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) + key_file_hash_1 = get_file_sha256_hash(key_file) + + initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) + key_file_hash_2 = get_file_sha256_hash(key_file) + + assert key_file_hash_1 == key_file_hash_2 + + @pytest.mark.slow def test_key_removal(key_file, tmp_path): initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) assert key_file.is_file() - remove_old_datastore_key() + get_datastore_encryptor().erase_key() assert not key_file.is_file() -def test_key_removal__no_key(key_file): - assert not key_file.is_file() - # Make sure no error thrown when we try to remove an non-existing key - remove_old_datastore_key() - - -def test_key_removal__no_key_2(key_file, tmp_path): +@pytest.mark.slow +def test_key_removal__no_key(key_file, tmp_path): assert not key_file.is_file() initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) assert key_file.is_file() - key_file.unlink() + get_datastore_encryptor().erase_key() assert not key_file.is_file() # Make sure no error thrown when we try to remove an non-existing key get_datastore_encryptor().erase_key() + + +@pytest.mark.slow +def test_reinitialize_datastore_encryptor(key_file, tmp_path): + initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) + key_file_hash_1 = get_file_sha256_hash(key_file) + + reinitialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) + key_file_hash_2 = get_file_sha256_hash(key_file) + + assert key_file_hash_1 != key_file_hash_2 From e673667b342de769ae1a8d6e9f7cebd2c7c6ee20 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 5 Oct 2021 16:44:59 -0400 Subject: [PATCH 12/18] Tests: Mark all tests in test_data_store_encryptor as slow --- .../server_utils/encryption/test_data_store_encryptor.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py index 4ae6b15e1..20bbf4b27 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py @@ -8,6 +8,9 @@ from monkey_island.cc.server_utils.encryption import ( reinitialize_datastore_encryptor, ) +# Mark all tests in this module as slow +pytestmark = pytest.mark.slow + PLAINTEXT = "Hello, Monkey!" MOCK_SECRET = "53CR31" @@ -25,7 +28,6 @@ def key_file(tmp_path): return tmp_path / KEY_FILENAME -@pytest.mark.slow def test_encryption(tmp_path): initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) @@ -36,14 +38,12 @@ def test_encryption(tmp_path): assert decrypted_data == PLAINTEXT -@pytest.mark.slow def test_key_creation(key_file, tmp_path): assert not key_file.is_file() initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) assert key_file.is_file() -@pytest.mark.slow def test_existing_key_reused(key_file, tmp_path): assert not key_file.is_file() @@ -56,7 +56,6 @@ def test_existing_key_reused(key_file, tmp_path): assert key_file_hash_1 == key_file_hash_2 -@pytest.mark.slow def test_key_removal(key_file, tmp_path): initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) assert key_file.is_file() @@ -65,7 +64,6 @@ def test_key_removal(key_file, tmp_path): assert not key_file.is_file() -@pytest.mark.slow def test_key_removal__no_key(key_file, tmp_path): assert not key_file.is_file() initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) @@ -78,7 +76,6 @@ def test_key_removal__no_key(key_file, tmp_path): get_datastore_encryptor().erase_key() -@pytest.mark.slow def test_reinitialize_datastore_encryptor(key_file, tmp_path): initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) key_file_hash_1 = get_file_sha256_hash(key_file) From 8310204e66d047565d656b908c05c866ad8a068c Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Wed, 6 Oct 2021 09:51:03 -0400 Subject: [PATCH 13/18] Tests: Test InvalidCiphertextError --- .../encryption/test_password_based_encryption.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_password_based_encryption.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_password_based_encryption.py index a231f3219..0e044c84a 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_password_based_encryption.py +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_password_based_encryption.py @@ -5,39 +5,45 @@ from tests.unit_tests.monkey_island.cc.services.utils.ciphertexts_for_encryption ) from monkey_island.cc.server_utils.encryption import ( + InvalidCiphertextError, InvalidCredentialsError, PasswordBasedStringEncryptor, ) +# Mark all tests in this module as slow +pytestmark = pytest.mark.slow + MONKEY_CONFIGS_DIR_PATH = "monkey_configs" STANDARD_PLAINTEXT_MONKEY_CONFIG_FILENAME = "monkey_config_standard.json" PASSWORD = "hello123" INCORRECT_PASSWORD = "goodbye321" -@pytest.mark.slow def test_encrypt_decrypt_string(monkey_config_json): pb_encryptor = PasswordBasedStringEncryptor(PASSWORD) encrypted_config = pb_encryptor.encrypt(monkey_config_json) assert pb_encryptor.decrypt(encrypted_config) == monkey_config_json -@pytest.mark.slow def test_decrypt_string__wrong_password(monkey_config_json): pb_encryptor = PasswordBasedStringEncryptor(INCORRECT_PASSWORD) with pytest.raises(InvalidCredentialsError): pb_encryptor.decrypt(VALID_CIPHER_TEXT) -@pytest.mark.slow def test_decrypt_string__malformed_corrupted(): pb_encryptor = PasswordBasedStringEncryptor(PASSWORD) with pytest.raises(ValueError): pb_encryptor.decrypt(MALFORMED_CIPHER_TEXT_CORRUPTED) -@pytest.mark.slow def test_decrypt_string__no_password(monkey_config_json): pb_encryptor = PasswordBasedStringEncryptor("") with pytest.raises(InvalidCredentialsError): pb_encryptor.decrypt(VALID_CIPHER_TEXT) + + +def test_decrypt_string__invalid_cyphertext(monkey_config_json): + pb_encryptor = PasswordBasedStringEncryptor("") + with pytest.raises(InvalidCiphertextError): + pb_encryptor.decrypt("") From 9ee00c304491b8e95537146ba15f2a7527c26713 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Wed, 6 Oct 2021 12:45:54 -0400 Subject: [PATCH 14/18] Tests: Reduce code duplication in test_data_store_encryptor.py --- .../encryption/test_data_store_encryptor.py | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py index 20bbf4b27..091fdb970 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py @@ -14,8 +14,6 @@ pytestmark = pytest.mark.slow PLAINTEXT = "Hello, Monkey!" MOCK_SECRET = "53CR31" -KEY_FILENAME = "test_key.bin" - @pytest.fixture(autouse=True) def cleanup_encryptor(): @@ -25,11 +23,11 @@ def cleanup_encryptor(): @pytest.fixture def key_file(tmp_path): - return tmp_path / KEY_FILENAME + return tmp_path / "test_key.bin" def test_encryption(tmp_path): - initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) + initialize_datastore_encryptor(tmp_path, MOCK_SECRET) encrypted_data = get_datastore_encryptor().encrypt(PLAINTEXT) assert encrypted_data != PLAINTEXT @@ -38,35 +36,35 @@ def test_encryption(tmp_path): assert decrypted_data == PLAINTEXT -def test_key_creation(key_file, tmp_path): +def test_key_creation(key_file): assert not key_file.is_file() - initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) + initialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) assert key_file.is_file() -def test_existing_key_reused(key_file, tmp_path): +def test_existing_key_reused(key_file): assert not key_file.is_file() - initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) + initialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) key_file_hash_1 = get_file_sha256_hash(key_file) - initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) + initialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) key_file_hash_2 = get_file_sha256_hash(key_file) assert key_file_hash_1 == key_file_hash_2 -def test_key_removal(key_file, tmp_path): - initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) +def test_key_removal(key_file): + initialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) assert key_file.is_file() get_datastore_encryptor().erase_key() assert not key_file.is_file() -def test_key_removal__no_key(key_file, tmp_path): +def test_key_removal__no_key(key_file): assert not key_file.is_file() - initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) + initialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) assert key_file.is_file() get_datastore_encryptor().erase_key() @@ -76,11 +74,11 @@ def test_key_removal__no_key(key_file, tmp_path): get_datastore_encryptor().erase_key() -def test_reinitialize_datastore_encryptor(key_file, tmp_path): - initialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) +def test_reinitialize_datastore_encryptor(key_file): + initialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) key_file_hash_1 = get_file_sha256_hash(key_file) - reinitialize_datastore_encryptor(tmp_path, MOCK_SECRET, KEY_FILENAME) + reinitialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) key_file_hash_2 = get_file_sha256_hash(key_file) assert key_file_hash_1 != key_file_hash_2 From 2d414a6f7d8d89fe69a807871654112873d14e85 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Thu, 7 Oct 2021 13:37:38 -0400 Subject: [PATCH 15/18] Island: Ensure old key files are deleted on reinitialization --- .../encryption/data_store_encryptor.py | 21 +++------- .../encryption/test_data_store_encryptor.py | 42 ++++++++++--------- 2 files changed, 27 insertions(+), 36 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py index 7e9a279d4..9ade1364d 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py @@ -49,29 +49,18 @@ class DataStoreEncryptor(IEncryptor): def decrypt(self, ciphertext: str): return self._key_based_encryptor.decrypt(ciphertext) - def erase_key(self): - if self._key_file_path.is_file(): - self._key_file_path.unlink() - - self._key_based_encryptor = None - def reinitialize_datastore_encryptor( key_file_dir: str, secret: str, key_file_name: str = "mongo_key.bin" ): - _delete_encryptor() + key_file_path = Path(key_file_dir) / key_file_name + + if key_file_path.is_file(): + key_file_path.unlink() + initialize_datastore_encryptor(key_file_dir, secret, key_file_name) -def _delete_encryptor(): - global _encryptor - - if _encryptor: - _encryptor.erase_key() - - _encryptor = None - - def initialize_datastore_encryptor( key_file_dir: str, secret: str, key_file_name: str = "mongo_key.bin" ): diff --git a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py index 091fdb970..8f6c8947a 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py @@ -54,26 +54,6 @@ def test_existing_key_reused(key_file): assert key_file_hash_1 == key_file_hash_2 -def test_key_removal(key_file): - initialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) - assert key_file.is_file() - - get_datastore_encryptor().erase_key() - assert not key_file.is_file() - - -def test_key_removal__no_key(key_file): - assert not key_file.is_file() - initialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) - assert key_file.is_file() - - get_datastore_encryptor().erase_key() - assert not key_file.is_file() - - # Make sure no error thrown when we try to remove an non-existing key - get_datastore_encryptor().erase_key() - - def test_reinitialize_datastore_encryptor(key_file): initialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) key_file_hash_1 = get_file_sha256_hash(key_file) @@ -82,3 +62,25 @@ def test_reinitialize_datastore_encryptor(key_file): key_file_hash_2 = get_file_sha256_hash(key_file) assert key_file_hash_1 != key_file_hash_2 + + +def test_reinitialize_when_encryptor_is_none(key_file): + with key_file.open(mode="w") as f: + f.write("") + + reinitialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) + assert ( + get_file_sha256_hash(key_file) + != "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + ) + + +def test_reinitialize_when_file_not_found(key_file): + assert not key_file.is_file() + reinitialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) + + encrypted_data = get_datastore_encryptor().encrypt(PLAINTEXT) + assert encrypted_data != PLAINTEXT + + decrypted_data = get_datastore_encryptor().decrypt(encrypted_data) + assert decrypted_data == PLAINTEXT From bdf485e014c952d2eb30d855131f4c28534e8b55 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Thu, 7 Oct 2021 13:50:06 -0400 Subject: [PATCH 16/18] Island: Rename data_store_encryptor initialization functions --- .../monkey_island/cc/resources/auth/auth.py | 2 +- .../cc/server_utils/encryption/__init__.py | 4 +-- .../encryption/data_store_encryptor.py | 8 +++--- .../cc/services/authentication.py | 18 +++++-------- .../unit_tests/monkey_island/cc/conftest.py | 4 +-- .../encryption/test_data_store_encryptor.py | 26 +++++++++---------- 6 files changed, 27 insertions(+), 35 deletions(-) diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 92a372a99..4e31778bf 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -44,7 +44,7 @@ class Authenticate(flask_restful.Resource): username, password = get_username_password_from_request(request) if _credentials_match_registered_user(username, password): - AuthenticationService.ensure_datastore_encryptor(username, password) + AuthenticationService.unlock_datastore_encryptor(username, password) access_token = _create_access_token(username) return make_response({"access_token": access_token, "error": ""}, 200) else: diff --git a/monkey/monkey_island/cc/server_utils/encryption/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/__init__.py index 7fbc882cb..16ac78cbe 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/__init__.py +++ b/monkey/monkey_island/cc/server_utils/encryption/__init__.py @@ -13,8 +13,8 @@ from .password_based_bytes_encryptor import ( ) from .data_store_encryptor import ( get_datastore_encryptor, - initialize_datastore_encryptor, - reinitialize_datastore_encryptor, + unlock_datastore_encryptor, + reset_datastore_encryptor, ) from .dict_encryptor import ( SensitiveField, diff --git a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py index 9ade1364d..3e5415f0d 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py @@ -50,18 +50,16 @@ class DataStoreEncryptor(IEncryptor): return self._key_based_encryptor.decrypt(ciphertext) -def reinitialize_datastore_encryptor( - key_file_dir: str, secret: str, key_file_name: str = "mongo_key.bin" -): +def reset_datastore_encryptor(key_file_dir: str, secret: str, key_file_name: str = "mongo_key.bin"): key_file_path = Path(key_file_dir) / key_file_name if key_file_path.is_file(): key_file_path.unlink() - initialize_datastore_encryptor(key_file_dir, secret, key_file_name) + unlock_datastore_encryptor(key_file_dir, secret, key_file_name) -def initialize_datastore_encryptor( +def unlock_datastore_encryptor( key_file_dir: str, secret: str, key_file_name: str = "mongo_key.bin" ): global _encryptor diff --git a/monkey/monkey_island/cc/services/authentication.py b/monkey/monkey_island/cc/services/authentication.py index 9cb0121c5..88b5f3fb0 100644 --- a/monkey/monkey_island/cc/services/authentication.py +++ b/monkey/monkey_island/cc/services/authentication.py @@ -1,7 +1,6 @@ from monkey_island.cc.server_utils.encryption import ( - get_datastore_encryptor, - initialize_datastore_encryptor, - reinitialize_datastore_encryptor, + reset_datastore_encryptor, + unlock_datastore_encryptor, ) @@ -16,19 +15,14 @@ class AuthenticationService: cls.KEY_FILE_DIRECTORY = key_file_directory @staticmethod - def ensure_datastore_encryptor(username: str, password: str): - if not get_datastore_encryptor(): - AuthenticationService._init_encryptor_from_credentials(username, password) + def unlock_datastore_encryptor(username: str, password: str): + secret = AuthenticationService._get_secret_from_credentials(username, password) + unlock_datastore_encryptor(AuthenticationService.KEY_FILE_DIRECTORY, secret) @staticmethod def reset_datastore_encryptor(username: str, password: str): secret = AuthenticationService._get_secret_from_credentials(username, password) - reinitialize_datastore_encryptor(AuthenticationService.KEY_FILE_DIRECTORY, secret) - - @staticmethod - def _init_encryptor_from_credentials(username: str, password: str): - secret = AuthenticationService._get_secret_from_credentials(username, password) - initialize_datastore_encryptor(AuthenticationService.KEY_FILE_DIRECTORY, secret) + reset_datastore_encryptor(AuthenticationService.KEY_FILE_DIRECTORY, secret) @staticmethod def _get_secret_from_credentials(username: str, password: str) -> str: diff --git a/monkey/tests/unit_tests/monkey_island/cc/conftest.py b/monkey/tests/unit_tests/monkey_island/cc/conftest.py index 22390dea7..dfd927f4a 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/conftest.py +++ b/monkey/tests/unit_tests/monkey_island/cc/conftest.py @@ -10,7 +10,7 @@ from tests.unit_tests.monkey_island.cc.server_utils.encryption.test_password_bas STANDARD_PLAINTEXT_MONKEY_CONFIG_FILENAME, ) -from monkey_island.cc.server_utils.encryption import initialize_datastore_encryptor +from monkey_island.cc.server_utils.encryption import unlock_datastore_encryptor @pytest.fixture @@ -30,4 +30,4 @@ def monkey_config_json(monkey_config): @pytest.fixture def uses_encryptor(data_for_tests_dir): secret = "m0nk3y_u53r:3cr3t_p455w0rd" - initialize_datastore_encryptor(data_for_tests_dir, secret) + unlock_datastore_encryptor(data_for_tests_dir, secret) diff --git a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py index 8f6c8947a..da4a9ec09 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_data_store_encryptor.py @@ -4,8 +4,8 @@ from common.utils.file_utils import get_file_sha256_hash from monkey_island.cc.server_utils.encryption import ( data_store_encryptor, get_datastore_encryptor, - initialize_datastore_encryptor, - reinitialize_datastore_encryptor, + reset_datastore_encryptor, + unlock_datastore_encryptor, ) # Mark all tests in this module as slow @@ -27,7 +27,7 @@ def key_file(tmp_path): def test_encryption(tmp_path): - initialize_datastore_encryptor(tmp_path, MOCK_SECRET) + unlock_datastore_encryptor(tmp_path, MOCK_SECRET) encrypted_data = get_datastore_encryptor().encrypt(PLAINTEXT) assert encrypted_data != PLAINTEXT @@ -38,46 +38,46 @@ def test_encryption(tmp_path): def test_key_creation(key_file): assert not key_file.is_file() - initialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) + unlock_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) assert key_file.is_file() def test_existing_key_reused(key_file): assert not key_file.is_file() - initialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) + unlock_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) key_file_hash_1 = get_file_sha256_hash(key_file) - initialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) + unlock_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) key_file_hash_2 = get_file_sha256_hash(key_file) assert key_file_hash_1 == key_file_hash_2 -def test_reinitialize_datastore_encryptor(key_file): - initialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) +def test_reset_datastore_encryptor(key_file): + unlock_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) key_file_hash_1 = get_file_sha256_hash(key_file) - reinitialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) + reset_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) key_file_hash_2 = get_file_sha256_hash(key_file) assert key_file_hash_1 != key_file_hash_2 -def test_reinitialize_when_encryptor_is_none(key_file): +def test_reset_when_encryptor_is_none(key_file): with key_file.open(mode="w") as f: f.write("") - reinitialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) + reset_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) assert ( get_file_sha256_hash(key_file) != "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" ) -def test_reinitialize_when_file_not_found(key_file): +def test_reset_when_file_not_found(key_file): assert not key_file.is_file() - reinitialize_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) + reset_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name) encrypted_data = get_datastore_encryptor().encrypt(PLAINTEXT) assert encrypted_data != PLAINTEXT From 1a0a07d550d9282bc1ead3b0ca1ca7b9fb5245d9 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Thu, 7 Oct 2021 14:39:19 -0400 Subject: [PATCH 17/18] Island: Reduce duplication in data_store_encryptor --- .../encryption/data_store_encryptor.py | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py index 3e5415f0d..fb4308a33 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py @@ -10,25 +10,27 @@ from .i_encryptor import IEncryptor from .key_based_encryptor import KeyBasedEncryptor from .password_based_bytes_encryptor import PasswordBasedBytesEncryptor +_KEY_FILE_NAME = "mongo_key.bin" + _encryptor: Union[None, IEncryptor] = None class DataStoreEncryptor(IEncryptor): _KEY_LENGTH_BYTES = 32 - def __init__(self, secret: str, key_file_path: Path): - self._key_file_path = key_file_path + def __init__(self, secret: str, key_file: Path): + self._key_file = key_file self._password_based_encryptor = PasswordBasedBytesEncryptor(secret) self._key_based_encryptor = self._initialize_key_based_encryptor() def _initialize_key_based_encryptor(self): - if os.path.exists(self._key_file_path): + if os.path.exists(self._key_file): return self._load_existing_key() return self._create_new_key() def _load_existing_key(self) -> KeyBasedEncryptor: - with open(self._key_file_path, "rb") as f: + with open(self._key_file, "rb") as f: encrypted_key = f.read() plaintext_key = self._password_based_encryptor.decrypt(encrypted_key) @@ -38,7 +40,7 @@ class DataStoreEncryptor(IEncryptor): plaintext_key = Random.new().read(DataStoreEncryptor._KEY_LENGTH_BYTES) encrypted_key = self._password_based_encryptor.encrypt(plaintext_key) - with open_new_securely_permissioned_file(self._key_file_path, "wb") as f: + with open_new_securely_permissioned_file(self._key_file, "wb") as f: f.write(encrypted_key) return KeyBasedEncryptor(plaintext_key) @@ -50,22 +52,24 @@ class DataStoreEncryptor(IEncryptor): return self._key_based_encryptor.decrypt(ciphertext) -def reset_datastore_encryptor(key_file_dir: str, secret: str, key_file_name: str = "mongo_key.bin"): - key_file_path = Path(key_file_dir) / key_file_name +def reset_datastore_encryptor(key_file_dir: str, secret: str, key_file_name: str = _KEY_FILE_NAME): + key_file = Path(key_file_dir) / key_file_name - if key_file_path.is_file(): - key_file_path.unlink() + if key_file.is_file(): + key_file.unlink() - unlock_datastore_encryptor(key_file_dir, secret, key_file_name) + _initialize_datastore_encryptor(key_file, secret) -def unlock_datastore_encryptor( - key_file_dir: str, secret: str, key_file_name: str = "mongo_key.bin" -): +def unlock_datastore_encryptor(key_file_dir: str, secret: str, key_file_name: str = _KEY_FILE_NAME): + key_file = Path(key_file_dir) / key_file_name + _initialize_datastore_encryptor(key_file, secret) + + +def _initialize_datastore_encryptor(key_file: Path, secret: str): global _encryptor - key_file_path = Path(key_file_dir) / key_file_name - _encryptor = DataStoreEncryptor(secret, key_file_path) + _encryptor = DataStoreEncryptor(secret, key_file) def get_datastore_encryptor() -> IEncryptor: From 97c3ed3b97c0a1ae93cb2d14d0bd7ada76a52bb9 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Thu, 7 Oct 2021 14:45:00 -0400 Subject: [PATCH 18/18] Island: Rename internal DataStoreEncryptor methods --- .../cc/server_utils/encryption/data_store_encryptor.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py index fb4308a33..e9ff96716 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py +++ b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py @@ -25,18 +25,18 @@ class DataStoreEncryptor(IEncryptor): def _initialize_key_based_encryptor(self): if os.path.exists(self._key_file): - return self._load_existing_key() + return self._load_key() - return self._create_new_key() + return self._create_key() - def _load_existing_key(self) -> KeyBasedEncryptor: + def _load_key(self) -> KeyBasedEncryptor: with open(self._key_file, "rb") as f: encrypted_key = f.read() plaintext_key = self._password_based_encryptor.decrypt(encrypted_key) return KeyBasedEncryptor(plaintext_key) - def _create_new_key(self) -> KeyBasedEncryptor: + def _create_key(self) -> KeyBasedEncryptor: plaintext_key = Random.new().read(DataStoreEncryptor._KEY_LENGTH_BYTES) encrypted_key = self._password_based_encryptor.encrypt(plaintext_key)