[apparmor] [patch] Raise AppArmorBug on unknown request_mask in logparser.py

[Christian Boltz]

Hello,

Am Donnerstag, 7. Januar 2016 schrieb Steve Beattie:
> On Sat, Dec 12, 2015 at 01:39:25AM +0100, Christian Boltz wrote:
...
> > (yes, I tested this before sending the patch ;-)
> 
> Sigh, yet another difference in behavior between python2 and python3.
> 
> For python2, this happens when AppArmorBug is used instead:
...
> So, for AppArmor 2.9 and 2.10, I'm fine with either AppArmorException
> or AppArmorBug being raised (i.e. Acked-by: Steve Beattie
> <steve at nxnw.org> for your choice, whether you want the enhanced
> reporting with the drawback of the double trace, or not).

Good question ;-)

(2.9 will need a slightly different patch because it can't handle \n in 
exception messages - that needs apparmor.fail. That probably means to 
simply replace the \n with a space - which is hard to read, but still 
better than just erroring out with "Unknown log mode foo".)

> For trunk/upcoming 2.11, I proposed that we use the following python 3
> only syntax:
...
> +                except AppArmorException as e:
> +                    # Drop the original AppArmorException by passing
> None as the parent exception 
> +                    raise
> AppArmorBug('%(msg)s\n\nThis error was caused by the log
> line:\n%(logline)s' % +                            {'msg': e.value,
> 'logline': line}) from None 

Nice, I didn't know the "from None" (or "from other_exception") syntax 
yet.

> This results in the following:
...
> Which is what I think you desire. 

Yes, that's much better than two backtraces.

In this special case, it's superfluous to display a backtrace at all 
because the relevant information (the log line) is in the exception 
message, but it's probably not worth special handling.

> But it also means we need to get
> serious about making the code base python3 only (e.g. use pyflakes3
> instead of pyflakes, etc.).
> 
> What do you think?

I tend to add another version check ;-)

My plan is:
- officially deprecate (but still support) py2 in 2.11 - and also give 
  people some time to switch from aa-status --enabled to aa-enabled
  (BTW: You didn't commit your aa-status patch to make apparmor.fail 
  optional yet.)
- no longer worry about py2 in 2.12

That said, here's the updated patch:


[ 38-more-useful-logparser-failure-reports.diff ]

=== modified file ./utils/apparmor/logparser.py
--- utils/apparmor/logparser.py 2016-01-07 14:53:52.853018485 +0100
+++ utils/apparmor/logparser.py 2016-01-07 15:11:58.033710237 +0100
@@ -17,7 +17,7 @@
 import sys
 import time
 import LibAppArmor
-from apparmor.common import AppArmorException, open_file_read, DebugLogger
+from apparmor.common import AppArmorException, AppArmorBug, open_file_read, DebugLogger
 
 from apparmor.aamode import validate_log_mode, log_str_to_mode, hide_log_mode, AA_MAY_EXEC
 
@@ -413,7 +413,17 @@
             event = self.parse_log_record(line)
             #print(event)
             if event:
-                self.add_event_to_tree(event)
+                try:
+                    self.add_event_to_tree(event)
+                except AppArmorException as e:
+                    ex_msg = ('%(msg)s\n\nThis error was caused by the log line:\n%(logline)s' %
+                            {'msg': e.value, 'logline': line})
+                    if sys.version_info < (3, 0):
+                        raise AppArmorBug(ex_msg)
+                    else:
+                        # Drop the original AppArmorException by passing None as the parent exception
+                        raise AppArmorBug(ex_msg) from None
+
         self.LOG.close()
         self.logmark = ''
         return self.log


Since this patch should work with py2 and py3, I propose it for trunk,
2.10 and 2.9.

As I wrote above, I'll replace \n in ex_msg with spaces in 2.9 because
it can't handle \n in exception messages.


Regards,

Christian Boltz
-- 
Wenn diese Liste über Mailman betrieben würde, dann würden wir den
ganzen Tag nichts anderes machen als eine Menschenkette zum nächsten
Computerladen aufrechtzuerhalten um RAM in einem konstanten fluss in
lists.suse.com einzubauen ;)         [Henne Vogelsang in suse-linux]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20160107/2085d699/attachment.pgp>