[apparmor] [patch] introduce AppArmorBug exceptions

[Steve Beattie]

On Sun, Nov 09, 2014 at 11:05:38PM +0100, Christian Boltz wrote:
> Hello,
> 
> while developing the capability rule class, I added lots of error 
> handling, including several checks for "things that should never 
> happen"[tm], and raise an exception in those cases.
> 
> "Things that should never happen" includes what it says, for example:
> - a (broken) log contains operation="capable" ... capname="" - we really
>   don't want to propose "capability  ," for that ;-)
> - someone manipulated the rule object in an invalid way (aka "bug")
> - someone manually adds a capability with empty or whitespace-only name 
>   to the rule object - without checking, this would result in a 
>   "capability  ," rule which is obviously too broad
> 
> I'd like to have a separate exception class for these things to
> a) clearly separate them from "expected" exceptions (for example invalid 
>    lines in a profile is more or less "expected")
> b) allow us to disable backtraces for the "expected" AppArmorExceptions,
>    while still having full debug output for unexpected issues
> 
> The naming of the new exception type is an interesting question - I'd
> just use "AppArmorBug", but Kshitij hopes we can use something like
> AppArmorSledgeHammerException or SledgehammerBug ;-)

I'm not entirely sure I see the distinction; these sound more like
assert()-like sanity checks. Which is fine, I don't mind having them
broken out into a separate Exception class (AppArmorAssert?).

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.

As for your short-term implmentation:

> === modified file 'utils/apparmor/common.py'
> --- utils/apparmor/common.py    2014-10-14 10:54:39 +0000
> +++ utils/apparmor/common.py    2014-11-09 21:48:19 +0000
> @@ -35,6 +35,15 @@
>      def __str__(self):
>          return repr(self.value)
>  
> +class AppArmorBug(Exception):
> +    '''This class represents AppArmor exceptions "that should never happen"'''

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 AppArmorExceptionClassToBeNamedLaterAfterManyLongTediousArgumentsOnIRC(AppArmorException):
	'''This class represents AppArmor exceptions "that should never happen"'''
	pass

Thanks.

-- 
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/20141112/b9e5d6c5/attachment.pgp>