Skip to content
This repository has been archived by the owner on Nov 29, 2021. It is now read-only.

Improve handling of dead hosts and improve scan progress calculation #266

Merged
merged 15 commits into from
May 11, 2020
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
32 changes: 31 additions & 1 deletion doc/OSP.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
12 changes: 10 additions & 2 deletions ospd/command/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ 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
Expand All @@ -424,10 +425,17 @@ def handle_xml(self, xml: Element) -> bytes:
if max_res:
max_res = int(max_res)

if progress and progress == '1':
bjoernricks marked this conversation as resolved.
Show resolved Hide resolved
progress = True
else:
progress = False

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)
Expand All @@ -436,7 +444,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)

Expand Down
115 changes: 60 additions & 55 deletions ospd/ospd.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
from ospd.xml import (
elements_as_text,
get_result_xml,
get_progress_xml,
get_elements_from_dict,
)

Expand Down Expand Up @@ -410,7 +411,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, 100)
self.set_scan_status(scan_id, ScanStatus.FINISHED)
logger.info("%s: Scan finished.", scan_id)

Expand Down Expand Up @@ -505,29 +506,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. """
Expand All @@ -538,7 +524,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:
Expand Down Expand Up @@ -584,35 +569,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 = []

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)
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 == 100:
alive_hosts.append(finished_host)
if progress == -1:
dead_hosts.append(finished_host)
bjoernricks marked this conversation as resolved.
Show resolved Hide resolved

self.scan_collection.set_host_dead(scan_id, dead_hosts)

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,
Expand All @@ -621,9 +613,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)

Expand Down Expand Up @@ -711,6 +700,32 @@ def get_scan_results_xml(
logger.debug('Returning %d results', len(results))
return results

def get_scan_progress_xml(self, scan_id: str):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I don't think this method should be in the daemon class but currently i can't find a better place at the moment. XML creation should not be part of the daemon class at all. Maybe just mark it as private for now by changing the name to _get_scan_progress_xml

""" 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.",
Expand All @@ -730,6 +745,7 @@ def get_scan_xml(
detailed: bool = True,
pop_res: bool = False,
max_res: int = 0,
progress: bool = False,
):
""" Gets scan in XML format.

Expand Down Expand Up @@ -757,6 +773,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
Expand Down Expand Up @@ -1178,23 +1197,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
)
Expand Down Expand Up @@ -1281,14 +1294,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)
Expand Down
Loading