diff --git a/utils/apparmor/aa.py b/utils/apparmor/aa.py index 36614ce8b..a774e2f43 100644 --- a/utils/apparmor/aa.py +++ b/utils/apparmor/aa.py @@ -652,8 +652,9 @@ def set_profile_flags(prof_filename, program, newflags): """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 @@ -680,14 +681,19 @@ def set_profile_flags(prof_filename, program, newflags): } 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) diff --git a/utils/apparmor/regex.py b/utils/apparmor/regex.py index d164b3664..2bf2ff278 100644 --- a/utils/apparmor/regex.py +++ b/utils/apparmor/regex.py @@ -44,7 +44,7 @@ RE_PROFILE_BARE_FILE_ENTRY = re.compile(RE_AUDIT_DENY + RE_OWNER + 'file' + RE_C 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
\s+.*)?' + RE_COMMA_EOL) RE_PROFILE_CHANGE_HAT = re.compile('^\s*\^(\"??.+?\"??)' + RE_COMMA_EOL) -RE_PROFILE_HAT_DEF = re.compile('^\s*(\^|hat\s+)(?P\"??.+?\"??)\s+((flags=)?\((?P.+)\)\s+)*\{' + RE_EOL) +RE_PROFILE_HAT_DEF = re.compile('^(?P\s*)(?P\^|hat\s+)(?P\"??.+?\"??)\s+((flags=)?\((?P.+)\)\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) diff --git a/utils/test/test-aa.py b/utils/test/test-aa.py index 9284806ac..f2234f9c9 100644 --- a/utils/test/test-aa.py +++ b/utils/test/test-aa.py @@ -106,7 +106,8 @@ class AaTest_get_profile_flags(AaTestWithTempdir): 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='@-@-@', expected_flags='@-@-@', check_new_flags=True, profile_name='/foo'): if old_flags: old_flags = ' %s' % old_flags @@ -119,13 +120,16 @@ class AaTest_set_profile_flags(AaTestWithTempdir): else: expected_flags = '' + if expected_more_rules == '@-@-@': + expected_more_rules = more_rules + if comment: comment = ' %s' % comment dummy_profile_content = ' #include \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 @@ class AaTest_set_profile_flags(AaTestWithTempdir): def test_set_flags_12(self): self._test_set_flags('profile xy "/foo bar"', 'flags=(complain)', 'audit', profile_name='xy') + # 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' + ) - # 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') + 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):