Improve structure

CHANGELOG.md

@@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Add dry run support. Generate 10 fake results per host. [#424](https://github.com/greenbone/ospd-openvas/pull/424)
 
 ### Changed
+- Stopping and interrupting scans. [#450](https://github.com/greenbone/ospd-openvas/pull/450)
 ### Removed
 
 - Remove source_iface preferences. [#418](https://github.com/greenbone/ospd-openvas/pull/418)

ospd_openvas/daemon.py

@@ -35,7 +35,7 @@
 import psutil
 
 from ospd.ospd import OSPDaemon
-from ospd.scan import ScanProgress
+from ospd.scan import ScanProgress, ScanStatus
 from ospd.server import BaseServer
 from ospd.main import main as daemon_main
 from ospd.vtfilter import VtsFilter
@@ -456,6 +456,8 @@ def __init__(
 
         self.scan_only_params = dict()
 
+        self.openvas_processes = {}
+
     def init(self, server: BaseServer) -> None:
 
         self.scan_collection.init()
@@ -481,7 +483,7 @@ def set_params_from_openvas_settings(self):
         """Set OSPD_PARAMS with the params taken from the openvas executable."""
         param_list = Openvas.get_settings()
 
-        for elem in param_list:
+        for elem in param_list:  # pylint: disable=consider-using-dict-items
             if elem not in OSPD_PARAMS:
                 self.scan_only_params[elem] = param_list[elem]
             else:
@@ -1132,40 +1134,19 @@ def report_openvas_results(self, db: BaseDB, scan_id: str) -> bool:
 
         return len(res_list) > 0
 
-    def is_openvas_process_alive(
-        self, kbdb: BaseDB, ovas_pid: str, scan_id: str
-    ) -> bool:
-        parent_exists = True
-        parent = None
-        try:
-            parent = psutil.Process(int(ovas_pid))
-        except psutil.NoSuchProcess:
-            logger.debug('Process with pid %s already stopped', ovas_pid)
-            parent_exists = False
-        except TypeError:
-            logger.debug(
-                'Scan with ID %s never started and stopped unexpectedly',
-                scan_id,
-            )
-            parent_exists = False
-
-        is_zombie = False
-        if parent and parent.status() == psutil.STATUS_ZOMBIE:
-            logger.debug(
-                ' %s: OpenVAS process is a zombie process',
-                scan_id,
-            )
-            is_zombie = True
-
-        if (not parent_exists or is_zombie) and kbdb:
-            if kbdb and kbdb.scan_is_stopped(scan_id):
-                return True
-            return False
-
-        return True
-
-    def stop_scan_cleanup(  # pylint: disable=arguments-differ
-        self, scan_id: str
+    @staticmethod
+    def is_openvas_process_alive(openvas_process: psutil.Popen) -> bool:
+
+        if openvas_process.status() == psutil.STATUS_ZOMBIE:
+            logger.debug("Process is a Zombie, waiting for it to clean up")
+            openvas_process.wait()
+        return openvas_process.is_running()
+
+    def stop_scan_cleanup(
+        self,
+        kbdb: BaseDB,
+        scan_id: str,
+        ovas_process: psutil.Popen,  # pylint: disable=arguments-differ
     ):
         """Set a key in redis to indicate the wrapper is stopped.
         It is done through redis because it is a new multiprocess
@@ -1175,42 +1156,49 @@ def stop_scan_cleanup(  # pylint: disable=arguments-differ
         via an invocation of openvas with the --scan-stop option to
         stop it."""
 
-        kbdb = self.main_db.find_kb_database_by_scan_id(scan_id)
         if kbdb:
+            # Set stop flag in redis
             kbdb.stop_scan(scan_id)
-            ovas_pid = kbdb.get_scan_process_id()
 
-            parent = None
-            try:
-                parent = psutil.Process(int(ovas_pid))
-            except psutil.NoSuchProcess:
-                logger.debug('Process with pid %s already stopped', ovas_pid)
-            except TypeError:
+            # Check if openvas is running
+            if ovas_process.is_running():
+                # Cleaning in case of Zombie Process
+                if ovas_process.status() == psutil.STATUS_ZOMBIE:
+                    logger.debug(
+                        '%s: Process with PID %s is a Zombie process.'
+                        ' Cleaning up...',
+                        scan_id,
+                        ovas_process.pid,
+                    )
+                    ovas_process.wait()
+                # Stop openvas process and wait until it stopped
+                else:
+                    can_stop_scan = Openvas.stop_scan(
+                        scan_id,
+                        not self.is_running_as_root and self.sudo_available,
+                    )
+                    if not can_stop_scan:
+                        logger.debug(
+                            'Not possible to stop scan process: %s.',
+                            ovas_process,
+                        )
+                        return
+
+                    logger.debug('Stopping process: %s', ovas_process)
+
+                    while ovas_process.is_running():
+                        if ovas_process.status() == psutil.STATUS_ZOMBIE:
+                            ovas_process.wait()
+                        else:
+                            time.sleep(0.1)
+            else:
                 logger.debug(
-                    'Scan with ID %s never started and stopped unexpectedly',
-                    scan_id,
-                )
-
-            if parent:
-                can_stop_scan = Openvas.stop_scan(
+                    "%s: Process with PID %s already stopped",
                     scan_id,
-                    not self.is_running_as_root and self.sudo_available,
+                    ovas_process.pid,
                 )
-                if not can_stop_scan:
-                    logger.debug(
-                        'Not possible to stop scan process: %s.',
-                        parent,
-                    )
-                    return False
-
-                logger.debug('Stopping process: %s', parent)
-
-                while parent:
-                    if parent.is_running():
-                        time.sleep(0.1)
-                    else:
-                        parent = None
 
+            # Clean redis db
             for scan_db in kbdb.get_scan_databases():
                 self.main_db.release_database(scan_db)
 
@@ -1285,25 +1273,24 @@ def exec_scan(self, scan_id: str):
             self.main_db.release_database(kbdb)
             return
 
-        result = Openvas.start_scan(
+        openvas_process = Openvas.start_scan(
             scan_id,
             not self.is_running_as_root and self.sudo_available,
             self._niceness,
         )
 
-        if result is None:
+        if openvas_process is None:
             self.main_db.release_database(kbdb)
             return
 
-        ovas_pid = result.pid
-        kbdb.add_scan_process_id(ovas_pid)
-        logger.debug('pid = %s', ovas_pid)
+        kbdb.add_scan_process_id(openvas_process.pid)
+        logger.debug('pid = %s', openvas_process.pid)
 
         # Wait until the scanner starts and loads all the preferences.
         while kbdb.get_status(scan_id) == 'new':
-            res = result.poll()
+            res = openvas_process.poll()
             if res and res < 0:
-                self.stop_scan_cleanup(scan_id)
+                self.stop_scan_cleanup(kbdb, scan_id, openvas_process)
                 logger.error(
                     'It was not possible run the task %s, since openvas ended '
                     'unexpectedly with errors during launching.',
@@ -1315,11 +1302,36 @@ def exec_scan(self, scan_id: str):
 
         got_results = False
         while True:
-            target_is_finished = kbdb.target_is_finished(scan_id)
+
             openvas_process_is_alive = self.is_openvas_process_alive(
-                kbdb, ovas_pid, scan_id
+                openvas_process
             )
-            if not target_is_finished and not openvas_process_is_alive:
+            target_is_finished = kbdb.target_is_finished(scan_id)
+            scan_stopped = self.get_scan_status(scan_id) == ScanStatus.STOPPED
+
+            # Report new Results and update status
+            got_results = self.report_openvas_results(kbdb, scan_id)
+            self.report_openvas_scan_status(kbdb, scan_id)
+
+            # Check if the client stopped the whole scan
+            if scan_stopped:
+                logger.debug('%s: Scan stopped by the client', scan_id)
+
+                self.stop_scan_cleanup(kbdb, scan_id, openvas_process)
+
+                # clean main_db, but wait for scanner to finish.
+                while not kbdb.target_is_finished(scan_id):
+                    logger.debug('%s: Waiting for openvas to finish', scan_id)
+                    time.sleep(1)
+                self.main_db.release_database(kbdb)
+                return
+
+            # Scan end. No kb in use for this scan id
+            if target_is_finished:
+                logger.debug('%s: Target is finished', scan_id)
+                break
+
+            if not openvas_process_is_alive:
                 logger.error(
                     'Task %s was unexpectedly stopped or killed.',
                     scan_id,
@@ -1350,25 +1362,6 @@ def exec_scan(self, scan_id: str):
                 time.sleep(0.05)
             got_results = False
 
-            # Check if the client stopped the whole scan
-            if kbdb.scan_is_stopped(scan_id):
-                logger.debug('%s: Scan stopped by the client', scan_id)
-
-                # clean main_db, but wait for scanner to finish.
-                while not kbdb.target_is_finished(scan_id):
-                    logger.debug('%s: Waiting the scan to finish', scan_id)
-                    time.sleep(1)
-                self.main_db.release_database(kbdb)
-                return
-
-            got_results = self.report_openvas_results(kbdb, scan_id)
-            self.report_openvas_scan_status(kbdb, scan_id)
-
-            # Scan end. No kb in use for this scan id
-            if kbdb.target_is_finished(scan_id):
-                logger.debug('%s: Target is finished', scan_id)
-                break
-
         # Delete keys from KB related to this scan task.
         logger.debug('%s: End Target. Release main database', scan_id)
         self.main_db.release_database(kbdb)

ospd_openvas/db.py

@@ -543,7 +543,7 @@ def target_is_finished(self, scan_id: str) -> bool:
         status = self._get_single_item('internal/{}'.format(scan_id))
 
         if status is None:
-            logger.info(
+            logger.error(
                 "%s: Target set as finished because redis returned None as "
                 "scanner status.",
                 scan_id,

ospd_openvas/openvas.py

@@ -16,11 +16,12 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 
+from typing import Optional, Dict, Any
 
 import logging
 import subprocess
+import psutil
 
-from typing import Optional, Dict, Any
 
 logger = logging.getLogger(__name__)
 
@@ -136,8 +137,10 @@ def load_vts_into_redis():
 
     @staticmethod
     def start_scan(
-        scan_id: str, sudo: bool = False, niceness: int = None
-    ) -> Optional[subprocess.Popen]:
+        scan_id: str,
+        sudo: bool = False,
+        niceness: int = None,
+    ) -> Optional[psutil.Popen]:
         """Calls openvas to start a scan process"""
         cmd = []
 
@@ -151,8 +154,8 @@ def start_scan(
         cmd += ['openvas', '--scan-start', scan_id]
 
         try:
-            return subprocess.Popen(cmd, shell=False)
-        except (subprocess.SubprocessError, OSError) as e:
+            return psutil.Popen(cmd, shell=False)
+        except (psutil.Error, OSError, FileNotFoundError) as e:
             # the command is not available
             logger.warning("Could not start scan process. Reason %s", e)
             return None