Merge pull request #1221 from guardicore/remove-ssl-perms-checks

Remove ssl perms checks

docs/content/setup/docker.md

@@ -68,6 +68,7 @@ been signed by a private certificate authority.
 
     ```bash
     mkdir ./monkey_island_data
+    chmod 700 ./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.
@@ -77,18 +78,18 @@ been signed by a private certificate authority.
         --rm \
         --name monkey-island \
         --network=host \
-        --user $(id -u ${USER}):$(id -g ${USER}) \
+        --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) Move your `.crt` and `.key` files to `./monkey_island_data`.
+1. 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.
+1. Make sure that your `.crt` and `.key` files are readable and writeable only by you.
 
     ```bash
-    chmod 400 <PATH_TO_KEY_FILE>
+    chmod 600 ./monkey_island_data/<KEY_FILE>
-    chmod 400 <PATH_TO_CRT_FILE>
+    chmod 600 ./monkey_island_data/<CRT_FILE>
     ```
 
 1.  Edit `./monkey_island_data/server_config.json` to configure Monkey Island
@@ -106,8 +107,8 @@ been signed by a private certificate authority.
         "start_mongodb": false
      },
       "ssl_certificate": {
-        "ssl_certificate_file": "<PATH_TO_CRT_FILE>",
+        "ssl_certificate_file": "/monkey_island_data/<CRT_FILE>",
-        "ssl_certificate_key_file": "<PATH_TO_KEY_FILE>",
+        "ssl_certificate_key_file": "/monkey_island_data/<KEY_FILE>"
       }
     }
     ```
@@ -118,7 +119,7 @@ been signed by a private certificate authority.
     sudo docker run \
         --name monkey-island \
         --network=host \
-        --user $(id -u ${USER}):$(id -g ${USER}) \
+        --user "$(id -u ${USER}):$(id -g ${USER})" \
         --volume "$(realpath ./monkey_island_data)":/monkey_island_data \
         guardicore/monkey-island:1.10.0
     ```

docs/content/setup/linux.md

@@ -41,12 +41,11 @@ private certificate authority.
 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
+1. Make sure that your `.crt` and `.key` files are readable only by you.
-   by you.
 
     ```bash
-    chmod 400 <PATH_TO_KEY_FILE>
+    chmod 600 <PATH_TO_KEY_FILE>
-    chmod 400 <PATH_TO_CRT_FILE>
+    chmod 600 <PATH_TO_CRT_FILE>
     ```
 
 1.  Edit `$HOME/.monkey_island/server_config.json` to configure Monkey Island
@@ -65,7 +64,7 @@ private certificate authority.
      },
       "ssl_certificate": {
         "ssl_certificate_file": "<PATH_TO_CRT_FILE>",
-        "ssl_certificate_key_file": "<PATH_TO_KEY_FILE>",
+        "ssl_certificate_key_file": "<PATH_TO_KEY_FILE>"
       }
     }
     ```

monkey/common/utils/exceptions.py

@@ -52,7 +52,3 @@ 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 """

monkey/monkey_island/cc/server_utils/file_utils.py

@@ -1,57 +1,5 @@
 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:
-    if is_windows_os():
-        return _has_expected_windows_permissions(path, expected_permissions)
-
-    return _has_expected_linux_permissions(path, expected_permissions)
-
-
-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:
-    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

monkey/monkey_island/cc/setup/island_config_options_validator.py

@@ -1,34 +1,13 @@
 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):
-    LINUX_READ_ONLY_BY_USER = 0o400
-    WINDOWS_READ_ONLY = 1179817
-
     _raise_if_not_isfile(options.crt_path)
-    _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, LINUX_READ_ONLY_BY_USER, WINDOWS_READ_ONLY)
 
 
 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, 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: {expected_permissions}"
-        )

monkey/monkey_island/linux/create_certificate.sh

@@ -21,15 +21,15 @@ umask 377
 
 echo "Generating key in $server_root/server.key..."
 openssl genrsa -out "$server_root"/server.key 2048
-chmod 400 "$server_root"/server.key
+chmod 600 "$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
+chmod 600 "$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
+chmod 600 "$server_root"/server.crt
 
 
 # Shove some new random data into the file to override the original seed we put in.

monkey/tests/unit_tests/monkey_island/cc/server_utils/test_file_utils.py

@@ -1,7 +1,4 @@
 import os
-import subprocess
-
-import pytest
 
 from monkey_island.cc.server_utils import file_utils
 
@@ -18,35 +15,3 @@ 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_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_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_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_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)

monkey/tests/unit_tests/monkey_island/cc/setup/test_island_config_options_validator.py

@@ -1,16 +1,11 @@
 import os
-import subprocess
 from collections.abc import Callable
 
 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
 
-LINUX_READ_ONLY_BY_USER = 0o400
-LINUX_RWX_BY_ALL = 0o777
-
 
 def certificate_test_island_config_options(crt_file, key_file):
     return IslandConfigOptions(
@@ -24,24 +19,13 @@ def certificate_test_island_config_options(crt_file, key_file):
 
 
 @pytest.fixture
-def linux_island_config_options(create_read_only_linux_file: Callable):
+def linux_island_config_options(create_empty_tmp_file: Callable):
-    crt_file = create_read_only_linux_file("test.crt")
+    crt_file = create_empty_tmp_file("test.crt")
-    key_file = create_read_only_linux_file("test.key")
+    key_file = create_empty_tmp_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_tmp_file: Callable) -> Callable:
-    def inner(file_name: str) -> str:
-        full_file_path = create_empty_tmp_file(file_name)
-        os.chmod(full_file_path, LINUX_READ_ONLY_BY_USER)
-
-        return full_file_path
-
-    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:
@@ -59,14 +43,6 @@ def test_linux_crt_path_does_not_exist(linux_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_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(linux_island_config_options)
-
-
 @pytest.mark.skipif(os.name != "posix", reason="Tests Posix (not Windows) permissions.")
 def test_linux_key_path_does_not_exist(linux_island_config_options):
     os.remove(linux_island_config_options.key_path)
@@ -75,42 +51,14 @@ def test_linux_key_path_does_not_exist(linux_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_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(linux_island_config_options)
-
-
 @pytest.fixture
-def windows_island_config_options(tmpdir: str, create_read_only_windows_file: Callable):
+def windows_island_config_options(tmpdir: str, create_empty_tmp_file: Callable):
-    crt_file = create_read_only_windows_file("test.crt")
+    crt_file = create_empty_tmp_file("test.crt")
-    key_file = create_read_only_windows_file("test.key")
+    key_file = create_empty_tmp_file("test.key")
 
     return certificate_test_island_config_options(crt_file, key_file)
 
 
-@pytest.fixture
-def create_read_only_windows_file(tmpdir: str, create_empty_tmp_file: Callable) -> Callable:
-    def inner(file_name: str) -> str:
-        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 full_file_path
-
-    return inner
-
-
-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:
@@ -128,31 +76,9 @@ def test_windows_crt_path_does_not_exist(windows_island_config_options):
         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)  # noqa DUO116
-
-    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)  # noqa DUO116
-
-    with pytest.raises(InsecurePermissionsError):
-        raise_on_invalid_options(windows_island_config_options)