[apparmor] GSoC review r59..62

Christian Boltz apparmor at cboltz.de
Tue Sep 17 21:41:42 UTC 2013


Hello,

the reviews for r59 and r61 are attached.

The code from r60 was moved to another file in r61, therefore I added 
all notes in the r61 review.

r62 only deleted disabled code, which means nothing to complain 
about ;-)


Regards,

Christian Boltz
-- 
> Das hatte ich (samt Kommentar aus der /etc/postfix/transport) doch
> schon in meiner letzten Mail erklärt ... ;)
Sandy ist schuld ;-)
Erst mit seiner Erklärung ist mir aufgefallen, dass ich es nicht 
verstanden habe. [> David Haller und Peter Mc Donough in opensuse-de]
-------------- next part --------------
------------------------------------------------------------
revno: 59
committer: Kshitij Gupta <kgupta8592 at gmail.com
branch nick: apparmor-profile-tools
timestamp: Tue 2013-09-17 11:46:17 +0530
message:
   
# a commit message never hurts ;-)

=== modified file 'apparmor/aa.py'
--- apparmor/aa.py	2013-09-12 09:12:15 +0000
+++ apparmor/aa.py	2013-09-17 06:16:17 +0000
@@ -2078,16 +2078,42 @@
     deleted = 0
     # Allow rules covered by denied rules shouldn't be deleted
     # only a subset allow rules may actually be denied

+    if include.get(incname, False):
+        deleted += delete_net_duplicates(profile['allow']['netdomain'], include[incname][incname]['allow']['netdomain'])
+         
+        deleted += delete_net_duplicates(profile['deny']['netdomain'], include[incname][incname]['deny']['netdomain'])
+         
+        deleted += delete_cap_duplicates(profile['allow']['capability'], include[incname][incname]['allow'])

# missing ['capability'] at the end of the line?

+        deleted += delete_cap_duplicates(profile['deny']['capability'], include[incname][incname]['deny']['capability'])
+         
+        deleted += delete_path_duplicates(profile, incname, 'allow')
+        deleted += delete_path_duplicates(profile, incname, 'deny')
+    elif filelist.get(incname, False):
+        deleted += delete_net_duplicates(profile['allow']['netdomain'], filelist[incname][incname]['allow']['netdomain'])
+         
+        deleted += delete_net_duplicates(profile['deny']['netdomain'], filelist[incname][incname]['deny']['netdomain'])
+         
+        deleted += delete_cap_duplicates(profile['allow']['capability'], filelist[incname][incname]['allow'])
         
# missing ['capability'] at the end of the line?

+        deleted += delete_cap_duplicates(profile['deny']['capability'], filelist[incname][incname]['deny']['capability'])
+         
+        deleted += delete_path_duplicates(profile, incname, 'allow')
+        deleted += delete_path_duplicates(profile, incname, 'deny')
+    
+    return deleted
+
     
     return deleted

# while at deleting duplicates - what about deleting one of the "return deleted"? ;-)



vim:ft=diff
-------------- next part --------------
------------------------------------------------------------
revno: 61
committer: Kshitij Gupta <kgupta8592 at gmail.com
branch nick: apparmor-profile-tools
timestamp: Tue 2013-09-17 22:30:48 +0530
message:
  seperated the code to check for duplicates into a separate module, will be
  using it to remove duplicates/superfluous rules/includes from base and other
  profiles in the aa-mergeprof


=== modified file 'Tools/aa-mergeprof'
--- Tools/aa-mergeprof	2013-09-12 09:12:15 +0000
+++ Tools/aa-mergeprof	2013-09-17 17:00:48 +0000
@@ -2,9 +2,10 @@
 
 import argparse
 import sys
+import cleanprof

# looks strange - especially because you import apparmor.cleanprofile two lines below

 import apparmor.aa as apparmor
-
+import apparmor.cleanprofile as cleanprofile


# BTW:
# python3 aa-mergeprof
#  File "aa-mergeprof", line 519
#      
#      ^
# SyntaxError: unexpected EOF while parsing




=== modified file 'apparmor/__init__.py'
--- apparmor/__init__.py	2013-09-12 09:12:15 +0000
+++ apparmor/__init__.py	2013-09-17 17:00:48 +0000
@@ -1,17 +1,15 @@
 import gettext
 import locale
 
 def init_localisation():
     locale.setlocale(locale.LC_ALL, '')
-    #cur_locale = locale.getlocale()
-    filename = '/usr/share/locale/%s/LC_MESSAGES/apparmor-utils.mo' % locale.getlocale()[0][0:2]
+    cur_locale = locale.getlocale()
+    filename = ''
+    #If a correct locale has been provided set filename else let a IOError be raised by '' path
+    if cur_locale[0]:
+        filename = '/usr/share/locale/%s/LC_MESSAGES/apparmor-utils.mo' % locale.getlocale()[0][0:2]

# this also works with LANG=C :-)
# Note to myself: submit updated version to openSUSE:Factory


=== added file 'apparmor/cleanprofile.py'
--- apparmor/cleanprofile.py	1970-01-01 00:00:00 +0000
+++ apparmor/cleanprofile.py	2013-09-17 17:00:48 +0000
@@ -0,0 +1,128 @@
        
+    def remove_duplicate_rules(self, program):
+        #Process the profile of the program
+        #Process every hat in the profile individually
+        file_includes = list(self.profile.filelist[self.profile.filename]['include'].keys())
+        #print(file_includes)
+        for hat in self.profile.aa[program].keys():
+            #The combined list of includes from profile and the file
+            includes = list(self.profile.aa[program][hat]['include'].keys()) + file_includes
+
+            allow_net_rules =  list(self.profile.aa[program][hat]['allow']['netdomain']['rule'].keys())

# allow_net_rules is not used anywhere - either it is superfluous, or the code to handle it is missing

+            print(deleted)
+            sys.exit(0)

# hmm, sys.exit in a module?
# I'd expect a 'return' ;-)





# [22:19:43] <cboltz> BTW: when I select "(N)ew" in logprof, it would be nice if the currently selected path 
#                     would be available for editing instead of having to type everything
# [22:20:16] <cboltz> (the old logprof has this feature ;-)
# [22:20:57] <kshitij8> i'll fix that. :)


vim:ft=diff


More information about the AppArmor mailing list