Merge pull request #1516 from guardicore/encryption-code-quality-improvements

Encryption code quality improvements
This commit is contained in:
Mike Salvatore 2021-10-08 08:05:05 -04:00 committed by GitHub
commit 356b3475cd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
22 changed files with 181 additions and 174 deletions

View File

@ -16,6 +16,8 @@ class MimikatzCredentialCollector(object):
def cred_list_to_cred_dict(creds: List[WindowsCredentials]):
cred_dict = {}
for cred in creds:
# TODO: This should be handled by the island, not the agent. There is already similar
# code in monkey_island/cc/models/report/report_dal.py.
# Lets not use "." and "$" in keys, because it will confuse mongo.
# Ideally we should refactor island not to use a dict and simply parse credential list.
key = cred.username.replace(".", ",").replace("$", "")

View File

@ -44,7 +44,7 @@ class Authenticate(flask_restful.Resource):
username, password = get_username_password_from_request(request)
if _credentials_match_registered_user(username, password):
AuthenticationService.ensure_datastore_encryptor(username, password)
AuthenticationService.unlock_datastore_encryptor(username, password)
access_token = _create_access_token(username)
return make_response({"access_token": access_token, "error": ""}, 200)
else:

View File

@ -1,26 +1,27 @@
from monkey_island.cc.server_utils.encryption.encryptors.i_encryptor import IEncryptor
from monkey_island.cc.server_utils.encryption.encryptors.key_based_encryptor import (
from .i_encryptor import IEncryptor
from .key_based_encryptor import (
KeyBasedEncryptor,
)
from monkey_island.cc.server_utils.encryption.encryptors.password_based_string_encryptior import (
from .password_based_string_encryptor import (
PasswordBasedStringEncryptor,
is_encrypted,
)
from monkey_island.cc.server_utils.encryption.encryptors.password_based_bytes_encryption import (
from .password_based_bytes_encryptor import (
PasswordBasedBytesEncryptor,
InvalidCredentialsError,
InvalidCiphertextError,
)
from .data_store_encryptor import (
initialize_datastore_encryptor,
get_datastore_encryptor,
remove_old_datastore_key,
unlock_datastore_encryptor,
reset_datastore_encryptor,
)
from .dict_encryption.dict_encryptor import (
from .dict_encryptor import (
SensitiveField,
encrypt_dict,
decrypt_dict,
FieldNotFoundError,
)
from .dict_encryption.field_encryptors.mimikatz_results_encryptor import MimikatzResultsEncryptor
from .dict_encryption.field_encryptors.string_list_encryptor import StringListEncryptor
from .field_encryptors.i_field_encryptor import IFieldEncryptor
from .field_encryptors.mimikatz_results_encryptor import MimikatzResultsEncryptor
from .field_encryptors.string_list_encryptor import StringListEncryptor

View File

@ -1,58 +1,75 @@
import os
from pathlib import Path
from typing import Union
from Crypto import Random # noqa: DUO133 # nosec: B413
from monkey_island.cc.server_utils.encryption import (
IEncryptor,
KeyBasedEncryptor,
PasswordBasedBytesEncryptor,
)
from monkey_island.cc.server_utils.file_utils import open_new_securely_permissioned_file
_KEY_FILENAME = "mongo_key.bin"
_BLOCK_SIZE = 32
from .i_encryptor import IEncryptor
from .key_based_encryptor import KeyBasedEncryptor
from .password_based_bytes_encryptor import PasswordBasedBytesEncryptor
_KEY_FILE_NAME = "mongo_key.bin"
_encryptor: Union[None, IEncryptor] = None
def _load_existing_key(key_file_path: str, secret: str) -> KeyBasedEncryptor:
with open(key_file_path, "rb") as f:
encrypted_key = f.read()
cipher_key = PasswordBasedBytesEncryptor(secret).decrypt(encrypted_key)
return KeyBasedEncryptor(cipher_key)
class DataStoreEncryptor(IEncryptor):
_KEY_LENGTH_BYTES = 32
def __init__(self, secret: str, key_file: Path):
self._key_file = key_file
self._password_based_encryptor = PasswordBasedBytesEncryptor(secret)
self._key_based_encryptor = self._initialize_key_based_encryptor()
def _initialize_key_based_encryptor(self):
if os.path.exists(self._key_file):
return self._load_key()
return self._create_key()
def _load_key(self) -> KeyBasedEncryptor:
with open(self._key_file, "rb") as f:
encrypted_key = f.read()
plaintext_key = self._password_based_encryptor.decrypt(encrypted_key)
return KeyBasedEncryptor(plaintext_key)
def _create_key(self) -> KeyBasedEncryptor:
plaintext_key = Random.new().read(DataStoreEncryptor._KEY_LENGTH_BYTES)
encrypted_key = self._password_based_encryptor.encrypt(plaintext_key)
with open_new_securely_permissioned_file(self._key_file, "wb") as f:
f.write(encrypted_key)
return KeyBasedEncryptor(plaintext_key)
def encrypt(self, plaintext: str) -> str:
return self._key_based_encryptor.encrypt(plaintext)
def decrypt(self, ciphertext: str):
return self._key_based_encryptor.decrypt(ciphertext)
def _create_new_key(key_file_path: str, secret: str) -> KeyBasedEncryptor:
cipher_key = _get_random_bytes()
encrypted_key = PasswordBasedBytesEncryptor(secret).encrypt(cipher_key)
with open_new_securely_permissioned_file(key_file_path, "wb") as f:
f.write(encrypted_key)
return KeyBasedEncryptor(cipher_key)
def reset_datastore_encryptor(key_file_dir: str, secret: str, key_file_name: str = _KEY_FILE_NAME):
key_file = Path(key_file_dir) / key_file_name
if key_file.is_file():
key_file.unlink()
_initialize_datastore_encryptor(key_file, secret)
def _get_random_bytes() -> bytes:
return Random.new().read(_BLOCK_SIZE)
def unlock_datastore_encryptor(key_file_dir: str, secret: str, key_file_name: str = _KEY_FILE_NAME):
key_file = Path(key_file_dir) / key_file_name
_initialize_datastore_encryptor(key_file, secret)
def remove_old_datastore_key(key_file_dir: str):
key_file_path = _get_key_file_path(key_file_dir)
if os.path.isfile(key_file_path):
os.remove(key_file_path)
def initialize_datastore_encryptor(key_file_dir: str, secret: str):
def _initialize_datastore_encryptor(key_file: Path, secret: str):
global _encryptor
key_file_path = _get_key_file_path(key_file_dir)
if os.path.exists(key_file_path):
_encryptor = _load_existing_key(key_file_path, secret)
else:
_encryptor = _create_new_key(key_file_path, secret)
def _get_key_file_path(key_file_dir: str) -> str:
return os.path.join(key_file_dir, _KEY_FILENAME)
_encryptor = DataStoreEncryptor(secret, key_file)
def get_datastore_encryptor() -> IEncryptor:

View File

@ -3,9 +3,7 @@ from typing import Callable, List, Type
import dpath.util
from monkey_island.cc.server_utils.encryption.dict_encryption.field_encryptors import (
IFieldEncryptor,
)
from .field_encryptors import IFieldEncryptor
class FieldNotFoundError(Exception):

View File

@ -1,9 +1,7 @@
import logging
from monkey_island.cc.server_utils.encryption import get_datastore_encryptor
from monkey_island.cc.server_utils.encryption.dict_encryption.field_encryptors import (
IFieldEncryptor,
)
from ..data_store_encryptor import get_datastore_encryptor
from . import IFieldEncryptor
logger = logging.getLogger(__name__)
@ -16,16 +14,9 @@ class MimikatzResultsEncryptor(IFieldEncryptor):
def encrypt(results: dict) -> dict:
for _, credentials in results.items():
for secret_type in MimikatzResultsEncryptor.secret_types:
try:
credentials[secret_type] = get_datastore_encryptor().encrypt(
credentials[secret_type]
)
except ValueError as e:
logger.error(
f"Failed encrypting sensitive field for "
f"user {credentials['username']}! Error: {e}"
)
credentials[secret_type] = get_datastore_encryptor().encrypt("")
credentials[secret_type] = get_datastore_encryptor().encrypt(
credentials[secret_type]
)
return results
@staticmethod

View File

@ -1,9 +1,7 @@
from typing import List
from monkey_island.cc.server_utils.encryption import get_datastore_encryptor
from monkey_island.cc.server_utils.encryption.dict_encryption.field_encryptors import (
IFieldEncryptor,
)
from ..data_store_encryptor import get_datastore_encryptor
from . import IFieldEncryptor
class StringListEncryptor(IFieldEncryptor):

View File

@ -7,7 +7,7 @@ from Crypto import Random # noqa: DUO133 # nosec: B413
from Crypto.Cipher import AES # noqa: DUO133 # nosec: B413
from Crypto.Util import Padding # noqa: DUO133
from monkey_island.cc.server_utils.encryption import IEncryptor
from .i_encryptor import IEncryptor
logger = logging.getLogger(__name__)

View File

@ -3,7 +3,7 @@ import logging
import pyAesCrypt
from monkey_island.cc.server_utils.encryption import IEncryptor
from .i_encryptor import IEncryptor
logger = logging.getLogger(__name__)
@ -47,10 +47,10 @@ class PasswordBasedBytesEncryptor(IEncryptor):
)
except ValueError as ex:
if str(ex).startswith("Wrong password"):
logger.info("Wrong password provided for decryption.")
logger.error("Wrong password provided for decryption.")
raise InvalidCredentialsError
else:
logger.info("The corrupt ciphertext provided.")
logger.error("The provided ciphertext was corrupt.")
raise InvalidCiphertextError
return plaintext_stream.getvalue()

View File

@ -3,10 +3,8 @@ import logging
import pyAesCrypt
from monkey_island.cc.server_utils.encryption import IEncryptor
from monkey_island.cc.server_utils.encryption.encryptors.password_based_bytes_encryption import (
PasswordBasedBytesEncryptor,
)
from .i_encryptor import IEncryptor
from .password_based_bytes_encryptor import PasswordBasedBytesEncryptor
logger = logging.getLogger(__name__)

View File

@ -1,7 +1,6 @@
from monkey_island.cc.server_utils.encryption import (
get_datastore_encryptor,
initialize_datastore_encryptor,
remove_old_datastore_key,
reset_datastore_encryptor,
unlock_datastore_encryptor,
)
@ -16,19 +15,14 @@ class AuthenticationService:
cls.KEY_FILE_DIRECTORY = key_file_directory
@staticmethod
def ensure_datastore_encryptor(username: str, password: str):
if not get_datastore_encryptor():
AuthenticationService._init_encryptor_from_credentials(username, password)
def unlock_datastore_encryptor(username: str, password: str):
secret = AuthenticationService._get_secret_from_credentials(username, password)
unlock_datastore_encryptor(AuthenticationService.KEY_FILE_DIRECTORY, secret)
@staticmethod
def reset_datastore_encryptor(username: str, password: str):
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)
reset_datastore_encryptor(AuthenticationService.KEY_FILE_DIRECTORY, secret)
@staticmethod
def _get_secret_from_credentials(username: str, password: str) -> str:

View File

@ -10,8 +10,7 @@ from tests.unit_tests.monkey_island.cc.server_utils.encryption.test_password_bas
STANDARD_PLAINTEXT_MONKEY_CONFIG_FILENAME,
)
from monkey_island.cc.server_utils.encryption import initialize_datastore_encryptor
from monkey_island.cc.services.authentication import AuthenticationService
from monkey_island.cc.server_utils.encryption import unlock_datastore_encryptor
@pytest.fixture
@ -28,11 +27,7 @@ def monkey_config_json(monkey_config):
return json.dumps(monkey_config)
MOCK_USERNAME = "m0nk3y_u53r"
MOCK_PASSWORD = "3cr3t_p455w0rd"
@pytest.fixture
def uses_encryptor(data_for_tests_dir):
secret = AuthenticationService._get_secret_from_credentials(MOCK_USERNAME, MOCK_PASSWORD)
initialize_datastore_encryptor(data_for_tests_dir, secret)
secret = "m0nk3y_u53r:3cr3t_p455w0rd"
unlock_datastore_encryptor(data_for_tests_dir, secret)

View File

@ -6,13 +6,9 @@ import pytest
from monkey_island.cc.models.telemetries import get_telemetry_by_query, save_telemetry
from monkey_island.cc.models.telemetries.telemetry import Telemetry
from monkey_island.cc.server_utils.encryption import SensitiveField
from monkey_island.cc.server_utils.encryption.dict_encryption.field_encryptors import (
MimikatzResultsEncryptor,
)
MOCK_CREDENTIALS = {
"Vakaris": {
"M0nk3y": {
"username": "M0nk3y",
"password": "",
"ntlm_hash": "e87f2f73e353f1d95e42ce618601b61f",
@ -24,7 +20,6 @@ MOCK_CREDENTIALS = {
MOCK_DATA_DICT = {
"network_info": {},
"credentials": deepcopy(MOCK_CREDENTIALS),
"mimikatz": deepcopy(MOCK_CREDENTIALS),
}
MOCK_TELEMETRY = {
@ -49,19 +44,6 @@ MOCK_NO_ENCRYPTION_NEEDED_TELEMETRY = {
"data": {"done": False},
}
MOCK_SENSITIVE_FIELDS = [
SensitiveField("data.credentials", MimikatzResultsEncryptor),
SensitiveField("data.mimikatz", MimikatzResultsEncryptor),
]
@pytest.fixture(autouse=True)
def patch_sensitive_fields(monkeypatch):
monkeypatch.setattr(
"monkey_island.cc.models.telemetries.telemetry_dal.sensitive_fields",
MOCK_SENSITIVE_FIELDS,
)
@pytest.fixture(autouse=True)
def fake_mongo(monkeypatch):
@ -69,28 +51,29 @@ def fake_mongo(monkeypatch):
monkeypatch.setattr("monkey_island.cc.models.telemetries.telemetry_dal.mongo", mongo)
@pytest.mark.slow
@pytest.mark.usefixtures("uses_database", "uses_encryptor")
def test_telemetry_encryption():
secret_keys = ["password", "lm_hash", "ntlm_hash"]
save_telemetry(MOCK_TELEMETRY)
assert (
not Telemetry.objects.first()["data"]["credentials"]["user"]["password"]
== MOCK_CREDENTIALS["user"]["password"]
)
assert (
not Telemetry.objects.first()["data"]["mimikatz"]["Vakaris"]["ntlm_hash"]
== MOCK_CREDENTIALS["Vakaris"]["ntlm_hash"]
)
assert (
get_telemetry_by_query({})[0]["data"]["credentials"]["user"]["password"]
== MOCK_CREDENTIALS["user"]["password"]
)
assert (
get_telemetry_by_query({})[0]["data"]["mimikatz"]["Vakaris"]["ntlm_hash"]
== MOCK_CREDENTIALS["Vakaris"]["ntlm_hash"]
)
encrypted_telemetry = Telemetry.objects.first()
for user in MOCK_CREDENTIALS.keys():
assert encrypted_telemetry["data"]["credentials"][user]["username"] == user
for s in secret_keys:
assert encrypted_telemetry["data"]["credentials"][user][s] != MOCK_CREDENTIALS[user][s]
decrypted_telemetry = get_telemetry_by_query({})[0]
for user in MOCK_CREDENTIALS.keys():
assert decrypted_telemetry["data"]["credentials"][user]["username"] == user
for s in secret_keys:
assert decrypted_telemetry["data"]["credentials"][user][s] == MOCK_CREDENTIALS[user][s]
@pytest.mark.slow
@pytest.mark.usefixtures("uses_database", "uses_encryptor")
def test_no_encryption_needed():
# Make sure telemetry save doesn't break when telemetry doesn't need encryption

View File

@ -5,10 +5,7 @@ import pytest
from monkey_island.cc.models import Report
from monkey_island.cc.models.report import get_report, save_report
from monkey_island.cc.server_utils.encryption import SensitiveField
from monkey_island.cc.server_utils.encryption.dict_encryption.field_encryptors import (
IFieldEncryptor,
)
from monkey_island.cc.server_utils.encryption import IFieldEncryptor, SensitiveField
MOCK_SENSITIVE_FIELD_CONTENTS = ["the_string", "the_string2"]
MOCK_REPORT_DICT = {

View File

@ -1,18 +1,34 @@
import pytest
from common.utils.file_utils import get_file_sha256_hash
from monkey_island.cc.server_utils.encryption import (
data_store_encryptor,
get_datastore_encryptor,
initialize_datastore_encryptor,
remove_old_datastore_key,
reset_datastore_encryptor,
unlock_datastore_encryptor,
)
# Mark all tests in this module as slow
pytestmark = pytest.mark.slow
PLAINTEXT = "Hello, Monkey!"
MOCK_SECRET = "53CR31"
@pytest.mark.usefixtures("uses_encryptor")
def test_encryption(data_for_tests_dir):
@pytest.fixture(autouse=True)
def cleanup_encryptor():
yield
data_store_encryptor._encryptor = None
@pytest.fixture
def key_file(tmp_path):
return tmp_path / "test_key.bin"
def test_encryption(tmp_path):
unlock_datastore_encryptor(tmp_path, MOCK_SECRET)
encrypted_data = get_datastore_encryptor().encrypt(PLAINTEXT)
assert encrypted_data != PLAINTEXT
@ -20,42 +36,51 @@ def test_encryption(data_for_tests_dir):
assert decrypted_data == PLAINTEXT
@pytest.fixture
def cleanup_encryptor():
yield
data_store_encryptor._encryptor = None
def test_key_creation(key_file):
assert not key_file.is_file()
unlock_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name)
assert key_file.is_file()
@pytest.mark.usefixtures("cleanup_encryptor")
@pytest.fixture
def initialized_encryptor_dir(tmpdir):
initialize_datastore_encryptor(tmpdir, MOCK_SECRET)
return tmpdir
def test_existing_key_reused(key_file):
assert not key_file.is_file()
unlock_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name)
key_file_hash_1 = get_file_sha256_hash(key_file)
unlock_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name)
key_file_hash_2 = get_file_sha256_hash(key_file)
assert key_file_hash_1 == key_file_hash_2
def test_key_creation(initialized_encryptor_dir):
assert (initialized_encryptor_dir / data_store_encryptor._KEY_FILENAME).isfile()
def test_reset_datastore_encryptor(key_file):
unlock_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name)
key_file_hash_1 = get_file_sha256_hash(key_file)
reset_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name)
key_file_hash_2 = get_file_sha256_hash(key_file)
assert key_file_hash_1 != key_file_hash_2
def test_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_reset_when_encryptor_is_none(key_file):
with key_file.open(mode="w") as f:
f.write("")
reset_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name)
assert (
get_file_sha256_hash(key_file)
!= "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
)
def test_key_removal__no_key(tmpdir):
assert not (tmpdir / data_store_encryptor._KEY_FILENAME).isfile()
# Make sure no error thrown when we try to remove an non-existing key
remove_old_datastore_key(tmpdir)
data_store_encryptor._factory = None
def test_reset_when_file_not_found(key_file):
assert not key_file.is_file()
reset_datastore_encryptor(key_file.parent, MOCK_SECRET, key_file.name)
encrypted_data = get_datastore_encryptor().encrypt(PLAINTEXT)
assert encrypted_data != PLAINTEXT
@pytest.mark.usefixtures("cleanup_encryptor")
def test_key_file_encryption(tmpdir, monkeypatch):
monkeypatch.setattr(data_store_encryptor, "_get_random_bytes", lambda: PLAINTEXT.encode())
initialize_datastore_encryptor(tmpdir, MOCK_SECRET)
key_file_path = data_store_encryptor._get_key_file_path(tmpdir)
key_file_contents = open(key_file_path, "rb").read()
assert not key_file_contents == PLAINTEXT.encode()
key_based_encryptor = data_store_encryptor._load_existing_key(key_file_path, MOCK_SECRET)
assert key_based_encryptor._key == PLAINTEXT.encode()
decrypted_data = get_datastore_encryptor().decrypt(encrypted_data)
assert decrypted_data == PLAINTEXT

View File

@ -5,39 +5,45 @@ from tests.unit_tests.monkey_island.cc.services.utils.ciphertexts_for_encryption
)
from monkey_island.cc.server_utils.encryption import (
InvalidCiphertextError,
InvalidCredentialsError,
PasswordBasedStringEncryptor,
)
# Mark all tests in this module as slow
pytestmark = pytest.mark.slow
MONKEY_CONFIGS_DIR_PATH = "monkey_configs"
STANDARD_PLAINTEXT_MONKEY_CONFIG_FILENAME = "monkey_config_standard.json"
PASSWORD = "hello123"
INCORRECT_PASSWORD = "goodbye321"
@pytest.mark.slow
def test_encrypt_decrypt_string(monkey_config_json):
pb_encryptor = PasswordBasedStringEncryptor(PASSWORD)
encrypted_config = pb_encryptor.encrypt(monkey_config_json)
assert pb_encryptor.decrypt(encrypted_config) == monkey_config_json
@pytest.mark.slow
def test_decrypt_string__wrong_password(monkey_config_json):
pb_encryptor = PasswordBasedStringEncryptor(INCORRECT_PASSWORD)
with pytest.raises(InvalidCredentialsError):
pb_encryptor.decrypt(VALID_CIPHER_TEXT)
@pytest.mark.slow
def test_decrypt_string__malformed_corrupted():
pb_encryptor = PasswordBasedStringEncryptor(PASSWORD)
with pytest.raises(ValueError):
pb_encryptor.decrypt(MALFORMED_CIPHER_TEXT_CORRUPTED)
@pytest.mark.slow
def test_decrypt_string__no_password(monkey_config_json):
pb_encryptor = PasswordBasedStringEncryptor("")
with pytest.raises(InvalidCredentialsError):
pb_encryptor.decrypt(VALID_CIPHER_TEXT)
def test_decrypt_string__invalid_cyphertext(monkey_config_json):
pb_encryptor = PasswordBasedStringEncryptor("")
with pytest.raises(InvalidCiphertextError):
pb_encryptor.decrypt("")

View File

@ -1,11 +1,12 @@
from monkey_island.cc.server_utils.encryption.dict_encryption.field_encryptors import (
StringListEncryptor,
)
import pytest
from monkey_island.cc.server_utils.encryption import StringListEncryptor
MOCK_STRING_LIST = ["test_1", "test_2"]
EMPTY_LIST = []
@pytest.mark.slow
def test_encryption_and_decryption(uses_encryptor):
encrypted_list = StringListEncryptor.encrypt(MOCK_STRING_LIST)
assert not encrypted_list == MOCK_STRING_LIST
@ -13,6 +14,7 @@ def test_encryption_and_decryption(uses_encryptor):
assert decrypted_list == MOCK_STRING_LIST
@pytest.mark.slow
def test_empty_list(uses_encryptor):
# Tests that no errors are raised
encrypted_list = StringListEncryptor.encrypt(EMPTY_LIST)