Merge util: enhance AARE file path validation

Fixes https://bugs.launchpad.net/apparmor/+bug/2106033

Improve the validation of AARE file paths by introducing a new regex
that supports paths starting with '{' (e.g. '{/,/org/freedesktop/DBus}').
These paths are notably used in snap.lxd.* profiles.

Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1607
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>

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),
+}

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

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):

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