[apparmor] [patch] aa.py: split off parse_profile_start() from parse_profile_data() and add tests
Christian Boltz
apparmor at cboltz.de
Mon Mar 2 19:44:44 UTC 2015
Hello,
Am Montag, 2. März 2015 schrieb Steve Beattie:
> On Sun, Mar 01, 2015 at 07:12:05PM +0100, Christian Boltz wrote:
> > [ split-off-parse_profile_start-and-add-tests.diff ]
>
> [SNIP]
>
> > === modified file 'utils/apparmor/aa.py'
> > --- utils/apparmor/aa.py 2015-02-20 20:36:55 +0000
> > +++ utils/apparmor/aa.py 2015-03-01 17:42:11 +0000
...
> > + (profile, hat, flags, in_contained_hat,
> > pps_set_profile, pps_set_hat_external) = parse_profile_start(line,
> > file, lineno, profile, hat, profile_data)
> You pass 6 arguments to parse_profile_start() here, but the
> declaration for it only takes 5 arguments. I'm guessing that's not
> intended?
Indeed, that's a leftover from the first version of the patch, where I
tried to pass profile_data forth and back, which didn't work too well.
Instead, I now have the two pps_set_* variables as part of the return
value which set the needed flags in profile_data.
Here's the updated patch, which also removes the two unused return
values in the tests:
[ split-off-parse_profile_start-and-add-tests.diff ]
=== modified file 'utils/test/test-aa.py'
--- utils/test/test-aa.py 2015-03-01 17:37:24.789364744 +0100
+++ utils/test/test-aa.py 2015-03-01 18:41:33.062731404 +0100
@@ -15,7 +15,7 @@ import shutil
import tempfile
from common_test import write_file
-from apparmor.aa import check_for_apparmor, get_profile_flags, is_skippable_file
+from apparmor.aa import check_for_apparmor, get_profile_flags, is_skippable_file, parse_profile_start
from apparmor.common import AppArmorException
class AaTestWithTempdir(unittest.TestCase):
@@ -147,6 +147,45 @@ class AaTest_is_skippable_file(unittest.
def test_skippable_13(self):
self.assertTrue(is_skippable_file('README'))
+class AaTest_parse_profile_start(unittest.TestCase):
+ def _parse(self, line, profile, hat):
+ return parse_profile_start(line, 'somefile', 1, profile, hat)
+ # (profile, hat, flags, in_contained_hat, pps_set_profile, pps_set_hat_external)
+
+ def test_parse_profile_start_01(self):
+ result = self._parse('/foo {', None, None)
+ expected = ('/foo', '/foo', None, False, False, False)
+ self.assertEqual(result, expected)
+
+ def test_parse_profile_start_02(self):
+ result = self._parse('/foo (complain) {', None, None)
+ expected = ('/foo', '/foo', 'complain', False, False, False)
+ self.assertEqual(result, expected)
+
+ def test_parse_profile_start_03(self):
+ result = self._parse('profile foo /foo {', None, None) # named profile
+ expected = ('foo /foo', 'foo /foo', None, False, False, False) # XXX yes, that's what happens with the current code :-/
+ self.assertEqual(result, expected)
+
+ def test_parse_profile_start_04(self):
+ result = self._parse('profile /foo {', '/bar', '/bar') # child profile
+ expected = ('/bar', '/foo', None, True, True, False)
+ self.assertEqual(result, expected)
+
+ def test_parse_profile_start_05(self):
+ result = self._parse('/foo//bar {', None, None) # external hat
+ expected = ('/foo', 'bar', None, False, False, True)
+ self.assertEqual(result, expected)
+
+
+ def test_parse_profile_start_invalid_01(self):
+ with self.assertRaises(AppArmorException):
+ self._parse('/foo {', '/bar', '/bar') # child profile without profile keyword
+
+ def test_parse_profile_start_invalid_02(self):
+ with self.assertRaises(AttributeError): # XXX does this need error handling in parse_profile_start?
+ self._parse('xy', '/bar', '/bar') # not a profile start
+
if __name__ == '__main__':
unittest.main(verbosity=2)
=== modified file 'utils/apparmor/aa.py'
--- utils/apparmor/aa.py 2015-02-20 20:36:55 +0000
+++ utils/apparmor/aa.py 2015-03-01 17:42:11 +0000
@@ -2632,6 +2642,47 @@
for p in profile_data.keys():
profiles[p] = deepcopy(profile_data[p])
+
+def parse_profile_start(line, file, lineno, profile, hat):
+ matches = RE_PROFILE_START.search(line).groups()
+
+ pps_set_profile = False
+ pps_set_hat_external = False
+
+ if profile:
+ #print(profile, hat)
+ if profile != hat or not matches[3]:
+ raise AppArmorException(_('%(profile)s profile in %(file)s contains syntax errors in line: %(line)s.') % { 'profile': profile, 'file': file, 'line': lineno + 1 })
+ # Keep track of the start of a profile
+ if profile and profile == hat and matches[3]:
+ # local profile
+ hat = matches[3]
+ in_contained_hat = True
+ pps_set_profile = True
+ else:
+ if matches[1]:
+ profile = matches[1]
+ else:
+ profile = matches[3]
+ #print(profile)
+ if len(profile.split('//')) >= 2:
+ profile, hat = profile.split('//')[:2]
+ else:
+ hat = None
+ in_contained_hat = False
+ if hat:
+ pps_set_hat_external = True
+ else:
+ hat = profile
+
+ flags = matches[6]
+
+ profile = strip_quotes(profile)
+ if hat:
+ hat = strip_quotes(hat)
+
+ return (profile, hat, flags, in_contained_hat, pps_set_profile, pps_set_hat_external)
+
def parse_profile_data(data, file, do_include):
profile_data = hasher()
profile = None
@@ -2655,41 +2708,15 @@
lastline = None
# Starting line of a profile
if RE_PROFILE_START.search(line):
- matches = RE_PROFILE_START.search(line).groups()
-
- if profile:
- #print(profile, hat)
- if profile != hat or not matches[3]:
- raise AppArmorException(_('%(profile)s profile in %(file)s contains syntax errors in line: %(line)s.') % { 'profile': profile, 'file': file, 'line': lineno + 1 })
- # Keep track of the start of a profile
- if profile and profile == hat and matches[3]:
- # local profile
- hat = matches[3]
- in_contained_hat = True
+ (profile, hat, flags, in_contained_hat, pps_set_profile, pps_set_hat_external) = parse_profile_start(line, file, lineno, profile, hat)
+ if pps_set_profile:
profile_data[profile][hat]['profile'] = True
- else:
- if matches[1]:
- profile = matches[1]
- else:
- profile = matches[3]
- #print(profile)
- if len(profile.split('//')) >= 2:
- profile, hat = profile.split('//')[:2]
- else:
- hat = None
- in_contained_hat = False
- if hat:
- profile_data[profile][hat]['external'] = True
- else:
- hat = profile
+ if pps_set_hat_external:
+ profile_data[profile][hat]['external'] = True
+
# Profile stored
existing_profiles[profile] = file
- flags = matches[6]
-
- profile = strip_quotes(profile)
- if hat:
- hat = strip_quotes(hat)
# save profile name and filename
profile_data[profile][hat]['name'] = profile
profile_data[profile][hat]['filename'] = file
Regards,
Christian Boltz
--
I have also tried to understand how to write and install systemd stuff
to start daemons and it definitely hasn't passed the "10 minute test".
It definitely lacks better documentation, or I've been too lazy, I
wouldn't exclude that from the equation :)
[Pascal Bleser in opensuse-factory]
More information about the AppArmor
mailing list