diff --git a/utils/apparmor/aa.py b/utils/apparmor/aa.py index 261254357..f349ee674 100644 --- a/utils/apparmor/aa.py +++ b/utils/apparmor/aa.py @@ -649,49 +649,39 @@ def change_profile_flags(filename, program, flag, set_flag): def set_profile_flags(prof_filename, program, newflags): """Reads the old profile file and updates the flags accordingly""" - regex_bin_flag = re.compile('^(\s*)("?(/.+?)"??|(profile\s+"?(.+?)"??))\s+((flags=)?\((.*)\)\s+)?\{\s*(#.*)?$') - # TODO: use RE_PROFILE_START (only difference: doesn't have a match group for the leading space) - # TODO: also use the global regex for matching the hat # 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 - regex_hat_flag = re.compile('^([a-z]*)\s+([A-Z]*)\s*(#.*)?$') - if os.path.isfile(prof_filename): - with open_file_read(prof_filename) as f_in: - temp_file = tempfile.NamedTemporaryFile('w', prefix=prof_filename, suffix='~', delete=False, dir=profile_dir) - shutil.copymode(prof_filename, temp_file.name) - with open_file_write(temp_file.name) as f_out: - for line in f_in: - comment = '' - if '#' in line: - comment = '#' + line.split('#', 1)[1].rstrip() - match = regex_bin_flag.search(line) - if not line.strip() or line.strip().startswith('#'): - pass - elif match: - matches = match.groups() - space = matches[0] - profile = matches[1] # profile name including quotes and "profile" keyword - if matches[2]: - binary = matches[2] - else: - binary = matches[4] - flag = matches[6] or 'flags=' - flags = matches[7] - if binary == program or program is None: - if newflags: - line = '%s%s %s(%s) {%s\n' % (space, profile, flag, newflags, comment) - else: - line = '%s%s {%s\n' % (space, profile, comment) - 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) - f_out.write(line) - os.rename(temp_file.name, prof_filename) + # 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*(#.*)?$') + with open_file_read(prof_filename) as f_in: + temp_file = tempfile.NamedTemporaryFile('w', prefix=prof_filename, suffix='~', delete=False, dir=profile_dir) + shutil.copymode(prof_filename, temp_file.name) + with open_file_write(temp_file.name) as f_out: + for line in f_in: + if RE_PROFILE_START.search(line): + matches = parse_profile_start_line(line, prof_filename) + space = matches['leadingspace'] or '' + profile = matches['profile'] + + if profile == program or program is None: + header_data = { + 'attachment': matches['attachment'] or '', + 'flags': newflags, + 'profile_keyword': matches['profile_keyword'], + 'header_comment': matches['comment'] or '', + } + 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) + f_out.write(line) + os.rename(temp_file.name, prof_filename) 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 5e90d78a9..5018fe875 100644 --- a/utils/test/test-aa.py +++ b/utils/test/test-aa.py @@ -119,6 +119,9 @@ class AaTest_set_profile_flags(AaTestWithTempdir): else: expected_flags = '' + 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) @@ -150,10 +153,8 @@ class AaTest_set_profile_flags(AaTestWithTempdir): def test_set_flags_nochange_09(self): self._test_set_flags('profile xy /foo', 'flags=(complain)', 'complain', profile_name='xy /foo') # XXX profile_name should be 'xy' def test_set_flags_nochange_10(self): - self._test_set_flags('profile "/foo"', 'flags=(complain)', 'complain') # superfluous quotes are kept - def test_set_flags_nochange_11(self): self._test_set_flags('profile "/foo bar"', 'flags=(complain)', 'complain', profile_name='/foo bar') - #def test_set_flags_nochange_12(self): + #def test_set_flags_nochange_11(self): # XXX changes the flags for the child profile (which happens to have the same profile name) to 'complain' # self._test_set_flags('/foo', 'flags=(complain)', 'complain', more_rules=' profile /foo {\n}') @@ -177,11 +178,11 @@ class AaTest_set_profile_flags(AaTestWithTempdir): def test_set_flags_09(self): self._test_set_flags('profile /foo', 'flags=(complain)', 'audit') def test_set_flags_10(self): - self._test_set_flags('profile xy /foo', 'flags=(complain)', 'audit', profile_name='xy /foo') # XXX profile_name should be just 'xy' + self._test_set_flags('profile xy /foo', 'flags=(complain)', 'audit', profile_name='xy') def test_set_flags_11(self): self._test_set_flags('profile "/foo bar"', 'flags=(complain)', 'audit', profile_name='/foo bar') def test_set_flags_12(self): - self._test_set_flags('profile xy "/foo bar"', 'flags=(complain)', 'audit', profile_name='xy "/foo bar') # profile name should just be 'xy' + 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 {' @@ -190,16 +191,15 @@ class AaTest_set_profile_flags(AaTestWithTempdir): # self._test_set_flags(' ^hat', '', 'audit') - # XXX current code/regex doesn't check for empty flags - #def test_set_flags_invalid_01(self): - # with self.assertRaises(AppArmorBug): - # self._test_set_flags('/foo', '()', None, check_new_flags=False) - #def test_set_flags_invalid_02(self): - # with self.assertRaises(AppArmorBug): - # self._test_set_flags('/foo', 'flags=()', None, check_new_flags=False) - #def test_set_flags_invalid_03(self): - # with self.assertRaises(AppArmorException): - # self._test_set_flags('/foo', '( )', ' ', check_new_flags=False) + def test_set_flags_invalid_01(self): + with self.assertRaises(AppArmorBug): + self._test_set_flags('/foo', '()', None, check_new_flags=False) + def test_set_flags_invalid_02(self): + with self.assertRaises(AppArmorBug): + self._test_set_flags('/foo', 'flags=()', None, check_new_flags=False) + def test_set_flags_invalid_03(self): + with self.assertRaises(AppArmorException): + self._test_set_flags('/foo', '( )', ' ', check_new_flags=False) def test_set_flags_other_profile(self): # test behaviour if the file doesn't contain the specified /foo profile @@ -214,11 +214,8 @@ class AaTest_set_profile_flags(AaTestWithTempdir): self.assertEqual(orig_prof, real_new_prof) def test_set_flags_file_not_found(self): - # XXX this exits silently - # the easiest solution would be to drop the - # if os.path.isfile(prof_filename): - # check and let open_file_read raise an exception - set_profile_flags('%s/file-not-found' % self.tmpdir, '/foo', 'audit') + with self.assertRaises(IOError): + set_profile_flags('%s/file-not-found' % self.tmpdir, '/foo', 'audit') class AaTest_is_skippable_file(AATest):