[apparmor] [patch] check for syntax error in aa.py get_profile_flags() and add tests

[Christian Boltz]

Hello,

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 ;-)


[ get_profile_flags-tests.diff ]

=== 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)
+
 
-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)
+
+    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]