diff --git a/parser/tst/simple_tests/vars/vars_dbus_12.sd b/parser/tst/simple_tests/vars/vars_dbus_12.sd new file mode 100644 index 000000000..641c6e1b9 --- /dev/null +++ b/parser/tst/simple_tests/vars/vars_dbus_12.sd @@ -0,0 +1,13 @@ +#=DESCRIPTION reference variables in dbus rules, embedded within alternation +#=EXRESULT PASS + +@{TLDS}=com org +@{DOMAINS}=gnome freedesktop + +/does/not/exist { + dbus (send, receive) + bus=session + path={{/@{TLDS}/foo,/com/@{DOMAINS}},{/@{TLDS}/FOO,/com/@{DOMAINS}/FOO}} + interface=@{TLDS}.freedesktop + peer=(name=@{TLDS}.freedesktop label=/@{TLDS}/freedesktop), +} diff --git a/utils/apparmor/aare.py b/utils/apparmor/aare.py index c38423baf..535512aa3 100644 --- a/utils/apparmor/aare.py +++ b/utils/apparmor/aare.py @@ -17,6 +17,16 @@ import re from apparmor.common import convert_regexp, AppArmorBug, AppArmorException +path_entry = r"(/[^\n,]*|(@{[^\n,}]*}[^\n,]*))" +# XXX Matching using this regex is not perfect but should be enough for practical scenarios. Limitations are: +# - We do not look recursively '{', therefore some weird-but-valid string such as {{/foo,/bar},{/baz,/qux}} are denied +# - Variables are not replaced: we have to accept "@{" regardless of whether it contains an actual path +AARE_PATH_REGEX = re.compile( + r"^" + path_entry + + r"|{" + path_entry + "," + path_entry + "*}$" +) + + class AARE: """AARE (AppArmor Regular Expression) wrapper class""" @@ -25,13 +35,8 @@ class AARE: 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 is_path and not AARE_PATH_REGEX.match(regex): + raise AppArmorException("Path doesn't start with / or variable: %s" % regex) if log_event: self.orig_regex = regex diff --git a/utils/test/test-aare.py b/utils/test/test-aare.py index 1593bfca2..442fba04b 100644 --- a/utils/test/test-aare.py +++ b/utils/test/test-aare.py @@ -238,6 +238,8 @@ class TestAAREIsPath(AATest): (('/foo*', True, '/foobar'), True), (('@{PROC}/', True, '/foobar'), False), (('foo*', False, 'foobar'), True), + (('{/a,/b}', True, '/a'), True), + (('{@{X},/b}', True, '/b'), True), ) def _run_test(self, params, expected): @@ -249,6 +251,18 @@ class TestAAREIsPath(AATest): with self.assertRaises(AppArmorException): AARE('foo*', True) + def test_path_bad_alternative(self): + with self.assertRaises(AppArmorException): + AARE('{/a,@{foo},invalid}', True) + + def test_path_bad_alternative2(self): + with self.assertRaises(AppArmorException): + AARE('{invalid,@{X},/X}', True) + + def test_path_bad_alternative3(self): + with self.assertRaises(AppArmorException): + AARE('{/foo,@{invalid,/X}', True) + class TestAARERepr(AATest): def test_repr(self): diff --git a/utils/test/test-parser-simple-tests.py b/utils/test/test-parser-simple-tests.py index 5f24bcf79..42f483390 100644 --- a/utils/test/test-parser-simple-tests.py +++ b/utils/test/test-parser-simple-tests.py @@ -433,7 +433,7 @@ syntax_failure = ( 'file/ok_quoted_4.sd', # quoted string including \" # misc - 'vars/vars_dbus_8.sd', # Path doesn't start with / or variable: {/@{TLDS}/foo,/com/@{DOMAINS}} + 'vars/vars_dbus_12.sd', # AARE starting with {{ are not handled 'vars/vars_simple_assignment_12.sd', # Redefining existing variable @{BAR} ('\' not handled) 'bare_include_tests/ok_2.sd', # two #include<...> in one line