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

Add new interface for adding alive test methods #329

Merged
merged 7 commits into from
Oct 14, 2020
Merged

Add new interface for adding alive test methods #329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2927cc7
Add new interface for adding alive test methods
ArnoStiefvater Aug 28, 2020
2a24d95
Add documentation for alive_test_methods element
ArnoStiefvater Oct 12, 2020
a084038
Add changelog entry
ArnoStiefvater Oct 13, 2020
384756c
Fix formatting.
ArnoStiefvater Oct 13, 2020
4a5305b
Add changelog entry to protocol description
ArnoStiefvater Oct 13, 2020
f22f3f5
Do not add empty option to options dict
ArnoStiefvater Oct 14, 2020
eaa4f32
Add tests for supplying alive test via elements
ArnoStiefvater Oct 14, 2020
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

### Added
- Add target option for supplying dedicated port list for alive detection (Boreas only) via OSP. [#323](https://github.com/greenbone/ospd/pull/323)
- Add target option for supplying alive test methods via separate elements. [#329](https://github.com/greenbone/ospd/pull/329)

### Removed
- Remove python3.5 support and deprecated methods. [#316](https://github.com/greenbone/ospd/pull/316)
Expand Down
77 changes: 66 additions & 11 deletions doc/OSP.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,11 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
<e>credentials</e>
<e>exclude_hosts</e>
<e>finished_hosts</e>
<e>alive_test</e>
<e>alive_test_ports</e>
<or>
<e>alive_test</e>
<e>alive_test_methods</e>
</or>
<e>reverse_lookup_unify</e>
<e>reverse_lookup_only</e>
</pattern>
Expand All @@ -196,45 +199,89 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
</ele>
<ele>
<name>credentials</name>
<summary>One or many credentials containing the credential for the given hosts.</summary>
<summary>One or many credentials containing the credential for the given hosts</summary>
<pattern>
<e>credential</e>
</pattern>
</ele>
<ele>
<name>exclude_hosts</name>
<summary>One or many hosts to exclude. The list is comma-separated. Each entry can be a IP address, a CIDR notation, a hostname, a IP range. IPs can be v4 or v6. Each wrapper must handle the exclude hosts.
<summary>One or many hosts to exclude. The list is comma-separated. Each entry can be a IP address, a CIDR notation, a hostname, a IP range. IPs can be v4 or v6. Each wrapper must handle the exclude hosts
</summary>
<type>string</type>
</ele>
<ele>
<name>finished_hosts</name>
<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>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>
<ele>
<name>alive_test</name>
<summary>Alive test type to be performed against the target.</summary>
<type>string</type>
<summary>Alive test type to be performed against the target</summary>
<type>integer</type>
</ele>
<ele>
<name>alive_test_methods</name>
<summary>Alive test methods to be performed against the target</summary>
<pattern>
<e>icmp</e>
</pattern>
<ele>
<name>icmp</name>
<summary>ICMP ping</summary>
<type>boolean</type>
</ele>
<pattern>
<e>tcp_syn</e>
</pattern>
<ele>
<name>tcp_syn</name>
<summary>TCP-SYN ping</summary>
<type>boolean</type>
</ele>
<pattern>
<e>tcp_ack</e>
</pattern>
<ele>
<name>tcp_ack</name>
<summary>TCP-ACK ping</summary>
<type>boolean</type>
</ele>
<pattern>
<e>arp</e>
</pattern>
<ele>
<name>arp</name>
<summary>ARP ping</summary>
<type>boolean</type>
</ele>
<pattern>
<e>consider_alive</e>
</pattern>
<ele>
<name>consider_alive</name>
<summary>Consider the target to be alive</summary>
<type>boolean</type>
</ele>
</ele>
<ele>
<name>alive_test_ports</name>
<summary>Dedicated port list for alive detection. Used for TCP-SYN and TCP-ACK ping when Boreas (scanner preference test_alive_hosts_only) is enabled. If no port list is provided ports 80, 137, 587, 3128, 8081 are used as defaults.</summary>
<summary>Dedicated port list for alive detection. Used for TCP-SYN and TCP-ACK ping when Boreas (scanner preference test_alive_hosts_only) is enabled. If no port list is provided ports 80, 137, 587, 3128, 8081 are used as defaults</summary>
<type>string</type>
</ele>
<ele>
<name>reverse_lookup_only</name>
<summary>Only scan IP addresses that can be resolved into a DNS name.</summary>
<summary>Only scan IP addresses that can be resolved into a DNS name</summary>
<type>string</type>
</ele>
<ele>
<name>reverse_lookup_unify</name>
<summary>If multiple IP addresses resolve to the same DNS name the DNS name will only get scanned once.</summary>
<summary>If multiple IP addresses resolve to the same DNS name the DNS name will only get scanned once</summary>
<type>string</type>
</ele>
<example>
<summary>Target without credentials.</summary>
<summary>Target without credentials</summary>
<e>
<target>
<hosts>example.org</hosts>
Expand All @@ -247,7 +294,7 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
</e>
</example>
<example>
<summary>Target with two credentials.</summary>
<summary>Target with two credentials</summary>
<e>
<target>
<hosts>192.168.1.0/24</hosts>
Expand Down Expand Up @@ -1486,6 +1533,14 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
<version>21.4</version>
</change>

<change>
<command>START_SCAN</command>
<summary>Add support for supplying alive test methods via separate elements.</summary>
<description>Target element received new optional target option alive_test_methods with subelements imcp, tcp_ack, tcp_syn, arp and consider_alive.
</description>
<version>21.4</version>
</change>

<change>
<command>GET_VTS</command>
<summary>Returned object extended with solution method</summary>
Expand Down
36 changes: 36 additions & 0 deletions ospd/protocol.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,39 @@ def process_credentials_elements(cred_tree: Element) -> Dict:

return credentials

@staticmethod
def process_alive_test_methods(
alive_test_tree: Element, options: Dict
) -> None:
"""Receive an XML object with the alive test methods to run
a scan with. Methods are added to the options Dict.

@param
<alive_test_methods>
</icmp>boolean(1 or 0)</icmp>
</tcp_ack>boolean(1 or 0)</tcp_ack>
</tcp_syn>boolean(1 or 0)</tcp_syn>
</arp>boolean(1 or 0)</arp>
</consider_alive>boolean(1 or 0)</consider_alive>
</alive_test_methods>
"""
for child in alive_test_tree:
if child.tag == 'icmp':
Copy link
Member

Choose a reason for hiding this comment

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

Are these methods optional? shouldn't be added only if they are set? Not sure if it make sense to add and empty element to the options dict.

Copy link
Member Author

Choose a reason for hiding this comment

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

All the options (imcp, tcp_ack ...) are optional. For example if you do not want icmp then you can either leave it out completely, set it to '0' or leave it empty. Everything not '1' is treated as not wanted.

Should I check if child.text is None and not include it in the options dict?

Just an additional remark. If <alive_test> and <alive_test_methods> is provided, <alive_test> takes precedence over <alive_test_methods>.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have now added a test for None. So if there is no text it will not be included options dict.
I also changed the argument description in the function.

if child.text is not None:
options['icmp'] = child.text
if child.tag == 'tcp_ack':
if child.text is not None:
options['tcp_ack'] = child.text
if child.tag == 'tcp_syn':
if child.text is not None:
options['tcp_syn'] = child.text
if child.tag == 'arp':
if child.text is not None:
options['arp'] = child.text
if child.tag == 'consider_alive':
if child.text is not None:
options['consider_alive'] = child.text

@classmethod
def process_target_element(cls, scanner_target: Element) -> Dict:
"""Receive an XML object with the target, ports and credentials to run
Expand Down Expand Up @@ -222,6 +255,9 @@ def process_target_element(cls, scanner_target: Element) -> Dict:
ports = child.text
if child.tag == 'credentials':
credentials = cls.process_credentials_elements(child)
if child.tag == 'alive_test_methods':
options['alive_test_methods'] = '1'
cls.process_alive_test_methods(child, options)
if child.tag == 'alive_test':
options['alive_test'] = child.text
if child.tag == 'alive_test_ports':
Expand Down
91 changes: 86 additions & 5 deletions tests/test_scan_and_result.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ def test_get_vts_filter_negative(self):
)
fs = FakeStream()
self.daemon.handle_command(
'<get_vts filter="modification_time&lt;19000203"></get_vts>', fs,
'<get_vts filter="modification_time&lt;19000203"></get_vts>',
fs,
)
response = fs.get_response()

Expand Down Expand Up @@ -741,7 +742,8 @@ def test_get_scan_results_clean(self):

fs = FakeStream()
self.daemon.handle_command(
'<get_scans scan_id="%s" pop_results="1"/>' % scan_id, fs,
'<get_scans scan_id="%s" pop_results="1"/>' % scan_id,
fs,
)

res_len = len(
Expand Down Expand Up @@ -771,7 +773,8 @@ def test_get_scan_results_restore(self):

fs = FakeStream(return_value=False)
self.daemon.handle_command(
'<get_scans scan_id="%s" pop_results="1"/>' % scan_id, fs,
'<get_scans scan_id="%s" pop_results="1"/>' % scan_id,
fs,
)

res_len = len(
Expand Down Expand Up @@ -893,6 +896,82 @@ def test_scan_get_target_options(self):
target_options = self.daemon.get_scan_target_options(scan_id)
self.assertEqual(target_options, {'alive_test': '0'})

def test_scan_get_target_options_alive_test_methods(self):
fs = FakeStream()
self.daemon.handle_command(
'<start_scan>'
'<scanner_params /><vts><vt id="1.2.3.4" />'
'</vts>'
'<targets>'
'<target><hosts>192.168.0.1</hosts>'
'<ports>22</ports>'
'<alive_test_methods>'
'<icmp>1</icmp>'
'<tcp_syn>1</tcp_syn>'
'<tcp_ack>1</tcp_ack>'
'<arp>1</arp>'
'<consider_alive>1</consider_alive>'
'</alive_test_methods>'
'</target>'
'</targets>'
'</start_scan>',
fs,
)
self.daemon.start_queued_scans()

response = fs.get_response()

scan_id = response.findtext('id')
time.sleep(1)
target_options = self.daemon.get_scan_target_options(scan_id)
self.assertEqual(
target_options,
{
'alive_test_methods': '1',
'icmp': '1',
'tcp_syn': '1',
'tcp_ack': '1',
'arp': '1',
'consider_alive': '1',
},
)

def test_scan_get_target_options_alive_test_methods_dont_add_empty_or_missing( # pylint: disable=line-too-long
self,
):
fs = FakeStream()
self.daemon.handle_command(
'<start_scan>'
'<scanner_params /><vts><vt id="1.2.3.4" />'
'</vts>'
'<targets>'
'<target><hosts>192.168.0.1</hosts>'
'<ports>22</ports>'
'<alive_test_methods>'
'<icmp>1</icmp>'
'<arp></arp>'
'<consider_alive></consider_alive>'
'</alive_test_methods>'
'</target>'
'</targets>'
'</start_scan>',
fs,
)
self.daemon.start_queued_scans()

response = fs.get_response()

scan_id = response.findtext('id')
time.sleep(1)
target_options = self.daemon.get_scan_target_options(scan_id)
self.assertEqual(
target_options,
{
'alive_test_methods': '1',
'icmp': '1',
},
)

def test_progress(self):

fs = FakeStream()
Expand Down Expand Up @@ -1087,7 +1166,8 @@ def test_get_scan_progress_xml(self):

fs = FakeStream()
self.daemon.handle_command(
'<get_scans scan_id="%s" details="0" progress="1"/>' % scan_id, fs,
'<get_scans scan_id="%s" details="0" progress="1"/>' % scan_id,
fs,
)
response = fs.get_response()

Expand Down Expand Up @@ -1164,7 +1244,8 @@ def test_scan_exists(self, mock_create_process, _mock_os):
)

self.daemon.handle_command(
cmd, fs,
cmd,
fs,
)
self.daemon.start_queued_scans()

Expand Down