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

[Steve Beattie]

On Sat, Nov 29, 2014 at 08:10:38PM +0100, Christian Boltz wrote:
> Hello,
> 
> 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.)
> 
> 
> [ aamode-validate_log_mode.diff ]
> 
> === modified file 'utils/apparmor/aamode.py'
> --- utils/apparmor/aamode.py    2014-11-29 08:15:17 +0000
> +++ utils/apparmor/aamode.py    2014-11-29 18:54:15 +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-11-29 19:03:55 +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_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).

> +    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'))
>  
>  if __name__ == '__main__':
>      unittest.main(verbosity=2)

-- 
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/20141201/8295060f/attachment.pgp>