[apparmor] [patch] merge 'path' if conditions in logparser.py / add_event_to_tree()

[Christian Boltz]

Hello,

Am Freitag, 27. Februar 2015 schrieb Steve Beattie:
> > logparser.py / add_event_to_tree() has 5 places to handle 'path'
> > events. This patch merges most if conditions to reduce that to 2
> > places.>
> > 
> >
> > This patch is just a cleanup without any behaviour change,
> > therefore 
> > trunk is probably enough (but I won't object if someone acks it for
> > 2.9 ;-)
> > 
> > Sidenote: I don't like things like
> >
> >     if 'file_' in e['operation']
> >
> > too much. Can/should we make this
> >
> >     if e['operation'].startswith('file_')
> >
> > (same question for 'xattr' and 'inode_' ;-)
> 
> Yes I would prefer the more strict startswith match for looser
> matches like the above, though as near as I can tell from looking at
> the log tests, xattr should be a keyword and was only generated by
> the v1 log format which is dead. You ought to be able to put 'xattr'
> in the list of operations you test against safely.
> 
> Also, 'getattr' is another operation that needs to be supported I
> think. If we fix that, then the patch should probably go into 2.9.

OK, here's the updated patch with
- startswith('file_') and 'xattr' moved to the list, which means those 
  two are matched more strict now
- 'getattr' added to the list


[ logparser-merge-path-handling.diff ]

=== modified file 'utils/apparmor/logparser.py'
--- utils/apparmor/logparser.py 2015-02-28 13:09:45 +0000
+++ utils/apparmor/logparser.py 2015-02-28 16:47:17 +0000
@@ -263,20 +263,16 @@
             else:
                 self.debug_logger.debug('add_event_to_tree: dropped exec event in %s' % e['profile'])
 
-        elif 'file_' in e['operation']:
-            self.add_to_tree(e['pid'], e['parent'], 'path',
-                             [profile, hat, prog, aamode, e['denied_mask'], e['name'], ''])
-        elif e['operation'] in ['open', 'truncate', 'mkdir', 'mknod', 'rename_src',
-                                'rename_dest', 'unlink', 'rmdir', 'symlink_create', 'link']:
+        elif ( e['operation'].startswith('file_') or
+            e['operation'] in ['open', 'truncate', 'mkdir', 'mknod', 'rename_src',
+                                'rename_dest', 'unlink', 'rmdir', 'symlink_create', 'link',
+                                'sysctl', 'getattr', 'setattr', 'xattr'] ):
             #print(e['operation'], e['name'])
             self.add_to_tree(e['pid'], e['parent'], 'path',
                              [profile, hat, prog, aamode, e['denied_mask'], e['name'], ''])
         elif e['operation'] == 'capable':
             self.add_to_tree(e['pid'], e['parent'], 'capability',
                              [profile, hat, prog, aamode, e['name'], ''])
-        elif e['operation'] == 'setattr' or 'xattr' in e['operation']:
-            self.add_to_tree(e['pid'], e['parent'], 'path',
-                             [profile, hat, prog, aamode, e['denied_mask'], e['name'], ''])
         elif 'inode_' in e['operation']:
             is_domain_change = False
             if e['operation'] == 'inode_permission' and (e['denied_mask'] & AA_MAY_EXEC) and aamode == 'PERMITTING':
@@ -294,10 +290,6 @@
                 self.add_to_tree(e['pid'], e['parent'], 'path',
                                  [profile, hat, prog, aamode, e['denied_mask'], e['name'], ''])
 
-        elif e['operation'] == 'sysctl':
-            self.add_to_tree(e['pid'], e['parent'], 'path',
-                             [profile, hat, prog, aamode, e['denied_mask'], e['name'], ''])
-
         elif e['operation'] == 'clone':
             parent, child = e['pid'], e['task']
             if not parent:




Regards,

Christian Boltz
-- 
[makeSUSEdvd] When it works, I will most likely hold a press conference
or something, so people will be informed by CNN. :-)
[houghi in opensuse]