On Mon, Nov 24, 2014 at 12:46:11PM -0800, Steve Beattie wrote:
> In python, a set() is very similar to a list, except that it guarantees
> uniqueness amongst its member elements (which means that there's an
> additional constraint that the elements have to be hashable). As a
> bonus, you also have common set operations available, like issubset(),
> issuperset(), union(), intersection(), etc. These may be really helpful
> for dealing with capability rules that contain multiple capabilities.
> 
> See https://docs.python.org/3/library/stdtypes.html#set for details.

Attached is a patch that converts from using a list to using a set(). It
doesn't take much advantage of that ability, other than to use
issubset() instead of manually walking the list and confirming that all
elements of one rule are in another. But more interesting stuff could be
done to delete a multi-capability subset from another superset
capability rule, if need be.

Signed-off-by: Steve Beattie <[email protected]>

-- 
Steve Beattie
<[email protected]>
http://NxNW.org/~steve/
---
 utils/apparmor/rule/capability.py |   13 ++++++-------
 utils/test/test-capability.py     |   20 ++++++++++----------
 2 files changed, 16 insertions(+), 17 deletions(-)

Index: b/utils/apparmor/rule/capability.py
===================================================================
--- a/utils/apparmor/rule/capability.py
+++ b/utils/apparmor/rule/capability.py
@@ -27,7 +27,7 @@ class CapabilityRule(BaseRule):
     '''Class to handle and store a single capability rule'''
 
     def init_vars(self):
-        self.capability = []  # XXX use {} instead to enforce in-rule duplicates ("capability chown chown,") are filtered?
+        self.capability = set()
         self.all_caps = False
 
     def get_clean(self, depth):
@@ -56,7 +56,7 @@ class CapabilityRule(BaseRule):
 
         if matches.group('capability'):
             capability = matches.group('capability').strip()
-            self.capability = re.split("[ \t]+", capability)
+            self.capability = set(re.split("[ \t]+", capability))
         else:
             self.all_caps = True
 
@@ -69,12 +69,12 @@ class CapabilityRule(BaseRule):
         if not parsed_log_event['name']:
             raise AppArmorBug('Invalid capability log event - name is empty')
 
-        self.capability = [ parsed_log_event['name'] ]
+        self.capability = set([parsed_log_event['name']])
 
         self.logmode = parsed_log_event['aamode']
 
     def set_param(self, capability, audit=False, deny=False):
-        self.capability = [ capability ]
+        self.capability = {capability}
         self.audit = audit
         self.deny = deny
 
@@ -90,9 +90,8 @@ class CapabilityRule(BaseRule):
         if not self.all_caps:
             if rule_obj.all_caps:
                 return False
-            for cap in rule_obj.capability:
-                if not cap in self.capability:
-                    return False
+            if not rule_obj.capability.issubset(self.capability):
+                return False
 
         if check_audit and rule_obj.audit != self.audit:
             return False
Index: b/utils/test/test-capability.py
===================================================================
--- a/utils/test/test-capability.py
+++ b/utils/test/test-capability.py
@@ -49,7 +49,7 @@ class CapabilityTest(unittest.TestCase):
             'allow_keyword':    False,
             'deny':             False,
             'audit':            False,
-            'capability':       [ ],
+            'capability':       set(),
             'all_caps':         True,
             'inlinecomment':    "",
         })
@@ -59,7 +59,7 @@ class CapabilityTest(unittest.TestCase):
             'allow_keyword':    False,
             'deny':             False,
             'audit':            False,
-            'capability':       [ 'sys_admin' ],
+            'capability':       {'sys_admin'},
             'all_caps':         False,
             'inlinecomment':    "",
         })
@@ -69,7 +69,7 @@ class CapabilityTest(unittest.TestCase):
             'allow_keyword':    False,
             'deny':             True,
             'audit':            False,
-            'capability':       [ 'sys_admin' ],
+            'capability':       {'sys_admin'},
             'all_caps':         False,
             'inlinecomment':    " # some comment",
         })
@@ -79,7 +79,7 @@ class CapabilityTest(unittest.TestCase):
             'allow_keyword':    False,
             'deny':             False,
             'audit':            False,
-            'capability':       [ 'sys_admin', 'dac_override' ],
+            'capability':       {'sys_admin', 'dac_override'},
             'all_caps':         False,
             'inlinecomment':    "",
         })
@@ -127,7 +127,7 @@ class CapabilityTest(unittest.TestCase):
             'allow_keyword':    False,
             'deny':             False,
             'audit':            False,
-            'capability':       ['net_raw'],
+            'capability':       {'net_raw'},
             'all_caps':         False,
             'inlinecomment':    "",
         })
@@ -172,7 +172,7 @@ class CapabilityTest(unittest.TestCase):
             'allow_keyword':    False,
             'deny':             False,
             'audit':            False,
-            'capability':       ['chown'],
+            'capability':       {'chown'},
             'all_caps':         False,
             'inlinecomment':    "",
         })
@@ -185,7 +185,7 @@ class CapabilityTest(unittest.TestCase):
             'allow_keyword':    False,
             'deny':             True,
             'audit':            True,
-            'capability':       ['chown'],
+            'capability':       {'chown'},
             'all_caps':         False,
             'inlinecomment':    "",
         })
@@ -214,7 +214,7 @@ class InvalidCapabilityTest(unittest.Tes
 
     def test_space_cap(self):
         obj = CapabilityRule()
-        obj.capability.append('  ')  # the whitespace capability ;-)
+        obj.set_param('  ')  # the whitespace capability ;-)
         with self.assertRaises(AppArmorBug):
             obj.get_clean(1)
 
@@ -359,10 +359,10 @@ class CapabilityCoveredTest(unittest.Tes
 
     def test_empty_init(self):
         obj = CapabilityRule()
-        obj.capability.append('sys_admin')
+        obj.set_param('sys_admin')
 
         obj2 = CapabilityRule()
-        obj2.capability.append('ptrace')
+        obj2.set_param('ptrace')
 
         self.assertFalse(self._is_covered(obj2, 'capability sys_admin,'))
         self.assertTrue(self._is_covered(obj2, 'capability ptrace,'))

Attachment: signature.asc
Description: Digital signature

-- 
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to