From 191fbea66554df545a69790b1fbab27e6a9320fa Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Thu, 30 Sep 2021 10:10:20 +0300 Subject: [PATCH 01/17] Refactor password based encryptor into PasswordBasedStringEncryptor and PasswordBasedByteEncryptor This change allows to encrypt strings and bytes without any additional conversion done on the caller --- .../cc/resources/configuration_export.py | 4 +- .../cc/resources/configuration_import.py | 4 +- .../cc/server_utils/encryption/__init__.py | 7 ++-- ...n.py => password_based_byte_encryption.py} | 31 +++++---------- .../password_based_string_encryption.py | 38 +++++++++++++++++++ .../cc/resources/test_configuration_import.py | 4 +- .../test_password_based_encryption.py | 13 ++++--- 7 files changed, 64 insertions(+), 37 deletions(-) rename monkey/monkey_island/cc/server_utils/encryption/{password_based_encryption.py => password_based_byte_encryption.py} (64%) create mode 100644 monkey/monkey_island/cc/server_utils/encryption/password_based_string_encryption.py diff --git a/monkey/monkey_island/cc/resources/configuration_export.py b/monkey/monkey_island/cc/resources/configuration_export.py index c550acc7d2d..111cfa1771a 100644 --- a/monkey/monkey_island/cc/resources/configuration_export.py +++ b/monkey/monkey_island/cc/resources/configuration_export.py @@ -4,7 +4,7 @@ from flask import request from monkey_island.cc.resources.auth.auth import jwt_required -from monkey_island.cc.server_utils.encryption import PasswordBasedEncryptor +from monkey_island.cc.server_utils.encryption import PasswordBasedStringEncryptor from monkey_island.cc.services.config import ConfigService @@ -21,7 +21,7 @@ def post(self): password = data["password"] plaintext_config = json.dumps(plaintext_config) - pb_encryptor = PasswordBasedEncryptor(password) + pb_encryptor = PasswordBasedStringEncryptor(password) config_export = pb_encryptor.encrypt(plaintext_config) return {"config_export": config_export, "encrypted": should_encrypt} diff --git a/monkey/monkey_island/cc/resources/configuration_import.py b/monkey/monkey_island/cc/resources/configuration_import.py index 6c0575e9453..3a66a2ed072 100644 --- a/monkey/monkey_island/cc/resources/configuration_import.py +++ b/monkey/monkey_island/cc/resources/configuration_import.py @@ -11,7 +11,7 @@ from monkey_island.cc.server_utils.encryption import ( InvalidCiphertextError, InvalidCredentialsError, - PasswordBasedEncryptor, + PasswordBasedStringEncryptor, is_encrypted, ) from monkey_island.cc.services.config import ConfigService @@ -72,7 +72,7 @@ def _get_plaintext_config_from_request(request_contents: dict) -> dict: try: config = request_contents["config"] if ConfigurationImport.is_config_encrypted(request_contents["config"]): - pb_encryptor = PasswordBasedEncryptor(request_contents["password"]) + pb_encryptor = PasswordBasedStringEncryptor(request_contents["password"]) config = pb_encryptor.decrypt(config) return json.loads(config) except (JSONDecodeError, InvalidCiphertextError): diff --git a/monkey/monkey_island/cc/server_utils/encryption/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/__init__.py index 7d806139ca7..84e6e625229 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/__init__.py +++ b/monkey/monkey_island/cc/server_utils/encryption/__init__.py @@ -1,11 +1,10 @@ from monkey_island.cc.server_utils.encryption.i_encryptor import IEncryptor from monkey_island.cc.server_utils.encryption.key_based_encryptor import KeyBasedEncryptor -from monkey_island.cc.server_utils.encryption.password_based_encryption import ( - InvalidCiphertextError, - InvalidCredentialsError, - PasswordBasedEncryptor, +from monkey_island.cc.server_utils.encryption.password_based_string_encryption import ( + PasswordBasedStringEncryptor, is_encrypted, ) +from .password_based_byte_encryption import InvalidCredentialsError, InvalidCiphertextError from monkey_island.cc.server_utils.encryption.data_store_encryptor import ( DataStoreEncryptor, get_datastore_encryptor, diff --git a/monkey/monkey_island/cc/server_utils/encryption/password_based_encryption.py b/monkey/monkey_island/cc/server_utils/encryption/password_based_byte_encryption.py similarity index 64% rename from monkey/monkey_island/cc/server_utils/encryption/password_based_encryption.py rename to monkey/monkey_island/cc/server_utils/encryption/password_based_byte_encryption.py index 20708ce3136..569dc0d12de 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/password_based_encryption.py +++ b/monkey/monkey_island/cc/server_utils/encryption/password_based_byte_encryption.py @@ -1,6 +1,6 @@ -import base64 import io import logging +from io import BytesIO import pyAesCrypt @@ -17,36 +17,28 @@ # Note: password != key -class PasswordBasedEncryptor(IEncryptor): +class PasswordBasedByteEncryptor(IEncryptor): _BUFFER_SIZE = pyAesCrypt.crypto.bufferSizeDef def __init__(self, password: str): self.password = password - def encrypt(self, plaintext: str) -> str: - plaintext_stream = io.BytesIO(plaintext.encode()) + def encrypt(self, plaintext: BytesIO) -> BytesIO: ciphertext_stream = io.BytesIO() - pyAesCrypt.encryptStream( - plaintext_stream, ciphertext_stream, self.password, self._BUFFER_SIZE - ) + pyAesCrypt.encryptStream(plaintext, ciphertext_stream, self.password, self._BUFFER_SIZE) - ciphertext_b64 = base64.b64encode(ciphertext_stream.getvalue()) - logger.info("String encrypted.") + return ciphertext_stream - return ciphertext_b64.decode() - - def decrypt(self, ciphertext: str): - ciphertext = base64.b64decode(ciphertext) - ciphertext_stream = io.BytesIO(ciphertext) + def decrypt(self, ciphertext: BytesIO) -> BytesIO: plaintext_stream = io.BytesIO() - ciphertext_stream_len = len(ciphertext_stream.getvalue()) + ciphertext_stream_len = len(ciphertext.getvalue()) try: pyAesCrypt.decryptStream( - ciphertext_stream, + ciphertext, plaintext_stream, self.password, self._BUFFER_SIZE, @@ -59,7 +51,7 @@ def decrypt(self, ciphertext: str): else: logger.info("The corrupt ciphertext provided.") raise InvalidCiphertextError - return plaintext_stream.getvalue().decode("utf-8") + return plaintext_stream class InvalidCredentialsError(Exception): @@ -68,8 +60,3 @@ class InvalidCredentialsError(Exception): class InvalidCiphertextError(Exception): """ Raised when ciphertext is corrupted """ - - -def is_encrypted(ciphertext: str) -> bool: - ciphertext = base64.b64decode(ciphertext) - return ciphertext.startswith(b"AES") diff --git a/monkey/monkey_island/cc/server_utils/encryption/password_based_string_encryption.py b/monkey/monkey_island/cc/server_utils/encryption/password_based_string_encryption.py new file mode 100644 index 00000000000..ea796a441ea --- /dev/null +++ b/monkey/monkey_island/cc/server_utils/encryption/password_based_string_encryption.py @@ -0,0 +1,38 @@ +import base64 +import io +import logging + +import pyAesCrypt + +from monkey_island.cc.server_utils.encryption import IEncryptor +from monkey_island.cc.server_utils.encryption.password_based_byte_encryption import ( + PasswordBasedByteEncryptor, +) + +logger = logging.getLogger(__name__) + + +class PasswordBasedStringEncryptor(IEncryptor): + + _BUFFER_SIZE = pyAesCrypt.crypto.bufferSizeDef + + def __init__(self, password: str): + self.password = password + + def encrypt(self, plaintext: str) -> str: + plaintext_stream = io.BytesIO(plaintext.encode()) + ciphertext = PasswordBasedByteEncryptor(self.password).encrypt(plaintext_stream) + + return base64.b64encode(ciphertext.getvalue()).decode() + + def decrypt(self, ciphertext: str) -> str: + ciphertext = base64.b64decode(ciphertext) + ciphertext_stream = io.BytesIO(ciphertext) + + plaintext_stream = PasswordBasedByteEncryptor(self.password).decrypt(ciphertext_stream) + return plaintext_stream.getvalue().decode("utf-8") + + +def is_encrypted(ciphertext: str) -> bool: + ciphertext = base64.b64decode(ciphertext) + return ciphertext.startswith(b"AES") diff --git a/monkey/tests/unit_tests/monkey_island/cc/resources/test_configuration_import.py b/monkey/tests/unit_tests/monkey_island/cc/resources/test_configuration_import.py index fb397f234d7..bf7ccff803b 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/resources/test_configuration_import.py +++ b/monkey/tests/unit_tests/monkey_island/cc/resources/test_configuration_import.py @@ -8,7 +8,7 @@ from common.utils.exceptions import InvalidConfigurationError from monkey_island.cc.resources.configuration_import import ConfigurationImport -from monkey_island.cc.server_utils.encryption import PasswordBasedEncryptor +from monkey_island.cc.server_utils.encryption import PasswordBasedStringEncryptor def test_is_config_encrypted__json(monkey_config_json): @@ -17,7 +17,7 @@ def test_is_config_encrypted__json(monkey_config_json): @pytest.mark.slow def test_is_config_encrypted__ciphertext(monkey_config_json): - pb_encryptor = PasswordBasedEncryptor(PASSWORD) + pb_encryptor = PasswordBasedStringEncryptor(PASSWORD) encrypted_config = pb_encryptor.encrypt(monkey_config_json) assert ConfigurationImport.is_config_encrypted(encrypted_config) 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 d00609481be..a231f321916 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 @@ -4,7 +4,10 @@ VALID_CIPHER_TEXT, ) -from monkey_island.cc.server_utils.encryption import InvalidCredentialsError, PasswordBasedEncryptor +from monkey_island.cc.server_utils.encryption import ( + InvalidCredentialsError, + PasswordBasedStringEncryptor, +) MONKEY_CONFIGS_DIR_PATH = "monkey_configs" STANDARD_PLAINTEXT_MONKEY_CONFIG_FILENAME = "monkey_config_standard.json" @@ -14,27 +17,27 @@ @pytest.mark.slow def test_encrypt_decrypt_string(monkey_config_json): - pb_encryptor = PasswordBasedEncryptor(PASSWORD) + 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 = PasswordBasedEncryptor(INCORRECT_PASSWORD) + 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 = PasswordBasedEncryptor(PASSWORD) + 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 = PasswordBasedEncryptor("") + pb_encryptor = PasswordBasedStringEncryptor("") with pytest.raises(InvalidCredentialsError): pb_encryptor.decrypt(VALID_CIPHER_TEXT) From fd1cb9d36ddd376c8b6d112f8fc157889a28a420 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Thu, 30 Sep 2021 12:02:43 +0300 Subject: [PATCH 02/17] Add a secret to datastore encryptor This change enables the encryption/decryption of mongo key with a custom secret --- .../encryption/data_store_encryptor.py | 30 ++++++++++------ monkey/tests/data_for_tests/mongo_key.bin | Bin 32 -> 327 bytes .../unit_tests/monkey_island/cc/conftest.py | 5 ++- .../test_string_list_encryptor.py | 8 ----- .../encryption/test_data_store_encryptor.py | 33 +++++++----------- .../test_scoutsuite_auth_service.py | 9 ++--- 6 files changed, 38 insertions(+), 47 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 215703c0248..df185a1c854 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,3 +1,4 @@ +import io import os # PyCrypto is deprecated, but we use pycryptodome, which uses the exact same imports but @@ -5,6 +6,9 @@ from Crypto import Random # noqa: DUO133 # nosec: B413 from monkey_island.cc.server_utils.encryption import KeyBasedEncryptor +from monkey_island.cc.server_utils.encryption.password_based_byte_encryption import ( + PasswordBasedByteEncryptor, +) from monkey_island.cc.server_utils.file_utils import open_new_securely_permissioned_file _encryptor = None @@ -14,24 +18,30 @@ class DataStoreEncryptor: _BLOCK_SIZE = 32 _KEY_FILENAME = "mongo_key.bin" - def __init__(self, key_file_dir): + def __init__(self, key_file_dir: str, secret: str): key_file = os.path.join(key_file_dir, self._KEY_FILENAME) if os.path.exists(key_file): - self._load_existing_key(key_file) + self._load_existing_key(key_file, secret) else: - self._init_key(key_file) + self._init_key(key_file, secret) self._key_base_encryptor = KeyBasedEncryptor(self._cipher_key) - def _init_key(self, password_file_path: str): + def _init_key(self, password_file_path: str, secret: str): self._cipher_key = Random.new().read(self._BLOCK_SIZE) + encrypted_key = ( + PasswordBasedByteEncryptor(secret).encrypt(io.BytesIO(self._cipher_key)).getvalue() + ) with open_new_securely_permissioned_file(password_file_path, "wb") as f: - f.write(self._cipher_key) + f.write(encrypted_key) - def _load_existing_key(self, key_file): - with open(key_file, "rb") as f: - self._cipher_key = f.read() + def _load_existing_key(self, key_file_path: str, secret: str): + with open(key_file_path, "rb") as f: + encrypted_key = f.read() + self._cipher_key = ( + PasswordBasedByteEncryptor(secret).decrypt(io.BytesIO(encrypted_key)).getvalue() + ) def enc(self, message: str): return self._key_base_encryptor.encrypt(message) @@ -40,10 +50,10 @@ def dec(self, enc_message: str): return self._key_base_encryptor.decrypt(enc_message) -def initialize_datastore_encryptor(key_file_dir): +def initialize_datastore_encryptor(key_file_dir: str, secret: str): global _encryptor - _encryptor = DataStoreEncryptor(key_file_dir) + _encryptor = DataStoreEncryptor(key_file_dir, secret) def get_datastore_encryptor(): diff --git a/monkey/tests/data_for_tests/mongo_key.bin b/monkey/tests/data_for_tests/mongo_key.bin index 6b8091efb87cf62909e0df7cb1b32a26fbbe1b13..edf082ae1b34cf2b5b99b1d7598909643aff89b3 100644 GIT binary patch literal 327 zcmZ>C4Q66skaiAobqsNJiFb-*D5!KyEp{%dEGSVh(=*UBU}#_%aA4gLUR!lWLqF{H z7WPDm&*jT!th~@Azpn1ayd?)pPMGU^Em3H&?domnOAUC*x%?JiYpsIra^0j-+g$zyQPc!dY-F28z?NHS=?+{PK z48HwJ#?F!Rvw3th&pKD->W5k~JkjPBwC>iroL6Oc;f|_YWJL42&CV?R>B^^{)yCdp G(gOfVLQz=& literal 32 qcmV+*0N?*B`K`wF4N^}E{V5abou0lKRFizfQd8~TCPNe92btmtybtgI diff --git a/monkey/tests/unit_tests/monkey_island/cc/conftest.py b/monkey/tests/unit_tests/monkey_island/cc/conftest.py index 9cca0caabe7..cde82e939f3 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/conftest.py +++ b/monkey/tests/unit_tests/monkey_island/cc/conftest.py @@ -27,6 +27,9 @@ def monkey_config_json(monkey_config): return json.dumps(monkey_config) +ENCRYPTOR_SECRET = "m0nk3y_u53r:53cr3t_p455w0rd" + + @pytest.fixture def uses_encryptor(data_for_tests_dir): - initialize_datastore_encryptor(data_for_tests_dir) + initialize_datastore_encryptor(data_for_tests_dir, ENCRYPTOR_SECRET) 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 d02ad5bbbf2..7ea21849b8c 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,6 +1,3 @@ -import pytest - -from monkey_island.cc.server_utils.encryption import initialize_datastore_encryptor from monkey_island.cc.server_utils.encryption.dict_encryption.field_encryptors import ( StringListEncryptor, ) @@ -9,11 +6,6 @@ EMPTY_LIST = [] -@pytest.fixture -def uses_encryptor(data_for_tests_dir): - initialize_datastore_encryptor(data_for_tests_dir) - - def test_encryption_and_decryption(uses_encryptor): encrypted_list = StringListEncryptor.encrypt(MOCK_STRING_LIST) assert not encrypted_list == MOCK_STRING_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 bb005fbf709..98229c6fe7c 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,35 +1,26 @@ import os +import pytest +from tests.unit_tests.monkey_island.cc.conftest import ENCRYPTOR_SECRET + from monkey_island.cc.server_utils.encryption import ( + DataStoreEncryptor, get_datastore_encryptor, initialize_datastore_encryptor, ) -PASSWORD_FILENAME = "mongo_key.bin" - PLAINTEXT = "Hello, Monkey!" -CYPHERTEXT = "vKgvD6SjRyIh1dh2AM/rnTa0NI/vjfwnbZLbMocWtE4e42WJmSUz2ordtbQrH1Fq" - - -def test_aes_cbc_encryption(data_for_tests_dir): - initialize_datastore_encryptor(data_for_tests_dir) - - assert get_datastore_encryptor().enc(PLAINTEXT) != PLAINTEXT -def test_aes_cbc_decryption(data_for_tests_dir): - initialize_datastore_encryptor(data_for_tests_dir) +@pytest.mark.usefixtures("uses_encryptor") +def test_encryption(data_for_tests_dir): + encrypted_data = get_datastore_encryptor().enc(PLAINTEXT) + assert encrypted_data != PLAINTEXT - assert get_datastore_encryptor().dec(CYPHERTEXT) == PLAINTEXT - - -def test_aes_cbc_enc_dec(data_for_tests_dir): - initialize_datastore_encryptor(data_for_tests_dir) - - assert get_datastore_encryptor().dec(get_datastore_encryptor().enc(PLAINTEXT)) == PLAINTEXT + decrypted_data = get_datastore_encryptor().dec(encrypted_data) + assert decrypted_data == PLAINTEXT def test_create_new_password_file(tmpdir): - initialize_datastore_encryptor(tmpdir) - - assert os.path.isfile(os.path.join(tmpdir, PASSWORD_FILENAME)) + initialize_datastore_encryptor(tmpdir, ENCRYPTOR_SECRET) + assert os.path.isfile(os.path.join(tmpdir, DataStoreEncryptor._KEY_FILENAME)) diff --git a/monkey/tests/unit_tests/monkey_island/cc/services/zero_trust/scoutsuite/test_scoutsuite_auth_service.py b/monkey/tests/unit_tests/monkey_island/cc/services/zero_trust/scoutsuite/test_scoutsuite_auth_service.py index 2e6c2fd5011..32b4f19ad20 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/services/zero_trust/scoutsuite/test_scoutsuite_auth_service.py +++ b/monkey/tests/unit_tests/monkey_island/cc/services/zero_trust/scoutsuite/test_scoutsuite_auth_service.py @@ -5,10 +5,7 @@ from common.config_value_paths import AWS_KEYS_PATH from monkey_island.cc.database import mongo -from monkey_island.cc.server_utils.encryption import ( - get_datastore_encryptor, - initialize_datastore_encryptor, -) +from monkey_island.cc.server_utils.encryption import get_datastore_encryptor from monkey_island.cc.services.config import ConfigService from monkey_island.cc.services.zero_trust.scoutsuite.scoutsuite_auth_service import ( is_aws_keys_setup, @@ -19,7 +16,7 @@ class MockObject: pass -@pytest.mark.usefixtures("uses_database") +@pytest.mark.usefixtures("uses_database", "uses_encryptor") def test_is_aws_keys_setup(tmp_path): # Mock default configuration ConfigService.init_default_config() @@ -29,8 +26,6 @@ def test_is_aws_keys_setup(tmp_path): mongo.db.config.find_one = MagicMock(return_value=ConfigService.default_config) assert not is_aws_keys_setup() - # Make sure noone changed config path and broke this function - initialize_datastore_encryptor(tmp_path) bogus_key_value = get_datastore_encryptor().enc("bogus_aws_key") dpath.util.set( ConfigService.default_config, AWS_KEYS_PATH + ["aws_secret_access_key"], bogus_key_value From 4f176939bba78c936dfc739ce86352ea794c9f45 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Thu, 30 Sep 2021 17:08:38 +0300 Subject: [PATCH 03/17] Split up the initialization of mongo_key into 2 parts: directory of mongo key initialization that happens during launch and initialization of key which happens after login or registration --- .../monkey_island/cc/resources/auth/auth.py | 24 ++++---- .../cc/resources/auth/credential_utils.py | 37 ++++++++++++ .../cc/resources/auth/password_utils.py | 12 ---- .../cc/resources/auth/registration.py | 22 +++---- .../encryption/data_store_encryptor.py | 59 +++++++++++++------ .../cc/ui/src/services/AuthService.js | 2 +- .../unit_tests/monkey_island/cc/conftest.py | 4 +- .../encryption/test_data_store_encryptor.py | 4 +- 8 files changed, 102 insertions(+), 62 deletions(-) create mode 100644 monkey/monkey_island/cc/resources/auth/credential_utils.py delete mode 100644 monkey/monkey_island/cc/resources/auth/password_utils.py diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 064395eafb9..9c1c8fc6203 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -1,4 +1,3 @@ -import json import logging from functools import wraps @@ -9,8 +8,13 @@ from jwt import PyJWTError 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 +from monkey_island.cc.resources.auth.credential_utils import ( + get_creds_from_request, + get_secret_from_request, + password_matches_hash, +) +from monkey_island.cc.server_utils.encryption.data_store_encryptor import setup_datastore_key logger = logging.getLogger(__name__) @@ -38,28 +42,20 @@ def post(self): "password": "my_password" } """ - (username, password) = _get_credentials_from_request(request) + username, password = get_creds_from_request(request) if _credentials_match_registered_user(username, password): + setup_datastore_key(get_secret_from_request(request)) 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): +def _credentials_match_registered_user(username: str, password: str): user = user_store.UserStore.username_table.get(username, None) - if user and password_utils.password_matches_hash(password, user.secret): + if user and password_matches_hash(password, user.secret): return True return False diff --git a/monkey/monkey_island/cc/resources/auth/credential_utils.py b/monkey/monkey_island/cc/resources/auth/credential_utils.py new file mode 100644 index 00000000000..1d7a0080335 --- /dev/null +++ b/monkey/monkey_island/cc/resources/auth/credential_utils.py @@ -0,0 +1,37 @@ +import json +from typing import Tuple + +import bcrypt +from flask import Request, request + +from monkey_island.cc.environment.user_creds import UserCreds + + +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")) + + +def get_user_credentials_from_request(_request) -> UserCreds: + username, password = get_creds_from_request(_request) + password_hash = hash_password(password) + + return UserCreds(username, password_hash) + + +def get_secret_from_request(_request) -> str: + username, password = get_creds_from_request(_request) + return f"{username}:{password}" + + +def get_creds_from_request(_request: Request) -> Tuple[str, str]: + cred_dict = json.loads(request.data) + username = cred_dict.get("username", "") + password = cred_dict.get("password", "") + return username, password diff --git a/monkey/monkey_island/cc/resources/auth/password_utils.py b/monkey/monkey_island/cc/resources/auth/password_utils.py deleted file mode 100644 index f470fd8822b..00000000000 --- a/monkey/monkey_island/cc/resources/auth/password_utils.py +++ /dev/null @@ -1,12 +0,0 @@ -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 12c17d6e5da..0877ee4a311 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/monkey/monkey_island/cc/resources/auth/registration.py @@ -1,13 +1,15 @@ -import json import logging 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 +from monkey_island.cc.resources.auth.credential_utils import ( + get_secret_from_request, + get_user_credentials_from_request, +) +from monkey_island.cc.server_utils.encryption.data_store_encryptor import setup_datastore_key from monkey_island.cc.setup.mongo.database_initializer import reset_database logger = logging.getLogger(__name__) @@ -19,21 +21,13 @@ def get(self): return {"needs_registration": is_registration_needed} def post(self): - credentials = _get_user_credentials_from_request(request) + # TODO delete the old key here, before creating new one + credentials = get_user_credentials_from_request(request) try: env_singleton.env.try_add_user(credentials) + setup_datastore_key(get_secret_from_request(request)) reset_database() 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/server_utils/encryption/data_store_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/data_store_encryptor.py index df185a1c854..e5a0540806d 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,8 +1,12 @@ +from __future__ import annotations + import io import os # PyCrypto is deprecated, but we use pycryptodome, which uses the exact same imports but # is maintained. +from typing import Union + from Crypto import Random # noqa: DUO133 # nosec: B413 from monkey_island.cc.server_utils.encryption import KeyBasedEncryptor @@ -11,37 +15,42 @@ ) from monkey_island.cc.server_utils.file_utils import open_new_securely_permissioned_file -_encryptor = None +_encryptor: Union[None, DataStoreEncryptor] = None class DataStoreEncryptor: _BLOCK_SIZE = 32 _KEY_FILENAME = "mongo_key.bin" - def __init__(self, key_file_dir: str, secret: str): - key_file = os.path.join(key_file_dir, self._KEY_FILENAME) + def __init__(self, key_file_dir: str): + self.key_file_path = os.path.join(key_file_dir, self._KEY_FILENAME) + self._key_base_encryptor = None - if os.path.exists(key_file): - self._load_existing_key(key_file, secret) + def init_key(self, secret: str): + if os.path.exists(self.key_file_path): + self._load_existing_key(secret) else: - self._init_key(key_file, secret) + self._create_new_key(secret) - self._key_base_encryptor = KeyBasedEncryptor(self._cipher_key) + def _load_existing_key(self, secret: str): + with open(self.key_file_path, "rb") as f: + encrypted_key = f.read() + cipher_key = ( + PasswordBasedByteEncryptor(secret).decrypt(io.BytesIO(encrypted_key)).getvalue() + ) + self._key_base_encryptor = KeyBasedEncryptor(cipher_key) - def _init_key(self, password_file_path: str, secret: str): - self._cipher_key = Random.new().read(self._BLOCK_SIZE) + def _create_new_key(self, secret: str): + cipher_key = Random.new().read(self._BLOCK_SIZE) encrypted_key = ( - PasswordBasedByteEncryptor(secret).encrypt(io.BytesIO(self._cipher_key)).getvalue() + PasswordBasedByteEncryptor(secret).encrypt(io.BytesIO(cipher_key)).getvalue() ) - with open_new_securely_permissioned_file(password_file_path, "wb") as f: + with open_new_securely_permissioned_file(self.key_file_path, "wb") as f: f.write(encrypted_key) + self._key_base_encryptor = KeyBasedEncryptor(cipher_key) - def _load_existing_key(self, key_file_path: str, secret: str): - with open(key_file_path, "rb") as f: - encrypted_key = f.read() - self._cipher_key = ( - PasswordBasedByteEncryptor(secret).decrypt(io.BytesIO(encrypted_key)).getvalue() - ) + def is_key_setup(self) -> bool: + return self._key_base_encryptor is not None def enc(self, message: str): return self._key_base_encryptor.encrypt(message) @@ -50,10 +59,22 @@ def dec(self, enc_message: str): return self._key_base_encryptor.decrypt(enc_message) -def initialize_datastore_encryptor(key_file_dir: str, secret: str): +def initialize_datastore_encryptor(key_file_dir: str): global _encryptor - _encryptor = DataStoreEncryptor(key_file_dir, secret) + _encryptor = DataStoreEncryptor(key_file_dir) + + +class EncryptorNotInitializedError(Exception): + pass + + +def setup_datastore_key(secret: str): + if _encryptor is None: + raise EncryptorNotInitializedError + else: + if not _encryptor.is_key_setup(): + _encryptor.init_key(secret) def get_datastore_encryptor(): diff --git a/monkey/monkey_island/cc/ui/src/services/AuthService.js b/monkey/monkey_island/cc/ui/src/services/AuthService.js index 11cf3704414..7838a8563ca 100644 --- a/monkey/monkey_island/cc/ui/src/services/AuthService.js +++ b/monkey/monkey_island/cc/ui/src/services/AuthService.js @@ -46,7 +46,7 @@ export default class AuthService { return this._authFetch(this.REGISTRATION_API_ENDPOINT, { method: 'POST', body: JSON.stringify({ - 'user': username, + 'username': username, 'password': password }) }).then(res => { diff --git a/monkey/tests/unit_tests/monkey_island/cc/conftest.py b/monkey/tests/unit_tests/monkey_island/cc/conftest.py index cde82e939f3..4bd05461029 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/conftest.py +++ b/monkey/tests/unit_tests/monkey_island/cc/conftest.py @@ -11,6 +11,7 @@ ) from monkey_island.cc.server_utils.encryption import initialize_datastore_encryptor +from monkey_island.cc.server_utils.encryption.data_store_encryptor import setup_datastore_key @pytest.fixture @@ -32,4 +33,5 @@ def monkey_config_json(monkey_config): @pytest.fixture def uses_encryptor(data_for_tests_dir): - initialize_datastore_encryptor(data_for_tests_dir, ENCRYPTOR_SECRET) + initialize_datastore_encryptor(data_for_tests_dir) + setup_datastore_key(ENCRYPTOR_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 98229c6fe7c..22f2e967644 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,7 @@ get_datastore_encryptor, initialize_datastore_encryptor, ) +from monkey_island.cc.server_utils.encryption.data_store_encryptor import setup_datastore_key PLAINTEXT = "Hello, Monkey!" @@ -22,5 +23,6 @@ def test_encryption(data_for_tests_dir): def test_create_new_password_file(tmpdir): - initialize_datastore_encryptor(tmpdir, ENCRYPTOR_SECRET) + initialize_datastore_encryptor(tmpdir) + setup_datastore_key(ENCRYPTOR_SECRET) assert os.path.isfile(os.path.join(tmpdir, DataStoreEncryptor._KEY_FILENAME)) From f97ec4e9edaaa0b9e6ca17c9774e73240cdb2363 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Fri, 1 Oct 2021 11:26:43 +0300 Subject: [PATCH 04/17] Implement data store encryptor key removal on registration and unit tests for data store encryptor Data store key needs to be deleted upon registration to create a new one. --- .../cc/resources/auth/registration.py | 4 +- .../cc/server_utils/encryption/__init__.py | 3 ++ .../encryption/data_store_encryptor.py | 26 ++++++++-- .../encryption/test_data_store_encryptor.py | 47 ++++++++++++++++++- 4 files changed, 71 insertions(+), 9 deletions(-) diff --git a/monkey/monkey_island/cc/resources/auth/registration.py b/monkey/monkey_island/cc/resources/auth/registration.py index 0877ee4a311..e6743302ff2 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/monkey/monkey_island/cc/resources/auth/registration.py @@ -9,7 +9,7 @@ get_secret_from_request, get_user_credentials_from_request, ) -from monkey_island.cc.server_utils.encryption.data_store_encryptor import setup_datastore_key +from monkey_island.cc.server_utils.encryption import remove_old_datastore_key, setup_datastore_key from monkey_island.cc.setup.mongo.database_initializer import reset_database logger = logging.getLogger(__name__) @@ -21,11 +21,11 @@ def get(self): return {"needs_registration": is_registration_needed} def post(self): - # TODO delete the old key here, before creating new one credentials = get_user_credentials_from_request(request) try: env_singleton.env.try_add_user(credentials) + remove_old_datastore_key() setup_datastore_key(get_secret_from_request(request)) reset_database() return make_response({"error": ""}, 200) diff --git a/monkey/monkey_island/cc/server_utils/encryption/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/__init__.py index 84e6e625229..531659a9eef 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/__init__.py +++ b/monkey/monkey_island/cc/server_utils/encryption/__init__.py @@ -9,6 +9,9 @@ DataStoreEncryptor, get_datastore_encryptor, initialize_datastore_encryptor, + remove_old_datastore_key, + setup_datastore_key, + EncryptorNotInitializedError, ) from .dict_encryption.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 e5a0540806d..49d39b50559 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 @@ -69,12 +69,28 @@ class EncryptorNotInitializedError(Exception): pass +def encryptor_initialized_key_not_set(f): + def inner_function(*args, **kwargs): + if _encryptor is None: + raise EncryptorNotInitializedError + else: + if not _encryptor.is_key_setup(): + return f(*args, **kwargs) + else: + pass + + return inner_function + + +@encryptor_initialized_key_not_set +def remove_old_datastore_key(): + if os.path.isfile(_encryptor.key_file_path): + os.remove(_encryptor.key_file_path) + + +@encryptor_initialized_key_not_set def setup_datastore_key(secret: str): - if _encryptor is None: - raise EncryptorNotInitializedError - else: - if not _encryptor.is_key_setup(): - _encryptor.init_key(secret) + _encryptor.init_key(secret) def get_datastore_encryptor(): 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 22f2e967644..74605484159 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 @@ -5,10 +5,13 @@ from monkey_island.cc.server_utils.encryption import ( DataStoreEncryptor, + EncryptorNotInitializedError, + data_store_encryptor, get_datastore_encryptor, initialize_datastore_encryptor, + remove_old_datastore_key, + setup_datastore_key, ) -from monkey_island.cc.server_utils.encryption.data_store_encryptor import setup_datastore_key PLAINTEXT = "Hello, Monkey!" @@ -22,7 +25,47 @@ def test_encryption(data_for_tests_dir): assert decrypted_data == PLAINTEXT -def test_create_new_password_file(tmpdir): +@pytest.fixture +def initialized_key_dir(tmpdir): initialize_datastore_encryptor(tmpdir) setup_datastore_key(ENCRYPTOR_SECRET) + yield tmpdir + data_store_encryptor._encryptor = None + + +def test_key_creation(initialized_key_dir): + assert os.path.isfile(os.path.join(initialized_key_dir, DataStoreEncryptor._KEY_FILENAME)) + + +def test_key_removal_fails_if_key_initialized(initialized_key_dir): + remove_old_datastore_key() + assert os.path.isfile(os.path.join(initialized_key_dir, DataStoreEncryptor._KEY_FILENAME)) + + +def test_key_removal(initialized_key_dir, monkeypatch): + monkeypatch.setattr(DataStoreEncryptor, "is_key_setup", lambda _: False) + remove_old_datastore_key() + assert not os.path.isfile(os.path.join(initialized_key_dir, DataStoreEncryptor._KEY_FILENAME)) + + +def test_key_removal__no_key(tmpdir): + initialize_datastore_encryptor(tmpdir) + assert not os.path.isfile(os.path.join(tmpdir, DataStoreEncryptor._KEY_FILENAME)) + # Make sure no error thrown when we try to remove an non-existing key + remove_old_datastore_key() + + data_store_encryptor._encryptor = None + + +def test_encryptor_not_initialized(): + with pytest.raises(EncryptorNotInitializedError): + remove_old_datastore_key() + setup_datastore_key() + + +def test_setup_datastore_key(tmpdir): + initialize_datastore_encryptor(tmpdir) + assert not os.path.isfile(os.path.join(tmpdir, DataStoreEncryptor._KEY_FILENAME)) + setup_datastore_key(ENCRYPTOR_SECRET) assert os.path.isfile(os.path.join(tmpdir, DataStoreEncryptor._KEY_FILENAME)) + assert get_datastore_encryptor().is_key_setup() From e280c4fb5ac9fb76fee7ad7697633403fe42b15d Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Fri, 1 Oct 2021 11:58:32 +0300 Subject: [PATCH 05/17] Move data store encryptor secret generation into the data store encryptor from credential_utils.py --- monkey/monkey_island/cc/resources/auth/auth.py | 5 ++--- .../cc/resources/auth/credential_utils.py | 5 ----- .../cc/resources/auth/registration.py | 8 +++----- .../encryption/data_store_encryptor.py | 7 ++++++- monkey/tests/data_for_tests/mongo_key.bin | Bin 327 -> 327 bytes .../unit_tests/monkey_island/cc/conftest.py | 5 +++-- .../encryption/test_data_store_encryptor.py | 6 +++--- 7 files changed, 17 insertions(+), 19 deletions(-) diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 9c1c8fc6203..b6c5adb6001 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -11,7 +11,6 @@ import monkey_island.cc.resources.auth.user_store as user_store from monkey_island.cc.resources.auth.credential_utils import ( get_creds_from_request, - get_secret_from_request, password_matches_hash, ) from monkey_island.cc.server_utils.encryption.data_store_encryptor import setup_datastore_key @@ -45,14 +44,14 @@ def post(self): username, password = get_creds_from_request(request) if _credentials_match_registered_user(username, password): - setup_datastore_key(get_secret_from_request(request)) + setup_datastore_key(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 _credentials_match_registered_user(username: str, password: str): +def _credentials_match_registered_user(username: str, password: str) -> bool: user = user_store.UserStore.username_table.get(username, None) if user and password_matches_hash(password, user.secret): diff --git a/monkey/monkey_island/cc/resources/auth/credential_utils.py b/monkey/monkey_island/cc/resources/auth/credential_utils.py index 1d7a0080335..689d4cc0be3 100644 --- a/monkey/monkey_island/cc/resources/auth/credential_utils.py +++ b/monkey/monkey_island/cc/resources/auth/credential_utils.py @@ -25,11 +25,6 @@ def get_user_credentials_from_request(_request) -> UserCreds: return UserCreds(username, password_hash) -def get_secret_from_request(_request) -> str: - username, password = get_creds_from_request(_request) - return f"{username}:{password}" - - def get_creds_from_request(_request: Request) -> Tuple[str, str]: cred_dict = json.loads(request.data) username = cred_dict.get("username", "") diff --git a/monkey/monkey_island/cc/resources/auth/registration.py b/monkey/monkey_island/cc/resources/auth/registration.py index e6743302ff2..f96a5ce82f6 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/monkey/monkey_island/cc/resources/auth/registration.py @@ -5,10 +5,7 @@ import monkey_island.cc.environment.environment_singleton as env_singleton from common.utils.exceptions import InvalidRegistrationCredentialsError, RegistrationNotNeededError -from monkey_island.cc.resources.auth.credential_utils import ( - get_secret_from_request, - get_user_credentials_from_request, -) +from monkey_island.cc.resources.auth.credential_utils import get_user_credentials_from_request from monkey_island.cc.server_utils.encryption import remove_old_datastore_key, setup_datastore_key from monkey_island.cc.setup.mongo.database_initializer import reset_database @@ -26,7 +23,8 @@ def post(self): try: env_singleton.env.try_add_user(credentials) remove_old_datastore_key() - setup_datastore_key(get_secret_from_request(request)) + username, password = get_user_credentials_from_request(request) + setup_datastore_key(username, password) reset_database() return make_response({"error": ""}, 200) except (InvalidRegistrationCredentialsError, RegistrationNotNeededError) as e: 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 49d39b50559..c08a9cb70a7 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 @@ -69,6 +69,10 @@ class EncryptorNotInitializedError(Exception): pass +def _get_secret_from_credentials(username: str, password: str) -> str: + return f"{username}:{password}" + + def encryptor_initialized_key_not_set(f): def inner_function(*args, **kwargs): if _encryptor is None: @@ -89,7 +93,8 @@ def remove_old_datastore_key(): @encryptor_initialized_key_not_set -def setup_datastore_key(secret: str): +def setup_datastore_key(username: str, password: str): + secret = _get_secret_from_credentials(username, password) _encryptor.init_key(secret) diff --git a/monkey/tests/data_for_tests/mongo_key.bin b/monkey/tests/data_for_tests/mongo_key.bin index edf082ae1b34cf2b5b99b1d7598909643aff89b3..7b49bd4dc6041141ac5c721db81475e277140970 100644 GIT binary patch delta 169 zcmV;a09OCU0>=W7rhj_z3zwx%98Yw$0#})@!*u_QNs9mqw$^9jn$Tckq ztfRp{Yv6jI^5w8(dJV53(>a=1E-`lTHRr7)uC}uLuyc|IIwC0m(UvV?$$&!CrH5@Q XlQyZyZ@{{c13p6K)|Pe{q542v#)DK8 delta 169 zcmV;a09OCU0>=W7rhhw#SABZSC@)sqvrJ zje?G4Q0WP$+75+%AT6gOX8tQ~(2cyz#_|F^kqWw85Ffrr>?7g`4>#Tle4q7M<^3z2 zaR7pM Date: Fri, 1 Oct 2021 12:34:21 +0300 Subject: [PATCH 06/17] Fix typos and rename files/classes related to data store encryptor. Change PasswordBasedBytesEncryptor interface to use bytes instead of io.BytesIO --- .../monkey_island/cc/resources/auth/auth.py | 4 +-- .../cc/resources/auth/credential_utils.py | 4 +-- .../cc/server_utils/encryption/__init__.py | 4 +-- .../encryption/data_store_encryptor.py | 25 ++++++++----------- ....py => password_based_bytes_encryption.py} | 19 +++++++------- ...py => password_based_string_encryptior.py} | 15 +++++------ 6 files changed, 32 insertions(+), 39 deletions(-) rename monkey/monkey_island/cc/server_utils/encryption/{password_based_byte_encryption.py => password_based_bytes_encryption.py} (77%) rename monkey/monkey_island/cc/server_utils/encryption/{password_based_string_encryption.py => password_based_string_encryptior.py} (58%) diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index b6c5adb6001..06fcb9e0703 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -10,7 +10,7 @@ import monkey_island.cc.environment.environment_singleton as env_singleton import monkey_island.cc.resources.auth.user_store as user_store from monkey_island.cc.resources.auth.credential_utils import ( - get_creds_from_request, + get_credentials_from_request, password_matches_hash, ) from monkey_island.cc.server_utils.encryption.data_store_encryptor import setup_datastore_key @@ -41,7 +41,7 @@ def post(self): "password": "my_password" } """ - username, password = get_creds_from_request(request) + username, password = get_credentials_from_request(request) if _credentials_match_registered_user(username, password): setup_datastore_key(username, password) diff --git a/monkey/monkey_island/cc/resources/auth/credential_utils.py b/monkey/monkey_island/cc/resources/auth/credential_utils.py index 689d4cc0be3..874906edb0e 100644 --- a/monkey/monkey_island/cc/resources/auth/credential_utils.py +++ b/monkey/monkey_island/cc/resources/auth/credential_utils.py @@ -19,13 +19,13 @@ def password_matches_hash(plaintext_password, password_hash): def get_user_credentials_from_request(_request) -> UserCreds: - username, password = get_creds_from_request(_request) + username, password = get_credentials_from_request(_request) password_hash = hash_password(password) return UserCreds(username, password_hash) -def get_creds_from_request(_request: Request) -> Tuple[str, str]: +def get_credentials_from_request(_request: Request) -> Tuple[str, str]: cred_dict = json.loads(request.data) username = cred_dict.get("username", "") password = cred_dict.get("password", "") diff --git a/monkey/monkey_island/cc/server_utils/encryption/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/__init__.py index 531659a9eef..e9c19d75e10 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/__init__.py +++ b/monkey/monkey_island/cc/server_utils/encryption/__init__.py @@ -1,10 +1,10 @@ from monkey_island.cc.server_utils.encryption.i_encryptor import IEncryptor from monkey_island.cc.server_utils.encryption.key_based_encryptor import KeyBasedEncryptor -from monkey_island.cc.server_utils.encryption.password_based_string_encryption import ( +from monkey_island.cc.server_utils.encryption.password_based_string_encryptior import ( PasswordBasedStringEncryptor, is_encrypted, ) -from .password_based_byte_encryption import InvalidCredentialsError, InvalidCiphertextError +from .password_based_bytes_encryption import InvalidCredentialsError, InvalidCiphertextError from monkey_island.cc.server_utils.encryption.data_store_encryptor import ( DataStoreEncryptor, get_datastore_encryptor, 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 c08a9cb70a7..624e663b436 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,6 +1,5 @@ from __future__ import annotations -import io import os # PyCrypto is deprecated, but we use pycryptodome, which uses the exact same imports but @@ -10,8 +9,8 @@ from Crypto import Random # noqa: DUO133 # nosec: B413 from monkey_island.cc.server_utils.encryption import KeyBasedEncryptor -from monkey_island.cc.server_utils.encryption.password_based_byte_encryption import ( - PasswordBasedByteEncryptor, +from monkey_island.cc.server_utils.encryption.password_based_bytes_encryption import ( + PasswordBasedBytesEncryptor, ) from monkey_island.cc.server_utils.file_utils import open_new_securely_permissioned_file @@ -24,7 +23,7 @@ class DataStoreEncryptor: def __init__(self, key_file_dir: str): self.key_file_path = os.path.join(key_file_dir, self._KEY_FILENAME) - self._key_base_encryptor = None + self._key_based_encryptor = None def init_key(self, secret: str): if os.path.exists(self.key_file_path): @@ -35,28 +34,24 @@ def init_key(self, secret: str): def _load_existing_key(self, secret: str): with open(self.key_file_path, "rb") as f: encrypted_key = f.read() - cipher_key = ( - PasswordBasedByteEncryptor(secret).decrypt(io.BytesIO(encrypted_key)).getvalue() - ) - self._key_base_encryptor = KeyBasedEncryptor(cipher_key) + cipher_key = PasswordBasedBytesEncryptor(secret).decrypt(encrypted_key) + self._key_based_encryptor = KeyBasedEncryptor(cipher_key) def _create_new_key(self, secret: str): cipher_key = Random.new().read(self._BLOCK_SIZE) - encrypted_key = ( - PasswordBasedByteEncryptor(secret).encrypt(io.BytesIO(cipher_key)).getvalue() - ) + encrypted_key = PasswordBasedBytesEncryptor(secret).encrypt(cipher_key) with open_new_securely_permissioned_file(self.key_file_path, "wb") as f: f.write(encrypted_key) - self._key_base_encryptor = KeyBasedEncryptor(cipher_key) + self._key_based_encryptor = KeyBasedEncryptor(cipher_key) def is_key_setup(self) -> bool: - return self._key_base_encryptor is not None + return self._key_based_encryptor is not None def enc(self, message: str): - return self._key_base_encryptor.encrypt(message) + return self._key_based_encryptor.encrypt(message) def dec(self, enc_message: str): - return self._key_base_encryptor.decrypt(enc_message) + return self._key_based_encryptor.decrypt(enc_message) def initialize_datastore_encryptor(key_file_dir: str): diff --git a/monkey/monkey_island/cc/server_utils/encryption/password_based_byte_encryption.py b/monkey/monkey_island/cc/server_utils/encryption/password_based_bytes_encryption.py similarity index 77% rename from monkey/monkey_island/cc/server_utils/encryption/password_based_byte_encryption.py rename to monkey/monkey_island/cc/server_utils/encryption/password_based_bytes_encryption.py index 569dc0d12de..2e7c05819a4 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/password_based_byte_encryption.py +++ b/monkey/monkey_island/cc/server_utils/encryption/password_based_bytes_encryption.py @@ -1,6 +1,5 @@ import io import logging -from io import BytesIO import pyAesCrypt @@ -17,28 +16,30 @@ # Note: password != key -class PasswordBasedByteEncryptor(IEncryptor): +class PasswordBasedBytesEncryptor(IEncryptor): _BUFFER_SIZE = pyAesCrypt.crypto.bufferSizeDef def __init__(self, password: str): self.password = password - def encrypt(self, plaintext: BytesIO) -> BytesIO: + def encrypt(self, plaintext: bytes) -> bytes: ciphertext_stream = io.BytesIO() - pyAesCrypt.encryptStream(plaintext, ciphertext_stream, self.password, self._BUFFER_SIZE) + pyAesCrypt.encryptStream( + io.BytesIO(plaintext), ciphertext_stream, self.password, self._BUFFER_SIZE + ) - return ciphertext_stream + return ciphertext_stream.getvalue() - def decrypt(self, ciphertext: BytesIO) -> BytesIO: + def decrypt(self, ciphertext: bytes) -> bytes: plaintext_stream = io.BytesIO() - ciphertext_stream_len = len(ciphertext.getvalue()) + ciphertext_stream_len = len(ciphertext) try: pyAesCrypt.decryptStream( - ciphertext, + io.BytesIO(ciphertext), plaintext_stream, self.password, self._BUFFER_SIZE, @@ -51,7 +52,7 @@ def decrypt(self, ciphertext: BytesIO) -> BytesIO: else: logger.info("The corrupt ciphertext provided.") raise InvalidCiphertextError - return plaintext_stream + return plaintext_stream.getvalue() class InvalidCredentialsError(Exception): diff --git a/monkey/monkey_island/cc/server_utils/encryption/password_based_string_encryption.py b/monkey/monkey_island/cc/server_utils/encryption/password_based_string_encryptior.py similarity index 58% rename from monkey/monkey_island/cc/server_utils/encryption/password_based_string_encryption.py rename to monkey/monkey_island/cc/server_utils/encryption/password_based_string_encryptior.py index ea796a441ea..716455a5bc3 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/password_based_string_encryption.py +++ b/monkey/monkey_island/cc/server_utils/encryption/password_based_string_encryptior.py @@ -1,12 +1,11 @@ import base64 -import io import logging import pyAesCrypt from monkey_island.cc.server_utils.encryption import IEncryptor -from monkey_island.cc.server_utils.encryption.password_based_byte_encryption import ( - PasswordBasedByteEncryptor, +from monkey_island.cc.server_utils.encryption.password_based_bytes_encryption import ( + PasswordBasedBytesEncryptor, ) logger = logging.getLogger(__name__) @@ -20,17 +19,15 @@ def __init__(self, password: str): self.password = password def encrypt(self, plaintext: str) -> str: - plaintext_stream = io.BytesIO(plaintext.encode()) - ciphertext = PasswordBasedByteEncryptor(self.password).encrypt(plaintext_stream) + ciphertext = PasswordBasedBytesEncryptor(self.password).encrypt(plaintext.encode()) - return base64.b64encode(ciphertext.getvalue()).decode() + return base64.b64encode(ciphertext).decode() def decrypt(self, ciphertext: str) -> str: ciphertext = base64.b64decode(ciphertext) - ciphertext_stream = io.BytesIO(ciphertext) - plaintext_stream = PasswordBasedByteEncryptor(self.password).decrypt(ciphertext_stream) - return plaintext_stream.getvalue().decode("utf-8") + plaintext_stream = PasswordBasedBytesEncryptor(self.password).decrypt(ciphertext) + return plaintext_stream.decode() def is_encrypted(ciphertext: str) -> bool: From ddae09278e1e990ad62c052c27c1d17849da4d09 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Fri, 1 Oct 2021 12:44:05 +0300 Subject: [PATCH 07/17] Refactor test_data_store_encryptor.py to use (path / to / file).isfile() syntax to check for presence of files --- .../encryption/test_data_store_encryptor.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 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 ef4ee20d044..1a394c02590 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,5 +1,3 @@ -import os - import pytest from tests.unit_tests.monkey_island.cc.conftest import MOCK_PASSWORD, MOCK_USERNAME @@ -34,23 +32,23 @@ def initialized_key_dir(tmpdir): def test_key_creation(initialized_key_dir): - assert os.path.isfile(os.path.join(initialized_key_dir, DataStoreEncryptor._KEY_FILENAME)) + assert (initialized_key_dir / DataStoreEncryptor._KEY_FILENAME).isfile() def test_key_removal_fails_if_key_initialized(initialized_key_dir): remove_old_datastore_key() - assert os.path.isfile(os.path.join(initialized_key_dir, DataStoreEncryptor._KEY_FILENAME)) + assert (initialized_key_dir / DataStoreEncryptor._KEY_FILENAME).isfile() def test_key_removal(initialized_key_dir, monkeypatch): monkeypatch.setattr(DataStoreEncryptor, "is_key_setup", lambda _: False) remove_old_datastore_key() - assert not os.path.isfile(os.path.join(initialized_key_dir, DataStoreEncryptor._KEY_FILENAME)) + assert not (initialized_key_dir / DataStoreEncryptor._KEY_FILENAME).isfile() def test_key_removal__no_key(tmpdir): initialize_datastore_encryptor(tmpdir) - assert not os.path.isfile(os.path.join(tmpdir, DataStoreEncryptor._KEY_FILENAME)) + assert not (tmpdir / DataStoreEncryptor._KEY_FILENAME).isfile() # Make sure no error thrown when we try to remove an non-existing key remove_old_datastore_key() @@ -65,7 +63,7 @@ def test_encryptor_not_initialized(): def test_setup_datastore_key(tmpdir): initialize_datastore_encryptor(tmpdir) - assert not os.path.isfile(os.path.join(tmpdir, DataStoreEncryptor._KEY_FILENAME)) + assert not (tmpdir / DataStoreEncryptor._KEY_FILENAME).isfile() setup_datastore_key(MOCK_USERNAME, MOCK_PASSWORD) - assert os.path.isfile(os.path.join(tmpdir, DataStoreEncryptor._KEY_FILENAME)) + assert (tmpdir / DataStoreEncryptor._KEY_FILENAME).isfile() assert get_datastore_encryptor().is_key_setup() From b2bbb62bdd5481b603e387b2865a1f1a890dd3d5 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Fri, 1 Oct 2021 12:48:08 +0300 Subject: [PATCH 08/17] Add CHANGELOG.md entry for #1463 (Encrypt the database key with user's credentials.) --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69408a7fcbc..0aec3d27700 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ Changelog](https://keepachangelog.com/en/1.0.0/). - Generate a random password when creating a new user for CommunicateAsNewUser PBA. #1434 - Credentials gathered from victim machines are no longer stored plaintext in the database. #1454 +- Encrypt the database key with user's credentials. #1463 ## [1.11.0] - 2021-08-13 From da169dddc976574f59572cd9d47805480dbfb93a Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Fri, 1 Oct 2021 15:24:48 +0300 Subject: [PATCH 09/17] Refactor DataStoreEncryptor by splitting up initialization related methods into EncryptorFactory This makes encryptor initialization workflow more straight-forward and the files become smaller, easier to read --- .../monkey_island/cc/resources/auth/auth.py | 8 +- .../cc/resources/auth/registration.py | 7 +- monkey/monkey_island/cc/server_setup.py | 5 +- .../cc/server_utils/encryption/__init__.py | 14 ++-- .../encryption/data_store_encryptor.py | 79 ++++++------------- .../encryption/encryptor_factory.py | 38 +++++++++ .../unit_tests/monkey_island/cc/conftest.py | 10 ++- .../encryption/test_data_store_encryptor.py | 44 +++++------ 8 files changed, 108 insertions(+), 97 deletions(-) create mode 100644 monkey/monkey_island/cc/server_utils/encryption/encryptor_factory.py diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 06fcb9e0703..124cd1f71a0 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -13,7 +13,10 @@ get_credentials_from_request, password_matches_hash, ) -from monkey_island.cc.server_utils.encryption.data_store_encryptor import setup_datastore_key +from monkey_island.cc.server_utils.encryption import ( + get_datastore_encryptor, + initialize_datastore_encryptor, +) logger = logging.getLogger(__name__) @@ -44,7 +47,8 @@ def post(self): username, password = get_credentials_from_request(request) if _credentials_match_registered_user(username, password): - setup_datastore_key(username, password) + if not get_datastore_encryptor(): + initialize_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/resources/auth/registration.py b/monkey/monkey_island/cc/resources/auth/registration.py index f96a5ce82f6..064e80179d1 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/monkey/monkey_island/cc/resources/auth/registration.py @@ -6,7 +6,10 @@ import monkey_island.cc.environment.environment_singleton as env_singleton from common.utils.exceptions import InvalidRegistrationCredentialsError, RegistrationNotNeededError from monkey_island.cc.resources.auth.credential_utils import get_user_credentials_from_request -from monkey_island.cc.server_utils.encryption import remove_old_datastore_key, setup_datastore_key +from monkey_island.cc.server_utils.encryption import ( + initialize_datastore_encryptor, + remove_old_datastore_key, +) from monkey_island.cc.setup.mongo.database_initializer import reset_database logger = logging.getLogger(__name__) @@ -24,7 +27,7 @@ def post(self): env_singleton.env.try_add_user(credentials) remove_old_datastore_key() username, password = get_user_credentials_from_request(request) - setup_datastore_key(username, password) + initialize_datastore_encryptor(username, password) reset_database() return make_response({"error": ""}, 200) except (InvalidRegistrationCredentialsError, RegistrationNotNeededError) as e: diff --git a/monkey/monkey_island/cc/server_setup.py b/monkey/monkey_island/cc/server_setup.py index 69ab0437a60..82c1b3a580c 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -11,6 +11,8 @@ # Add the monkey_island directory to the path, to make sure imports that don't start with # "monkey_island." work. +from monkey_island.cc.server_utils.encryption import initialize_encryptor_factory + MONKEY_ISLAND_DIR_BASE_PATH = str(Path(__file__).parent.parent) if str(MONKEY_ISLAND_DIR_BASE_PATH) not in sys.path: sys.path.insert(0, MONKEY_ISLAND_DIR_BASE_PATH) @@ -27,7 +29,6 @@ GEVENT_EXCEPTION_LOG, MONGO_CONNECTION_TIMEOUT, ) -from monkey_island.cc.server_utils.encryption import initialize_datastore_encryptor # noqa: E402 from monkey_island.cc.server_utils.island_logger import reset_logger, setup_logging # noqa: E402 from monkey_island.cc.services.initialize import initialize_services # noqa: E402 from monkey_island.cc.services.reporting.exporter_init import populate_exporter_list # noqa: E402 @@ -87,7 +88,7 @@ def _configure_logging(config_options): def _initialize_globals(config_options: IslandConfigOptions, server_config_path: str): env_singleton.initialize_from_file(server_config_path) - initialize_datastore_encryptor(config_options.data_dir) + initialize_encryptor_factory(config_options.data_dir) initialize_services(config_options.data_dir) diff --git a/monkey/monkey_island/cc/server_utils/encryption/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/__init__.py index e9c19d75e10..fa9692ca3fb 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/__init__.py +++ b/monkey/monkey_island/cc/server_utils/encryption/__init__.py @@ -4,15 +4,15 @@ PasswordBasedStringEncryptor, is_encrypted, ) -from .password_based_bytes_encryption import InvalidCredentialsError, InvalidCiphertextError -from monkey_island.cc.server_utils.encryption.data_store_encryptor import ( - DataStoreEncryptor, - get_datastore_encryptor, - initialize_datastore_encryptor, +from .encryptor_factory import ( + FactoryNotInitializedError, remove_old_datastore_key, - setup_datastore_key, - EncryptorNotInitializedError, + get_encryptor_factory, + get_secret_from_credentials, + initialize_encryptor_factory, ) +from .data_store_encryptor import initialize_datastore_encryptor, get_datastore_encryptor +from .password_based_bytes_encryption import InvalidCredentialsError, InvalidCiphertextError from .dict_encryption.dict_encryptor import ( SensitiveField, encrypt_dict, 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 624e663b436..f584017837c 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 @@ -8,7 +8,11 @@ from Crypto import Random # noqa: DUO133 # nosec: B413 -from monkey_island.cc.server_utils.encryption import KeyBasedEncryptor +from monkey_island.cc.server_utils.encryption import FactoryNotInitializedError, KeyBasedEncryptor +from monkey_island.cc.server_utils.encryption.encryptor_factory import ( + get_encryptor_factory, + get_secret_from_credentials, +) from monkey_island.cc.server_utils.encryption.password_based_bytes_encryption import ( PasswordBasedBytesEncryptor, ) @@ -19,33 +23,27 @@ class DataStoreEncryptor: _BLOCK_SIZE = 32 - _KEY_FILENAME = "mongo_key.bin" - - def __init__(self, key_file_dir: str): - self.key_file_path = os.path.join(key_file_dir, self._KEY_FILENAME) - self._key_based_encryptor = None - def init_key(self, secret: str): - if os.path.exists(self.key_file_path): - self._load_existing_key(secret) + def __init__(self, key_file_path: str, secret: str): + if os.path.exists(key_file_path): + self._key_based_encryptor = DataStoreEncryptor._load_existing_key(key_file_path, secret) else: - self._create_new_key(secret) + self._key_based_encryptor = DataStoreEncryptor._create_new_key(key_file_path, secret) - def _load_existing_key(self, secret: str): - with open(self.key_file_path, "rb") as f: + @staticmethod + def _load_existing_key(key_file_path: str, secret: str): + with open(key_file_path, "rb") as f: encrypted_key = f.read() cipher_key = PasswordBasedBytesEncryptor(secret).decrypt(encrypted_key) - self._key_based_encryptor = KeyBasedEncryptor(cipher_key) + return KeyBasedEncryptor(cipher_key) - def _create_new_key(self, secret: str): - cipher_key = Random.new().read(self._BLOCK_SIZE) + @staticmethod + def _create_new_key(key_file_path: str, secret: str): + cipher_key = Random.new().read(DataStoreEncryptor._BLOCK_SIZE) encrypted_key = PasswordBasedBytesEncryptor(secret).encrypt(cipher_key) - with open_new_securely_permissioned_file(self.key_file_path, "wb") as f: + with open_new_securely_permissioned_file(key_file_path, "wb") as f: f.write(encrypted_key) - self._key_based_encryptor = KeyBasedEncryptor(cipher_key) - - def is_key_setup(self) -> bool: - return self._key_based_encryptor is not None + return KeyBasedEncryptor(cipher_key) def enc(self, message: str): return self._key_based_encryptor.encrypt(message) @@ -54,43 +52,14 @@ def dec(self, enc_message: str): return self._key_based_encryptor.decrypt(enc_message) -def initialize_datastore_encryptor(key_file_dir: str): +def initialize_datastore_encryptor(username: str, password: str): global _encryptor - _encryptor = DataStoreEncryptor(key_file_dir) - - -class EncryptorNotInitializedError(Exception): - pass - - -def _get_secret_from_credentials(username: str, password: str) -> str: - return f"{username}:{password}" - - -def encryptor_initialized_key_not_set(f): - def inner_function(*args, **kwargs): - if _encryptor is None: - raise EncryptorNotInitializedError - else: - if not _encryptor.is_key_setup(): - return f(*args, **kwargs) - else: - pass - - return inner_function - - -@encryptor_initialized_key_not_set -def remove_old_datastore_key(): - if os.path.isfile(_encryptor.key_file_path): - os.remove(_encryptor.key_file_path) - - -@encryptor_initialized_key_not_set -def setup_datastore_key(username: str, password: str): - secret = _get_secret_from_credentials(username, password) - _encryptor.init_key(secret) + factory = get_encryptor_factory() + if not factory: + raise FactoryNotInitializedError + secret = get_secret_from_credentials(username, password) + _encryptor = DataStoreEncryptor(factory.key_file_path, secret) def get_datastore_encryptor(): diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptor_factory.py b/monkey/monkey_island/cc/server_utils/encryption/encryptor_factory.py new file mode 100644 index 00000000000..67d3e6ee4f5 --- /dev/null +++ b/monkey/monkey_island/cc/server_utils/encryption/encryptor_factory.py @@ -0,0 +1,38 @@ +from __future__ import annotations + +import os +from ctypes import Union + +_factory: Union[None, EncryptorFactory] = None + + +class EncryptorFactory: + + _KEY_FILENAME = "mongo_key.bin" + + def __init__(self, key_file_dir: str): + self.key_file_path = os.path.join(key_file_dir, self._KEY_FILENAME) + + +class FactoryNotInitializedError(Exception): + pass + + +def get_secret_from_credentials(username: str, password: str) -> str: + return f"{username}:{password}" + + +def remove_old_datastore_key(): + if _factory is None: + raise FactoryNotInitializedError + if os.path.isfile(_factory.key_file_path): + os.remove(_factory.key_file_path) + + +def initialize_encryptor_factory(key_file_dir: str): + global _factory + _factory = EncryptorFactory(key_file_dir) + + +def get_encryptor_factory(): + return _factory diff --git a/monkey/tests/unit_tests/monkey_island/cc/conftest.py b/monkey/tests/unit_tests/monkey_island/cc/conftest.py index ee3e8aafac7..cbd141ac7a2 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/conftest.py +++ b/monkey/tests/unit_tests/monkey_island/cc/conftest.py @@ -10,8 +10,10 @@ STANDARD_PLAINTEXT_MONKEY_CONFIG_FILENAME, ) -from monkey_island.cc.server_utils.encryption import initialize_datastore_encryptor -from monkey_island.cc.server_utils.encryption.data_store_encryptor import setup_datastore_key +from monkey_island.cc.server_utils.encryption import ( + initialize_datastore_encryptor, + initialize_encryptor_factory, +) @pytest.fixture @@ -34,5 +36,5 @@ def monkey_config_json(monkey_config): @pytest.fixture def uses_encryptor(data_for_tests_dir): - initialize_datastore_encryptor(data_for_tests_dir) - setup_datastore_key(MOCK_USERNAME, MOCK_PASSWORD) + initialize_encryptor_factory(data_for_tests_dir) + initialize_datastore_encryptor(MOCK_USERNAME, MOCK_PASSWORD) 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 1a394c02590..e8901862b2d 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 @@ -2,14 +2,15 @@ from tests.unit_tests.monkey_island.cc.conftest import MOCK_PASSWORD, MOCK_USERNAME from monkey_island.cc.server_utils.encryption import ( - DataStoreEncryptor, - EncryptorNotInitializedError, + FactoryNotInitializedError, data_store_encryptor, + encryptor_factory, get_datastore_encryptor, initialize_datastore_encryptor, + initialize_encryptor_factory, remove_old_datastore_key, - setup_datastore_key, ) +from monkey_island.cc.server_utils.encryption.encryptor_factory import EncryptorFactory PLAINTEXT = "Hello, Monkey!" @@ -25,45 +26,38 @@ def test_encryption(data_for_tests_dir): @pytest.fixture def initialized_key_dir(tmpdir): - initialize_datastore_encryptor(tmpdir) - setup_datastore_key(MOCK_USERNAME, MOCK_PASSWORD) + initialize_encryptor_factory(tmpdir) + initialize_datastore_encryptor(MOCK_USERNAME, MOCK_PASSWORD) yield tmpdir data_store_encryptor._encryptor = None + encryptor_factory._factory = None def test_key_creation(initialized_key_dir): - assert (initialized_key_dir / DataStoreEncryptor._KEY_FILENAME).isfile() - - -def test_key_removal_fails_if_key_initialized(initialized_key_dir): - remove_old_datastore_key() - assert (initialized_key_dir / DataStoreEncryptor._KEY_FILENAME).isfile() + assert (initialized_key_dir / EncryptorFactory._KEY_FILENAME).isfile() def test_key_removal(initialized_key_dir, monkeypatch): - monkeypatch.setattr(DataStoreEncryptor, "is_key_setup", lambda _: False) remove_old_datastore_key() - assert not (initialized_key_dir / DataStoreEncryptor._KEY_FILENAME).isfile() + assert not (initialized_key_dir / EncryptorFactory._KEY_FILENAME).isfile() def test_key_removal__no_key(tmpdir): - initialize_datastore_encryptor(tmpdir) - assert not (tmpdir / DataStoreEncryptor._KEY_FILENAME).isfile() + initialize_encryptor_factory(tmpdir) + assert not (tmpdir / EncryptorFactory._KEY_FILENAME).isfile() # Make sure no error thrown when we try to remove an non-existing key remove_old_datastore_key() - - data_store_encryptor._encryptor = None + encryptor_factory._factory = None def test_encryptor_not_initialized(): - with pytest.raises(EncryptorNotInitializedError): + with pytest.raises(FactoryNotInitializedError): remove_old_datastore_key() - setup_datastore_key() + initialize_datastore_encryptor(MOCK_USERNAME, MOCK_PASSWORD) -def test_setup_datastore_key(tmpdir): - initialize_datastore_encryptor(tmpdir) - assert not (tmpdir / DataStoreEncryptor._KEY_FILENAME).isfile() - setup_datastore_key(MOCK_USERNAME, MOCK_PASSWORD) - assert (tmpdir / DataStoreEncryptor._KEY_FILENAME).isfile() - assert get_datastore_encryptor().is_key_setup() +def test_initialize_encryptor(tmpdir): + initialize_encryptor_factory(tmpdir) + assert not (tmpdir / EncryptorFactory._KEY_FILENAME).isfile() + initialize_datastore_encryptor(MOCK_USERNAME, MOCK_PASSWORD) + assert (tmpdir / EncryptorFactory._KEY_FILENAME).isfile() From 26ba02a1d093ef465cd6be80a7c9e62a6fd2fad8 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Fri, 1 Oct 2021 15:33:46 +0300 Subject: [PATCH 10/17] Refactor get_credentials_from_request to get_username_password_from_request This better indicates that get_username_password_from_request returns a username/password pair rather than UserCreds structure --- monkey/monkey_island/cc/resources/auth/auth.py | 4 ++-- monkey/monkey_island/cc/resources/auth/credential_utils.py | 4 ++-- monkey/monkey_island/cc/resources/auth/registration.py | 7 +++++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 124cd1f71a0..9a693e80d8f 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -10,7 +10,7 @@ import monkey_island.cc.environment.environment_singleton as env_singleton import monkey_island.cc.resources.auth.user_store as user_store from monkey_island.cc.resources.auth.credential_utils import ( - get_credentials_from_request, + get_username_password_from_request, password_matches_hash, ) from monkey_island.cc.server_utils.encryption import ( @@ -44,7 +44,7 @@ def post(self): "password": "my_password" } """ - username, password = get_credentials_from_request(request) + username, password = get_username_password_from_request(request) if _credentials_match_registered_user(username, password): if not get_datastore_encryptor(): diff --git a/monkey/monkey_island/cc/resources/auth/credential_utils.py b/monkey/monkey_island/cc/resources/auth/credential_utils.py index 874906edb0e..a0823d42b93 100644 --- a/monkey/monkey_island/cc/resources/auth/credential_utils.py +++ b/monkey/monkey_island/cc/resources/auth/credential_utils.py @@ -19,13 +19,13 @@ def password_matches_hash(plaintext_password, password_hash): def get_user_credentials_from_request(_request) -> UserCreds: - username, password = get_credentials_from_request(_request) + username, password = get_username_password_from_request(_request) password_hash = hash_password(password) return UserCreds(username, password_hash) -def get_credentials_from_request(_request: Request) -> Tuple[str, str]: +def get_username_password_from_request(_request: Request) -> Tuple[str, str]: cred_dict = json.loads(request.data) username = cred_dict.get("username", "") password = cred_dict.get("password", "") diff --git a/monkey/monkey_island/cc/resources/auth/registration.py b/monkey/monkey_island/cc/resources/auth/registration.py index 064e80179d1..82dbcfe3a1d 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/monkey/monkey_island/cc/resources/auth/registration.py @@ -5,7 +5,10 @@ import monkey_island.cc.environment.environment_singleton as env_singleton from common.utils.exceptions import InvalidRegistrationCredentialsError, RegistrationNotNeededError -from monkey_island.cc.resources.auth.credential_utils import get_user_credentials_from_request +from monkey_island.cc.resources.auth.credential_utils import ( + get_user_credentials_from_request, + get_username_password_from_request, +) from monkey_island.cc.server_utils.encryption import ( initialize_datastore_encryptor, remove_old_datastore_key, @@ -26,7 +29,7 @@ def post(self): try: env_singleton.env.try_add_user(credentials) remove_old_datastore_key() - username, password = get_user_credentials_from_request(request) + username, password = get_username_password_from_request(request) initialize_datastore_encryptor(username, password) reset_database() return make_response({"error": ""}, 200) From 9d6dc3b0266fae42fd79c6f4704bf49da926d679 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Fri, 1 Oct 2021 17:33:55 +0300 Subject: [PATCH 11/17] Move all encryptor building related code to encryptor_factory.py from data_store_encryptor.py --- .../cc/server_utils/encryption/__init__.py | 2 +- .../encryption/data_store_encryptor.py | 46 ++----------- .../encryption/encryptor_factory.py | 69 ++++++++++++++----- .../encryption/test_data_store_encryptor.py | 7 +- 4 files changed, 65 insertions(+), 59 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/encryption/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/__init__.py index fa9692ca3fb..1c3e422dbd9 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/__init__.py +++ b/monkey/monkey_island/cc/server_utils/encryption/__init__.py @@ -8,7 +8,7 @@ FactoryNotInitializedError, remove_old_datastore_key, get_encryptor_factory, - get_secret_from_credentials, + _get_secret_from_credentials, initialize_encryptor_factory, ) from .data_store_encryptor import initialize_datastore_encryptor, get_datastore_encryptor 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 f584017837c..949102c8479 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,49 +1,17 @@ from __future__ import annotations -import os - # PyCrypto is deprecated, but we use pycryptodome, which uses the exact same imports but # is maintained. from typing import Union -from Crypto import Random # noqa: DUO133 # nosec: B413 - -from monkey_island.cc.server_utils.encryption import FactoryNotInitializedError, KeyBasedEncryptor -from monkey_island.cc.server_utils.encryption.encryptor_factory import ( - get_encryptor_factory, - get_secret_from_credentials, -) -from monkey_island.cc.server_utils.encryption.password_based_bytes_encryption import ( - PasswordBasedBytesEncryptor, -) -from monkey_island.cc.server_utils.file_utils import open_new_securely_permissioned_file +from monkey_island.cc.server_utils.encryption import KeyBasedEncryptor _encryptor: Union[None, DataStoreEncryptor] = None class DataStoreEncryptor: - _BLOCK_SIZE = 32 - - def __init__(self, key_file_path: str, secret: str): - if os.path.exists(key_file_path): - self._key_based_encryptor = DataStoreEncryptor._load_existing_key(key_file_path, secret) - else: - self._key_based_encryptor = DataStoreEncryptor._create_new_key(key_file_path, secret) - - @staticmethod - def _load_existing_key(key_file_path: str, secret: str): - with open(key_file_path, "rb") as f: - encrypted_key = f.read() - cipher_key = PasswordBasedBytesEncryptor(secret).decrypt(encrypted_key) - return KeyBasedEncryptor(cipher_key) - - @staticmethod - def _create_new_key(key_file_path: str, secret: str): - cipher_key = Random.new().read(DataStoreEncryptor._BLOCK_SIZE) - 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 __init__(self, key_based_encryptor: KeyBasedEncryptor): + self._key_based_encryptor = key_based_encryptor def enc(self, message: str): return self._key_based_encryptor.encrypt(message) @@ -52,14 +20,10 @@ def dec(self, enc_message: str): return self._key_based_encryptor.decrypt(enc_message) -def initialize_datastore_encryptor(username: str, password: str): +def initialize_datastore_encryptor(key_based_encryptor: KeyBasedEncryptor): global _encryptor - factory = get_encryptor_factory() - if not factory: - raise FactoryNotInitializedError - secret = get_secret_from_credentials(username, password) - _encryptor = DataStoreEncryptor(factory.key_file_path, secret) + _encryptor = DataStoreEncryptor(key_based_encryptor) def get_datastore_encryptor(): diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptor_factory.py b/monkey/monkey_island/cc/server_utils/encryption/encryptor_factory.py index 67d3e6ee4f5..0ae3e70a65a 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/encryptor_factory.py +++ b/monkey/monkey_island/cc/server_utils/encryption/encryptor_factory.py @@ -1,38 +1,75 @@ -from __future__ import annotations - import os -from ctypes import Union -_factory: Union[None, EncryptorFactory] = None +from Crypto import Random + +from monkey_island.cc.server_utils.encryption import ( + KeyBasedEncryptor, + initialize_datastore_encryptor, +) +from monkey_island.cc.server_utils.encryption.password_based_bytes_encryption import ( + PasswordBasedBytesEncryptor, +) +from monkey_island.cc.server_utils.file_utils import open_new_securely_permissioned_file + +_KEY_FILENAME = "mongo_key.bin" +_BLOCK_SIZE = 32 class EncryptorFactory: + def __init__(self): + self.key_file_path = None + self.secret = None + + def set_key_file_path(self, key_file_path: str): + self.key_file_path = key_file_path - _KEY_FILENAME = "mongo_key.bin" + def set_secret(self, username: str, password: str): + self.secret = _get_secret_from_credentials(username, password) - def __init__(self, key_file_dir: str): - self.key_file_path = os.path.join(key_file_dir, self._KEY_FILENAME) + def initialize_encryptor(self): + if os.path.exists(self.key_file_path): + key_based_encryptor = _load_existing_key(self.key_file_path, self.secret) + else: + key_based_encryptor = _create_new_key(self.key_file_path, self.secret) + initialize_datastore_encryptor(key_based_encryptor) -class FactoryNotInitializedError(Exception): +class KeyPathNotSpecifiedError(Exception): pass -def get_secret_from_credentials(username: str, password: str) -> str: +def _load_existing_key(key_file_path: str, secret: str): + with open(key_file_path, "rb") as f: + encrypted_key = f.read() + cipher_key = PasswordBasedBytesEncryptor(secret).decrypt(encrypted_key) + return KeyBasedEncryptor(cipher_key) + + +def _create_new_key(key_file_path: str, secret: str): + 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 _get_random_bytes() -> bytes: + return Random.new().read(_BLOCK_SIZE) + + +def _get_secret_from_credentials(username: str, password: str) -> str: return f"{username}:{password}" def remove_old_datastore_key(): - if _factory is None: - raise FactoryNotInitializedError + if not _factory.key_file_path: + raise KeyPathNotSpecifiedError if os.path.isfile(_factory.key_file_path): os.remove(_factory.key_file_path) -def initialize_encryptor_factory(key_file_dir: str): - global _factory - _factory = EncryptorFactory(key_file_dir) - - def get_encryptor_factory(): return _factory + + +_factory = EncryptorFactory() 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 e8901862b2d..1d4d23273b2 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,6 +10,7 @@ initialize_encryptor_factory, remove_old_datastore_key, ) +from monkey_island.cc.server_utils.encryption.data_store_encryptor import DataStoreEncryptor from monkey_island.cc.server_utils.encryption.encryptor_factory import EncryptorFactory PLAINTEXT = "Hello, Monkey!" @@ -37,7 +38,7 @@ def test_key_creation(initialized_key_dir): assert (initialized_key_dir / EncryptorFactory._KEY_FILENAME).isfile() -def test_key_removal(initialized_key_dir, monkeypatch): +def test_key_removal(initialized_key_dir): remove_old_datastore_key() assert not (initialized_key_dir / EncryptorFactory._KEY_FILENAME).isfile() @@ -61,3 +62,7 @@ def test_initialize_encryptor(tmpdir): assert not (tmpdir / EncryptorFactory._KEY_FILENAME).isfile() initialize_datastore_encryptor(MOCK_USERNAME, MOCK_PASSWORD) assert (tmpdir / EncryptorFactory._KEY_FILENAME).isfile() + + +def test_key_file_encryption(tmpdir, monkeypatch): + monkeypatch(DataStoreEncryptor._) From 34d065ce698824edb4d51e2dd7c520733e6b00f6 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Mon, 4 Oct 2021 09:29:40 +0300 Subject: [PATCH 12/17] Move encryptors into a separate folder This separates encryptor classes from other encryption related infrastructure that we have cc\server_utils\encryption --- .../cc/server_utils/encryption/__init__.py | 19 +++++++++---------- .../encryption/encryptors/__init__.py | 0 .../{ => encryptors}/i_encryptor.py | 0 .../{ => encryptors}/key_based_encryptor.py | 0 .../password_based_bytes_encryption.py | 0 .../password_based_string_encryptior.py | 2 +- 6 files changed, 10 insertions(+), 11 deletions(-) create mode 100644 monkey/monkey_island/cc/server_utils/encryption/encryptors/__init__.py 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_encryption.py (100%) rename monkey/monkey_island/cc/server_utils/encryption/{ => encryptors}/password_based_string_encryptior.py (90%) diff --git a/monkey/monkey_island/cc/server_utils/encryption/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/__init__.py index 1c3e422dbd9..ea04a25e10c 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/__init__.py +++ b/monkey/monkey_island/cc/server_utils/encryption/__init__.py @@ -1,18 +1,17 @@ -from monkey_island.cc.server_utils.encryption.i_encryptor import IEncryptor -from monkey_island.cc.server_utils.encryption.key_based_encryptor import KeyBasedEncryptor -from monkey_island.cc.server_utils.encryption.password_based_string_encryptior import ( +from monkey_island.cc.server_utils.encryption.encryptors.i_encryptor import IEncryptor +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 ( PasswordBasedStringEncryptor, is_encrypted, ) -from .encryptor_factory import ( - FactoryNotInitializedError, - remove_old_datastore_key, - get_encryptor_factory, - _get_secret_from_credentials, - initialize_encryptor_factory, +from monkey_island.cc.server_utils.encryption.encryptors.password_based_bytes_encryption import ( + PasswordBasedBytesEncryptor, + InvalidCredentialsError, + InvalidCiphertextError, ) from .data_store_encryptor import initialize_datastore_encryptor, get_datastore_encryptor -from .password_based_bytes_encryption import InvalidCredentialsError, InvalidCiphertextError from .dict_encryption.dict_encryptor import ( SensitiveField, encrypt_dict, diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptors/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/encryptors/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/monkey/monkey_island/cc/server_utils/encryption/i_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/encryptors/i_encryptor.py similarity index 100% rename from monkey/monkey_island/cc/server_utils/encryption/i_encryptor.py rename to monkey/monkey_island/cc/server_utils/encryption/encryptors/i_encryptor.py diff --git a/monkey/monkey_island/cc/server_utils/encryption/key_based_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/encryptors/key_based_encryptor.py similarity index 100% rename from monkey/monkey_island/cc/server_utils/encryption/key_based_encryptor.py rename to monkey/monkey_island/cc/server_utils/encryption/encryptors/key_based_encryptor.py diff --git a/monkey/monkey_island/cc/server_utils/encryption/password_based_bytes_encryption.py b/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryption.py similarity index 100% rename from monkey/monkey_island/cc/server_utils/encryption/password_based_bytes_encryption.py rename to monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_bytes_encryption.py diff --git a/monkey/monkey_island/cc/server_utils/encryption/password_based_string_encryptior.py b/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptior.py similarity index 90% rename from monkey/monkey_island/cc/server_utils/encryption/password_based_string_encryptior.py rename to monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptior.py index 716455a5bc3..9f99f735bc5 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/password_based_string_encryptior.py +++ b/monkey/monkey_island/cc/server_utils/encryption/encryptors/password_based_string_encryptior.py @@ -4,7 +4,7 @@ import pyAesCrypt from monkey_island.cc.server_utils.encryption import IEncryptor -from monkey_island.cc.server_utils.encryption.password_based_bytes_encryption import ( +from monkey_island.cc.server_utils.encryption.encryptors.password_based_bytes_encryption import ( PasswordBasedBytesEncryptor, ) From 3ec26bcef84d593f1a5f8651878273a1ecec9018 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Mon, 4 Oct 2021 12:03:30 +0300 Subject: [PATCH 13/17] Refactor data store encryptor to IEncryptor interface, move data store encryptor creation related code to data_store_encryptor.py, move the reponsibility to initialize data store encryptor to AuthenticationService --- .../monkey_island/cc/resources/auth/auth.py | 8 +- .../cc/resources/auth/registration.py | 8 +- monkey/monkey_island/cc/server_setup.py | 3 - .../encryption/data_store_encryptor.py | 58 ++++++++++---- .../mimikatz_results_encryptor.py | 8 +- .../field_encryptors/string_list_encryptor.py | 4 +- .../encryption/encryptor_factory.py | 75 ------------------- .../technique_report_tools.py | 4 +- .../cc/services/authentication.py | 35 +++++++++ monkey/monkey_island/cc/services/config.py | 26 +++---- .../monkey_island/cc/services/initialize.py | 2 + .../services/telemetry/processing/exploit.py | 2 +- .../telemetry/processing/system_info.py | 2 +- .../scoutsuite/scoutsuite_auth_service.py | 2 +- 14 files changed, 109 insertions(+), 128 deletions(-) delete mode 100644 monkey/monkey_island/cc/server_utils/encryption/encryptor_factory.py create mode 100644 monkey/monkey_island/cc/services/authentication.py diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 9a693e80d8f..92a372a99eb 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -13,10 +13,7 @@ get_username_password_from_request, password_matches_hash, ) -from monkey_island.cc.server_utils.encryption import ( - get_datastore_encryptor, - initialize_datastore_encryptor, -) +from monkey_island.cc.services.authentication import AuthenticationService logger = logging.getLogger(__name__) @@ -47,8 +44,7 @@ def post(self): username, password = get_username_password_from_request(request) if _credentials_match_registered_user(username, password): - if not get_datastore_encryptor(): - initialize_datastore_encryptor(username, password) + AuthenticationService.ensure_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/resources/auth/registration.py b/monkey/monkey_island/cc/resources/auth/registration.py index 82dbcfe3a1d..670fa4d19a9 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/monkey/monkey_island/cc/resources/auth/registration.py @@ -9,10 +9,7 @@ get_user_credentials_from_request, get_username_password_from_request, ) -from monkey_island.cc.server_utils.encryption import ( - initialize_datastore_encryptor, - remove_old_datastore_key, -) +from monkey_island.cc.services.authentication import AuthenticationService from monkey_island.cc.setup.mongo.database_initializer import reset_database logger = logging.getLogger(__name__) @@ -28,9 +25,8 @@ def post(self): try: env_singleton.env.try_add_user(credentials) - remove_old_datastore_key() username, password = get_username_password_from_request(request) - initialize_datastore_encryptor(username, password) + AuthenticationService.reset_datastore_encryptor(username, password) reset_database() return make_response({"error": ""}, 200) except (InvalidRegistrationCredentialsError, RegistrationNotNeededError) as e: diff --git a/monkey/monkey_island/cc/server_setup.py b/monkey/monkey_island/cc/server_setup.py index 82c1b3a580c..fdb94b67f56 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -11,8 +11,6 @@ # Add the monkey_island directory to the path, to make sure imports that don't start with # "monkey_island." work. -from monkey_island.cc.server_utils.encryption import initialize_encryptor_factory - MONKEY_ISLAND_DIR_BASE_PATH = str(Path(__file__).parent.parent) if str(MONKEY_ISLAND_DIR_BASE_PATH) not in sys.path: sys.path.insert(0, MONKEY_ISLAND_DIR_BASE_PATH) @@ -88,7 +86,6 @@ def _configure_logging(config_options): def _initialize_globals(config_options: IslandConfigOptions, server_config_path: str): env_singleton.initialize_from_file(server_config_path) - initialize_encryptor_factory(config_options.data_dir) initialize_services(config_options.data_dir) 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 949102c8479..fb38e95a886 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,29 +1,57 @@ -from __future__ import annotations - -# PyCrypto is deprecated, but we use pycryptodome, which uses the exact same imports but -# is maintained. +import os from typing import Union -from monkey_island.cc.server_utils.encryption import KeyBasedEncryptor +from Crypto import Random # noqa: DUO133 # nosec: B413 + +from monkey_island.cc.server_utils.encryption import IEncryptor, KeyBasedEncryptor +from monkey_island.cc.server_utils.encryption.encryptors.password_based_bytes_encryption import ( + PasswordBasedBytesEncryptor, +) +from monkey_island.cc.server_utils.file_utils import open_new_securely_permissioned_file + +_KEY_FILENAME = "mongo_key.bin" +_BLOCK_SIZE = 32 + +_encryptor: Union[None, IEncryptor] = None + -_encryptor: Union[None, DataStoreEncryptor] = None +def _load_existing_key(key_file_path: str, secret: str): + 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: - def __init__(self, key_based_encryptor: KeyBasedEncryptor): - self._key_based_encryptor = key_based_encryptor +def _create_new_key(key_file_path: str, secret: str): + 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 enc(self, message: str): - return self._key_based_encryptor.encrypt(message) - def dec(self, enc_message: str): - return self._key_based_encryptor.decrypt(enc_message) +def _get_random_bytes() -> bytes: + return Random.new().read(_BLOCK_SIZE) -def initialize_datastore_encryptor(key_based_encryptor: KeyBasedEncryptor): +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): global _encryptor - _encryptor = DataStoreEncryptor(key_based_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): + return os.path.join(key_file_dir, _KEY_FILENAME) def get_datastore_encryptor(): 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 6261f5147d6..ff2ee314ec8 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 @@ -17,7 +17,7 @@ def encrypt(results: dict) -> dict: for _, credentials in results.items(): for secret_type in MimikatzResultsEncryptor.secret_types: try: - credentials[secret_type] = get_datastore_encryptor().enc( + credentials[secret_type] = get_datastore_encryptor().encrypt( credentials[secret_type] ) except ValueError as e: @@ -25,12 +25,14 @@ def encrypt(results: dict) -> dict: f"Failed encrypting sensitive field for " f"user {credentials['username']}! Error: {e}" ) - credentials[secret_type] = get_datastore_encryptor().enc("") + credentials[secret_type] = get_datastore_encryptor().encrypt("") return results @staticmethod def decrypt(results: dict) -> dict: for _, credentials in results.items(): for secret_type in MimikatzResultsEncryptor.secret_types: - credentials[secret_type] = get_datastore_encryptor().dec(credentials[secret_type]) + credentials[secret_type] = get_datastore_encryptor().decrypt( + credentials[secret_type] + ) return results 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 46eef09cb30..04374c46236 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 @@ -9,8 +9,8 @@ class StringListEncryptor(IFieldEncryptor): @staticmethod def encrypt(value: List[str]): - return [get_datastore_encryptor().enc(string) for string in value] + return [get_datastore_encryptor().encrypt(string) for string in value] @staticmethod def decrypt(value: List[str]): - return [get_datastore_encryptor().dec(string) for string in value] + return [get_datastore_encryptor().decrypt(string) for string in value] diff --git a/monkey/monkey_island/cc/server_utils/encryption/encryptor_factory.py b/monkey/monkey_island/cc/server_utils/encryption/encryptor_factory.py deleted file mode 100644 index 0ae3e70a65a..00000000000 --- a/monkey/monkey_island/cc/server_utils/encryption/encryptor_factory.py +++ /dev/null @@ -1,75 +0,0 @@ -import os - -from Crypto import Random - -from monkey_island.cc.server_utils.encryption import ( - KeyBasedEncryptor, - initialize_datastore_encryptor, -) -from monkey_island.cc.server_utils.encryption.password_based_bytes_encryption import ( - PasswordBasedBytesEncryptor, -) -from monkey_island.cc.server_utils.file_utils import open_new_securely_permissioned_file - -_KEY_FILENAME = "mongo_key.bin" -_BLOCK_SIZE = 32 - - -class EncryptorFactory: - def __init__(self): - self.key_file_path = None - self.secret = None - - def set_key_file_path(self, key_file_path: str): - self.key_file_path = key_file_path - - def set_secret(self, username: str, password: str): - self.secret = _get_secret_from_credentials(username, password) - - def initialize_encryptor(self): - if os.path.exists(self.key_file_path): - key_based_encryptor = _load_existing_key(self.key_file_path, self.secret) - else: - key_based_encryptor = _create_new_key(self.key_file_path, self.secret) - initialize_datastore_encryptor(key_based_encryptor) - - -class KeyPathNotSpecifiedError(Exception): - pass - - -def _load_existing_key(key_file_path: str, secret: str): - with open(key_file_path, "rb") as f: - encrypted_key = f.read() - cipher_key = PasswordBasedBytesEncryptor(secret).decrypt(encrypted_key) - return KeyBasedEncryptor(cipher_key) - - -def _create_new_key(key_file_path: str, secret: str): - 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 _get_random_bytes() -> bytes: - return Random.new().read(_BLOCK_SIZE) - - -def _get_secret_from_credentials(username: str, password: str) -> str: - return f"{username}:{password}" - - -def remove_old_datastore_key(): - if not _factory.key_file_path: - raise KeyPathNotSpecifiedError - if os.path.isfile(_factory.key_file_path): - os.remove(_factory.key_file_path) - - -def get_encryptor_factory(): - return _factory - - -_factory = EncryptorFactory() diff --git a/monkey/monkey_island/cc/services/attack/technique_reports/technique_report_tools.py b/monkey/monkey_island/cc/services/attack/technique_reports/technique_report_tools.py index 16884678b32..5bb61bc141e 100644 --- a/monkey/monkey_island/cc/services/attack/technique_reports/technique_report_tools.py +++ b/monkey/monkey_island/cc/services/attack/technique_reports/technique_report_tools.py @@ -29,7 +29,7 @@ def censor_password(password, plain_chars=3, secret_chars=5): """ if not password: return "" - password = get_datastore_encryptor().dec(password) + password = get_datastore_encryptor().decrypt(password) return password[0:plain_chars] + "*" * secret_chars @@ -42,5 +42,5 @@ def censor_hash(hash_, plain_chars=5): """ if not hash_: return "" - hash_ = get_datastore_encryptor().dec(hash_) + hash_ = get_datastore_encryptor().decrypt(hash_) return hash_[0:plain_chars] + " ..." diff --git a/monkey/monkey_island/cc/services/authentication.py b/monkey/monkey_island/cc/services/authentication.py new file mode 100644 index 00000000000..ac5400bfd72 --- /dev/null +++ b/monkey/monkey_island/cc/services/authentication.py @@ -0,0 +1,35 @@ +from monkey_island.cc.server_utils.encryption import ( + get_datastore_encryptor, + initialize_datastore_encryptor, +) +from monkey_island.cc.server_utils.encryption.data_store_encryptor import remove_old_datastore_key + + +class AuthenticationService: + KEY_FILE_DIRECTORY = None + + # TODO: A number of these services should be instance objects instead of + # static/singleton hybrids. At the moment, this requires invasive refactoring that's + # not a priority. + @classmethod + def initialize(cls, key_file_directory): + 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) + + @staticmethod + def reset_datastore_encryptor(username: str, password: str): + remove_old_datastore_key(AuthenticationService.KEY_FILE_DIRECTORY) + AuthenticationService._init_encryptor_from_credentials(username, password) + + @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) + + @staticmethod + def _get_secret_from_credentials(username: str, password: str) -> str: + return f"{username}:{password}" diff --git a/monkey/monkey_island/cc/services/config.py b/monkey/monkey_island/cc/services/config.py index 973ca104a6a..6ddcd896f33 100644 --- a/monkey/monkey_island/cc/services/config.py +++ b/monkey/monkey_island/cc/services/config.py @@ -90,9 +90,9 @@ def get_config_value(config_key_as_arr, is_initial_config=False, should_decrypt= if should_decrypt: if config_key_as_arr in ENCRYPTED_CONFIG_VALUES: if isinstance(config, str): - config = get_datastore_encryptor().dec(config) + config = get_datastore_encryptor().decrypt(config) elif isinstance(config, list): - config = [get_datastore_encryptor().dec(x) for x in config] + config = [get_datastore_encryptor().decrypt(x) for x in config] return config @staticmethod @@ -130,7 +130,7 @@ def add_item_to_config_set_if_dont_exist(item_path_array, item_value, should_enc if item_value in items_from_config: return if should_encrypt: - item_value = get_datastore_encryptor().enc(item_value) + item_value = get_datastore_encryptor().encrypt(item_value) mongo.db.config.update( {"name": "newconfig"}, {"$addToSet": {item_key: item_value}}, upsert=False ) @@ -350,10 +350,10 @@ def decrypt_flat_config(flat_config, is_island=False): ] else: flat_config[key] = [ - get_datastore_encryptor().dec(item) for item in flat_config[key] + get_datastore_encryptor().decrypt(item) for item in flat_config[key] ] else: - flat_config[key] = get_datastore_encryptor().dec(flat_config[key]) + flat_config[key] = get_datastore_encryptor().decrypt(flat_config[key]) return flat_config @staticmethod @@ -379,25 +379,25 @@ def _encrypt_or_decrypt_config(config, is_decrypt=False): ) else: config_arr[i] = ( - get_datastore_encryptor().dec(config_arr[i]) + get_datastore_encryptor().decrypt(config_arr[i]) if is_decrypt - else get_datastore_encryptor().enc(config_arr[i]) + else get_datastore_encryptor().encrypt(config_arr[i]) ) else: parent_config_arr[config_arr_as_array[-1]] = ( - get_datastore_encryptor().dec(config_arr) + get_datastore_encryptor().decrypt(config_arr) if is_decrypt - else get_datastore_encryptor().enc(config_arr) + else get_datastore_encryptor().encrypt(config_arr) ) @staticmethod def decrypt_ssh_key_pair(pair, encrypt=False): if encrypt: - pair["public_key"] = get_datastore_encryptor().enc(pair["public_key"]) - pair["private_key"] = get_datastore_encryptor().enc(pair["private_key"]) + pair["public_key"] = get_datastore_encryptor().encrypt(pair["public_key"]) + pair["private_key"] = get_datastore_encryptor().encrypt(pair["private_key"]) else: - pair["public_key"] = get_datastore_encryptor().dec(pair["public_key"]) - pair["private_key"] = get_datastore_encryptor().dec(pair["private_key"]) + pair["public_key"] = get_datastore_encryptor().decrypt(pair["public_key"]) + pair["private_key"] = get_datastore_encryptor().decrypt(pair["private_key"]) return pair @staticmethod diff --git a/monkey/monkey_island/cc/services/initialize.py b/monkey/monkey_island/cc/services/initialize.py index 6ff0d2706d4..b6e37bbc7cf 100644 --- a/monkey/monkey_island/cc/services/initialize.py +++ b/monkey/monkey_island/cc/services/initialize.py @@ -1,3 +1,4 @@ +from monkey_island.cc.services.authentication import AuthenticationService from monkey_island.cc.services.post_breach_files import PostBreachFilesService from monkey_island.cc.services.run_local_monkey import LocalMonkeyRunService @@ -5,3 +6,4 @@ def initialize_services(data_dir): PostBreachFilesService.initialize(data_dir) LocalMonkeyRunService.initialize(data_dir) + AuthenticationService.initialize(key_file_directory=data_dir) diff --git a/monkey/monkey_island/cc/services/telemetry/processing/exploit.py b/monkey/monkey_island/cc/services/telemetry/processing/exploit.py index 7c156930a3e..e302be5f58c 100644 --- a/monkey/monkey_island/cc/services/telemetry/processing/exploit.py +++ b/monkey/monkey_island/cc/services/telemetry/processing/exploit.py @@ -76,4 +76,4 @@ def encrypt_exploit_creds(telemetry_json): credential = attempts[i][field] if credential: # PowerShell exploiter's telem may have `None` here if len(credential) > 0: - attempts[i][field] = get_datastore_encryptor().enc(credential) + attempts[i][field] = get_datastore_encryptor().encrypt(credential) diff --git a/monkey/monkey_island/cc/services/telemetry/processing/system_info.py b/monkey/monkey_island/cc/services/telemetry/processing/system_info.py index ba72e822bde..7d7f404ce19 100644 --- a/monkey/monkey_island/cc/services/telemetry/processing/system_info.py +++ b/monkey/monkey_island/cc/services/telemetry/processing/system_info.py @@ -70,7 +70,7 @@ def encrypt_system_info_ssh_keys(ssh_info): for idx, user in enumerate(ssh_info): for field in ["public_key", "private_key", "known_hosts"]: if ssh_info[idx][field]: - ssh_info[idx][field] = get_datastore_encryptor().enc(ssh_info[idx][field]) + ssh_info[idx][field] = get_datastore_encryptor().encrypt(ssh_info[idx][field]) def process_credential_info(telemetry_json): diff --git a/monkey/monkey_island/cc/services/zero_trust/scoutsuite/scoutsuite_auth_service.py b/monkey/monkey_island/cc/services/zero_trust/scoutsuite/scoutsuite_auth_service.py index 89aa002fae1..b54b3252c12 100644 --- a/monkey/monkey_island/cc/services/zero_trust/scoutsuite/scoutsuite_auth_service.py +++ b/monkey/monkey_island/cc/services/zero_trust/scoutsuite/scoutsuite_auth_service.py @@ -41,7 +41,7 @@ def set_aws_keys(access_key_id: str, secret_access_key: str, session_token: str) def _set_aws_key(key_type: str, key_value: str): path_to_keys = AWS_KEYS_PATH - encrypted_key = get_datastore_encryptor().enc(key_value) + encrypted_key = get_datastore_encryptor().encrypt(key_value) ConfigService.set_config_value(path_to_keys + [key_type], encrypted_key) From ea6fe37b44459696ee19a4f4c9523eb2cd084ccb Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Mon, 4 Oct 2021 12:13:55 +0300 Subject: [PATCH 14/17] Fix scoutsuite unit test to use updated datastore encryptor interface --- .../zero_trust/scoutsuite/test_scoutsuite_auth_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/services/zero_trust/scoutsuite/test_scoutsuite_auth_service.py b/monkey/tests/unit_tests/monkey_island/cc/services/zero_trust/scoutsuite/test_scoutsuite_auth_service.py index 32b4f19ad20..97437791533 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/services/zero_trust/scoutsuite/test_scoutsuite_auth_service.py +++ b/monkey/tests/unit_tests/monkey_island/cc/services/zero_trust/scoutsuite/test_scoutsuite_auth_service.py @@ -26,7 +26,7 @@ def test_is_aws_keys_setup(tmp_path): mongo.db.config.find_one = MagicMock(return_value=ConfigService.default_config) assert not is_aws_keys_setup() - bogus_key_value = get_datastore_encryptor().enc("bogus_aws_key") + bogus_key_value = get_datastore_encryptor().encrypt("bogus_aws_key") dpath.util.set( ConfigService.default_config, AWS_KEYS_PATH + ["aws_secret_access_key"], bogus_key_value ) From a2b09a9e7afe582cb67347778b39cbac20e6b5eb Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Mon, 4 Oct 2021 14:21:07 +0300 Subject: [PATCH 15/17] Fix unit tests for data store encryptor --- .../encryption/data_store_encryptor.py | 8 +-- .../unit_tests/monkey_island/cc/conftest.py | 10 ++- .../encryption/test_data_store_encryptor.py | 65 +++++++++---------- .../cc/services/reporting/test_report.py | 2 +- .../monkey_island/cc/services/test_config.py | 2 + .../cc/services/test_config_manipulator.py | 4 ++ 6 files changed, 44 insertions(+), 47 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 fb38e95a886..6ac0a23ad1c 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 @@ -15,14 +15,14 @@ _encryptor: Union[None, IEncryptor] = None -def _load_existing_key(key_file_path: str, secret: str): +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) -def _create_new_key(key_file_path: str, secret: str): +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: @@ -50,9 +50,9 @@ def initialize_datastore_encryptor(key_file_dir: str, secret: str): _encryptor = _create_new_key(key_file_path, secret) -def _get_key_file_path(key_file_dir: str): +def _get_key_file_path(key_file_dir: str) -> str: return os.path.join(key_file_dir, _KEY_FILENAME) -def get_datastore_encryptor(): +def get_datastore_encryptor() -> IEncryptor: return _encryptor diff --git a/monkey/tests/unit_tests/monkey_island/cc/conftest.py b/monkey/tests/unit_tests/monkey_island/cc/conftest.py index cbd141ac7a2..7d9642201a2 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/conftest.py +++ b/monkey/tests/unit_tests/monkey_island/cc/conftest.py @@ -10,10 +10,8 @@ STANDARD_PLAINTEXT_MONKEY_CONFIG_FILENAME, ) -from monkey_island.cc.server_utils.encryption import ( - initialize_datastore_encryptor, - initialize_encryptor_factory, -) +from monkey_island.cc.server_utils.encryption import initialize_datastore_encryptor +from monkey_island.cc.services.authentication import AuthenticationService @pytest.fixture @@ -36,5 +34,5 @@ def monkey_config_json(monkey_config): @pytest.fixture def uses_encryptor(data_for_tests_dir): - initialize_encryptor_factory(data_for_tests_dir) - initialize_datastore_encryptor(MOCK_USERNAME, MOCK_PASSWORD) + secret = AuthenticationService._get_secret_from_credentials(MOCK_USERNAME, MOCK_PASSWORD) + initialize_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 1d4d23273b2..7c379af1c02 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,68 +1,61 @@ import pytest -from tests.unit_tests.monkey_island.cc.conftest import MOCK_PASSWORD, MOCK_USERNAME from monkey_island.cc.server_utils.encryption import ( - FactoryNotInitializedError, data_store_encryptor, - encryptor_factory, get_datastore_encryptor, initialize_datastore_encryptor, - initialize_encryptor_factory, remove_old_datastore_key, ) -from monkey_island.cc.server_utils.encryption.data_store_encryptor import DataStoreEncryptor -from monkey_island.cc.server_utils.encryption.encryptor_factory import EncryptorFactory PLAINTEXT = "Hello, Monkey!" +MOCK_SECRET = "53CR31" @pytest.mark.usefixtures("uses_encryptor") def test_encryption(data_for_tests_dir): - encrypted_data = get_datastore_encryptor().enc(PLAINTEXT) + encrypted_data = get_datastore_encryptor().encrypt(PLAINTEXT) assert encrypted_data != PLAINTEXT - decrypted_data = get_datastore_encryptor().dec(encrypted_data) + decrypted_data = get_datastore_encryptor().decrypt(encrypted_data) assert decrypted_data == PLAINTEXT @pytest.fixture -def initialized_key_dir(tmpdir): - initialize_encryptor_factory(tmpdir) - initialize_datastore_encryptor(MOCK_USERNAME, MOCK_PASSWORD) - yield tmpdir +def cleanup_encryptor(): + yield data_store_encryptor._encryptor = None - encryptor_factory._factory = None -def test_key_creation(initialized_key_dir): - assert (initialized_key_dir / EncryptorFactory._KEY_FILENAME).isfile() - +@pytest.mark.usefixtures("cleanup_encryptor") +@pytest.fixture +def initialized_encryptor_dir(tmpdir): + initialize_datastore_encryptor(tmpdir, MOCK_SECRET) + return tmpdir -def test_key_removal(initialized_key_dir): - remove_old_datastore_key() - assert not (initialized_key_dir / EncryptorFactory._KEY_FILENAME).isfile() +def test_key_creation(initialized_encryptor_dir): + assert (initialized_encryptor_dir / data_store_encryptor._KEY_FILENAME).isfile() -def test_key_removal__no_key(tmpdir): - initialize_encryptor_factory(tmpdir) - assert not (tmpdir / EncryptorFactory._KEY_FILENAME).isfile() - # Make sure no error thrown when we try to remove an non-existing key - remove_old_datastore_key() - encryptor_factory._factory = None +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_encryptor_not_initialized(): - with pytest.raises(FactoryNotInitializedError): - remove_old_datastore_key() - initialize_datastore_encryptor(MOCK_USERNAME, MOCK_PASSWORD) - -def test_initialize_encryptor(tmpdir): - initialize_encryptor_factory(tmpdir) - assert not (tmpdir / EncryptorFactory._KEY_FILENAME).isfile() - initialize_datastore_encryptor(MOCK_USERNAME, MOCK_PASSWORD) - assert (tmpdir / EncryptorFactory._KEY_FILENAME).isfile() +def test_key_removal__no_key(tmpdir): + assert not (tmpdir / data_store_encryptor._KEY_FILENAME).isfile() + # Make sure no error thrown when we try to remove an non-existing key + remove_old_datastore_key(tmpdir) + data_store_encryptor._factory = None +@pytest.mark.usefixtures("cleanup_encryptor") def test_key_file_encryption(tmpdir, monkeypatch): - monkeypatch(DataStoreEncryptor._) + 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() + + key_based_encryptor = data_store_encryptor._load_existing_key(key_file_path, MOCK_SECRET) + assert key_based_encryptor._key == PLAINTEXT.encode() diff --git a/monkey/tests/unit_tests/monkey_island/cc/services/reporting/test_report.py b/monkey/tests/unit_tests/monkey_island/cc/services/reporting/test_report.py index 6829965f265..a162997076d 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/services/reporting/test_report.py +++ b/monkey/tests/unit_tests/monkey_island/cc/services/reporting/test_report.py @@ -156,7 +156,7 @@ def test_get_stolen_creds_exploit(fake_mongo): assert expected_stolen_creds_exploit == stolen_creds_exploit -@pytest.mark.usefixtures("uses_database") +@pytest.mark.usefixtures("uses_database", "uses_encryptor") def test_get_stolen_creds_system_info(fake_mongo): fake_mongo.db.monkey.insert_one(MONKEY_TELEM) save_telemetry(SYSTEM_INFO_TELEMETRY_TELEM) diff --git a/monkey/tests/unit_tests/monkey_island/cc/services/test_config.py b/monkey/tests/unit_tests/monkey_island/cc/services/test_config.py index 799fc40e171..75b3152e550 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/services/test_config.py +++ b/monkey/tests/unit_tests/monkey_island/cc/services/test_config.py @@ -18,12 +18,14 @@ def mock_port_in_env_singleton(monkeypatch, PORT): monkeypatch.setattr("monkey_island.cc.services.config.env_singleton", mock_singleton) +@pytest.mark.usefixtures("uses_encryptor") def test_set_server_ips_in_config_command_servers(config, IPS, PORT): ConfigService.set_server_ips_in_config(config) expected_config_command_servers = [f"{ip}:{PORT}" for ip in IPS] assert config["internal"]["island_server"]["command_servers"] == expected_config_command_servers +@pytest.mark.usefixtures("uses_encryptor") def test_set_server_ips_in_config_current_server(config, IPS, PORT): ConfigService.set_server_ips_in_config(config) expected_config_current_server = f"{IPS[0]}:{PORT}" diff --git a/monkey/tests/unit_tests/monkey_island/cc/services/test_config_manipulator.py b/monkey/tests/unit_tests/monkey_island/cc/services/test_config_manipulator.py index 12cd44c1015..1935d6f79a4 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/services/test_config_manipulator.py +++ b/monkey/tests/unit_tests/monkey_island/cc/services/test_config_manipulator.py @@ -1,7 +1,10 @@ +import pytest + from monkey_island.cc.services.config_manipulator import update_config_on_mode_set from monkey_island.cc.services.mode.mode_enum import IslandModeEnum +@pytest.mark.usefixtures("uses_encryptor") def test_update_config_on_mode_set_advanced(config, monkeypatch): monkeypatch.setattr("monkey_island.cc.services.config.ConfigService.get_config", lambda: config) monkeypatch.setattr( @@ -14,6 +17,7 @@ def test_update_config_on_mode_set_advanced(config, monkeypatch): assert manipulated_config == config +@pytest.mark.usefixtures("uses_encryptor") def test_update_config_on_mode_set_ransomware(config, monkeypatch): monkeypatch.setattr("monkey_island.cc.services.config.ConfigService.get_config", lambda: config) monkeypatch.setattr( From 3b5dd6ac3ecba55be16bad9a4c727ce3c031bcc2 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Mon, 4 Oct 2021 14:23:50 +0300 Subject: [PATCH 16/17] Remove database initialization during island startup Database initialization can not be done because island doesn't know the key needed for encrypting collections. Since the key only appears after registration, database setup also should happen only after registration --- monkey/monkey_island/cc/app.py | 2 -- monkey/monkey_island/cc/services/database.py | 5 ----- 2 files changed, 7 deletions(-) diff --git a/monkey/monkey_island/cc/app.py b/monkey/monkey_island/cc/app.py index 0bc20852f58..5f877d318fd 100644 --- a/monkey/monkey_island/cc/app.py +++ b/monkey/monkey_island/cc/app.py @@ -54,7 +54,6 @@ from monkey_island.cc.resources.zero_trust.zero_trust_report import ZeroTrustReport from monkey_island.cc.server_utils.consts import MONKEY_ISLAND_ABS_PATH from monkey_island.cc.server_utils.custom_json_encoder import CustomJSONEncoder -from monkey_island.cc.services.database import Database from monkey_island.cc.services.remote_run_aws import RemoteRunAwsService from monkey_island.cc.services.representations import output_json @@ -108,7 +107,6 @@ def init_app_services(app): with app.app_context(): database.init() - Database.init_db() # If running on AWS, this will initialize the instance data, which is used "later" in the # execution of the island. diff --git a/monkey/monkey_island/cc/services/database.py b/monkey/monkey_island/cc/services/database.py index afd4ecc0292..fb46cd726db 100644 --- a/monkey/monkey_island/cc/services/database.py +++ b/monkey/monkey_island/cc/services/database.py @@ -33,11 +33,6 @@ def drop_collection(collection_name: str): mongo.db[collection_name].drop() logger.info("Dropped collection {}".format(collection_name)) - @staticmethod - def init_db(): - if not mongo.db.collection_names(): - Database.reset_db() - @staticmethod def is_mitigations_missing() -> bool: return bool(AttackMitigations.COLLECTION_NAME not in mongo.db.list_collection_names()) From ddff2f0aa411f99c0eb787774cad3748478e1304 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Mon, 4 Oct 2021 14:26:30 +0300 Subject: [PATCH 17/17] Refactor a couple of imports into a shorter import statement --- monkey/monkey_island/cc/server_utils/encryption/__init__.py | 6 +++++- .../cc/server_utils/encryption/data_store_encryptor.py | 5 +++-- monkey/monkey_island/cc/services/authentication.py | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/encryption/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/__init__.py index ea04a25e10c..1094236341d 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/__init__.py +++ b/monkey/monkey_island/cc/server_utils/encryption/__init__.py @@ -11,7 +11,11 @@ InvalidCredentialsError, InvalidCiphertextError, ) -from .data_store_encryptor import initialize_datastore_encryptor, get_datastore_encryptor +from .data_store_encryptor import ( + initialize_datastore_encryptor, + get_datastore_encryptor, + remove_old_datastore_key, +) from .dict_encryption.dict_encryptor import ( SensitiveField, encrypt_dict, 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 6ac0a23ad1c..f7add80f31b 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,8 +3,9 @@ from Crypto import Random # noqa: DUO133 # nosec: B413 -from monkey_island.cc.server_utils.encryption import IEncryptor, KeyBasedEncryptor -from monkey_island.cc.server_utils.encryption.encryptors.password_based_bytes_encryption import ( +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 diff --git a/monkey/monkey_island/cc/services/authentication.py b/monkey/monkey_island/cc/services/authentication.py index ac5400bfd72..9d3d3baa772 100644 --- a/monkey/monkey_island/cc/services/authentication.py +++ b/monkey/monkey_island/cc/services/authentication.py @@ -1,8 +1,8 @@ from monkey_island.cc.server_utils.encryption import ( get_datastore_encryptor, initialize_datastore_encryptor, + remove_old_datastore_key, ) -from monkey_island.cc.server_utils.encryption.data_store_encryptor import remove_old_datastore_key class AuthenticationService: