Check sudo permissions before call openvas. #87

jjnicola · 2019-06-25T15:46:10Z

Check if the user running ospd-openvas has permissions to call openvas
with sudo and without password.
Depending on it, it will run openvas with sudo or without it.

How to give the user permission to call openvas with sudo and without password:
$ sudo visudo
Add at the end of the file
<user> ALL = NOPASSWD: /<install/path/sbin/openvas

Check if the user running ospd-openvas has permissions to call openvas with sudo and without password. Depending on it, it will run openvas with sudo or without it.

bjoernricks

For sudo_available I would propose to use the property decorator https://docs.python.org/3/library/functions.html#property

It allows to use calling a method when a property on the class instance is accessed.

bjoernricks · 2019-06-26T06:07:42Z

ospd_openvas/wrapper.py

+        except subprocess.CalledProcessError as e:
+            logger.debug('It was not possible to call openvas with sudo. '
+                         'The scanner will run as non-root user. Reason %s', e)
+            return _sudo_available


Suggested change

return _sudo_available

self._sudo_available = False

bjoernricks · 2019-06-26T06:08:54Z

ospd_openvas/wrapper.py

+        try:
+            result = subprocess.check_call(
+                ['sudo', '-n', 'openvas', '-s'], stdout=subprocess.PIPE
+            )


Suggested change

)

)

self._sudo_available = True

bjoernricks · 2019-06-26T06:19:50Z

ospd_openvas/wrapper.py

+        if result == 0:
+            _sudo_available = True
+
+        return _sudo_available


Just remove these lines above.

bjoernricks · 2019-06-26T06:21:08Z

ospd_openvas/wrapper.py

+        """ Checks that sudo is available and set the global var. """
+        _sudo_available = False
+        try:
+            result = subprocess.check_call(


check_call doesn't return the returncode. If the returncode is != 0 it raises an exception https://docs.python.org/3/library/subprocess.html#subprocess.check_output So no need to check the result.

bjoernricks · 2019-06-26T06:23:51Z

ospd_openvas/wrapper.py

@@ -715,6 +716,23 @@ def get_detection_vt_as_xml_str(

        return tostring(_detection).decode('utf-8')

+    def sudo_check(self):


Suggested change

def sudo_check(self):

@property

def sudo_available(self):

bjoernricks · 2019-06-26T06:24:56Z

ospd_openvas/wrapper.py

@@ -715,6 +716,23 @@ def get_detection_vt_as_xml_str(

        return tostring(_detection).decode('utf-8')

+    def sudo_check(self):
+        """ Checks that sudo is available and set the global var. """


Suggested change

""" Checks that sudo is available and set the global var. """

""" Checks that sudo is available and set the global var. """

if self._sudo_available is not None:

return self._sudo_available

bjoernricks · 2019-06-26T06:25:09Z

ospd_openvas/wrapper.py

@@ -715,6 +716,23 @@ def get_detection_vt_as_xml_str(

        return tostring(_detection).decode('utf-8')

+    def sudo_check(self):
+        """ Checks that sudo is available and set the global var. """
+        _sudo_available = False


Suggested change

_sudo_available = False

bjoernricks · 2019-06-26T06:25:53Z

ospd_openvas/wrapper.py

+        """ Checks that sudo is available and set the global var. """
+        _sudo_available = False
+        try:
+            result = subprocess.check_call(


Suggested change

result = subprocess.check_call(

subprocess.check_call(

bjoernricks · 2019-06-26T06:33:03Z

ospd_openvas/wrapper.py

+                    if self.sudo_available:
+                        cmd = ['sudo', 'openvas', '--scan-stop', scan_id]
+                    else:
+                        cmd = ['openvas', '--scan-stop', scan_id]


These lines can be simplified by something like:

cmd = ['openvas', '--scan-stop', scan_id] if self.sudo_available: cmd = ['sudo'] + cmd

bjoernricks · 2019-06-26T06:44:14Z

ospd_openvas/wrapper.py

+            cmd = ['sudo', 'openvas', '--scan-start', openvas_scan_id]
+        else:
+            cmd = ['openvas', '--scan-start', openvas_scan_id]
+


Can be simplified as above.

Maybe it would even better to extract calling openvas into an own method/function e.g.

def call_openvas(args, use_sudo=False, niceness=None): cmd = args if use_sudo: cmd = ['sudo'] + cmd if niceness is not None: cmd = ['nice', ...] + cmd try: subproces. .... ...

but i am not 100 percent sure about that because we are calling different functions of subrocess at the moment with slightly different arguments.

codecov · 2019-06-26T08:59:43Z

Codecov Report

Merging #87 into master will decrease coverage by 0.22%.
The diff coverage is 37.5%.

@@            Coverage Diff             @@
##           master      #87      +/-   ##
==========================================
- Coverage    72.1%   71.88%   -0.23%     
==========================================
  Files           4        4              
  Lines         864      875      +11     
==========================================
+ Hits          623      629       +6     
- Misses        241      246       +5

Impacted Files	Coverage Δ
ospd_openvas/wrapper.py	`59.02% <37.5%> (-0.09%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5cbc020...8594ead. Read the comment docs.

codecov · 2019-06-26T08:59:44Z

Codecov Report

Merging #87 into master will decrease coverage by 0.15%.
The diff coverage is 44.44%.

@@            Coverage Diff             @@
##           master      #87      +/-   ##
==========================================
- Coverage    72.1%   71.94%   -0.16%     
==========================================
  Files           4        4              
  Lines         864      877      +13     
==========================================
+ Hits          623      631       +8     
- Misses        241      246       +5

Impacted Files	Coverage Δ
ospd_openvas/wrapper.py	`59.15% <44.44%> (+0.05%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5cbc020...d75ef15. Read the comment docs.

Fix file name of ospd-scanner INSTALL guide

Check sudo permissions before call openvas.

89802d1

Check if the user running ospd-openvas has permissions to call openvas with sudo and without password. Depending on it, it will run openvas with sudo or without it.

jjnicola requested review from bjoernricks, mattmundell, janowagner and timopollmeier June 25, 2019 15:46

jjnicola added the Work in progress Waiting for Feature Specification validation. label Jun 25, 2019

bjoernricks suggested changes Jun 26, 2019

View reviewed changes

jjnicola added 3 commits June 26, 2019 04:26

Add test for sudo_check()

0edbf2b

Set the variable instead of returning true or false.

7f52bac

Simplify cmd to call openvas with sudo and nice.

8594ead

jjnicola force-pushed the sudo-opt branch from a17b2c5 to 8594ead Compare June 26, 2019 08:52

Use property decorator for sudo_available

a034dd5

bjoernricks approved these changes Jun 26, 2019

View reviewed changes

jjnicola force-pushed the sudo-opt branch from 1a28a20 to 5991e88 Compare June 26, 2019 11:48

Adjust tests

d75ef15

jjnicola force-pushed the sudo-opt branch from 5991e88 to d75ef15 Compare June 26, 2019 11:49

jjnicola merged commit 597abf5 into greenbone:master Jun 26, 2019

jjnicola deleted the sudo-opt branch June 26, 2019 12:36

jjnicola removed the Work in progress Waiting for Feature Specification validation. label Jun 28, 2019

jjnicola mentioned this pull request Oct 21, 2019

[2.0.0] Add option to set the group of the socket greenbone/ospd#161

Closed

cfi-gb mentioned this pull request Aug 3, 2020

[20.08] Document sudo configuration. #303

Merged

ArnoStiefvater pushed a commit to ArnoStiefvater/ospd-openvas that referenced this pull request Oct 25, 2021

Merge pull request greenbone#87 from wiegandm/INSTALL-link-fix

977e2e8

Fix file name of ospd-scanner INSTALL guide

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check sudo permissions before call openvas. #87

Check sudo permissions before call openvas. #87

jjnicola commented Jun 25, 2019 •

edited

Loading

bjoernricks left a comment

bjoernricks Jun 26, 2019

bjoernricks Jun 26, 2019

bjoernricks Jun 26, 2019

bjoernricks Jun 26, 2019

bjoernricks Jun 26, 2019

bjoernricks Jun 26, 2019

bjoernricks Jun 26, 2019

bjoernricks Jun 26, 2019

bjoernricks Jun 26, 2019

bjoernricks Jun 26, 2019

codecov bot commented Jun 26, 2019

codecov bot commented Jun 26, 2019 •

edited

Loading

		@@ -715,6 +716,23 @@ def get_detection_vt_as_xml_str(

		return tostring(_detection).decode('utf-8')

		def sudo_check(self):

Check sudo permissions before call openvas. #87

Check sudo permissions before call openvas. #87

Conversation

jjnicola commented Jun 25, 2019 • edited Loading

bjoernricks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 26, 2019

Codecov Report

codecov bot commented Jun 26, 2019 • edited Loading

Codecov Report

jjnicola commented Jun 25, 2019 •

edited

Loading

codecov bot commented Jun 26, 2019 •

edited

Loading