[apparmor] [patch] AARE class

John Johansen john.johansen at canonical.com
Wed Dec 9 20:15:39 UTC 2015


On 11/16/2015 12:48 PM, Christian Boltz wrote:
> Hello,
> 
> Am Montag, 16. November 2015 schrieb Christian Boltz:
>> Am Samstag, 24. Oktober 2015 schrieb Christian Boltz:
>>> [patch] AARE class
>>>
>>> The AARE class is meant to handle the internals of path AppArmor
>>> regexes at various places / rule types (filename, signal peer etc.).
>>> The goal is to use it in rule classes to hide all regex magic, so
>>> that the rule class can just use the match() method.
>>>
>>> The reason for delaying re.compile to match() is performance - I'd
>>> guess a logprof run calls match() only for profiles with existing
>>> log
>>> events, so we can save 90% of the re.compile() calls.
>>>
>>> A possible optimization (on my TODO list) is to check if the given
>>> path is a regex (contains {...,...}, [...], *, **, ?) or if we can
>>> do
>>> a plain string comparison.
>>>
>>> Handling/expanding variables is also on my TODO list.
>>>
>>>
>>> The patch also includes several tests.
>>
>> Here's v2 with the following changes:
>> - handle log events properly (they contain raw strings, not regexes)
>> - add convert_expression_to_aare()
>> - rename self.regex_compiled to self._regex_compiled
>> - add tests for convert_expression_to_aare(), AARE with log_event=True
>> and __repr__()
> 
> After sending out v2, I noticed that equal() is a bad name and should be
> is_equal() instead. Therefore here's v3 with the function renamed to is_equal():
> 
> 
So I gave this a quick run through, and did see anything. I wouldn't call
it a real review but better than nothing.

Acked-by: John Johansen <john.johansen at canonical.com>

> [ 15-aare-class-and-tests.diff ]
> 
> === modified file ./utils/apparmor/aare.py
> --- utils/apparmor/aare.py      2015-11-16 21:42:15.073101290 +0100
> +++ utils/apparmor/aare.py      2015-11-16 21:44:37.348324136 +0100
> @@ -0,0 +1,83 @@
> +# ----------------------------------------------------------------------
> +#    Copyright (C) 2015 Christian Boltz <apparmor at cboltz.de>
> +#
> +#    This program is free software; you can redistribute it and/or
> +#    modify it under the terms of version 2 of the GNU General Public
> +#    License as published by the Free Software Foundation.
> +#
> +#    This program is distributed in the hope that it will be useful,
> +#    but WITHOUT ANY WARRANTY; without even the implied warranty of
> +#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +#    GNU General Public License for more details.
> +#
> +# ----------------------------------------------------------------------
> +
> +import re
> +
> +from apparmor.common import convert_regexp, AppArmorBug, AppArmorException
> +
> +class AARE(object):
> +    '''AARE (AppArmor Regular Expression) wrapper class'''
> +
> +    def __init__(self, regex, is_path, log_event=None):
> +        '''create an AARE instance for the given AppArmor regex
> +        If is_path is true, the regex is expected to be a path and therefore must start with / or a variable.'''
> +        # using the specified variables when matching.
> +
> +        if is_path:
> +            if regex.startswith('/'):
> +                pass
> +            elif regex.startswith('@{'):
> +                pass  # XXX ideally check variable content - each part must start with / - or another variable, which must start with /
> +            else:
> +                raise AppArmorException("Path doesn't start with / or variable: %s" % regex)
> +
> +        if log_event:
> +            self.orig_regex = regex
> +            self.regex = convert_expression_to_aare(regex)
> +        else:
> +            self.orig_regex = None
> +            self.regex = regex
> +
> +        self._regex_compiled = None  # done on first use in match() - that saves us some re.compile() calls
> +        # self.variables = variables  # XXX
> +
> +    def __repr__(self):
> +        '''returns a "printable" representation of AARE'''
> +        return "AARE('%s')" % self.regex
> +
> +    def match(self, expression):
> +        '''check if the given expression (string or AARE) matches the regex'''
> +
> +        if type(expression) == AARE:
> +            if expression.orig_regex:
> +                expression = expression.orig_regex
> +            else:
> +                return self.is_equal(expression)  # better safe than sorry
> +        elif type(expression) != str:
> +            raise AppArmorBug('AARE.match() called with unknown object: %s' % str(expression))
> +
> +        if self._regex_compiled is None:
> +            self._regex_compiled = re.compile(convert_regexp(self.regex))
> +
> +        return bool(self._regex_compiled.match(expression))
> +
> +    def is_equal(self, expression):
> +        '''check if the given expression is equal'''
> +
> +        if type(expression) == AARE:
> +            return self.regex == expression.regex
> +        elif type(expression) == str:
> +            return self.regex == expression
> +        else:
> +            raise AppArmorBug('AARE.is_equal() called with unknown object: %s' % str(expression))
> +
> +
> +def convert_expression_to_aare(expression):
> +    '''convert an expression (taken from audit.log) to an AARE string'''
> +
> +    aare_escape_chars = ['\\', '?', '*', '[', ']', '{', '}', '"']
> +    for char in aare_escape_chars:
> +        expression = expression.replace(char, '\\' + char)
> +
> +    return expression
> === modified file ./utils/test/test-aare.py
> --- utils/test/test-aare.py     2015-11-16 21:42:15.073101290 +0100
> +++ utils/test/test-aare.py     2015-11-16 21:45:14.624121206 +0100
> @@ -14,7 +14,8 @@
>  from common_test import AATest, setup_all_loops
>  
>  import re
> -from apparmor.common import convert_regexp
> +from apparmor.common import convert_regexp, AppArmorBug, AppArmorException
> +from apparmor.aare import AARE, convert_expression_to_aare
>  
>  class TestConvert_regexp(AATest):
>      tests = [
> @@ -34,7 +35,24 @@
>      def _run_test(self, params, expected):
>          self.assertEqual(convert_regexp(params), expected)
>  
> -class TestExamplesConvert_regexp(AATest):
> +class Test_convert_expression_to_aare(AATest):
> +    tests = [
> +        # note that \ always needs to be escaped in python, so \\ is actually just \ in the string
> +        ('/foo',        '/foo'                  ),
> +        ('/foo?',       '/foo\\?'               ),
> +        ('/foo*',       '/foo\\*'               ),
> +        ('/foo[bar]',   '/foo\\[bar\\]'         ),
> +        ('/foo{bar}',   '/foo\\{bar\\}'         ),
> +        ('/foo{',       '/foo\\{'               ),
> +        ('/foo\\',      '/foo\\\\'              ),
> +        ('/foo"',       '/foo\\"'               ),
> +        ('}]"\\[{',     '\\}\\]\\"\\\\\\[\\{'   ),
> +    ]
> +
> +    def _run_test(self, params, expected):
> +        self.assertEqual(convert_expression_to_aare(params), expected)
> +
> +class TestConvert_regexpAndAAREMatch(AATest):
>      tests = [
>          #  aare                  path to check                         match expected?
>          (['/foo/**/bar/',       '/foo/user/tools/bar/'              ], True),
> @@ -117,6 +135,94 @@
>          parsed_regex = re.compile(convert_regexp(regex))
>          self.assertEqual(bool(parsed_regex.search(path)), expected, 'Incorrectly Parsed regex: %s' %regex)
>  
> +        aare_obj = AARE(regex, True)
> +        self.assertEqual(aare_obj.match(path), expected, 'Incorrectly parsed AARE object: %s' % regex)
> +
> +    def test_multi_usage(self):
> +        aare_obj = AARE('/foo/*', True)
> +        self.assertTrue(aare_obj.match('/foo/bar'))
> +        self.assertFalse(aare_obj.match('/foo/bar/'))
> +        self.assertTrue(aare_obj.match('/foo/asdf'))
> +
> +    def test_match_against_AARE_1(self):
> +        aare_obj_1 = AARE('@{foo}/[a-d]**', True)
> +        aare_obj_2 = AARE('@{foo}/[a-d]**', True)
> +        self.assertTrue(aare_obj_1.match(aare_obj_2))
> +        self.assertTrue(aare_obj_1.is_equal(aare_obj_2))
> +
> +    def test_match_against_AARE_2(self):
> +        aare_obj_1 = AARE('@{foo}/[a-d]**', True)
> +        aare_obj_2 = AARE('@{foo}/*[a-d]*', True)
> +        self.assertFalse(aare_obj_1.match(aare_obj_2))
> +        self.assertFalse(aare_obj_1.is_equal(aare_obj_2))
> +
> +    def test_match_invalid_1(self):
> +        aare_obj = AARE('@{foo}/[a-d]**', True)
> +        with self.assertRaises(AppArmorBug):
> +            aare_obj.match(set())
> +
> +class TestAAREMatchFromLog(AATest):
> +    tests = [
> +        #  AARE                 log event                  match expected?
> +        (['/foo/bar',           '/foo/bar'              ], True),
> +        (['/foo/*',             '/foo/bar'              ], True),
> +        (['/**',                '/foo/bar'              ], True),
> +        (['/foo/*',             '/bar/foo'              ], False),
> +        (['/foo/*',             '/foo/"*'               ], True),
> +        (['/foo/bar',           '/foo/*'                ], False),
> +        (['/foo/?',             '/foo/('                ], True),
> +        (['/foo/{bar,baz}',     '/foo/bar'              ], True),
> +        (['/foo/{bar,baz}',     '/foo/bars'             ], False),
> +    ]
> +
> +    def _run_test(self, params, expected):
> +        regex, log_event = params
> +        aare_obj_1 = AARE(regex, True)
> +        aare_obj_2 = AARE(log_event, True, log_event=True)
> +        self.assertEqual(aare_obj_1.match(aare_obj_2), expected)
> +
> +class TestAAREIsEqual(AATest):
> +    tests = [
> +        # regex         is path?    check for       expected
> +        (['/foo',       True,       '/foo'      ],  True ),
> +        (['@{foo}',     True,       '@{foo}'    ],  True ),
> +        (['/**',        True,       '/foo'      ],  False),
> +    ]
> +
> +    def _run_test(self, params, expected):
> +        regex, is_path, check_for = params
> +        aare_obj_1 = AARE(regex, is_path)
> +        aare_obj_2 = AARE(check_for, is_path)
> +        self.assertEqual(expected, aare_obj_1.is_equal(check_for))
> +        self.assertEqual(expected, aare_obj_1.is_equal(aare_obj_2))
> +
> +    def test_is_equal_invalid_1(self):
> +        aare_obj = AARE('/foo/**', True)
> +        with self.assertRaises(AppArmorBug):
> +            aare_obj.is_equal(42)
> +
> +class TestAAREIsPath(AATest):
> +    tests = [
> +        # regex         is path?    match for       expected
> +        (['/foo*',      True,       '/foobar'   ],  True ),
> +        (['@{PROC}/',   True,       '/foobar'   ],  False),
> +        (['foo*',       False,      'foobar'    ],  True ),
> +    ]
> +
> +    def _run_test(self, params, expected):
> +        regex, is_path, check_for = params
> +        aare_obj = AARE(regex, is_path)
> +        self.assertEqual(expected, aare_obj.match(check_for))
> +
> +    def test_path_missing_slash(self):
> +        with self.assertRaises(AppArmorException):
> +            AARE('foo*', True)
> +
> +class TestAARERepr(AATest):
> +    def test_repr(self):
> +        obj = AARE('/foo', True)
> +        self.assertEqual(str(obj), "AARE('/foo')")
> +
>  
>  setup_all_loops(__name__)
>  if __name__ == '__main__':
> 
> 
> 
> 
> Regards,
> 
> Christian Boltz
> 




More information about the AppArmor mailing list