[apparmor] [patch] fix severity.py / handle_variable_rank for filenames containing @

[Christian Boltz]

Hello,

Am Freitag, 10. Oktober 2014 schrieb Steve Beattie:
> On Fri, Oct 10, 2014 at 09:21:34PM +0200, Christian Boltz wrote:
> > if a filename mentioned in audit.log contains an @, aa-logprof
> > crashes with
...
> > handle_variable_rank() checked with   if '@' in resource:
> > and if it finds it, expects it can match a variable, which means  
> > @{.....} If a filename contains a   @   this fails.
> 
> Ugh.
> 
> > The patch fixes the if condition so that it does a regex match.

> Can you cache the result of doing the regex_variable.search() call,
> rather than doing it twice?

Good idea. Here's the updated patch:

=== modified file 'utils/apparmor/severity.py'
--- utils/apparmor/severity.py  2014-02-13 18:01:03 +0000
+++ utils/apparmor/severity.py  2014-10-10 23:01:41 +0000
@@ -143,9 +143,9 @@
         """Returns the max possible rank for file resources containing variables"""
         regex_variable = re.compile('@{([^{.]*)}')
         rank = None
-        if '@' in resource:
-            variable = regex_variable.search(resource).groups()[0]
-            variable = '@{%s}' % variable
+        matches = regex_variable.search(resource)
+        if matches:
+            variable = '@{%s}' % matches.groups()[0]
             #variables = regex_variable.findall(resource)
             for replacement in self.severity['VARIABLES'][variable]:
                 resource_replaced = self.variable_replace(variable, replacement, resource)


> Also, some unit tests that exercise this method,
> handle_variable_rank(), of the Severity class would be nice.

We already have them in utils/test/severity_test.py
Adding some filenames containing @ is probably a good idea ;-)

=== modified file 'utils/test/severity_test.py'
--- utils/test/severity_test.py 2014-07-22 19:25:25 +0000
+++ utils/test/severity_test.py 2014-10-10 23:00:46 +0000
@@ -52,6 +52,8 @@
         self.assertEqual(rank, 9, 'Wrong rank')
         self.assertEqual(sev_db.rank('/etc/apparmor/**', 'r') , 6,  'Invalid Rank')
         self.assertEqual(sev_db.rank('/etc/**', 'r') , 10,  'Invalid Rank')
+        self.assertEqual(sev_db.rank('/usr/foo at bar', 'r') , 10,  'Invalid Rank')  ## filename containing @
+        self.assertEqual(sev_db.rank('/home/foo at bar', 'rw') , 6,  'Invalid Rank')  ## filename containing @
 
         # Load all variables for /sbin/klogd and test them
         sev_db.load_variables('profiles/sbin.klogd')


Regards,

Christian Boltz
-- 
[skipping broken packages while installation]
As Michael said, never expect things to finish if you skip glibc.
[Duncan Mac-Vicar Prett in
https://bugzilla.novell.com/show_bug.cgi?id=215445]