Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Create mongo key file securely #1232

Merged
merged 31 commits into from
Jun 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
26ae50f
island: Create mongo key file securely before using it
shreyamalviya Jun 14, 2021
5d7d86a
island: Modify log message when creating secure directory on Windows
shreyamalviya Jun 14, 2021
ff85360
island: Add functions to create a file securely on Linux and Windows
shreyamalviya Jun 14, 2021
8dd4bb5
island: Use 'x' instead of '_' when creating a secure file
shreyamalviya Jun 14, 2021
8b932e1
island: Add os.O_EXCL flag so that an error is thrown if trying to cr…
shreyamalviya Jun 14, 2021
5fe0c80
island: Can't use `with` with `os.open()`, use `os.close()` to close …
shreyamalviya Jun 14, 2021
248d577
tests: Add unit tests for securly creating a file
shreyamalviya Jun 14, 2021
6d360ef
tests: Fix assertion in `test_create_secure_directory__perm_linux()`
shreyamalviya Jun 14, 2021
c0d9489
tests: Extract duplicate code in Windows tests in test_utils
shreyamalviya Jun 14, 2021
37eda4e
island: Fix secure file creation on Windows
shreyamalviya Jun 14, 2021
1467a53
island: Use win32file.CloseHandle() to close file descriptor on Windows
shreyamalviya Jun 14, 2021
7ddb986
tests: Fix file creation unit tests in test_utils.py
shreyamalviya Jun 14, 2021
1170b17
island: Fix Windows' secure file creation by using a different file flag
shreyamalviya Jun 14, 2021
443b66e
tests: Remove accidental code in `_get_acl_and_sid_from_path()` in te…
shreyamalviya Jun 14, 2021
5ea046e
island: Format cc/environment/utils.py with black
shreyamalviya Jun 14, 2021
d7565fc
island: Use stat.S_IRWXU in place of 0o700 in cc/environment/utils.py
shreyamalviya Jun 15, 2021
9187334
tests: Add comment to `test_create_secure_directory__perm_windows()` …
shreyamalviya Jun 15, 2021
b5f092a
island: Move code from cc/environment/utils.py to cc/server_utils/fil…
shreyamalviya Jun 15, 2021
5abcadc
tests: Move tests from test_utils.py to test_file_utils.py
shreyamalviya Jun 15, 2021
e011654
island, tests: Run isort and black on previously changed files
shreyamalviya Jun 15, 2021
e90bf52
island: Use `Path().touch()` instead of `os.open()` when securely cre…
shreyamalviya Jun 15, 2021
8b2c3ef
island: Remove execute bit from "secure" file creation
mssalvatore Jun 15, 2021
6b4a090
island: use constants for permissions mode in test_file_utils.py
mssalvatore Jun 15, 2021
14371f3
island: Return file descriptor when creating secure file
shreyamalviya Jun 15, 2021
b648452
island: Fix comment and statement formatting in file_utils.py
shreyamalviya Jun 15, 2021
37889d0
island: Extract code to `_get_null_value_for_win32()` in file_utils.py
shreyamalviya Jun 15, 2021
22c3c5a
tests: Fix secure file creation tests as per latest changes
shreyamalviya Jun 15, 2021
64ac1fe
island: Add type hinting in file_utils.py
shreyamalviya Jun 15, 2021
80bfd90
island: Specify mode to open new secure file in, in encryptor.py
shreyamalviya Jun 15, 2021
327ff7a
island: Remove isfile() check from get_file_descriptor_for_new_secure…
mssalvatore Jun 15, 2021
44bdfa5
island: Rename create_secure_file tests
mssalvatore Jun 15, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 0 additions & 51 deletions monkey/monkey_island/cc/environment/utils.py

This file was deleted.

2 changes: 1 addition & 1 deletion monkey/monkey_island/cc/server_utils/consts.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
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
from monkey_island.cc.server_utils.file_utils import is_windows_os

__author__ = "itay.mizeretz"

Expand Down
4 changes: 3 additions & 1 deletion monkey/monkey_island/cc/server_utils/encryptor.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
from Crypto import Random # noqa: DUO133 # nosec: B413
from Crypto.Cipher import AES # noqa: DUO133 # nosec: B413

from monkey_island.cc.server_utils.file_utils import get_file_descriptor_for_new_secure_file

__author__ = "itay.mizeretz"

_encryptor = None
Expand All @@ -25,7 +27,7 @@ def __init__(self, password_file_dir):

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

def _load_existing_key(self, password_file):
Expand Down
105 changes: 105 additions & 0 deletions monkey/monkey_island/cc/server_utils/file_utils.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,110 @@
import logging
import os
import platform
import stat

LOG = logging.getLogger(__name__)


def is_windows_os() -> bool:
return platform.system() == "Windows"


if is_windows_os():
import win32file
import win32job
import win32security

import monkey_island.cc.server_utils.windows_permissions as windows_permissions


def expand_path(path: str) -> str:
return os.path.expandvars(os.path.expanduser(path))


def create_secure_directory(path: str):
if not os.path.isdir(path):
if is_windows_os():
_create_secure_directory_windows(path)
else:
_create_secure_directory_linux(path)


def _create_secure_directory_linux(path: str):
try:
# Don't split directory creation and permission setting
# because it will temporarily create an accessible directory which anyone can use.
os.mkdir(path, mode=stat.S_IRWXU)

except Exception as ex:
LOG.error(f'Could not create a directory at "{path}": {str(ex)}')
raise ex


def _create_secure_directory_windows(path: str):
try:
security_attributes = win32security.SECURITY_ATTRIBUTES()
security_attributes.SECURITY_DESCRIPTOR = (
windows_permissions.get_security_descriptor_for_owner_only_perms()
)
win32file.CreateDirectory(path, security_attributes)

except Exception as ex:
LOG.error(f'Could not create a directory at "{path}": {str(ex)}')
raise ex


def get_file_descriptor_for_new_secure_file(path: str) -> int:
if is_windows_os():
return _get_file_descriptor_for_new_secure_file_windows(path)
else:
return _get_file_descriptor_for_new_secure_file_linux(path)


def _get_file_descriptor_for_new_secure_file_linux(path: str) -> int:
try:
mode = stat.S_IRUSR | stat.S_IWUSR
flags = (
os.O_RDWR | os.O_CREAT | os.O_EXCL
) # read/write, create new, throw error if file exists
fd = os.open(path, flags, mode)

return fd

except Exception as ex:
LOG.error(f'Could not create a file at "{path}": {str(ex)}')
raise ex


def _get_file_descriptor_for_new_secure_file_windows(path: str) -> int:
try:
file_access = win32file.GENERIC_READ | win32file.GENERIC_WRITE
# subsequent open operations on the object will succeed only if read access is requested
file_sharing = win32file.FILE_SHARE_READ
security_attributes = win32security.SECURITY_ATTRIBUTES()
security_attributes.SECURITY_DESCRIPTOR = (
windows_permissions.get_security_descriptor_for_owner_only_perms()
)
file_creation = win32file.CREATE_NEW # fails if file exists
file_attributes = win32file.FILE_FLAG_BACKUP_SEMANTICS
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need this flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using win32file.FILE_ATTRIBUTE_NORMAL throws an access denied error. win32file.FILE_FLAG_BACKUP_SEMANTICS doesn't probably because of:

The operating system ensures that the calling process overrides file security checks, provided it has the necessary permission to do so.

(Source: http://timgolden.me.uk/pywin32-docs/win32file_FILE_FLAG_BACKUP_SEMANTICS.html)


fd = win32file.CreateFile(
path,
file_access,
file_sharing,
security_attributes,
file_creation,
file_attributes,
_get_null_value_for_win32(),
)

return fd

except Exception as ex:
LOG.error(f'Could not create a file at "{path}": {str(ex)}')
raise ex


def _get_null_value_for_win32() -> None:
# https://stackoverflow.com/questions/46800142/in-python-with-pywin32-win32job-the-createjobobject-function-how-do-i-pass-nu # noqa: E501
return win32job.CreateJobObject(None, "")
2 changes: 1 addition & 1 deletion monkey/monkey_island/cc/setup/config_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

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.file_utils import create_secure_directory
from monkey_island.cc.setup.island_config_options import IslandConfigOptions


Expand Down
2 changes: 1 addition & 1 deletion monkey/monkey_island/cc/setup/mongo/mongo_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import time

from monkey_island.cc.database import get_db_version, is_db_server_up
from monkey_island.cc.environment.utils import create_secure_directory
from monkey_island.cc.server_utils.file_utils import create_secure_directory
from monkey_island.cc.setup.mongo import mongo_connector
from monkey_island.cc.setup.mongo.mongo_connector import MONGO_DB_HOST, MONGO_DB_NAME, MONGO_DB_PORT
from monkey_island.cc.setup.mongo.mongo_db_process import MongoDbProcess
Expand Down
65 changes: 0 additions & 65 deletions monkey/tests/unit_tests/monkey_island/cc/environment/test_utils.py

This file was deleted.

Loading