Merge pull request #1870 from guardicore/1826-ssh-key-todo

1826 ssh key todo

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

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

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)

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
         )
@@ -166,20 +185,12 @@ class ConfigService:
         )
 
     @staticmethod
-    def ssh_add_keys(public_key, private_key, user, ip):
+    def ssh_add_keys(public_key, private_key):
-        if not ConfigService.ssh_key_exists(
+        ConfigService.add_item_to_config_set_if_dont_exist(
-            ConfigService.get_config_value(SSH_KEYS_PATH, False, False), user, ip
+            SSH_KEYS_PATH,
-        ):
+            {"public_key": public_key, "private_key": private_key},
-            ConfigService.add_item_to_config_set_if_dont_exist(
+            should_encrypt=True,
-                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 _filter_none_values(data):
         if isinstance(data, dict):
@@ -348,7 +359,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 +386,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 +403,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)

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=keypair["public_key"],
-        public_key=encrypted_keys["public_key"],
+        private_key=keypair["private_key"],
-        private_key=encrypted_keys["private_key"],
-        ip=ip,
     )
 
 
@@ -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

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)

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)