mir/apparmor
2
0
Fork 0
mirror of https://gitlab.com/apparmor/apparmor synced 2025-08-29 13:28:19 +00:00
Code Issues Releases Wiki Activity

rewrite set_profile_flags() to use write_header()

Browse Source 
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.


Acked-by: Steve Beattie <steve@nxnw.org>
This commit is contained in:
Christian Boltz 2015-04-03 17:20:14 +02:00
parent b705af0221
commit 38b1e3d30f
2 changed files with 48 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

72
utils/apparmor/aa.py
View File

@ -649,49 +649,39 @@ def change_profile_flags(filename, program, flag, set_flag):
def set_profile_flags(prof_filename, program, newflags): def set_profile_flags(prof_filename, program, newflags):
"""Reads the old profile file and updates the flags accordingly""" """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 # 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 # 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*(#.*)?$') # TODO: use RE_PROFILE_HAT_DEF for matching the hat (regex_hat_flag is totally broken!)
if os.path.isfile(prof_filename): #regex_hat_flag = re.compile('^([a-z]*)\s+([A-Z]*)\s*(#.*)?$')
with open_file_read(prof_filename) as f_in: with open_file_read(prof_filename) as f_in:
temp_file = tempfile.NamedTemporaryFile('w', prefix=prof_filename, suffix='~', delete=False, dir=profile_dir) temp_file = tempfile.NamedTemporaryFile('w', prefix=prof_filename, suffix='~', delete=False, dir=profile_dir)
shutil.copymode(prof_filename, temp_file.name) shutil.copymode(prof_filename, temp_file.name)
with open_file_write(temp_file.name) as f_out: with open_file_write(temp_file.name) as f_out:
for line in f_in: for line in f_in:
comment = '' if RE_PROFILE_START.search(line):
if '#' in line: matches = parse_profile_start_line(line, prof_filename)
comment = '#' + line.split('#', 1)[1].rstrip() space = matches['leadingspace'] or ''
match = regex_bin_flag.search(line) profile = matches['profile']
if not line.strip() or line.strip().startswith('#'):
pass if profile == program or program is None:
elif match: header_data = {
matches = match.groups() 'attachment': matches['attachment'] or '',
space = matches[0] 'flags': newflags,
profile = matches[1] # profile name including quotes and "profile" keyword 'profile_keyword': matches['profile_keyword'],
if matches[2]: 'header_comment': matches['comment'] or '',
binary = matches[2] }
else: line = write_header(header_data, len(space)/2, profile, False, True)
binary = matches[4] line = '%s\n' % line[0]
flag = matches[6] or 'flags=' #else:
flags = matches[7] # match = regex_hat_flag.search(line)
if binary == program or program is None: # if match:
if newflags: # hat, flags = match.groups()[:2]
line = '%s%s %s(%s) {%s\n' % (space, profile, flag, newflags, comment) # if newflags:
else: # line = '%s flags=(%s) {%s\n' % (hat, newflags, comment)
line = '%s%s {%s\n' % (space, profile, comment) # else:
else: # line = '%s {%s\n' % (hat, comment)
match = regex_hat_flag.search(line) f_out.write(line)
if match: os.rename(temp_file.name, prof_filename)
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): def profile_exists(program):
"""Returns True if profile exists, False otherwise""" """Returns True if profile exists, False otherwise"""

37
utils/test/test-aa.py
View File

@ -119,6 +119,9 @@ class AaTest_set_profile_flags(AaTestWithTempdir):
else: else:
expected_flags = '' expected_flags = ''
if comment:
comment = ' %s' % comment
dummy_profile_content = ' #include <abstractions/base>\n capability chown,\n /bar r,' dummy_profile_content = ' #include <abstractions/base>\n capability chown,\n /bar r,'
prof_template = '%s%s%s {%s\n%s\n%s\n}\n' 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) 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): 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' 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): 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') 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' # 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}') # 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): def test_set_flags_09(self):
self._test_set_flags('profile /foo', 'flags=(complain)', 'audit') self._test_set_flags('profile /foo', 'flags=(complain)', 'audit')
def test_set_flags_10(self): 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): def test_set_flags_11(self):
self._test_set_flags('profile "/foo bar"', 'flags=(complain)', 'audit', profile_name='/foo bar') self._test_set_flags('profile "/foo bar"', 'flags=(complain)', 'audit', profile_name='/foo bar')
def test_set_flags_12(self): 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 {' # 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') # self._test_set_flags(' ^hat', '', 'audit')
# XXX current code/regex doesn't check for empty flags def test_set_flags_invalid_01(self):
#def test_set_flags_invalid_01(self): with self.assertRaises(AppArmorBug):
# with self.assertRaises(AppArmorBug): self._test_set_flags('/foo', '()', None, check_new_flags=False)
# self._test_set_flags('/foo', '()', None, check_new_flags=False) def test_set_flags_invalid_02(self):
#def test_set_flags_invalid_02(self): with self.assertRaises(AppArmorBug):
# with self.assertRaises(AppArmorBug): self._test_set_flags('/foo', 'flags=()', None, check_new_flags=False)
# self._test_set_flags('/foo', 'flags=()', None, check_new_flags=False) def test_set_flags_invalid_03(self):
#def test_set_flags_invalid_03(self): with self.assertRaises(AppArmorException):
# with self.assertRaises(AppArmorException): self._test_set_flags('/foo', '( )', ' ', check_new_flags=False)
# self._test_set_flags('/foo', '( )', ' ', check_new_flags=False)
def test_set_flags_other_profile(self): def test_set_flags_other_profile(self):
# test behaviour if the file doesn't contain the specified /foo profile # 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) self.assertEqual(orig_prof, real_new_prof)
def test_set_flags_file_not_found(self): def test_set_flags_file_not_found(self):
# XXX this exits silently with self.assertRaises(IOError):
# the easiest solution would be to drop the set_profile_flags('%s/file-not-found' % self.tmpdir, '/foo', 'audit')
# 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')
class AaTest_is_skippable_file(AATest): class AaTest_is_skippable_file(AATest):