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

[Tyler Hicks]

On 2015-12-02 22:00:32, Christian Boltz wrote:
> 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 ;-)

I was simply making this call site apparmor.easyprof.* call sites which
catch AppArmorExceptions and print the error to stderr. If we want to
make this a global change, across all AppArmorExceptions, then we should
probably stop catching those exceptions in aa-easyprof.

Cleanup patch to follow.

> > 
> > 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.

Acked-by: Tyler Hicks <tyhicks at canonical.com>

Tyler

> 
> 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]
> 
> 
> -- 
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20151215/b2f80fdf/attachment-0001.pgp>