[apparmor] [PATCH] utils: Don't enforce ordering of dbus rule attributes

Tyler Hicks tyhicks at canonical.com
Wed Feb 8 23:56:27 UTC 2017


https://launchpad.net/bugs/1628286

The utils were enforcing that the dbus rule attributes were strictly
ordered in the following fashion:

 bus -> path -> interface -> member -> peer

However, the parser has always accepted the attributes in any order. If
the system contained a profile which did not use the strict ordering
enforced by the utils, the utils would refuse to operate at all.

This patch eases the restriction on the ordering at the expense of the
utils no longer being able to detect and reject a single attribute that
is repeated multiple times. In that situation, only the last occurrence
of the attribute will be honored by the utils.

Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
Cc: Christian Boltz <apparmor at cboltz.de>
---
 utils/apparmor/rule/dbus.py            | 12 ++++++------
 utils/test/test-dbus.py                |  6 ++++++
 utils/test/test-parser-simple-tests.py |  8 +++-----
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/utils/apparmor/rule/dbus.py b/utils/apparmor/rule/dbus.py
index 60f1ecf..58dc7b5 100644
--- a/utils/apparmor/rule/dbus.py
+++ b/utils/apparmor/rule/dbus.py
@@ -40,11 +40,11 @@ RE_FLAG         = '(?P<%s>(\S+|"[^"]+"|\(\s*\S+\s*\)|\(\s*"[^"]+"\)\s*))'    # s
 RE_DBUS_DETAILS  = re.compile(
     '^' +
     '(\s+(?P<access>'       + RE_ACCESS_KEYWORDS + '))?' +  # optional access keyword(s)
-    '(\s+(bus\s*=\s*'       + RE_FLAG % 'bus'       + '))?' +  # optional bus= system | session | AARE, (...) optional
-    '(\s+(path\s*=\s*'      + RE_FLAG % 'path'      + '))?' +  # optional path=AARE, (...) optional
-    '(\s+(name\s*=\s*'      + RE_FLAG % 'name'      + '))?' +  # optional name=AARE, (...) optional
-    '(\s+(interface\s*=\s*' + RE_FLAG % 'interface' + '))?' +  # optional interface=AARE, (...) optional
-    '(\s+(member\s*=\s*'    + RE_FLAG % 'member'    + '))?' +  # optional member=AARE, (...) optional
+    '((\s+(bus\s*=\s*'      + RE_FLAG % 'bus'       + '))?|' +  # optional bus= system | session | AARE, (...) optional
+    '(\s+(path\s*=\s*'      + RE_FLAG % 'path'      + '))?|' +  # optional path=AARE, (...) optional
+    '(\s+(name\s*=\s*'      + RE_FLAG % 'name'      + '))?|' +  # optional name=AARE, (...) optional
+    '(\s+(interface\s*=\s*' + RE_FLAG % 'interface' + '))?|' +  # optional interface=AARE, (...) optional
+    '(\s+(member\s*=\s*'    + RE_FLAG % 'member'    + '))?|' +  # optional member=AARE, (...) optional
     '(\s+(peer\s*=\s*\((,|\s)*'    +  # optional peer=( name=AARE and/or label=AARE ), (...) required
             '(' +
                 '(' + '(,|\s)*' + ')' +  # empty peer=()
@@ -57,7 +57,7 @@ RE_DBUS_DETAILS  = re.compile(
                 '|'  # or
                 '(' + 'label\s*=\s*' + RE_PROFILE_NAME % 'peerlabel3' + '(,|\s)+' + 'name\s*=\s*'  + RE_PROFILE_NAME % 'peername3'  + ')' +  # peer label + name (match name peername3/peerlabel3)
             ')'
-        '(,|\s)*\)))?'
+        '(,|\s)*\)))?){0,6}'
     '\s*$')
 
 
diff --git a/utils/test/test-dbus.py b/utils/test/test-dbus.py
index 5b676bc..f1bcb25 100644
--- a/utils/test/test-dbus.py
+++ b/utils/test/test-dbus.py
@@ -89,6 +89,10 @@ class DbusTestParse(DbusTest):
         ('dbus peer=(, label = bar,  name = foo  ),'    , exp(False, False, False, '',        None  ,               True ,  None,       True,   None,           True,   None,       True,   None,       True,   None,   True,   'foo',      False,  'bar,',     False)),  # XXX peerlabel includes the comma
         ('dbus peer=(  label = bar , name = foo  ),'    , exp(False, False, False, '',        None  ,               True ,  None,       True,   None,           True,   None,       True,   None,       True,   None,   True,   'foo',      False,  'bar',      False)),
         ('dbus peer=(  label = "bar"  name = "foo" ),'  , exp(False, False, False, '',        None  ,               True ,  None,       True,   None,           True,   None,       True,   None,       True,   None,   True,   'foo',      False,  'bar',      False)),
+        ('dbus path=/foo/bar bus=session,'              , exp(False, False, False, '',        None  ,               True ,  'session',  False,  '/foo/bar',     False,  None,       True,   None,       True,   None,   True,   None,       True,   None,       True)),
+        ('dbus bus=system path=/foo/bar bus=session,'   , exp(False, False, False, '',        None  ,               True ,  'session',  False,  '/foo/bar',     False,  None,       True,   None,       True,   None,   True,   None,       True,   None,       True)),
+        ('dbus send peer=(label="foo") bus=session,'    , exp(False, False, False, '',        {'send'},             False,  'session',  False,  None,           True,   None,       True,   None,       True,   None,   True,   None,       True,   'foo',      False)),
+        ('dbus bus=1 bus=2 bus=3 bus=4 bus=5 bus=6,'    , exp(False, False, False, '',        None  ,               True ,  '6',        False,  None,           True,   None,       True,   None,       True,   None,   True,   None,       True,   None,       True)),
     ]
 
     def _run_test(self, rawrule, expected):
@@ -108,6 +112,8 @@ class DbusTestParseInvalid(DbusTest):
         ('dbus peer=(label=foo) path=,'  , AppArmorException),
         ('dbus (invalid),'               , AppArmorException),
         ('dbus peer=,'                   , AppArmorException),
+        ('dbus bus=session bind bus=system,', AppArmorException),
+        ('dbus bus=1 bus=2 bus=3 bus=4 bus=5 bus=6 bus=7,', AppArmorException),
     ]
 
     def _run_test(self, rawrule, expected):
diff --git a/utils/test/test-parser-simple-tests.py b/utils/test/test-parser-simple-tests.py
index 11617b5..92d81c3 100644
--- a/utils/test/test-parser-simple-tests.py
+++ b/utils/test/test-parser-simple-tests.py
@@ -52,6 +52,7 @@ exception_not_raised = [
     'change_profile/onx_conflict_unsafe1.sd',
     'change_profile/onx_conflict_unsafe2.sd',
 
+    'dbus/bad_modifier_2.sd',
     'dbus/bad_regex_01.sd',
     'dbus/bad_regex_02.sd',
     'dbus/bad_regex_03.sd',
@@ -154,6 +155,8 @@ exception_not_raised = [
     'vars/vars_dbus_bad_01.sd',
     'vars/vars_dbus_bad_02.sd',
     'vars/vars_dbus_bad_03.sd',
+    'vars/vars_dbus_bad_04.sd',
+    'vars/vars_dbus_bad_05.sd',
     'vars/vars_dbus_bad_06.sd',
     'vars/vars_dbus_bad_07.sd',
     'vars/vars_file_evaluation_7.sd',
@@ -262,11 +265,6 @@ syntax_failure = [
     'xtrans/simple_ok_pix_1.sd',  # Invalid mode pIx
     'xtrans/simple_ok_pux_1.sd',  # Invalid mode rPux
 
-    # dbus regex mismatch
-    'vars/vars_dbus_4.sd',
-    'vars/vars_dbus_9.sd',
-    'vars/vars_dbus_2.sd',
-
     # misc
     'vars/vars_dbus_8.sd',  # Path doesn't start with / or variable: {/@{TLDS}/foo,/com/@{DOMAINS}}
     'vars/vars_simple_assignment_12.sd',  # Redefining existing variable @{BAR} ('\' not handled)
-- 
2.7.4




More information about the AppArmor mailing list