From afc61553ebeddf0e2527d2d1fdf0c97a8957e962 Mon Sep 17 00:00:00 2001 From: Juan Jose Nicola Date: Wed, 6 May 2020 13:06:45 +0200 Subject: [PATCH 1/8] Do not send log and host details message of dead hosts to the client. --- ospd_openvas/daemon.py | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/ospd_openvas/daemon.py b/ospd_openvas/daemon.py index 953ceedc..fe30cccd 100644 --- a/ospd_openvas/daemon.py +++ b/ospd_openvas/daemon.py @@ -927,7 +927,7 @@ def report_openvas_results( qod=rqod, ) - # To process non scanned dead hosts when + # To process non-scanned dead hosts when # test_alive_host_only in openvas is enable if msg[0] == 'DEADHOST': hosts = msg[3].split(',') @@ -935,22 +935,6 @@ def report_openvas_results( if _host: host_progress_batch[_host] = 100 finished_host_batch.append(_host) - res_list.add_scan_log_to_list( - host=_host, - hostname=rhostname, - name=rname, - value=msg[4], - port=msg[2], - qod=rqod, - test_id='', - ) - timestamp = time.ctime(time.time()) - res_list.add_scan_log_to_list( - host=_host, name='HOST_START', value=timestamp, - ) - res_list.add_scan_log_to_list( - host=_host, name='HOST_END', value=timestamp, - ) res = db.get_result() From 92ec710e282d126dc862e1d70b0785e1aa12fd65 Mon Sep 17 00:00:00 2001 From: Juan Jose Nicola Date: Fri, 8 May 2020 17:32:44 +0200 Subject: [PATCH 2/8] The dead host are not taken in account for the scan progress calculation Therefore the host progress is marked as -1 --- ospd_openvas/daemon.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ospd_openvas/daemon.py b/ospd_openvas/daemon.py index fe30cccd..9e2362e3 100644 --- a/ospd_openvas/daemon.py +++ b/ospd_openvas/daemon.py @@ -809,12 +809,14 @@ def update_progress(self, scan_id: str, current_host: str, msg: str): launched, total = msg.split('/') except ValueError: return + if float(total) == 0: return elif float(total) == -1: - host_prog = 100 + host_prog = -1 # Host dead else: host_prog = (float(launched) / float(total)) * 100 + self.set_scan_host_progress( scan_id, host=current_host, progress=host_prog ) From fd387eb682bf714fcbc823e9137b274bb3a3179a Mon Sep 17 00:00:00 2001 From: Juan Jose Nicola Date: Fri, 8 May 2020 17:34:07 +0200 Subject: [PATCH 3/8] Amount of non scanned dead hosts are received as a number instead of single host list. This reduce the amount of memory used for storing the dead hosts and the processing time. No messages HOST_START, dead host, HOST_END are sent to the client if the dead hosts are handle in batch. This is the method use by Boreas method in OpenVAS. --- ospd_openvas/daemon.py | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/ospd_openvas/daemon.py b/ospd_openvas/daemon.py index 9e2362e3..8ee0573a 100644 --- a/ospd_openvas/daemon.py +++ b/ospd_openvas/daemon.py @@ -861,8 +861,7 @@ def report_openvas_results( res = db.get_result() res_list = ResultList() - host_progress_batch = dict() - finished_host_batch = list() + total_dead = 0 while res: msg = res.split('|||') roid = msg[3].strip() @@ -932,26 +931,19 @@ def report_openvas_results( # To process non-scanned dead hosts when # test_alive_host_only in openvas is enable if msg[0] == 'DEADHOST': - hosts = msg[3].split(',') - for _host in hosts: - if _host: - host_progress_batch[_host] = 100 - finished_host_batch.append(_host) - + try: + total_dead = int(msg[4]) + except TypeError: + logger.debug('Error processing dead host count') res = db.get_result() # Insert result batch into the scan collection table. if len(res_list): self.scan_collection.add_result_list(scan_id, res_list) - if host_progress_batch: - self.set_scan_progress_batch( - scan_id, host_progress=host_progress_batch - ) - - if finished_host_batch: - self.set_scan_host_finished( - scan_id, finished_hosts=finished_host_batch + if total_dead: + self.scan_collection.set_amount_dead_hosts( + scan_id, total_dead=total_dead ) def report_openvas_timestamp_scan_host( From b4e5db24ca5aa5346ddd747e7d299567c8f24145 Mon Sep 17 00:00:00 2001 From: Juan Jose Nicola Date: Fri, 8 May 2020 17:38:28 +0200 Subject: [PATCH 4/8] Use the new method sort_finished_host() instead of set_scan_host_finished() A finished host at this point could be alive or dead. Depending on the host progress it will be set as dead or alive(finished). --- ospd_openvas/daemon.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/ospd_openvas/daemon.py b/ospd_openvas/daemon.py index 8ee0573a..8ed4cd8e 100644 --- a/ospd_openvas/daemon.py +++ b/ospd_openvas/daemon.py @@ -1163,16 +1163,18 @@ def exec_scan(self, scan_id: str): ) if scan_db.host_is_finished(openvas_scan_id): - self.set_scan_host_finished( - scan_id, finished_hosts=current_host - ) self.report_openvas_scan_status( scan_db, scan_id, current_host ) + self.report_openvas_timestamp_scan_host( scan_db, scan_id, current_host ) + self.sort_host_finished( + scan_id, finished_hosts=current_host + ) + kbdb.remove_scan_database(scan_db) self.main_db.release_database(scan_db) From 4fac30a816ca4182179f8992f843ff62061e11e7 Mon Sep 17 00:00:00 2001 From: Juan Jose Nicola Date: Fri, 8 May 2020 17:40:32 +0200 Subject: [PATCH 5/8] Use the list of excluded host sent by the client. Finished hosts are only for progress calculation. Only the host in the exclude host list will not be scanned. When resuming a stopped task, the client should sent in the exclude host list, the list of host to exclude, and the list of finished host, if they must not be scanned again. --- ospd_openvas/preferencehandler.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/ospd_openvas/preferencehandler.py b/ospd_openvas/preferencehandler.py index b5d967fb..2a4b9a1f 100644 --- a/ospd_openvas/preferencehandler.py +++ b/ospd_openvas/preferencehandler.py @@ -451,17 +451,6 @@ def prepare_host_options_for_openvas(self): stores the list of hosts that must not be scanned in the kb. """ exclude_hosts = self.scan_collection.get_exclude_hosts(self.scan_id) - # Get unfinished hosts, in case it is a resumed scan. And added - # into exclude_hosts scan preference. Set progress for the finished ones - # to 100%. - finished_hosts = self.scan_collection.get_hosts_finished(self.scan_id) - if finished_hosts: - if exclude_hosts: - finished_hosts_str = ','.join(finished_hosts) - exclude_hosts = exclude_hosts + ',' + finished_hosts_str - else: - exclude_hosts = ','.join(finished_hosts) - if exclude_hosts: pref_val = "exclude_hosts|||" + exclude_hosts self.kbdb.add_scan_preferences(self._openvas_scan_id, [pref_val]) From a32f813648c3851149444e1c1c6cfa8151263ed5 Mon Sep 17 00:00:00 2001 From: Juan Jose Nicola Date: Fri, 8 May 2020 17:47:52 +0200 Subject: [PATCH 6/8] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3284c2e4..d914245d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,8 @@ 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. [#235](https://github.com/greenbone/ospd-openvas/pull/235) +- Don't send host details and log messages to the client when Boreas is enabled. [#252](https://github.com/greenbone/ospd-openvas/pull/252) +- Progress bar calculation do not takes in accound dead hosts. [#252](https://github.com/greenbone/ospd-openvas/pull/252) ### Fixed - Check vt_aux for None before trying to access it. [#177](https://github.com/greenbone/ospd-openvas/pull/177) @@ -43,6 +45,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Removed - Remove use_mac_addr, vhost_ip and vhost scan preferences. [#184](https://github.com/greenbone/ospd-openvas/pull/184) +- Handling of finished host for resume task. [#252](https://github.com/greenbone/ospd-openvas/pull/252) ## [1.0.1] (unreleased) From f30b98d61d4926d31cdea82d9708f7f40902d3d8 Mon Sep 17 00:00:00 2001 From: Juan Jose Nicola Date: Mon, 11 May 2020 13:31:38 +0200 Subject: [PATCH 7/8] Add/remove/adjust unittests --- tests/test_daemon.py | 20 +++++++++++++++- tests/test_preferencehandler.py | 42 --------------------------------- 2 files changed, 19 insertions(+), 43 deletions(-) diff --git a/tests/test_daemon.py b/tests/test_daemon.py index 29c6a74e..74f8b419 100644 --- a/tests/test_daemon.py +++ b/tests/test_daemon.py @@ -615,6 +615,24 @@ def test_get_openvas_result(self, mock_add_scan_log_to_list, MockDBClass): value='Host dead', ) + @patch('ospd_openvas.daemon.ScanDB') + def test_get_openvas_result_dead_hosts(self, MockDBClass): + w = DummyDaemon() + + target_element = w.create_xml_target() + targets = OspRequest.process_target_element(target_element) + w.create_scan('123-456', targets, None, []) + + results = ["DEADHOST||| ||| ||| |||4", None] + mock_db = MockDBClass.return_value + mock_db.get_result.side_effect = results + w.scan_collection.set_amount_dead_hosts = MagicMock() + + w.report_openvas_results(mock_db, '123-456', 'localhost') + w.scan_collection.set_amount_dead_hosts.assert_called_with( + '123-456', total_dead=4, + ) + @patch('ospd_openvas.db.KbDB') def test_openvas_is_alive_already_stopped(self, mock_db): w = DummyDaemon() @@ -647,7 +665,7 @@ def test_update_progress(self, mock_set_scan_host_progress): w.update_progress('123-456', 'localhost', msg) mock_set_scan_host_progress.assert_called_with( - '123-456', host='localhost', progress=100 + '123-456', host='localhost', progress=-1 ) diff --git a/tests/test_preferencehandler.py b/tests/test_preferencehandler.py index 7b5bda06..7bb8dacd 100644 --- a/tests/test_preferencehandler.py +++ b/tests/test_preferencehandler.py @@ -349,48 +349,8 @@ def test_set_host_options(self, mock_kb): w = DummyDaemon() exc = '192.168.0.1' - fin = ['192.168.0.2'] w.scan_collection.get_exclude_hosts = MagicMock(return_value=exc) - w.scan_collection.get_hosts_finished = MagicMock(return_value=fin) - - p = PreferenceHandler('1234-1234', mock_kb, w.scan_collection, None) - p._openvas_scan_id = '456-789' - p.kbdb.add_scan_preferences = MagicMock() - p.prepare_host_options_for_openvas() - - p.kbdb.add_scan_preferences.assert_called_with( - p._openvas_scan_id, ['exclude_hosts|||192.168.0.1,192.168.0.2'], - ) - - @patch('ospd_openvas.db.KbDB') - def test_set_host_options_no_exclude(self, mock_kb): - w = DummyDaemon() - - exc = '' - fin = ['192.168.0.2'] - - w.scan_collection.get_exclude_hosts = MagicMock(return_value=exc) - w.scan_collection.get_hosts_finished = MagicMock(return_value=fin) - - p = PreferenceHandler('1234-1234', mock_kb, w.scan_collection, None) - p._openvas_scan_id = '456-789' - p.kbdb.add_scan_preferences = MagicMock() - p.prepare_host_options_for_openvas() - - p.kbdb.add_scan_preferences.assert_called_with( - p._openvas_scan_id, ['exclude_hosts|||192.168.0.2'], - ) - - @patch('ospd_openvas.db.KbDB') - def test_set_host_options_no_finished(self, mock_kb): - w = DummyDaemon() - - exc = '192.168.0.1' - fin = [] - - w.scan_collection.get_exclude_hosts = MagicMock(return_value=exc) - w.scan_collection.get_hosts_finished = MagicMock(return_value=fin) p = PreferenceHandler('1234-1234', mock_kb, w.scan_collection, None) p._openvas_scan_id = '456-789' @@ -406,10 +366,8 @@ def test_set_host_options_none(self, mock_kb): w = DummyDaemon() exc = '' - fin = [] w.scan_collection.get_exclude_hosts = MagicMock(return_value=exc) - w.scan_collection.get_hosts_finished = MagicMock(return_value=fin) p = PreferenceHandler('1234-1234', mock_kb, w.scan_collection, None) p._openvas_scan_id = '456-789' From d6e857c2296fd36fe2272139e5d2fef40f206393 Mon Sep 17 00:00:00 2001 From: Juan Jose Nicola Date: Mon, 11 May 2020 15:32:06 +0200 Subject: [PATCH 8/8] Use constant for dead host progress. Also catch TypeError when cast the progress value to float. --- ospd_openvas/daemon.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/ospd_openvas/daemon.py b/ospd_openvas/daemon.py index 8ed4cd8e..715668bb 100644 --- a/ospd_openvas/daemon.py +++ b/ospd_openvas/daemon.py @@ -34,7 +34,7 @@ import psutil -from ospd.ospd import OSPDaemon +from ospd.ospd import OSPDaemon, PROGRESS_DEAD_HOST from ospd.server import BaseServer from ospd.main import main as daemon_main from ospd.cvss import CVSS @@ -810,12 +810,15 @@ def update_progress(self, scan_id: str, current_host: str, msg: str): except ValueError: return - if float(total) == 0: + try: + if float(total) == 0: + return + elif float(total) == PROGRESS_DEAD_HOST: + host_prog = PROGRESS_DEAD_HOST + else: + host_prog = (float(launched) / float(total)) * 100 + except TypeError: return - elif float(total) == -1: - host_prog = -1 # Host dead - else: - host_prog = (float(launched) / float(total)) * 100 self.set_scan_host_progress( scan_id, host=current_host, progress=host_prog