[apparmor] [1/4] Add ChangeProfileRule and ChangeProfileRuleset classes
Christian Boltz
apparmor at cboltz.de
Thu May 28 19:51:31 UTC 2015
Hello,
Am Mittwoch, 27. Mai 2015 schrieb Steve Beattie:
> On Sat, May 09, 2015 at 10:35:54PM +0200, Christian Boltz wrote:
> > this patch adds utils/apparmor/rule/change_profile.py with the
> > ChangeProfileRule and ChangeProfileRuleset classes. These classes
> > are meant to handle change_profile rules.
> >
> > In comparison to the current code in aa.py, ChangeProfileRule has
> > some added features:
> > - support for audit and allow/deny keywords (for which John promised
> > a parser patch really soon)
> >
> > - support for change_profile rules with an exec condition
> >
> > Also add the improved regex RE_PROFILE_CHANGE_PROFILE_2 to regex.py.
> >
> >
> > [ 01-add-ChangeProfileRule.diff ]
...
> > === modified file utils/apparmor/rule/change_profile.py
...
> > + self.execcond = None
> > + self.all_execconds = False
> > + if execcond == ChangeProfileRule.ALL:
> > + self.all_execconds = True
> > + elif type(execcond) == str:
> > + if execcond.strip():
> > + if execcond.startswith('/'):
> > + self.execcond = execcond
> > + else:
> > + raise AppArmorException('Exec condition in
> > change_profile rule does not start with /: %s' % str(execcond))
> This is problematic in the situation where paths begin with an @{}
> variable (e.g. 'change_profile @{MY_PROGRAM} -> my_program_profile,').
We can prepare for variables nevertheless ;-)
For the moment, I added an ... or execcond.startswith('@')
On the long term, maybe a regex check would be better - but that's
something for a follow-up patch.
Given that there are variables, *, **, ?, [...] and {...} (did I miss
something?), maybe we'll even end up with a separate class for paths
that can be used in various rule types.
Oh, and we'll need to pass in the variable content and aliases to
is_covered() and is_equal(). Nice[tm].
> Granted, the parser doesn't currently accept this syntax, so...
AFAIK it doesn't accept any execcond yet (John, what's the status of
that patch?)
> > + else:
> > + raise AppArmorBug('Empty exec condition in
> > change_profile rule') + else:
> > + raise AppArmorBug('Passed unknown object to
> > ChangeProfileRule: %s' % str(execcond))
> That said, I also agree with Seth's comment, it might be nice to
> restructure the if else clauses like so:
[...quoting of overlong lines destroyed by KMail...]
> Otherwise, this is looking okay.
:-)
Here's the updated patch - changes are
- s/network/change_profile/ in a comment
- the if conditions discussed above changed to avoid a nesting level:
+ if execcond == ChangeProfileRule.ALL:
+ self.all_execconds = True
+ elif type(execcond) == str:
+ if not execcond.strip():
+ raise AppArmorBug('Empty exec condition in change_profile rule')
+ elif execcond.startswith('/') or execcond.startswith('@'):
+ self.execcond = execcond
+ else:
+ raise AppArmorException('Exec condition in change_profile rule does not start with /: %s' % str(execcond))
+ else:
+ raise AppArmorBug('Passed unknown object to ChangeProfileRule: %s' % str(execcond))
I know assigning self.execcond is still inside the elif block, but IMHO
that's more readable than lots of 'not' condtions - especially with
two or'ed conditions (which would become 'and not').
I can (and probably will) change that when we have some common code or
even a class for handling paths.
[ 01-add-ChangeProfileRule.diff ]
=== modified file utils/apparmor/regex.py
--- utils/apparmor/regex.py 2015-05-28 21:06:08.080116292 +0200
+++ utils/apparmor/regex.py 2015-05-28 21:07:16.476136726 +0200
@@ -72,6 +72,16 @@
'\s+((flags=)?\((?P<flags>.+)\)\s+)?\{' +
RE_EOL)
+
+RE_PROFILE_CHANGE_PROFILE_2 = re.compile(
+ RE_AUDIT_DENY +
+ 'change_profile' +
+ '(\s+' + RE_PROFILE_PATH % 'execcond' + ')?' + # optionally exec condition
+ '(\s+->\s*' + RE_PROFILE_NAME % 'targetprofile' + ')?' + # optionally '->' target profile
+ RE_COMMA_EOL)
+
+
+
def parse_profile_start_line(line, filename):
matches = RE_PROFILE_START.search(line)
=== modified file utils/apparmor/rule/change_profile.py
--- utils/apparmor/rule/change_profile.py 2015-05-28 21:06:08.080116292 +0200
+++ utils/apparmor/rule/change_profile.py 2015-05-28 21:12:42.794156021 +0200
@@ -0,0 +1,172 @@
+#!/usr/bin/env python
+# ----------------------------------------------------------------------
+# Copyright (C) 2013 Kshitij Gupta <kgupta8592 at gmail.com>
+# Copyright (C) 2015 Christian Boltz <apparmor at cboltz.de>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of version 2 of the GNU General Public
+# License as published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# ----------------------------------------------------------------------
+
+from apparmor.regex import RE_PROFILE_CHANGE_PROFILE_2, strip_quotes
+from apparmor.common import AppArmorBug, AppArmorException
+from apparmor.rule import BaseRule, BaseRuleset, parse_modifiers, quote_if_needed
+
+# setup module translations
+from apparmor.translations import init_translation
+_ = init_translation()
+
+
+class ChangeProfileRule(BaseRule):
+ '''Class to handle and store a single change_profile rule'''
+
+ # Nothing external should reference this class, all external users
+ # should reference the class field ChangeProfileRule.ALL
+ class __ChangeProfileAll(object):
+ pass
+
+ ALL = __ChangeProfileAll
+
+ def __init__(self, execcond, targetprofile, audit=False, deny=False, allow_keyword=False,
+ comment='', log_event=None):
+
+ '''
+ CHANGE_PROFILE RULE = 'change_profile' [ EXEC COND ] [ -> PROGRAMCHILD ]
+ '''
+
+ super(ChangeProfileRule, self).__init__(audit=audit, deny=deny,
+ allow_keyword=allow_keyword,
+ comment=comment,
+ log_event=log_event)
+
+ self.execcond = None
+ self.all_execconds = False
+ if execcond == ChangeProfileRule.ALL:
+ self.all_execconds = True
+ elif type(execcond) == str:
+ if not execcond.strip():
+ raise AppArmorBug('Empty exec condition in change_profile rule')
+ elif execcond.startswith('/') or execcond.startswith('@'):
+ self.execcond = execcond
+ else:
+ raise AppArmorException('Exec condition in change_profile rule does not start with /: %s' % str(execcond))
+ else:
+ raise AppArmorBug('Passed unknown object to ChangeProfileRule: %s' % str(execcond))
+
+ self.targetprofile = None
+ self.all_targetprofiles = False
+ if targetprofile == ChangeProfileRule.ALL:
+ self.all_targetprofiles = True
+ elif type(targetprofile) == str:
+ if targetprofile.strip():
+ self.targetprofile = targetprofile
+ else:
+ raise AppArmorBug('Empty target profile in change_profile rule')
+ else:
+ raise AppArmorBug('Passed unknown object to ChangeProfileRule: %s' % str(targetprofile))
+
+ @classmethod
+ def _match(cls, raw_rule):
+ return RE_PROFILE_CHANGE_PROFILE_2.search(raw_rule)
+
+ @classmethod
+ def _parse(cls, raw_rule):
+ '''parse raw_rule and return ChangeProfileRule'''
+
+ matches = cls._match(raw_rule)
+ if not matches:
+ raise AppArmorException(_("Invalid change_profile rule '%s'") % raw_rule)
+
+ audit, deny, allow_keyword, comment = parse_modifiers(matches)
+
+ if matches.group('execcond'):
+ execcond = strip_quotes(matches.group('execcond'))
+ else:
+ execcond = ChangeProfileRule.ALL
+
+ if matches.group('targetprofile'):
+ targetprofile = strip_quotes(matches.group('targetprofile'))
+ else:
+ targetprofile = ChangeProfileRule.ALL
+
+ return ChangeProfileRule(execcond, targetprofile,
+ audit=audit, deny=deny, allow_keyword=allow_keyword, comment=comment)
+
+ def get_clean(self, depth=0):
+ '''return rule (in clean/default formatting)'''
+
+ space = ' ' * depth
+
+ if self.all_execconds:
+ execcond = ''
+ elif self.execcond:
+ execcond = ' %s' % quote_if_needed(self.execcond)
+ else:
+ raise AppArmorBug('Empty execcond in change_profile rule')
+
+ if self.all_targetprofiles:
+ targetprofile = ''
+ elif self.targetprofile:
+ targetprofile = ' -> %s' % quote_if_needed(self.targetprofile)
+ else:
+ raise AppArmorBug('Empty target profile in change_profile rule')
+
+ return('%s%schange_profile%s%s,%s' % (space, self.modifiers_str(), execcond, targetprofile, self.comment))
+
+ 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
+
+ # still here? -> then it is covered
+ return True
+
+ def is_equal_localvars(self, rule_obj):
+ '''compare if rule-specific variables are equal'''
+
+ if not type(rule_obj) == ChangeProfileRule:
+ raise AppArmorBug('Passed non-change_profile rule: %s' % str(rule_obj))
+
+ if (self.execcond != rule_obj.execcond
+ or self.all_execconds != rule_obj.all_execconds):
+ return False
+
+ if (self.targetprofile != rule_obj.targetprofile
+ or self.all_targetprofiles != rule_obj.all_targetprofiles):
+ return False
+
+ return True
+
+
+class ChangeProfileRuleset(BaseRuleset):
+ '''Class to handle and store a collection of change_profile rules'''
+
+ def get_glob(self, path_or_rule):
+ '''Return the next possible glob. For change_profile rules, that can be "change_profile EXECCOND,",
+ "change_profile -> TARGET_PROFILE," or "change_profile," (all change_profile).
+ Also, EXECCOND filename can be globbed'''
+ # XXX implement all options mentioned above ;-)
+ return 'change_profile,'
Regards,
Christian Boltz
--
BUGS
It is not yet possible to change operating system by writing
to /proc/sys/kernel/ostype. -- Linux sysctl(2) manpage
More information about the AppArmor
mailing list