Hello,
Am Dienstag, 25. November 2014 schrieb Steve Beattie:
> 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]>
Thanks for the patch!
I'll include it in my patch set with some small changes:
- I simplified self.capability = set([parsed_log_event['name']])
to self.capability = {parsed_log_event['name']}
- test_empty_init() is meant to make sure that obj and obj2 have
separate storage instances, which is also the reason why I used
obj.capability.add (which _adds_ to obj.capability) instead of
obj.set_param (which replaces obj.capability)
(I added a comment explaining the goal of the test)
- you missed to replace [ ] with { } in test_write_manually() ;-)
The updated patch is attached in case you want to check it.
Regards,
Christian Boltz
--
That depends. Opinions on this change about every week. During even
weeks, the "kernel driver makes the device useful" approach is chosen.
During odd weeks, the "we need userspace crap like usb_modeswitch to
make the device useful" approach is chosen.
[Stefan Seyfried in opensuse-factory]
diff -u -p -r v3-utils/apparmor/rule/capability.py utils/apparmor/rule/capability.py
--- v3-utils/apparmor/rule/capability.py 2014-11-23 00:31:35.379705888 +0100
+++ utils/apparmor/rule/capability.py 2014-11-26 00:02:28.392854443 +0100
@@ -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() # enforces in-rule deduplicates ("capability chown chown,")
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 = {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
diff -u -p -r v3-utils/test/test-capability.py utils/test/test-capability.py
--- v3-utils/test/test-capability.py 2014-11-23 00:22:18.084673035 +0100
+++ utils/test/test-capability.py 2014-11-25 23:53:30.004319684 +0100
@@ -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': "",
})
@@ -90,7 +90,7 @@ class CapabilityTest(unittest.TestCase):
# 'allow_keyword': False,
# 'deny': False,
# 'audit': False,
- # 'capability': [],
+ # 'capability': {},
# '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)
@@ -252,7 +252,7 @@ class WriteCapabilityTest(unittest.TestC
def test_write_manually(self):
obj = CapabilityRule()
- obj.capability = ['ptrace', 'audit_write']
+ obj.capability = {'ptrace', 'audit_write'}
obj.allow_keyword = True
expected = ' allow capability audit_write ptrace,'
@@ -358,11 +358,12 @@ class CapabilityCoveredTest(unittest.Tes
obj.is_covered(testobj)
def test_empty_init(self):
+ # add to internal set instead of using .set_* (which overwrites the internal set) to make sure obj and obj2 use separate storage
obj = CapabilityRule()
- obj.capability.append('sys_admin')
+ obj.capability.add('sys_admin')
obj2 = CapabilityRule()
- obj2.capability.append('ptrace')
+ obj2.capability.add('ptrace')
self.assertFalse(self._is_covered(obj2, 'capability sys_admin,'))
self.assertTrue(self._is_covered(obj2, 'capability ptrace,'))
--
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
