[apparmor] [patch] Fix parsing/storing bare file rules

[Christian Boltz]

Hello,

Am Donnerstag, 19. November 2015 schrieb Kshitij Gupta:
> On Wed, Oct 28, 2015 at 4:15 AM, Christian Boltz wrote:
...
> > BTW: I noticed this while playing with a more strict
> > profile_storage() that uses more dict()s instead of a big hasher()
> > monster.
> 
> *stricter* profile_storage() sounds rather nice and useful.

Indeed. However, it also comes with some road bumps. Therefore I'll keep
it as a local patch for a while to be sure it doesn't break anything.

If you want to test my current patch:

=== modified file ./utils/apparmor/aa.py
--- utils/apparmor/aa.py        2015-10-29 11:56:37.409610655 +0100
+++ utils/apparmor/aa.py        2015-10-29 12:05:36.469744975 +0100
@@ -455,7 +455,8 @@
     # Note that this function doesn't explicitely init all those keys (yet).
     # It will be extended over time, with the final goal to get rid of hasher().
 
-    profile = hasher()
+    #profile = hasher()
+    profile = dict()  # let's see what explodes...
 
     # profile['info'] isn't used anywhere, but can be helpful in debugging.
     profile['info'] = {'profile': profilename, 'hat': hat, 'calledby': calledby}
@@ -466,11 +467,40 @@
     profile['rlimit']           = RlimitRuleset()
     profile['signal']           = SignalRuleset()
 
+    profile['alias']            = dict()
+    profile['include']          = dict()
+    profile['localinclude']     = dict()
+    profile['repo']             = dict()
+    profile['lvar']             = dict()
+
+    profile['attachment']       = ''
+    profile['flags']            = ''
+    profile['external']         = False
+    profile['header_comment']   = ''
+    profile['profile_keyword']  = False
+    profile['profile']          = False  # profile or hat?
+
+    profile['allow'] = dict()
     profile['allow']['path'] = hasher()
-    profile['allow']['dbus'] = list()
-    profile['allow']['mount'] = list()
-    profile['allow']['ptrace'] = list()
+    profile['allow']['link'] = hasher()
+
+    # dbus, mount, ptrace, pivot_root, unix have a .get() fallback to list() - initialize them nevertheless
+    profile['allow']['dbus']    = list()
+    profile['allow']['mount']   = list()
+    profile['allow']['ptrace']  = list()
     profile['allow']['pivot_root'] = list()
+    profile['allow']['unix']    = list()
+
+    profile['deny'] = dict()
+    profile['deny']['link']     = hasher()
+    profile['deny']['path']     = hasher()
+
+    # dbus, mount, ptrace, pivot_root, unix have a .get() fallback to list() - initialize them nevertheless
+    profile['deny']['dbus']     = list()
+    profile['deny']['mount']    = list()
+    profile['deny']['ptrace']   = list()
+    profile['deny']['pivot_root'] = list()
+    profile['deny']['unix']     = list()
 
     return profile
 
@@ -3543,7 +3573,7 @@
                 profile_data[name]['repo']['id']):
             repo = profile_data[name]['repo']
             string += '# REPOSITORY: %s %s %s\n' % (repo['url'], repo['user'], repo['id'])
-        elif profile_data[name]['repo']['neversubmit']:
+        elif profile_data[name]['repo'].get('neversubmit'):
             string += '# REPOSITORY: NEVERSUBMIT\n'
 
 #     if profile_data[name].get('initial_comment', False):


(the neversubmit change was such a road bump ;-)

> > [ 18-fix-bare-file-rule.diff ]
> > 
> > === modified file ./utils/apparmor/aa.py
...
> > -            audit, allow, allow_keyword, comment =
> > parse_modifiers(matches)
> > +            audit, deny, allow_keyword, comment =
> > parse_modifiers(matches)> 
> >              # TODO: honor allow_keyword and comment
> > 
> > +            if deny:
> > +                allow = 'deny'
> 
> I know we have this sort of thing at multiple places(I'll take that
> blame) but it still is hilarious. Its basically "if False: true =
> False"

Well, 'allow' probably isn't the perfect variable name [1] - something 
like 'perm' or 'allow_mode' would be more intuitive ;-)

However, I don't plan to change it - it's used too often to get it 
right ;-)  Besides that, its usage should fade out over time by 
converting more code to rule classes.


Regards,

Christian Boltz

[1] The old perl code used $allow, so I can imagine why you took 'allow'
    as variable name.
-- 
> And don't be afraid of Henne, he's a nice guy :-)
Pffft Lies, all lies! I'm the meanest son of a gun you know.
Admit it! 8-)
[> Vincent Untz and Henne Vogelsang in opensuse-project]