[apparmor] [patch] aamode.py - fix LOG_MODE_RE

[Christian Boltz]

Hello,

Am Montag, 1. Dezember 2014 schrieb Steve Beattie:
> On Sat, Nov 29, 2014 at 08:10:38PM +0100, Christian Boltz wrote:
> > LOG_MODE_RE (used in validate_log_mode() in aamode.py) just checked
> > if the given parameter contains one of the possible matches. This
> > resulted in "invalid" [1] being a valid log mode (from audit.log
> > requested_mask or denied_mask) because it contains 'a', which is a
> > valid file mode.
> > 
> > This patch wraps the regex into   ^(...)*$   to make sure the full
> > string contains only allowed file modes.
> > 
> > (The question is if the log (!) really contains things like Pix -
> > but if not, that's worth another patch ;-)
> > 
> > The patch also adds some tests for validate_log_mode().
> > 
> > (Just in case you wonder about the order - I first noticed the regex
> > bug, then wrote the tests.)

> > +    def test_validate_log_mode_5(self):
> > +        self.assertTrue(validate_log_mode(''))
> 
> Is this the correct result? I don't think the old version would have
> returned True in that situation, and it seems fishy to do so. I think
> you want '+' instead of '*' (Kleene star).

Indeed, that's the only sanity check that worked in the old code ;-)
Good catch!

Here's the updated patch with two changes:
- change the regex to use "+" instead of "*" (which means an empty 
  string will return False)
- rename test_validate_log_mode_5() to test_validate_log_mode_invalid_4() 
  and expect False for ''


[ aamode-validate_log_mode-v2.diff ]

=== modified file 'utils/apparmor/aamode.py'
--- utils/apparmor/aamode.py    2014-12-01 19:56:31 +0000
+++ utils/apparmor/aamode.py    2014-12-01 20:05:41 +0000
@@ -68,7 +68,7 @@
              'N': AA_EXEC_NT
              }
 
-LOG_MODE_RE = re.compile('(r|w|l|m|k|a|x|ix|ux|px|pux|cx|nx|pix|cix|Ux|Px|PUx|Cx|Nx|Pix|Cix)')
+LOG_MODE_RE = re.compile('^(r|w|l|m|k|a|x|ix|ux|px|pux|cx|nx|pix|cix|Ux|Px|PUx|Cx|Nx|Pix|Cix)+$')
 MODE_MAP_SET = {"r", "w", "l", "m", "k", "a", "x", "i", "u", "p", "c", "n", "I", "U", "P", "C", "N"}
 
 def str_to_mode(string):

=== modified file 'utils/test/test-aamode.py'
--- utils/test/test-aamode.py   2014-11-29 08:15:17 +0000
+++ utils/test/test-aamode.py   2014-12-01 20:06:12 +0000
@@ -11,7 +11,7 @@
 
 import unittest
 
-from apparmor.aamode import split_log_mode, sub_str_to_mode
+from apparmor.aamode import split_log_mode, sub_str_to_mode, validate_log_mode
 from apparmor.common import AppArmorBug
 
 class AamodeTest_split_log_mode(unittest.TestCase):
@@ -51,6 +51,24 @@
     def test_sub_str_to_mode_dupes(self):
         self.assertEqual(sub_str_to_mode('rwrwrw'), {'r', 'w'})
 
+class AamodeTest_validate_log_mode(unittest.TestCase):
+    def test_validate_log_mode_1(self):
+        self.assertTrue(validate_log_mode('a'))
+    def test_validate_log_mode_2(self):
+        self.assertTrue(validate_log_mode('rw'))
+    def test_validate_log_mode_3(self):
+        self.assertTrue(validate_log_mode('Pixrw'))
+    def test_validate_log_mode_4(self):
+        self.assertTrue(validate_log_mode('rrrr'))
+
+    def test_validate_log_mode_invalid_1(self):
+        self.assertFalse(validate_log_mode('c'))  # 'c' (create) must be converted to 'a' before calling validate_log_mode()
+    def test_validate_log_mode_invalid_2(self):
+        self.assertFalse(validate_log_mode('R'))  # only lowercase 'r' is valid
+    def test_validate_log_mode_invalid_3(self):
+        self.assertFalse(validate_log_mode('foo'))
+    def test_validate_log_mode_invalid_4(self):
+        self.assertFalse(validate_log_mode(''))
 
 if __name__ == '__main__':
     unittest.main(verbosity=2)



Regards,

Christian Boltz
-- 
Ein großer Teil Deines Freundeskreises arbeitet wahrscheinlich
im IT-Bereich. Nicht verwunderlich, dass eine Hotline als
Problemlösungsansatz sofort verworfen wird... [Guido auf
http://blog.koehntopp.de/archives/3095-Booking-geht-ran.html#c27967]