[apparmor] backtrace from logparser.py prefetch_next_log_line

[Christian Boltz]

Hello,

Am Sonntag, 14. September 2014 schrieb Kshitij Gupta:
> On Sun, Sep 14, 2014 at 1:55 AM, Christian Boltz wrote:
...
> Encodings cant let one live in peace.

Indeed :-/
 
> > Maybe it would be a good idea not to assume any encoding and use raw
> > bytes instead? (Patches welcome ;-)
> > 
> > You can use   aa-logprof -f testfile   (attached) to reproduce this
> > bug.
> Thanks for the test file. :-)
> 
> I've tried a tiny hack-ish fix (based on your open('testfile', 'rb')
> patch). The strings given in byte can be typecasted to string due to
> which,
> 
> b'2014-07-12T19:44:19.023487+02:00 geeko
> org.gtk.Private.GoaVolumeMonitor[9603]: g_dbus_connection_real_closed:
> Remote peer vanished with error: Fehler beim Empfang der Nachricht:
> Die Verbindung wurde vom Kommunikationspartner zur\xfcckgesetzt
> (g-io-error-quark, 0). Exiting.\r\n'
> 
> becomes:
> 
> "b'2014-07-12T19:44:19.023487+02:00 geeko
> org.gtk.Private.GoaVolumeMonitor[9603]: g_dbus_connection_real_closed:
> Remote peer vanished with error: Fehler beim Empfang der Nachricht:
> Die Verbindung wurde vom Kommunikationspartner zur\\xfcckgesetzt
> (g-io-error-quark, 0). Exiting.\\r\\n'"
> 
> notice the prefixed b' and suffixed ' .

Also note that \r\n becomes \\r\\n

> This happens only in Python3
> for Python2 no prefix or suffix are added.
> After typecasting to string the regex's can be applied and hopefully
> they can ignore the prefix and suffix part and find the relevant
> things or maybe we can update the regex to do so?
> 
> Opinions?

As I already wrote on IRC, your patch breaks if the test log has more 
than one line ;-)

> === modified file 'utils/apparmor/logparser.py'
> --- utils/apparmor/logparser.py    2014-08-20 22:55:44 +0000
> +++ utils/apparmor/logparser.py    2014-09-14 14:43:37 +0000
> @@ -59,7 +59,7 @@
>      def prefetch_next_log_entry(self):
>          if self.next_log_entry:
>              sys.stderr.out('A log entry already present: %s' %
> self.next_log_entry)
> -        self.next_log_entry = self.LOG.readline()
> +        self.next_log_entry = str(self.LOG.readline())
>          while not self.RE_LOG_v2_6_syslog.search(self.next_log_entry)
> and not self.RE_LOG_v2_6_audit.search(self.next_log_entry) and not
> (self.logmark and self.logmark in self.next_log_entry):
>              self.next_log_entry = self.LOG.readline()

This self.LOG.readline() also needs to be wrapped with str() - but 
unfortunately after this change...

>              if not self.next_log_entry:
>                  break

... this "break" will never be reached and aa-logprof enters an endless 
loop instead of break'ing when reaching EOF :-(

> @@ -330,7 +330,7 @@
>          #event_type = None
>          try:
>              #print(self.filename)
> -            self.LOG = open_file_read(self.filename)
> +            self.LOG = open(self.filename, 'rb')
>          except IOError:
>              raise AppArmorException('Can not read AppArmor logfile: '
> + self.filename)
>          #LOG = open_file_read(log_open)

I have a working proof-of-concept patch, but only tested it with py3.
The errors= parameter was introduced in py3, which means we'll need 
different code for py2 and a version switch.

See https://docs.python.org/3/library/functions.html#open for details
about the error= parameter.

The patch assumes that we'll never have to rely on a line containing
special characters. I'm 99% sure this won't cause a problem because the
log lines we are interested in will be encoded if they contain special
characters. However, users can specify anything for "aa-logprof -m $MARK",
and "anything" can in theory contain quite funny characters.


=== modified file 'utils/apparmor/logparser.py'
--- utils/apparmor/logparser.py 2014-08-20 22:55:44 +0000
+++ utils/apparmor/logparser.py 2014-09-22 22:28:27 +0000
@@ -330,7 +330,7 @@
         #event_type = None
         try:
             #print(self.filename)
-            self.LOG = open_file_read(self.filename)
+            self.LOG = open(self.filename, 'r', errors='surrogateescape')   # py3 only!
         except IOError:
             raise AppArmorException('Can not read AppArmor logfile: ' + self.filename)
         #LOG = open_file_read(log_open)



Regards,

Christian Boltz
-- 
12.2 will be released also in 2013 and not 2011 (which would save 
us all some work, but violation of linear time is currently not an 
option ;) [Marcus Meissner in opensuse-announce]