From 9dc16077540d85f03e1083a561be74f7ee5411c0 Mon Sep 17 00:00:00 2001
From: Shay Nehmad <shay.nehmad@guardicore.com>
Date: Thu, 3 Oct 2019 11:36:47 +0300
Subject: [PATCH 01/15] Added user deactivation as another "security" layer for
 the user deletion in windows

---
 .../utils/windows/auto_new_user.py            | 21 +++++++++++++++----
 .../infection_monkey/utils/windows/users.py   | 13 +++++++++++-
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/monkey/infection_monkey/utils/windows/auto_new_user.py b/monkey/infection_monkey/utils/windows/auto_new_user.py
index d95ac0bf0..fd879ef6b 100644
--- a/monkey/infection_monkey/utils/windows/auto_new_user.py
+++ b/monkey/infection_monkey/utils/windows/auto_new_user.py
@@ -2,7 +2,8 @@ import logging
 import subprocess
 
 from infection_monkey.post_breach.actions.add_user import BackdoorUser
-from infection_monkey.utils.windows.users import get_windows_commands_to_delete_user, get_windows_commands_to_add_user
+from infection_monkey.utils.windows.users import get_windows_commands_to_delete_user, get_windows_commands_to_add_user, \
+    get_windows_commands_to_deactivate_user
 
 logger = logging.getLogger(__name__)
 
@@ -48,7 +49,8 @@ class AutoNewUser(object):
                 self.username,
                 ".",  # Use current domain.
                 self.password,
-                win32con.LOGON32_LOGON_INTERACTIVE,  # Logon type - interactive (normal user).
+                win32con.LOGON32_LOGON_INTERACTIVE,  # Logon type - interactive (normal user). Need this to open ping
+                                                     # using a shell.
                 win32con.LOGON32_PROVIDER_DEFAULT)  # Which logon provider to use - whatever Windows offers.
         except Exception as err:
             raise NewUserError("Can't logon as {}. Error: {}".format(self.username, str(err)))
@@ -61,9 +63,20 @@ class AutoNewUser(object):
         # Logoff
         self.logon_handle.Close()
 
-        # Try to delete user
+        # Try to disable and then delete the user.
+        self.try_deactivate_user()
+        self.try_disable_user()
+
+    def try_disable_user(self):
         try:
-            _ = subprocess.Popen(
+            _ = subprocess.check_output(
                 get_windows_commands_to_delete_user(self.username), stderr=subprocess.STDOUT, shell=True)
         except Exception as err:
             raise NewUserError("Can't delete user {}. Info: {}".format(self.username, err))
+
+    def try_deactivate_user(self):
+        try:
+            _ = subprocess.check_output(
+                get_windows_commands_to_deactivate_user(self.username), stderr=subprocess.STDOUT, shell=True)
+        except Exception as err:
+            raise NewUserError("Can't deactivate user {}. Info: {}".format(self.username, err))
diff --git a/monkey/infection_monkey/utils/windows/users.py b/monkey/infection_monkey/utils/windows/users.py
index 0e6847cff..eac5318d5 100644
--- a/monkey/infection_monkey/utils/windows/users.py
+++ b/monkey/infection_monkey/utils/windows/users.py
@@ -1,3 +1,6 @@
+ACTIVE_NO_NET_USER = '/ACTIVE:NO'
+
+
 def get_windows_commands_to_add_user(username, password, should_be_active=False):
     windows_cmds = [
         'net',
@@ -6,7 +9,7 @@ def get_windows_commands_to_add_user(username, password, should_be_active=False)
         password,
         '/add']
     if not should_be_active:
-        windows_cmds.append('/ACTIVE:NO')
+        windows_cmds.append(ACTIVE_NO_NET_USER)
     return windows_cmds
 
 
@@ -16,3 +19,11 @@ def get_windows_commands_to_delete_user(username):
         'user',
         username,
         '/delete']
+
+
+def get_windows_commands_to_deactivate_user(username):
+    return [
+        'net',
+        'user',
+        username,
+        ACTIVE_NO_NET_USER]

From f5aeb0a38e7ca4f8f673c40c3cb11ec7b5361ef7 Mon Sep 17 00:00:00 2001
From: Shay Nehmad <shay.nehmad@guardicore.com>
Date: Thu, 3 Oct 2019 12:02:14 +0300
Subject: [PATCH 02/15] Moved win32event to its correct location

Cause exception on Linux
---
 .../post_breach/actions/communicate_as_new_user.py             | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
index 296179d41..28797b6d0 100644
--- a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
+++ b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
@@ -5,8 +5,6 @@ import string
 import subprocess
 import time
 
-import win32event
-
 from infection_monkey.utils.windows.auto_new_user import AutoNewUser, NewUserError
 from common.data.post_breach_consts import POST_BREACH_COMMUNICATE_AS_NEW_USER
 from infection_monkey.post_breach.pba import PBA
@@ -68,6 +66,7 @@ class CommunicateAsNewUser(PBA):
         import win32con
         import win32process
         import win32api
+        import win32event
 
         try:
             with AutoNewUser(username, PASSWORD) as new_user:

From 04e18179319d941b7233c9fcbc85b681facf07af Mon Sep 17 00:00:00 2001
From: Shay Nehmad <shay.nehmad@guardicore.com>
Date: Thu, 3 Oct 2019 12:05:15 +0300
Subject: [PATCH 03/15] Added debug log with the deletion commands

---
 .../post_breach/actions/communicate_as_new_user.py           | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
index 28797b6d0..3118fcca9 100644
--- a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
+++ b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
@@ -55,8 +55,9 @@ class CommunicateAsNewUser(PBA):
             exit_status = os.system(final_command)
             self.send_ping_result_telemetry(exit_status, commandline, username)
             # delete the user, async in case it gets stuck.
-            _ = subprocess.Popen(
-                get_linux_commands_to_delete_user(username), stderr=subprocess.STDOUT, shell=True)
+            commands_to_delete_user = get_linux_commands_to_delete_user(username)
+            logger.debug("Trying to delete the user {} with commands {}".format(username, str(commands_to_delete_user)))
+            _ = subprocess.Popen(commands_to_delete_user, stderr=subprocess.STDOUT, shell=True)
             # Leaking the process on purpose - nothing we can do if it's stuck.
         except subprocess.CalledProcessError as e:
             PostBreachTelem(self, (e.output, False)).send()

From 16f8c7841ea9e09a12f247114b6c4faf963c8a9f Mon Sep 17 00:00:00 2001
From: Shay Nehmad <shay.nehmad@guardicore.com>
Date: Thu, 3 Oct 2019 12:25:26 +0300
Subject: [PATCH 04/15] Changed to similar levels of abstracion in user
 creation and deletion + not async

---
 .../post_breach/actions/communicate_as_new_user.py        | 5 +++--
 monkey/infection_monkey/utils/linux/users.py              | 8 ++++----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
index 3118fcca9..dde24d811 100644
--- a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
+++ b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
@@ -54,10 +54,11 @@ class CommunicateAsNewUser(PBA):
             final_command = ' '.join(linux_cmds)
             exit_status = os.system(final_command)
             self.send_ping_result_telemetry(exit_status, commandline, username)
-            # delete the user, async in case it gets stuck.
+            # delete the user.
             commands_to_delete_user = get_linux_commands_to_delete_user(username)
             logger.debug("Trying to delete the user {} with commands {}".format(username, str(commands_to_delete_user)))
-            _ = subprocess.Popen(commands_to_delete_user, stderr=subprocess.STDOUT, shell=True)
+            delete_user_output = subprocess.check_output(" ".join(commands_to_delete_user), stderr=subprocess.STDOUT, shell=True)
+            logger.debug("Deletion output: {}".format(delete_user_output))
             # Leaking the process on purpose - nothing we can do if it's stuck.
         except subprocess.CalledProcessError as e:
             PostBreachTelem(self, (e.output, False)).send()
diff --git a/monkey/infection_monkey/utils/linux/users.py b/monkey/infection_monkey/utils/linux/users.py
index 1acc87d72..d58e04b7d 100644
--- a/monkey/infection_monkey/utils/linux/users.py
+++ b/monkey/infection_monkey/utils/linux/users.py
@@ -3,12 +3,12 @@ import datetime
 
 def get_linux_commands_to_add_user(username):
     return [
-        'useradd',
+        'adduser',  # https://linux.die.net/man/8/adduser
         '-M',  # Do not create homedir
-        '--expiredate',
+        '--expiredate',  # The date on which the user account will be disabled.
         datetime.datetime.today().strftime('%Y-%m-%d'),
-        '--inactive',
-        '0',
+        '--inactive',  # The number of days after a password expires until the account is permanently disabled.
+        '0',  # A value of 0 disables the account as soon as the password has expired
         '-c',  # Comment
         'MONKEY_USER',  # Comment
         username]

From 3f5272b83b0e8589804ebf7c8596726525f1cc10 Mon Sep 17 00:00:00 2001
From: Shay Nehmad <shay.nehmad@guardicore.com>
Date: Thu, 3 Oct 2019 14:21:19 +0300
Subject: [PATCH 05/15] Changed adduser to useradd for compatibility

---
 monkey/infection_monkey/utils/linux/users.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/monkey/infection_monkey/utils/linux/users.py b/monkey/infection_monkey/utils/linux/users.py
index d58e04b7d..2bca94c50 100644
--- a/monkey/infection_monkey/utils/linux/users.py
+++ b/monkey/infection_monkey/utils/linux/users.py
@@ -3,7 +3,7 @@ import datetime
 
 def get_linux_commands_to_add_user(username):
     return [
-        'adduser',  # https://linux.die.net/man/8/adduser
+        'useradd',  # https://linux.die.net/man/8/useradd
         '-M',  # Do not create homedir
         '--expiredate',  # The date on which the user account will be disabled.
         datetime.datetime.today().strftime('%Y-%m-%d'),

From 321c93063e5282858608b70feff3cea6836dd8d3 Mon Sep 17 00:00:00 2001
From: Shay Nehmad <shay.nehmad@guardicore.com>
Date: Thu, 3 Oct 2019 14:58:55 +0300
Subject: [PATCH 06/15] Refactored new user in linux to AutoNewLinuxUser and
 created AutoNewUser ABC

---
 .../actions/communicate_as_new_user.py        | 23 ++---
 .../utils/windows/auto_new_user.py            | 85 +++++++++++++++----
 2 files changed, 78 insertions(+), 30 deletions(-)

diff --git a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
index dde24d811..2ab4476d8 100644
--- a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
+++ b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
@@ -5,7 +5,7 @@ import string
 import subprocess
 import time
 
-from infection_monkey.utils.windows.auto_new_user import AutoNewUser, NewUserError
+from infection_monkey.utils.windows.auto_new_user import NewUserError, create_auto_new_user
 from common.data.post_breach_consts import POST_BREACH_COMMUNICATE_AS_NEW_USER
 from infection_monkey.post_breach.pba import PBA
 from infection_monkey.telemetry.post_breach_telem import PostBreachTelem
@@ -47,19 +47,12 @@ class CommunicateAsNewUser(PBA):
 
     def communicate_as_new_user_linux(self, username):
         try:
-            # add user + ping
-            linux_cmds = get_linux_commands_to_add_user(username)
-            commandline = "ping -c 1 {}".format(PING_TEST_DOMAIN)
-            linux_cmds.extend([";", "sudo", "-u", username, commandline])
-            final_command = ' '.join(linux_cmds)
-            exit_status = os.system(final_command)
-            self.send_ping_result_telemetry(exit_status, commandline, username)
-            # delete the user.
-            commands_to_delete_user = get_linux_commands_to_delete_user(username)
-            logger.debug("Trying to delete the user {} with commands {}".format(username, str(commands_to_delete_user)))
-            delete_user_output = subprocess.check_output(" ".join(commands_to_delete_user), stderr=subprocess.STDOUT, shell=True)
-            logger.debug("Deletion output: {}".format(delete_user_output))
-            # Leaking the process on purpose - nothing we can do if it's stuck.
+            with create_auto_new_user(username, PASSWORD) as new_user:
+                commandline = "sudo -u {username} ping -c 1 {domain}".format(
+                    username=new_user.username,
+                    domain=PING_TEST_DOMAIN)
+                exit_status = os.system(commandline)
+                self.send_ping_result_telemetry(exit_status, commandline, new_user.username)
         except subprocess.CalledProcessError as e:
             PostBreachTelem(self, (e.output, False)).send()
 
@@ -71,7 +64,7 @@ class CommunicateAsNewUser(PBA):
         import win32event
 
         try:
-            with AutoNewUser(username, PASSWORD) as new_user:
+            with create_auto_new_user(username, PASSWORD) as new_user:
                 # Using os.path is OK, as this is on windows for sure
                 ping_app_path = os.path.join(os.environ["WINDIR"], "system32", "PING.exe")
                 if not os.path.exists(ping_app_path):
diff --git a/monkey/infection_monkey/utils/windows/auto_new_user.py b/monkey/infection_monkey/utils/windows/auto_new_user.py
index fd879ef6b..c22b40c54 100644
--- a/monkey/infection_monkey/utils/windows/auto_new_user.py
+++ b/monkey/infection_monkey/utils/windows/auto_new_user.py
@@ -1,7 +1,9 @@
 import logging
 import subprocess
+import abc
 
-from infection_monkey.post_breach.actions.add_user import BackdoorUser
+from infection_monkey.utils.environment import is_windows_os
+from infection_monkey.utils.linux.users import get_linux_commands_to_add_user, get_linux_commands_to_delete_user
 from infection_monkey.utils.windows.users import get_windows_commands_to_delete_user, get_windows_commands_to_add_user, \
     get_windows_commands_to_deactivate_user
 
@@ -12,26 +14,79 @@ class NewUserError(Exception):
     pass
 
 
-class AutoNewUser(object):
+class AutoNewUser:
     """
-    RAII object to use for creating and using a new user in Windows. Use with `with`.
+    RAII object to use for creating and using a new user. Use with `with`.
     User will be created when the instance is instantiated.
-    User will log on at the start of the `with` scope.
-    User will log off and get deleted at the end of said `with` scope.
+    User will be available for use (log on for Windows, for example) at the start of the `with` scope.
+    User will be removed (deactivated and deleted for Windows, for example) at the end of said `with` scope.
 
     Example:
-             # Created                           # Logged on
-        with AutoNewUser("user", "pass") as new_user:
+             # Created                                                 # Logged on
+        with AutoNewUser("user", "pass", is_on_windows()) as new_user:
             ...
             ...
         # Logged off and deleted
         ...
+        """
+    __metaclass__ = abc.ABCMeta
+
+    def __init__(self, username, password):
+        raise NotImplementedError()
+
+    @abc.abstractmethod
+    def __enter__(self):
+        raise NotImplementedError()
+
+    @abc.abstractmethod
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        raise NotImplementedError()
+
+
+def create_auto_new_user(username, password, is_windows=is_windows_os()):
+    if is_windows:
+        return AutoNewWindowsUser(username, password)
+    else:
+        return AutoNewLinuxUser(username, password)
+
+
+class AutoNewLinuxUser(AutoNewUser):
+    """
+    See AutoNewUser's documentation for details.
     """
     def __init__(self, username, password):
         """
         Creates a user with the username + password.
         :raises: subprocess.CalledProcessError if failed to add the user.
         """
+        super(AutoNewLinuxUser, self).__init__(username, password)
+        self.username = username
+        self.password = password
+
+        commands_to_add_user = get_linux_commands_to_add_user(username)
+        logger.debug("Trying to add {} with commands {}".format(self.username, str(commands_to_add_user)))
+        _ = subprocess.check_output(' '.join(commands_to_add_user), stderr=subprocess.STDOUT, shell=True)
+
+    def __enter__(self):
+        pass  # No initialization/logging on needed in Linux
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        # delete the user.
+        commands_to_delete_user = get_linux_commands_to_delete_user(self.username)
+        logger.debug("Trying to delete {} with commands {}".format(self.username, str(commands_to_delete_user)))
+        _ = subprocess.check_output(" ".join(commands_to_delete_user), stderr=subprocess.STDOUT, shell=True)
+
+
+class AutoNewWindowsUser(AutoNewUser):
+    """
+    See AutoNewUser's documentation for details.
+    """
+    def __init__(self, username, password):
+        """
+        Creates a user with the username + password.
+        :raises: subprocess.CalledProcessError if failed to add the user.
+        """
+        super(AutoNewWindowsUser, self).__init__(username, password)
         self.username = username
         self.password = password
 
@@ -65,14 +120,7 @@ class AutoNewUser(object):
 
         # Try to disable and then delete the user.
         self.try_deactivate_user()
-        self.try_disable_user()
-
-    def try_disable_user(self):
-        try:
-            _ = subprocess.check_output(
-                get_windows_commands_to_delete_user(self.username), stderr=subprocess.STDOUT, shell=True)
-        except Exception as err:
-            raise NewUserError("Can't delete user {}. Info: {}".format(self.username, err))
+        self.try_delete_user()
 
     def try_deactivate_user(self):
         try:
@@ -80,3 +128,10 @@ class AutoNewUser(object):
                 get_windows_commands_to_deactivate_user(self.username), stderr=subprocess.STDOUT, shell=True)
         except Exception as err:
             raise NewUserError("Can't deactivate user {}. Info: {}".format(self.username, err))
+
+    def try_delete_user(self):
+        try:
+            _ = subprocess.check_output(
+                get_windows_commands_to_delete_user(self.username), stderr=subprocess.STDOUT, shell=True)
+        except Exception as err:
+            raise NewUserError("Can't delete user {}. Info: {}".format(self.username, err))

From 6b315d96c0bb0d6ed1ef1c0efcc45662e57c038e Mon Sep 17 00:00:00 2001
From: Shay Nehmad <shay.nehmad@guardicore.com>
Date: Thu, 3 Oct 2019 15:06:17 +0300
Subject: [PATCH 07/15] Fixed NotImplemented error in __init__ method

---
 .../post_breach/actions/communicate_as_new_user.py         | 4 ++--
 monkey/infection_monkey/utils/windows/auto_new_user.py     | 7 ++-----
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
index 2ab4476d8..034383140 100644
--- a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
+++ b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
@@ -47,7 +47,7 @@ class CommunicateAsNewUser(PBA):
 
     def communicate_as_new_user_linux(self, username):
         try:
-            with create_auto_new_user(username, PASSWORD) as new_user:
+            with create_auto_new_user(username, PASSWORD, False) as new_user:
                 commandline = "sudo -u {username} ping -c 1 {domain}".format(
                     username=new_user.username,
                     domain=PING_TEST_DOMAIN)
@@ -64,7 +64,7 @@ class CommunicateAsNewUser(PBA):
         import win32event
 
         try:
-            with create_auto_new_user(username, PASSWORD) as new_user:
+            with create_auto_new_user(username, PASSWORD, True) as new_user:
                 # Using os.path is OK, as this is on windows for sure
                 ping_app_path = os.path.join(os.environ["WINDIR"], "system32", "PING.exe")
                 if not os.path.exists(ping_app_path):
diff --git a/monkey/infection_monkey/utils/windows/auto_new_user.py b/monkey/infection_monkey/utils/windows/auto_new_user.py
index c22b40c54..89fc464fc 100644
--- a/monkey/infection_monkey/utils/windows/auto_new_user.py
+++ b/monkey/infection_monkey/utils/windows/auto_new_user.py
@@ -32,7 +32,8 @@ class AutoNewUser:
     __metaclass__ = abc.ABCMeta
 
     def __init__(self, username, password):
-        raise NotImplementedError()
+        self.username = username
+        self.password = password
 
     @abc.abstractmethod
     def __enter__(self):
@@ -60,8 +61,6 @@ class AutoNewLinuxUser(AutoNewUser):
         :raises: subprocess.CalledProcessError if failed to add the user.
         """
         super(AutoNewLinuxUser, self).__init__(username, password)
-        self.username = username
-        self.password = password
 
         commands_to_add_user = get_linux_commands_to_add_user(username)
         logger.debug("Trying to add {} with commands {}".format(self.username, str(commands_to_add_user)))
@@ -87,8 +86,6 @@ class AutoNewWindowsUser(AutoNewUser):
         :raises: subprocess.CalledProcessError if failed to add the user.
         """
         super(AutoNewWindowsUser, self).__init__(username, password)
-        self.username = username
-        self.password = password
 
         windows_cmds = get_windows_commands_to_add_user(self.username, self.password, True)
         _ = subprocess.check_output(windows_cmds, stderr=subprocess.STDOUT, shell=True)

From 129fd7d2de5be365717c3e9aa74753d30e18bac1 Mon Sep 17 00:00:00 2001
From: Shay Nehmad <shay.nehmad@guardicore.com>
Date: Thu, 3 Oct 2019 15:08:55 +0300
Subject: [PATCH 08/15] Not using new_user.username as it causes exception
 NoneType

---
 .../post_breach/actions/communicate_as_new_user.py          | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
index 034383140..213682fc7 100644
--- a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
+++ b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
@@ -47,12 +47,12 @@ class CommunicateAsNewUser(PBA):
 
     def communicate_as_new_user_linux(self, username):
         try:
-            with create_auto_new_user(username, PASSWORD, False) as new_user:
+            with create_auto_new_user(username, PASSWORD, False) as _:
                 commandline = "sudo -u {username} ping -c 1 {domain}".format(
-                    username=new_user.username,
+                    username=username,
                     domain=PING_TEST_DOMAIN)
                 exit_status = os.system(commandline)
-                self.send_ping_result_telemetry(exit_status, commandline, new_user.username)
+                self.send_ping_result_telemetry(exit_status, commandline, username)
         except subprocess.CalledProcessError as e:
             PostBreachTelem(self, (e.output, False)).send()
 

From 1ffdc7528fbef70e2cf30af1906e2c0d9a0b6fd8 Mon Sep 17 00:00:00 2001
From: Shay Nehmad <shay.nehmad@guardicore.com>
Date: Thu, 3 Oct 2019 15:14:57 +0300
Subject: [PATCH 09/15] Added some doc, and moved separated classes to files.

---
 .../actions/communicate_as_new_user.py        |   8 +-
 .../infection_monkey/utils/auto_new_user.py   |  62 ++++++++
 monkey/infection_monkey/utils/linux/users.py  |  28 ++++
 .../utils/windows/auto_new_user.py            | 134 ------------------
 .../infection_monkey/utils/windows/users.py   |  62 ++++++++
 5 files changed, 155 insertions(+), 139 deletions(-)
 create mode 100644 monkey/infection_monkey/utils/auto_new_user.py
 delete mode 100644 monkey/infection_monkey/utils/windows/auto_new_user.py

diff --git a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
index 213682fc7..057f23ccb 100644
--- a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
+++ b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
@@ -3,14 +3,12 @@ import os
 import random
 import string
 import subprocess
-import time
 
-from infection_monkey.utils.windows.auto_new_user import NewUserError, create_auto_new_user
+from infection_monkey.utils.auto_new_user import NewUserError, create_auto_new_user
 from common.data.post_breach_consts import POST_BREACH_COMMUNICATE_AS_NEW_USER
 from infection_monkey.post_breach.pba import PBA
 from infection_monkey.telemetry.post_breach_telem import PostBreachTelem
 from infection_monkey.utils.environment import is_windows_os
-from infection_monkey.utils.linux.users import get_linux_commands_to_delete_user, get_linux_commands_to_add_user
 
 PING_TEST_DOMAIN = "google.com"
 
@@ -47,7 +45,7 @@ class CommunicateAsNewUser(PBA):
 
     def communicate_as_new_user_linux(self, username):
         try:
-            with create_auto_new_user(username, PASSWORD, False) as _:
+            with create_auto_new_user(username, PASSWORD, is_windows=False) as _:
                 commandline = "sudo -u {username} ping -c 1 {domain}".format(
                     username=username,
                     domain=PING_TEST_DOMAIN)
@@ -64,7 +62,7 @@ class CommunicateAsNewUser(PBA):
         import win32event
 
         try:
-            with create_auto_new_user(username, PASSWORD, True) as new_user:
+            with create_auto_new_user(username, PASSWORD, is_windows=True) as new_user:
                 # Using os.path is OK, as this is on windows for sure
                 ping_app_path = os.path.join(os.environ["WINDIR"], "system32", "PING.exe")
                 if not os.path.exists(ping_app_path):
diff --git a/monkey/infection_monkey/utils/auto_new_user.py b/monkey/infection_monkey/utils/auto_new_user.py
new file mode 100644
index 000000000..492c8cf9d
--- /dev/null
+++ b/monkey/infection_monkey/utils/auto_new_user.py
@@ -0,0 +1,62 @@
+import logging
+import abc
+
+from infection_monkey.utils.environment import is_windows_os
+from infection_monkey.utils.linux.users import AutoNewLinuxUser
+from infection_monkey.utils.windows.users import AutoNewWindowsUser
+
+logger = logging.getLogger(__name__)
+
+
+class NewUserError(Exception):
+    pass
+
+
+class AutoNewUser:
+    """
+    RAII object to use for creating and using a new user. Use with `with`.
+    User will be created when the instance is instantiated.
+    User will be available for use (log on for Windows, for example) at the start of the `with` scope.
+    User will be removed (deactivated and deleted for Windows, for example) at the end of said `with` scope.
+
+    Example:
+             # Created                                                 # Logged on
+        with AutoNewUser("user", "pass", is_on_windows()) as new_user:
+            ...
+            ...
+        # Logged off and deleted
+        ...
+        """
+    __metaclass__ = abc.ABCMeta
+
+    def __init__(self, username, password):
+        self.username = username
+        self.password = password
+
+    @abc.abstractmethod
+    def __enter__(self):
+        raise NotImplementedError()
+
+    @abc.abstractmethod
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        raise NotImplementedError()
+
+
+def create_auto_new_user(username, password, is_windows=is_windows_os()):
+    """
+    Factory method for creating an AutoNewUser. See AutoNewUser's documentation for more information.
+    Example usage:
+        with create_auto_new_user(username, PASSWORD) as new_user:
+            ...
+    :param username: The username of the new user.
+    :param password: The password of the new user.
+    :param is_windows: If True, a new Windows user is created. Otherwise, a Linux user is created. Leave blank for
+    automatic detection.
+    :return: The new AutoNewUser object - use with a `with` scope.
+    """
+    if is_windows:
+        return AutoNewWindowsUser(username, password)
+    else:
+        return AutoNewLinuxUser(username, password)
+
+
diff --git a/monkey/infection_monkey/utils/linux/users.py b/monkey/infection_monkey/utils/linux/users.py
index 2bca94c50..9efdca196 100644
--- a/monkey/infection_monkey/utils/linux/users.py
+++ b/monkey/infection_monkey/utils/linux/users.py
@@ -1,4 +1,7 @@
 import datetime
+import subprocess
+
+from infection_monkey.utils.auto_new_user import AutoNewUser, logger
 
 
 def get_linux_commands_to_add_user(username):
@@ -19,3 +22,28 @@ def get_linux_commands_to_delete_user(username):
         'deluser',
         username
     ]
+
+
+class AutoNewLinuxUser(AutoNewUser):
+    """
+    See AutoNewUser's documentation for details.
+    """
+    def __init__(self, username, password):
+        """
+        Creates a user with the username + password.
+        :raises: subprocess.CalledProcessError if failed to add the user.
+        """
+        super(AutoNewLinuxUser, self).__init__(username, password)
+
+        commands_to_add_user = get_linux_commands_to_add_user(username)
+        logger.debug("Trying to add {} with commands {}".format(self.username, str(commands_to_add_user)))
+        _ = subprocess.check_output(' '.join(commands_to_add_user), stderr=subprocess.STDOUT, shell=True)
+
+    def __enter__(self):
+        pass  # No initialization/logging on needed in Linux
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        # delete the user.
+        commands_to_delete_user = get_linux_commands_to_delete_user(self.username)
+        logger.debug("Trying to delete {} with commands {}".format(self.username, str(commands_to_delete_user)))
+        _ = subprocess.check_output(" ".join(commands_to_delete_user), stderr=subprocess.STDOUT, shell=True)
\ No newline at end of file
diff --git a/monkey/infection_monkey/utils/windows/auto_new_user.py b/monkey/infection_monkey/utils/windows/auto_new_user.py
deleted file mode 100644
index 89fc464fc..000000000
--- a/monkey/infection_monkey/utils/windows/auto_new_user.py
+++ /dev/null
@@ -1,134 +0,0 @@
-import logging
-import subprocess
-import abc
-
-from infection_monkey.utils.environment import is_windows_os
-from infection_monkey.utils.linux.users import get_linux_commands_to_add_user, get_linux_commands_to_delete_user
-from infection_monkey.utils.windows.users import get_windows_commands_to_delete_user, get_windows_commands_to_add_user, \
-    get_windows_commands_to_deactivate_user
-
-logger = logging.getLogger(__name__)
-
-
-class NewUserError(Exception):
-    pass
-
-
-class AutoNewUser:
-    """
-    RAII object to use for creating and using a new user. Use with `with`.
-    User will be created when the instance is instantiated.
-    User will be available for use (log on for Windows, for example) at the start of the `with` scope.
-    User will be removed (deactivated and deleted for Windows, for example) at the end of said `with` scope.
-
-    Example:
-             # Created                                                 # Logged on
-        with AutoNewUser("user", "pass", is_on_windows()) as new_user:
-            ...
-            ...
-        # Logged off and deleted
-        ...
-        """
-    __metaclass__ = abc.ABCMeta
-
-    def __init__(self, username, password):
-        self.username = username
-        self.password = password
-
-    @abc.abstractmethod
-    def __enter__(self):
-        raise NotImplementedError()
-
-    @abc.abstractmethod
-    def __exit__(self, exc_type, exc_val, exc_tb):
-        raise NotImplementedError()
-
-
-def create_auto_new_user(username, password, is_windows=is_windows_os()):
-    if is_windows:
-        return AutoNewWindowsUser(username, password)
-    else:
-        return AutoNewLinuxUser(username, password)
-
-
-class AutoNewLinuxUser(AutoNewUser):
-    """
-    See AutoNewUser's documentation for details.
-    """
-    def __init__(self, username, password):
-        """
-        Creates a user with the username + password.
-        :raises: subprocess.CalledProcessError if failed to add the user.
-        """
-        super(AutoNewLinuxUser, self).__init__(username, password)
-
-        commands_to_add_user = get_linux_commands_to_add_user(username)
-        logger.debug("Trying to add {} with commands {}".format(self.username, str(commands_to_add_user)))
-        _ = subprocess.check_output(' '.join(commands_to_add_user), stderr=subprocess.STDOUT, shell=True)
-
-    def __enter__(self):
-        pass  # No initialization/logging on needed in Linux
-
-    def __exit__(self, exc_type, exc_val, exc_tb):
-        # delete the user.
-        commands_to_delete_user = get_linux_commands_to_delete_user(self.username)
-        logger.debug("Trying to delete {} with commands {}".format(self.username, str(commands_to_delete_user)))
-        _ = subprocess.check_output(" ".join(commands_to_delete_user), stderr=subprocess.STDOUT, shell=True)
-
-
-class AutoNewWindowsUser(AutoNewUser):
-    """
-    See AutoNewUser's documentation for details.
-    """
-    def __init__(self, username, password):
-        """
-        Creates a user with the username + password.
-        :raises: subprocess.CalledProcessError if failed to add the user.
-        """
-        super(AutoNewWindowsUser, self).__init__(username, password)
-
-        windows_cmds = get_windows_commands_to_add_user(self.username, self.password, True)
-        _ = subprocess.check_output(windows_cmds, stderr=subprocess.STDOUT, shell=True)
-
-    def __enter__(self):
-        # Importing these only on windows, as they won't exist on linux.
-        import win32security
-        import win32con
-
-        try:
-            # Logon as new user: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-logonusera
-            self.logon_handle = win32security.LogonUser(
-                self.username,
-                ".",  # Use current domain.
-                self.password,
-                win32con.LOGON32_LOGON_INTERACTIVE,  # Logon type - interactive (normal user). Need this to open ping
-                                                     # using a shell.
-                win32con.LOGON32_PROVIDER_DEFAULT)  # Which logon provider to use - whatever Windows offers.
-        except Exception as err:
-            raise NewUserError("Can't logon as {}. Error: {}".format(self.username, str(err)))
-        return self
-
-    def get_logon_handle(self):
-        return self.logon_handle
-
-    def __exit__(self, exc_type, exc_val, exc_tb):
-        # Logoff
-        self.logon_handle.Close()
-
-        # Try to disable and then delete the user.
-        self.try_deactivate_user()
-        self.try_delete_user()
-
-    def try_deactivate_user(self):
-        try:
-            _ = subprocess.check_output(
-                get_windows_commands_to_deactivate_user(self.username), stderr=subprocess.STDOUT, shell=True)
-        except Exception as err:
-            raise NewUserError("Can't deactivate user {}. Info: {}".format(self.username, err))
-
-    def try_delete_user(self):
-        try:
-            _ = subprocess.check_output(
-                get_windows_commands_to_delete_user(self.username), stderr=subprocess.STDOUT, shell=True)
-        except Exception as err:
-            raise NewUserError("Can't delete user {}. Info: {}".format(self.username, err))
diff --git a/monkey/infection_monkey/utils/windows/users.py b/monkey/infection_monkey/utils/windows/users.py
index eac5318d5..850774977 100644
--- a/monkey/infection_monkey/utils/windows/users.py
+++ b/monkey/infection_monkey/utils/windows/users.py
@@ -1,3 +1,7 @@
+import subprocess
+
+from infection_monkey.utils.auto_new_user import AutoNewUser, NewUserError
+
 ACTIVE_NO_NET_USER = '/ACTIVE:NO'
 
 
@@ -27,3 +31,61 @@ def get_windows_commands_to_deactivate_user(username):
         'user',
         username,
         ACTIVE_NO_NET_USER]
+
+
+class AutoNewWindowsUser(AutoNewUser):
+    """
+    See AutoNewUser's documentation for details.
+    """
+    def __init__(self, username, password):
+        """
+        Creates a user with the username + password.
+        :raises: subprocess.CalledProcessError if failed to add the user.
+        """
+        super(AutoNewWindowsUser, self).__init__(username, password)
+
+        windows_cmds = get_windows_commands_to_add_user(self.username, self.password, True)
+        _ = subprocess.check_output(windows_cmds, stderr=subprocess.STDOUT, shell=True)
+
+    def __enter__(self):
+        # Importing these only on windows, as they won't exist on linux.
+        import win32security
+        import win32con
+
+        try:
+            # Logon as new user: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-logonusera
+            self.logon_handle = win32security.LogonUser(
+                self.username,
+                ".",  # Use current domain.
+                self.password,
+                win32con.LOGON32_LOGON_INTERACTIVE,  # Logon type - interactive (normal user). Need this to open ping
+                                                     # using a shell.
+                win32con.LOGON32_PROVIDER_DEFAULT)  # Which logon provider to use - whatever Windows offers.
+        except Exception as err:
+            raise NewUserError("Can't logon as {}. Error: {}".format(self.username, str(err)))
+        return self
+
+    def get_logon_handle(self):
+        return self.logon_handle
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        # Logoff
+        self.logon_handle.Close()
+
+        # Try to disable and then delete the user.
+        self.try_deactivate_user()
+        self.try_delete_user()
+
+    def try_deactivate_user(self):
+        try:
+            _ = subprocess.check_output(
+                get_windows_commands_to_deactivate_user(self.username), stderr=subprocess.STDOUT, shell=True)
+        except Exception as err:
+            raise NewUserError("Can't deactivate user {}. Info: {}".format(self.username, err))
+
+    def try_delete_user(self):
+        try:
+            _ = subprocess.check_output(
+                get_windows_commands_to_delete_user(self.username), stderr=subprocess.STDOUT, shell=True)
+        except Exception as err:
+            raise NewUserError("Can't delete user {}. Info: {}".format(self.username, err))
\ No newline at end of file

From 44a1f70da9df3f90d80213d097c1c7d7fab2dd80 Mon Sep 17 00:00:00 2001
From: Shay Nehmad <shay.nehmad@guardicore.com>
Date: Thu, 3 Oct 2019 15:20:50 +0300
Subject: [PATCH 10/15] Fixed circular imports

---
 .../actions/communicate_as_new_user.py        |  3 +-
 .../infection_monkey/utils/auto_new_user.py   | 28 -------------------
 .../utils/auto_new_user_factory.py            | 21 ++++++++++++++
 .../infection_monkey/utils/new_user_error.py  |  2 ++
 .../infection_monkey/utils/windows/users.py   |  3 +-
 5 files changed, 27 insertions(+), 30 deletions(-)
 create mode 100644 monkey/infection_monkey/utils/auto_new_user_factory.py
 create mode 100644 monkey/infection_monkey/utils/new_user_error.py

diff --git a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
index 057f23ccb..7658f5a94 100644
--- a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
+++ b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
@@ -4,7 +4,8 @@ import random
 import string
 import subprocess
 
-from infection_monkey.utils.auto_new_user import NewUserError, create_auto_new_user
+from infection_monkey.utils.new_user_error import NewUserError
+from infection_monkey.utils.auto_new_user_factory import create_auto_new_user
 from common.data.post_breach_consts import POST_BREACH_COMMUNICATE_AS_NEW_USER
 from infection_monkey.post_breach.pba import PBA
 from infection_monkey.telemetry.post_breach_telem import PostBreachTelem
diff --git a/monkey/infection_monkey/utils/auto_new_user.py b/monkey/infection_monkey/utils/auto_new_user.py
index 492c8cf9d..e8e42b309 100644
--- a/monkey/infection_monkey/utils/auto_new_user.py
+++ b/monkey/infection_monkey/utils/auto_new_user.py
@@ -1,17 +1,9 @@
 import logging
 import abc
 
-from infection_monkey.utils.environment import is_windows_os
-from infection_monkey.utils.linux.users import AutoNewLinuxUser
-from infection_monkey.utils.windows.users import AutoNewWindowsUser
-
 logger = logging.getLogger(__name__)
 
 
-class NewUserError(Exception):
-    pass
-
-
 class AutoNewUser:
     """
     RAII object to use for creating and using a new user. Use with `with`.
@@ -40,23 +32,3 @@ class AutoNewUser:
     @abc.abstractmethod
     def __exit__(self, exc_type, exc_val, exc_tb):
         raise NotImplementedError()
-
-
-def create_auto_new_user(username, password, is_windows=is_windows_os()):
-    """
-    Factory method for creating an AutoNewUser. See AutoNewUser's documentation for more information.
-    Example usage:
-        with create_auto_new_user(username, PASSWORD) as new_user:
-            ...
-    :param username: The username of the new user.
-    :param password: The password of the new user.
-    :param is_windows: If True, a new Windows user is created. Otherwise, a Linux user is created. Leave blank for
-    automatic detection.
-    :return: The new AutoNewUser object - use with a `with` scope.
-    """
-    if is_windows:
-        return AutoNewWindowsUser(username, password)
-    else:
-        return AutoNewLinuxUser(username, password)
-
-
diff --git a/monkey/infection_monkey/utils/auto_new_user_factory.py b/monkey/infection_monkey/utils/auto_new_user_factory.py
new file mode 100644
index 000000000..898226d46
--- /dev/null
+++ b/monkey/infection_monkey/utils/auto_new_user_factory.py
@@ -0,0 +1,21 @@
+from infection_monkey.utils.environment import is_windows_os
+from infection_monkey.utils.linux.users import AutoNewLinuxUser
+from infection_monkey.utils.windows.users import AutoNewWindowsUser
+
+
+def create_auto_new_user(username, password, is_windows=is_windows_os()):
+    """
+    Factory method for creating an AutoNewUser. See AutoNewUser's documentation for more information.
+    Example usage:
+        with create_auto_new_user(username, PASSWORD) as new_user:
+            ...
+    :param username: The username of the new user.
+    :param password: The password of the new user.
+    :param is_windows: If True, a new Windows user is created. Otherwise, a Linux user is created. Leave blank for
+    automatic detection.
+    :return: The new AutoNewUser object - use with a `with` scope.
+    """
+    if is_windows:
+        return AutoNewWindowsUser(username, password)
+    else:
+        return AutoNewLinuxUser(username, password)
diff --git a/monkey/infection_monkey/utils/new_user_error.py b/monkey/infection_monkey/utils/new_user_error.py
new file mode 100644
index 000000000..8fe44d7bc
--- /dev/null
+++ b/monkey/infection_monkey/utils/new_user_error.py
@@ -0,0 +1,2 @@
+class NewUserError(Exception):
+    pass
diff --git a/monkey/infection_monkey/utils/windows/users.py b/monkey/infection_monkey/utils/windows/users.py
index 850774977..6193da60e 100644
--- a/monkey/infection_monkey/utils/windows/users.py
+++ b/monkey/infection_monkey/utils/windows/users.py
@@ -1,6 +1,7 @@
 import subprocess
 
-from infection_monkey.utils.auto_new_user import AutoNewUser, NewUserError
+from infection_monkey.utils.auto_new_user import AutoNewUser
+from infection_monkey.utils.new_user_error import NewUserError
 
 ACTIVE_NO_NET_USER = '/ACTIVE:NO'
 

From c4d53d14c66085d39d7201c5b8de676002821d05 Mon Sep 17 00:00:00 2001
From: Shay Nehmad <shay.nehmad@guardicore.com>
Date: Thu, 3 Oct 2019 15:29:23 +0300
Subject: [PATCH 11/15] Fixed linuxnewuser logger and added some logs to
 windowsnewuser for symmetry.

---
 monkey/infection_monkey/utils/linux/users.py   |  5 ++++-
 monkey/infection_monkey/utils/windows/users.py | 13 +++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/monkey/infection_monkey/utils/linux/users.py b/monkey/infection_monkey/utils/linux/users.py
index 9efdca196..25e3bdd77 100644
--- a/monkey/infection_monkey/utils/linux/users.py
+++ b/monkey/infection_monkey/utils/linux/users.py
@@ -1,7 +1,10 @@
 import datetime
+import logging
 import subprocess
 
-from infection_monkey.utils.auto_new_user import AutoNewUser, logger
+from infection_monkey.utils.auto_new_user import AutoNewUser
+
+logger = logging.getLogger(__name__)
 
 
 def get_linux_commands_to_add_user(username):
diff --git a/monkey/infection_monkey/utils/windows/users.py b/monkey/infection_monkey/utils/windows/users.py
index 6193da60e..3bbf9522e 100644
--- a/monkey/infection_monkey/utils/windows/users.py
+++ b/monkey/infection_monkey/utils/windows/users.py
@@ -1,3 +1,4 @@
+import logging
 import subprocess
 
 from infection_monkey.utils.auto_new_user import AutoNewUser
@@ -5,6 +6,7 @@ from infection_monkey.utils.new_user_error import NewUserError
 
 ACTIVE_NO_NET_USER = '/ACTIVE:NO'
 
+logger = logging.getLogger(__name__)
 
 def get_windows_commands_to_add_user(username, password, should_be_active=False):
     windows_cmds = [
@@ -46,6 +48,7 @@ class AutoNewWindowsUser(AutoNewUser):
         super(AutoNewWindowsUser, self).__init__(username, password)
 
         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, shell=True)
 
     def __enter__(self):
@@ -79,14 +82,20 @@ class AutoNewWindowsUser(AutoNewUser):
 
     def try_deactivate_user(self):
         try:
+            commands_to_deactivate_user = get_windows_commands_to_deactivate_user(self.username)
+            logger.debug(
+                "Trying to deactivate {} with commands {}".format(self.username, str(commands_to_deactivate_user)))
             _ = subprocess.check_output(
-                get_windows_commands_to_deactivate_user(self.username), stderr=subprocess.STDOUT, shell=True)
+                commands_to_deactivate_user, stderr=subprocess.STDOUT, shell=True)
         except Exception as err:
             raise NewUserError("Can't deactivate user {}. Info: {}".format(self.username, err))
 
     def try_delete_user(self):
         try:
+            commands_to_delete_user = get_windows_commands_to_delete_user(self.username)
+            logger.debug(
+                "Trying to deactivate {} with commands {}".format(self.username, str(commands_to_delete_user)))
             _ = subprocess.check_output(
-                get_windows_commands_to_delete_user(self.username), stderr=subprocess.STDOUT, shell=True)
+                commands_to_delete_user, stderr=subprocess.STDOUT, shell=True)
         except Exception as err:
             raise NewUserError("Can't delete user {}. Info: {}".format(self.username, err))
\ No newline at end of file

From c7d3fd9fdc95dcd7075168d43f57ce960216ba52 Mon Sep 17 00:00:00 2001
From: Shay Nehmad <shay.nehmad@guardicore.com>
Date: Thu, 3 Oct 2019 16:53:11 +0300
Subject: [PATCH 12/15] Moved runas to autonewuser class and deleted
 duplication from communicate_as_new_user

---
 .../actions/communicate_as_new_user.py        | 100 +++---------------
 .../infection_monkey/utils/auto_new_user.py   |   4 +
 monkey/infection_monkey/utils/linux/users.py  |   6 ++
 .../infection_monkey/utils/windows/users.py   |  55 +++++++++-
 4 files changed, 81 insertions(+), 84 deletions(-)

diff --git a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
index 7658f5a94..b98361ec9 100644
--- a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
+++ b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
@@ -13,12 +13,10 @@ from infection_monkey.utils.environment import is_windows_os
 
 PING_TEST_DOMAIN = "google.com"
 
-PING_WAIT_TIMEOUT_IN_MILLISECONDS = 20 * 1000
-
 CREATED_PROCESS_AS_USER_PING_SUCCESS_FORMAT = "Created process '{}' as user '{}', and successfully pinged."
 CREATED_PROCESS_AS_USER_PING_FAILED_FORMAT = "Created process '{}' as user '{}', but failed to ping (exit status {})."
 
-USERNAME = "somenewuser"
+USERNAME_PREFIX = "somenewuser"
 PASSWORD = "N3WPa55W0rD!1"
 
 logger = logging.getLogger(__name__)
@@ -35,90 +33,26 @@ class CommunicateAsNewUser(PBA):
 
     def run(self):
         username = CommunicateAsNewUser.get_random_new_user_name()
-        if is_windows_os():
-            self.communicate_as_new_user_windows(username)
-        else:
-            self.communicate_as_new_user_linux(username)
+        try:
+            with create_auto_new_user(username, PASSWORD) as new_user:
+                ping_commandline = CommunicateAsNewUser.get_commandline_for_ping()
+                exit_status = new_user.run_as(ping_commandline)
+                self.send_ping_result_telemetry(exit_status, ping_commandline, username)
+        except subprocess.CalledProcessError as e:
+            PostBreachTelem(self, (e.output, False)).send()
+        except NewUserError as e:
+            PostBreachTelem(self, (str(e), False)).send()
 
     @staticmethod
     def get_random_new_user_name():
-        return USERNAME + ''.join(random.choice(string.ascii_lowercase) for _ in range(5))
+        return USERNAME_PREFIX + ''.join(random.choice(string.ascii_lowercase) for _ in range(5))
 
-    def communicate_as_new_user_linux(self, username):
-        try:
-            with create_auto_new_user(username, PASSWORD, is_windows=False) as _:
-                commandline = "sudo -u {username} ping -c 1 {domain}".format(
-                    username=username,
-                    domain=PING_TEST_DOMAIN)
-                exit_status = os.system(commandline)
-                self.send_ping_result_telemetry(exit_status, commandline, username)
-        except subprocess.CalledProcessError as e:
-            PostBreachTelem(self, (e.output, False)).send()
-
-    def communicate_as_new_user_windows(self, username):
-        # Importing these only on windows, as they won't exist on linux.
-        import win32con
-        import win32process
-        import win32api
-        import win32event
-
-        try:
-            with create_auto_new_user(username, PASSWORD, is_windows=True) as new_user:
-                # Using os.path is OK, as this is on windows for sure
-                ping_app_path = os.path.join(os.environ["WINDIR"], "system32", "PING.exe")
-                if not os.path.exists(ping_app_path):
-                    PostBreachTelem(self, ("{} not found.".format(ping_app_path), False)).send()
-                    return  # Can't continue without ping.
-
-                try:
-                    # Open process as that user:
-                    # https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessasusera
-                    commandline = "{} {} {} {}".format(ping_app_path, PING_TEST_DOMAIN, "-n", "1")
-                    process_handle, thread_handle, _, _ = win32process.CreateProcessAsUser(
-                        new_user.get_logon_handle(),  # A handle to the primary token that represents a user.
-                        None,  # The name of the module to be executed.
-                        commandline,  # The command line to be executed.
-                        None,  # Process attributes
-                        None,  # Thread attributes
-                        True,  # Should inherit handles
-                        win32con.NORMAL_PRIORITY_CLASS,  # The priority class and the creation of the process.
-                        None,  # An environment block for the new process. If this parameter is NULL, the new process
-                        # uses the environment of the calling process.
-                        None,  # CWD. If this parameter is NULL, the new process will have the same current drive and
-                        # directory as the calling process.
-                        win32process.STARTUPINFO()  # STARTUPINFO structure.
-                        # https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/ns-processthreadsapi-startupinfoa
-                    )
-
-                    logger.debug(
-                        "Waiting for ping process to finish. Timeout: {}ms".format(PING_WAIT_TIMEOUT_IN_MILLISECONDS))
-
-                    # Ignoring return code, as we'll use `GetExitCode` to determine the state of the process later.
-                    _ = win32event.WaitForSingleObject(  # Waits until the specified object is signaled, or time-out.
-                        process_handle,  # Ping process handle
-                        PING_WAIT_TIMEOUT_IN_MILLISECONDS  # Timeout in milliseconds
-                    )
-
-                    ping_exit_code = win32process.GetExitCodeProcess(process_handle)
-
-                    self.send_ping_result_telemetry(ping_exit_code, commandline, username)
-                except Exception as e:
-                    # If failed on 1314, it's possible to try to elevate the rights of the current user with the
-                    #  "Replace a process level token" right, using Local Security Policy editing.
-                    PostBreachTelem(self, (
-                        "Failed to open process as user {}. Error: {}".format(username, str(e)), False)).send()
-                finally:
-                    try:
-                        win32api.CloseHandle(process_handle)
-                        win32api.CloseHandle(thread_handle)
-                    except Exception as err:
-                        logger.error("Close handle error: " + str(err))
-        except subprocess.CalledProcessError as err:
-            PostBreachTelem(self, (
-                "Couldn't create the user '{}'. Error output is: '{}'".format(username, str(err)),
-                False)).send()
-        except NewUserError as e:
-            PostBreachTelem(self, (str(e), False)).send()
+    @staticmethod
+    def get_commandline_for_ping(domain=PING_TEST_DOMAIN, is_windows=is_windows_os()):
+        if is_windows:
+            return "{} {} {} {}".format("PING.exe", domain, "-n", "1")
+        else:
+            return "ping -c 1 {domain}".format(domain=PING_TEST_DOMAIN)
 
     def send_ping_result_telemetry(self, exit_status, commandline, username):
         """
diff --git a/monkey/infection_monkey/utils/auto_new_user.py b/monkey/infection_monkey/utils/auto_new_user.py
index e8e42b309..c4b8d2f1a 100644
--- a/monkey/infection_monkey/utils/auto_new_user.py
+++ b/monkey/infection_monkey/utils/auto_new_user.py
@@ -32,3 +32,7 @@ class AutoNewUser:
     @abc.abstractmethod
     def __exit__(self, exc_type, exc_val, exc_tb):
         raise NotImplementedError()
+
+    @abc.abstractmethod
+    def run_as(self, command):
+        raise NotImplementedError()
diff --git a/monkey/infection_monkey/utils/linux/users.py b/monkey/infection_monkey/utils/linux/users.py
index 25e3bdd77..46bc0ed87 100644
--- a/monkey/infection_monkey/utils/linux/users.py
+++ b/monkey/infection_monkey/utils/linux/users.py
@@ -1,5 +1,6 @@
 import datetime
 import logging
+import os
 import subprocess
 
 from infection_monkey.utils.auto_new_user import AutoNewUser
@@ -31,6 +32,7 @@ class AutoNewLinuxUser(AutoNewUser):
     """
     See AutoNewUser's documentation for details.
     """
+
     def __init__(self, username, password):
         """
         Creates a user with the username + password.
@@ -45,6 +47,10 @@ class AutoNewLinuxUser(AutoNewUser):
     def __enter__(self):
         pass  # No initialization/logging on needed in Linux
 
+    def run_as(self, command):
+        command_as_new_user = "sudo -u {username} {command}".format(username=self.username, command=command)
+        return os.system(command_as_new_user)
+
     def __exit__(self, exc_type, exc_val, exc_tb):
         # delete the user.
         commands_to_delete_user = get_linux_commands_to_delete_user(self.username)
diff --git a/monkey/infection_monkey/utils/windows/users.py b/monkey/infection_monkey/utils/windows/users.py
index 3bbf9522e..1bc3f2738 100644
--- a/monkey/infection_monkey/utils/windows/users.py
+++ b/monkey/infection_monkey/utils/windows/users.py
@@ -5,6 +5,7 @@ from infection_monkey.utils.auto_new_user import AutoNewUser
 from infection_monkey.utils.new_user_error import NewUserError
 
 ACTIVE_NO_NET_USER = '/ACTIVE:NO'
+WAIT_TIMEOUT_IN_MILLISECONDS = 20 * 1000
 
 logger = logging.getLogger(__name__)
 
@@ -40,6 +41,58 @@ class AutoNewWindowsUser(AutoNewUser):
     """
     See AutoNewUser's documentation for details.
     """
+
+    def run_as(self, command):
+        # Importing these only on windows, as they won't exist on linux.
+        import win32con
+        import win32process
+        import win32api
+        import win32event
+
+        exit_code = -1
+        process_handle = None
+        thread_handle = None
+
+        try:
+            # Open process as that user:
+            # https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessasusera
+            process_handle, thread_handle, _, _ = win32process.CreateProcessAsUser(
+                self.get_logon_handle(),  # A handle to the primary token that represents a user.
+                None,  # The name of the module to be executed.
+                command,  # The command line to be executed.
+                None,  # Process attributes
+                None,  # Thread attributes
+                True,  # Should inherit handles
+                win32con.NORMAL_PRIORITY_CLASS,  # The priority class and the creation of the process.
+                None,  # An environment block for the new process. If this parameter is NULL, the new process
+                # uses the environment of the calling process.
+                None,  # CWD. If this parameter is NULL, the new process will have the same current drive and
+                # directory as the calling process.
+                win32process.STARTUPINFO()  # STARTUPINFO structure.
+                # https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/ns-processthreadsapi-startupinfoa
+            )
+
+            logger.debug(
+                "Waiting for process to finish. Timeout: {}ms".format(WAIT_TIMEOUT_IN_MILLISECONDS))
+
+            # Ignoring return code, as we'll use `GetExitCode` to determine the state of the process later.
+            _ = win32event.WaitForSingleObject(  # Waits until the specified object is signaled, or time-out.
+                process_handle,  # Ping process handle
+                WAIT_TIMEOUT_IN_MILLISECONDS  # Timeout in milliseconds
+            )
+
+            exit_code = win32process.GetExitCodeProcess(process_handle)
+        finally:
+            try:
+                if process_handle is not None:
+                    win32api.CloseHandle(process_handle)
+                if thread_handle is not None:
+                    win32api.CloseHandle(thread_handle)
+            except Exception as err:
+                logger.error("Close handle error: " + str(err))
+
+        return exit_code
+
     def __init__(self, username, password):
         """
         Creates a user with the username + password.
@@ -94,7 +147,7 @@ class AutoNewWindowsUser(AutoNewUser):
         try:
             commands_to_delete_user = get_windows_commands_to_delete_user(self.username)
             logger.debug(
-                "Trying to deactivate {} with commands {}".format(self.username, str(commands_to_delete_user)))
+                "Trying to delete {} with commands {}".format(self.username, str(commands_to_delete_user)))
             _ = subprocess.check_output(
                 commands_to_delete_user, stderr=subprocess.STDOUT, shell=True)
         except Exception as err:

From 90be53e920d81185a897ccb0355336214d8f3384 Mon Sep 17 00:00:00 2001
From: Shay Nehmad <shay.nehmad@guardicore.com>
Date: Thu, 3 Oct 2019 16:58:32 +0300
Subject: [PATCH 13/15] Forgot to return the instance upon __enter__ call on
 LinuxNewUser

---
 monkey/infection_monkey/utils/linux/users.py  |  2 +-
 .../infection_monkey/utils/windows/users.py   | 57 +++++++++----------
 2 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/monkey/infection_monkey/utils/linux/users.py b/monkey/infection_monkey/utils/linux/users.py
index 46bc0ed87..e4968c522 100644
--- a/monkey/infection_monkey/utils/linux/users.py
+++ b/monkey/infection_monkey/utils/linux/users.py
@@ -45,7 +45,7 @@ class AutoNewLinuxUser(AutoNewUser):
         _ = subprocess.check_output(' '.join(commands_to_add_user), stderr=subprocess.STDOUT, shell=True)
 
     def __enter__(self):
-        pass  # No initialization/logging on needed in Linux
+        return self  # No initialization/logging on needed in Linux
 
     def run_as(self, command):
         command_as_new_user = "sudo -u {username} {command}".format(username=self.username, command=command)
diff --git a/monkey/infection_monkey/utils/windows/users.py b/monkey/infection_monkey/utils/windows/users.py
index 1bc3f2738..857cddd07 100644
--- a/monkey/infection_monkey/utils/windows/users.py
+++ b/monkey/infection_monkey/utils/windows/users.py
@@ -41,6 +41,34 @@ class AutoNewWindowsUser(AutoNewUser):
     """
     See AutoNewUser's documentation for details.
     """
+    def __init__(self, username, password):
+        """
+        Creates a user with the username + password.
+        :raises: subprocess.CalledProcessError if failed to add the user.
+        """
+        super(AutoNewWindowsUser, self).__init__(username, password)
+
+        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, shell=True)
+
+    def __enter__(self):
+        # Importing these only on windows, as they won't exist on linux.
+        import win32security
+        import win32con
+
+        try:
+            # Logon as new user: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-logonusera
+            self.logon_handle = win32security.LogonUser(
+                self.username,
+                ".",  # Use current domain.
+                self.password,
+                win32con.LOGON32_LOGON_INTERACTIVE,  # Logon type - interactive (normal user). Need this to open ping
+                                                     # using a shell.
+                win32con.LOGON32_PROVIDER_DEFAULT)  # Which logon provider to use - whatever Windows offers.
+        except Exception as err:
+            raise NewUserError("Can't logon as {}. Error: {}".format(self.username, str(err)))
+        return self
 
     def run_as(self, command):
         # Importing these only on windows, as they won't exist on linux.
@@ -93,35 +121,6 @@ class AutoNewWindowsUser(AutoNewUser):
 
         return exit_code
 
-    def __init__(self, username, password):
-        """
-        Creates a user with the username + password.
-        :raises: subprocess.CalledProcessError if failed to add the user.
-        """
-        super(AutoNewWindowsUser, self).__init__(username, password)
-
-        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, shell=True)
-
-    def __enter__(self):
-        # Importing these only on windows, as they won't exist on linux.
-        import win32security
-        import win32con
-
-        try:
-            # Logon as new user: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-logonusera
-            self.logon_handle = win32security.LogonUser(
-                self.username,
-                ".",  # Use current domain.
-                self.password,
-                win32con.LOGON32_LOGON_INTERACTIVE,  # Logon type - interactive (normal user). Need this to open ping
-                                                     # using a shell.
-                win32con.LOGON32_PROVIDER_DEFAULT)  # Which logon provider to use - whatever Windows offers.
-        except Exception as err:
-            raise NewUserError("Can't logon as {}. Error: {}".format(self.username, str(err)))
-        return self
-
     def get_logon_handle(self):
         return self.logon_handle
 

From 46868e9996c2d49d7202c024a6f986dc93506f15 Mon Sep 17 00:00:00 2001
From: Shay Nehmad <shay.nehmad@guardicore.com>
Date: Thu, 3 Oct 2019 18:35:51 +0300
Subject: [PATCH 14/15] CR fixes

---
 .../post_breach/actions/communicate_as_new_user.py          | 6 ++----
 monkey/infection_monkey/utils/auto_new_user.py              | 4 ++++
 monkey/infection_monkey/utils/linux/users.py                | 2 +-
 monkey/infection_monkey/utils/windows/users.py              | 6 ++++--
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
index b98361ec9..5c1af693a 100644
--- a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
+++ b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
@@ -49,10 +49,8 @@ class CommunicateAsNewUser(PBA):
 
     @staticmethod
     def get_commandline_for_ping(domain=PING_TEST_DOMAIN, is_windows=is_windows_os()):
-        if is_windows:
-            return "{} {} {} {}".format("PING.exe", domain, "-n", "1")
-        else:
-            return "ping -c 1 {domain}".format(domain=PING_TEST_DOMAIN)
+        format_string = "PING.exe {domain} -n 1" if is_windows else "ping -c 1 {domain}"
+        format_string.format(domain=domain)
 
     def send_ping_result_telemetry(self, exit_status, commandline, username):
         """
diff --git a/monkey/infection_monkey/utils/auto_new_user.py b/monkey/infection_monkey/utils/auto_new_user.py
index c4b8d2f1a..e749020d6 100644
--- a/monkey/infection_monkey/utils/auto_new_user.py
+++ b/monkey/infection_monkey/utils/auto_new_user.py
@@ -35,4 +35,8 @@ class AutoNewUser:
 
     @abc.abstractmethod
     def run_as(self, command):
+        """
+        Run the given command as the new user that was created.
+        :param command: The command to run - give as shell commandline (e.g. "ping google.com -n 1")
+        """
         raise NotImplementedError()
diff --git a/monkey/infection_monkey/utils/linux/users.py b/monkey/infection_monkey/utils/linux/users.py
index e4968c522..34becb8f7 100644
--- a/monkey/infection_monkey/utils/linux/users.py
+++ b/monkey/infection_monkey/utils/linux/users.py
@@ -55,4 +55,4 @@ class AutoNewLinuxUser(AutoNewUser):
         # delete the user.
         commands_to_delete_user = get_linux_commands_to_delete_user(self.username)
         logger.debug("Trying to delete {} with commands {}".format(self.username, str(commands_to_delete_user)))
-        _ = subprocess.check_output(" ".join(commands_to_delete_user), stderr=subprocess.STDOUT, shell=True)
\ No newline at end of file
+        _ = subprocess.check_output(" ".join(commands_to_delete_user), stderr=subprocess.STDOUT, shell=True)
diff --git a/monkey/infection_monkey/utils/windows/users.py b/monkey/infection_monkey/utils/windows/users.py
index 857cddd07..cf6eb73c4 100644
--- a/monkey/infection_monkey/utils/windows/users.py
+++ b/monkey/infection_monkey/utils/windows/users.py
@@ -9,6 +9,7 @@ WAIT_TIMEOUT_IN_MILLISECONDS = 20 * 1000
 
 logger = logging.getLogger(__name__)
 
+
 def get_windows_commands_to_add_user(username, password, should_be_active=False):
     windows_cmds = [
         'net',
@@ -41,6 +42,7 @@ class AutoNewWindowsUser(AutoNewUser):
     """
     See AutoNewUser's documentation for details.
     """
+
     def __init__(self, username, password):
         """
         Creates a user with the username + password.
@@ -64,7 +66,7 @@ class AutoNewWindowsUser(AutoNewUser):
                 ".",  # Use current domain.
                 self.password,
                 win32con.LOGON32_LOGON_INTERACTIVE,  # Logon type - interactive (normal user). Need this to open ping
-                                                     # using a shell.
+                # using a shell.
                 win32con.LOGON32_PROVIDER_DEFAULT)  # Which logon provider to use - whatever Windows offers.
         except Exception as err:
             raise NewUserError("Can't logon as {}. Error: {}".format(self.username, str(err)))
@@ -150,4 +152,4 @@ class AutoNewWindowsUser(AutoNewUser):
             _ = subprocess.check_output(
                 commands_to_delete_user, stderr=subprocess.STDOUT, shell=True)
         except Exception as err:
-            raise NewUserError("Can't delete user {}. Info: {}".format(self.username, err))
\ No newline at end of file
+            raise NewUserError("Can't delete user {}. Info: {}".format(self.username, err))

From 30f59c4f2bcc7ba4dd684386862a55cadb0ed017 Mon Sep 17 00:00:00 2001
From: Shay Nehmad <shay.nehmad@guardicore.com>
Date: Thu, 3 Oct 2019 18:36:51 +0300
Subject: [PATCH 15/15] forgot to add return

---
 .../post_breach/actions/communicate_as_new_user.py              | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
index 5c1af693a..04dff1441 100644
--- a/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
+++ b/monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py
@@ -50,7 +50,7 @@ class CommunicateAsNewUser(PBA):
     @staticmethod
     def get_commandline_for_ping(domain=PING_TEST_DOMAIN, is_windows=is_windows_os()):
         format_string = "PING.exe {domain} -n 1" if is_windows else "ping -c 1 {domain}"
-        format_string.format(domain=domain)
+        return format_string.format(domain=domain)
 
     def send_ping_result_telemetry(self, exit_status, commandline, username):
         """