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

Niceness #109

Merged
merged 10 commits into from
Jun 3, 2019
16 changes: 15 additions & 1 deletion ospd/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
from enum import Enum

LOGGER = logging.getLogger(__name__)
NICENESS = 10

# Default file locations as used by a OpenVAS default installation
KEY_FILE = "/usr/var/lib/gvm/private/CA/serverkey.pem"
Expand Down Expand Up @@ -808,6 +809,15 @@ def network_port(string):
'port must be in ]0,65535] interval')
return value

def niceness(string):
""" Check if provided string is a valid niceness value. """
try:
value = int(string)
except ValueError:
raise argparse.ArgumentTypeError(
'niceness must be an integer')
return value

def cacert_file(cacert):
""" Check if provided file is a valid CA Certificate """
try:
Expand Down Expand Up @@ -868,6 +878,9 @@ def filename(string):
help='Path to the logging file.')
parser.add_argument('--version', action='store_true',
help='Print version then exit.')
parser.add_argument('--niceness', default=NICENESS, type=niceness,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason for defining an own type instead using the default int type https://docs.python.org/3/library/argparse.html#type ?

help='Start the scan with the given niceness. Default 10')
jjnicola marked this conversation as resolved.
Show resolved Hide resolved

return parser


Expand Down Expand Up @@ -920,6 +933,7 @@ def get_common_args(parser, args=None):
common_args['foreground'] = options.foreground
common_args['log_file'] = options.log_file
common_args['version'] = options.version
common_args['niceness'] = options.niceness

return common_args

Expand Down Expand Up @@ -952,7 +966,7 @@ def main(name, klass):
cargs = get_common_args(parser)
logging.getLogger().setLevel(cargs['log_level'])
wrapper = klass(certfile=cargs['certfile'], keyfile=cargs['keyfile'],
cafile=cargs['cafile'])
cafile=cargs['cafile'], niceness=cargs['niceness'])

if cargs['version']:
print_version(wrapper)
Expand Down
2 changes: 1 addition & 1 deletion ospd/ospd.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ class OSPDaemon(object):
specific options eg. the w3af profile for w3af wrapper.
"""

def __init__(self, certfile, keyfile, cafile,
def __init__(self, certfile, keyfile, cafile, niceness,
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 would make niceness optional

Suggested change
def __init__(self, certfile, keyfile, cafile, niceness,
def __init__(self, certfile, keyfile, cafile, niceness=None,

customvtfilter=None, wrapper_logger=None):
""" Initializes the daemon's internal data. """
# @todo: Actually it makes sense to move the certificate params to
Expand Down
7 changes: 5 additions & 2 deletions ospd/ospd_ssh.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,13 @@ class OSPDaemonSimpleSSH(OSPDaemon):
an array.
"""

def __init__(self, certfile, keyfile, cafile):
def __init__(self, certfile, keyfile, cafile, niceness):
Copy link
Contributor

Choose a reason for hiding this comment

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

Just this single change is missing to make niceness optional here too

Suggested change
def __init__(self, certfile, keyfile, cafile, niceness):
def __init__(self, certfile, keyfile, cafile, niceness=None):

""" Initializes the daemon and add parameters needed to remote SSH execution. """

super(OSPDaemonSimpleSSH, self).__init__(certfile=certfile, keyfile=keyfile,
cafile=cafile)
cafile=cafile, niceness=niceness)

self.NICENESS = niceness
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why self.NICENESS is all uppercase here? I would mark it as a private variable by using

Suggested change
self.NICENESS = niceness
self._niceness = niceness


if paramiko is None:
raise ImportError('paramiko needs to be installed in order to use'
Expand Down Expand Up @@ -140,6 +142,7 @@ def run_command(self, scan_id, host, cmd):
self.add_scan_error(scan_id, host=host, value=str(err))
return None

cmd = "nice -n %s %s" % (self.NICENESS, cmd)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
cmd = "nice -n %s %s" % (self.NICENESS, cmd)
if self.NICENESS is not None:
cmd = "nice -n %s %s" % (self.NICENESS, cmd)

_, stdout, _ = ssh.exec_command(cmd)
result = stdout.readlines()
ssh.close()
Expand Down
16 changes: 8 additions & 8 deletions tests/testSSHDaemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,31 +64,31 @@ class TestSSH(unittest.TestCase):

def testNoParamiko(self):
ospd_ssh.paramiko = None
self.assertRaises(ImportError, OSPDaemonSimpleSSH, 'cert', 'key', 'ca')
self.assertRaises(ImportError, OSPDaemonSimpleSSH, 'cert', 'key', 'ca', '10')

def testRunCommand(self):
ospd_ssh.paramiko = fakeparamiko
daemon = OSPDaemonSimpleSSH('cert', 'key', 'ca')
daemon = OSPDaemonSimpleSSH('cert', 'key', 'ca', '10')
scanid = daemon.create_scan(None, [['host.example.com', '80, 443', '', ''],],
dict(port=5, ssh_timeout=15,
username_password='dummy:pw'), '')
res = daemon.run_command(scanid, 'host.example.com', 'cat /etc/passwd')
self.assertTrue(isinstance(res, list))
self.assertEqual(commands, ['cat /etc/passwd'])
self.assertEqual(commands, ['nice -n 10 cat /etc/passwd'])

def testRunCommandLegacyCredential(self):
ospd_ssh.paramiko = fakeparamiko
daemon = OSPDaemonSimpleSSH('cert', 'key', 'ca')
daemon = OSPDaemonSimpleSSH('cert', 'key', 'ca', '10')
scanid = daemon.create_scan(None, [['host.example.com', '80, 443', '', ''],],
dict(port=5, ssh_timeout=15,
username='dummy', password='pw'), '')
res = daemon.run_command(scanid, 'host.example.com', 'cat /etc/passwd')
self.assertTrue(isinstance(res, list))
self.assertEqual(commands, ['cat /etc/passwd'])
self.assertEqual(commands, ['nice -n 10 cat /etc/passwd'])

def testRunCommandNewCredential(self):
ospd_ssh.paramiko = fakeparamiko
daemon = OSPDaemonSimpleSSH('cert', 'key', 'ca')
daemon = OSPDaemonSimpleSSH('cert', 'key', 'ca', '10')

cred_dict = {'ssh': {'type': 'up',
'password': 'mypass',
Expand All @@ -103,11 +103,11 @@ def testRunCommandNewCredential(self):
dict(port=5, ssh_timeout=15), '')
res = daemon.run_command(scanid, 'host.example.com', 'cat /etc/passwd')
self.assertTrue(isinstance(res, list))
self.assertEqual(commands, ['cat /etc/passwd'])
self.assertEqual(commands, ['nice -n 10 cat /etc/passwd'])

def testRunCommandNoCredential(self):
ospd_ssh.paramiko = fakeparamiko
daemon = OSPDaemonSimpleSSH('cert', 'key', 'ca')
daemon = OSPDaemonSimpleSSH('cert', 'key', 'ca', '10')
scanid = daemon.create_scan(None, [['host.example.com', '80, 443', '', ''],],
dict(port=5, ssh_timeout=15), '')
self.assertRaises(ValueError, daemon.run_command,
Expand Down
2 changes: 1 addition & 1 deletion tests/testScanAndResult.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def __init__(self, type_, **kwargs):

class DummyWrapper(OSPDaemon):
def __init__(self, results, checkresult=True):
OSPDaemon.__init__(self, 'cert', 'key', 'ca')
OSPDaemon.__init__(self, 'cert', 'key', 'ca', '10')
self.checkresult = checkresult
self.results = results

Expand Down