[apparmor] [patch] raise exception if sub_str_to_mode() is called with invalid mode string (was: Re: [patch] add tests for aamode.py)

[Christian Boltz]

Hello,

Am Montag, 22. Dezember 2014 schrieb John Johansen:
> On 11/29/2014 11:19 AM, Christian Boltz wrote:
> > See for example
> > 
> >     def test_sub_str_to_mode_8(self):
> >         self.assertEqual(sub_str_to_mode('asdf42'), {'a'})
> > 
> > Now the question is if sub_str_to_mode shoud be non-silent and print
> > a warning or raise an exception instead. (Given that
> > valid_log_mode, if called, restricts the mode to LOG_MODE_RE,
> > raising an exception shouldn't break anything.)
> > 
> > I'll wait for your opinion before writing a patch ;-)
> 
> I'd say raise an exception

Yes, that's the safe solution ;-)  so here's the patch to implement it:


Raise an exception if sub_str_to_mode() is called with invalid mode 
string or if a mode_char is not in MODE_HASH.

Also update the testcase for "asdf42" (which raises AppArmorBug now) and
add a test that simulates MODE_HASH and MODE_MAP_SET getting out of 
sync (tests the second part of the if condition).


[ aamode-sub_str_to_mode_exception.diff ]

=== modified file 'utils/apparmor/aamode.py'
--- utils/apparmor/aamode.py    2014-12-02 17:45:41 +0000
+++ utils/apparmor/aamode.py    2014-12-22 18:17:14 +0000
@@ -90,10 +90,10 @@
     mode = set()
 
     for mode_char in string:
-        if mode_char not in MODE_MAP_SET:
-            break
-        if MODE_HASH.get(mode_char, False):
+        if mode_char in MODE_MAP_SET and MODE_HASH.get(mode_char, False):
             mode |= MODE_HASH[mode_char]
+        else:
+            raise AppArmorBug("Mode string '%s' contains invalid char '%s'" % (string, mode_char))
 
     return mode
 

=== modified file 'utils/test/test-aamode.py'
--- utils/test/test-aamode.py   2014-12-01 21:49:54 +0000
+++ utils/test/test-aamode.py   2014-12-22 18:19:26 +0000
@@ -46,11 +46,22 @@
         self.assertEqual(sub_str_to_mode('cix'), {'i', 'x', 'C', 'execunsafe'})
     def test_sub_str_to_mode_7(self):
         self.assertEqual(sub_str_to_mode('rwlk'), {'k', 'r', 'l', 'w'})
-    def test_sub_str_to_mode_8(self):
-        self.assertEqual(sub_str_to_mode('asdf42'), {'a'})
     def test_sub_str_to_mode_dupes(self):
         self.assertEqual(sub_str_to_mode('rwrwrw'), {'r', 'w'})
 
+    def test_sub_str_to_mode_invalid_1(self):
+        with self.assertRaises(AppArmorBug):
+            sub_str_to_mode('asdf42')
+
+    def test_sub_str_to_mode_invalid_2(self):
+        import apparmor.aamode
+        apparmor.aamode.MODE_HASH = {'x': 'foo'}  # simulate MODE_HASH and MODE_MAP_SET getting out of sync
+
+        with self.assertRaises(AppArmorBug):
+            sub_str_to_mode('r')
+
+
+
 class AamodeTest_validate_log_mode(unittest.TestCase):
     def test_validate_log_mode_1(self):
         self.assertTrue(validate_log_mode('a'))


Regards,

Christian Boltz
-- 
Ja, aber damit wirst Du Glenn nicht zufriedenstellen können. Unser 
Baby will ein Edelfläschchen mit frischer, vorgereinigter Milch, 
mit feinster Bourbonvanille versetzt und einem Schuss edelstem 
Madagaskar-Rum. Garniert in einem Früchtetraum aus Mango, Aprikose 
und Maracuja. [Philipp Zacharias in suse-linux]