From 885a871be82b14021b490fc3fb30ec0900aa1b49 Mon Sep 17 00:00:00 2001 From: Shreya Malviya Date: Fri, 1 Apr 2022 17:09:50 +0530 Subject: [PATCH 1/4] Agent: Add timeouts to utils/linux/users.py --- monkey/infection_monkey/utils/linux/users.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/monkey/infection_monkey/utils/linux/users.py b/monkey/infection_monkey/utils/linux/users.py index 002c63f96..df01f9f18 100644 --- a/monkey/infection_monkey/utils/linux/users.py +++ b/monkey/infection_monkey/utils/linux/users.py @@ -3,6 +3,7 @@ import logging import shlex import subprocess +from common.common_consts.timeouts import SHORT_REQUEST_TIMEOUT from infection_monkey.utils.auto_new_user import AutoNewUser logger = logging.getLogger(__name__) @@ -43,7 +44,9 @@ class AutoNewLinuxUser(AutoNewUser): logger.debug( "Trying to add {} with commands {}".format(self.username, str(commands_to_add_user)) ) - _ = subprocess.check_output(commands_to_add_user, stderr=subprocess.STDOUT) + _ = subprocess.check_output( + commands_to_add_user, stderr=subprocess.STDOUT, timeout=SHORT_REQUEST_TIMEOUT + ) def __enter__(self): return self # No initialization/logging on needed in Linux @@ -52,7 +55,7 @@ class AutoNewLinuxUser(AutoNewUser): command_as_new_user = shlex.split( "sudo -u {username} {command}".format(username=self.username, command=command) ) - return subprocess.call(command_as_new_user) + return subprocess.call(command_as_new_user, timeout=SHORT_REQUEST_TIMEOUT) def __exit__(self, _exc_type, value, traceback): # delete the user. @@ -62,4 +65,6 @@ class AutoNewLinuxUser(AutoNewUser): self.username, str(commands_to_delete_user) ) ) - _ = subprocess.check_output(commands_to_delete_user, stderr=subprocess.STDOUT) + _ = subprocess.check_output( + commands_to_delete_user, stderr=subprocess.STDOUT, timeout=SHORT_REQUEST_TIMEOUT + ) From 88788d24d0157afd2d8bae30ccd5bc0ac57395c8 Mon Sep 17 00:00:00 2001 From: Shreya Malviya Date: Fri, 1 Apr 2022 18:05:20 +0530 Subject: [PATCH 2/4] Agent: Add timeouts to utils/windows/users.py --- monkey/infection_monkey/utils/windows/users.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/monkey/infection_monkey/utils/windows/users.py b/monkey/infection_monkey/utils/windows/users.py index e0da2ded3..9e3af52a1 100644 --- a/monkey/infection_monkey/utils/windows/users.py +++ b/monkey/infection_monkey/utils/windows/users.py @@ -1,6 +1,7 @@ import logging import subprocess +from common.common_consts.timeouts import SHORT_REQUEST_TIMEOUT from infection_monkey.utils.auto_new_user import AutoNewUser from infection_monkey.utils.environment import is_windows_os from infection_monkey.utils.new_user_error import NewUserError @@ -49,7 +50,9 @@ class AutoNewWindowsUser(AutoNewUser): windows_cmds = get_windows_commands_to_add_user(self.username, self.password, True) logger.debug("Trying to add {} with commands {}".format(self.username, str(windows_cmds))) - _ = subprocess.check_output(windows_cmds, stderr=subprocess.STDOUT) + _ = subprocess.check_output( + windows_cmds, stderr=subprocess.STDOUT, timeout=SHORT_REQUEST_TIMEOUT + ) def __enter__(self): try: @@ -124,7 +127,9 @@ class AutoNewWindowsUser(AutoNewUser): self.username, str(commands_to_deactivate_user) ) ) - _ = subprocess.check_output(commands_to_deactivate_user, stderr=subprocess.STDOUT) + _ = subprocess.check_output( + commands_to_deactivate_user, stderr=subprocess.STDOUT, timeout=SHORT_REQUEST_TIMEOUT + ) except Exception as err: raise NewUserError("Can't deactivate user {}. Info: {}".format(self.username, err)) @@ -136,6 +141,8 @@ class AutoNewWindowsUser(AutoNewUser): self.username, str(commands_to_delete_user) ) ) - _ = subprocess.check_output(commands_to_delete_user, stderr=subprocess.STDOUT) + _ = subprocess.check_output( + commands_to_delete_user, stderr=subprocess.STDOUT, timeout=SHORT_REQUEST_TIMEOUT + ) except Exception as err: raise NewUserError("Can't delete user {}. Info: {}".format(self.username, err)) From b312c509ce5bffd60a52178cf7323884450bb6af Mon Sep 17 00:00:00 2001 From: Shreya Malviya Date: Fri, 1 Apr 2022 18:11:55 +0530 Subject: [PATCH 3/4] UT: Fix tests for new user creation --- .../tests/unit_tests/infection_monkey/utils/linux/test_users.py | 2 +- .../infection_monkey/utils/windows/test_windows_users.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/monkey/tests/unit_tests/infection_monkey/utils/linux/test_users.py b/monkey/tests/unit_tests/infection_monkey/utils/linux/test_users.py index 8b0408c0a..2c079309f 100644 --- a/monkey/tests/unit_tests/infection_monkey/utils/linux/test_users.py +++ b/monkey/tests/unit_tests/infection_monkey/utils/linux/test_users.py @@ -9,7 +9,7 @@ TEST_USER = "test_user" @pytest.fixture def subprocess_check_output_spy(monkeypatch): - def mock_check_output(command, stderr): + def mock_check_output(command, stderr, timeout): mock_check_output.command = command mock_check_output.command = "" diff --git a/monkey/tests/unit_tests/infection_monkey/utils/windows/test_windows_users.py b/monkey/tests/unit_tests/infection_monkey/utils/windows/test_windows_users.py index cb5e0746b..09762fe57 100644 --- a/monkey/tests/unit_tests/infection_monkey/utils/windows/test_windows_users.py +++ b/monkey/tests/unit_tests/infection_monkey/utils/windows/test_windows_users.py @@ -10,7 +10,7 @@ TEST_USER = "test_user" @pytest.fixture def subprocess_check_output_spy(monkeypatch): - def mock_check_output(command, stderr): + def mock_check_output(command, stderr, timeout): mock_check_output.command = command mock_check_output.command = "" From bb798898c1918e4a6d250e167671c59ce9ded2bb Mon Sep 17 00:00:00 2001 From: Shreya Malviya Date: Mon, 4 Apr 2022 13:03:50 +0530 Subject: [PATCH 4/4] Agent: Catch subprocess exceptions in utils/*/users.py --- monkey/infection_monkey/utils/linux/users.py | 25 +++++++++++++------ .../infection_monkey/utils/windows/users.py | 9 ++++--- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/monkey/infection_monkey/utils/linux/users.py b/monkey/infection_monkey/utils/linux/users.py index df01f9f18..112670dbb 100644 --- a/monkey/infection_monkey/utils/linux/users.py +++ b/monkey/infection_monkey/utils/linux/users.py @@ -44,9 +44,12 @@ class AutoNewLinuxUser(AutoNewUser): logger.debug( "Trying to add {} with commands {}".format(self.username, str(commands_to_add_user)) ) - _ = subprocess.check_output( - commands_to_add_user, stderr=subprocess.STDOUT, timeout=SHORT_REQUEST_TIMEOUT - ) + try: + _ = subprocess.check_output( + commands_to_add_user, stderr=subprocess.STDOUT, timeout=SHORT_REQUEST_TIMEOUT + ) + except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as err: + logger.error(f"An exception occurred when creating a new linux user: {str(err)}") def __enter__(self): return self # No initialization/logging on needed in Linux @@ -55,7 +58,12 @@ class AutoNewLinuxUser(AutoNewUser): command_as_new_user = shlex.split( "sudo -u {username} {command}".format(username=self.username, command=command) ) - return subprocess.call(command_as_new_user, timeout=SHORT_REQUEST_TIMEOUT) + try: + return subprocess.call(command_as_new_user, timeout=SHORT_REQUEST_TIMEOUT) + except subprocess.TimeoutExpired as err: + logger.error( + f"An exception occurred when running a command as a new linux user: {str(err)}" + ) def __exit__(self, _exc_type, value, traceback): # delete the user. @@ -65,6 +73,9 @@ class AutoNewLinuxUser(AutoNewUser): self.username, str(commands_to_delete_user) ) ) - _ = subprocess.check_output( - commands_to_delete_user, stderr=subprocess.STDOUT, timeout=SHORT_REQUEST_TIMEOUT - ) + try: + _ = subprocess.check_output( + commands_to_delete_user, stderr=subprocess.STDOUT, timeout=SHORT_REQUEST_TIMEOUT + ) + except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as err: + logger.error(f"An exception occurred when deleting the new linux user: {str(err)}") diff --git a/monkey/infection_monkey/utils/windows/users.py b/monkey/infection_monkey/utils/windows/users.py index 9e3af52a1..498e747c5 100644 --- a/monkey/infection_monkey/utils/windows/users.py +++ b/monkey/infection_monkey/utils/windows/users.py @@ -50,9 +50,12 @@ class AutoNewWindowsUser(AutoNewUser): windows_cmds = get_windows_commands_to_add_user(self.username, self.password, True) logger.debug("Trying to add {} with commands {}".format(self.username, str(windows_cmds))) - _ = subprocess.check_output( - windows_cmds, stderr=subprocess.STDOUT, timeout=SHORT_REQUEST_TIMEOUT - ) + try: + _ = subprocess.check_output( + windows_cmds, stderr=subprocess.STDOUT, timeout=SHORT_REQUEST_TIMEOUT + ) + except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as err: + logger.error(f"An exception occurred when creating a new windows user: {str(err)}") def __enter__(self): try: