diff --git a/utils/apparmor/aa.py b/utils/apparmor/aa.py index 58e68f7d3..b70b3f12b 100644 --- a/utils/apparmor/aa.py +++ b/utils/apparmor/aa.py @@ -624,17 +624,9 @@ def get_profile_flags(filename, program): raise AppArmorException(_('%s contains no profile') % filename) def change_profile_flags(prof_filename, program, flag, set_flag): - old_flags = get_profile_flags(prof_filename, program) - - newflags = add_or_remove_flag(old_flags, flag, set_flag) - - newflags = ', '.join(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: 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 @@ -651,6 +643,8 @@ def change_profile_flags(prof_filename, program, flag, set_flag): matches = parse_profile_start_line(line, prof_filename) space = matches['leadingspace'] or '' profile = matches['profile'] + old_flags = matches['flags'] + newflags = ', '.join(add_or_remove_flag(old_flags, flag, set_flag)) if (matches['attachment'] is not None): profile_glob = AARE(matches['attachment'], True) @@ -674,6 +668,8 @@ def change_profile_flags(prof_filename, program, flag, set_flag): space = matches.group('leadingspace') or '' hat_keyword = matches.group('hat_keyword') hat = matches.group('hat') + old_flags = matches['flags'] + newflags = ', '.join(add_or_remove_flag(old_flags, flag, set_flag)) comment = matches.group('comment') or '' if comment: comment = ' %s' % comment @@ -687,9 +683,9 @@ def change_profile_flags(prof_filename, program, flag, set_flag): if not found: if program is None: - raise AppArmorBug("%(file)s doesn't contain a valid profile (syntax error?)" % {'file': prof_filename}) + raise AppArmorException("%(file)s doesn't contain a valid profile (syntax error?)" % {'file': prof_filename}) else: - raise AppArmorBug("%(file)s doesn't contain a valid profile for %(profile)s (syntax error?)" % {'file': prof_filename, 'profile': program}) + raise AppArmorException("%(file)s doesn't contain a valid profile for %(profile)s (syntax error?)" % {'file': prof_filename, 'profile': program}) def profile_exists(program): """Returns True if profile exists, False otherwise""" diff --git a/utils/test/test-aa.py b/utils/test/test-aa.py index 8e3f16905..2bccae256 100644 --- a/utils/test/test-aa.py +++ b/utils/test/test-aa.py @@ -310,32 +310,32 @@ class AaTest_change_profile_flags(AaTestWithTempdir): def test_set_flags_with_hat_01(self): self._test_change_profile_flags('/foo', 'flags=(complain)', 'audit', True, 'audit, complain', more_rules='\n ^foobar {\n}\n', - expected_more_rules='\n ^foobar flags=(audit, complain) {\n}\n' # XXX complain should not be added to the child profile + expected_more_rules='\n ^foobar flags=(audit) {\n}\n' ) def test_change_profile_flags_with_hat_02(self): self._test_change_profile_flags('/foo', 'flags=(complain)', 'audit', False, 'complain', profile_name=None, more_rules='\n ^foobar flags=(audit) {\n}\n', - expected_more_rules='\n ^foobar flags=(complain) {\n}\n' # XXX complain should NOT be added to child profile + expected_more_rules='\n ^foobar {\n}\n' ) def test_change_profile_flags_with_hat_03(self): self._test_change_profile_flags('/foo', 'flags=(complain)', 'audit', True, 'audit, complain', - more_rules='\n^foobar (attach_disconnected) { # comment\n}\n', # XXX attach_disconnected will be lost! - expected_more_rules='\n^foobar flags=(audit, complain) { # comment\n}\n' # XXX complain should not be added + more_rules='\n^foobar (attach_disconnected) { # comment\n}\n', + expected_more_rules='\n^foobar flags=(attach_disconnected, audit) { # comment\n}\n' ) def test_change_profile_flags_with_hat_04(self): self._test_change_profile_flags('/foo', '', 'audit', True, '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' + more_rules='\n hat foobar (attach_disconnected) { # comment\n}\n', + expected_more_rules='\n hat foobar flags=(attach_disconnected, audit) { # comment\n}\n' ) def test_change_profile_flags_with_hat_05(self): self._test_change_profile_flags('/foo', '(audit)', 'audit', False, '', - more_rules='\n hat foobar (attach_disconnected) { # comment\n}\n', # XXX attach_disconnected will be lost - expected_more_rules='\n hat foobar { # comment\n}\n' + more_rules='\n hat foobar (attach_disconnected) { # comment\n}\n', + expected_more_rules='\n hat foobar flags=(attach_disconnected) { # comment\n}\n' ) # test handling of child profiles @@ -343,7 +343,7 @@ class AaTest_change_profile_flags(AaTestWithTempdir): self._test_change_profile_flags('/foo', 'flags=(complain)', 'audit', True, 'audit, complain', profile_name=None, more_rules='\n profile /bin/bar {\n}\n', - expected_more_rules='\n profile /bin/bar flags=(audit, complain) {\n}\n' # XXX complain should not be added + expected_more_rules='\n profile /bin/bar flags=(audit) {\n}\n' ) def test_change_profile_flags_with_child_02(self): @@ -355,16 +355,13 @@ class AaTest_change_profile_flags(AaTestWithTempdir): def test_change_profile_flags_invalid_01(self): - with self.assertRaises(AppArmorException): - # XXX new flag 'None' should raise AppArmorBug + with self.assertRaises(AppArmorBug): self._test_change_profile_flags('/foo', '()', None, False, '', check_new_flags=False) def test_change_profile_flags_invalid_02(self): - with self.assertRaises(AppArmorException): - # XXX new flag 'None' should raise AppArmorBug + with self.assertRaises(AppArmorBug): self._test_change_profile_flags('/foo', 'flags=()', None, True, '', check_new_flags=False) def test_change_profile_flags_invalid_03(self): - with self.assertRaises(AppArmorException): - # XXX empty new flag should raise AppArmorBug + with self.assertRaises(AppArmorBug): self._test_change_profile_flags('/foo', '( )', '', True, '', check_new_flags=False) def test_change_profile_flags_invalid_04(self): with self.assertRaises(AppArmorBug):