Merge branch '2817-replace-agent-singleton' into develop

Issue #2817
PR #3068

Author: mssalvatore

CHANGELOG.md

@@ -19,6 +19,8 @@ Changelog](https://keepachangelog.com/en/1.0.0/).
 ### Security
 - Fixed plaintext private key in SSHKey pair list in UI. #2950
 - MongoDB version from 4.x to 6.0.4. #2706
+- Replaced the `SystemSingleton` component, which could allow local users to
+  execute a DoS attack against agents. #2817
 
 ## [2.0.0] - 2023-02-08
 ### Added

monkey/infection_monkey/monkey.py

@@ -83,7 +83,6 @@
     PluginSourceExtractor,
 )
 from infection_monkey.puppet.puppet import Puppet
-from infection_monkey.system_singleton import SystemSingleton
 from infection_monkey.utils import agent_process, environment
 from infection_monkey.utils.file_utils import mark_file_for_deletion_on_windows
 from infection_monkey.utils.ids import get_agent_id, get_machine_id
@@ -107,12 +106,9 @@ def __init__(self, args, ipc_logger_queue: multiprocessing.Queue, log_path: Path
         logger.info(f"Agent ID: {self._agent_id}")
         logger.info(f"Process ID: {os.getpid()}")
 
-        # Spawn the manager before the acquiring the singleton in case the file handle gets copied
-        # over to the manager process
         context = multiprocessing.get_context("spawn")
         self._manager = context.Manager()
 
-        self._singleton = SystemSingleton()
         self._opts = self._get_arguments(args)
 
         self._ipc_logger_queue = ipc_logger_queue
@@ -222,12 +218,10 @@ def start(self):
         self._agent_event_forwarder.start()
         self._plugin_event_forwarder.start()
 
-        if self._is_another_monkey_running():
-            logger.info("Another instance of the monkey is already running")
-            return
-
         logger.info("Agent is starting...")
 
+        # This check must be done after the agent event forwarder is started, otherwise the agent
+        # will be unable to send a shutdown event to the Island.
         should_stop = self._control_channel.should_agent_stop()
         if should_stop:
             logger.info("The Monkey Island has instructed this agent to stop")
@@ -435,9 +429,6 @@ def _subscribe_events(self):
                 PropagationEvent, notify_relay_on_propagation(self._relay)
             )
 
-    def _is_another_monkey_running(self):
-        return not self._singleton.try_lock()
-
     def cleanup(self):
         logger.info("Agent cleanup started")
         deleted = None
@@ -459,7 +450,9 @@ def cleanup(self):
             self._publish_agent_shutdown_event()
 
             self._plugin_event_forwarder.flush()
-            self._agent_event_forwarder.flush()
+
+            if self._agent_event_forwarder:
+                self._agent_event_forwarder.flush()
 
             self._heart.stop()
 
@@ -471,13 +464,10 @@ def cleanup(self):
                 InfectionMonkey._self_delete()
         finally:
             self._plugin_event_forwarder.stop()
-            self._agent_event_forwarder.stop()
+            if self._agent_event_forwarder:
+                self._agent_event_forwarder.stop()
             self._delete_plugin_dir()
             self._manager.shutdown()
-            try:
-                self._singleton.unlock()
-            except AssertionError as err:
-                logger.warning(f"Failed to release the singleton: {err}")
 
         logger.info("Agent is shutting down")
 

monkey/infection_monkey/plugin_event_forwarder.py

@@ -64,8 +64,9 @@ def stop(self, timeout=None):
         :param timeout: The number of seconds to wait for the PluginEventForwarder to stop
         """
         logger.info("Stopping plugin event forwarder")
-        self._stop.set()
-        self._thread.join(timeout)
+        if self._thread.is_alive():
+            self._stop.set()
+            self._thread.join(timeout)
         self.flush()
 
     def flush(self):