On Fri, Mar 06, 2015 at 09:03:46PM +0100, Christian Boltz wrote: > Am Donnerstag, 5. März 2015 schrieb Christian Boltz: > > this patch adds various tests for set_profile_flags, and documents > > various interesting[tm] things I discovered while writing the tests > > (see the inline comments for details). > > Here's v2 - _run_tests() is a bit longer now, but it has the big > advantage of comparing the whole profile file instead of only checking > with get_profile_flags, which means it ensures that no accidential > changes are done. > > The patch also adds a read_file() function to common_test.py, and some > more tests that weren't included in v1. > > > [ 15-add-tests-for-set_profile_flags.diff ]
Acked-by: Steve Beattie <[email protected]> (though with all these patches, we'll probably want to come back and make things a bit more pep8 compliant). > --- utils/test/common_test.py 2015-03-05 00:03:03.148023849 +0100 > +++ utils/test/common_test.py 2015-03-06 18:24:05.924266984 +0100 > @@ -96,6 +96,13 @@ def write_file(directory, file, contents > f.write(contents) > return path > > +def read_file(path): > + '''read and return file contents''' > + with open(path, 'r') as f: > + return f.read() > + > + > + > if __name__ == "__main__": > #import sys;sys.argv = ['', 'Test.test_RegexParser'] > unittest.main() > --- utils/test/test-aa.py 2015-03-06 20:53:28.846654433 +0100 > +++ utils/test/test-aa.py 2015-03-06 20:48:28.807235416 +0100 > @@ -13,9 +13,9 @@ import unittest > import os > import shutil > import tempfile > -from common_test import write_file > +from common_test import read_file, write_file > > -from apparmor.aa import check_for_apparmor, get_profile_flags, > is_skippable_file, parse_profile_start, serialize_parse_profile_start > +from apparmor.aa import check_for_apparmor, get_profile_flags, > set_profile_flags, is_skippable_file, parse_profile_start, > serialize_parse_profile_start > from apparmor.common import AppArmorException, AppArmorBug > > class AaTestWithTempdir(unittest.TestCase): > @@ -104,6 +104,121 @@ class AaTest_get_profile_flags(AaTestWit > with self.assertRaises(AppArmorException): > self._test_get_flags('/no-such-profile flags=(complain)', > 'complain') > > +class AaTest_set_profile_flags(AaTestWithTempdir): > + def _test_set_flags(self, profile, old_flags, new_flags, whitespace='', > comment='', more_rules='', > + expected_flags='@-@-@', check_new_flags=True, > profile_name='/foo'): > + if old_flags: > + old_flags = ' %s' % old_flags > + > + if expected_flags == '@-@-@': > + expected_flags = new_flags > + > + if expected_flags: > + expected_flags = ' flags=(%s)' % (expected_flags) > + else: > + expected_flags = '' > + > + 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) > + new_prof = prof_template % (whitespace, profile, expected_flags, > comment, more_rules, dummy_profile_content) > + > + self.file = write_file(self.tmpdir, 'profile', old_prof) > + set_profile_flags(self.file, profile_name, new_flags) > + if check_new_flags: > + real_new_prof = read_file(self.file) > + self.assertEqual(new_prof, real_new_prof) > + > + # tests that actually don't change the flags > + def test_set_flags_nochange_01(self): > + self._test_set_flags('/foo', '', '') > + def test_set_flags_nochange_02(self): > + self._test_set_flags('/foo', '( complain )', ' complain ', > whitespace=' ') > + def test_set_flags_nochange_03(self): > + self._test_set_flags('/foo', '(complain)', 'complain') > + def test_set_flags_nochange_04(self): > + self._test_set_flags('/foo', 'flags=(complain)', 'complain') > + def test_set_flags_nochange_05(self): > + self._test_set_flags('/foo', 'flags=(complain, audit)', 'complain, > audit', whitespace=' ') > + def test_set_flags_nochange_06(self): > + self._test_set_flags('/foo', 'flags=(complain, audit)', 'complain, > audit', whitespace=' ', comment='# a comment') > + def test_set_flags_nochange_07(self): > + self._test_set_flags('/foo', 'flags=(complain, audit)', 'complain, > audit', whitespace=' ', more_rules=' # a comment\n#another comment') > + def test_set_flags_nochange_08(self): > + 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): > + self._test_set_flags('profile "/foo bar"', 'flags=(complain)', > 'complain', profile_name='/foo bar') > + #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}') > + > + # tests that change the flags > + def test_set_flags_01(self): > + self._test_set_flags('/foo', '', 'audit') > + def test_set_flags_02(self): > + self._test_set_flags('/foo', '( complain )', 'audit ', > whitespace=' ') > + def test_set_flags_04(self): > + self._test_set_flags('/foo', '(complain)', 'audit') > + def test_set_flags_05(self): > + self._test_set_flags('/foo', 'flags=(complain)', 'audit') > + def test_set_flags_06(self): > + self._test_set_flags('/foo', 'flags=(complain, audit)', None, > whitespace=' ') > + 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)', ' ') > + 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' > + 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' > + > + > + # XXX regex_hat_flag in set_profile_flags() is totally broken - it > matches for ' ' and ' X ', but doesn't match for ' ^foo {' > + # oh, it matches on a line like 'dbus' and changes it to 'dbus > flags=(...)' if there's no leading whitespace (and no comma) > + #def test_set_flags_hat_01(self): > + # 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_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') > + > + # the file should not be changed > + real_new_prof = read_file(self.file) > + 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') > + > > class AaTest_is_skippable_file(unittest.TestCase): > def test_not_skippable_01(self): -- 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
