[apparmor] [patch] split off _is_covered_*() helper functions

Christian Boltz apparmor at cboltz.de
Sun Dec 20 23:45:18 UTC 2015


Hello,

is_covered_localvars() in the rule classes need the same set of checks
again and again. This patch adds the helper functions _is_covered_list(),
_is_covered_aare() and _is_covered_plain() to check against lists, AARE
and plain variables like str.

The helpers check if the values from the other rule are valid (either
ALL or the value need to be set) and then checks if the value is covered
by the other rule's values.

This results in replacing 7 lines with 2 in the rule classes and avoids
repeating code over and over.

Note that the helper functions depend on the *Rule.rule_name variable in
the exception message, therefore rule_name gets added to all rule
classes.


[ 40-split-off-_is_covered-helpers.diff ]                                                                                                                                                                                                                                      
                                                                                                                                                                                                                                                                               
=== modified file ./utils/apparmor/rule/__init__.py                                                                                                                                                                                                                            
--- utils/apparmor/rule/__init__.py     2015-12-21 00:01:14.420513715 +0100                                                                                                                                                                                                    
+++ utils/apparmor/rule/__init__.py     2015-12-21 00:01:52.320279789 +0100                                                                                                                                                                                                    
@@ -152,6 +152,51 @@                                                                                                                                                                                                                                                           
         '''check if the rule-specific parts of other_rule is covered by this rule object'''                                                                                                                                                                                   
         raise NotImplementedError("'%s' needs to implement is_covered_localvars(), but didn't" % (str(self)))                                                                                                                                                                 
                                                                                                                                                                                                                                                                               
+    def _is_covered_plain(self, self_value, self_all, other_value, other_all, cond_name):                                                                                                                                                                                     
+        '''check if other_* is covered by self_* - for plain str, int etc.'''                                                                                                                                                                                                 
+                                                                                                                                                                                                                                                                              
+        if not other_value and not other_all:                                                                                                                                                                                                                                 
+            raise AppArmorBug('No %(cond_name)s specified in other %(rule_name)s rule' % {'cond_name': cond_name, 'rule_name': self.rule_name})                                                                                                                               
+                                                                                                                                                                                                                                                                              
+        if not self_all:
+            if other_all:
+                return False
+            if self_value != other_value:
+                return False
+
+        # still here? -> then it is covered
+        return True
+
+    def _is_covered_list(self, self_value, self_all, other_value, other_all, cond_name):
+        '''check if other_* is covered by self_* - for lists'''
+
+        if not other_value and not other_all:
+            raise AppArmorBug('No %(cond_name)s specified in other %(rule_name)s rule' % {'cond_name': cond_name, 'rule_name': self.rule_name})
+
+        if not self_all:
+            if other_all:
+                return False
+            if not other_value.issubset(self_value):
+                return False
+
+        # still here? -> then it is covered
+        return True
+
+    def _is_covered_aare(self, self_value, self_all, other_value, other_all, cond_name):
+        '''check if other_* is covered by self_* - for AARE'''
+
+        if not other_value and not other_all:
+            raise AppArmorBug('No %(cond_name)s specified in other %(rule_name)s rule' % {'cond_name': cond_name, 'rule_name': self.rule_name})
+
+        if not self_all:
+            if other_all:
+                return False
+            if not self_value.match(other_value.regex):  # XXX should check against other_value (without .regex) - but that gives different (more strict) results
+                return False
+
+        # still here? -> then it is covered
+        return True
+
     def is_equal(self, rule_obj, strict=False):
         '''compare if rule_obj == self
            Calls is_equal_localvars() to compare rule-specific variables'''
=== modified file ./utils/apparmor/rule/capability.py
--- utils/apparmor/rule/capability.py   2015-12-12 13:34:40.545997223 +0100
+++ utils/apparmor/rule/capability.py   2015-12-20 23:16:28.161336267 +0100
@@ -33,6 +33,8 @@
 
     ALL = __CapabilityAll
 
+    rule_name = 'capability'
+
     def __init__(self, cap_list, audit=False, deny=False, allow_keyword=False,
                  comment='', log_event=None):
 
@@ -101,14 +103,8 @@
     def is_covered_localvars(self, other_rule):
         '''check if other_rule is covered by this rule object'''
 
-        if not other_rule.capability and not other_rule.all_caps:
-            raise AppArmorBug('No capability specified')
-
-        if not self.all_caps:
-            if other_rule.all_caps:
-                return False
-            if not other_rule.capability.issubset(self.capability):
-                return False
+        if not self._is_covered_list(self.capability, self.all_caps, other_rule.capability, other_rule.all_caps, 'capability'):
+            return False
 
         # still here? -> then it is covered
         return True
=== modified file 'utils/apparmor/rule/change_profile.py'
--- utils/apparmor/rule/change_profile.py       2015-12-17 22:33:36 +0000
+++ utils/apparmor/rule/change_profile.py       2015-12-20 23:26:32 +0000
@@ -32,6 +32,8 @@
 
     ALL = __ChangeProfileAll
 
+    rule_name = 'change_profile'
+
     def __init__(self, execcond, targetprofile, audit=False, deny=False, allow_keyword=False,
                  comment='', log_event=None):
 
@@ -121,24 +123,12 @@
     def is_covered_localvars(self, other_rule):
         '''check if other_rule is covered by this rule object'''
 
-        if not other_rule.execcond and not other_rule.all_execconds:
-            raise AppArmorBug('No execcond specified in other change_profile rule')
-
-        if not other_rule.targetprofile and not other_rule.all_targetprofiles:
-            raise AppArmorBug('No target profile specified in other change_profile rule')
-
-        if not self.all_execconds:
-            if other_rule.all_execconds:
-                return False
-            if other_rule.execcond != self.execcond:
-                # TODO: honor globbing and variables
-                return False
-
-        if not self.all_targetprofiles:
-            if other_rule.all_targetprofiles:
-                return False
-            if other_rule.targetprofile != self.targetprofile:
-                return False
+        if not self._is_covered_plain(self.execcond, self.all_execconds, other_rule.execcond, other_rule.all_execconds, 'exec condition'):
+            # TODO: honor globbing and variables
+            return False
+
+        if not self._is_covered_plain(self.targetprofile, self.all_targetprofiles, other_rule.targetprofile, other_rule.all_targetprofiles, 'target profile'):
+            return False
 
         # still here? -> then it is covered
         return True
=== modified file ./utils/apparmor/rule/network.py
--- utils/apparmor/rule/network.py      2015-12-12 13:34:40.545997223 +0100
+++ utils/apparmor/rule/network.py      2015-12-20 23:23:23.990712992 +0100
@@ -54,6 +54,8 @@
 
     ALL = __NetworkAll
 
+    rule_name = 'network'
+
     def __init__(self, domain, type_or_protocol, audit=False, deny=False, allow_keyword=False,
                  comment='', log_event=None):
 
@@ -151,23 +153,11 @@
     def is_covered_localvars(self, other_rule):
         '''check if other_rule is covered by this rule object'''
 
-        if not other_rule.domain and not other_rule.all_domains:
-            raise AppArmorBug('No domain specified in other network rule')
-
-        if not other_rule.type_or_protocol and not other_rule.all_type_or_protocols:
-            raise AppArmorBug('No type or protocol specified in other network rule')
+        if not self._is_covered_plain(self.domain, self.all_domains, other_rule.domain, other_rule.all_domains, 'domain'):
+            return False
 
-        if not self.all_domains:
-            if other_rule.all_domains:
-                return False
-            if other_rule.domain != self.domain:
-                return False
-
-        if not self.all_type_or_protocols:
-            if other_rule.all_type_or_protocols:
-                return False
-            if other_rule.type_or_protocol != self.type_or_protocol:
-                return False
+        if not self._is_covered_plain(self.type_or_protocol, self.all_type_or_protocols, other_rule.type_or_protocol, other_rule.all_type_or_protocols, 'type or protocol'):
+            return False
 
         # still here? -> then it is covered
         return True
=== modified file ./utils/apparmor/rule/ptrace.py
--- utils/apparmor/rule/ptrace.py       2015-12-21 00:01:14.420513715 +0100
+++ utils/apparmor/rule/ptrace.py       2015-12-21 00:01:52.320279789 +0100
@@ -50,6 +50,8 @@
 
     ALL = __PtraceAll
 
+    rule_name = 'ptrace'
+
     def __init__(self, access, peer, audit=False, deny=False, allow_keyword=False,
                  comment='', log_event=None):
 
@@ -133,23 +135,11 @@
     def is_covered_localvars(self, other_rule):
         '''check if other_rule is covered by this rule object'''
 
-        if not other_rule.access and not other_rule.all_access:
-            raise AppArmorBug('No access specified in other ptrace rule')
+        if not self._is_covered_plain(self.access, self.all_access, other_rule.access, other_rule.all_access, 'access'):
+            return False
 
-        if not other_rule.peer and not other_rule.all_peers:
-            raise AppArmorBug('No peer specified in other ptrace rule')
-
-        if not self.all_access:
-            if other_rule.all_access:
-                return False
-            if other_rule.access != self.access:
-                return False
-
-        if not self.all_peers:
-            if other_rule.all_peers:
-                return False
-            if not self.peer.match(other_rule.peer.regex):
-                return False
+        if not self._is_covered_aare(self.peer, self.all_peers, other_rule.peer, other_rule.all_peers, 'peer'):
+            return False
 
         # still here? -> then it is covered
         return True
=== modified file ./utils/apparmor/rule/rlimit.py
--- utils/apparmor/rule/rlimit.py       2015-12-12 13:34:40.545997223 +0100
+++ utils/apparmor/rule/rlimit.py       2015-12-21 00:04:57.871134048 +0100
@@ -46,6 +46,8 @@
 
     ALL = __RlimitAll
 
+    rule_name = 'rlimit'
+
     def __init__(self, rlimit, value, audit=False, deny=False, allow_keyword=False,
                  comment='', log_event=None):
 
@@ -201,15 +203,12 @@
     def is_covered_localvars(self, other_rule):
         '''check if other_rule is covered by this rule object'''
 
-        if not other_rule.rlimit:
-            raise AppArmorBug('No rlimit specified in other rlimit rule')
+        if not self._is_covered_plain(self.rlimit, False, other_rule.rlimit, False, 'rlimit'):  # rlimit can't be ALL, therefore using False
+            return False
 
         if not other_rule.value and not other_rule.all_values:
             raise AppArmorBug('No target profile specified in other rlimit rule')
 
-        if other_rule.rlimit != self.rlimit:
-            return False
-
         if not self.all_values:
             if other_rule.all_values:
                 return False
=== modified file ./utils/apparmor/rule/signal.py
--- utils/apparmor/rule/signal.py       2015-12-21 00:01:14.420513715 +0100
+++ utils/apparmor/rule/signal.py       2015-12-21 00:01:52.320279789 +0100
@@ -71,6 +71,8 @@
 
     ALL = __SignalAll
 
+    rule_name = 'signal'
+
     def __init__(self, access, signal, peer, audit=False, deny=False, allow_keyword=False,
                  comment='', log_event=None):
 
@@ -180,33 +182,15 @@
     def is_covered_localvars(self, other_rule):
         '''check if other_rule is covered by this rule object'''
 
-        if not other_rule.access and not other_rule.all_access:
-            raise AppArmorBug('No access specified in other signal rule')
-
-        if not other_rule.signal and not other_rule.all_signals:
-            raise AppArmorBug('No signal specified in other signal rule')
+        if not self._is_covered_plain(self.access, self.all_access, other_rule.access, other_rule.all_access, 'access'):
+            return False
 
-        if not other_rule.peer and not other_rule.all_peers:
-            raise AppArmorBug('No peer specified in other signal rule')
+        if not self._is_covered_plain(self.signal, self.all_signals, other_rule.signal, other_rule.all_signals, 'signal'):
+            return False
 
-        if not self.all_access:
-            if other_rule.all_access:
-                return False
-            if other_rule.access != self.access:
-                return False
-
-        if not self.all_signals:
-            if other_rule.all_signals:
-                return False
-            if other_rule.signal != self.signal:
-                return False
-
-        if not self.all_peers:
-            if other_rule.all_peers:
-                return False
-            if not self.peer.match(other_rule.peer.regex):
-                return False
+        if not self._is_covered_aare(self.peer, self.all_peers, other_rule.peer, other_rule.all_peers, 'peer'):
+            return False
 
         # still here? -> then it is covered
         return True


Regards,

Christian Boltz
-- 
I have the ideal solution for you to speed up the writing of
the manuals: http://www.lipsum.com/ - I am sure almost nobody
will notice the difference. ;-)   [houghi in opensuse-wiki]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20151221/b8d54bd5/attachment-0001.pgp>


More information about the AppArmor mailing list