[apparmor] [patch] aa.py get_output(): raise exception on non-executable or non-existing programs

[Christian Boltz]

Hello,

if the program specified as get_output param isn't executable or doesn't
exist at all, get_output() returns with ret = -1.

Raising an exception looks like a better option, especially because
other possible exec failures already raise an exception ("Unable to
fork").

Note: get_output is only used by get_reqs() which also does the
os.access() check for x permissions (and raises an exception), so in
practise raising an exception in get_output() doesn't change anything.


This change also allows to rewrite and simplify get_output() quite a bit.


Another minor change (and fix) is in the removal of the last line. The
old code removed the last line if output contained at least two items.
This had two not-so-nice effects:
- an empty output resulted in [''] instead of []
- if a command didn't add a \n on the last line, this line was deleted
  nevertheless

The patch changes that to always remove the last line if it is empty,
which fixes both issues mentioned above.


Also add a test to ensure the exception is really raised, and adjust the
test that expects an empty stdout.




[ 67-get_output-dont-ignore-non-executable.diff ]

--- utils/test/test-aa.py       2016-02-01 20:06:26.082212471 +0100
+++ utils/test/test-aa.py       2016-02-01 20:12:46.228094146 +0100
@@ -77,12 +77,16 @@
     tests = [
         (['./fake_ldd', '/AATest/lib64/libc-2.22.so'],  (0, ['        /AATest/lib64/ld-linux-x86-64.so.2 (0x0000556858473000)', '        linux-vdso.so.1 (0x00007ffe98912000)']     )),
         (['./fake_ldd', '/tmp/aa-test-foo'],            (0, ['        not a dynamic executable']                                                                                    )),
-        (['./fake_ldd', 'invalid'],                     (1, ['']                                                                                                                    )),  # stderr is not part of output
+        (['./fake_ldd', 'invalid'],                     (1, []                                                                                                                      )),  # stderr is not part of output
     ]
     def _run_test(self, params, expected):
         self.assertEqual(get_output(params), expected)
 
+    def test_get_output_nonexisting(self):
+        with self.assertRaises(AppArmorException):
+            ret, output = get_output(['./_file_/_not_/_found_'])
+
 class AATest_get_reqs(AATest):
     tests = [
         ('/AATest/bin/bash',    ['/AATest/lib64/libreadline.so.6', '/AATest/lib64/libtinfo.so.6', '/AATest/lib64/libdl.so.2', '/AATest/lib64/libc.so.6', '/AATest/lib64/ld-linux-x86-64.so.2']),

--- utils/apparmor/aa.py        2016-02-01 20:06:26.086212449 +0100
+++ utils/apparmor/aa.py        2016-02-01 20:12:27.192200872 +0100
@@ -334,27 +334,21 @@
 
 def get_output(params):
-    """Returns the return code output by running the program with the args given in the list"""
+    '''Runs the program with the given args and returns the return code and stdout (as list of lines)'''
-    program = params[0]
-    # args = params[1:]
-    ret = -1
-    output = []
-    # program is executable
-    if os.access(program, os.X_OK):
-        try:
-            # Get the output of the program
-            output = subprocess.check_output(params)
-        except OSError as e:
-            raise AppArmorException(_("Unable to fork: %(program)s\n\t%(error)s") % { 'program': program, 'error': str(e) })
-            # If exit-codes besides 0
-        except subprocess.CalledProcessError as e:
-            output = e.output
-            output = output.decode('utf-8').split('\n')
-            ret = e.returncode
-        else:
-            ret = 0
-            output = output.decode('utf-8').split('\n')
+    try:
+        # Get the output of the program
+        output = subprocess.check_output(params)
+        ret = 0
+    except OSError as e:
+        raise AppArmorException(_("Unable to fork: %(program)s\n\t%(error)s") % { 'program': params[0], 'error': str(e) })
+    except subprocess.CalledProcessError as e:  # If exit-codes besides 0
+        output = e.output
+        ret = e.returncode
+
+    output = output.decode('utf-8').split('\n')
+
     # Remove the extra empty string caused due to \n if present
-    if len(output) > 1:
+    if output[len(output) - 1] == '':
         output.pop()
+
     return (ret, output)
 




Regards,

Christian Boltz
-- 
Der geistige Horizont ist der Abstand zwischen Brett und Kopf.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20160221/37b5d501/attachment.pgp>