[apparmor] [PATCH v2] utils: Handle the safe/unsafe change_profile exec modes

Tyler Hicks tyhicks at canonical.com
Sat Jul 16 01:34:47 UTC 2016


https://launchpad.net/bugs/1584069

This patch adds support for the safe and unsafe exec modes for
change_profile rules. The logic is pretty simple at this point because
the kernel's default for exec modes changed in newer versions.
Therefore, this patch simply retains any specified exec mode in parsed
rules. If an exec mode is not specified in a rule, there is no attempt
to force the usage of "safe" because older kernels do not support it.

Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
Acked-by: Seth Arnold <seth.arnold at canonical.com>
---

* Changes since v1:
  - Added Seth's acked-by
  - Addressed feedback from Christian
    + Embed execmode name in RE_SAFE_OR_UNSAFE
    + AppArmorBug() when an invalid execmode is used in a new
      ChangeProfileRule()
    + Don't use logprof_value_or_all() when setting execmode_txt
    + Only return "Exec Mode" element from logprof_header_localvars() when
      an execmode is set
    + Add invalid execcmode test to InvalidChangeProfileInit()
    + Make 'safe' execmode equivalent to '' and None

 utils/apparmor/regex.py                |   2 +
 utils/apparmor/rule/change_profile.py  |  41 ++++++++--
 utils/test/test-change_profile.py      | 136 +++++++++++++++++++--------------
 utils/test/test-parser-simple-tests.py |   5 ++
 4 files changed, 123 insertions(+), 61 deletions(-)

diff --git a/utils/apparmor/regex.py b/utils/apparmor/regex.py
index 756c3e0..1e7e16d 100644
--- a/utils/apparmor/regex.py
+++ b/utils/apparmor/regex.py
@@ -30,6 +30,7 @@ RE_PROFILE_NAME         = '(?P<%s>(\S+|"[^"]+"))'    # string without spaces, or
 RE_PATH                 = '/\S+|"/[^"]+"'  # filename (starting with '/') without spaces, or quoted filename.
 RE_PROFILE_PATH         = '(?P<%s>(' + RE_PATH + '))'  # quoted or unquoted filename. %s is the match group name
 RE_PROFILE_PATH_OR_VAR  = '(?P<%s>(' + RE_PATH + '|@{\S+}\S*|"@{\S+}[^"]*"))'  # quoted or unquoted filename or variable. %s is the match group name
+RE_SAFE_OR_UNSAFE       = '(?P<execmode>(safe|unsafe))'
 
 RE_PROFILE_END          = re.compile('^\s*\}' + RE_EOL)
 RE_PROFILE_CAP          = re.compile(RE_AUDIT_DENY + 'capability(?P<capability>(\s+\S+)+)?' + RE_COMMA_EOL)
@@ -77,6 +78,7 @@ RE_PROFILE_START          = re.compile(
 RE_PROFILE_CHANGE_PROFILE = re.compile(
     RE_AUDIT_DENY +
     'change_profile' +
+    '(\s+' + RE_SAFE_OR_UNSAFE + ')?' +  # optionally exec mode
     '(\s+' + RE_PROFILE_PATH_OR_VAR % 'execcond' + ')?' +  # optionally exec condition
     '(\s+->\s*' + RE_PROFILE_NAME % 'targetprofile' + ')?' +  # optionally '->' target profile
     RE_COMMA_EOL)
diff --git a/utils/apparmor/rule/change_profile.py b/utils/apparmor/rule/change_profile.py
index 6063b70..7eb5062 100644
--- a/utils/apparmor/rule/change_profile.py
+++ b/utils/apparmor/rule/change_profile.py
@@ -34,11 +34,13 @@ class ChangeProfileRule(BaseRule):
 
     rule_name = 'change_profile'
 
-    def __init__(self, execcond, targetprofile, audit=False, deny=False, allow_keyword=False,
+    equiv_execmodes = [ 'safe', '', None ]
+
+    def __init__(self, execmode, execcond, targetprofile, audit=False, deny=False, allow_keyword=False,
                  comment='', log_event=None):
 
         '''
-            CHANGE_PROFILE RULE = 'change_profile' [ EXEC COND ] [ -> PROGRAMCHILD ]
+            CHANGE_PROFILE RULE = 'change_profile' [ [ EXEC MODE ] EXEC COND ] [ -> PROGRAMCHILD ]
         '''
 
         super(ChangeProfileRule, self).__init__(audit=audit, deny=deny,
@@ -46,6 +48,13 @@ class ChangeProfileRule(BaseRule):
                                              comment=comment,
                                              log_event=log_event)
 
+        if execmode:
+            if execmode != 'safe' and execmode != 'unsafe':
+                raise AppArmorBug('Unknown exec mode (%s) in change_profile rule' % execmode)
+            elif not execcond or execcond == ChangeProfileRule.ALL:
+                raise AppArmorException('Exec condition is required when unsafe or safe keywords are present')
+        self.execmode = execmode
+
         self.execcond = None
         self.all_execconds = False
         if execcond == ChangeProfileRule.ALL:
@@ -86,6 +95,8 @@ class ChangeProfileRule(BaseRule):
 
         audit, deny, allow_keyword, comment = parse_modifiers(matches)
 
+        execmode = matches.group('execmode')
+
         if matches.group('execcond'):
             execcond = strip_quotes(matches.group('execcond'))
         else:
@@ -96,7 +107,7 @@ class ChangeProfileRule(BaseRule):
         else:
             targetprofile = ChangeProfileRule.ALL
 
-        return ChangeProfileRule(execcond, targetprofile,
+        return ChangeProfileRule(execmode, execcond, targetprofile,
                            audit=audit, deny=deny, allow_keyword=allow_keyword, comment=comment)
 
     def get_clean(self, depth=0):
@@ -104,6 +115,11 @@ class ChangeProfileRule(BaseRule):
 
         space = '  ' * depth
 
+        if self.execmode:
+            execmode = ' %s' % self.execmode
+        else:
+            execmode = ''
+
         if self.all_execconds:
             execcond = ''
         elif self.execcond:
@@ -118,11 +134,16 @@ class ChangeProfileRule(BaseRule):
         else:
             raise AppArmorBug('Empty target profile in change_profile rule')
 
-        return('%s%schange_profile%s%s,%s' % (space, self.modifiers_str(), execcond, targetprofile, self.comment))
+        return('%s%schange_profile%s%s%s,%s' % (space, self.modifiers_str(), execmode, execcond, targetprofile, self.comment))
 
     def is_covered_localvars(self, other_rule):
         '''check if other_rule is covered by this rule object'''
 
+        if self.execmode != other_rule.execmode and \
+           (self.execmode not in ChangeProfileRule.equiv_execmodes or \
+            other_rule.execmode not in ChangeProfileRule.equiv_execmodes):
+            return False
+
         if not self._is_covered_plain(self.execcond, self.all_execconds, other_rule.execcond, other_rule.all_execconds, 'exec condition'):
             # TODO: honor globbing and variables
             return False
@@ -139,6 +160,11 @@ class ChangeProfileRule(BaseRule):
         if not type(rule_obj) == ChangeProfileRule:
             raise AppArmorBug('Passed non-change_profile rule: %s' % str(rule_obj))
 
+        if self.execmode != rule_obj.execmode and \
+           (self.execmode not in ChangeProfileRule.equiv_execmodes or \
+            rule_obj.execmode not in ChangeProfileRule.equiv_execmodes):
+            return False
+
         if (self.execcond != rule_obj.execcond
                 or self.all_execconds != rule_obj.all_execconds):
             return False
@@ -150,10 +176,15 @@ class ChangeProfileRule(BaseRule):
         return True
 
     def logprof_header_localvars(self):
+        headers = []
+
+        if self.execmode:
+            headers += [_('Exec Mode'), self.execmode]
+
         execcond_txt        = logprof_value_or_all(self.execcond,       self.all_execconds)
         targetprofiles_txt  = logprof_value_or_all(self.targetprofile,  self.all_targetprofiles)
 
-        return [
+        return headers + [
             _('Exec Condition'), execcond_txt,
             _('Target Profile'), targetprofiles_txt,
         ]
diff --git a/utils/test/test-change_profile.py b/utils/test/test-change_profile.py
index 40003d2..32a684c 100644
--- a/utils/test/test-change_profile.py
+++ b/utils/test/test-change_profile.py
@@ -25,7 +25,7 @@ from apparmor.translations import init_translation
 _ = init_translation()
 
 exp = namedtuple('exp', ['audit', 'allow_keyword', 'deny', 'comment',
-        'execcond', 'all_execconds', 'targetprofile', 'all_targetprofiles'])
+        'execmode', 'execcond', 'all_execconds', 'targetprofile', 'all_targetprofiles'])
 
 # --- tests for single ChangeProfileRule --- #
 
@@ -33,6 +33,7 @@ class ChangeProfileTest(AATest):
     def _compare_obj(self, obj, expected):
         self.assertEqual(expected.allow_keyword, obj.allow_keyword)
         self.assertEqual(expected.audit, obj.audit)
+        self.assertEqual(expected.execmode, obj.execmode)
         self.assertEqual(expected.execcond, obj.execcond)
         self.assertEqual(expected.targetprofile, obj.targetprofile)
         self.assertEqual(expected.all_execconds, obj.all_execconds)
@@ -42,29 +43,33 @@ class ChangeProfileTest(AATest):
 
 class ChangeProfileTestParse(ChangeProfileTest):
     tests = [
-        # rawrule                                            audit  allow  deny   comment        execcond  all?   targetprof  all?
-        ('change_profile,'                             , exp(False, False, False, ''           , None  ,   True , None     , True )),
-        ('change_profile /foo,'                        , exp(False, False, False, ''           , '/foo',   False, None     , True )),
-        ('change_profile /foo -> /bar,'                , exp(False, False, False, ''           , '/foo',   False, '/bar'   , False)),
-        ('deny change_profile /foo -> /bar, # comment' , exp(False, False, True , ' # comment' , '/foo',   False, '/bar'   , False)),
-        ('audit allow change_profile /foo,'            , exp(True , True , False, ''           , '/foo',   False, None     , True )),
-        ('change_profile -> /bar,'                     , exp(False, False, False, ''           , None  ,   True , '/bar'   , False)),
-        ('audit allow change_profile -> /bar,'         , exp(True , True , False, ''           , None  ,   True , '/bar'   , False)),
+        # rawrule                                            audit  allow  deny   comment        execmode    execcond  all?   targetprof  all?
+        ('change_profile,'                             , exp(False, False, False, ''           , None      , None  ,   True , None     , True )),
+        ('change_profile /foo,'                        , exp(False, False, False, ''           , None      , '/foo',   False, None     , True )),
+        ('change_profile safe /foo,'                   , exp(False, False, False, ''           , 'safe'    , '/foo',   False, None     , True )),
+        ('change_profile unsafe /foo,'                 , exp(False, False, False, ''           , 'unsafe'  , '/foo',   False, None     , True )),
+        ('change_profile /foo -> /bar,'                , exp(False, False, False, ''           , None      , '/foo',   False, '/bar'   , False)),
+        ('change_profile safe /foo -> /bar,'           , exp(False, False, False, ''           , 'safe'    , '/foo',   False, '/bar'   , False)),
+        ('change_profile unsafe /foo -> /bar,'         , exp(False, False, False, ''           , 'unsafe'  , '/foo',   False, '/bar'   , False)),
+        ('deny change_profile /foo -> /bar, # comment' , exp(False, False, True , ' # comment' , None      , '/foo',   False, '/bar'   , False)),
+        ('audit allow change_profile safe /foo,'       , exp(True , True , False, ''           , 'safe'    , '/foo',   False, None     , True )),
+        ('change_profile -> /bar,'                     , exp(False, False, False, ''           , None      , None  ,   True , '/bar'   , False)),
+        ('audit allow change_profile -> /bar,'         , exp(True , True , False, ''           , None      , None  ,   True , '/bar'   , False)),
         # quoted versions
-        ('change_profile "/foo",'                      , exp(False, False, False, ''           , '/foo',   False, None     , True )),
-        ('change_profile "/foo" -> "/bar",'            , exp(False, False, False, ''           , '/foo',   False, '/bar'   , False)),
-        ('deny change_profile "/foo" -> "/bar", # cmt' , exp(False, False, True, ' # cmt'      , '/foo',   False, '/bar'   , False)),
-        ('audit allow change_profile "/foo",'          , exp(True , True , False, ''           , '/foo',   False, None     , True )),
-        ('change_profile -> "/bar",'                   , exp(False, False, False, ''           , None  ,   True , '/bar'   , False)),
-        ('audit allow change_profile -> "/bar",'       , exp(True , True , False, ''           , None  ,   True , '/bar'   , False)),
+        ('change_profile "/foo",'                      , exp(False, False, False, ''           , None      , '/foo',   False, None     , True )),
+        ('change_profile "/foo" -> "/bar",'            , exp(False, False, False, ''           , None      , '/foo',   False, '/bar'   , False)),
+        ('deny change_profile "/foo" -> "/bar", # cmt' , exp(False, False, True, ' # cmt'      , None      , '/foo',   False, '/bar'   , False)),
+        ('audit allow change_profile "/foo",'          , exp(True , True , False, ''           , None      , '/foo',   False, None     , True )),
+        ('change_profile -> "/bar",'                   , exp(False, False, False, ''           , None      , None  ,   True , '/bar'   , False)),
+        ('audit allow change_profile -> "/bar",'       , exp(True , True , False, ''           , None      , None  ,   True , '/bar'   , False)),
         # with globbing and/or named profiles
-        ('change_profile,'                             , exp(False, False, False, ''           , None  ,   True , None     , True )),
-        ('change_profile /*,'                          , exp(False, False, False, ''           , '/*'  ,   False, None     , True )),
-        ('change_profile /* -> bar,'                   , exp(False, False, False, ''           , '/*'  ,   False, 'bar'    , False)),
-        ('deny change_profile /** -> bar, # comment'   , exp(False, False, True , ' # comment' , '/**' ,   False, 'bar'    , False)),
-        ('audit allow change_profile /**,'             , exp(True , True , False, ''           , '/**' ,   False, None     , True )),
-        ('change_profile -> "ba r",'                   , exp(False, False, False, ''           , None  ,   True , 'ba r'   , False)),
-        ('audit allow change_profile -> "ba r",'       , exp(True , True , False, ''           , None  ,   True , 'ba r'   , False)),
+        ('change_profile,'                             , exp(False, False, False, ''           , None      , None  ,   True , None     , True )),
+        ('change_profile /*,'                          , exp(False, False, False, ''           , None      , '/*'  ,   False, None     , True )),
+        ('change_profile /* -> bar,'                   , exp(False, False, False, ''           , None      , '/*'  ,   False, 'bar'    , False)),
+        ('deny change_profile /** -> bar, # comment'   , exp(False, False, True , ' # comment' , None      , '/**' ,   False, 'bar'    , False)),
+        ('audit allow change_profile /**,'             , exp(True , True , False, ''           , None      , '/**' ,   False, None     , True )),
+        ('change_profile -> "ba r",'                   , exp(False, False, False, ''           , None      , None  ,   True , 'ba r'   , False)),
+        ('audit allow change_profile -> "ba r",'       , exp(True , True , False, ''           , None      , None  ,   True , 'ba r'   , False)),
      ]
 
     def _run_test(self, rawrule, expected):
@@ -77,6 +82,8 @@ class ChangeProfileTestParseInvalid(ChangeProfileTest):
     tests = [
         ('change_profile -> ,'                     , AppArmorException),
         ('change_profile foo -> ,'                 , AppArmorException),
+        ('change_profile notsafe,'                 , AppArmorException),
+        ('change_profile safety -> /bar,'          , AppArmorException),
     ]
 
     def _run_test(self, rawrule, expected):
@@ -116,10 +123,10 @@ class ChangeProfileTestParseFromLog(ChangeProfileTest):
             'name': None,
         })
 
-        obj = ChangeProfileRule(ChangeProfileRule.ALL, parsed_event['name2'], log_event=parsed_event)
+        obj = ChangeProfileRule(None, ChangeProfileRule.ALL, parsed_event['name2'], log_event=parsed_event)
 
-        #              audit  allow  deny   comment        execcond  all?   targetprof     all?
-        expected = exp(False, False, False, ''           , None,     True,  '/foo/rename', False)
+        #              audit  allow  deny   comment        execmode execcond  all?   targetprof     all?
+        expected = exp(False, False, False, ''           , None,    None,     True,  '/foo/rename', False)
 
         self._compare_obj(obj, expected)
 
@@ -128,13 +135,15 @@ class ChangeProfileTestParseFromLog(ChangeProfileTest):
 
 class ChangeProfileFromInit(ChangeProfileTest):
     tests = [
-        # ChangeProfileRule object                                  audit  allow  deny   comment        execcond    all?   targetprof  all?
-        (ChangeProfileRule('/foo', '/bar', deny=True)          , exp(False, False, True , ''           , '/foo',   False, '/bar'    , False)),
-        (ChangeProfileRule('/foo', '/bar')                     , exp(False, False, False, ''           , '/foo',   False, '/bar'    , False)),
-        (ChangeProfileRule('/foo', ChangeProfileRule.ALL)      , exp(False, False, False, ''           , '/foo',   False,  None     , True )),
-        (ChangeProfileRule(ChangeProfileRule.ALL, '/bar')      , exp(False, False, False, ''           , None  ,   True , '/bar'    , False)),
-        (ChangeProfileRule(ChangeProfileRule.ALL,
-                             ChangeProfileRule.ALL)            , exp(False, False, False, ''           , None  ,   True , None      , True )),
+        # ChangeProfileRule object                                             audit  allow  deny   comment        execmode execcond    all?   targetprof  all?
+        (ChangeProfileRule(None    , '/foo', '/bar', deny=True)          , exp(False, False, True , ''           , None    , '/foo',   False, '/bar'    , False)),
+        (ChangeProfileRule(None    , '/foo', '/bar')                     , exp(False, False, False, ''           , None    , '/foo',   False, '/bar'    , False)),
+        (ChangeProfileRule('safe'  , '/foo', '/bar')                     , exp(False, False, False, ''           , 'safe'  , '/foo',   False, '/bar'    , False)),
+        (ChangeProfileRule('unsafe', '/foo', '/bar')                     , exp(False, False, False, ''           , 'unsafe', '/foo',   False, '/bar'    , False)),
+        (ChangeProfileRule(None    , '/foo', ChangeProfileRule.ALL)      , exp(False, False, False, ''           , None  , '/foo',   False,  None     , True )),
+        (ChangeProfileRule(None    , ChangeProfileRule.ALL, '/bar')      , exp(False, False, False, ''           , None  , None  ,   True , '/bar'    , False)),
+        (ChangeProfileRule(None    , ChangeProfileRule.ALL,
+                             ChangeProfileRule.ALL)            , exp(False, False, False, ''           , None, None  ,   True , None      , True )),
     ]
 
     def _run_test(self, obj, expected):
@@ -144,20 +153,21 @@ class ChangeProfileFromInit(ChangeProfileTest):
 class InvalidChangeProfileInit(AATest):
     tests = [
         # init params                     expected exception
-        (['/foo', ''               ]    , AppArmorBug), # empty targetprofile
-        ([''    , '/bar'           ]    , AppArmorBug), # empty execcond
-        (['    ', '/bar'           ]    , AppArmorBug), # whitespace execcond
-        (['/foo', '   '            ]    , AppArmorBug), # whitespace targetprofile
-        (['xyxy', '/bar'           ]    , AppArmorException), # invalid execcond
-        ([dict(), '/bar'           ]    , AppArmorBug), # wrong type for execcond
-        ([None  , '/bar'           ]    , AppArmorBug), # wrong type for execcond
-        (['/foo', dict()           ]    , AppArmorBug), # wrong type for targetprofile
-        (['/foo', None             ]    , AppArmorBug), # wrong type for targetprofile
+        ([None    , '/foo', ''               ]    , AppArmorBug), # empty targetprofile
+        ([None    , ''    , '/bar'           ]    , AppArmorBug), # empty execcond
+        ([None    , '    ', '/bar'           ]    , AppArmorBug), # whitespace execcond
+        ([None    , '/foo', '   '            ]    , AppArmorBug), # whitespace targetprofile
+        ([None    , 'xyxy', '/bar'           ]    , AppArmorException), # invalid execcond
+        ([None    , dict(), '/bar'           ]    , AppArmorBug), # wrong type for execcond
+        ([None    , None  , '/bar'           ]    , AppArmorBug), # wrong type for execcond
+        ([None    , '/foo', dict()           ]    , AppArmorBug), # wrong type for targetprofile
+        ([None    , '/foo', None             ]    , AppArmorBug), # wrong type for targetprofile
+        (['maybe' , '/foo', '/bar'           ]    , AppArmorBug), # invalid keyword for execmode
     ]
 
     def _run_test(self, params, expected):
         with self.assertRaises(expected):
-            ChangeProfileRule(params[0], params[1])
+            ChangeProfileRule(params[0], params[1], params[2])
 
     def test_missing_params_1(self):
         with self.assertRaises(TypeError):
@@ -184,14 +194,14 @@ class InvalidChangeProfileTest(AATest):
         self._check_invalid_rawrule('dbus,')  # not a change_profile rule
 
     def test_empty_net_data_1(self):
-        obj = ChangeProfileRule('/foo', '/bar')
+        obj = ChangeProfileRule(None, '/foo', '/bar')
         obj.execcond = ''
         # no execcond set, and ALL not set
         with self.assertRaises(AppArmorBug):
             obj.get_clean(1)
 
     def test_empty_net_data_2(self):
-        obj = ChangeProfileRule('/foo', '/bar')
+        obj = ChangeProfileRule(None, '/foo', '/bar')
         obj.targetprofile = ''
         # no targetprofile set, and ALL not set
         with self.assertRaises(AppArmorBug):
@@ -206,7 +216,7 @@ class WriteChangeProfileTestAATest(AATest):
         ('   deny change_profile         /foo      -> bar,# foo bar'   , 'deny change_profile /foo -> bar, # foo bar'),
         ('   deny change_profile         /foo      ,# foo bar'         , 'deny change_profile /foo, # foo bar'),
         ('   allow change_profile   ->    /bar     ,# foo bar'         , 'allow change_profile -> /bar, # foo bar'),
-        ('   allow change_profile   /** ->    /bar     ,# foo bar'     , 'allow change_profile /** -> /bar, # foo bar'),
+        ('   allow change_profile   unsafe  /** ->    /bar     ,# foo bar'     , 'allow change_profile unsafe /** -> /bar, # foo bar'),
         ('   allow change_profile   "/fo o" ->    "/b ar",'            , 'allow change_profile "/fo o" -> "/b ar",'),
     ]
 
@@ -220,7 +230,7 @@ class WriteChangeProfileTestAATest(AATest):
         self.assertEqual(rawrule.strip(), raw, 'unexpected raw rule')
 
     def test_write_manually(self):
-        obj = ChangeProfileRule('/foo', 'bar', allow_keyword=True)
+        obj = ChangeProfileRule(None, '/foo', 'bar', allow_keyword=True)
 
         expected = '    allow change_profile /foo -> bar,'
 
@@ -248,6 +258,8 @@ class ChangeProfileCoveredTest_01(ChangeProfileCoveredTest):
         #   rule                                        equal     strict equal    covered     covered exact
         ('           change_profile,'               , [ False   , False         , False     , False     ]),
         ('           change_profile /foo,'          , [ True    , True          , True      , True      ]),
+        ('           change_profile safe /foo,'     , [ True    , False         , True      , True      ]),
+        ('           change_profile unsafe /foo,'   , [ False   , False         , False     , False     ]),
         ('           change_profile /foo, # comment', [ True    , False         , True      , True      ]),
         ('     allow change_profile /foo,'          , [ True    , False         , True      , True      ]),
         ('           change_profile     /foo,'      , [ True    , False         , True      , True      ]),
@@ -269,6 +281,7 @@ class ChangeProfileCoveredTest_02(ChangeProfileCoveredTest):
         (      'change_profile /foo,'              , [ False   , False         , True      , False     ]),
         ('audit change_profile /foo,'              , [ True    , True          , True      , True      ]),
         (      'change_profile /foo -> /bar,'      , [ False   , False         , True      , False     ]),
+        (      'change_profile safe /foo -> /bar,' , [ False   , False         , True      , False     ]),
         ('audit change_profile /foo -> /bar,'      , [ False   , False         , True      , True      ]), # XXX is "covered exact" correct here?
         (      'change_profile,'                   , [ False   , False         , False     , False     ]),
         ('audit change_profile,'                   , [ False   , False         , False     , False     ]),
@@ -319,12 +332,23 @@ class ChangeProfileCoveredTest_05(ChangeProfileCoveredTest):
         (      'deny change_profile,'              , [ False   , False         , False     , False     ]),
     ]
 
+class ChangeProfileCoveredTest_06(ChangeProfileCoveredTest):
+    rule = 'change_profile safe /foo,'
+
+    tests = [
+        #   rule                                       equal     strict equal    covered     covered exact
+        (      'deny change_profile /foo,'         , [ False   , False         , False     , False     ]),
+        ('audit deny change_profile /foo,'         , [ False   , False         , False     , False     ]),
+        (           'change_profile /foo,'         , [ True    , False         , True      , True      ]),
+        (      'deny change_profile /bar,'         , [ False   , False         , False     , False     ]),
+        (      'deny change_profile,'              , [ False   , False         , False     , False     ]),
+    ]
 
 class ChangeProfileCoveredTest_Invalid(AATest):
     def test_borked_obj_is_covered_1(self):
         obj = ChangeProfileRule.parse('change_profile /foo,')
 
-        testobj = ChangeProfileRule('/foo', '/bar')
+        testobj = ChangeProfileRule(None, '/foo', '/bar')
         testobj.execcond = ''
 
         with self.assertRaises(AppArmorBug):
@@ -333,7 +357,7 @@ class ChangeProfileCoveredTest_Invalid(AATest):
     def test_borked_obj_is_covered_2(self):
         obj = ChangeProfileRule.parse('change_profile /foo,')
 
-        testobj = ChangeProfileRule('/foo', '/bar')
+        testobj = ChangeProfileRule(None, '/foo', '/bar')
         testobj.targetprofile = ''
 
         with self.assertRaises(AppArmorBug):
@@ -357,14 +381,14 @@ class ChangeProfileCoveredTest_Invalid(AATest):
 
 class ChangeProfileLogprofHeaderTest(AATest):
     tests = [
-        ('change_profile,',                         [                               _('Exec Condition'), _('ALL'),  _('Target Profile'), _('ALL'),   ]),
-        ('change_profile -> /bin/ping,',            [                               _('Exec Condition'), _('ALL'),  _('Target Profile'), '/bin/ping',]),
-        ('change_profile /bar -> /bin/bar,',        [                               _('Exec Condition'), '/bar',    _('Target Profile'), '/bin/bar', ]),
-        ('change_profile /foo,',                    [                               _('Exec Condition'), '/foo',    _('Target Profile'), _('ALL'),   ]),
-        ('audit change_profile -> /bin/ping,',      [_('Qualifier'), 'audit',       _('Exec Condition'), _('ALL'),  _('Target Profile'), '/bin/ping',]),
-        ('deny change_profile /bar -> /bin/bar,',   [_('Qualifier'), 'deny',        _('Exec Condition'), '/bar',    _('Target Profile'), '/bin/bar', ]),
-        ('allow change_profile /foo,',              [_('Qualifier'), 'allow',       _('Exec Condition'), '/foo',    _('Target Profile'), _('ALL'),   ]),
-        ('audit deny change_profile,',              [_('Qualifier'), 'audit deny',  _('Exec Condition'), _('ALL'),  _('Target Profile'), _('ALL'),   ]),
+        ('change_profile,',                         [                                                         _('Exec Condition'), _('ALL'),  _('Target Profile'), _('ALL'),   ]),
+        ('change_profile -> /bin/ping,',            [                                                         _('Exec Condition'), _('ALL'),  _('Target Profile'), '/bin/ping',]),
+        ('change_profile /bar -> /bin/bar,',        [                                                         _('Exec Condition'), '/bar',    _('Target Profile'), '/bin/bar', ]),
+        ('change_profile safe /foo,',                    [                          _('Exec Mode'), 'safe',   _('Exec Condition'), '/foo',    _('Target Profile'), _('ALL'),   ]),
+        ('audit change_profile -> /bin/ping,',      [_('Qualifier'), 'audit',                                 _('Exec Condition'), _('ALL'),  _('Target Profile'), '/bin/ping',]),
+        ('deny change_profile /bar -> /bin/bar,',   [_('Qualifier'), 'deny',                                  _('Exec Condition'), '/bar',    _('Target Profile'), '/bin/bar', ]),
+        ('allow change_profile unsafe /foo,',       [_('Qualifier'), 'allow',       _('Exec Mode'), 'unsafe', _('Exec Condition'), '/foo',    _('Target Profile'), _('ALL'),   ]),
+        ('audit deny change_profile,',              [_('Qualifier'), 'audit deny',                            _('Exec Condition'), _('ALL'),  _('Target Profile'), _('ALL'),   ]),
     ]
 
     def _run_test(self, params, expected):
diff --git a/utils/test/test-parser-simple-tests.py b/utils/test/test-parser-simple-tests.py
index 304ff98..66b77ab 100644
--- a/utils/test/test-parser-simple-tests.py
+++ b/utils/test/test-parser-simple-tests.py
@@ -47,6 +47,11 @@ exception_not_raised = [
     'capability/bad_3.sd',
     'capability/bad_4.sd',
     'change_hat/bad_parsing.sd',
+
+    # The tools don't detect conflicting change_profile exec modes
+    'change_profile/onx_conflict_unsafe1.sd',
+    'change_profile/onx_conflict_unsafe2.sd',
+
     'dbus/bad_regex_01.sd',
     'dbus/bad_regex_02.sd',
     'dbus/bad_regex_03.sd',
-- 
2.7.4




More information about the AppArmor mailing list