[apparmor] [patch] introduce AppArmorBug exceptions

[Steve Beattie]

On Thu, Nov 13, 2014 at 06:59:30PM +0100, Christian Boltz wrote:
> I'd prefer the name AppArmorBug because it makes clear something is 
> really broken. AppArmorAssert sounds too harmless ;-)

Heh, as opposed to "Oh, it's just a bug." :-)

> For the distinction, let me give you an example:
> 
> a) AppArmorException: aa-logprof parses the profiles and finds a line 
>    that doesn't match any of the RE_* patterns. That's probably a syntax
>    error in the profile - something that can happen after manually 
>    editing a profile.
> 
> b) AppArmorBug: parse_audit_allow() finds out that matches.group('allow') 
>    contains "maybe" (hint: the regex explicitely checks for "allow|deny",
>    so someone must have handed over a match object from a wrong regex -
>    that's clearly a bug.

Right. That kind of stuff is typically what people use asserts
to catch. I don't really care what the name is, so long as we all
understand what it means.

> > That said, for broken policy type stuff, it'd be nice to suggest to
> > the user what's gone wrong and suggestions on how to fix it (yes, the
> > parser needs a lot of work on that, too; alas, lex and yacc are not
> > the friendliest for accomplishing such things.
> 
> My TODO list is already long enough ;-)

So say we all.

> > As for your short-term implmentation:
> 
> > All the behavior here is a duplicate of the AppArmorException.  So it
> > would probably be better to inherit from it and override the behaviors
> > you want to change; e.g. for the time being:
> > 
> > class
> > AppArmorExceptionClassToBeNamedLaterAfterManyLongTediousArgumentsOnIR
> > C(AppArmorException): 
> >     '''This class represents AppArmor exceptions "that should never happen"''' 
> >     pass
> 
> KMail thinks your class name is too long ;-)

KMail's not the only one. :)

> Actually the goal is to use cgitb for all exceptions except 
> AppArmorException (which should just print the summary, but not a 
> backtrace) - but that's another patch (IIRC Kshitij has a draft for that 
> already).
> 
> This also means that AppArmorBug should be based on "Exception" so that 
> it doesn't inherit the (planned) changed behaviour of AppArmorException. 
> However I don't see a problem with making AppArmorBug behaving like a 
> normal Exception, so the updated patch is:

Sure. Once we have more complex implementations of these, we can
re-examine whether a class hierarchy for them makes sense.

Acked-by: Steve Beattie <steve at nxnw.org>

Thanks.

> === modified file 'utils/apparmor/common.py'
> --- utils/apparmor/common.py    2014-10-14 10:54:39 +0000
> +++ utils/apparmor/common.py    2014-11-13 17:38:31 +0000
> @@ -35,6 +35,10 @@
>      def __str__(self):
>          return repr(self.value)
>  
> +class AppArmorBug(Exception):
> +    '''This class represents AppArmor exceptions "that should never happen"'''
> +    pass
> +
>  #
>  # Utility functions
>  #

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- 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/20141114/a8d6774c/attachment.pgp>