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 c550acc7d..111cfa177 100644 --- a/monkey/monkey_island/cc/resources/configuration_export.py +++ b/monkey/monkey_island/cc/resources/configuration_export.py @@ -4,7 +4,7 @@ import flask_restful 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 @@ class ConfigurationExport(flask_restful.Resource): 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 6c0575e94..3a66a2ed0 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.resources.auth.auth import jwt_required 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 @@ class ConfigurationImport(flask_restful.Resource): 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 7d806139c..84e6e6252 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 20708ce31..569dc0d12 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 @@ logger = logging.getLogger(__name__) # 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 @@ class PasswordBasedEncryptor(IEncryptor): 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 000000000..ea796a441 --- /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 fb397f234..bf7ccff80 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 tests.unit_tests.monkey_island.cc.services.utils.ciphertexts_for_encryption 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 d00609481..a231f3219 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 @@ from tests.unit_tests.monkey_island.cc.services.utils.ciphertexts_for_encryption 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 @@ INCORRECT_PASSWORD = "goodbye321" @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 215703c02..df185a1c8 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 @@ import os 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 @@ class DataStoreEncryptor: 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 9cca0caab..cde82e939 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 d02ad5bbb..7ea21849b 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 @@ MOCK_STRING_LIST = ["test_1", "test_2"] 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 bb005fbf7..98229c6fe 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) +@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().enc(PLAINTEXT) != PLAINTEXT - - -def test_aes_cbc_decryption(data_for_tests_dir): - initialize_datastore_encryptor(data_for_tests_dir) - - 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 2e6c2fd50..32b4f19ad 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 @@ import pytest 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 | 63 ++++++++++++------- .../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, 104 insertions(+), 64 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 064395eaf..9c1c8fc62 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 flask_jwt_extended.exceptions import JWTExtendedException 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 @@ class Authenticate(flask_restful.Resource): "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 000000000..1d7a00803 --- /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 f470fd882..000000000 --- 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 12c17d6e5..0877ee4a3 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 @@ class Registration(flask_restful.Resource): 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 df185a1c8..e5a054080 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.encryption.password_based_byte_encryption imp ) 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 _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(encrypted_key) - - def _load_existing_key(self, key_file_path: str, secret: str): - with open(key_file_path, "rb") as f: + def _load_existing_key(self, secret: str): + with open(self.key_file_path, "rb") as f: encrypted_key = f.read() - self._cipher_key = ( + cipher_key = ( PasswordBasedByteEncryptor(secret).decrypt(io.BytesIO(encrypted_key)).getvalue() ) + self._key_base_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() + ) + 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 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 @@ class DataStoreEncryptor: 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 11cf37044..7838a8563 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 cde82e939..4bd054610 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 tests.unit_tests.monkey_island.cc.server_utils.encryption.test_password_bas ) from monkey_island.cc.server_utils.encryption import initialize_datastore_encryptor +from monkey_island.cc.server_utils.encryption.data_store_encryptor import setup_datastore_key @pytest.fixture @@ -32,4 +33,5 @@ ENCRYPTOR_SECRET = "m0nk3y_u53r:53cr3t_p455w0rd" @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 98229c6fe..22f2e9676 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 @@ 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 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 0877ee4a3..e6743302f 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/monkey/monkey_island/cc/resources/auth/registration.py @@ -9,7 +9,7 @@ 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.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 @@ class Registration(flask_restful.Resource): 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 84e6e6252..531659a9e 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 @@ from monkey_island.cc.server_utils.encryption.data_store_encryptor import ( 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 e5a054080..49d39b505 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 22f2e9676..746054841 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 tests.unit_tests.monkey_island.cc.conftest import ENCRYPTOR_SECRET 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 9c1c8fc62..b6c5adb60 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.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_secret_from_request, password_matches_hash, ) from monkey_island.cc.server_utils.encryption.data_store_encryptor import setup_datastore_key @@ -45,14 +44,14 @@ class Authenticate(flask_restful.Resource): 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 1d7a00803..689d4cc0b 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 e6743302f..f96a5ce82 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/monkey/monkey_island/cc/resources/auth/registration.py @@ -5,10 +5,7 @@ from flask import make_response, request 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 @@ class Registration(flask_restful.Resource): 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 49d39b505..c08a9cb70 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 b6c5adb60..06fcb9e07 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -10,7 +10,7 @@ from jwt import PyJWTError 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 @@ class Authenticate(flask_restful.Resource): "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 689d4cc0b..874906edb 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 531659a9e..e9c19d75e 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 c08a9cb70..624e663b4 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 typing import Union 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 @@ class DataStoreEncryptor: 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 569dc0d12..2e7c05819 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 @@ logger = logging.getLogger(__name__) # 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 @@ class PasswordBasedByteEncryptor(IEncryptor): 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 ea796a441..716455a5b 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 @@ class PasswordBasedStringEncryptor(IEncryptor): 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 ef4ee20d0..1a394c025 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 69408a7fc..0aec3d277 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 06fcb9e07..124cd1f71 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -13,7 +13,10 @@ from monkey_island.cc.resources.auth.credential_utils import ( 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 @@ class Authenticate(flask_restful.Resource): 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 f96a5ce82..064e80179 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/monkey/monkey_island/cc/resources/auth/registration.py @@ -6,7 +6,10 @@ from flask import make_response, request 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 @@ class Registration(flask_restful.Resource): 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 69ab0437a..82c1b3a58 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -11,6 +11,8 @@ from gevent.pywsgi import WSGIServer # 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 @@ from monkey_island.cc.server_utils.consts import ( # noqa: E402 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 e9c19d75e..fa9692ca3 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 @@ from monkey_island.cc.server_utils.encryption.password_based_string_encryptior i 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 624e663b4..f58401783 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 typing import Union 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 @@ _encryptor: Union[None, DataStoreEncryptor] = None 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 @@ class DataStoreEncryptor: 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 000000000..67d3e6ee4 --- /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 ee3e8aafa..cbd141ac7 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 @@ from tests.unit_tests.monkey_island.cc.server_utils.encryption.test_password_bas STANDARD_PLAINTEXT_MONKEY_CONFIG_FILENAME, ) -from monkey_island.cc.server_utils.encryption import initialize_datastore_encryptor -from monkey_island.cc.server_utils.encryption.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 @@ MOCK_PASSWORD = "3cr3t_p455w0rd" @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 1a394c025..e8901862b 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 @@ import pytest 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 124cd1f71..9a693e80d 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -10,7 +10,7 @@ from jwt import PyJWTError 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 @@ class Authenticate(flask_restful.Resource): "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 874906edb..a0823d42b 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 064e80179..82dbcfe3a 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/monkey/monkey_island/cc/resources/auth/registration.py @@ -5,7 +5,10 @@ from flask import make_response, request 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 @@ class Registration(flask_restful.Resource): 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 fa9692ca3..1c3e422db 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 @@ from .encryptor_factory import ( 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 f58401783..949102c84 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 @@ class DataStoreEncryptor: 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 67d3e6ee4..0ae3e70a6 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 - _KEY_FILENAME = "mongo_key.bin" + def set_key_file_path(self, key_file_path: str): + self.key_file_path = key_file_path - def __init__(self, key_file_dir: str): - self.key_file_path = os.path.join(key_file_dir, self._KEY_FILENAME) + 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 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 e8901862b..1d4d23273 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 @@ from monkey_island.cc.server_utils.encryption import ( 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 1c3e422db..ea04a25e1 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 000000000..e69de29bb 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 716455a5b..9f99f735b 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 logging 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 | 62 ++++++++++----- .../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, 111 insertions(+), 130 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 9a693e80d..92a372a99 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -13,10 +13,7 @@ from monkey_island.cc.resources.auth.credential_utils import ( 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 @@ class Authenticate(flask_restful.Resource): 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 82dbcfe3a..670fa4d19 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/monkey/monkey_island/cc/resources/auth/registration.py @@ -9,10 +9,7 @@ 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, -) +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 @@ class Registration(flask_restful.Resource): 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 82c1b3a58..fdb94b67f 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -11,8 +11,6 @@ from gevent.pywsgi import WSGIServer # 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 949102c84..fb38e95a8 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 -_encryptor: Union[None, DataStoreEncryptor] = None +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 -class DataStoreEncryptor: - 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) - - def dec(self, enc_message: str): - return self._key_based_encryptor.decrypt(enc_message) +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 initialize_datastore_encryptor(key_based_encryptor: KeyBasedEncryptor): +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 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 6261f5147..ff2ee314e 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 @@ class MimikatzResultsEncryptor(IFieldEncryptor): 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 @@ class MimikatzResultsEncryptor(IFieldEncryptor): 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 46eef09cb..04374c462 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 @@ from monkey_island.cc.server_utils.encryption.dict_encryption.field_encryptors i 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 0ae3e70a6..000000000 --- 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 16884678b..5bb61bc14 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 000000000..ac5400bfd --- /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 973ca104a..6ddcd896f 100644 --- a/monkey/monkey_island/cc/services/config.py +++ b/monkey/monkey_island/cc/services/config.py @@ -90,9 +90,9 @@ class ConfigService: 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 @@ class ConfigService: 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 @@ class ConfigService: ] 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 @@ class ConfigService: ) 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 6ff0d2706..b6e37bbc7 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 @@ from monkey_island.cc.services.run_local_monkey import LocalMonkeyRunService 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 7c156930a..e302be5f5 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 ba72e822b..7d7f404ce 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 89aa002fa..b54b3252c 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 32b4f19ad..974377915 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 fb38e95a8..6ac0a23ad 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 @@ _BLOCK_SIZE = 32 _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 cbd141ac7..7d9642201 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 @@ from tests.unit_tests.monkey_island.cc.server_utils.encryption.test_password_bas STANDARD_PLAINTEXT_MONKEY_CONFIG_FILENAME, ) -from monkey_island.cc.server_utils.encryption import ( - initialize_datastore_encryptor, - 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 @@ MOCK_PASSWORD = "3cr3t_p455w0rd" @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 1d4d23273..7c379af1c 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(initialized_encryptor_dir): + remove_old_datastore_key(initialized_encryptor_dir) + assert not (initialized_encryptor_dir / data_store_encryptor._KEY_FILENAME).isfile() def test_key_removal__no_key(tmpdir): - initialize_encryptor_factory(tmpdir) - assert not (tmpdir / EncryptorFactory._KEY_FILENAME).isfile() + 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() - encryptor_factory._factory = None - - -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() + 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 6829965f2..a16299707 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 799fc40e1..75b3152e5 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 12cd44c10..1935d6f79 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 0bc20852f..5f877d318 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.scoutsuite_auth.scoutsuite_auth impor 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 afd4ecc02..fb46cd726 100644 --- a/monkey/monkey_island/cc/services/database.py +++ b/monkey/monkey_island/cc/services/database.py @@ -33,11 +33,6 @@ class Database(object): 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 ea04a25e1..109423634 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 @@ from monkey_island.cc.server_utils.encryption.encryptors.password_based_bytes_en 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 6ac0a23ad..f7add80f3 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 typing import Union 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 ac5400bfd..9d3d3baa7 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: