Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data dir #994

Merged
merged 9 commits into from
Feb 25, 2021
2 changes: 2 additions & 0 deletions monkey/monkey_island/cc/consts.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,5 @@
DEFAULT_LOGGER_CONFIG_PATH = os.path.join(
MONKEY_ISLAND_ABS_PATH, "cc", "island_logger_default_config.json"
)

DEFAULT_DATA_DIR = os.path.join(MONKEY_ISLAND_ABS_PATH, "cc")
Copy link
Contributor

@VakarisZ VakarisZ Feb 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Term "Data" is misleading. This is a path to island codebase, that's what I'd call this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default it is, only because that's what we're doing today. We need to move away from storing data generated at runtime alongside our code. Since we're so close to a release, I don't want to change that right now, except when running from an AppImage.

53 changes: 32 additions & 21 deletions monkey/monkey_island/cc/encryptor.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,50 +5,61 @@
from Crypto import Random # noqa: DOU133
from Crypto.Cipher import AES # noqa: DOU133

from monkey_island.cc.consts import MONKEY_ISLAND_ABS_PATH

__author__ = "itay.mizeretz"

_encryptor = None


class Encryptor:
_BLOCK_SIZE = 32
_DB_PASSWORD_FILENAME = os.path.join(MONKEY_ISLAND_ABS_PATH, 'cc/mongo_key.bin')
_PASSWORD_FILENAME = "mongo_key.bin"

def __init__(self):
self._load_key()
def __init__(self, data_dir):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Data_dir is too abstract. I'd rename this to something like password_file_dir

password_file = os.path.expanduser(
os.path.join(data_dir, self._PASSWORD_FILENAME)
)

def _init_key(self):
if os.path.exists(password_file):
self._load_existing_key(password_file)
else:
self._init_key(password_file)

def _init_key(self, password_file):
self._cipher_key = Random.new().read(self._BLOCK_SIZE)
with open(self._DB_PASSWORD_FILENAME, 'wb') as f:
with open(password_file, "wb") as f:
f.write(self._cipher_key)

def _load_existing_key(self):
with open(self._DB_PASSWORD_FILENAME, 'rb') as f:
def _load_existing_key(self, password_file):
with open(password_file, "rb") as f:
self._cipher_key = f.read()

def _load_key(self):
if os.path.exists(self._DB_PASSWORD_FILENAME):
self._load_existing_key()
else:
self._init_key()

def _pad(self, message):
return message + (self._BLOCK_SIZE - (len(message) % self._BLOCK_SIZE)) * chr(
self._BLOCK_SIZE - (len(message) % self._BLOCK_SIZE))
self._BLOCK_SIZE - (len(message) % self._BLOCK_SIZE)
)

def _unpad(self, message: str):
return message[0:-ord(message[len(message) - 1])]
return message[0 : -ord(message[len(message) - 1])]

def enc(self, message: str):
cipher_iv = Random.new().read(AES.block_size)
cipher = AES.new(self._cipher_key, AES.MODE_CBC, cipher_iv)
return base64.b64encode(cipher_iv + cipher.encrypt(self._pad(message).encode())).decode()
return base64.b64encode(
cipher_iv + cipher.encrypt(self._pad(message).encode())
).decode()

def dec(self, enc_message):
enc_message = base64.b64decode(enc_message)
cipher_iv = enc_message[0:AES.block_size]
cipher_iv = enc_message[0 : AES.block_size]
cipher = AES.new(self._cipher_key, AES.MODE_CBC, cipher_iv)
return self._unpad(cipher.decrypt(enc_message[AES.block_size:]).decode())
return self._unpad(cipher.decrypt(enc_message[AES.block_size :]).decode())


def initialize_encryptor(data_dir):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once again, this is not data_dir. In the context of encryptor, this is where password is, so it's password_file_dir_path or something like that.

global _encryptor

_encryptor = Encryptor(data_dir)


encryptor = Encryptor()
def encryptor():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice application of singleton pattern, but I'd still prefer method names to be verbs. So maybe get_encryptor()?

return _encryptor
25 changes: 17 additions & 8 deletions monkey/monkey_island/cc/environment/environment_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from typing import Dict, List

import monkey_island.cc.environment.server_config_generator as server_config_generator
from monkey_island.cc.consts import DEFAULT_DATA_DIR
from monkey_island.cc.environment.user_creds import UserCreds
from monkey_island.cc.resources.auth.auth_user import User
from monkey_island.cc.resources.auth.user_store import UserStore
Expand All @@ -18,6 +19,7 @@ def __init__(self, file_path):
self.deployment = None
self.user_creds = None
self.aws = None
self.data_dir = None

self._load_from_file(self._server_config_path)

Expand All @@ -26,7 +28,7 @@ def _load_from_file(self, file_path):

if not Path(file_path).is_file():
server_config_generator.create_default_config_file(file_path)
with open(file_path, 'r') as f:
with open(file_path, "r") as f:
config_content = f.read()

self._load_from_json(config_content)
Expand All @@ -37,22 +39,29 @@ def _load_from_json(self, config_json: str) -> EnvironmentConfig:

def _load_from_dict(self, dict_data: Dict):
user_creds = UserCreds.get_from_dict(dict_data)
aws = dict_data['aws'] if 'aws' in dict_data else None
aws = dict_data["aws"] if "aws" in dict_data else None
data_dir = (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Var name should be more explicit if possible

dict_data["data_dir"] if "data_dir" in dict_data else DEFAULT_DATA_DIR
)

self.server_config = dict_data['server_config']
self.deployment = dict_data['deployment']
self.server_config = dict_data["server_config"]
self.deployment = dict_data["deployment"]
self.user_creds = user_creds
self.aws = aws
self.data_dir = data_dir

def save_to_file(self):
with open(self._server_config_path, 'w') as f:
with open(self._server_config_path, "w") as f:
f.write(json.dumps(self.to_dict(), indent=2))

def to_dict(self) -> Dict:
config_dict = {'server_config': self.server_config,
'deployment': self.deployment}
config_dict = {
"server_config": self.server_config,
"deployment": self.deployment,
"data_dir": self.data_dir,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same complaint, this should be more specific. In the context of environment config, this could be something like path_to_dir_of_config_files or something.

}
if self.aws:
config_dict.update({'aws': self.aws})
config_dict.update({"aws": self.aws})
config_dict.update(self.user_creds.to_dict())
return config_dict

Expand Down
3 changes: 3 additions & 0 deletions monkey/monkey_island/cc/environment/environment_singleton.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
}

env = None
config = None


def set_env(env_type: str, env_config: EnvironmentConfig):
Expand All @@ -41,6 +42,8 @@ def set_to_standard():


def initialize_from_file(file_path):
global config

try:
config = EnvironmentConfig(file_path)

Expand Down
23 changes: 17 additions & 6 deletions monkey/monkey_island/cc/environment/test_environment_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import pytest

from monkey_island.cc.consts import MONKEY_ISLAND_ABS_PATH
from monkey_island.cc.consts import DEFAULT_DATA_DIR, MONKEY_ISLAND_ABS_PATH
from monkey_island.cc.environment.environment_config import EnvironmentConfig
from monkey_island.cc.environment.user_creds import UserCreds

Expand All @@ -22,6 +22,7 @@
STANDARD_WITH_CREDENTIALS = os.path.join(
TEST_RESOURCES_DIR, "server_config_standard_with_credentials.json"
)
DATA_DIR = os.path.join(TEST_RESOURCES_DIR, "server_config_data_dir.json")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a path to a file server_config_data_dir.json. Name DATA_DIR implies that it's a directory full of some kind of data. Find a more specific name for this variable, that would convey what server_config_data_dir.json is, like PATH_TO_SOMETHING



@pytest.fixture
Expand All @@ -32,28 +33,31 @@ def config_file(tmpdir):
def test_get_with_credentials():
config_dict = EnvironmentConfig(WITH_CREDENTIALS).to_dict()

assert len(config_dict.keys()) == 4
assert len(config_dict.keys()) == 5
assert config_dict["server_config"] == "password"
assert config_dict["deployment"] == "develop"
assert config_dict["user"] == "test"
assert config_dict["password_hash"] == "abcdef"
assert config_dict["data_dir"] == DEFAULT_DATA_DIR


def test_get_with_no_credentials():
config_dict = EnvironmentConfig(NO_CREDENTIALS).to_dict()

assert len(config_dict.keys()) == 2
assert len(config_dict.keys()) == 3
assert config_dict["server_config"] == "password"
assert config_dict["deployment"] == "develop"
assert config_dict["data_dir"] == DEFAULT_DATA_DIR


def test_get_with_partial_credentials():
config_dict = EnvironmentConfig(PARTIAL_CREDENTIALS).to_dict()

assert len(config_dict.keys()) == 3
assert len(config_dict.keys()) == 4
assert config_dict["server_config"] == "password"
assert config_dict["deployment"] == "develop"
assert config_dict["user"] == "test"
assert config_dict["data_dir"] == DEFAULT_DATA_DIR


def test_save_to_file(config_file):
Expand All @@ -66,12 +70,13 @@ def test_save_to_file(config_file):
with open(config_file, "r") as f:
from_file = json.load(f)

assert len(from_file.keys()) == 5
assert len(from_file.keys()) == 6
assert from_file["server_config"] == "standard"
assert from_file["deployment"] == "develop"
assert from_file["user"] == "test"
assert from_file["password_hash"] == "abcdef"
assert from_file["aws"] == "test_aws"
assert from_file["data_dir"] == DEFAULT_DATA_DIR


def test_add_user(config_file):
Expand All @@ -87,7 +92,7 @@ def test_add_user(config_file):
with open(config_file, "r") as f:
from_file = json.load(f)

assert len(from_file.keys()) == 4
assert len(from_file.keys()) == 5
assert from_file["user"] == new_user
assert from_file["password_hash"] == new_password_hash

Expand All @@ -112,3 +117,9 @@ def test_generate_default_file(config_file):
assert environment_config.user_creds.username == ""
assert environment_config.user_creds.password_hash == ""
assert environment_config.aws is None
assert environment_config.data_dir == DEFAULT_DATA_DIR


def test_data_dir():
environment_config = EnvironmentConfig(DATA_DIR)
assert environment_config.data_dir == "/test/data/dir"
2 changes: 2 additions & 0 deletions monkey/monkey_island/cc/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
BootloaderHttpServer # noqa: E402
from monkey_island.cc.database import get_db_version # noqa: E402
from monkey_island.cc.database import is_db_server_up # noqa: E402
from monkey_island.cc.encryptor import initialize_encryptor # noqa: E402
from monkey_island.cc.network_utils import local_ip_addresses # noqa: E402
from monkey_island.cc.resources.monkey_download import \
MonkeyDownload # noqa: E402
Expand All @@ -37,6 +38,7 @@
def main(should_setup_only=False, server_config_filename=DEFAULT_SERVER_CONFIG_PATH):
logger.info("Starting bootloader server")
env_singleton.initialize_from_file(server_config_filename)
initialize_encryptor(env_singleton.config.data_dir)

mongo_url = os.environ.get('MONGO_URL', env_singleton.env.get_mongo_url())
bootloader_server_thread = Thread(target=BootloaderHttpServer(mongo_url).serve_forever, daemon=True)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def censor_password(password, plain_chars=3, secret_chars=5):
"""
if not password:
return ""
password = encryptor.dec(password)
password = encryptor().dec(password)
return password[0:plain_chars] + '*' * secret_chars


Expand All @@ -42,5 +42,5 @@ def censor_hash(hash_, plain_chars=5):
"""
if not hash_:
return ""
hash_ = encryptor.dec(hash_)
hash_ = encryptor().dec(hash_)
return hash_[0: plain_chars] + ' ...'
20 changes: 10 additions & 10 deletions monkey/monkey_island/cc/services/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def get_config_value(config_key_as_arr, is_initial_config=False, should_decrypt=
config = config[config_key_part]
if should_decrypt:
if config_key_as_arr in ENCRYPTED_CONFIG_ARRAYS:
config = [encryptor.dec(x) for x in config]
config = [encryptor().dec(x) for x in config]
return config

@staticmethod
Expand Down Expand Up @@ -103,7 +103,7 @@ def add_item_to_config_set_if_dont_exist(item_key, item_value, should_encrypt):
if item_value in items_from_config:
return
if should_encrypt:
item_value = encryptor.enc(item_value)
item_value = encryptor().enc(item_value)
mongo.db.config.update(
{'name': 'newconfig'},
{'$addToSet': {item_key: item_value}},
Expand Down Expand Up @@ -288,9 +288,9 @@ def decrypt_flat_config(flat_config, is_island=False):
if flat_config[key] and isinstance(flat_config[key][0], dict) and 'public_key' in flat_config[key][0]:
flat_config[key] = [ConfigService.decrypt_ssh_key_pair(item) for item in flat_config[key]]
else:
flat_config[key] = [encryptor.dec(item) for item in flat_config[key]]
flat_config[key] = [encryptor().dec(item) for item in flat_config[key]]
else:
flat_config[key] = encryptor.dec(flat_config[key])
flat_config[key] = encryptor().dec(flat_config[key])
return flat_config

@staticmethod
Expand All @@ -311,19 +311,19 @@ def _encrypt_or_decrypt_config(config, is_decrypt=False):
config_arr[i] = ConfigService.decrypt_ssh_key_pair(config_arr[i]) if is_decrypt else \
ConfigService.decrypt_ssh_key_pair(config_arr[i], True)
else:
config_arr[i] = encryptor.dec(config_arr[i]) if is_decrypt else encryptor.enc(config_arr[i])
config_arr[i] = encryptor().dec(config_arr[i]) if is_decrypt else encryptor().enc(config_arr[i])
else:
parent_config_arr[config_arr_as_array[-1]] = \
encryptor.dec(config_arr) if is_decrypt else encryptor.enc(config_arr)
encryptor().dec(config_arr) if is_decrypt else encryptor().enc(config_arr)

@staticmethod
def decrypt_ssh_key_pair(pair, encrypt=False):
if encrypt:
pair['public_key'] = encryptor.enc(pair['public_key'])
pair['private_key'] = encryptor.enc(pair['private_key'])
pair['public_key'] = encryptor().enc(pair['public_key'])
pair['private_key'] = encryptor().enc(pair['private_key'])
else:
pair['public_key'] = encryptor.dec(pair['public_key'])
pair['private_key'] = encryptor.dec(pair['private_key'])
pair['public_key'] = encryptor().dec(pair['public_key'])
pair['private_key'] = encryptor().dec(pair['private_key'])
return pair

@staticmethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,4 @@ def encrypt_exploit_creds(telemetry_json):
for field in ['password', 'lm_hash', 'ntlm_hash']:
credential = attempts[i][field]
if len(credential) > 0:
attempts[i][field] = encryptor.enc(credential)
attempts[i][field] = encryptor().enc(credential)
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,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] = encryptor.enc(ssh_info[idx][field])
ssh_info[idx][field] = encryptor().enc(ssh_info[idx][field])


def process_credential_info(telemetry_json):
Expand Down
37 changes: 37 additions & 0 deletions monkey/monkey_island/cc/test_encryptor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import os

from monkey_island.cc.consts import MONKEY_ISLAND_ABS_PATH
from monkey_island.cc.encryptor import initialize_encryptor, encryptor


TEST_DATA_DIR = os.path.join(MONKEY_ISLAND_ABS_PATH, "cc", "testing")
PASSWORD_FILENAME = "mongo_key.bin"

PLAINTEXT = "Hello, Monkey!"
CYPHERTEXT = "vKgvD6SjRyIh1dh2AM/rnTa0NI/vjfwnbZLbMocWtE4e42WJmSUz2ordtbQrH1Fq"


def test_aes_cbc_encryption():
initialize_encryptor(TEST_DATA_DIR)

assert encryptor().enc(PLAINTEXT) != PLAINTEXT


def test_aes_cbc_decryption():
initialize_encryptor(TEST_DATA_DIR)

assert encryptor().dec(CYPHERTEXT) == PLAINTEXT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think more important than both of these cases would be to test assert encryptor().dec(encryptor().enc(PLAINTEXT)) == PLAINTEXT. Currently, if encryption fails, but still generates something your test would pass.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I waffled back and forth on this. I'll add it.



def test_create_new_password_file(tmpdir):
initialize_encryptor(tmpdir)

assert os.path.isfile(os.path.join(tmpdir, PASSWORD_FILENAME))


def test_expand_home(monkeypatch, tmpdir):
monkeypatch.setenv("HOME", str(tmpdir))

initialize_encryptor("~/")

assert os.path.isfile(os.path.join(tmpdir, "mongo_key.bin"))
Loading