[apparmor] [patch] check for syntax error in aa.py get_profile_flags() and add tests
Steve Beattie
steve at nxnw.org
Mon Mar 2 18:15:39 UTC 2015
On Sun, Mar 01, 2015 at 04:27:44PM +0100, Christian Boltz wrote:
> Am Sonntag, 1. März 2015 schrieb Christian Boltz:
> > this patch adds some tests for aa.py get_profile_flags().
> >
> > It also adds a check to get_profile_flags() to catch an invalid
> > syntax: /foo ( ) {
> > was accepted by get_profile_flags, while
> > /foo () {
> > failed.
> >
> > When testing with the parser, both result in a syntax error,
> > therefore the patch makes sure it also fails in get_profile_flags().
> >
> >
> > I propose this patch for trunk and 2.9
>
> Damn, there was an error in the error handling :-/
>
> Here's v2 with the exception text fixed and improved ;-)
Acked-by: Steve Beattie <steve at nxnw.org> for trunk and 2.9 as is.
Stylistically a couple of points inline:
> === modified file 'utils/apparmor/aa.py'
> --- utils/apparmor/aa.py 2015-02-20 20:36:55 +0000
> +++ utils/apparmor/aa.py 2015-03-01 14:43:32 +0000
> @@ -615,6 +615,8 @@
> matches = RE_PROFILE_START.search(line).groups()
> profile = matches[1] or matches[3]
> flags = matches[6]
> + if flags and flags.strip() == '':
> + raise AppArmorException(_('Invalid syntax in %(filename)s for profile %(profile)s: Empty set of flags.' % { 'filename': filename, 'profile': profile } ))
> if profile == program or program is None:
> return flags
>
>
> === modified file 'utils/test/test-aa.py'
> --- utils/test/test-aa.py 2015-02-04 12:16:29 +0000
> +++ utils/test/test-aa.py 2015-03-01 14:44:37 +0000
> @@ -15,9 +15,19 @@
> import tempfile
> from common_test import write_file
>
> -from apparmor.aa import check_for_apparmor, is_skippable_file
> +from apparmor.aa import check_for_apparmor, get_profile_flags, is_skippable_file
> +from apparmor.common import AppArmorException
> +
> +class AaTestWithTempdir(unittest.TestCase):
> + def setUp(self):
> + self.tmpdir = tempfile.mkdtemp(prefix='aa-py-')
> +
> + def tearDown(self):
> + if os.path.exists(self.tmpdir):
> + shutil.rmtree(self.tmpdir)
> +
This should probably move to test/common_test.py and renamed to
something like AATestcase; then other testcase classes in other
scripts can inherit from it.
>
> -class AaTest_check_for_apparmor(unittest.TestCase):
> +class AaTest_check_for_apparmor(AaTestWithTempdir):
> FILESYSTEMS_WITH_SECURITYFS = 'nodev\tdevtmpfs\nnodev\tsecurityfs\nnodev\tsockfs\n\text3\n\text2\n\text4'
> FILESYSTEMS_WITHOUT_SECURITYFS = 'nodev\tdevtmpfs\nnodev\tsockfs\n\text3\n\text2\n\text4'
>
> @@ -28,13 +37,6 @@
> MOUNTS_WITHOUT_SECURITYFS = ( 'proc /proc proc rw,relatime 0 0\n'
> '/dev/sda1 / ext3 rw,noatime,data=ordered 0 0' )
>
> - def setUp(self):
> - self.tmpdir = tempfile.mkdtemp(prefix='aa-py-')
> -
> - def tearDown(self):
> - if os.path.exists(self.tmpdir):
> - shutil.rmtree(self.tmpdir)
> -
> def test_check_for_apparmor_None_1(self):
> filesystems = write_file(self.tmpdir, 'filesystems', self.FILESYSTEMS_WITHOUT_SECURITYFS)
> mounts = write_file(self.tmpdir, 'mounts', self.MOUNTS_WITH_SECURITYFS)
> @@ -70,6 +72,38 @@
> mounts = write_file(self.tmpdir, 'mounts', self.MOUNTS_WITH_SECURITYFS % self.tmpdir)
> self.assertEqual('%s/security/apparmor' % self.tmpdir, check_for_apparmor(filesystems, mounts))
>
> +class AaTest_get_profile_flags(AaTestWithTempdir):
> + def _test_get_flags(self, profile_header, expected_flags):
> + file = write_file(self.tmpdir, 'profile', '%s {\n}\n' % profile_header)
> + flags = get_profile_flags(file, '/foo')
> + self.assertEqual(flags, expected_flags)
Given that the header and the call to get_profile_flags need
to be in sync with respect to the profile name '/foo', I'd have
probably used a default parameter for it and only overriden it in
the test_get_flags_other_profile() test. Otherwise someone adding
additional testcases may trip over that requirement.
> + def test_get_flags_01(self):
> + self._test_get_flags('/foo', None)
> + def test_get_flags_02(self):
> + self._test_get_flags('/foo ( complain )', ' complain ')
> + def test_get_flags_04(self):
> + self._test_get_flags('/foo (complain)', 'complain')
> + def test_get_flags_05(self):
> + self._test_get_flags('/foo flags=(complain)', 'complain')
> + def test_get_flags_06(self):
> + self._test_get_flags('/foo flags=(complain, audit)', 'complain, audit')
> +
> + def test_get_flags_invalid_01(self):
> + with self.assertRaises(AppArmorException):
> + self._test_get_flags('/foo ()', None)
> + def test_get_flags_invalid_02(self):
> + with self.assertRaises(AppArmorException):
> + self._test_get_flags('/foo flags=()', None)
> + def test_get_flags_invalid_03(self):
> + with self.assertRaises(AppArmorException):
> + self._test_get_flags('/foo ( )', ' ')
> +
> + def test_get_flags_other_profile(self):
> + with self.assertRaises(AppArmorException):
> + self._test_get_flags('/no-such-profile flags=(complain)', 'complain')
> +
> +
> class AaTest_is_skippable_file(unittest.TestCase):
> def test_not_skippable_01(self):
> self.assertFalse(is_skippable_file('bin.ping'))
>
>
>
>
> Regards,
>
> Christian Boltz
> --
> > Jaja ... Ich bin eine arme Sau ... Aber das Passwortproblem konnte
> > ich mit einem Zettel unter der Tastatur gut lösen *scnr* :-))
> Nenn all Deine Paßwörter geheim und Du hast kein Problem. Ist
> ungefähr genauso sicher wie ein Zettel unter der Tastatur.
> [> Konrad Neitzel und Bernd Brodesser in suse-linux]
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
--
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/20150302/beff0830/attachment.pgp>
More information about the AppArmor
mailing list