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

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.


Acked-by: Kshitij Gupta <kgupta8592@gmail.com>

utils/apparmor/aa.py

@@ -332,29 +332,23 @@ def head(file):
         raise AppArmorException(_('Unable to read first line from %s: File Not Found') % file)
 
 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]
+    try:
-    # args = params[1:]
+        # Get the output of the program
-    ret = -1
+        output = subprocess.check_output(params)
-    output = []
+        ret = 0
-    # program is executable
+    except OSError as e:
-    if os.access(program, os.X_OK):
+        raise AppArmorException(_("Unable to fork: %(program)s\n\t%(error)s") % { 'program': params[0], 'error': str(e) })
-        try:
+    except subprocess.CalledProcessError as e:  # If exit code != 0
-            # Get the output of the program
+        output = e.output
-            output = subprocess.check_output(params)
+        ret = e.returncode
-        except OSError as e:
+
-            raise AppArmorException(_("Unable to fork: %(program)s\n\t%(error)s") % { 'program': program, 'error': str(e) })
+    output = output.decode('utf-8').split('\n')
-            # 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')
     # 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)
 
 def get_reqs(file):

utils/test/test-aa.py

@@ -77,11 +77,15 @@ class AATest_get_output(AATest):
     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']),