[apparmor] [patch] add tests for set_profile_flags() (and some fun)

Steve Beattie steve at nxnw.org
Wed Apr 1 17:32:06 UTC 2015


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 <steve at nxnw.org> (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='@- at -@', check_new_flags=True, profile_name='/foo'):
> +        if old_flags:
> +            old_flags = ' %s' % old_flags
> +
> +        if expected_flags == '@- at -@':
> +            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
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150401/476080e2/attachment.pgp>


More information about the AppArmor mailing list