[apparmor] [PATCH] utils: Print aa-easyprof error to stderr upon manifest parsing error

[Christian Boltz]

Hello,

Am Dienstag, 1. Dezember 2015 schrieb Christian Boltz:
> Am Montag, 30. November 2015 schrieb Tyler Hicks:
> > A common usage of aa-easyprof is to pipe its stdout to a file
> > representing an AppArmor profile. Errors must go to stderr.
> > 
> > https://launchpad.net/bugs/1521400
> > 
> > Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
> > ---
> > 
> >  utils/aa-easyprof | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/utils/aa-easyprof b/utils/aa-easyprof
> > index 93861ae..8e9db71 100755
> > --- a/utils/aa-easyprof
> > +++ b/utils/aa-easyprof
> > 
> > @@ -53,7 +53,10 @@ if __name__ == "__main__":
> >              error("Could not read '%s': %s (%d)\n" % (opt.manifest,
> > 
> > os.strerror(e.errno), e.errno)) -        profiles =
> > apparmor.easyprof.parse_manifest(manifest, opt) +        try:
> > +            profiles = apparmor.easyprof.parse_manifest(manifest,
> > opt)
> > +        except AppArmorException as e:
> > +            error(e.value)
> > 
> >      else: # fake up a tuple list when processing command line args
> >      
> >          profiles.append( (binary, opt) )
> 
> NAK.
> 
> aa-easyprof is using apparmor.fail for exception handling, so it
> doesn't make sense to fix this only for aa-easyprof. We should fix
> this in apparmor.fail so that everybody benefits ;-)
> 
> As a replacement for your patch, I propose:
> 
> 
> [patch] Let the apparmor.fail error handler print to stderr
> 
> The patch also switches to using error() instead of a plain print()
> for AppArmorException, which means prefixing the error message with
> 'ERROR: '
> 
> References: https://bugs.launchpad.net/apparmor/+bug/1521400
> 
> 
> I propose this patch for trunk and 2.10.

Some more testing shows that apparmor.fail needs
    from __future__ import print_function
to ensure compability with python 2.x.

Here's the updated patch:


[ 23-apparmor-fail-print-to-stderr.diff ]

--- utils/apparmor/fail.py      2015-12-02 21:53:34.635564749 +0100
+++ utils/apparmor/fail.py      2015-12-02 21:54:34.795174617 +0100
@@ -8,12 +8,16 @@
 #
 # ------------------------------------------------------------------
 
+from __future__ import print_function  # needed in py2 for print('...', file=sys.stderr)
+
 import cgitb
 import os
 import sys
 import tempfile
 import traceback
 
+from apparmor.common import error
+
 #
 # Exception handling
 #
@@ -27,8 +31,8 @@
     (ex_cls, ex, tb) = exc_info
 
     if ex_cls.__name__  == 'AppArmorException':  # I didn't find a way to get this working with isinstance() :-/
-        print('')
-        print(ex.value)
+        print('', file=sys.stderr)
+        error(ex.value)
     else:
         (fd, path) = tempfile.mkstemp(prefix='apparmor-bugreport-', suffix='.txt')
         file = os.fdopen(fd, 'w')
@@ -40,13 +44,13 @@
         file.write('Please consider reporting a bug at https://bugs.launchpad.net/apparmor/\n')
         file.write('and attach this file.\n')
 
-        print(''.join(traceback.format_exception(*exc_info)))
-        print('')
-        print('An unexpected error occoured!')
-        print('')
-        print('For details, see %s' % path)
-        print('Please consider reporting a bug at https://bugs.launchpad.net/apparmor/')
-        print('and attach this file.')
+        print(''.join(traceback.format_exception(*exc_info)), file=sys.stderr)
+        print('', file=sys.stderr)
+        print('An unexpected error occoured!', file=sys.stderr)
+        print('', file=sys.stderr)
+        print('For details, see %s' % path, file=sys.stderr)
+        print('Please consider reporting a bug at https://bugs.launchpad.net/apparmor/', file=sys.stderr)
+        print('and attach this file.', file=sys.stderr)
 
 def enable_aa_exception_handler():
     '''Setup handle_exception() as exception handler'''



Regards,

Christian Boltz
-- 
I have also tried to understand how to write and install systemd stuff
to start daemons and it definitely hasn't passed the "10 minute test".
It definitely lacks better documentation, or I've been too lazy, I
wouldn't exclude that from the equation :)
[Pascal Bleser in opensuse-factory]