On Thu, Mar 06, 2014 at 10:10:16PM +0100, Christian Boltz wrote: > Am Mittwoch, 5. März 2014 schrieb Steve Beattie: > > D-Bus rules in particular seem to get written as multi-line rules. > > This patch adds very simple hackish support for multiple lines. > > Essentially, what it does is if the parsing of a line doesn't match > > anything and falls all the way through, it saves the line and > > prepends it to the next line that occurs in the profile, but *only* > > if the line does not have a trailing comma to indicate the end of a > > rule. If the trailing comma exists, then it assumes that it's a rule > > that it doesn't understand and aborts. > > Just for the records: this works for all rule types, not only for dbus > rules.
Correct.
> Is it intentional that RE_RULE_HAS_COMMA does not start with ^ ?
Let's just say that RE_RULE_HAS_COMMA was a bit over-engineered. And
yeah, it should have been anchored at the beginning of the line.
I've updated the regex in the attached patch, as well as added a second
to detect trailing comments (that need to filtered). Hopefully, the way
I split them up is more readable.
> > + # we're dealing with a multiline statement
> > + if lastline:
> > + line = '%s %s' % (lastline, line)
>
> You can probably (untested ;-) also set "lastline = None" here (since
> you added it to line, it isn't needed anymore). This also makes the
> following >10 "lastline = None" superfluous.
Yeah, adjusted patch.
> > Index: b/utils/test/test-dbus_parse.py
> > ===================================================================
> > --- a/utils/test/test-dbus_parse.py
> > +++ b/utils/test/test-dbus_parse.py
>
> Hmm, that's a strange filename for testing a regex that works for all
> rule types ;-)
Heh, okay, hint taken, split out into a separate test script, which
exercises both new regexs. More tests can be added here for
other/additional test cases.
>
> > + def test_without_comma_curly_brace_03(self):
> > + '''simple no comma check w/empty curly braces'''
> > + self._check('member={} ', False)
> > +
> > if __name__ == '__main__':
> > unittest.main()
>
> I'd merge the testcases into arrays, so that you have one array for
> "with comma" and one for "without comma", and then loop over the arrays.
> This should keep the code more readable, even if we add 50 additional
> test rules.
I created an array, but I use each string in a 'no comma' and 'with
comma' situation, using %s replacement with either a comma or a space.
Kinda overkill, but works. I also used an array with generator for the
new split comment out regex.
> Speaking about additional test rules - please also add testcases for:
>
> audit "/tmp/foo, # bar"
>
> audit "/tmp/foo, # bar" rw,
>
> audit "/tmp/foo, # bar" rw, # comment
Good catches, I've added cases for the above.
> member={Hello,AddMatch,RemoveMatch,
>
> dbus (r, w,
So... the former is invalid syntax under the parser (though it
is if you encapsulate the alternation in "s), while the latter is
accepted (in the context of additional text to make a complete rule
on additional lines). However, given that until this patch set, the
tools wouldn't parse multi-line rules *at all* and the complexity of
getting the above situations correct, at this point I'm inclined to
say that you probably shouldn't organize your rules like so, similar
to how we asserted you shouldn't use multi line rules if you wanted
to use the tools until now.
Anyway. Updated patch attached.
--
Steve Beattie
<[email protected]>
http://NxNW.org/~steve/
Subject: utils: add simple parsing of multi-line rules [v2]
D-Bus rules in particular seem to get written as multi-line rules. This
patch adds very simple hackish support for multiple lines. Essentially,
what it does is if the parsing of a line doesn't match anything and
falls all the way through, it saves the line and prepends it to the next
line that occurs in the profile, but *only* if the line does not have a
trailing comma to indicate the end of a rule. If the trailing comma
exists, then it assumes that it's a rule that it doesn't understand and
aborts.
With this patch, the simpler tools (aa-enforce, aa-complain, etc.) can
parse policies containing multi-line rules to an extent and continue to
function correctly. Again, aa-logprof and aa-genprof may have issues on
the writing back of profiles, so some assistance testing here would be
appreciated.
Some testcases are added to exercise the regex that looks for a rule
with a trailing comma but can still handle rules that have (,) or {,}
in them.
Patch history:
v1 - initial version
v2 - simplify and rearrange rule-ending comma search regex, since
we only care about the trailing comma
- add a new regex to search for trailing comments to filter out
- simplify reset of lastline variable
- restructure tests into a new script, and add more tests
Signed-off-by: Steve Beattie <[email protected]>
---
utils/apparmor/aa.py | 22 +++++++
utils/test/test-regex_matches.py | 109 +++++++++++++++++++++++++++++++++++++++
2 files changed, 130 insertions(+), 1 deletion(-)
Index: b/utils/apparmor/aa.py
===================================================================
--- a/utils/apparmor/aa.py
+++ b/utils/apparmor/aa.py
@@ -2615,7 +2615,15 @@ RE_PROFILE_CHANGE_HAT = re.compile('^\s*
RE_PROFILE_HAT_DEF = re.compile('^\s*\^(\"??.+?\"??)\s+((flags=)?\((.+)\)\s+)*\{\s*(#.*)?$')
RE_NETWORK_FAMILY_TYPE = re.compile('\s+(\S+)\s+(\S+)\s*,$')
RE_NETWORK_FAMILY = re.compile('\s+(\S+)\s*,$')
-RE_PROFILE_DBUS = re.compile('^\s*(audit\s+)?(allow\s+|deny\s+)?(dbus[^#]*)\s*(#.*)?$')
+RE_PROFILE_DBUS = re.compile('^\s*(audit\s+)?(allow\s+|deny\s+)?(dbus[^#]*\s*,)\s*(#.*)?$')
+
+# match anything that's not " or #, or matching quotes with anything except quotes inside
+__re_no_or_quoted_hash = '([^#"]|"[^"]*")*'
+
+RE_RULE_HAS_COMMA = re.compile('^' + __re_no_or_quoted_hash +
+ ',\s*(#.*)?$') # match comma plus any trailing comment
+RE_HAS_COMMENT_SPLIT = re.compile('^(?P<not_comment>' + __re_no_or_quoted_hash + ')' + # store in 'not_comment' group
+ '(?P<comment>#.*)$') # match trailing comment and store in 'comment' group
def parse_profile_data(data, file, do_include):
profile_data = hasher()
@@ -2625,6 +2633,7 @@ def parse_profile_data(data, file, do_in
repo_data = None
parsed_profiles = []
initial_comment = ''
+ lastline = None
if do_include:
profile = file
@@ -2633,6 +2642,10 @@ def parse_profile_data(data, file, do_in
line = line.strip()
if not line:
continue
+ # we're dealing with a multiline statement
+ if lastline:
+ line = '%s %s' % (lastline, line)
+ lastline = None
# Starting line of a profile
if RE_PROFILE_START.search(line):
matches = RE_PROFILE_START.search(line).groups()
@@ -3007,6 +3020,13 @@ def parse_profile_data(data, file, do_in
else:
initial_comment = initial_comment + line + '\n'
+ elif not RE_RULE_HAS_COMMA.search(line):
+ # Bah, line continues on to the next line
+ if RE_HAS_COMMENT_SPLIT.search(line):
+ # filter trailing comments
+ lastline = RE_HAS_COMMENT_SPLIT.search(line).group('not_comment')
+ else:
+ lastline = line
else:
raise AppArmorException(_('Syntax Error: Unknown line found in file: %s line: %s') % (file, lineno + 1))
Index: b/utils/test/test-regex_matches.py
===================================================================
--- /dev/null
+++ b/utils/test/test-regex_matches.py
@@ -0,0 +1,109 @@
+#! /usr/bin/env python
+# ------------------------------------------------------------------
+#
+# Copyright (C) 2014 Canonical Ltd.
+#
+# 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 published by the Free Software Foundation.
+#
+# ------------------------------------------------------------------
+
+import apparmor.aa as aa
+import unittest
+
+
+class AARegexHasComma(unittest.TestCase):
+ '''Tests for apparmor.aa.RE_RULE_HAS_COMMA'''
+
+ def _check(self, line, expected=True):
+ result = aa.RE_RULE_HAS_COMMA.search(line)
+ if expected:
+ self.assertTrue(result, 'Couldn\'t find a comma in "%s"' % line)
+ else:
+ self.assertEqual(None, result, 'Found an unexpected comma in "%s"' % line)
+
+regex_has_comma_testcases = [
+ ('dbus send%s', 'simple'),
+ ('dbus (r, w, bind, eavesdrop)%s', 'embedded parens 01'),
+ ('dbus (r, w,, bind, eavesdrop) %s', 'embedded parens 02'),
+ ('dbus (r, w,, ) %s', 'embedded parens 03'),
+ ('dbus () %s', 'empty parens'),
+ ('member={Hello,AddMatch,RemoveMatch,GetNameOwner,NameHasOwner,StartServiceByName} %s ', 'embedded curly braces 01'),
+ ('member={Hello,,,,,,AddMatch,,,NameHasOwner,StartServiceByName} %s ', 'embedded curly braces 02'),
+ ('member={,Hello,,,,,,AddMatch,,,NameHasOwner,} %s ', 'embedded curly braces 03'),
+ ('member={} %s ', 'empty curly braces'),
+ ('dbus send%s# this is a comment', 'comment 01'),
+ ('dbus send%s# this is a comment,', 'comment 02'),
+ ('audit "/tmp/foo, bar" rw%s', 'quotes 01'),
+ ('audit "/tmp/foo, bar" rw%s # comment', 'quotes 02'),
+ ('audit "/tmp/foo, # bar" rw%s', 'comment embedded in quote 01'),
+ ('audit "/tmp/foo, # bar" rw%s # comment', 'comment embedded in quote 02'),
+ # the following fail due to inadequacies in the regex
+ # ('dbus (r, w, %s', 'incomplete dbus action'),
+ # ('member={Hello,AddMatch,RemoveMatch, %s', 'incomplete {} regex'), # also invalid policy
+]
+
+def setup_has_comma_testcases():
+ i = 0
+ for (test_string, description) in regex_has_comma_testcases:
+ i += 1
+ def stub_test_comma(self, test_string=test_string):
+ self._check(test_string % ',')
+ def stub_test_no_comma(self, test_string=test_string):
+ self._check(test_string % ' ', False)
+ stub_test_comma.__doc__ = "test %s (w/comma)" % (description)
+ stub_test_no_comma.__doc__ = "test %s (no comma)" % (description)
+ setattr(AARegexHasComma, 'test_comma_%d' % (i), stub_test_comma)
+ setattr(AARegexHasComma, 'test_no_comma_%d' % (i), stub_test_no_comma)
+
+class AARegexSplitComment(unittest.TestCase):
+ '''Tests for RE_HAS_COMMENT_SPLIT'''
+
+ def _check(self, line, expected, comment=None, not_comment=None):
+ result = aa.RE_HAS_COMMENT_SPLIT.search(line)
+ if expected:
+ self.assertTrue(result, 'Couldn\'t find a comment in "%s"' % line)
+ self.assertEqual(result.group('comment'), comment, 'Expected comment "%s", got "%s"'
+ % (comment, result.group('comment')))
+ self.assertEqual(result.group('not_comment'), not_comment, 'Expected not comment "%s", got "%s"'
+ % (not_comment, result.group('not_comment')))
+ else:
+ self.assertEqual(None, result, 'Found an unexpected comment "%s" in "%s"'
+ % ("" if result is None else result.group('comment'), line ))
+
+# Tuples of (string, expected result), where expected result is False if
+# the string should not be considered as having a comment, or a second
+# tuple of the not comment and comment sections split apart
+regex_split_comment_testcases = [
+ ('dbus send # this is a comment', ('dbus send ', '# this is a comment')),
+ ('dbus send member=no_comment', False),
+ ('dbus send member=no_comment, ', False),
+ ('audit "/tmp/foo, # bar" rw', False),
+ ('audit "/tmp/foo, # bar" rw # comment', ('audit "/tmp/foo, # bar" rw ', '# comment')),
+]
+
+def setup_split_comment_testcases():
+ i = 0
+ for (test_string, result) in regex_split_comment_testcases:
+ i += 1
+ def stub_test(self, test_string=test_string, result=result):
+ if result is False:
+ self._check(test_string, False)
+ else:
+ self._check(test_string, True, not_comment=result[0], comment=result[1])
+ stub_test.__doc__ = "test '%s'" % (test_string)
+ setattr(AARegexSplitComment, 'test_split_comment_%d' % (i), stub_test)
+
+if __name__ == '__main__':
+ verbosity = 2
+
+ setup_has_comma_testcases()
+ setup_split_comment_testcases()
+
+ test_suite = unittest.TestSuite()
+ test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexHasComma))
+ test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexSplitComment))
+ result = unittest.TextTestRunner(verbosity=verbosity).run(test_suite)
+ if not result.wasSuccessful():
+ exit(1)
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
