Merge pull request #266 from jjnicola/dead-host

Improve handling of dead hosts and improve scan progress calculation
greenbone · May 11, 2020 · 6786774 · 6786774
2 parents 2a0c91a + 79c6388
commit 6786774
Show file tree

Hide file tree

Showing 7 changed files with 265 additions and 215 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Add details attribute to get_vts command. [#222](https://github.com/greenbone/ospd/pull/222)
 - Add [pontos](https://github.com/greenbone/pontos) as dev dependency for
   managing the version information in ospd [#254](https://github.com/greenbone/ospd/pull/254)
+- Add more info about scan progress with progress attribute in get_scans cmd. [#266](https://github.com/greenbone/ospd/pull/266)
 
 ### Changes
 - Modify __init__() method and use new syntax for super(). [#186](https://github.com/greenbone/ospd/pull/186)
@@ -30,11 +31,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   a bit different then `pipenv install`. It installs dev packages by default and
   also ospd in editable mode. This means after running poetry install ospd will
   directly be importable in the virtual python environment. [#252](https://github.com/greenbone/ospd/pull/252)
+- Progress bar calculation does not take in account the dead hosts. [#266](https://github.com/greenbone/ospd/pull/266)
 
 ### Fixed
 - Fix stop scan. Wait for the scan process to be stopped before delete it from the process table. [#204](https://github.com/greenbone/ospd/pull/204)
 - Fix get_scanner_details(). [#210](https://github.com/greenbone/ospd/pull/210)
 
+### Removed
+- Remove support for resume task. [#266](https://github.com/greenbone/ospd/pull/266)
+
 ## [2.0.1] (unreleased)
 
 ### Added

diff --git a/doc/OSP.xml b/doc/OSP.xml
@@ -208,7 +208,7 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
     </ele>
     <ele>
       <name>finished_hosts</name>
-      <summary>One or many finished hosts to exclude when resuming a task. The list is comma-separated. Each entry can be an IP address, a CIDR notation, a hostname, a IP range. IPs can be v4 or v6. The listed hosts will be set as finished before starting the scan. Each wrapper must handle the finished hosts.
+      <summary>One or many finished hosts to exclude when the client resumes a task. The list is comma-separated. Each entry can be an IP address, a CIDR notation, a hostname, a IP range. IPs can be v4 or v6. The listed hosts will be set as finished before starting the scan. Each wrapper must handle the finished hosts.
       </summary>
       <type>string</type>
     </ele>
@@ -440,6 +440,7 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
             <attributes>
               <scan_id>ID of a specific scan to get</scan_id>
               <details>Whether to return the full scan report</details>
+              <progress>Whether to return a detailed progress information</progress>
               <pop_results>Whether to remove the fetched results</pop_results>
               <max_results>Maximum number of results to fetch. Only considered if pop_results is enabled. Default = None, which means that all available results are returned</max_results>
             </attributes>
@@ -521,6 +522,11 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
         <summary>Whether to get full scan reports</summary>
         <type>boolean</type>
       </attrib>
+      <attrib>
+        <name>progress</name>
+        <summary>Whether to return a detailed progress information</summary>
+        <type>boolean</type>
+      </attrib>
       <attrib>
         <name>pop_results</name>
         <summary>Whether to remove the fetched results</summary>
@@ -624,6 +630,30 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
         </get_scans_response>
       </response>
     </example>
+    <example>
+      <summary>Get a scan progress summary</summary>
+      <request>
+        <get_scans scan_id="f14747d3-a4d7-4e79-99bb-a0a1276cb78c" details="0" progress="1"/>
+      </request>
+      <response>
+        <get_scans_response status_text="OK" status="200">
+          <scan id="9750f1f8-07aa-49cc-9c31-2f9e469c8f65" target="192.168.1.252"
+                end_time="1432824234" progress="100" status="finished" start_time="1432824206">
+            <progress>
+              <host name="192.168.0.176">0.0</host>
+              <host name="192.168.0.77">68.00013854253257</host>
+              <host name="192.168.0.148">26.0009697977279</host>
+              <host name="192.168.0.1">0.0</host>
+              <overall>38.800221668052096</overall>
+              <count_alive>1</count_alive>
+              <count_dead>249</count_dead>
+              <count_excluded>0</count_excluded>
+              <count_total>254</count_total>
+            </progress>
+          </scan>
+        </get_scans_response>
+      </response>
+    </example>
   </command>
   <command>
     <name>delete_scan</name>

diff --git a/ospd/command/command.py b/ospd/command/command.py
@@ -412,22 +412,25 @@ def handle_xml(self, xml: Element) -> bytes:
         details = xml.get('details')
         pop_res = xml.get('pop_results')
         max_res = xml.get('max_results')
+        progress = xml.get('progress')
 
         if details and details == '0':
             details = False
         else:
             details = True
-            if pop_res and pop_res == '1':
-                pop_res = True
-            else:
-                pop_res = False
+            pop_res = pop_res and pop_res == '1'
+
             if max_res:
                 max_res = int(max_res)
 
+        progress = progress and progress == '1'
+
         responses = []
         if scan_id and scan_id in self._daemon.scan_collection.ids_iterator():
             self._daemon.check_scan_process(scan_id)
-            scan = self._daemon.get_scan_xml(scan_id, details, pop_res, max_res)
+            scan = self._daemon.get_scan_xml(
+                scan_id, details, pop_res, max_res, progress
+            )
             responses.append(scan)
         elif scan_id:
             text = "Failed to find scan '{0}'".format(scan_id)
@@ -436,7 +439,7 @@ def handle_xml(self, xml: Element) -> bytes:
             for scan_id in self._daemon.scan_collection.ids_iterator():
                 self._daemon.check_scan_process(scan_id)
                 scan = self._daemon.get_scan_xml(
-                    scan_id, details, pop_res, max_res
+                    scan_id, details, pop_res, max_res, progress
                 )
                 responses.append(scan)
 

diff --git a/ospd/ospd.py b/ospd/ospd.py
@@ -56,6 +56,7 @@
 from ospd.xml import (
     elements_as_text,
     get_result_xml,
+    get_progress_xml,
     get_elements_from_dict,
 )
 
@@ -82,6 +83,9 @@
     },
 }  # type: Dict
 
+PROGRESS_FINISHED = 100
+PROGRESS_DEAD_HOST = -1
+
 
 def _terminate_process_group(process: multiprocessing.Process) -> None:
     os.killpg(os.getpgid(process.pid), 15)
@@ -410,7 +414,7 @@ def exec_scan(self, scan_id: str):
 
     def finish_scan(self, scan_id: str) -> None:
         """ Sets a scan as finished. """
-        self.set_scan_progress(scan_id, 100)
+        self.scan_collection.set_progress(scan_id, PROGRESS_FINISHED)
         self.set_scan_status(scan_id, ScanStatus.FINISHED)
         logger.info("%s: Scan finished.", scan_id)
 
@@ -505,29 +509,14 @@ def calculate_progress(self, scan_id: str) -> float:
 
         return self.scan_collection.calculate_target_progress(scan_id)
 
-    def process_exclude_hosts(self, scan_id: str, exclude_hosts: str) -> None:
-        """ Process the exclude hosts before launching the scans."""
-
-        exc_hosts_list = ''
-        if not exclude_hosts:
-            return
-        exc_hosts_list = target_str_to_list(exclude_hosts)
-        self.remove_scan_hosts_from_target_progress(scan_id, exc_hosts_list)
-
     def process_finished_hosts(self, scan_id: str, finished_hosts: str) -> None:
-        """ Process the finished hosts before launching the scans.
-        Set finished hosts as finished with 100% to calculate
-        the scan progress."""
+        """ Process the finished hosts before launching the scans."""
 
-        exc_hosts_list = ''
         if not finished_hosts:
             return
 
-        exc_hosts_list = target_str_to_list(finished_hosts)
-
-        for host in exc_hosts_list:
-            self.set_scan_host_finished(scan_id, finished_hosts=host)
-            self.set_scan_host_progress(scan_id, host=host, progress=100)
+        exc_finished_hosts_list = target_str_to_list(finished_hosts)
+        self.scan_collection.set_host_finished(scan_id, exc_finished_hosts_list)
 
     def start_scan(self, scan_id: str, target: Dict) -> None:
         """ Starts the scan with scan_id. """
@@ -538,7 +527,6 @@ def start_scan(self, scan_id: str, target: Dict) -> None:
 
         logger.info("%s: Scan started.", scan_id)
 
-        self.process_exclude_hosts(scan_id, target.get('exclude_hosts'))
         self.process_finished_hosts(scan_id, target.get('finished_hosts'))
 
         try:
@@ -584,35 +572,42 @@ def handle_timeout(self, scan_id: str, host: str) -> None:
             value="{0} exec timeout.".format(self.get_scanner_name()),
         )
 
-    def remove_scan_hosts_from_target_progress(
-        self, scan_id: str, exc_hosts_list: List
-    ) -> None:
-        """ Remove a list of hosts from the main scan progress table."""
-        self.scan_collection.remove_hosts_from_target_progress(
-            scan_id, exc_hosts_list
-        )
-
-    def set_scan_host_finished(
+    def sort_host_finished(
         self, scan_id: str, finished_hosts: Union[List[str], str],
     ) -> None:
-        """ Add the host in a list of finished hosts """
+        """ Check if the finished host in the list was alive or dead
+        and update the corresponding alive_count or dead_count. """
         if isinstance(finished_hosts, str):
             finished_hosts = [finished_hosts]
 
-        self.scan_collection.set_host_finished(scan_id, finished_hosts)
+        alive_hosts = []
+        dead_hosts = []
+
+        current_hosts = self.scan_collection.get_current_target_progress(
+            scan_id
+        )
+        for finished_host in finished_hosts:
+            progress = current_hosts.get(finished_host)
+            if progress == PROGRESS_FINISHED:
+                alive_hosts.append(finished_host)
+            if progress == PROGRESS_DEAD_HOST:
+                dead_hosts.append(finished_host)
+
+        self.scan_collection.set_host_dead(scan_id, dead_hosts)
 
-    def set_scan_progress(self, scan_id: str, progress: int) -> None:
-        """ Sets scan_id scan's progress which is a number
-        between 0 and 100. """
-        self.scan_collection.set_progress(scan_id, progress)
+        self.scan_collection.set_host_finished(scan_id, alive_hosts)
+
+        self.scan_collection.remove_hosts_from_target_progress(
+            scan_id, finished_hosts
+        )
 
     def set_scan_progress_batch(
         self, scan_id: str, host_progress: Dict[str, int]
     ):
         self.scan_collection.set_host_progress(scan_id, host_progress)
 
         scan_progress = self.calculate_progress(scan_id)
-        self.set_scan_progress(scan_id, scan_progress)
+        self.scan_collection.set_progress(scan_id, scan_progress)
 
     def set_scan_host_progress(
         self, scan_id: str, host: str = None, progress: int = None,
@@ -621,9 +616,6 @@ def set_scan_host_progress(
         Each time a host progress is updated, the scan progress
         is updated too.
         """
-        if host and progress < 0 or progress > 100:
-            return
-
         host_progress = {host: progress}
         self.set_scan_progress_batch(scan_id, host_progress)
 
@@ -711,6 +703,32 @@ def get_scan_results_xml(
         logger.debug('Returning %d results', len(results))
         return results
 
+    def _get_scan_progress_xml(self, scan_id: str):
+        """ Gets scan_id scan's progress in XML format.
+
+        @return: String of scan progress in xml.
+        """
+        current_progress = dict()
+
+        current_progress[
+            'current_hosts'
+        ] = self.scan_collection.get_current_target_progress(scan_id)
+        current_progress['overall'] = self.scan_collection.get_progress(scan_id)
+        current_progress['count_alive'] = self.scan_collection.get_count_alive(
+            scan_id
+        )
+        current_progress['count_dead'] = self.scan_collection.get_count_dead(
+            scan_id
+        )
+        current_progress[
+            'count_excluded'
+        ] = self.scan_collection.simplify_exclude_host_count(scan_id)
+        current_progress['count_total'] = self.scan_collection.get_host_count(
+            scan_id
+        )
+
+        return get_progress_xml(current_progress)
+
     @deprecated(
         version="20.8",
         reason="Please use ospd.xml.get_elements_from_dict instead.",
@@ -730,6 +748,7 @@ def get_scan_xml(
         detailed: bool = True,
         pop_res: bool = False,
         max_res: int = 0,
+        progress: bool = False,
     ):
         """ Gets scan in XML format.
 
@@ -757,6 +776,9 @@ def get_scan_xml(
             response.append(
                 self.get_scan_results_xml(scan_id, pop_res, max_res)
             )
+        if progress:
+            response.append(self._get_scan_progress_xml(scan_id))
+
         return response
 
     @staticmethod
@@ -1178,23 +1200,17 @@ def create_scan(
         @target: Target to scan.
         @options: Miscellaneous scan options.
 
-        @return: New scan's ID. None if the scan_id already exists and the
-                 scan status is RUNNING or FINISHED.
+        @return: New scan's ID. None if the scan_id already exists.
         """
         status = None
         scan_exists = self.scan_exists(scan_id)
         if scan_id and scan_exists:
             status = self.get_scan_status(scan_id)
-
-        if scan_exists and status == ScanStatus.STOPPED:
-            logger.info("Scan %s exists. Resuming scan.", scan_id)
-        elif scan_exists and (
-            status == ScanStatus.RUNNING or status == ScanStatus.FINISHED
-        ):
             logger.info(
                 "Scan %s exists with status %s.", scan_id, status.name.lower()
             )
             return
+
         return self.scan_collection.create_scan(
             scan_id, targets, options, vt_selection
         )
@@ -1238,7 +1254,7 @@ def check_scan_process(self, scan_id: str) -> None:
         scan_process = self.scan_processes[scan_id]
         progress = self.get_scan_progress(scan_id)
 
-        if progress < 100 and not scan_process.is_alive():
+        if progress < PROGRESS_FINISHED and not scan_process.is_alive():
             if not self.get_scan_status(scan_id) == ScanStatus.STOPPED:
                 self.set_scan_status(scan_id, ScanStatus.STOPPED)
                 self.add_scan_error(
@@ -1247,7 +1263,7 @@ def check_scan_process(self, scan_id: str) -> None:
 
                 logger.info("%s: Scan stopped with errors.", scan_id)
 
-        elif progress == 100:
+        elif progress == PROGRESS_FINISHED:
             scan_process.join(0)
 
     def get_scan_progress(self, scan_id: str):
@@ -1281,14 +1297,6 @@ def get_scan_vts(self, scan_id: str) -> Dict:
         """ Gives a scan's vts. """
         return self.scan_collection.get_vts(scan_id)
 
-    def get_scan_unfinished_hosts(self, scan_id: str) -> List:
-        """ Get a list of unfinished hosts."""
-        return self.scan_collection.get_hosts_unfinished(scan_id)
-
-    def get_scan_finished_hosts(self, scan_id: str) -> List:
-        """ Get a list of unfinished hosts."""
-        return self.scan_collection.get_hosts_finished(scan_id)
-
     def get_scan_start_time(self, scan_id: str) -> str:
         """ Gives a scan's start time. """
         return self.scan_collection.get_start_time(scan_id)