From c9a53833e2b25fc638e752ed9c2365dfc658c17c Mon Sep 17 00:00:00 2001 From: Shreya Date: Wed, 2 Jun 2021 19:39:47 +0530 Subject: [PATCH 01/58] Add support for custom certificate (partially) --- monkey/common/utils/exceptions.py | 4 ++++ monkey/monkey_island/cc/server_setup.py | 5 ++-- .../monkey_island/cc/server_utils/consts.py | 3 +++ .../cc/setup/certificate/certificate_setup.py | 23 +++++++++++++++++++ .../cc/setup/island_config_options.py | 6 +++++ 5 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 monkey/monkey_island/cc/setup/certificate/certificate_setup.py diff --git a/monkey/common/utils/exceptions.py b/monkey/common/utils/exceptions.py index 8396b423b..632c08991 100644 --- a/monkey/common/utils/exceptions.py +++ b/monkey/common/utils/exceptions.py @@ -52,3 +52,7 @@ class FindingWithoutDetailsError(Exception): class DomainControllerNameFetchError(FailedExploitationError): """ Raise on failed attempt to extract domain controller's name """ + + +class InsecurePermissionsError(Exception): + """ Raise when a file does not have permissions that are secure enough """ diff --git a/monkey/monkey_island/cc/server_setup.py b/monkey/monkey_island/cc/server_setup.py index 4eaa13131..faec1ec96 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -22,13 +22,13 @@ from monkey_island.cc.arg_parser import IslandCmdArgs # noqa: E402 from monkey_island.cc.arg_parser import parse_cli_args # noqa: E402 from monkey_island.cc.resources.monkey_download import MonkeyDownload # noqa: E402 from monkey_island.cc.server_utils.bootloader_server import BootloaderHttpServer # noqa: E402 -from monkey_island.cc.server_utils.consts import MONKEY_ISLAND_ABS_PATH # noqa: E402 from monkey_island.cc.server_utils.encryptor import initialize_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 from monkey_island.cc.services.utils.network_utils import local_ip_addresses # noqa: E402 from monkey_island.cc.setup.island_config_options import IslandConfigOptions # noqa: E402 +from monkey_island.cc.setup.certificate.certificate_setup import setup_certificate # noqa: E402 from monkey_island.cc.setup.mongo.database_initializer import init_collections # noqa: E402 from monkey_island.cc.setup.mongo.mongo_setup import ( # noqa: E402 MONGO_URL, @@ -83,8 +83,7 @@ def _start_island_server(should_setup_only, config_options: IslandConfigOptions) populate_exporter_list() app = init_app(MONGO_URL) - crt_path = str(Path(MONKEY_ISLAND_ABS_PATH, "cc", "server.crt")) - key_path = str(Path(MONKEY_ISLAND_ABS_PATH, "cc", "server.key")) + crt_path, key_path = setup_certificate(config_options) init_collections() diff --git a/monkey/monkey_island/cc/server_utils/consts.py b/monkey/monkey_island/cc/server_utils/consts.py index a14c69d0b..2a50e01aa 100644 --- a/monkey/monkey_island/cc/server_utils/consts.py +++ b/monkey/monkey_island/cc/server_utils/consts.py @@ -46,3 +46,6 @@ DEFAULT_DEVELOP_SERVER_CONFIG_PATH = os.path.join( DEFAULT_LOG_LEVEL = "INFO" DEFAULT_START_MONGO_DB = True + +DEFAULT_CRT_PATH = str(Path(MONKEY_ISLAND_ABS_PATH, "cc", "server.crt")) +DEFAULT_KEY_PATH = str(Path(MONKEY_ISLAND_ABS_PATH, "cc", "server.key")) diff --git a/monkey/monkey_island/cc/setup/certificate/certificate_setup.py b/monkey/monkey_island/cc/setup/certificate/certificate_setup.py new file mode 100644 index 000000000..959025e03 --- /dev/null +++ b/monkey/monkey_island/cc/setup/certificate/certificate_setup.py @@ -0,0 +1,23 @@ +import os + +from common.utils.exceptions import InsecurePermissionsError +from monkey_island.setup.island_config_options import IslandConfigOptions + + +def setup_certificate(config_options: IslandConfigOptions) -> (str, str): + crt_path = config_options.crt_path + key_path = config_options.key_path + + # check paths + for file in [crt_path, key_path]: + if not os.path.exists(file): + raise FileNotFoundError(f"File not found at {file}. Exiting.") + + if not has_sufficient_permissions(file): + raise InsecurePermissionsError(f"{file} has insecure permissions. Exiting.") + + return crt_path, key_path + + +def has_sufficient_permissions(): + pass diff --git a/monkey/monkey_island/cc/setup/island_config_options.py b/monkey/monkey_island/cc/setup/island_config_options.py index 5ce62ba2e..0df903587 100644 --- a/monkey/monkey_island/cc/setup/island_config_options.py +++ b/monkey/monkey_island/cc/setup/island_config_options.py @@ -3,7 +3,9 @@ from __future__ import annotations import os from monkey_island.cc.server_utils.consts import ( + DEFAULT_CRT_PATH, DEFAULT_DATA_DIR, + DEFAULT_KEY_PATH, DEFAULT_LOG_LEVEL, DEFAULT_START_MONGO_DB, ) @@ -14,8 +16,12 @@ class IslandConfigOptions: self.data_dir = os.path.expandvars( os.path.expanduser(config_contents.get("data_dir", DEFAULT_DATA_DIR)) ) + self.log_level = config_contents.get("log_level", DEFAULT_LOG_LEVEL) self.start_mongodb = config_contents.get( "mongodb", {"start_mongodb": DEFAULT_START_MONGO_DB} ).get("start_mongodb", DEFAULT_START_MONGO_DB) + + self.crt_path = config_contents.get("cert_path", DEFAULT_CRT_PATH) + self.key_path = config_contents.get("cert_path", DEFAULT_KEY_PATH) From c1463b4a1814c668454ed9401d75482f9bbc7885 Mon Sep 17 00:00:00 2001 From: Shreya Date: Thu, 3 Jun 2021 16:11:33 +0530 Subject: [PATCH 02/58] Implement `has_sufficient_permissions` function for checking certificate files --- .../cc/setup/certificate/certificate_setup.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/monkey/monkey_island/cc/setup/certificate/certificate_setup.py b/monkey/monkey_island/cc/setup/certificate/certificate_setup.py index 959025e03..2cb2f1e03 100644 --- a/monkey/monkey_island/cc/setup/certificate/certificate_setup.py +++ b/monkey/monkey_island/cc/setup/certificate/certificate_setup.py @@ -14,10 +14,17 @@ def setup_certificate(config_options: IslandConfigOptions) -> (str, str): raise FileNotFoundError(f"File not found at {file}. Exiting.") if not has_sufficient_permissions(file): - raise InsecurePermissionsError(f"{file} has insecure permissions. Exiting.") + raise InsecurePermissionsError( + f"{file} has insecure permissions. Required permissions: r--------. Exiting." + ) return crt_path, key_path -def has_sufficient_permissions(): - pass +def has_sufficient_permissions(path: str) -> bool: + required_permissions = "0o400" + + file_mode = os.stat(path).st_mode + file_permissions = oct(file_mode & 0o777) + + return file_permissions == required_permissions From 6f1154f91178fbfe9e714b087741a1bbf6a9dcd3 Mon Sep 17 00:00:00 2001 From: Shreya Date: Thu, 3 Jun 2021 16:17:47 +0530 Subject: [PATCH 03/58] Add log message for which certificate is being used --- .../monkey_island/cc/setup/certificate/certificate_setup.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/monkey/monkey_island/cc/setup/certificate/certificate_setup.py b/monkey/monkey_island/cc/setup/certificate/certificate_setup.py index 2cb2f1e03..692dd5aa9 100644 --- a/monkey/monkey_island/cc/setup/certificate/certificate_setup.py +++ b/monkey/monkey_island/cc/setup/certificate/certificate_setup.py @@ -1,8 +1,11 @@ +import logging import os from common.utils.exceptions import InsecurePermissionsError from monkey_island.setup.island_config_options import IslandConfigOptions +logger = logging.getLogger(__name__) + def setup_certificate(config_options: IslandConfigOptions) -> (str, str): crt_path = config_options.crt_path @@ -18,6 +21,8 @@ def setup_certificate(config_options: IslandConfigOptions) -> (str, str): f"{file} has insecure permissions. Required permissions: r--------. Exiting." ) + logger.INFO(f"Using certificate path: {crt_path}, and key path: {key_path}.") + return crt_path, key_path From a2bd59c3772f974b420b3145704978eb2f62829f Mon Sep 17 00:00:00 2001 From: Shreya Date: Thu, 3 Jun 2021 18:38:13 +0530 Subject: [PATCH 04/58] Move `has_sufficient_permissions` to a separate file in utils/ --- .../cc/services/utils/file_permissions.py | 8 ++++++++ .../cc/setup/certificate/certificate_setup.py | 12 ++---------- 2 files changed, 10 insertions(+), 10 deletions(-) create mode 100644 monkey/monkey_island/cc/services/utils/file_permissions.py diff --git a/monkey/monkey_island/cc/services/utils/file_permissions.py b/monkey/monkey_island/cc/services/utils/file_permissions.py new file mode 100644 index 000000000..05587ad09 --- /dev/null +++ b/monkey/monkey_island/cc/services/utils/file_permissions.py @@ -0,0 +1,8 @@ +import os + + +def has_sufficient_permissions(path: str, required_permissions: str) -> bool: + file_mode = os.stat(path).st_mode + file_permissions = oct(file_mode & 0o777) + + return file_permissions == required_permissions diff --git a/monkey/monkey_island/cc/setup/certificate/certificate_setup.py b/monkey/monkey_island/cc/setup/certificate/certificate_setup.py index 692dd5aa9..0ae7535e0 100644 --- a/monkey/monkey_island/cc/setup/certificate/certificate_setup.py +++ b/monkey/monkey_island/cc/setup/certificate/certificate_setup.py @@ -2,6 +2,7 @@ import logging import os from common.utils.exceptions import InsecurePermissionsError +from monkey_island.cc.services.utils.file_permissions import has_sufficient_permissions from monkey_island.setup.island_config_options import IslandConfigOptions logger = logging.getLogger(__name__) @@ -16,7 +17,7 @@ def setup_certificate(config_options: IslandConfigOptions) -> (str, str): if not os.path.exists(file): raise FileNotFoundError(f"File not found at {file}. Exiting.") - if not has_sufficient_permissions(file): + if not has_sufficient_permissions(path=file, required_permissions="0o400"): raise InsecurePermissionsError( f"{file} has insecure permissions. Required permissions: r--------. Exiting." ) @@ -24,12 +25,3 @@ def setup_certificate(config_options: IslandConfigOptions) -> (str, str): logger.INFO(f"Using certificate path: {crt_path}, and key path: {key_path}.") return crt_path, key_path - - -def has_sufficient_permissions(path: str) -> bool: - required_permissions = "0o400" - - file_mode = os.stat(path).st_mode - file_permissions = oct(file_mode & 0o777) - - return file_permissions == required_permissions From 88ae762618aca37a1df79c575fadfa034f1caa88 Mon Sep 17 00:00:00 2001 From: Shreya Date: Fri, 4 Jun 2021 12:17:21 +0530 Subject: [PATCH 05/58] Expand cert and key path in IslandConfigOptions --- monkey/monkey_island/cc/server_utils/consts.py | 1 + monkey/monkey_island/cc/setup/island_config_options.py | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/consts.py b/monkey/monkey_island/cc/server_utils/consts.py index 2a50e01aa..333008c6d 100644 --- a/monkey/monkey_island/cc/server_utils/consts.py +++ b/monkey/monkey_island/cc/server_utils/consts.py @@ -45,6 +45,7 @@ DEFAULT_DEVELOP_SERVER_CONFIG_PATH = os.path.join( ) DEFAULT_LOG_LEVEL = "INFO" + DEFAULT_START_MONGO_DB = True DEFAULT_CRT_PATH = str(Path(MONKEY_ISLAND_ABS_PATH, "cc", "server.crt")) diff --git a/monkey/monkey_island/cc/setup/island_config_options.py b/monkey/monkey_island/cc/setup/island_config_options.py index 0df903587..e8a0f7016 100644 --- a/monkey/monkey_island/cc/setup/island_config_options.py +++ b/monkey/monkey_island/cc/setup/island_config_options.py @@ -23,5 +23,9 @@ class IslandConfigOptions: "mongodb", {"start_mongodb": DEFAULT_START_MONGO_DB} ).get("start_mongodb", DEFAULT_START_MONGO_DB) - self.crt_path = config_contents.get("cert_path", DEFAULT_CRT_PATH) - self.key_path = config_contents.get("cert_path", DEFAULT_KEY_PATH) + self.crt_path = os.path.expandvars( + os.path.expanduser(config_contents.get("cert_path", DEFAULT_CRT_PATH)) + ) + self.key_path = os.path.expandvars( + os.path.expanduser(config_contents.get("key_path", DEFAULT_KEY_PATH)) + ) From d740173f79ec73bf71ed555b68ee2e764fb3f0dc Mon Sep 17 00:00:00 2001 From: Shreya Date: Fri, 4 Jun 2021 13:20:01 +0530 Subject: [PATCH 06/58] Post-rebase fixes --- .../monkey_island/cc/setup/certificate/certificate_setup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/monkey/monkey_island/cc/setup/certificate/certificate_setup.py b/monkey/monkey_island/cc/setup/certificate/certificate_setup.py index 0ae7535e0..8363b675d 100644 --- a/monkey/monkey_island/cc/setup/certificate/certificate_setup.py +++ b/monkey/monkey_island/cc/setup/certificate/certificate_setup.py @@ -3,7 +3,7 @@ import os from common.utils.exceptions import InsecurePermissionsError from monkey_island.cc.services.utils.file_permissions import has_sufficient_permissions -from monkey_island.setup.island_config_options import IslandConfigOptions +from monkey_island.cc.setup.island_config_options import IslandConfigOptions logger = logging.getLogger(__name__) @@ -22,6 +22,6 @@ def setup_certificate(config_options: IslandConfigOptions) -> (str, str): f"{file} has insecure permissions. Required permissions: r--------. Exiting." ) - logger.INFO(f"Using certificate path: {crt_path}, and key path: {key_path}.") + logger.info(f"Using certificate path: {crt_path}, and key path: {key_path}.") return crt_path, key_path From 53a126482f3a6491529128ed1eb016428d74ad5b Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 7 Jun 2021 13:14:31 +0530 Subject: [PATCH 07/58] Extract file checking activities --- .../cc/services/utils/file_handling.py | 22 +++++++++++++++++++ .../cc/services/utils/file_permissions.py | 8 ------- .../cc/setup/certificate/certificate_setup.py | 17 +++++--------- 3 files changed, 28 insertions(+), 19 deletions(-) create mode 100644 monkey/monkey_island/cc/services/utils/file_handling.py delete mode 100644 monkey/monkey_island/cc/services/utils/file_permissions.py diff --git a/monkey/monkey_island/cc/services/utils/file_handling.py b/monkey/monkey_island/cc/services/utils/file_handling.py new file mode 100644 index 000000000..0715e6efe --- /dev/null +++ b/monkey/monkey_island/cc/services/utils/file_handling.py @@ -0,0 +1,22 @@ +import os + +from common.utils.exceptions import InsecurePermissionsError + + +def ensure_file_existence(file: str) -> None: + if not os.path.exists(file): + raise FileNotFoundError(f"File not found at {file}. Exiting.") + + +def ensure_file_permissions(file: str) -> None: + if not file_has_sufficient_permissions(path=file, required_permissions="0o400"): + raise InsecurePermissionsError( + f"{file} has insecure permissions. Required permissions: r--------. Exiting." + ) + + +def file_has_sufficient_permissions(path: str, required_permissions: str) -> bool: + file_mode = os.stat(path).st_mode + file_permissions = oct(file_mode & 0o777) + + return file_permissions == required_permissions diff --git a/monkey/monkey_island/cc/services/utils/file_permissions.py b/monkey/monkey_island/cc/services/utils/file_permissions.py deleted file mode 100644 index 05587ad09..000000000 --- a/monkey/monkey_island/cc/services/utils/file_permissions.py +++ /dev/null @@ -1,8 +0,0 @@ -import os - - -def has_sufficient_permissions(path: str, required_permissions: str) -> bool: - file_mode = os.stat(path).st_mode - file_permissions = oct(file_mode & 0o777) - - return file_permissions == required_permissions diff --git a/monkey/monkey_island/cc/setup/certificate/certificate_setup.py b/monkey/monkey_island/cc/setup/certificate/certificate_setup.py index 8363b675d..1eaab4f04 100644 --- a/monkey/monkey_island/cc/setup/certificate/certificate_setup.py +++ b/monkey/monkey_island/cc/setup/certificate/certificate_setup.py @@ -1,8 +1,9 @@ import logging -import os -from common.utils.exceptions import InsecurePermissionsError -from monkey_island.cc.services.utils.file_permissions import has_sufficient_permissions +from monkey_island.cc.services.utils.file_handling import ( + ensure_file_existence, + ensure_file_permissions, +) from monkey_island.cc.setup.island_config_options import IslandConfigOptions logger = logging.getLogger(__name__) @@ -12,15 +13,9 @@ def setup_certificate(config_options: IslandConfigOptions) -> (str, str): crt_path = config_options.crt_path key_path = config_options.key_path - # check paths for file in [crt_path, key_path]: - if not os.path.exists(file): - raise FileNotFoundError(f"File not found at {file}. Exiting.") - - if not has_sufficient_permissions(path=file, required_permissions="0o400"): - raise InsecurePermissionsError( - f"{file} has insecure permissions. Required permissions: r--------. Exiting." - ) + ensure_file_existence(file) + ensure_file_permissions(file) logger.info(f"Using certificate path: {crt_path}, and key path: {key_path}.") From 4ad49d19c75bc7661fe41a8acf1fe36b3b1ecbdb Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 7 Jun 2021 13:21:51 +0530 Subject: [PATCH 08/58] Rename "required" permissions to "expected" permissions --- monkey/monkey_island/cc/services/utils/file_handling.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/monkey/monkey_island/cc/services/utils/file_handling.py b/monkey/monkey_island/cc/services/utils/file_handling.py index 0715e6efe..a96e8baac 100644 --- a/monkey/monkey_island/cc/services/utils/file_handling.py +++ b/monkey/monkey_island/cc/services/utils/file_handling.py @@ -9,14 +9,14 @@ def ensure_file_existence(file: str) -> None: def ensure_file_permissions(file: str) -> None: - if not file_has_sufficient_permissions(path=file, required_permissions="0o400"): + if not file_has_expected_permissions(path=file, expected_permissions="0o400"): raise InsecurePermissionsError( f"{file} has insecure permissions. Required permissions: r--------. Exiting." ) -def file_has_sufficient_permissions(path: str, required_permissions: str) -> bool: +def file_has_expected_permissions(path: str, expected_permissions: str) -> bool: file_mode = os.stat(path).st_mode file_permissions = oct(file_mode & 0o777) - return file_permissions == required_permissions + return file_permissions == expected_permissions From 5ba8effe1a45f744251fb7661ef0938bd6e5ce55 Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 7 Jun 2021 13:26:14 +0530 Subject: [PATCH 09/58] Use octal representation for permissions --- monkey/monkey_island/cc/services/utils/file_handling.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monkey/monkey_island/cc/services/utils/file_handling.py b/monkey/monkey_island/cc/services/utils/file_handling.py index a96e8baac..114405647 100644 --- a/monkey/monkey_island/cc/services/utils/file_handling.py +++ b/monkey/monkey_island/cc/services/utils/file_handling.py @@ -11,7 +11,7 @@ def ensure_file_existence(file: str) -> None: def ensure_file_permissions(file: str) -> None: if not file_has_expected_permissions(path=file, expected_permissions="0o400"): raise InsecurePermissionsError( - f"{file} has insecure permissions. Required permissions: r--------. Exiting." + f"{file} has insecure permissions. Required permissions: 400. Exiting." ) From 8c1e76ffbed8b1a1389ee099452b93866bed69d2 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 09:54:09 -0400 Subject: [PATCH 10/58] docs: Reword docker supported operating systems --- docs/content/setup/docker.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/content/setup/docker.md b/docs/content/setup/docker.md index 7cee1e6fe..341969ed6 100644 --- a/docs/content/setup/docker.md +++ b/docs/content/setup/docker.md @@ -7,6 +7,10 @@ weight: 4 tags: ["setup", "docker", "linux", "windows"] --- +## Supported operating systems + +The Infection Monkey Docker container works on Linux only. It is not compatible with Docker for Windows or Docker for Mac. + ## Deployment ### Linux @@ -25,10 +29,6 @@ sudo docker run --name monkey-island --network=host -d guardicore/monkey-island: Wait until the Island is done setting up and it will be available on https://localhost:5000 -### Windows and Mac OS X - -Not supported yet, since docker doesn't support `--network=host` parameter on these OS's. - ## Upgrading Currently, there's no "upgrade-in-place" option when a new version is released. From 16ed2e59e8078c6245de620fbe8a3002561265f7 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 09:56:02 -0400 Subject: [PATCH 11/58] docs: Add steps for user-provided certificate for docker container --- docs/content/setup/docker.md | 119 +++++++++++++++++++++++++++++++---- 1 file changed, 108 insertions(+), 11 deletions(-) diff --git a/docs/content/setup/docker.md b/docs/content/setup/docker.md index 341969ed6..50b195932 100644 --- a/docs/content/setup/docker.md +++ b/docs/content/setup/docker.md @@ -13,21 +13,118 @@ The Infection Monkey Docker container works on Linux only. It is not compatible ## Deployment -### Linux +### 1. Load the docker images +1. Pull the MongoDB v4.2 Docker image: -To extract the `tar.gz` file, run `tar -xvzf monkey-island-docker.tar.gz`. + ```bash + sudo docker pull mongo:4.2 + ``` -Once you've extracted the container from the tar.gz file, run the following commands: +1. Extract the Monkey Island Docker tarball: -```sh -sudo docker load -i dk.monkeyisland.1.10.0.tar -sudo docker pull mongo:4.2 -sudo mkdir -p /var/monkey-mongo/data/db -sudo docker run --name monkey-mongo --network=host -v /var/monkey-mongo/data/db:/data/db -d mongo:4.2 -sudo docker run --name monkey-island --network=host -d guardicore/monkey-island:1.10.0 -``` + ```bash + tar -xvzf monkey-island-docker.tar.gz + ``` -Wait until the Island is done setting up and it will be available on https://localhost:5000 +1. Load the Monkey Island Docker image: + + ```bash + sudo docker load -i dk.monkeyisland.1.10.0.tar + ``` + +### 2. Start MongoDB + +1. Start a MongoDB Docker container: + + ```bash + sudo docker run \ + --name monkey-mongo \ + --network=host \ + --volume db:/data/db \ + --detach mongo:4.2 + ``` + +### 3a. Start Monkey Island with default certificate + +By default, Infection Monkey comes with a [self-signed SSL certificate](https://aboutssl.org/what-is-self-sign-certificate/). In +enterprise or other security-sensitive environments, it is recommended that the +user [provide Infection Monkey with a +certificate](#3b-start-monkey-island-with-user-provided-certificate) that has +been signed by a private certificate authority. + +1. Run the Monkey Island server + ```bash + sudo docker run \ + --name monkey-island \ + --network=host \ + guardicore/monkey-island:1.10.0 + ``` + +### 3b. Start Monkey Island with User-Provided Certificate + +1. Create a directory named `monkey_island_data`. This will serve as the + location where Infection Monkey stores its configuration and runtime + artifacts. + + ```bash + mkdir ./monkey_island_data + ``` + +1. Run Monkey Island with the `--setup-only` flag to populate the `./monkey_island_data` directory with a default `server_config.json` file. + + ```bash + sudo docker run \ + --rm \ + --name monkey-island \ + --network=host \ + --user $(id -u ${USER}):$(id -g ${USER}) \ + --volume "$(realpath ./monkey_island_data)":/monkey_island_data \ + guardicore/monkey-island:1.10.0 --setup-only + ``` + +1. (Optional but recommended) Copy your `.crt` and `.key` files to `./monkey_island_data`. + +1. Make sure that your `.crt` and `.key` files are read-only and readable only by you. + + ```bash + chmod 400 ./monkey_island_data/{*.key,*.crt} + ``` + +1. Edit `./monkey_island_data/server_config.json` to configure Monkey Island + to use your certificate. Your config should look something like this: + + ```json {linenos=inline,hl_lines=["11-14"]} + { + "data_dir": "/monkey_island_data", + "log_level": "DEBUG", + "environment": { + "server_config": "password", + "deployment": "docker" + }, + "mongodb": { + "start_mongodb": false + }, + "ssl_certificate": { + "ssl_certificate_file": "", + "ssl_certificate_key_file": "", + } + } + ``` + +1. Start the Monkey Island server: + + ```bash + sudo docker run \ + --name monkey-island \ + --network=host \ + --user $(id -u ${USER}):$(id -g ${USER}) \ + --volume "$(realpath ./monkey_island_data)":/monkey_island_data \ + guardicore/monkey-island:1.10.0 + ``` + +### 4. Accessing Monkey Island + +After the Monkey Island docker container starts, you can access Monkey Island by pointing your browser at `https://localhost:5000`. ## Upgrading From 6c04124303b402a87109fdd7f1c09f1fdf8854c9 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 09:56:52 -0400 Subject: [PATCH 12/58] docs: Add `--volume` workaround to docker troubleshooting Resolves #1032 --- docs/content/setup/docker.md | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/docs/content/setup/docker.md b/docs/content/setup/docker.md index 50b195932..e325de002 100644 --- a/docs/content/setup/docker.md +++ b/docs/content/setup/docker.md @@ -140,12 +140,27 @@ using the *Export config* button and then import it to the new Monkey Island. ## Troubleshooting ### The Monkey Island container crashes due to a 'UnicodeDecodeError' -`UnicodeDecodeError: 'utf-8' codec can't decode byte 0xee in position 0: invalid continuation byte` -You may encounter this error because of the existence of different MongoDB keys in the `monkey-island` and `monkey-mongo` containers. +You will encounter a `UnicodeDecodeError` if the `monkey-island` container is +using a different secret key to encrypt sensitive data than was initially used +to store data in the `monkey-mongo` container. -Starting a new container from the `guardicore/monkey-island:1.10.0` image generates a new secret key for storing sensitive information in MongoDB. If you have an old database instance running (from a previous run of Monkey), the key in the `monkey-mongo` container is different than the newly generated key in the `monkey-island` container. Since encrypted data (obtained from the previous run) is stored in MongoDB with the old key, decryption fails and you get this error. +``` +UnicodeDecodeError: 'utf-8' codec can't decode byte 0xee in position 0: invalid continuation byte +``` -You can fix this in two ways: +Starting a new container from the `guardicore/monkey-island:1.10.0` image +generates a new secret key for storing sensitive information in MongoDB. If you +have an old database instance running (from a previous instance of Infection +Monkey), the data stored in the `monkey-mongo` container has been encrypted +with a key that is different from the one that Monkey Island is currently +using. When MongoDB attempts to decrypt its data with the new key, decryption +fails and you get this error. + +You can fix this in one of three ways: 1. Instead of starting a new container for the Monkey Island, you can run `docker container start -a monkey-island` to restart the existing container, which will contain the correct key material. -2. Kill and remove the existing MongoDB container, and start a new one. This will remove the old database entirely. Then, start the new Monkey Island container. +1. Kill and remove the existing MongoDB container, and start a new one. This will remove the old database entirely. Then, start the new Monkey Island container. +1. When you start the Monkey Island container, use `--volume + monkey_island_data:/monkey_island_data`. This will store all of Monkey + Island's runtime artifacts (including the encryption key file) in a docker + volume that can be reused by subsequent Monkey Island containers. From 82c3273e698e3f3dc48fc41bc5d1bae9d1477ec8 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 10:37:06 -0400 Subject: [PATCH 13/58] docs: Fix minor capitalization issue in docker setup --- docs/content/setup/docker.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/setup/docker.md b/docs/content/setup/docker.md index e325de002..4d05b9b1a 100644 --- a/docs/content/setup/docker.md +++ b/docs/content/setup/docker.md @@ -60,7 +60,7 @@ been signed by a private certificate authority. guardicore/monkey-island:1.10.0 ``` -### 3b. Start Monkey Island with User-Provided Certificate +### 3b. Start Monkey Island with user-provided certificate 1. Create a directory named `monkey_island_data`. This will serve as the location where Infection Monkey stores its configuration and runtime From 227039f30c18dba82993ed6a3bd3bbc7da2907c5 Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 7 Jun 2021 19:44:42 +0530 Subject: [PATCH 14/58] Add `_expand_path()` to wrap `os.path.expandvars()\' and `os.path.expanduser()\' --- .../cc/setup/island_config_options.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/monkey/monkey_island/cc/setup/island_config_options.py b/monkey/monkey_island/cc/setup/island_config_options.py index e8a0f7016..664159944 100644 --- a/monkey/monkey_island/cc/setup/island_config_options.py +++ b/monkey/monkey_island/cc/setup/island_config_options.py @@ -13,8 +13,8 @@ from monkey_island.cc.server_utils.consts import ( class IslandConfigOptions: def __init__(self, config_contents: dict): - self.data_dir = os.path.expandvars( - os.path.expanduser(config_contents.get("data_dir", DEFAULT_DATA_DIR)) + self.data_dir = IslandConfigOptions._expand_path( + config_contents.get("data_dir", DEFAULT_DATA_DIR) ) self.log_level = config_contents.get("log_level", DEFAULT_LOG_LEVEL) @@ -23,9 +23,13 @@ class IslandConfigOptions: "mongodb", {"start_mongodb": DEFAULT_START_MONGO_DB} ).get("start_mongodb", DEFAULT_START_MONGO_DB) - self.crt_path = os.path.expandvars( - os.path.expanduser(config_contents.get("cert_path", DEFAULT_CRT_PATH)) + self.crt_path = IslandConfigOptions._expand_path( + config_contents.get("cert_path", DEFAULT_CRT_PATH) ) - self.key_path = os.path.expandvars( - os.path.expanduser(config_contents.get("key_path", DEFAULT_KEY_PATH)) + self.key_path = IslandConfigOptions._expand_path( + config_contents.get("key_path", DEFAULT_KEY_PATH) ) + + @staticmethod + def _expand_path(path: str) -> str: + return os.path.expandvars(os.path.expanduser(path)) From 2b73ec75c8c57ed2716410773c881c157a429f05 Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 7 Jun 2021 19:59:09 +0530 Subject: [PATCH 15/58] Move monkey_island/cc/setup/certificate/certificate_setup.py to monkey/monkey_island/cc/setup/certificate_setup.py --- monkey/monkey_island/cc/server_setup.py | 2 +- .../cc/setup/{certificate => }/certificate_setup.py | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename monkey/monkey_island/cc/setup/{certificate => }/certificate_setup.py (100%) diff --git a/monkey/monkey_island/cc/server_setup.py b/monkey/monkey_island/cc/server_setup.py index faec1ec96..61c6d19c8 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -27,8 +27,8 @@ from monkey_island.cc.server_utils.island_logger import reset_logger, setup_logg 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 from monkey_island.cc.services.utils.network_utils import local_ip_addresses # noqa: E402 +from monkey_island.cc.setup.certificate_setup import setup_certificate # noqa: E402 from monkey_island.cc.setup.island_config_options import IslandConfigOptions # noqa: E402 -from monkey_island.cc.setup.certificate.certificate_setup import setup_certificate # noqa: E402 from monkey_island.cc.setup.mongo.database_initializer import init_collections # noqa: E402 from monkey_island.cc.setup.mongo.mongo_setup import ( # noqa: E402 MONGO_URL, diff --git a/monkey/monkey_island/cc/setup/certificate/certificate_setup.py b/monkey/monkey_island/cc/setup/certificate_setup.py similarity index 100% rename from monkey/monkey_island/cc/setup/certificate/certificate_setup.py rename to monkey/monkey_island/cc/setup/certificate_setup.py From 42a9a798006d2709c24e881961b1c8e3b2c2d01d Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 7 Jun 2021 20:07:07 +0530 Subject: [PATCH 16/58] Modify server_config.json ssl cert fields --- monkey/monkey_island/cc/server_setup.py | 1 + monkey/monkey_island/cc/server_utils/consts.py | 5 +++++ monkey/monkey_island/cc/setup/island_config_options.py | 9 +++++++-- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/monkey/monkey_island/cc/server_setup.py b/monkey/monkey_island/cc/server_setup.py index 61c6d19c8..fbd61982a 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -99,6 +99,7 @@ def _start_island_server(should_setup_only, config_options: IslandConfigOptions) http_server = WSGIServer( ("0.0.0.0", env_singleton.env.get_island_port()), app, + # TODO: modify next two lines? certfile=os.environ.get("SERVER_CRT", crt_path), keyfile=os.environ.get("SERVER_KEY", key_path), ) diff --git a/monkey/monkey_island/cc/server_utils/consts.py b/monkey/monkey_island/cc/server_utils/consts.py index 333008c6d..9149d81b1 100644 --- a/monkey/monkey_island/cc/server_utils/consts.py +++ b/monkey/monkey_island/cc/server_utils/consts.py @@ -50,3 +50,8 @@ DEFAULT_START_MONGO_DB = True DEFAULT_CRT_PATH = str(Path(MONKEY_ISLAND_ABS_PATH, "cc", "server.crt")) DEFAULT_KEY_PATH = str(Path(MONKEY_ISLAND_ABS_PATH, "cc", "server.key")) + +DEFAULT_CERTIFICATE_PATHS = { + "ssl_certificate_file": DEFAULT_CRT_PATH, + "ssl_certificate_key_file": DEFAULT_KEY_PATH, +} diff --git a/monkey/monkey_island/cc/setup/island_config_options.py b/monkey/monkey_island/cc/setup/island_config_options.py index 664159944..fb94e6396 100644 --- a/monkey/monkey_island/cc/setup/island_config_options.py +++ b/monkey/monkey_island/cc/setup/island_config_options.py @@ -3,6 +3,7 @@ from __future__ import annotations import os from monkey_island.cc.server_utils.consts import ( + DEFAULT_CERTIFICATE_PATHS, DEFAULT_CRT_PATH, DEFAULT_DATA_DIR, DEFAULT_KEY_PATH, @@ -24,10 +25,14 @@ class IslandConfigOptions: ).get("start_mongodb", DEFAULT_START_MONGO_DB) self.crt_path = IslandConfigOptions._expand_path( - config_contents.get("cert_path", DEFAULT_CRT_PATH) + config_contents.get("ssl_certificate", DEFAULT_CERTIFICATE_PATHS).get( + "ssl_certificate_file", DEFAULT_CRT_PATH + ) ) self.key_path = IslandConfigOptions._expand_path( - config_contents.get("key_path", DEFAULT_KEY_PATH) + config_contents.get("ssl_certificate", DEFAULT_CERTIFICATE_PATHS).get( + "ssl_certificate_key_file", DEFAULT_KEY_PATH + ) ) @staticmethod From 4f601ca5dc57593ef1f159ede7caa142c42236f3 Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 7 Jun 2021 20:10:25 +0530 Subject: [PATCH 17/58] Pass file paths to setup_certificate() instead of IslandConfigOptions --- monkey/monkey_island/cc/server_setup.py | 2 +- monkey/monkey_island/cc/setup/certificate_setup.py | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/monkey/monkey_island/cc/server_setup.py b/monkey/monkey_island/cc/server_setup.py index fbd61982a..63918f567 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -83,7 +83,7 @@ def _start_island_server(should_setup_only, config_options: IslandConfigOptions) populate_exporter_list() app = init_app(MONGO_URL) - crt_path, key_path = setup_certificate(config_options) + crt_path, key_path = setup_certificate(config_options.crt_path, config_options.key_path) init_collections() diff --git a/monkey/monkey_island/cc/setup/certificate_setup.py b/monkey/monkey_island/cc/setup/certificate_setup.py index 1eaab4f04..273bbee1b 100644 --- a/monkey/monkey_island/cc/setup/certificate_setup.py +++ b/monkey/monkey_island/cc/setup/certificate_setup.py @@ -4,15 +4,11 @@ from monkey_island.cc.services.utils.file_handling import ( ensure_file_existence, ensure_file_permissions, ) -from monkey_island.cc.setup.island_config_options import IslandConfigOptions logger = logging.getLogger(__name__) -def setup_certificate(config_options: IslandConfigOptions) -> (str, str): - crt_path = config_options.crt_path - key_path = config_options.key_path - +def setup_certificate(crt_path: str, key_path: str) -> (str, str): for file in [crt_path, key_path]: ensure_file_existence(file) ensure_file_permissions(file) From a42040343b4056aa3d5cde4420e7d1aa8c154ca4 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 11:07:04 -0400 Subject: [PATCH 18/58] docs: Add a skeleton setup page for Linux AppImage package --- docs/content/setup/linux.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 docs/content/setup/linux.md diff --git a/docs/content/setup/linux.md b/docs/content/setup/linux.md new file mode 100644 index 000000000..d844d3d9e --- /dev/null +++ b/docs/content/setup/linux.md @@ -0,0 +1,14 @@ +--- +title: "Linux" +date: 2020-05-26T20:57:28+03:00 +draft: false +pre: ' ' +weight: 4 +tags: ["setup", "AppImage", "linux"] +--- + +## Supported operating systems + +## Deployment + +## Upgrading From 871fce1549d97f0b843b4a124a218ea68a9095bb Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 11:08:10 -0400 Subject: [PATCH 19/58] docs: Add setup instructions for linux AppImage package --- docs/content/setup/linux.md | 66 +++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/docs/content/setup/linux.md b/docs/content/setup/linux.md index d844d3d9e..a5ad1f99b 100644 --- a/docs/content/setup/linux.md +++ b/docs/content/setup/linux.md @@ -11,4 +11,70 @@ tags: ["setup", "AppImage", "linux"] ## Deployment +1. Make the AppImage package executable: + ```bash + chmod u+x Infection_Monkey_v1.11.0.AppImage + ``` +1. Start Monkey Island by running the Infection Monkey AppImage package: + ```bash + ./Infection_Monkey_v1.11.0.AppImage + ``` +1. Access the Monkey Island web UI by pointing your browser at + `https://localhost:5000`. + +### Start Monkey Island with user-provided certificate + +By default, Infection Monkey comes with a [self-signed SSL +certificate](https://aboutssl.org/what-is-self-sign-certificate/). In +enterprise or other security-sensitive environments, it is recommended that the +user provide Infection Monkey with a certificate that has been signed by a +private certificate authority. + +1. Run the Infection Monkey AppImage package with the `--setup-only` flag to + populate the `$HOME/.monkey_island` directory with a default + `server_config.json` file. + + ```bash + ./Infection_Monkey_v1.11.0.AppImage --setup-only + ``` + +1. (Optional but recommended) Move your `.crt` and `.key` files to + `$HOME/.monkey_island`. + +1. Make sure that your `.crt` and `.key` files are read-only and readable only + by you. + + ```bash + chmod 400 $HOME/.monkey_island/{*.key,*.crt} + ``` + +1. Edit `$HOME/.monkey_island/server_config.json` to configure Monkey Island + to use your certificate. Your config should look something like this: + + ```json {linenos=inline,hl_lines=["11-14"]} + { + "data_dir": "~/.monkey_island", + "log_level": "DEBUG", + "environment": { + "server_config": "password", + "deployment": "linux" + }, + "mongodb": { + "start_mongodb": true + }, + "ssl_certificate": { + "ssl_certificate_file": "", + "ssl_certificate_key_file": "", + } + } + ``` + +1. Start Monkey Island by running the Infection Monkey AppImage package: + ```bash + ./Infection_Monkey_v1.11.0.AppImage + ``` + +1. Access the Monkey Island web UI by pointing your browser at + `https://localhost:5000`. + ## Upgrading From eb044207da8626dd1da7945392180aa0bcbd6831 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 11:10:41 -0400 Subject: [PATCH 20/58] docs: Small change to docker setup to keep consistent with linux.md --- docs/content/setup/docker.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/setup/docker.md b/docs/content/setup/docker.md index e325de002..dc4067589 100644 --- a/docs/content/setup/docker.md +++ b/docs/content/setup/docker.md @@ -82,7 +82,7 @@ been signed by a private certificate authority. guardicore/monkey-island:1.10.0 --setup-only ``` -1. (Optional but recommended) Copy your `.crt` and `.key` files to `./monkey_island_data`. +1. (Optional but recommended) Move your `.crt` and `.key` files to `./monkey_island_data`. 1. Make sure that your `.crt` and `.key` files are read-only and readable only by you. From ea0d6f0141032af9f95215c9af8e13bf7892e77a Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 12:14:49 -0400 Subject: [PATCH 21/58] island: Add a generalized testing function to test_island_config_options --- .../monkey_island/cc/setup/test_island_config_options.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py index ae26d5145..df3c0dabd 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py @@ -51,8 +51,12 @@ def set_home_env(monkeypatch, tmpdir): def assert_island_config_options_data_dir_equals(config_file_contents, expected_data_dir): + assert_island_config_option_equals(config_file_contents, "data_dir", expected_data_dir) + + +def assert_island_config_option_equals(config_file_contents, option_name, expected_value): options = IslandConfigOptions(config_file_contents) - assert options.data_dir == expected_data_dir + assert getattr(options, option_name) == expected_value def test_island_config_options__log_level(): From f2a2efc2a7d5f0844e2b566d9f981ba881e88cd2 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 12:30:03 -0400 Subject: [PATCH 22/58] island: Remove redundant "test_island_config_options" from tests The file is named "test_island_config_options.py". Including "island_config_options" in every test/function name is reduntant. --- .../cc/setup/test_island_config_options.py | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py index df3c0dabd..79cd15f25 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py @@ -18,30 +18,28 @@ TEST_CONFIG_FILE_CONTENTS_UNSPECIFIED = {} TEST_CONFIG_FILE_CONTENTS_NO_STARTMONGO = {"mongodb": {}} -def test_island_config_options__data_dir_specified(): - assert_island_config_options_data_dir_equals(TEST_CONFIG_FILE_CONTENTS_SPECIFIED, "/tmp") +def test_data_dir_specified(): + assert_data_dir_equals(TEST_CONFIG_FILE_CONTENTS_SPECIFIED, "/tmp") -def test_island_config_options__data_dir_uses_default(): - assert_island_config_options_data_dir_equals( - TEST_CONFIG_FILE_CONTENTS_UNSPECIFIED, DEFAULT_DATA_DIR - ) +def test_data_dir_uses_default(): + assert_data_dir_equals(TEST_CONFIG_FILE_CONTENTS_UNSPECIFIED, DEFAULT_DATA_DIR) -def test_island_config_options__data_dir_expanduser(monkeypatch, tmpdir): +def test_data_dir_expanduser(monkeypatch, tmpdir): set_home_env(monkeypatch, tmpdir) DATA_DIR_NAME = "test_data_dir" - assert_island_config_options_data_dir_equals( + assert_data_dir_equals( {"data_dir": os.path.join("~", DATA_DIR_NAME)}, os.path.join(tmpdir, DATA_DIR_NAME) ) -def test_island_config_options__data_dir_expandvars(monkeypatch, tmpdir): +def test_data_dir_expandvars(monkeypatch, tmpdir): set_home_env(monkeypatch, tmpdir) DATA_DIR_NAME = "test_data_dir" - assert_island_config_options_data_dir_equals( + assert_data_dir_equals( {"data_dir": os.path.join("$HOME", DATA_DIR_NAME)}, os.path.join(tmpdir, DATA_DIR_NAME) ) @@ -50,26 +48,26 @@ def set_home_env(monkeypatch, tmpdir): monkeypatch.setenv("HOME", str(tmpdir)) -def assert_island_config_options_data_dir_equals(config_file_contents, expected_data_dir): +def assert_data_dir_equals(config_file_contents, expected_data_dir): assert_island_config_option_equals(config_file_contents, "data_dir", expected_data_dir) -def assert_island_config_option_equals(config_file_contents, option_name, expected_value): - options = IslandConfigOptions(config_file_contents) - assert getattr(options, option_name) == expected_value - - -def test_island_config_options__log_level(): +def test_log_level(): options = IslandConfigOptions(TEST_CONFIG_FILE_CONTENTS_SPECIFIED) assert options.log_level == "test" options = IslandConfigOptions(TEST_CONFIG_FILE_CONTENTS_UNSPECIFIED) assert options.log_level == DEFAULT_LOG_LEVEL -def test_island_config_options__mongodb(): +def test_mongodb(): options = IslandConfigOptions(TEST_CONFIG_FILE_CONTENTS_SPECIFIED) assert not options.start_mongodb options = IslandConfigOptions(TEST_CONFIG_FILE_CONTENTS_UNSPECIFIED) assert options.start_mongodb == DEFAULT_START_MONGO_DB options = IslandConfigOptions(TEST_CONFIG_FILE_CONTENTS_NO_STARTMONGO) assert options.start_mongodb == DEFAULT_START_MONGO_DB + + +def assert_island_config_option_equals(config_file_contents, option_name, expected_value): + options = IslandConfigOptions(config_file_contents) + assert getattr(options, option_name) == expected_value From 4231f316db86eb19ec07a63b15b22a94f8d3d653 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 12:41:11 -0400 Subject: [PATCH 23/58] island: Add tests for ssl_certificate_file --- .../cc/setup/test_island_config_options.py | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py index 79cd15f25..7219ba0fd 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py @@ -1,6 +1,7 @@ import os from monkey_island.cc.server_utils.consts import ( + DEFAULT_CRT_PATH, DEFAULT_DATA_DIR, DEFAULT_LOG_LEVEL, DEFAULT_START_MONGO_DB, @@ -11,6 +12,10 @@ TEST_CONFIG_FILE_CONTENTS_SPECIFIED = { "data_dir": "/tmp", "log_level": "test", "mongodb": {"start_mongodb": False}, + "ssl_certificate": { + "ssl_certificate_file": "/tmp/test.crt", + "ssl_certificate_key_file": "/tmp/test.key", + }, } TEST_CONFIG_FILE_CONTENTS_UNSPECIFIED = {} @@ -68,6 +73,43 @@ def test_mongodb(): assert options.start_mongodb == DEFAULT_START_MONGO_DB +def test_crt_path_uses_default(): + assert_ssl_certificate_file_equals(TEST_CONFIG_FILE_CONTENTS_UNSPECIFIED, DEFAULT_CRT_PATH) + + +def test_crt_path_specified(): + assert_ssl_certificate_file_equals( + TEST_CONFIG_FILE_CONTENTS_SPECIFIED, + TEST_CONFIG_FILE_CONTENTS_SPECIFIED["ssl_certificate"]["ssl_certificate_file"], + ) + + +def test_crt_path_expanduser(monkeypatch, tmpdir): + set_home_env(monkeypatch, tmpdir) + FILE_NAME = "test.crt" + + assert_ssl_certificate_file_equals( + {"ssl_certificate": {"ssl_certificate_file": os.path.join("~", FILE_NAME)}}, + os.path.join(tmpdir, FILE_NAME), + ) + + +def test_crt_path_expandvars(monkeypatch, tmpdir): + set_home_env(monkeypatch, tmpdir) + FILE_NAME = "test.crt" + + assert_ssl_certificate_file_equals( + {"ssl_certificate": {"ssl_certificate_file": os.path.join("$HOME", FILE_NAME)}}, + os.path.join(tmpdir, FILE_NAME), + ) + + +def assert_ssl_certificate_file_equals(config_file_contents, expected_ssl_certificate_file): + assert_island_config_option_equals( + config_file_contents, "crt_path", expected_ssl_certificate_file + ) + + def assert_island_config_option_equals(config_file_contents, option_name, expected_value): options = IslandConfigOptions(config_file_contents) assert getattr(options, option_name) == expected_value From f0a109a14519d09518a32796b728c2108b7316a2 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 12:53:32 -0400 Subject: [PATCH 24/58] island: Add tests for ssl_certificate_key_file --- .../cc/setup/test_island_config_options.py | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py index 7219ba0fd..193cc7350 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py @@ -3,6 +3,7 @@ import os from monkey_island.cc.server_utils.consts import ( DEFAULT_CRT_PATH, DEFAULT_DATA_DIR, + DEFAULT_KEY_PATH, DEFAULT_LOG_LEVEL, DEFAULT_START_MONGO_DB, ) @@ -110,6 +111,43 @@ def assert_ssl_certificate_file_equals(config_file_contents, expected_ssl_certif ) +def test_key_path_uses_default(): + assert_ssl_certificate_key_file_equals(TEST_CONFIG_FILE_CONTENTS_UNSPECIFIED, DEFAULT_KEY_PATH) + + +def test_key_path_specified(): + assert_ssl_certificate_key_file_equals( + TEST_CONFIG_FILE_CONTENTS_SPECIFIED, + TEST_CONFIG_FILE_CONTENTS_SPECIFIED["ssl_certificate"]["ssl_certificate_key_file"], + ) + + +def test_key_path_expanduser(monkeypatch, tmpdir): + set_home_env(monkeypatch, tmpdir) + FILE_NAME = "test.key" + + assert_ssl_certificate_key_file_equals( + {"ssl_certificate": {"ssl_certificate_key_file": os.path.join("~", FILE_NAME)}}, + os.path.join(tmpdir, FILE_NAME), + ) + + +def test_key_path_expandvars(monkeypatch, tmpdir): + set_home_env(monkeypatch, tmpdir) + FILE_NAME = "test.key" + + assert_ssl_certificate_key_file_equals( + {"ssl_certificate": {"ssl_certificate_key_file": os.path.join("$HOME", FILE_NAME)}}, + os.path.join(tmpdir, FILE_NAME), + ) + + +def assert_ssl_certificate_key_file_equals(config_file_contents, expected_ssl_certificate_file): + assert_island_config_option_equals( + config_file_contents, "key_path", expected_ssl_certificate_file + ) + + def assert_island_config_option_equals(config_file_contents, option_name, expected_value): options = IslandConfigOptions(config_file_contents) assert getattr(options, option_name) == expected_value From e4866b1286b9c3d7e98e2df78f741f11295e0764 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 12:57:42 -0400 Subject: [PATCH 25/58] island: Change _expand_path() from a static to regular function _expand_path() is a utility function used by IslandConfigOptions. It doesn't need to be part of the class. It can potentially be reused by other modules that require the same functionality. --- .../cc/setup/island_config_options.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/monkey/monkey_island/cc/setup/island_config_options.py b/monkey/monkey_island/cc/setup/island_config_options.py index fb94e6396..c497af529 100644 --- a/monkey/monkey_island/cc/setup/island_config_options.py +++ b/monkey/monkey_island/cc/setup/island_config_options.py @@ -14,9 +14,7 @@ from monkey_island.cc.server_utils.consts import ( class IslandConfigOptions: def __init__(self, config_contents: dict): - self.data_dir = IslandConfigOptions._expand_path( - config_contents.get("data_dir", DEFAULT_DATA_DIR) - ) + self.data_dir = _expand_path(config_contents.get("data_dir", DEFAULT_DATA_DIR)) self.log_level = config_contents.get("log_level", DEFAULT_LOG_LEVEL) @@ -24,17 +22,17 @@ class IslandConfigOptions: "mongodb", {"start_mongodb": DEFAULT_START_MONGO_DB} ).get("start_mongodb", DEFAULT_START_MONGO_DB) - self.crt_path = IslandConfigOptions._expand_path( + self.crt_path = _expand_path( config_contents.get("ssl_certificate", DEFAULT_CERTIFICATE_PATHS).get( "ssl_certificate_file", DEFAULT_CRT_PATH ) ) - self.key_path = IslandConfigOptions._expand_path( + self.key_path = _expand_path( config_contents.get("ssl_certificate", DEFAULT_CERTIFICATE_PATHS).get( "ssl_certificate_key_file", DEFAULT_KEY_PATH ) ) - @staticmethod - def _expand_path(path: str) -> str: - return os.path.expandvars(os.path.expanduser(path)) + +def _expand_path(path: str) -> str: + return os.path.expandvars(os.path.expanduser(path)) From 0519153aaf7df97068d16e32a7e2b73cdb9c948e Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 13:04:38 -0400 Subject: [PATCH 26/58] island: Move _expand_path() to file_utils.py so it can be reused --- monkey/monkey_island/cc/server_utils/file_utils.py | 5 +++++ .../monkey_island/cc/setup/island_config_options.py | 13 ++++--------- 2 files changed, 9 insertions(+), 9 deletions(-) create mode 100644 monkey/monkey_island/cc/server_utils/file_utils.py diff --git a/monkey/monkey_island/cc/server_utils/file_utils.py b/monkey/monkey_island/cc/server_utils/file_utils.py new file mode 100644 index 000000000..225fb8732 --- /dev/null +++ b/monkey/monkey_island/cc/server_utils/file_utils.py @@ -0,0 +1,5 @@ +import os + + +def expand_path(path: str) -> str: + return os.path.expandvars(os.path.expanduser(path)) diff --git a/monkey/monkey_island/cc/setup/island_config_options.py b/monkey/monkey_island/cc/setup/island_config_options.py index c497af529..9704e5f45 100644 --- a/monkey/monkey_island/cc/setup/island_config_options.py +++ b/monkey/monkey_island/cc/setup/island_config_options.py @@ -1,7 +1,5 @@ from __future__ import annotations -import os - from monkey_island.cc.server_utils.consts import ( DEFAULT_CERTIFICATE_PATHS, DEFAULT_CRT_PATH, @@ -10,11 +8,12 @@ from monkey_island.cc.server_utils.consts import ( DEFAULT_LOG_LEVEL, DEFAULT_START_MONGO_DB, ) +from monkey_island.cc.server_utils.file_utils import expand_path class IslandConfigOptions: def __init__(self, config_contents: dict): - self.data_dir = _expand_path(config_contents.get("data_dir", DEFAULT_DATA_DIR)) + self.data_dir = expand_path(config_contents.get("data_dir", DEFAULT_DATA_DIR)) self.log_level = config_contents.get("log_level", DEFAULT_LOG_LEVEL) @@ -22,17 +21,13 @@ class IslandConfigOptions: "mongodb", {"start_mongodb": DEFAULT_START_MONGO_DB} ).get("start_mongodb", DEFAULT_START_MONGO_DB) - self.crt_path = _expand_path( + self.crt_path = expand_path( config_contents.get("ssl_certificate", DEFAULT_CERTIFICATE_PATHS).get( "ssl_certificate_file", DEFAULT_CRT_PATH ) ) - self.key_path = _expand_path( + self.key_path = expand_path( config_contents.get("ssl_certificate", DEFAULT_CERTIFICATE_PATHS).get( "ssl_certificate_key_file", DEFAULT_KEY_PATH ) ) - - -def _expand_path(path: str) -> str: - return os.path.expandvars(os.path.expanduser(path)) From 87440112975b82f3cb2747371ba3cc2ade243146 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 13:17:22 -0400 Subject: [PATCH 27/58] island: move set_home_env() to conftest.py so it can be reused --- .../cc/setup/test_island_config_options.py | 36 ++++++++----------- .../unit_tests/monkey_island/conftest.py | 7 ++++ 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py index 193cc7350..1554980a2 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options.py @@ -32,28 +32,24 @@ def test_data_dir_uses_default(): assert_data_dir_equals(TEST_CONFIG_FILE_CONTENTS_UNSPECIFIED, DEFAULT_DATA_DIR) -def test_data_dir_expanduser(monkeypatch, tmpdir): - set_home_env(monkeypatch, tmpdir) +def test_data_dir_expanduser(patched_home_env): DATA_DIR_NAME = "test_data_dir" assert_data_dir_equals( - {"data_dir": os.path.join("~", DATA_DIR_NAME)}, os.path.join(tmpdir, DATA_DIR_NAME) + {"data_dir": os.path.join("~", DATA_DIR_NAME)}, + os.path.join(patched_home_env, DATA_DIR_NAME), ) -def test_data_dir_expandvars(monkeypatch, tmpdir): - set_home_env(monkeypatch, tmpdir) +def test_data_dir_expandvars(patched_home_env): DATA_DIR_NAME = "test_data_dir" assert_data_dir_equals( - {"data_dir": os.path.join("$HOME", DATA_DIR_NAME)}, os.path.join(tmpdir, DATA_DIR_NAME) + {"data_dir": os.path.join("$HOME", DATA_DIR_NAME)}, + os.path.join(patched_home_env, DATA_DIR_NAME), ) -def set_home_env(monkeypatch, tmpdir): - monkeypatch.setenv("HOME", str(tmpdir)) - - def assert_data_dir_equals(config_file_contents, expected_data_dir): assert_island_config_option_equals(config_file_contents, "data_dir", expected_data_dir) @@ -85,23 +81,21 @@ def test_crt_path_specified(): ) -def test_crt_path_expanduser(monkeypatch, tmpdir): - set_home_env(monkeypatch, tmpdir) +def test_crt_path_expanduser(patched_home_env): FILE_NAME = "test.crt" assert_ssl_certificate_file_equals( {"ssl_certificate": {"ssl_certificate_file": os.path.join("~", FILE_NAME)}}, - os.path.join(tmpdir, FILE_NAME), + os.path.join(patched_home_env, FILE_NAME), ) -def test_crt_path_expandvars(monkeypatch, tmpdir): - set_home_env(monkeypatch, tmpdir) +def test_crt_path_expandvars(patched_home_env): FILE_NAME = "test.crt" assert_ssl_certificate_file_equals( {"ssl_certificate": {"ssl_certificate_file": os.path.join("$HOME", FILE_NAME)}}, - os.path.join(tmpdir, FILE_NAME), + os.path.join(patched_home_env, FILE_NAME), ) @@ -122,23 +116,21 @@ def test_key_path_specified(): ) -def test_key_path_expanduser(monkeypatch, tmpdir): - set_home_env(monkeypatch, tmpdir) +def test_key_path_expanduser(patched_home_env): FILE_NAME = "test.key" assert_ssl_certificate_key_file_equals( {"ssl_certificate": {"ssl_certificate_key_file": os.path.join("~", FILE_NAME)}}, - os.path.join(tmpdir, FILE_NAME), + os.path.join(patched_home_env, FILE_NAME), ) -def test_key_path_expandvars(monkeypatch, tmpdir): - set_home_env(monkeypatch, tmpdir) +def test_key_path_expandvars(patched_home_env): FILE_NAME = "test.key" assert_ssl_certificate_key_file_equals( {"ssl_certificate": {"ssl_certificate_key_file": os.path.join("$HOME", FILE_NAME)}}, - os.path.join(tmpdir, FILE_NAME), + os.path.join(patched_home_env, FILE_NAME), ) diff --git a/monkey/tests/unit_tests/monkey_island/conftest.py b/monkey/tests/unit_tests/monkey_island/conftest.py index 037bee72b..f45eb3ed6 100644 --- a/monkey/tests/unit_tests/monkey_island/conftest.py +++ b/monkey/tests/unit_tests/monkey_island/conftest.py @@ -6,3 +6,10 @@ import pytest @pytest.fixture(scope="module") def server_configs_dir(data_for_tests_dir): return os.path.join(data_for_tests_dir, "server_configs") + + +@pytest.fixture +def patched_home_env(monkeypatch, tmpdir): + monkeypatch.setenv("HOME", str(tmpdir)) + + return tmpdir From bf0fe10ea9a66551eedfb5ca9ec9f728b50eea8f Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 13:18:04 -0400 Subject: [PATCH 28/58] island: Add unit tests for expand_path() --- .../cc/server_utils/test_file_utils.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py diff --git a/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py new file mode 100644 index 000000000..cff716135 --- /dev/null +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py @@ -0,0 +1,17 @@ +import os + +from monkey_island.cc.server_utils import file_utils + + +def test_expand_user(patched_home_env): + input_path = os.path.join("~", "test") + expected_path = os.path.join(patched_home_env, "test") + + assert file_utils.expand_path(input_path) == expected_path + + +def test_expand_vars(patched_home_env): + input_path = os.path.join("$HOME", "test") + expected_path = os.path.join(patched_home_env, "test") + + assert file_utils.expand_path(input_path) == expected_path From 4e1b4fbf6b85f078109fcdd5d81844b405632300 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 13:21:16 -0400 Subject: [PATCH 29/58] island: Replace calls to os.{expandpath,expandusers} with expand_path() --- monkey/monkey_island/cc/server_utils/consts.py | 5 +++-- monkey/monkey_island/cc/setup/config_setup.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/consts.py b/monkey/monkey_island/cc/server_utils/consts.py index 9149d81b1..02f23428d 100644 --- a/monkey/monkey_island/cc/server_utils/consts.py +++ b/monkey/monkey_island/cc/server_utils/consts.py @@ -2,6 +2,7 @@ import os from pathlib import Path from monkey_island.cc.environment.utils import is_windows_os +from monkey_island.cc.server_utils import file_utils __author__ = "itay.mizeretz" @@ -25,7 +26,7 @@ SERVER_CONFIG_FILENAME = "server_config.json" MONKEY_ISLAND_ABS_PATH = _get_monkey_island_abs_path() -DEFAULT_DATA_DIR = os.path.expandvars(get_default_data_dir()) +DEFAULT_DATA_DIR = file_utils.expand_path(get_default_data_dir()) DEFAULT_MONKEY_TTL_EXPIRY_DURATION_IN_SECONDS = 60 * 5 @@ -36,7 +37,7 @@ MONGO_EXECUTABLE_PATH = ( _MONGO_EXECUTABLE_PATH_WIN if is_windows_os() else _MONGO_EXECUTABLE_PATH_LINUX ) -DEFAULT_SERVER_CONFIG_PATH = os.path.expandvars( +DEFAULT_SERVER_CONFIG_PATH = file_utils.expand_path( os.path.join(DEFAULT_DATA_DIR, SERVER_CONFIG_FILENAME) ) diff --git a/monkey/monkey_island/cc/setup/config_setup.py b/monkey/monkey_island/cc/setup/config_setup.py index 6cc036760..3621ea7fc 100644 --- a/monkey/monkey_island/cc/setup/config_setup.py +++ b/monkey/monkey_island/cc/setup/config_setup.py @@ -1,9 +1,9 @@ -import os from typing import Tuple from monkey_island.cc.arg_parser import IslandCmdArgs from monkey_island.cc.environment import server_config_handler from monkey_island.cc.environment.utils import create_secure_directory +from monkey_island.cc.server_utils import file_utils from monkey_island.cc.server_utils.consts import DEFAULT_DATA_DIR, DEFAULT_SERVER_CONFIG_PATH from monkey_island.cc.setup.island_config_options import IslandConfigOptions @@ -16,7 +16,7 @@ def setup_data_dir(island_args: IslandCmdArgs) -> Tuple[IslandConfigOptions, str def _setup_config_by_cmd_arg(server_config_path) -> Tuple[IslandConfigOptions, str]: - server_config_path = os.path.expandvars(os.path.expanduser(server_config_path)) + server_config_path = file_utils.expand_path(server_config_path) config = server_config_handler.load_server_config_from_file(server_config_path) create_secure_directory(config.data_dir, create_parent_dirs=True) return config, server_config_path From 36314f09ae6b8cee95d1545b91686eaa03da6e11 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 13:23:53 -0400 Subject: [PATCH 30/58] island: Use certificate provided in config, not environment variables --- monkey/monkey_island/cc/server_setup.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/monkey/monkey_island/cc/server_setup.py b/monkey/monkey_island/cc/server_setup.py index 63918f567..7e32ce26b 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -1,6 +1,5 @@ import json import logging -import os import sys from pathlib import Path from threading import Thread @@ -99,9 +98,8 @@ def _start_island_server(should_setup_only, config_options: IslandConfigOptions) http_server = WSGIServer( ("0.0.0.0", env_singleton.env.get_island_port()), app, - # TODO: modify next two lines? - certfile=os.environ.get("SERVER_CRT", crt_path), - keyfile=os.environ.get("SERVER_KEY", key_path), + certfile=crt_path, + keyfile=key_path, ) _log_init_info() http_server.serve_forever() From a45848ce0cca03306df248a1278cd54fa2f9ab11 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 14:07:00 -0400 Subject: [PATCH 31/58] island: Move file_has_expected_permissions() to file_utils.py Rename to `has_expected_permissions()` as `file_has_expected_permissions()` is now reduntant. Add unit tests --- .../cc/server_utils/file_utils.py | 7 +++++ .../cc/services/utils/file_handling.py | 10 ++----- .../cc/server_utils/test_file_utils.py | 27 +++++++++++++++++++ 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/file_utils.py b/monkey/monkey_island/cc/server_utils/file_utils.py index 225fb8732..6a474355a 100644 --- a/monkey/monkey_island/cc/server_utils/file_utils.py +++ b/monkey/monkey_island/cc/server_utils/file_utils.py @@ -3,3 +3,10 @@ import os def expand_path(path: str) -> str: return os.path.expandvars(os.path.expanduser(path)) + + +def has_expected_permissions(path: str, expected_permissions: int) -> bool: + file_mode = os.stat(path).st_mode + file_permissions = file_mode & 0o777 + + return file_permissions == expected_permissions diff --git a/monkey/monkey_island/cc/services/utils/file_handling.py b/monkey/monkey_island/cc/services/utils/file_handling.py index 114405647..e6c4839d2 100644 --- a/monkey/monkey_island/cc/services/utils/file_handling.py +++ b/monkey/monkey_island/cc/services/utils/file_handling.py @@ -1,6 +1,7 @@ import os from common.utils.exceptions import InsecurePermissionsError +from monkey_island.cc.server_utils.file_utils import has_expected_permissions def ensure_file_existence(file: str) -> None: @@ -9,14 +10,7 @@ def ensure_file_existence(file: str) -> None: def ensure_file_permissions(file: str) -> None: - if not file_has_expected_permissions(path=file, expected_permissions="0o400"): + if not has_expected_permissions(path=file, expected_permissions="0o400"): raise InsecurePermissionsError( f"{file} has insecure permissions. Required permissions: 400. Exiting." ) - - -def file_has_expected_permissions(path: str, expected_permissions: str) -> bool: - file_mode = os.stat(path).st_mode - file_permissions = oct(file_mode & 0o777) - - return file_permissions == expected_permissions diff --git a/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py index cff716135..79409ba7a 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py @@ -1,5 +1,7 @@ import os +import pytest + from monkey_island.cc.server_utils import file_utils @@ -15,3 +17,28 @@ def test_expand_vars(patched_home_env): expected_path = os.path.join(patched_home_env, "test") assert file_utils.expand_path(input_path) == expected_path + + +@pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") +def test_has_expected_permissions_true(tmpdir): + file_name = f"{tmpdir}/test" + + create_empty_file(file_name) + os.chmod(file_name, 0o754) + + assert file_utils.has_expected_permissions(file_name, 0o754) + + +@pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") +def test_has_expected_permissions_false(tmpdir): + file_name = f"{tmpdir}/test" + + create_empty_file(file_name) + os.chmod(file_name, 0o755) + + assert not file_utils.has_expected_permissions(file_name, 0o700) + + +def create_empty_file(file_name): + with open(file_name, "w"): + pass From c19dc9dcad74f50f75c9d69bb13fe2bb4504b149 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 14:33:04 -0400 Subject: [PATCH 32/58] island: Add config validation to IslandConfigOptions --- monkey/monkey_island/cc/server_setup.py | 29 +++++++++++++++---- .../cc/services/utils/file_handling.py | 16 ---------- .../cc/setup/certificate_setup.py | 18 ------------ .../cc/setup/island_config_options.py | 25 +++++++++++++++- 4 files changed, 47 insertions(+), 41 deletions(-) delete mode 100644 monkey/monkey_island/cc/services/utils/file_handling.py delete mode 100644 monkey/monkey_island/cc/setup/certificate_setup.py diff --git a/monkey/monkey_island/cc/server_setup.py b/monkey/monkey_island/cc/server_setup.py index 7e32ce26b..8adcbdaa5 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -26,7 +26,7 @@ from monkey_island.cc.server_utils.island_logger import reset_logger, setup_logg 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 from monkey_island.cc.services.utils.network_utils import local_ip_addresses # noqa: E402 -from monkey_island.cc.setup.certificate_setup import setup_certificate # noqa: E402 +from monkey_island.cc.setup import island_config_options # noqa: E402 from monkey_island.cc.setup.island_config_options import IslandConfigOptions # noqa: E402 from monkey_island.cc.setup.mongo.database_initializer import init_collections # noqa: E402 from monkey_island.cc.setup.mongo.mongo_setup import ( # noqa: E402 @@ -43,6 +43,8 @@ def run_monkey_island(): island_args = parse_cli_args() config_options, server_config_path = _setup_data_dir(island_args) + _exit_on_invalid_config_options(config_options) + _configure_logging(config_options) _initialize_globals(config_options, server_config_path) @@ -66,6 +68,14 @@ def _setup_data_dir(island_args: IslandCmdArgs) -> Tuple[IslandConfigOptions, st exit(1) +def _exit_on_invalid_config_options(config_options: IslandConfigOptions): + try: + island_config_options.raise_on_invalid_options(config_options) + except Exception as ex: + print(f"Configuration error: {ex}") + exit(1) + + def _configure_logging(config_options): reset_logger() setup_logging(config_options.data_dir, config_options.log_level) @@ -82,8 +92,6 @@ def _start_island_server(should_setup_only, config_options: IslandConfigOptions) populate_exporter_list() app = init_app(MONGO_URL) - crt_path, key_path = setup_certificate(config_options.crt_path, config_options.key_path) - init_collections() if should_setup_only: @@ -92,14 +100,23 @@ def _start_island_server(should_setup_only, config_options: IslandConfigOptions) bootloader_server_thread = _start_bootloader_server() + logger.info( + f"Using certificate path: {config_options.crt_path}, and key path: " + "{config_options.key_path}." + ) + if env_singleton.env.is_debug(): - app.run(host="0.0.0.0", debug=True, ssl_context=(crt_path, key_path)) + app.run( + host="0.0.0.0", + debug=True, + ssl_context=(config_options.crt_path, config_options.key_path), + ) else: http_server = WSGIServer( ("0.0.0.0", env_singleton.env.get_island_port()), app, - certfile=crt_path, - keyfile=key_path, + certfile=config_options.crt_path, + keyfile=config_options.key_path, ) _log_init_info() http_server.serve_forever() diff --git a/monkey/monkey_island/cc/services/utils/file_handling.py b/monkey/monkey_island/cc/services/utils/file_handling.py deleted file mode 100644 index e6c4839d2..000000000 --- a/monkey/monkey_island/cc/services/utils/file_handling.py +++ /dev/null @@ -1,16 +0,0 @@ -import os - -from common.utils.exceptions import InsecurePermissionsError -from monkey_island.cc.server_utils.file_utils import has_expected_permissions - - -def ensure_file_existence(file: str) -> None: - if not os.path.exists(file): - raise FileNotFoundError(f"File not found at {file}. Exiting.") - - -def ensure_file_permissions(file: str) -> None: - if not has_expected_permissions(path=file, expected_permissions="0o400"): - raise InsecurePermissionsError( - f"{file} has insecure permissions. Required permissions: 400. Exiting." - ) diff --git a/monkey/monkey_island/cc/setup/certificate_setup.py b/monkey/monkey_island/cc/setup/certificate_setup.py deleted file mode 100644 index 273bbee1b..000000000 --- a/monkey/monkey_island/cc/setup/certificate_setup.py +++ /dev/null @@ -1,18 +0,0 @@ -import logging - -from monkey_island.cc.services.utils.file_handling import ( - ensure_file_existence, - ensure_file_permissions, -) - -logger = logging.getLogger(__name__) - - -def setup_certificate(crt_path: str, key_path: str) -> (str, str): - for file in [crt_path, key_path]: - ensure_file_existence(file) - ensure_file_permissions(file) - - logger.info(f"Using certificate path: {crt_path}, and key path: {key_path}.") - - return crt_path, key_path diff --git a/monkey/monkey_island/cc/setup/island_config_options.py b/monkey/monkey_island/cc/setup/island_config_options.py index 9704e5f45..78865acbe 100644 --- a/monkey/monkey_island/cc/setup/island_config_options.py +++ b/monkey/monkey_island/cc/setup/island_config_options.py @@ -1,5 +1,8 @@ from __future__ import annotations +import os + +from common.utils.exceptions import InsecurePermissionsError from monkey_island.cc.server_utils.consts import ( DEFAULT_CERTIFICATE_PATHS, DEFAULT_CRT_PATH, @@ -8,7 +11,7 @@ from monkey_island.cc.server_utils.consts import ( DEFAULT_LOG_LEVEL, DEFAULT_START_MONGO_DB, ) -from monkey_island.cc.server_utils.file_utils import expand_path +from monkey_island.cc.server_utils.file_utils import expand_path, has_expected_permissions class IslandConfigOptions: @@ -31,3 +34,23 @@ class IslandConfigOptions: "ssl_certificate_key_file", DEFAULT_KEY_PATH ) ) + + +def raise_on_invalid_options(options: IslandConfigOptions): + _raise_if_not_isfile(options.crt_path) + _raise_if_incorrect_permissions(options.crt_path, 0o400) + + _raise_if_not_isfile(options.key_path) + _raise_if_incorrect_permissions(options.key_path, 0o400) + + +def _raise_if_not_isfile(f: str): + if not os.path.isfile(f): + raise FileNotFoundError(f"{f} does not exist or is not a regular file.") + + +def _raise_if_incorrect_permissions(f: str, expected_permissions: int): + if not has_expected_permissions(f, expected_permissions): + raise InsecurePermissionsError( + f"The file {f} has incorrect permissions. Expected: {oct(expected_permissions)}" + ) From 78af0d86aabf368b0c5038c87a1e080d14c2ee30 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 19:28:03 -0400 Subject: [PATCH 33/58] island: Move IslandConfigOptions validation to separate module --- monkey/monkey_island/cc/server_setup.py | 4 +-- .../cc/setup/island_config_options.py | 25 +------------------ .../setup/island_config_options_validator.py | 25 +++++++++++++++++++ 3 files changed, 28 insertions(+), 26 deletions(-) create mode 100644 monkey/monkey_island/cc/setup/island_config_options_validator.py diff --git a/monkey/monkey_island/cc/server_setup.py b/monkey/monkey_island/cc/server_setup.py index 8adcbdaa5..ee1774240 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -26,7 +26,7 @@ from monkey_island.cc.server_utils.island_logger import reset_logger, setup_logg 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 from monkey_island.cc.services.utils.network_utils import local_ip_addresses # noqa: E402 -from monkey_island.cc.setup import island_config_options # noqa: E402 +from monkey_island.cc.setup import island_config_options_validator # noqa: E402 from monkey_island.cc.setup.island_config_options import IslandConfigOptions # noqa: E402 from monkey_island.cc.setup.mongo.database_initializer import init_collections # noqa: E402 from monkey_island.cc.setup.mongo.mongo_setup import ( # noqa: E402 @@ -70,7 +70,7 @@ def _setup_data_dir(island_args: IslandCmdArgs) -> Tuple[IslandConfigOptions, st def _exit_on_invalid_config_options(config_options: IslandConfigOptions): try: - island_config_options.raise_on_invalid_options(config_options) + island_config_options_validator.raise_on_invalid_options(config_options) except Exception as ex: print(f"Configuration error: {ex}") exit(1) diff --git a/monkey/monkey_island/cc/setup/island_config_options.py b/monkey/monkey_island/cc/setup/island_config_options.py index 78865acbe..9704e5f45 100644 --- a/monkey/monkey_island/cc/setup/island_config_options.py +++ b/monkey/monkey_island/cc/setup/island_config_options.py @@ -1,8 +1,5 @@ from __future__ import annotations -import os - -from common.utils.exceptions import InsecurePermissionsError from monkey_island.cc.server_utils.consts import ( DEFAULT_CERTIFICATE_PATHS, DEFAULT_CRT_PATH, @@ -11,7 +8,7 @@ from monkey_island.cc.server_utils.consts import ( DEFAULT_LOG_LEVEL, DEFAULT_START_MONGO_DB, ) -from monkey_island.cc.server_utils.file_utils import expand_path, has_expected_permissions +from monkey_island.cc.server_utils.file_utils import expand_path class IslandConfigOptions: @@ -34,23 +31,3 @@ class IslandConfigOptions: "ssl_certificate_key_file", DEFAULT_KEY_PATH ) ) - - -def raise_on_invalid_options(options: IslandConfigOptions): - _raise_if_not_isfile(options.crt_path) - _raise_if_incorrect_permissions(options.crt_path, 0o400) - - _raise_if_not_isfile(options.key_path) - _raise_if_incorrect_permissions(options.key_path, 0o400) - - -def _raise_if_not_isfile(f: str): - if not os.path.isfile(f): - raise FileNotFoundError(f"{f} does not exist or is not a regular file.") - - -def _raise_if_incorrect_permissions(f: str, expected_permissions: int): - if not has_expected_permissions(f, expected_permissions): - raise InsecurePermissionsError( - f"The file {f} has incorrect permissions. Expected: {oct(expected_permissions)}" - ) diff --git a/monkey/monkey_island/cc/setup/island_config_options_validator.py b/monkey/monkey_island/cc/setup/island_config_options_validator.py new file mode 100644 index 000000000..0a5247003 --- /dev/null +++ b/monkey/monkey_island/cc/setup/island_config_options_validator.py @@ -0,0 +1,25 @@ +import os + +from common.utils.exceptions import InsecurePermissionsError +from monkey_island.cc.server_utils.file_utils import has_expected_permissions +from monkey_island.cc.setup.island_config_options import IslandConfigOptions + + +def raise_on_invalid_options(options: IslandConfigOptions): + _raise_if_not_isfile(options.crt_path) + _raise_if_incorrect_permissions(options.crt_path, 0o400) + + _raise_if_not_isfile(options.key_path) + _raise_if_incorrect_permissions(options.key_path, 0o400) + + +def _raise_if_not_isfile(f: str): + if not os.path.isfile(f): + raise FileNotFoundError(f"{f} does not exist or is not a regular file.") + + +def _raise_if_incorrect_permissions(f: str, expected_permissions: int): + if not has_expected_permissions(f, expected_permissions): + raise InsecurePermissionsError( + f"The file {f} has incorrect permissions. Expected: {oct(expected_permissions)}" + ) From b80dd5935282ed4c07f26a5e6546c439b3bd467e Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 19:44:06 -0400 Subject: [PATCH 34/58] tests: move create_empty_file() to conftest.py --- .../monkey_island/cc/server_utils/test_file_utils.py | 9 ++------- monkey/tests/unit_tests/monkey_island/conftest.py | 9 +++++++++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py index 79409ba7a..e9dc3eddc 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py @@ -20,7 +20,7 @@ def test_expand_vars(patched_home_env): @pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") -def test_has_expected_permissions_true(tmpdir): +def test_has_expected_permissions_true(tmpdir, create_empty_file): file_name = f"{tmpdir}/test" create_empty_file(file_name) @@ -30,15 +30,10 @@ def test_has_expected_permissions_true(tmpdir): @pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") -def test_has_expected_permissions_false(tmpdir): +def test_has_expected_permissions_false(tmpdir, create_empty_file): file_name = f"{tmpdir}/test" create_empty_file(file_name) os.chmod(file_name, 0o755) assert not file_utils.has_expected_permissions(file_name, 0o700) - - -def create_empty_file(file_name): - with open(file_name, "w"): - pass diff --git a/monkey/tests/unit_tests/monkey_island/conftest.py b/monkey/tests/unit_tests/monkey_island/conftest.py index f45eb3ed6..2baa97a02 100644 --- a/monkey/tests/unit_tests/monkey_island/conftest.py +++ b/monkey/tests/unit_tests/monkey_island/conftest.py @@ -13,3 +13,12 @@ def patched_home_env(monkeypatch, tmpdir): monkeypatch.setenv("HOME", str(tmpdir)) return tmpdir + + +@pytest.fixture +def create_empty_file(): + def inner(file_name): + with open(file_name, "w"): + pass + + return inner From 63fb396bbb270b1c19d0b234a5c57c2a06d9212a Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 7 Jun 2021 19:55:33 -0400 Subject: [PATCH 35/58] island: Add unit tests for island_config_options_validator --- .../test_island_config_options_validator.py | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py new file mode 100644 index 000000000..9c2353c5b --- /dev/null +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py @@ -0,0 +1,62 @@ +import os + +import pytest + +from common.utils.exceptions import InsecurePermissionsError +from monkey_island.cc.setup.island_config_options import IslandConfigOptions +from monkey_island.cc.setup.island_config_options_validator import raise_on_invalid_options + + +@pytest.fixture +def island_config_options(tmpdir, create_empty_file): + crt_file = os.path.join(tmpdir, "test.crt") + create_empty_file(crt_file) + os.chmod(crt_file, 0o400) + + key_file = os.path.join(tmpdir, "test.key") + create_empty_file(key_file) + os.chmod(key_file, 0o400) + return IslandConfigOptions( + { + "ssl_certificate": { + "ssl_certificate_file": crt_file, + "ssl_certificate_key_file": key_file, + } + } + ) + + +def test_valid_crt_and_key_paths(island_config_options): + try: + raise_on_invalid_options(island_config_options) + except Exception as ex: + print(ex) + assert False + + +def test_crt_path_does_not_exist(island_config_options): + os.remove(island_config_options.crt_path) + + with pytest.raises(FileNotFoundError): + raise_on_invalid_options(island_config_options) + + +def test_crt_path_insecure_permissions(island_config_options): + os.chmod(island_config_options.crt_path, 0o777) + + with pytest.raises(InsecurePermissionsError): + raise_on_invalid_options(island_config_options) + + +def test_key_path_does_not_exist(island_config_options): + os.remove(island_config_options.key_path) + + with pytest.raises(FileNotFoundError): + raise_on_invalid_options(island_config_options) + + +def test_key_path_insecure_permissions(island_config_options): + os.chmod(island_config_options.key_path, 0o777) + + with pytest.raises(InsecurePermissionsError): + raise_on_invalid_options(island_config_options) From 4b119ab4cedabf0b8dc84e8b9a35ee7edf3d74e9 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 8 Jun 2021 06:35:15 -0400 Subject: [PATCH 36/58] island: Skip some island_config_options_validator tests on Windows --- .../cc/setup/test_island_config_options_validator.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py index 9c2353c5b..1b1be6718 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py @@ -26,6 +26,7 @@ def island_config_options(tmpdir, create_empty_file): ) +@pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") def test_valid_crt_and_key_paths(island_config_options): try: raise_on_invalid_options(island_config_options) @@ -34,6 +35,7 @@ def test_valid_crt_and_key_paths(island_config_options): assert False +@pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") def test_crt_path_does_not_exist(island_config_options): os.remove(island_config_options.crt_path) @@ -41,6 +43,7 @@ def test_crt_path_does_not_exist(island_config_options): raise_on_invalid_options(island_config_options) +@pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") def test_crt_path_insecure_permissions(island_config_options): os.chmod(island_config_options.crt_path, 0o777) @@ -48,6 +51,7 @@ def test_crt_path_insecure_permissions(island_config_options): raise_on_invalid_options(island_config_options) +@pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") def test_key_path_does_not_exist(island_config_options): os.remove(island_config_options.key_path) @@ -55,6 +59,7 @@ def test_key_path_does_not_exist(island_config_options): raise_on_invalid_options(island_config_options) +@pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") def test_key_path_insecure_permissions(island_config_options): os.chmod(island_config_options.key_path, 0o777) From 9086d9313761ecd483f818d60c47af1e51415153 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 8 Jun 2021 06:40:59 -0400 Subject: [PATCH 37/58] docs: Make chmod command less specific in docker setup --- docs/content/setup/docker.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/content/setup/docker.md b/docs/content/setup/docker.md index 4d05b9b1a..c839f4aef 100644 --- a/docs/content/setup/docker.md +++ b/docs/content/setup/docker.md @@ -87,7 +87,8 @@ been signed by a private certificate authority. 1. Make sure that your `.crt` and `.key` files are read-only and readable only by you. ```bash - chmod 400 ./monkey_island_data/{*.key,*.crt} + chmod 400 + chmod 400 ``` 1. Edit `./monkey_island_data/server_config.json` to configure Monkey Island From 526019a6ead6e463efcf695100693fcdfaa2d8e1 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 8 Jun 2021 06:51:25 -0400 Subject: [PATCH 38/58] docs: Make chmod command less specific in linux setup --- docs/content/setup/linux.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/content/setup/linux.md b/docs/content/setup/linux.md index a5ad1f99b..85e4a0f13 100644 --- a/docs/content/setup/linux.md +++ b/docs/content/setup/linux.md @@ -45,7 +45,8 @@ private certificate authority. by you. ```bash - chmod 400 $HOME/.monkey_island/{*.key,*.crt} + chmod 400 + chmod 400 ``` 1. Edit `$HOME/.monkey_island/server_config.json` to configure Monkey Island From 3841dd7f7b47f55c83f4e13d8c17940db0fe28e0 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 8 Jun 2021 07:17:59 -0400 Subject: [PATCH 39/58] island: Set tighter permissions on certs in create_certificate.sh --- monkey/monkey_island/linux/create_certificate.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/monkey/monkey_island/linux/create_certificate.sh b/monkey/monkey_island/linux/create_certificate.sh index ca7d397e0..cbbe5261b 100644 --- a/monkey/monkey_island/linux/create_certificate.sh +++ b/monkey/monkey_island/linux/create_certificate.sh @@ -21,10 +21,16 @@ umask 377 echo "Generating key in $server_root/server.key..." openssl genrsa -out "$server_root"/server.key 2048 +chmod 400 "$server_root"/server.key + echo "Generating csr in $server_root/server.csr..." openssl req -new -key "$server_root"/server.key -out "$server_root"/server.csr -subj "/C=GB/ST=London/L=London/O=Global Security/OU=Monkey Department/CN=monkey.com" +chmod 400 "$server_root"/server.csr + echo "Generating certificate in $server_root/server.crt..." openssl x509 -req -days 366 -in "$server_root"/server.csr -signkey "$server_root"/server.key -out "$server_root"/server.crt +chmod 400 "$server_root"/server.crt + # Shove some new random data into the file to override the original seed we put in. if [ "$CREATED_RND_FILE" = true ] ; then From 10e7b1966995b9d38b979f5f18b4d0c506269eb5 Mon Sep 17 00:00:00 2001 From: Shreya Date: Wed, 9 Jun 2021 11:12:30 +0530 Subject: [PATCH 40/58] Fix consts.py (mix up during rebase) --- monkey/monkey_island/cc/server_utils/consts.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/consts.py b/monkey/monkey_island/cc/server_utils/consts.py index 02f23428d..ef5d0733c 100644 --- a/monkey/monkey_island/cc/server_utils/consts.py +++ b/monkey/monkey_island/cc/server_utils/consts.py @@ -38,11 +38,7 @@ MONGO_EXECUTABLE_PATH = ( ) DEFAULT_SERVER_CONFIG_PATH = file_utils.expand_path( - os.path.join(DEFAULT_DATA_DIR, SERVER_CONFIG_FILENAME) -) - -DEFAULT_DEVELOP_SERVER_CONFIG_PATH = os.path.join( - MONKEY_ISLAND_ABS_PATH, "cc", f"{SERVER_CONFIG_FILENAME}.develop" + os.path.join(MONKEY_ISLAND_ABS_PATH, "cc", SERVER_CONFIG_FILENAME) ) DEFAULT_LOG_LEVEL = "INFO" From 011ab2a393955fc339615f6eff6b9d3c2e03a142 Mon Sep 17 00:00:00 2001 From: Shreya Date: Wed, 9 Jun 2021 11:14:06 +0530 Subject: [PATCH 41/58] Modify `has_expected_permissions()` to check files on Windows --- .../cc/server_utils/file_utils.py | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/file_utils.py b/monkey/monkey_island/cc/server_utils/file_utils.py index 6a474355a..83670d0c9 100644 --- a/monkey/monkey_island/cc/server_utils/file_utils.py +++ b/monkey/monkey_island/cc/server_utils/file_utils.py @@ -1,12 +1,43 @@ import os +from monkey_island.cc.environment.utils import is_windows_os + def expand_path(path: str) -> str: return os.path.expandvars(os.path.expanduser(path)) def has_expected_permissions(path: str, expected_permissions: int) -> bool: - file_mode = os.stat(path).st_mode - file_permissions = file_mode & 0o777 + if is_windows_os(): + import win32api # noqa: E402 + import win32security # noqa: E402 + + admins_sid, _, _ = win32security.LookupAccountName("", "Administrators") + user_sid, _, _ = win32security.LookupAccountName("", win32api.GetUserName()) + + security_descriptor = win32security.GetNamedSecurityInfo( + path, win32security.SE_FILE_OBJECT, win32security.DACL_SECURITY_INFORMATION + ) + + acl = security_descriptor.GetSecurityDescriptorDacl() + + for i in range(acl.GetAceCount()): + ace = acl.GetAce(i) + sid = ace[-1] + permissions = ace[1] + if sid == user_sid: + if oct(permissions & 0o777) != expected_permissions: + return False + elif sid == admins_sid: + continue + else: + if oct(permissions) != 0: # everyone but user & admins should have no permissions + return False + + return True + + else: + file_mode = os.stat(path).st_mode + file_permissions = file_mode & 0o777 return file_permissions == expected_permissions From f1d85dbc448f78e37e0802787d1956b218b1f7be Mon Sep 17 00:00:00 2001 From: Shreya Date: Wed, 9 Jun 2021 12:43:55 +0530 Subject: [PATCH 42/58] Change default cert permissions in bat script for creating default cert --- .../monkey_island/windows/create_certificate.bat | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/monkey/monkey_island/windows/create_certificate.bat b/monkey/monkey_island/windows/create_certificate.bat index 645c6fa25..3062f5c57 100644 --- a/monkey/monkey_island/windows/create_certificate.bat +++ b/monkey/monkey_island/windows/create_certificate.bat @@ -16,3 +16,17 @@ copy "%mydir%windows\openssl.cfg" "%mydir%bin\openssl\openssl.cfg" "%mydir%bin\openssl\openssl.exe" genrsa -out "%mydir%cc\server.key" 1024 "%mydir%bin\openssl\openssl.exe" req -new -config "%mydir%bin\openssl\openssl.cfg" -key "%mydir%cc\server.key" -out "%mydir%cc\server.csr" -subj "/OU=Monkey Department/CN=monkey.com" "%mydir%bin\openssl\openssl.exe" x509 -req -days 366 -in "%mydir%cc\server.csr" -signkey "%mydir%cc\server.key" -out "%mydir%cc\server.crt" + + +:: Change file permissions +SET adminsIdentity="BUILTIN\Administrators" +FOR /f %%O IN ('whoami') DO SET ownIdentity=%%O + +FOR %%F IN ("%mydir%cc\server.key", "%mydir%cc\server.csr", "%mydir%cc\server.crt") DO ( + + :: Remove all others and add admins rule (with full control) + echo y| cacls %%F" /p %adminsIdentity%:F + + :: Add user rule (with read) + echo y| cacls %%F /e /p %ownIdentity%:R +) From 438a63b0f441bca2429b8e72c95c00f07cf6c15c Mon Sep 17 00:00:00 2001 From: Shreya Date: Wed, 9 Jun 2021 13:34:26 +0530 Subject: [PATCH 43/58] Fix Windows file permission checking --- monkey/monkey_island/cc/server_utils/file_utils.py | 4 ++-- .../cc/setup/island_config_options_validator.py | 12 +++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/file_utils.py b/monkey/monkey_island/cc/server_utils/file_utils.py index 83670d0c9..777209171 100644 --- a/monkey/monkey_island/cc/server_utils/file_utils.py +++ b/monkey/monkey_island/cc/server_utils/file_utils.py @@ -26,12 +26,12 @@ def has_expected_permissions(path: str, expected_permissions: int) -> bool: sid = ace[-1] permissions = ace[1] if sid == user_sid: - if oct(permissions & 0o777) != expected_permissions: + if permissions != expected_permissions: return False elif sid == admins_sid: continue else: - if oct(permissions) != 0: # everyone but user & admins should have no permissions + if permissions != 2032127: # everyone but user & admins should have no permissions return False return True diff --git a/monkey/monkey_island/cc/setup/island_config_options_validator.py b/monkey/monkey_island/cc/setup/island_config_options_validator.py index 0a5247003..fdf25ef85 100644 --- a/monkey/monkey_island/cc/setup/island_config_options_validator.py +++ b/monkey/monkey_island/cc/setup/island_config_options_validator.py @@ -1,16 +1,17 @@ import os from common.utils.exceptions import InsecurePermissionsError +from monkey_island.cc.environment.utils import is_windows_os from monkey_island.cc.server_utils.file_utils import has_expected_permissions from monkey_island.cc.setup.island_config_options import IslandConfigOptions def raise_on_invalid_options(options: IslandConfigOptions): _raise_if_not_isfile(options.crt_path) - _raise_if_incorrect_permissions(options.crt_path, 0o400) + _raise_if_incorrect_permissions(options.crt_path, 0o400, 1179817) _raise_if_not_isfile(options.key_path) - _raise_if_incorrect_permissions(options.key_path, 0o400) + _raise_if_incorrect_permissions(options.key_path, 0o400, 1179817) def _raise_if_not_isfile(f: str): @@ -18,7 +19,12 @@ def _raise_if_not_isfile(f: str): raise FileNotFoundError(f"{f} does not exist or is not a regular file.") -def _raise_if_incorrect_permissions(f: str, expected_permissions: int): +def _raise_if_incorrect_permissions( + f: str, linux_expected_permissions: int, windows_expected_permissions: int +): + expected_permissions = ( + windows_expected_permissions if is_windows_os() else linux_expected_permissions + ) if not has_expected_permissions(f, expected_permissions): raise InsecurePermissionsError( f"The file {f} has incorrect permissions. Expected: {oct(expected_permissions)}" From fcd758e24f2f28328abcae9941c983acf6315f5a Mon Sep 17 00:00:00 2001 From: shreyamalviya Date: Wed, 9 Jun 2021 15:59:47 +0530 Subject: [PATCH 44/58] Fix Windows file permissions checking --- .../monkey_island/cc/server_utils/file_utils.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/file_utils.py b/monkey/monkey_island/cc/server_utils/file_utils.py index 777209171..82184e168 100644 --- a/monkey/monkey_island/cc/server_utils/file_utils.py +++ b/monkey/monkey_island/cc/server_utils/file_utils.py @@ -9,9 +9,16 @@ def expand_path(path: str) -> str: def has_expected_permissions(path: str, expected_permissions: int) -> bool: if is_windows_os(): + # checks that admin has any permissions, user has `expected_permissions`, + # and everyone else has no permissions + import win32api # noqa: E402 import win32security # noqa: E402 + FULL_CONTROL = 2032127 + ACE_TYPE_ALLOW = 0 + ACE_TYPE_DENY = 1 + admins_sid, _, _ = win32security.LookupAccountName("", "Administrators") user_sid, _, _ = win32security.LookupAccountName("", win32api.GetUserName()) @@ -23,15 +30,18 @@ def has_expected_permissions(path: str, expected_permissions: int) -> bool: for i in range(acl.GetAceCount()): ace = acl.GetAce(i) - sid = ace[-1] + ace_type, _ = ace[0] # 0 for allow, 1 for deny permissions = ace[1] + sid = ace[-1] + if sid == user_sid: - if permissions != expected_permissions: + if not (permissions == expected_permissions and ace_type == ACE_TYPE_ALLOW): return False elif sid == admins_sid: continue + # TODO: consider removing; so many system accounts/groups exist, it's likely to fail else: - if permissions != 2032127: # everyone but user & admins should have no permissions + if not (permissions == FULL_CONTROL and ace_type == ACE_TYPE_DENY): return False return True From cd2f627cc17e579850b9bab6e40d8bdbd9292040 Mon Sep 17 00:00:00 2001 From: shreyamalviya Date: Wed, 9 Jun 2021 16:01:47 +0530 Subject: [PATCH 45/58] Add tests for Windows file permissions checking --- .../cc/server_utils/test_file_utils.py | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py index e9dc3eddc..cfe7d0e77 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py @@ -1,6 +1,7 @@ import os import pytest +import subprocess from monkey_island.cc.server_utils import file_utils @@ -20,7 +21,7 @@ def test_expand_vars(patched_home_env): @pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") -def test_has_expected_permissions_true(tmpdir, create_empty_file): +def test_has_expected_permissions_true_linux(tmpdir, create_empty_file): file_name = f"{tmpdir}/test" create_empty_file(file_name) @@ -30,10 +31,30 @@ def test_has_expected_permissions_true(tmpdir, create_empty_file): @pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") -def test_has_expected_permissions_false(tmpdir, create_empty_file): +def test_has_expected_permissions_false_linux(tmpdir, create_empty_file): file_name = f"{tmpdir}/test" create_empty_file(file_name) os.chmod(file_name, 0o755) assert not file_utils.has_expected_permissions(file_name, 0o700) + + +@pytest.mark.skipif(os.name == "posix", reason="Tests Windows (not Posix) permissions.") +def test_has_expected_permissions_true_windows(tmpdir, create_empty_file): + file_name = f"{tmpdir}/test" + + create_empty_file(file_name) + subprocess.run(f"echo y| cacls {file_name} /p %USERNAME%:F", shell=True) + + assert file_utils.has_expected_permissions(file_name, 2032127) + + +@pytest.mark.skipif(os.name == "posix", reason="Tests Windows (not Posix) permissions.") +def test_has_expected_permissions_false_windows(tmpdir, create_empty_file): + file_name = f"{tmpdir}/test" + + create_empty_file(file_name) + subprocess.run(f"echo y| cacls {file_name} /p %USERNAME%:R", shell=True) + + assert not file_utils.has_expected_permissions(file_name, 2032127) From dc8e2b018d8f1bc772fdc4f987d369d2ae5ebbbe Mon Sep 17 00:00:00 2001 From: shreyamalviya Date: Wed, 9 Jun 2021 16:12:51 +0530 Subject: [PATCH 46/58] Fix/ignore flake8 and fix isort warnings --- monkey/monkey_island/cc/setup/config_setup.py | 2 +- .../monkey_island/cc/server_utils/test_file_utils.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/monkey/monkey_island/cc/setup/config_setup.py b/monkey/monkey_island/cc/setup/config_setup.py index 3621ea7fc..110df9f41 100644 --- a/monkey/monkey_island/cc/setup/config_setup.py +++ b/monkey/monkey_island/cc/setup/config_setup.py @@ -4,7 +4,7 @@ from monkey_island.cc.arg_parser import IslandCmdArgs from monkey_island.cc.environment import server_config_handler from monkey_island.cc.environment.utils import create_secure_directory from monkey_island.cc.server_utils import file_utils -from monkey_island.cc.server_utils.consts import DEFAULT_DATA_DIR, DEFAULT_SERVER_CONFIG_PATH +from monkey_island.cc.server_utils.consts import DEFAULT_SERVER_CONFIG_PATH from monkey_island.cc.setup.island_config_options import IslandConfigOptions diff --git a/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py index cfe7d0e77..e912ca043 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py @@ -1,7 +1,7 @@ import os +import subprocess import pytest -import subprocess from monkey_island.cc.server_utils import file_utils @@ -45,7 +45,7 @@ def test_has_expected_permissions_true_windows(tmpdir, create_empty_file): file_name = f"{tmpdir}/test" create_empty_file(file_name) - subprocess.run(f"echo y| cacls {file_name} /p %USERNAME%:F", shell=True) + subprocess.run(f"echo y| cacls {file_name} /p %USERNAME%:F", shell=True) # noqa: DUO116 assert file_utils.has_expected_permissions(file_name, 2032127) @@ -55,6 +55,6 @@ def test_has_expected_permissions_false_windows(tmpdir, create_empty_file): file_name = f"{tmpdir}/test" create_empty_file(file_name) - subprocess.run(f"echo y| cacls {file_name} /p %USERNAME%:R", shell=True) + subprocess.run(f"echo y| cacls {file_name} /p %USERNAME%:R", shell=True) # noqa: DUO116 assert not file_utils.has_expected_permissions(file_name, 2032127) From 945e1adf58daa93211a1ea95c65e4f4d9354e24c Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Wed, 9 Jun 2021 09:47:23 -0400 Subject: [PATCH 47/58] island: Split has_expected_permissions() into os-specific functions --- .../cc/server_utils/file_utils.py | 81 ++++++++++--------- 1 file changed, 44 insertions(+), 37 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/file_utils.py b/monkey/monkey_island/cc/server_utils/file_utils.py index 82184e168..cf8d344e0 100644 --- a/monkey/monkey_island/cc/server_utils/file_utils.py +++ b/monkey/monkey_island/cc/server_utils/file_utils.py @@ -9,45 +9,52 @@ def expand_path(path: str) -> str: def has_expected_permissions(path: str, expected_permissions: int) -> bool: if is_windows_os(): - # checks that admin has any permissions, user has `expected_permissions`, - # and everyone else has no permissions + return _has_expected_windows_permissions(path, expected_permissions) - import win32api # noqa: E402 - import win32security # noqa: E402 + return _has_expected_linux_permissions(path, expected_permissions) - FULL_CONTROL = 2032127 - ACE_TYPE_ALLOW = 0 - ACE_TYPE_DENY = 1 - admins_sid, _, _ = win32security.LookupAccountName("", "Administrators") - user_sid, _, _ = win32security.LookupAccountName("", win32api.GetUserName()) - - security_descriptor = win32security.GetNamedSecurityInfo( - path, win32security.SE_FILE_OBJECT, win32security.DACL_SECURITY_INFORMATION - ) - - acl = security_descriptor.GetSecurityDescriptorDacl() - - for i in range(acl.GetAceCount()): - ace = acl.GetAce(i) - ace_type, _ = ace[0] # 0 for allow, 1 for deny - permissions = ace[1] - sid = ace[-1] - - if sid == user_sid: - if not (permissions == expected_permissions and ace_type == ACE_TYPE_ALLOW): - return False - elif sid == admins_sid: - continue - # TODO: consider removing; so many system accounts/groups exist, it's likely to fail - else: - if not (permissions == FULL_CONTROL and ace_type == ACE_TYPE_DENY): - return False - - return True - - else: - file_mode = os.stat(path).st_mode - file_permissions = file_mode & 0o777 +def _has_expected_linux_permissions(path: str, expected_permissions: int) -> bool: + file_mode = os.stat(path).st_mode + file_permissions = file_mode & 0o777 return file_permissions == expected_permissions + + +def _has_expected_windows_permissions(path: str, expected_permissions: int) -> bool: + # checks that admin has any permissions, user has `expected_permissions`, + # and everyone else has no permissions + + import win32api # noqa: E402 + import win32security # noqa: E402 + + FULL_CONTROL = 2032127 + ACE_TYPE_ALLOW = 0 + ACE_TYPE_DENY = 1 + + admins_sid, _, _ = win32security.LookupAccountName("", "Administrators") + user_sid, _, _ = win32security.LookupAccountName("", win32api.GetUserName()) + + security_descriptor = win32security.GetNamedSecurityInfo( + path, win32security.SE_FILE_OBJECT, win32security.DACL_SECURITY_INFORMATION + ) + + acl = security_descriptor.GetSecurityDescriptorDacl() + + for i in range(acl.GetAceCount()): + ace = acl.GetAce(i) + ace_type, _ = ace[0] # 0 for allow, 1 for deny + permissions = ace[1] + sid = ace[-1] + + if sid == user_sid: + if not (permissions == expected_permissions and ace_type == ACE_TYPE_ALLOW): + return False + elif sid == admins_sid: + continue + # TODO: consider removing; so many system accounts/groups exist, it's likely to fail + else: + if not (permissions == FULL_CONTROL and ace_type == ACE_TYPE_DENY): + return False + + return True From 84b066442333ac85c113bd48e838c49308fb7919 Mon Sep 17 00:00:00 2001 From: shreyamalviya Date: Wed, 9 Jun 2021 19:37:58 +0530 Subject: [PATCH 48/58] Modify comment in monkey_island/cc/server_utils/file_utils.py --- monkey/monkey_island/cc/server_utils/file_utils.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/file_utils.py b/monkey/monkey_island/cc/server_utils/file_utils.py index cf8d344e0..e5a386684 100644 --- a/monkey/monkey_island/cc/server_utils/file_utils.py +++ b/monkey/monkey_island/cc/server_utils/file_utils.py @@ -22,8 +22,7 @@ def _has_expected_linux_permissions(path: str, expected_permissions: int) -> boo def _has_expected_windows_permissions(path: str, expected_permissions: int) -> bool: - # checks that admin has any permissions, user has `expected_permissions`, - # and everyone else has no permissions + # checks that user has `expected_permissions` and everyone else has no permissions import win32api # noqa: E402 import win32security # noqa: E402 From 424aceb1169f409a6cfbe6f3b22a96518bcf87ff Mon Sep 17 00:00:00 2001 From: shreyamalviya Date: Wed, 9 Jun 2021 19:38:45 +0530 Subject: [PATCH 49/58] Use constants instead of permission masks --- .../cc/setup/island_config_options_validator.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/monkey/monkey_island/cc/setup/island_config_options_validator.py b/monkey/monkey_island/cc/setup/island_config_options_validator.py index fdf25ef85..66df126be 100644 --- a/monkey/monkey_island/cc/setup/island_config_options_validator.py +++ b/monkey/monkey_island/cc/setup/island_config_options_validator.py @@ -7,11 +7,14 @@ from monkey_island.cc.setup.island_config_options import IslandConfigOptions def raise_on_invalid_options(options: IslandConfigOptions): + LINUX_READ_ONLY_BY_USER = 0o400 + WINDOWS_READ_ONLY = 1179817 + _raise_if_not_isfile(options.crt_path) - _raise_if_incorrect_permissions(options.crt_path, 0o400, 1179817) + _raise_if_incorrect_permissions(options.crt_path, LINUX_READ_ONLY_BY_USER, WINDOWS_READ_ONLY) _raise_if_not_isfile(options.key_path) - _raise_if_incorrect_permissions(options.key_path, 0o400, 1179817) + _raise_if_incorrect_permissions(options.key_path, LINUX_READ_ONLY_BY_USER, WINDOWS_READ_ONLY) def _raise_if_not_isfile(f: str): From 91aad66e164bc24725092f506ef5b84a0fd17ba5 Mon Sep 17 00:00:00 2001 From: shreyamalviya Date: Wed, 9 Jun 2021 19:44:35 +0530 Subject: [PATCH 50/58] Modify log message when checking file permissions Removed code to display it in octal since it makes no sense on Windows. Added `oct()` around linux permissions when expected_permissions is being defined. --- .../monkey_island/cc/setup/island_config_options_validator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/monkey/monkey_island/cc/setup/island_config_options_validator.py b/monkey/monkey_island/cc/setup/island_config_options_validator.py index 66df126be..fcfa524ca 100644 --- a/monkey/monkey_island/cc/setup/island_config_options_validator.py +++ b/monkey/monkey_island/cc/setup/island_config_options_validator.py @@ -26,9 +26,9 @@ def _raise_if_incorrect_permissions( f: str, linux_expected_permissions: int, windows_expected_permissions: int ): expected_permissions = ( - windows_expected_permissions if is_windows_os() else linux_expected_permissions + windows_expected_permissions if is_windows_os() else oct(linux_expected_permissions) ) if not has_expected_permissions(f, expected_permissions): raise InsecurePermissionsError( - f"The file {f} has incorrect permissions. Expected: {oct(expected_permissions)}" + f"The file {f} has incorrect permissions. Expected: {expected_permissions}" ) From 8a321f029094e4af6f6321837377b2193994d223 Mon Sep 17 00:00:00 2001 From: shreyamalviya Date: Wed, 9 Jun 2021 21:52:07 +0530 Subject: [PATCH 51/58] Add Windows tests for island_config_options_validator.py --- .../test_island_config_options_validator.py | 112 +++++++++++++++--- 1 file changed, 95 insertions(+), 17 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py index 1b1be6718..e3984099c 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py @@ -1,4 +1,5 @@ import os +import subprocess import pytest @@ -7,15 +8,19 @@ from monkey_island.cc.setup.island_config_options import IslandConfigOptions from monkey_island.cc.setup.island_config_options_validator import raise_on_invalid_options +LINUX_READ_ONLY_BY_USER = 0o400 +LINUX_RWX_BY_ALL = 0o777 + @pytest.fixture -def island_config_options(tmpdir, create_empty_file): +def linux_island_config_options(tmpdir, create_empty_file): crt_file = os.path.join(tmpdir, "test.crt") create_empty_file(crt_file) - os.chmod(crt_file, 0o400) + os.chmod(crt_file, LINUX_READ_ONLY_BY_USER) key_file = os.path.join(tmpdir, "test.key") create_empty_file(key_file) - os.chmod(key_file, 0o400) + os.chmod(key_file, LINUX_READ_ONLY_BY_USER) + return IslandConfigOptions( { "ssl_certificate": { @@ -27,41 +32,114 @@ def island_config_options(tmpdir, create_empty_file): @pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") -def test_valid_crt_and_key_paths(island_config_options): +def test_linux_valid_crt_and_key_paths(linux_island_config_options): try: - raise_on_invalid_options(island_config_options) + raise_on_invalid_options(linux_island_config_options) except Exception as ex: print(ex) assert False @pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") -def test_crt_path_does_not_exist(island_config_options): - os.remove(island_config_options.crt_path) +def test_linux_crt_path_does_not_exist(linux_island_config_options): + os.remove(linux_island_config_options.crt_path) with pytest.raises(FileNotFoundError): - raise_on_invalid_options(island_config_options) + raise_on_invalid_options(linux_island_config_options) @pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") -def test_crt_path_insecure_permissions(island_config_options): - os.chmod(island_config_options.crt_path, 0o777) +def test_linux_crt_path_insecure_permissions(linux_island_config_options): + os.chmod(linux_island_config_options.crt_path, LINUX_RWX_BY_ALL) with pytest.raises(InsecurePermissionsError): - raise_on_invalid_options(island_config_options) + raise_on_invalid_options(linux_island_config_options) @pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") -def test_key_path_does_not_exist(island_config_options): - os.remove(island_config_options.key_path) +def test_linux_key_path_does_not_exist(linux_island_config_options): + os.remove(linux_island_config_options.key_path) with pytest.raises(FileNotFoundError): - raise_on_invalid_options(island_config_options) + raise_on_invalid_options(linux_island_config_options) @pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") -def test_key_path_insecure_permissions(island_config_options): - os.chmod(island_config_options.key_path, 0o777) +def test_linux_key_path_insecure_permissions(linux_island_config_options): + os.chmod(linux_island_config_options.key_path, LINUX_RWX_BY_ALL) with pytest.raises(InsecurePermissionsError): - raise_on_invalid_options(island_config_options) + raise_on_invalid_options(linux_island_config_options) + + +@pytest.fixture +def windows_island_config_options(tmpdir, create_empty_file): + crt_file = os.path.join(tmpdir, "test.crt") + create_empty_file(crt_file) + cmd_to_change_permissions = get_windows_cmd_to_change_permissions(crt_file, 'R') + subprocess.run(cmd_to_change_permissions, shell=True) + + key_file = os.path.join(tmpdir, "test.key") + create_empty_file(key_file) + cmd_to_change_permissions = get_windows_cmd_to_change_permissions(key_file, 'R') + subprocess.run(cmd_to_change_permissions, shell=True) + + return IslandConfigOptions( + { + "ssl_certificate": { + "ssl_certificate_file": crt_file, + "ssl_certificate_key_file": key_file, + } + } + ) + + +def get_windows_cmd_to_change_permissions(file_name, permissions): + """ + :param file_name: name of file + :param permissions: can be: N (None), R (Read), W (Write), C (Change (write)), F (Full control) + """ + return f"echo y| cacls {file_name} /p %USERNAME%:{permissions}" + + +@pytest.mark.skipif(os.name == "posix", reason="Tests Windows (not Posix) permissions.") +def test_windows_valid_crt_and_key_paths(windows_island_config_options): + try: + raise_on_invalid_options(windows_island_config_options) + except Exception as ex: + print(ex) + assert False + + +@pytest.mark.skipif(os.name == "posix", reason="Tests Windows (not Posix) permissions.") +def test_windows_crt_path_does_not_exist(windows_island_config_options): + os.remove(windows_island_config_options.crt_path) + + with pytest.raises(FileNotFoundError): + raise_on_invalid_options(windows_island_config_options) + + +@pytest.mark.skipif(os.name == "posix", reason="Tests Windows (not Posix) permissions.") +def test_windows_crt_path_insecure_permissions(windows_island_config_options): + cmd_to_change_permissions = get_windows_cmd_to_change_permissions(windows_island_config_options.crt_path, 'W') + subprocess.run(cmd_to_change_permissions, shell=True) + + with pytest.raises(InsecurePermissionsError): + raise_on_invalid_options(windows_island_config_options) + + +@pytest.mark.skipif(os.name == "posix", reason="Tests Windows (not Posix) permissions.") +def test_windows_key_path_does_not_exist(windows_island_config_options): + os.remove(windows_island_config_options.key_path) + + with pytest.raises(FileNotFoundError): + raise_on_invalid_options(windows_island_config_options) + + +@pytest.mark.skipif(os.name == "posix", reason="Tests Windows (not Posix) permissions.") +def test_windows_key_path_insecure_permissions(windows_island_config_options): + cmd_to_change_permissions = get_windows_cmd_to_change_permissions(windows_island_config_options.key_path, 'W') + subprocess.run(cmd_to_change_permissions, shell=True) + + with pytest.raises(InsecurePermissionsError): + raise_on_invalid_options(windows_island_config_options) From 91a7c42a85494ad65a5cb5e36a1883f75501abec Mon Sep 17 00:00:00 2001 From: shreyamalviya Date: Wed, 9 Jun 2021 21:56:44 +0530 Subject: [PATCH 52/58] Format test_island_config_options_validator.py with black --- .../setup/test_island_config_options_validator.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py index e3984099c..d9df683b3 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py @@ -7,10 +7,10 @@ from common.utils.exceptions import InsecurePermissionsError from monkey_island.cc.setup.island_config_options import IslandConfigOptions from monkey_island.cc.setup.island_config_options_validator import raise_on_invalid_options - LINUX_READ_ONLY_BY_USER = 0o400 LINUX_RWX_BY_ALL = 0o777 + @pytest.fixture def linux_island_config_options(tmpdir, create_empty_file): crt_file = os.path.join(tmpdir, "test.crt") @@ -76,12 +76,12 @@ def test_linux_key_path_insecure_permissions(linux_island_config_options): def windows_island_config_options(tmpdir, create_empty_file): crt_file = os.path.join(tmpdir, "test.crt") create_empty_file(crt_file) - cmd_to_change_permissions = get_windows_cmd_to_change_permissions(crt_file, 'R') + cmd_to_change_permissions = get_windows_cmd_to_change_permissions(crt_file, "R") subprocess.run(cmd_to_change_permissions, shell=True) key_file = os.path.join(tmpdir, "test.key") create_empty_file(key_file) - cmd_to_change_permissions = get_windows_cmd_to_change_permissions(key_file, 'R') + cmd_to_change_permissions = get_windows_cmd_to_change_permissions(key_file, "R") subprocess.run(cmd_to_change_permissions, shell=True) return IslandConfigOptions( @@ -121,7 +121,9 @@ def test_windows_crt_path_does_not_exist(windows_island_config_options): @pytest.mark.skipif(os.name == "posix", reason="Tests Windows (not Posix) permissions.") def test_windows_crt_path_insecure_permissions(windows_island_config_options): - cmd_to_change_permissions = get_windows_cmd_to_change_permissions(windows_island_config_options.crt_path, 'W') + cmd_to_change_permissions = get_windows_cmd_to_change_permissions( + windows_island_config_options.crt_path, "W" + ) subprocess.run(cmd_to_change_permissions, shell=True) with pytest.raises(InsecurePermissionsError): @@ -138,7 +140,9 @@ def test_windows_key_path_does_not_exist(windows_island_config_options): @pytest.mark.skipif(os.name == "posix", reason="Tests Windows (not Posix) permissions.") def test_windows_key_path_insecure_permissions(windows_island_config_options): - cmd_to_change_permissions = get_windows_cmd_to_change_permissions(windows_island_config_options.key_path, 'W') + cmd_to_change_permissions = get_windows_cmd_to_change_permissions( + windows_island_config_options.key_path, "W" + ) subprocess.run(cmd_to_change_permissions, shell=True) with pytest.raises(InsecurePermissionsError): From 5aaa844289088718260b967026ef788e534ae309 Mon Sep 17 00:00:00 2001 From: shreyamalviya Date: Wed, 9 Jun 2021 22:00:44 +0530 Subject: [PATCH 53/58] Add missing constant in config_setup.py --- monkey/monkey_island/cc/setup/config_setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monkey/monkey_island/cc/setup/config_setup.py b/monkey/monkey_island/cc/setup/config_setup.py index 110df9f41..3621ea7fc 100644 --- a/monkey/monkey_island/cc/setup/config_setup.py +++ b/monkey/monkey_island/cc/setup/config_setup.py @@ -4,7 +4,7 @@ from monkey_island.cc.arg_parser import IslandCmdArgs from monkey_island.cc.environment import server_config_handler from monkey_island.cc.environment.utils import create_secure_directory from monkey_island.cc.server_utils import file_utils -from monkey_island.cc.server_utils.consts import DEFAULT_SERVER_CONFIG_PATH +from monkey_island.cc.server_utils.consts import DEFAULT_DATA_DIR, DEFAULT_SERVER_CONFIG_PATH from monkey_island.cc.setup.island_config_options import IslandConfigOptions From 7fe10af1b240c85293463b74b7946b43f53587b0 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Wed, 9 Jun 2021 13:55:48 -0400 Subject: [PATCH 54/58] island: Pass int, not str to has_expected_permissions() --- .../monkey_island/cc/setup/island_config_options_validator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monkey/monkey_island/cc/setup/island_config_options_validator.py b/monkey/monkey_island/cc/setup/island_config_options_validator.py index fcfa524ca..5febe16ab 100644 --- a/monkey/monkey_island/cc/setup/island_config_options_validator.py +++ b/monkey/monkey_island/cc/setup/island_config_options_validator.py @@ -26,7 +26,7 @@ def _raise_if_incorrect_permissions( f: str, linux_expected_permissions: int, windows_expected_permissions: int ): expected_permissions = ( - windows_expected_permissions if is_windows_os() else oct(linux_expected_permissions) + windows_expected_permissions if is_windows_os() else linux_expected_permissions ) if not has_expected_permissions(f, expected_permissions): raise InsecurePermissionsError( From cf5b1378f2520aa21c888eb05656cc1aeb922d40 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Wed, 9 Jun 2021 14:03:49 -0400 Subject: [PATCH 55/58] island: Consolidate duplicate code in test_island_config_options_validator --- .../test_island_config_options_validator.py | 69 +++++++++++-------- 1 file changed, 40 insertions(+), 29 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py index d9df683b3..8f79e0009 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py @@ -1,5 +1,6 @@ import os import subprocess +from collections.abc import Callable import pytest @@ -11,16 +12,7 @@ LINUX_READ_ONLY_BY_USER = 0o400 LINUX_RWX_BY_ALL = 0o777 -@pytest.fixture -def linux_island_config_options(tmpdir, create_empty_file): - crt_file = os.path.join(tmpdir, "test.crt") - create_empty_file(crt_file) - os.chmod(crt_file, LINUX_READ_ONLY_BY_USER) - - key_file = os.path.join(tmpdir, "test.key") - create_empty_file(key_file) - os.chmod(key_file, LINUX_READ_ONLY_BY_USER) - +def certificate_test_island_config_options(crt_file, key_file): return IslandConfigOptions( { "ssl_certificate": { @@ -31,6 +23,26 @@ def linux_island_config_options(tmpdir, create_empty_file): ) +@pytest.fixture +def linux_island_config_options(create_read_only_linux_file: Callable): + crt_file = create_read_only_linux_file("test.crt") + key_file = create_read_only_linux_file("test.key") + + return certificate_test_island_config_options(crt_file, key_file) + + +@pytest.fixture +def create_read_only_linux_file(tmpdir: str, create_empty_file: Callable) -> Callable: + def inner(file_name: str) -> str: + new_file = os.path.join(tmpdir, file_name) + create_empty_file(new_file) + os.chmod(new_file, LINUX_READ_ONLY_BY_USER) + + return new_file + + return inner + + @pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") def test_linux_valid_crt_and_key_paths(linux_island_config_options): try: @@ -73,25 +85,24 @@ def test_linux_key_path_insecure_permissions(linux_island_config_options): @pytest.fixture -def windows_island_config_options(tmpdir, create_empty_file): - crt_file = os.path.join(tmpdir, "test.crt") - create_empty_file(crt_file) - cmd_to_change_permissions = get_windows_cmd_to_change_permissions(crt_file, "R") - subprocess.run(cmd_to_change_permissions, shell=True) +def windows_island_config_options(tmpdir: str, create_read_only_windows_file: Callable): + crt_file = create_read_only_windows_file("test.crt") + key_file = create_read_only_windows_file("test.key") - key_file = os.path.join(tmpdir, "test.key") - create_empty_file(key_file) - cmd_to_change_permissions = get_windows_cmd_to_change_permissions(key_file, "R") - subprocess.run(cmd_to_change_permissions, shell=True) + return certificate_test_island_config_options(crt_file, key_file) - return IslandConfigOptions( - { - "ssl_certificate": { - "ssl_certificate_file": crt_file, - "ssl_certificate_key_file": key_file, - } - } - ) + +@pytest.fixture +def create_read_only_windows_file(tmpdir: str, create_empty_file: Callable) -> Callable: + def inner(file_name: str) -> str: + new_file = os.path.join(tmpdir, file_name) + create_empty_file(new_file) + cmd_to_change_permissions = get_windows_cmd_to_change_permissions(new_file, "R") + subprocess.run(cmd_to_change_permissions, shell=True) # noqa DUO116 + + return new_file + + return inner def get_windows_cmd_to_change_permissions(file_name, permissions): @@ -124,7 +135,7 @@ def test_windows_crt_path_insecure_permissions(windows_island_config_options): cmd_to_change_permissions = get_windows_cmd_to_change_permissions( windows_island_config_options.crt_path, "W" ) - subprocess.run(cmd_to_change_permissions, shell=True) + subprocess.run(cmd_to_change_permissions, shell=True) # noqa DUO116 with pytest.raises(InsecurePermissionsError): raise_on_invalid_options(windows_island_config_options) @@ -143,7 +154,7 @@ def test_windows_key_path_insecure_permissions(windows_island_config_options): cmd_to_change_permissions = get_windows_cmd_to_change_permissions( windows_island_config_options.key_path, "W" ) - subprocess.run(cmd_to_change_permissions, shell=True) + subprocess.run(cmd_to_change_permissions, shell=True) # noqa DUO116 with pytest.raises(InsecurePermissionsError): raise_on_invalid_options(windows_island_config_options) From 67d4f18d650f09542a499237ff300c66bb7c4ce0 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Wed, 9 Jun 2021 14:12:34 -0400 Subject: [PATCH 56/58] tests: Refactor create_empty_file() -> create_empty_tmp_file() --- .../cc/server_utils/test_file_utils.py | 24 +++++++------------ .../test_island_config_options_validator.py | 18 +++++++------- .../unit_tests/monkey_island/conftest.py | 10 +++++--- 3 files changed, 23 insertions(+), 29 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py b/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py index e912ca043..6297aada9 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py +++ b/monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py @@ -21,40 +21,32 @@ def test_expand_vars(patched_home_env): @pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") -def test_has_expected_permissions_true_linux(tmpdir, create_empty_file): - file_name = f"{tmpdir}/test" - - create_empty_file(file_name) +def test_has_expected_permissions_true_linux(tmpdir, create_empty_tmp_file): + file_name = create_empty_tmp_file("test") os.chmod(file_name, 0o754) assert file_utils.has_expected_permissions(file_name, 0o754) @pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.") -def test_has_expected_permissions_false_linux(tmpdir, create_empty_file): - file_name = f"{tmpdir}/test" - - create_empty_file(file_name) +def test_has_expected_permissions_false_linux(tmpdir, create_empty_tmp_file): + file_name = create_empty_tmp_file("test") os.chmod(file_name, 0o755) assert not file_utils.has_expected_permissions(file_name, 0o700) @pytest.mark.skipif(os.name == "posix", reason="Tests Windows (not Posix) permissions.") -def test_has_expected_permissions_true_windows(tmpdir, create_empty_file): - file_name = f"{tmpdir}/test" - - create_empty_file(file_name) +def test_has_expected_permissions_true_windows(tmpdir, create_empty_tmp_file): + file_name = create_empty_tmp_file("test") subprocess.run(f"echo y| cacls {file_name} /p %USERNAME%:F", shell=True) # noqa: DUO116 assert file_utils.has_expected_permissions(file_name, 2032127) @pytest.mark.skipif(os.name == "posix", reason="Tests Windows (not Posix) permissions.") -def test_has_expected_permissions_false_windows(tmpdir, create_empty_file): - file_name = f"{tmpdir}/test" - - create_empty_file(file_name) +def test_has_expected_permissions_false_windows(tmpdir, create_empty_tmp_file): + file_name = create_empty_tmp_file("test") subprocess.run(f"echo y| cacls {file_name} /p %USERNAME%:R", shell=True) # noqa: DUO116 assert not file_utils.has_expected_permissions(file_name, 2032127) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py index 8f79e0009..b6d9eeb85 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py @@ -32,13 +32,12 @@ def linux_island_config_options(create_read_only_linux_file: Callable): @pytest.fixture -def create_read_only_linux_file(tmpdir: str, create_empty_file: Callable) -> Callable: +def create_read_only_linux_file(tmpdir: str, create_empty_tmp_file: Callable) -> Callable: def inner(file_name: str) -> str: - new_file = os.path.join(tmpdir, file_name) - create_empty_file(new_file) - os.chmod(new_file, LINUX_READ_ONLY_BY_USER) + full_file_path = create_empty_tmp_file(file_name) + os.chmod(full_file_path, LINUX_READ_ONLY_BY_USER) - return new_file + return full_file_path return inner @@ -93,14 +92,13 @@ def windows_island_config_options(tmpdir: str, create_read_only_windows_file: Ca @pytest.fixture -def create_read_only_windows_file(tmpdir: str, create_empty_file: Callable) -> Callable: +def create_read_only_windows_file(tmpdir: str, create_empty_tmp_file: Callable) -> Callable: def inner(file_name: str) -> str: - new_file = os.path.join(tmpdir, file_name) - create_empty_file(new_file) - cmd_to_change_permissions = get_windows_cmd_to_change_permissions(new_file, "R") + full_file_path = create_empty_tmp_file(file_name) + cmd_to_change_permissions = get_windows_cmd_to_change_permissions(full_file_path, "R") subprocess.run(cmd_to_change_permissions, shell=True) # noqa DUO116 - return new_file + return full_file_path return inner diff --git a/monkey/tests/unit_tests/monkey_island/conftest.py b/monkey/tests/unit_tests/monkey_island/conftest.py index 2baa97a02..538576aef 100644 --- a/monkey/tests/unit_tests/monkey_island/conftest.py +++ b/monkey/tests/unit_tests/monkey_island/conftest.py @@ -1,4 +1,5 @@ import os +from collections.abc import Callable import pytest @@ -16,9 +17,12 @@ def patched_home_env(monkeypatch, tmpdir): @pytest.fixture -def create_empty_file(): - def inner(file_name): - with open(file_name, "w"): +def create_empty_tmp_file(tmpdir: str) -> Callable: + def inner(file_name: str): + new_file = os.path.join(tmpdir, file_name) + with open(new_file, "w"): pass + return new_file + return inner From 9131f86215d51cd1785bb79169b82b740198563b Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Wed, 9 Jun 2021 14:20:58 -0400 Subject: [PATCH 57/58] island: remove misleading comment We don't check admin permissions at all, and admin is included in "everyone else". --- monkey/monkey_island/cc/server_utils/file_utils.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/file_utils.py b/monkey/monkey_island/cc/server_utils/file_utils.py index e5a386684..668aa4356 100644 --- a/monkey/monkey_island/cc/server_utils/file_utils.py +++ b/monkey/monkey_island/cc/server_utils/file_utils.py @@ -22,8 +22,6 @@ def _has_expected_linux_permissions(path: str, expected_permissions: int) -> boo def _has_expected_windows_permissions(path: str, expected_permissions: int) -> bool: - # checks that user has `expected_permissions` and everyone else has no permissions - import win32api # noqa: E402 import win32security # noqa: E402 From 6aa76497eced804ccaa4343bea4ec618869c128a Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Wed, 9 Jun 2021 14:48:59 -0400 Subject: [PATCH 58/58] island: Use config file in ~/.monkey_island if it exists --- monkey/monkey_island/cc/setup/config_setup.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/monkey/monkey_island/cc/setup/config_setup.py b/monkey/monkey_island/cc/setup/config_setup.py index 3621ea7fc..d1e3e984b 100644 --- a/monkey/monkey_island/cc/setup/config_setup.py +++ b/monkey/monkey_island/cc/setup/config_setup.py @@ -4,7 +4,7 @@ from monkey_island.cc.arg_parser import IslandCmdArgs from monkey_island.cc.environment import server_config_handler from monkey_island.cc.environment.utils import create_secure_directory from monkey_island.cc.server_utils import file_utils -from monkey_island.cc.server_utils.consts import DEFAULT_DATA_DIR, DEFAULT_SERVER_CONFIG_PATH +from monkey_island.cc.server_utils.consts import DEFAULT_SERVER_CONFIG_PATH from monkey_island.cc.setup.island_config_options import IslandConfigOptions @@ -23,8 +23,12 @@ def _setup_config_by_cmd_arg(server_config_path) -> Tuple[IslandConfigOptions, s def _setup_default_config() -> Tuple[IslandConfigOptions, str]: - server_config_path = DEFAULT_SERVER_CONFIG_PATH - create_secure_directory(DEFAULT_DATA_DIR, create_parent_dirs=False) - server_config_handler.create_default_server_config_file() + default_config = server_config_handler.load_server_config_from_file(DEFAULT_SERVER_CONFIG_PATH) + default_data_dir = default_config.data_dir + + create_secure_directory(default_data_dir, create_parent_dirs=False) + + server_config_path = server_config_handler.create_default_server_config_file(default_data_dir) config = server_config_handler.load_server_config_from_file(server_config_path) + return config, server_config_path