[apparmor] [patch 4/3] hide raw_rule within parse class method
Steve Beattie
steve at nxnw.org
Mon Dec 8 22:25:08 UTC 2014
On Wed, Dec 03, 2014 at 10:39:13AM -0800, Steve Beattie wrote:
> > For readabiliy, it might be a good idea to keep parse_capability() as a
> > separate function inside the CapabilityRule class and call it from
> > __init__.
>
> So, I started down this path, but I really didn't like how convoluted
> it was making the logic in __init__; it required verifying that one
> and only one of cap_list and raw_rule were passed and splitting up
> the parse_capability function a bit at the very least.
>
> I came up with another solution, for which I've attached a patch. I
> moved the parse() entry point to the base class, which then calls
> the intended-for-internal-consumption-only class method _parse(). I
> removed the raw_rule argument to both __init__ methods and haver
> BaseRule.parse() set raw_rule directly.
>
> Thus, the two intended entry points are __init__(), where you're
> expected to have already dissected whatever rule into digestible
> components *or* the Rule.parse() class method, where only a raw_rule
> is given. In this way, using the obvious methods of interacting
> with the CapabilityRule class, you can't have a mismatch between
> the stored capabilities and the raw_rule unless there's a bug in the
> parsing function *or* you've gone digging into the internals of the
> object.
>
> And if you do go around getting grotty with the class fields like
> raw_rule directly, (1) it's not recommended, and (2) you are expected
> to know what you're doing (e.g. you're a test script).
Christian, any comments on this?
I've updated the patch to add some comments to the base class about
what rule subclasses need to implement.
Signed-off-by: Steve Beattie <steve at nxnw.org>
---
utils/apparmor/rule/__init__.py | 44 ++++++++++++++++++++++++++++++++++----
utils/apparmor/rule/capability.py | 13 ++++-------
2 files changed, 45 insertions(+), 12 deletions(-)
Index: b/utils/apparmor/rule/__init__.py
===================================================================
--- a/utils/apparmor/rule/__init__.py
+++ b/utils/apparmor/rule/__init__.py
@@ -23,18 +23,44 @@ _ = init_translation()
class BaseRule(object):
'''Base class to handle and store a single rule'''
+ # type specific rules should inherit from this class.
+ # Methods that subclasses need to implement:
+ # __init__
+ # _parse(cls, raw_rule) (as a class method)
+ # - parses a raw rule and returns an object of the Rule subclass
+ # get_clean(depth)
+ # - return rule in clean format
+ # is_covered(self, other_rule)
+ # - check if other_rule is covered by this rule (i.e. is a
+ # subset of this rule's permissions)
+ # is_equal_localvars(self, other_rule)
+ # - equality check for the rule-specific fields
+
def __init__(self, audit=False, deny=False, allow_keyword=False,
- comment='', log_event=None, raw_rule=''):
+ comment='', log_event=None):
'''initialize variables needed by all rule types'''
self.audit = audit
self.deny = deny
self.allow_keyword = allow_keyword
-
self.comment = comment
-
- self.raw_rule = raw_rule.strip() if raw_rule else None
self.log_event = log_event
+ # Set only in the parse() class method
+ self.raw_rule = None
+
+ @classmethod
+ def parse(cls, raw_rule):
+ rule = cls._parse(raw_rule)
+ rule.raw_rule = raw_rule.strip()
+ return rule
+
+ # @abstractmethod FIXME - uncomment when python3 only
+ @classmethod
+ def _parse(cls, raw_rule):
+ '''returns a Rule object created from parsing the raw rule.
+ required to be implemented by subclasses; raise exception if not'''
+ raise AppArmorBug("'%s' needs to implement _parse(), but didn't" % (str(cls)))
+
def get_raw(self, depth=0):
'''return raw rule (with original formatting, and leading whitespace in the depth parameter)'''
if self.raw_rule:
@@ -58,6 +84,16 @@ class BaseRule(object):
return self.is_equal_localvars(rule_obj)
+ # @abstractmethod FIXME - uncomment when python3 only
+ def is_covered(self, other_rule, check_allow_deny=True, check_audit=False):
+ '''check if other_rule is covered by this rule object'''
+ raise AppArmorBug("'%s' needs to implement is_covered(), but didn't" % (str(cls)))
+
+ # @abstractmethod FIXME - uncomment when python3 only
+ def is_equal_localvars(self, other_rule):
+ '''compare if rule-specific variables are equal'''
+ raise AppArmorBug("'%s' needs to implement is_equal_localvars(), but didn't" % (str(cls)))
+
def modifiers_str(self):
'''return the allow/deny and audit keyword as string, including whitespace'''
Index: b/utils/apparmor/rule/capability.py
===================================================================
--- a/utils/apparmor/rule/capability.py
+++ b/utils/apparmor/rule/capability.py
@@ -35,13 +35,12 @@ class CapabilityRule(BaseRule):
ALL = __CapabilityAll
def __init__(self, cap_list, audit=False, deny=False, allow_keyword=False,
- comment='', log_event=None, raw_rule=None):
+ comment='', log_event=None):
super(CapabilityRule, self).__init__(audit=audit, deny=deny,
allow_keyword=allow_keyword,
comment=comment,
- log_event=log_event,
- raw_rule=raw_rule)
+ log_event=log_event)
# Because we support having multiple caps in one rule,
# initializer needs to accept a list of caps.
self.all_caps = False
@@ -61,8 +60,8 @@ class CapabilityRule(BaseRule):
if len(cap.strip()) == 0:
raise AppArmorBug('Passed empty capability to CapabilityRule: %s' % str(cap_list))
- @staticmethod
- def parse(raw_rule):
+ @classmethod
+ def _parse(cls, raw_rule):
return parse_capability(raw_rule)
def get_clean(self, depth=0):
@@ -133,8 +132,6 @@ def parse_capability(raw_rule):
if not matches:
raise AppArmorException(_("Invalid capability rule '%s'") % raw_rule)
- raw_rule = raw_rule.strip()
-
audit, deny, allow_keyword, comment = parse_modifiers(matches)
capability = []
@@ -147,4 +144,4 @@ def parse_capability(raw_rule):
return CapabilityRule(capability, audit=audit, deny=deny,
allow_keyword=allow_keyword,
- comment=comment, raw_rule=raw_rule)
+ comment=comment)
--
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20141208/6316e67d/attachment.pgp>
More information about the AppArmor
mailing list