From 12937f98801fe0650608e822b48b18d4edc945e0 Mon Sep 17 00:00:00 2001 From: Ilija Lazoroski Date: Wed, 6 Apr 2022 16:56:42 +0200 Subject: [PATCH 1/5] Island, UT: Add String field encryptor This encryptor is going to be used by dict_encryptor to encrypt/decrypt the ssh_keypairs dictionaries --- .../cc/server_utils/encryption/__init__.py | 1 + .../encryption/field_encryptors/__init__.py | 1 + .../field_encryptors/string_encryptor.py | 12 ++++++++++++ .../encryption/test_string_encryptor.py | 17 +++++++++++++++++ 4 files changed, 31 insertions(+) create mode 100644 monkey/monkey_island/cc/server_utils/encryption/field_encryptors/string_encryptor.py create mode 100644 monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_string_encryptor.py diff --git a/monkey/monkey_island/cc/server_utils/encryption/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/__init__.py index 4cfe67fe2..7fa6c77a4 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/__init__.py +++ b/monkey/monkey_island/cc/server_utils/encryption/__init__.py @@ -24,3 +24,4 @@ from .dict_encryptor import ( ) from .field_encryptors.i_field_encryptor import IFieldEncryptor from .field_encryptors.string_list_encryptor import StringListEncryptor +from .field_encryptors.string_encryptor import StringEncryptor diff --git a/monkey/monkey_island/cc/server_utils/encryption/field_encryptors/__init__.py b/monkey/monkey_island/cc/server_utils/encryption/field_encryptors/__init__.py index 1ceedf768..84a635ece 100644 --- a/monkey/monkey_island/cc/server_utils/encryption/field_encryptors/__init__.py +++ b/monkey/monkey_island/cc/server_utils/encryption/field_encryptors/__init__.py @@ -1,2 +1,3 @@ from .i_field_encryptor import IFieldEncryptor from .string_list_encryptor import StringListEncryptor +from .string_encryptor import StringEncryptor diff --git a/monkey/monkey_island/cc/server_utils/encryption/field_encryptors/string_encryptor.py b/monkey/monkey_island/cc/server_utils/encryption/field_encryptors/string_encryptor.py new file mode 100644 index 000000000..28f0f2c93 --- /dev/null +++ b/monkey/monkey_island/cc/server_utils/encryption/field_encryptors/string_encryptor.py @@ -0,0 +1,12 @@ +from ..data_store_encryptor import get_datastore_encryptor +from . import IFieldEncryptor + + +class StringEncryptor(IFieldEncryptor): + @staticmethod + def encrypt(value: str): + return get_datastore_encryptor().encrypt(value) + + @staticmethod + def decrypt(value: str): + return get_datastore_encryptor().decrypt(value) diff --git a/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_string_encryptor.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_string_encryptor.py new file mode 100644 index 000000000..637ab1da1 --- /dev/null +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_string_encryptor.py @@ -0,0 +1,17 @@ +from monkey_island.cc.server_utils.encryption import StringEncryptor + +MOCK_STRING = "m0nk3y" +EMPTY_STRING = "" + + +def test_encryptor(uses_encryptor): + encrypted_string = StringEncryptor.encrypt(MOCK_STRING) + assert not encrypted_string == MOCK_STRING + decrypted_string = StringEncryptor.decrypt(encrypted_string) + assert decrypted_string == MOCK_STRING + + +def test_empty_string(uses_encryptor): + # Tests that no erros are raised + encrypted_string = StringEncryptor.encrypt(EMPTY_STRING) + StringEncryptor.decrypt(encrypted_string) From 30ccb2aee384fe329cb6932ca7eb62ddfb875289 Mon Sep 17 00:00:00 2001 From: Ilija Lazoroski Date: Thu, 7 Apr 2022 11:14:22 +0200 Subject: [PATCH 2/5] Island: Use dict_encryptor to encrypt/decrypt ssh_key_pairs Remove unneeded decrypt_ssh_keypairs --- monkey/monkey_island/cc/services/config.py | 41 +++++++++++++--------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/monkey/monkey_island/cc/services/config.py b/monkey/monkey_island/cc/services/config.py index 863aa79a1..a45889e70 100644 --- a/monkey/monkey_island/cc/services/config.py +++ b/monkey/monkey_island/cc/services/config.py @@ -21,7 +21,13 @@ from common.config_value_paths import ( ) from monkey_island.cc.database import mongo from monkey_island.cc.server_utils.consts import ISLAND_PORT -from monkey_island.cc.server_utils.encryption import get_datastore_encryptor +from monkey_island.cc.server_utils.encryption import ( + SensitiveField, + StringEncryptor, + decrypt_dict, + encrypt_dict, + get_datastore_encryptor, +) from monkey_island.cc.services.config_manipulator import update_config_per_mode from monkey_island.cc.services.config_schema.config_schema import SCHEMA from monkey_island.cc.services.mode.island_mode_service import ModeNotSetError, get_mode @@ -41,6 +47,11 @@ ENCRYPTED_CONFIG_VALUES = [ AWS_KEYS_PATH + ["aws_session_token"], ] +SENSITIVE_SSH_KEY_FIELDS = [ + SensitiveField(path="private_key", field_encryptor=StringEncryptor), + SensitiveField(path="public_key", field_encryptor=StringEncryptor), +] + class ConfigService: default_config = None @@ -94,7 +105,12 @@ class ConfigService: if isinstance(config, str): config = get_datastore_encryptor().decrypt(config) elif isinstance(config, list): - config = [get_datastore_encryptor().decrypt(x) for x in config] + if config: + if isinstance(config[0], str): + config = [get_datastore_encryptor().decrypt(x) for x in config] + elif isinstance(config[0], dict) and "public_key" in config[0]: + config = [decrypt_dict(SENSITIVE_SSH_KEY_FIELDS, x) for x in config] + return config @staticmethod @@ -132,7 +148,10 @@ class ConfigService: if item_value in items_from_config: return if should_encrypt: - item_value = get_datastore_encryptor().encrypt(item_value) + if isinstance(item_value, dict): + item_value = encrypt_dict(SENSITIVE_SSH_KEY_FIELDS, item_value) + else: + item_value = get_datastore_encryptor().encrypt(item_value) mongo.db.config.update( {"name": "newconfig"}, {"$addToSet": {item_key: item_value}}, upsert=False ) @@ -348,7 +367,7 @@ class ConfigService: and "public_key" in flat_config[key][0] ): flat_config[key] = [ - ConfigService.decrypt_ssh_key_pair(item) for item in flat_config[key] + decrypt_dict(SENSITIVE_SSH_KEY_FIELDS, item) for item in flat_config[key] ] else: flat_config[key] = [ @@ -375,9 +394,9 @@ class ConfigService: # Check if array of shh key pairs and then decrypt if isinstance(config_arr[i], dict) and "public_key" in config_arr[i]: config_arr[i] = ( - ConfigService.decrypt_ssh_key_pair(config_arr[i]) + decrypt_dict(SENSITIVE_SSH_KEY_FIELDS, config_arr[i]) if is_decrypt - else ConfigService.decrypt_ssh_key_pair(config_arr[i], True) + else encrypt_dict(SENSITIVE_SSH_KEY_FIELDS, config_arr[i]) ) else: config_arr[i] = ( @@ -392,16 +411,6 @@ class ConfigService: else get_datastore_encryptor().encrypt(config_arr) ) - @staticmethod - def decrypt_ssh_key_pair(pair, encrypt=False): - if encrypt: - 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().decrypt(pair["public_key"]) - pair["private_key"] = get_datastore_encryptor().decrypt(pair["private_key"]) - return pair - @staticmethod def is_test_telem_export_enabled(): return ConfigService.get_config_value(EXPORT_MONKEY_TELEMS_PATH) From f2a8dcc90895b0518f90af2c9a9de167ec718a6e Mon Sep 17 00:00:00 2001 From: Ilija Lazoroski Date: Wed, 6 Apr 2022 16:59:10 +0200 Subject: [PATCH 3/5] Island: Remove encryption of ssh keys in ssh_key_processor --- .../credentials/secrets/ssh_key_processor.py | 21 ++----------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/monkey/monkey_island/cc/services/telemetry/processing/credentials/secrets/ssh_key_processor.py b/monkey/monkey_island/cc/services/telemetry/processing/credentials/secrets/ssh_key_processor.py index 0273732da..be8ecf08a 100644 --- a/monkey/monkey_island/cc/services/telemetry/processing/credentials/secrets/ssh_key_processor.py +++ b/monkey/monkey_island/cc/services/telemetry/processing/credentials/secrets/ssh_key_processor.py @@ -1,7 +1,5 @@ from typing import Mapping -from monkey_island.cc.models import Monkey -from monkey_island.cc.server_utils.encryption import get_datastore_encryptor from monkey_island.cc.services.config import ConfigService from monkey_island.cc.services.telemetry.processing.credentials import Credentials @@ -21,17 +19,9 @@ def process_ssh_key(keypair: Mapping, credentials: Credentials): if not _contains_both_keys(keypair): raise SSHKeyProcessingError("Private or public key missing") - # TODO investigate if IP is needed at all - ip = Monkey.get_single_monkey_by_guid(credentials.monkey_guid).ip_addresses[0] - username = credentials.identities[0]["username"] - - encrypted_keys = _encrypt_ssh_keys(keypair) - ConfigService.ssh_add_keys( - user=username, - public_key=encrypted_keys["public_key"], - private_key=encrypted_keys["private_key"], - ip=ip, + public_key=keypair["public_key"], + private_key=keypair["private_key"], ) @@ -40,10 +30,3 @@ def _contains_both_keys(ssh_key: Mapping) -> bool: return ssh_key["public_key"] and ssh_key["private_key"] except KeyError: return False - - -def _encrypt_ssh_keys(ssh_key: Mapping) -> Mapping: - encrypted_keys = {} - for field in ["public_key", "private_key"]: - encrypted_keys[field] = get_datastore_encryptor().encrypt(ssh_key[field]) - return encrypted_keys From 2d800e4502ff3a1d5fabadc1499feae678489c92 Mon Sep 17 00:00:00 2001 From: Ilija Lazoroski Date: Wed, 6 Apr 2022 17:01:30 +0200 Subject: [PATCH 4/5] Island: Remove identification of ssh keys by username and ip `ssh_key_exists` is identifing ssh keys based on username and ip which is wrong. --- monkey/monkey_island/cc/services/config.py | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/monkey/monkey_island/cc/services/config.py b/monkey/monkey_island/cc/services/config.py index a45889e70..f0977ef66 100644 --- a/monkey/monkey_island/cc/services/config.py +++ b/monkey/monkey_island/cc/services/config.py @@ -185,20 +185,12 @@ class ConfigService: ) @staticmethod - def ssh_add_keys(public_key, private_key, user, ip): - if not ConfigService.ssh_key_exists( - ConfigService.get_config_value(SSH_KEYS_PATH, False, False), user, ip - ): - ConfigService.add_item_to_config_set_if_dont_exist( - SSH_KEYS_PATH, - {"public_key": public_key, "private_key": private_key, "user": user, "ip": ip}, - # SSH keys already encrypted in process_ssh_info() - should_encrypt=False, - ) - - @staticmethod - def ssh_key_exists(keys, user, ip): - return [key for key in keys if key["user"] == user and key["ip"] == ip] + def ssh_add_keys(public_key, private_key): + ConfigService.add_item_to_config_set_if_dont_exist( + SSH_KEYS_PATH, + {"public_key": public_key, "private_key": private_key}, + should_encrypt=True, + ) def _filter_none_values(data): if isinstance(data, dict): From 20e3cc0e503f8ca413c90d7d58ff92d45ff8fc8c Mon Sep 17 00:00:00 2001 From: Ilija Lazoroski Date: Wed, 6 Apr 2022 18:48:04 +0200 Subject: [PATCH 5/5] UT: Fix ssh key processor test --- .../processing/credentials/test_ssh_key_processing.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/services/telemetry/processing/credentials/test_ssh_key_processing.py b/monkey/tests/unit_tests/monkey_island/cc/services/telemetry/processing/credentials/test_ssh_key_processing.py index 900166847..6c3161cd3 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/services/telemetry/processing/credentials/test_ssh_key_processing.py +++ b/monkey/tests/unit_tests/monkey_island/cc/services/telemetry/processing/credentials/test_ssh_key_processing.py @@ -4,7 +4,6 @@ import dpath.util import pytest from tests.unit_tests.monkey_island.cc.services.telemetry.processing.credentials.conftest import ( CREDENTIAL_TELEM_TEMPLATE, - fake_ip_address, ) from common.config_value_paths import SSH_KEYS_PATH, USER_LIST_PATH @@ -41,6 +40,4 @@ def test_ssh_credential_parsing(): assert len(ssh_keypairs) == 1 assert ssh_keypairs[0]["private_key"] == fake_private_key assert ssh_keypairs[0]["public_key"] == fake_public_key - assert ssh_keypairs[0]["user"] == fake_username - assert ssh_keypairs[0]["ip"] == fake_ip_address assert fake_username in dpath.util.get(config, USER_LIST_PATH)