[apparmor] [patch] Let set_profile_flags() change the flags for all hats
Christian Boltz
apparmor at cboltz.de
Wed May 13 21:24:29 UTC 2015
Hello,
as discussed in the meeting yesterday, this patch lets
set_profile_flags() change the flags for all hats.
It did this in the old 2.8 code, but didn't in 2.9.x (first there was a
broken hat regex, then I commented out the hat handling to avoid
breakage cause by the broken regex).
This patch makes sure the hat flags get set when setting the flags for
the main profile.
Also change RE_PROFILE_HAT_DEF to use more named matches
(leadingwhitespace and hat_keyword). Luckily all code that uses the
regex uses named matches already, which means adding another (...) pair
doesn't hurt.
Finally adjust the tests:
- change _test_set_flags to accept another optional parameter
expected_more_rules (used to specify the expected hat definition)
- add tests for hats (with '^foobar' and 'hat foobar' syntax)
- add tests for child profiles, one of them commented out (see below)
Remaining known issues (also added as TODO notes):
- The hat and child profile flags are *overwritten* with the flags used
for the main profile. (That's well-known behaviour from 2.8 :-/ but we
have more flags now, which makes this more annoying.)
The correct behaviour would be to add or remove the specified flag,
while keeping other flags unchanged.
- Child profiles are not handled/changed if you specify the 'program'
parameter. This means:
- 'aa-complain smbldap-useradd' or 'aa-complain /usr/sbin/smbldap-useradd'
_will not_ change the flags for the nscd child profile
- 'aa-complain /etc/apparmor.d/usr.sbin.smbldap-useradd' _will_ change
the flags for the nscd child profile (and any other profile and
child profile in that file)
Even with those remaining issues (which need bigger changes in
set_profile_flags() and maybe also in the whole flags handling), the
patch improves things and fixes the regression from the 2.8 code.
I propose this patch for trunk and 2.9.
[ 05-set-profile-flags-hats.diff ]
=== modified file utils/apparmor/aa.py
--- utils/apparmor/aa.py 2015-05-13 22:50:16.026160403 +0200
+++ utils/apparmor/aa.py 2015-05-13 22:49:51.436636155 +0200
@@ -653,8 +653,9 @@
"""Reads the old profile file and updates the flags accordingly"""
# TODO: count the number of matching lines (separated by profile and hat?) and return it
# so that code calling this function can make sure to only report success if there was a match
- # TODO: use RE_PROFILE_HAT_DEF for matching the hat (regex_hat_flag is totally broken!)
- #regex_hat_flag = re.compile('^([a-z]*)\s+([A-Z]*)\s*(#.*)?$')
+ # TODO: existing (unrelated) flags of hats and child profiles are overwritten - ideally, we should
+ # keep them and only add or remove a given flag
+ # TODO: change child profile flags even if program is specified
found = False
@@ -681,14 +682,19 @@
}
line = write_header(header_data, len(space)/2, profile, False, True)
line = '%s\n' % line[0]
- #else:
- # match = regex_hat_flag.search(line)
- # if match:
- # hat, flags = match.groups()[:2]
- # if newflags:
- # line = '%s flags=(%s) {%s\n' % (hat, newflags, comment)
- # else:
- # line = '%s {%s\n' % (hat, comment)
+ elif RE_PROFILE_HAT_DEF.search(line):
+ matches = RE_PROFILE_HAT_DEF.search(line)
+ space = matches.group('leadingspace') or ''
+ hat_keyword = matches.group('hat_keyword')
+ hat = matches.group('hat')
+ comment = matches.group('comment') or ''
+ if comment:
+ comment = ' %s' % comment
+
+ if newflags:
+ line = '%s%s%s flags=(%s) {%s\n' % (space, hat_keyword, hat, newflags, comment)
+ else:
+ line = '%s%s%s {%s\n' % (space, hat_keyword, hat, comment)
f_out.write(line)
os.rename(temp_file.name, prof_filename)
=== modified file utils/apparmor/regex.py
--- utils/apparmor/regex.py 2015-05-13 22:50:16.028160283 +0200
+++ utils/apparmor/regex.py 2015-05-13 22:49:51.438636035 +0200
@@ -43,7 +43,7 @@
RE_PROFILE_PATH_ENTRY = re.compile(RE_AUDIT_DENY + RE_OWNER + '(file\s+)?([\"@/].*?)\s+(\S+)(\s+->\s*(.*?))?' + RE_COMMA_EOL)
RE_PROFILE_NETWORK = re.compile(RE_AUDIT_DENY + 'network(?P<details>\s+.*)?' + RE_COMMA_EOL)
RE_PROFILE_CHANGE_HAT = re.compile('^\s*\^(\"??.+?\"??)' + RE_COMMA_EOL)
-RE_PROFILE_HAT_DEF = re.compile('^\s*(\^|hat\s+)(?P<hat>\"??.+?\"??)\s+((flags=)?\((?P<flags>.+)\)\s+)*\{' + RE_EOL)
+RE_PROFILE_HAT_DEF = re.compile('^(?P<leadingspace>\s*)(?P<hat_keyword>\^|hat\s+)(?P<hat>\"??.+?\"??)\s+((flags=)?\((?P<flags>.+)\)\s+)*\{' + RE_EOL)
RE_PROFILE_DBUS = re.compile(RE_AUDIT_DENY + '(dbus\s*,|dbus\s+[^#]*\s*,)' + RE_EOL)
RE_PROFILE_MOUNT = re.compile(RE_AUDIT_DENY + '((mount|remount|umount|unmount)(\s+[^#]*)?\s*,)' + RE_EOL)
RE_PROFILE_SIGNAL = re.compile(RE_AUDIT_DENY + '(signal\s*,|signal\s+[^#]*\s*,)' + RE_EOL)
=== modified file utils/test/test-aa.py
--- utils/test/test-aa.py 2015-05-13 22:50:43.166510000 +0200
+++ utils/test/test-aa.py 2015-05-13 23:07:59.378110888 +0200
@@ -106,7 +106,8 @@
self._test_get_flags('/no-such-profile flags=(complain)', 'complain')
class AaTest_set_profile_flags(AaTestWithTempdir):
- def _test_set_flags(self, profile, old_flags, new_flags, whitespace='', comment='', more_rules='',
+ def _test_set_flags(self, profile, old_flags, new_flags, whitespace='', comment='',
+ more_rules='', expected_more_rules='@- at -@',
expected_flags='@- at -@', check_new_flags=True, profile_name='/foo'):
if old_flags:
old_flags = ' %s' % old_flags
@@ -119,13 +120,16 @@
else:
expected_flags = ''
+ if expected_more_rules == '@- at -@':
+ expected_more_rules = more_rules
+
if comment:
comment = ' %s' % comment
dummy_profile_content = ' #include <abstractions/base>\n capability chown,\n /bar r,'
prof_template = '%s%s%s {%s\n%s\n%s\n}\n'
- old_prof = prof_template % (whitespace, profile, old_flags, comment, more_rules, dummy_profile_content)
- new_prof = prof_template % (whitespace, profile, expected_flags, comment, more_rules, dummy_profile_content)
+ old_prof = prof_template % (whitespace, profile, old_flags, comment, more_rules, dummy_profile_content)
+ new_prof = prof_template % (whitespace, profile, expected_flags, comment, expected_more_rules, dummy_profile_content)
self.file = write_file(self.tmpdir, 'profile', old_prof)
set_profile_flags(self.file, profile_name, new_flags)
@@ -184,11 +188,46 @@
def test_set_flags_12(self):
self._test_set_flags('profile xy "/foo bar"', 'flags=(complain)', 'audit', profile_name='xy')
-
- # XXX regex_hat_flag in set_profile_flags() is totally broken - it matches for ' ' and ' X ', but doesn't match for ' ^foo {'
- # oh, it matches on a line like 'dbus' and changes it to 'dbus flags=(...)' if there's no leading whitespace (and no comma)
- #def test_set_flags_hat_01(self):
- # self._test_set_flags(' ^hat', '', 'audit')
+ # test handling of hat flags
+ def test_set_flags_with_hat_01(self):
+ self._test_set_flags('/foo', 'flags=(complain)', 'audit',
+ more_rules='\n ^foobar {\n}\n',
+ expected_more_rules='\n ^foobar flags=(audit) {\n}\n'
+ )
+
+ def test_set_flags_with_hat_02(self):
+ self._test_set_flags('/foo', 'flags=(complain)', 'audit',
+ profile_name=None,
+ more_rules='\n ^foobar {\n}\n',
+ expected_more_rules='\n ^foobar flags=(audit) {\n}\n'
+ )
+
+ def test_set_flags_with_hat_03(self):
+ self._test_set_flags('/foo', 'flags=(complain)', 'audit',
+ more_rules='\n^foobar (attach_disconnected) { # comment\n}\n', # XXX attach_disconnected will be lost!
+ expected_more_rules='\n^foobar flags=(audit) { # comment\n}\n'
+ )
+
+ def test_set_flags_with_hat_04(self):
+ self._test_set_flags('/foo', '', 'audit',
+ more_rules='\n hat foobar (attach_disconnected) { # comment\n}\n', # XXX attach_disconnected will be lost!
+ expected_more_rules='\n hat foobar flags=(audit) { # comment\n}\n'
+ )
+
+ # test handling of child profiles
+ def test_set_flags_with_child_01(self):
+ self._test_set_flags('/foo', 'flags=(complain)', 'audit',
+ profile_name=None,
+ more_rules='\n profile /bin/bar {\n}\n',
+ expected_more_rules='\n profile /bin/bar flags=(audit) {\n}\n'
+ )
+
+ #def test_set_flags_with_child_02(self):
+ # XXX child profile flags aren't changed if profile parameter is not None
+ #self._test_set_flags('/foo', 'flags=(complain)', 'audit',
+ # more_rules='\n profile /bin/bar {\n}\n',
+ # expected_more_rules='\n profile /bin/bar flags=(audit) {\n}\n'
+ #)
def test_set_flags_invalid_01(self):
Regards,
Christian Boltz
--
Es kommt mir (auch wegen der zahlreichen PMs) so vor als ob ich der
einzige bin der das noch nicht "gehört" hatte. Nächstes Mal wähle
ich gleich als Subject "Sag mal schnell einer das Passwort für xyz".
Besser als Brute-Force! [Rüdiger Meier in suse-linux]
More information about the AppArmor
mailing list