[apparmor] [patch] Ignore file_perm events without request_mask

[Christian Boltz]

Hello,

Am Montag, 6. Juli 2015 schrieb Steve Beattie:
> On Sun, Jun 21, 2015 at 08:46:49PM +0200, Christian Boltz wrote:
> > for some (not yet known) reason, we get file_perm events without
> > request_mask set, which causes an aa-logprof crash.
> > 
> > Reproducer log entry:
> > Jun 19 12:00:55 piorun kernel: [4475115.459952] audit: type=1400
> > audit(1434708055.676:19629): apparmor="ALLOWED"
> > operation="file_perm" profile="/usr/sbin/apache2" pid=3512
> > comm="apache2" laddr=::ffff:193.0.236.159 lport=80
> > faddr=::ffff:192.168.103.80 fport=61985 family="inet6"
> > sock_type="stream" protocol=6
> > 
> > This patch changes logparser.py to ignore those events.
> > 
> > References: https://bugs.launchpad.net/apparmor/+bug/1466812/
> > 
> > I propose this patch for trunk and 2.9.
> > 
> > [ 54-logparser-fileperm-without-request_mask-lp1466812.diff ]
> > 
> > === modified file utils/apparmor/logparser.py
> > --- utils/apparmor/logparser.py 2015-04-03 17:41:51.355126519 +0200
> > +++ utils/apparmor/logparser.py 2015-06-21 20:36:30.170413044 +0200
> > @@ -253,6 +253,11 @@
> > 
> >                                  'rename_dest', 'unlink', 'rmdir',
> >                                  'symlink_create', 'link',
> > 
> >                                  'sysctl', 'getattr', 'setattr', 'xattr'] ):
> > +            # for some reason, we get file_perm log events without
> > request_mask, see https://bugs.launchpad.net/apparmor/+bug/1466812/
> > +            if e['operation'] == 'file_perm' and e['request_mask']
> > is None: +                self.debug_logger.debug('UNHANDLED
> > (missing request_mask): %s' % e) +                return
> > +
> > 
> >              # Map c (create) to a and d (delete) to w (logging is
> >              more detailed than the profile language) rmask =
> >              e['request_mask']
> >              rmask = rmask.replace('c', 'a')
> 
> I'm mostly on board here, the only issue I have is that the last
> action is to just 'return' whereas the other short-circuit exits
> from the function do 'return None'. But looking at the sole caller
> to add_event_to_tree(), the returned value is never checked (and the
> natural function exit does not return a value).
> 
> I'd like for things to be consistent one way or the other. Perhaps for
> 2.9 we do 'return None' to make for a minimal patch, and for trunk we
> fix the other early returns not to return anything?

I'd prefer another way, but that is a bigger change, so I'll probably
delay it after the 2.10 release ;-)

Currently it's very hard to write tests for add_event_to_tree() because
the last thing it does is self.add_to_tree(parsed_event). My plan is to 
change that to return() the event, and then let the caller (or a small 
function in the middle?) calls self.add_to_tree(). This will make 
testing much easier because we can simply check the return value.

Therefore I just changed the patch to return None to be consistent.

Updated patch:


[ 54-logparser-fileperm-without-request_mask-lp1466812.diff ]

=== modified file utils/apparmor/logparser.py
--- utils/apparmor/logparser.py 2015-04-03 17:41:51.355126519 +0200
+++ utils/apparmor/logparser.py 2015-06-21 20:36:30.170413044 +0200
@@ -253,6 +253,11 @@
                                 'rename_dest', 'unlink', 'rmdir', 'symlink_create', 'link',
                                 'sysctl', 'getattr', 'setattr', 'xattr'] ):
 
+            # for some reason, we get file_perm log events without request_mask, see https://bugs.launchpad.net/apparmor/+bug/1466812/
+            if e['operation'] == 'file_perm' and e['request_mask'] is None:
+                self.debug_logger.debug('UNHANDLED (missing request_mask): %s' % e)
+                return None
+
             # Map c (create) to a and d (delete) to w (logging is more detailed than the profile language)
             rmask = e['request_mask']
             rmask = rmask.replace('c', 'a')





Regards,

Christian Boltz
-- 
Hochleistungswebspace
Das sind public-html-Verzeichnisse, die jeden Morgen zwanzig Liegestütze
machen, und mit Testosteron vollgepumpt sind.  [Markus Schaber]