On Sun, Mar 15, 2015 at 11:55:35PM +0100, Christian Boltz wrote: > Hello, > > this patch makes set_profile_flags more strict: > - raise AppArmorBug if newflags contains only whitespace > - raise AppArmorBug if the file doesn't contain the specified profile or > no profile at all > > The tests are adjusted to expect AppArmorBug instead of a silent > failure. Also, some tests are added for profile=None, which means to > change the flags for all profiles in a file. > - test_set_flags_08 is now test_set_flags_invalid_04 > - test_set_flags_invalid_03 is changed to only contain one reason for a > failure, not two ;-) > > > [ 23-more-strict-set_profile_flags.diff ]
Acked-by: Steve Beattie <[email protected]>. > === modified file utils/apparmor/aa.py > --- utils/apparmor/aa.py 2015-03-15 23:46:02.005030993 +0100 > +++ utils/apparmor/aa.py 2015-03-15 23:17:33.917992932 +0100 > @@ -1,6 +1,6 @@ > # ---------------------------------------------------------------------- > # Copyright (C) 2013 Kshitij Gupta <[email protected]> > -# Copyright (C) 2014 Christian Boltz <[email protected]> > +# Copyright (C) 2014-2015 Christian Boltz <[email protected]> > # > # This program is free software; you can redistribute it and/or > # modify it under the terms of version 2 of the GNU General Public > @@ -31,7 +31,7 @@ > > from copy import deepcopy > > -from apparmor.common import (AppArmorException, open_file_read, valid_path, > hasher, > +from apparmor.common import (AppArmorException, AppArmorBug, open_file_read, > valid_path, hasher, > open_file_write, convert_regexp, DebugLogger) > > import apparmor.ui as aaui > @@ -653,6 +653,12 @@ > # 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*(#.*)?$') > + > + found = False > + > + if newflags and newflags.strip() == '': > + raise AppArmorBug('New flags for %s contain only whitespace' % > 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) > @@ -664,6 +670,7 @@ > profile = matches['profile'] > > if profile == program or program is None: > + found = True > header_data = { > 'attachment': matches['attachment'] or '', > 'flags': newflags, > @@ -683,6 +690,12 @@ > f_out.write(line) > os.rename(temp_file.name, prof_filename) > > + if not found: > + if program is None: > + raise AppArmorBug("%(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}) > + > def profile_exists(program): > """Returns True if profile exists, False otherwise""" > # Check cache of profiles > === modified file utils/test/test-aa.py > --- utils/test/test-aa.py 2015-03-15 23:46:02.008030815 +0100 > +++ utils/test/test-aa.py 2015-03-15 23:49:00.282475772 +0100 > @@ -1,7 +1,7 @@ > #! /usr/bin/env python > # ------------------------------------------------------------------ > # > -# Copyright (C) 2014 Christian Boltz > +# Copyright (C) 2014-2015 Christian Boltz > # > # This program is free software; you can redistribute it and/or > # modify it under the terms of version 2 of the GNU General Public > @@ -154,7 +154,9 @@ > self._test_set_flags('profile xy /foo', 'flags=(complain)', > 'complain', profile_name='xy') > 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_11(self): > + self._test_set_flags('/foo', '(complain)', 'complain', > profile_name=None) > - #def test_set_flags_nochange_11(self): > + #def test_set_flags_nochange_12(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}') > > @@ -172,9 +174,7 @@ > def test_set_flags_07(self): > self._test_set_flags('/foo', 'flags=(complain, audit)', '', > expected_flags=None) > def test_set_flags_08(self): > - # XXX this creates an invalid profile with "flags=( )" > - # should raise an exception instead > - self._test_set_flags('/foo', 'flags=(complain, audit)', ' ') > + self._test_set_flags('/foo', '( complain )', 'audit ', > whitespace=' ', profile_name=None) > def test_set_flags_09(self): > self._test_set_flags('profile /foo', 'flags=(complain)', 'audit') > def test_set_flags_10(self): > @@ -199,15 +199,30 @@ > 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) > + self._test_set_flags('/foo', '( )', '', check_new_flags=False) > + def test_set_flags_invalid_04(self): > + with self.assertRaises(AppArmorBug): > + self._test_set_flags('/foo', 'flags=(complain, audit)', ' ', > check_new_flags=False) # whitespace-only newflags > > def test_set_flags_other_profile(self): > # test behaviour if the file doesn't contain the specified /foo > profile > orig_prof = '/no-such-profile flags=(complain) {\n}' > self.file = write_file(self.tmpdir, 'profile', orig_prof) > > - # XXX this silently fails - should it raise an exception instead if > it doesn't find the requested profile in the file? > - set_profile_flags(self.file, '/foo', 'audit') > + with self.assertRaises(AppArmorBug): > + set_profile_flags(self.file, '/foo', 'audit') > + > + # the file should not be changed > + real_new_prof = read_file(self.file) > + self.assertEqual(orig_prof, real_new_prof) > + > + def test_set_flags_no_profile_found(self): > + # test behaviour if the file doesn't contain any profile > + orig_prof = '# /comment flags=(complain) {\n# }' > + self.file = write_file(self.tmpdir, 'profile', orig_prof) > + > + with self.assertRaises(AppArmorBug): > + set_profile_flags(self.file, None, 'audit') > > # the file should not be changed > real_new_prof = read_file(self.file) -- Steve Beattie <[email protected]> http://NxNW.org/~steve/
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
