[apparmor] [patch] rewrite set_profile_flags() to use write_header()
Christian Boltz
apparmor at cboltz.de
Sun Mar 15 21:20:16 UTC 2015
Hallo,
Changes in set_profile_flags():
- rewrite set_profile_flags to use parse_profile_start_line() and
write_header().
- replace the silent failure for non-existing files with a proper
exception (using lazy programming - the check is done by removing the
"if os.path.isfile()" check, open_file_read then raises the
exception ;-)
- comment out regex_hat_flag and the code that was supposed to handle
hat flags, which were totally broken. We'll need another patch to fix
it, and we also need to decide if we want to do that because it
introduces a behaviour change (currently, aa-complain etc. don't
change hat flags).
The tests for set_profile_flags() are also updated:
- prepend a space to comments because write_header always adds a space
between '{' and the comment
- remove a test with superfluous quotes that are no longer kept (that's
just a profile cleanup, so dropping that test is the easiest way)
- update test_set_flags_10 and test_set_flags_12 to use the correct
profile name
- enable the tests for invalid (empty) flags
- update the test for a non-existing file
Note: test_set_flags_10, test_set_flags_12 and test_set_flags_nochange_09
will fail with this patch applied. The next patch will fix that.
[ 21-rewrite-set_profile_flags.diff ]
=== modified file utils/apparmor/aa.py
--- utils/apparmor/aa.py 2015-03-15 19:31:32.278118000 +0100
+++ utils/apparmor/aa.py 2015-03-15 19:38:01.605323373 +0100
@@ -649,49 +649,39 @@
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"""
=== modified file utils/test/test-aa.py
--- utils/test/test-aa.py 2015-03-15 19:31:32.278118000 +0100
+++ utils/test/test-aa.py 2015-03-15 19:38:56.158129654 +0100
@@ -119,6 +119,9 @@ class AaTest_set_profile_flags(AaTestWit
else:
expected_flags = ''
+ 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)
@@ -147,10 +150,8 @@ class AaTest_set_profile_flags(AaTestWit
self._test_set_flags('profile /foo', 'flags=(complain)', 'complain')
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):
+ def test_set_flags_nochange_10(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}')
@@ -178,11 +178,11 @@
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(AaTestWit
# 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(AaTestWit
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):
Regards,
Christian Boltz
--
+++ vcdimager.changes
+Thu Apr 7 15:49:40 UTC 2011 - jw[at]novell.com [...]
+- new copyright patch added, to fulfill Joerg's last wish in bnc#672491
More information about the AppArmor
mailing list