[apparmor] [PATCH] utils: Clean up file rule parsing

Tyler Hicks tyhicks at canonical.com
Wed Apr 23 17:55:13 UTC 2014


This patch backs out most of the changes from r2448 in favor of a better
approach.

The optional "file" keyword is handled under the pre-existing
RE_PROFILE_PATH_ENTRY regex and a new regex, RE_PROFILE_BARE_FILE_ENTRY,
is created for handling bare file rules.

Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
Cc: Christian Boltz <apparmor at cboltz.de>
---

This patch is meant to address the feedback from cboltz regarding the
duplicated code in aa.py (not the duplicated test code):

  https://lists.ubuntu.com/archives/apparmor/2014-April/005616.html

 utils/apparmor/aa.py             | 218 ++++++++++++++-------------------------
 utils/apparmor/aamode.py         |   1 +
 utils/test/test-regex_matches.py |  82 ++++++---------
 3 files changed, 107 insertions(+), 194 deletions(-)

diff --git a/utils/apparmor/aa.py b/utils/apparmor/aa.py
index ea1487b..05ed2d0 100644
--- a/utils/apparmor/aa.py
+++ b/utils/apparmor/aa.py
@@ -2615,8 +2615,8 @@ RE_PROFILE_VARIABLE = re.compile('^\s*(@\{?\w+\}?)\s*(\+?=)\s*(@*.+?)\s*,?\s*(#.
 RE_PROFILE_CONDITIONAL = re.compile('^\s*if\s+(not\s+)?(\$\{?\w*\}?)\s*\{\s*(#.*)?$')
 RE_PROFILE_CONDITIONAL_VARIABLE = re.compile('^\s*if\s+(not\s+)?defined\s+(@\{?\w+\}?)\s*\{\s*(#.*)?$')
 RE_PROFILE_CONDITIONAL_BOOLEAN = re.compile('^\s*if\s+(not\s+)?defined\s+(\$\{?\w+\}?)\s*\{\s*(#.*)?$')
-RE_PROFILE_FILE_ENTRY = re.compile('^\s*(audit\s+)?(allow\s+|deny\s+)?(owner\s+)?file(?:\s+([\"@/].*?)\s+(\S+)(\s+->\s*(.*?))?)?\s*,\s*(#.*)?$')
-RE_PROFILE_PATH_ENTRY = re.compile('^\s*(audit\s+)?(allow\s+|deny\s+)?(owner\s+)?([\"@/].*?)\s+(\S+)(\s+->\s*(.*?))?\s*,\s*(#.*)?$')
+RE_PROFILE_BARE_FILE_ENTRY = re.compile('^\s*(audit\s+)?(allow\s+|deny\s+)?(owner\s+)?file\s*,\s*(#.*)?$')
+RE_PROFILE_PATH_ENTRY = re.compile('^\s*(audit\s+)?(allow\s+|deny\s+)?(owner\s+)?(file\s+)?([\"@/].*?)\s+(\S+)(\s+->\s*(.*?))?\s*,\s*(#.*)?$')
 RE_PROFILE_NETWORK = re.compile('^\s*(audit\s+)?(allow\s+|deny\s+)?network(.*)\s*(#.*)?$')
 RE_PROFILE_CHANGE_HAT = re.compile('^\s*\^(\"??.+?\"??)\s*,\s*(#.*)?$')
 RE_PROFILE_HAT_DEF = re.compile('^\s*\^(\"??.+?\"??)\s+((flags=)?\((.+)\)\s+)*\{\s*(#.*)?$')
@@ -2854,6 +2854,29 @@ def parse_profile_data(data, file, do_include):
             # Conditional Boolean defined
             pass
 
+        elif RE_PROFILE_BARE_FILE_ENTRY.search(line):
+            matches = RE_PROFILE_BARE_FILE_ENTRY.search(line).groups()
+
+            if not profile:
+                raise AppArmorException(_('Syntax Error: Unexpected bare file rule found in file: %s line: %s') % (file, lineno + 1))
+
+            allow = 'allow'
+            if matches[1] and matches[1].strip() == 'deny':
+                allow = 'deny'
+
+            mode = apparmor.aamode.AA_BARE_FILE_MODE
+            if not matches[2]:
+                mode |= AA_OTHER(apparmor.aamode.AA_BARE_FILE_MODE)
+
+            audit = set()
+            if matches[0]:
+                audit = mode
+
+            path_rule = profile_data[profile][hat][allow]['path'][ALL]
+            path_rule['mode'] = mode
+            path_rule['audit'] = audit
+            path_rule['file_prefix'] = True
+
         elif RE_PROFILE_PATH_ENTRY.search(line):
             matches = RE_PROFILE_PATH_ENTRY.search(line).groups()
 
@@ -2872,8 +2895,12 @@ def parse_profile_data(data, file, do_include):
             if matches[2]:
                 user = True
 
-            path = matches[3].strip()
-            mode = matches[4]
+            file_prefix = False
+            if matches[3]:
+                file_prefix = True
+
+            path = matches[4].strip()
+            mode = matches[5]
             nt_name = matches[6]
             if nt_name:
                 nt_name = nt_name.strip()
@@ -2895,68 +2922,8 @@ def parse_profile_data(data, file, do_include):
 
             profile_data[profile][hat][allow]['path'][path]['mode'] = profile_data[profile][hat][allow]['path'][path].get('mode', set()) | tmpmode
 
-            if nt_name:
-                profile_data[profile][hat][allow]['path'][path]['to'] = nt_name
-
-            if audit:
-                profile_data[profile][hat][allow]['path'][path]['audit'] = profile_data[profile][hat][allow]['path'][path].get('audit', set()) | tmpmode
-            else:
-                profile_data[profile][hat][allow]['path'][path]['audit'] = set()
-
-        elif RE_PROFILE_FILE_ENTRY.search(line):
-            matches = RE_PROFILE_FILE_ENTRY.search(line).groups()
-
-            if not profile:
-                raise AppArmorException(_('Syntax Error: Unexpected file entry found in file: %s line: %s') % (file, lineno + 1))
-
-            audit = False
-            if matches[0]:
-                audit = True
-
-            allow = 'allow'
-            if matches[1] and matches[1].strip() == 'deny':
-                allow = 'deny'
-
-            user = False
-            if matches[2]:
-                user = True
-
-            path = None
-            if matches[3]:
-                path = matches[3].strip()
-
-            mode = None
-            if matches[4]:
-                mode = matches[4]
-
-            nt_name = None
-            if matches[6]:
-                nt_name = matches[6].strip()
-
-            if not path and not mode and not nt_name:
-                path = ALL
-            elif (path and not mode) or (nt_name and (not path or not mode)):
-                raise AppArmorException(_('Syntax Error: Invalid file entry found in file: %s line: %s') % (file, lineno + 1))
-
-            p_re = convert_regexp(path)
-            try:
-                re.compile(p_re)
-            except:
-                raise AppArmorException(_('Syntax Error: Invalid Regex %s in file: %s line: %s') % (path, file, lineno + 1))
-
-            tmpmode = set()
-            if mode:
-                if not validate_profile_mode(mode, allow, nt_name):
-                    raise AppArmorException(_('Invalid mode %s in file: %s line: %s') % (mode, file, lineno + 1))
-
-                if user:
-                    tmpmode = str_to_mode('%s::' % mode)
-                else:
-                    tmpmode = str_to_mode(mode)
-
-            profile_data[profile][hat][allow]['path'][path]['mode'] = profile_data[profile][hat][allow]['path'][path].get('mode', set()) | tmpmode
-
-            profile_data[profile][hat][allow]['path'][path]['file_prefix'] = True
+            if file_prefix:
+                profile_data[profile][hat][allow]['path'][path]['file_prefix'] = True
 
             if nt_name:
                 profile_data[profile][hat][allow]['path'][path]['to'] = nt_name
@@ -3563,17 +3530,14 @@ def write_path_rules(prof_data, depth, allow):
         for path in sorted(prof_data[allow]['path'].keys()):
             filestr = ''
             if prof_data[allow]['path'][path].get('file_prefix', False):
-                filestr = 'file '
+                filestr = 'file'
             mode = prof_data[allow]['path'][path]['mode']
             audit = prof_data[allow]['path'][path]['audit']
             tail = ''
             if prof_data[allow]['path'][path].get('to', False):
                 tail = ' -> %s' % prof_data[allow]['path'][path]['to']
-            user = None
-            other = None
-            if mode or audit:
-                user, other = split_mode(mode)
-                user_audit, other_audit = split_mode(audit)
+            user, other = split_mode(mode)
+            user_audit, other_audit = split_mode(audit)
 
             while user or other:
                 ownerstr = ''
@@ -3598,22 +3562,27 @@ def write_path_rules(prof_data, depth, allow):
                         user = user - tmpmode
                         other = other - tmpmode
 
+                if path == ALL:
+                    path = ''
+
                 if tmpmode & tmpaudit:
                     modestr = mode_to_str(tmpmode & tmpaudit)
+                    if modestr:
+                        modestr = ' ' + modestr
                     path = quote_if_needed(path)
-                    data.append('%saudit %s%s%s%s %s%s,' % (pre, allowstr, ownerstr, filestr, path, modestr, tail))
+                    if filestr and path:
+                        filestr += ' '
+                    data.append('%saudit %s%s%s%s%s%s,' % (pre, allowstr, ownerstr, filestr, path, modestr, tail))
                     tmpmode = tmpmode - tmpaudit
 
                 if tmpmode:
                     modestr = mode_to_str(tmpmode)
+                    if modestr:
+                        modestr = ' ' + modestr
                     path = quote_if_needed(path)
-                    data.append('%s%s%s%s%s %s%s,' % (pre, allowstr, ownerstr, filestr, path, modestr, tail))
-
-            if filestr and path == ALL:
-                auditstr = ''
-                if audit == 0:
-                    auditstr = 'audit '
-                data.append('%s%s%s%s%s,' % (pre, auditstr, allowstr, filestr, tail))
+                    if filestr and path:
+                        filestr += ' '
+                    data.append('%s%s%s%s%s%s%s,' % (pre, allowstr, ownerstr, filestr, path, modestr, tail))
 
         data.append('')
     return data
@@ -4141,41 +4110,25 @@ def serialize_profile_from_old_profile(profile_data, name, options):
                     #To-Do
                     pass
 
-            elif RE_PROFILE_PATH_ENTRY.search(line):
-                matches = RE_PROFILE_PATH_ENTRY.search(line).groups()
-                audit = False
-                if matches[0]:
-                    audit = True
+            elif RE_PROFILE_BARE_FILE_ENTRY.search(line):
+                matches = RE_PROFILE_BARE_FILE_ENTRY.search(line).groups()
+
                 allow = 'allow'
-                if matches[1] and matches[1].split() == 'deny':
+                if matches[1] and matches[1].strip() == 'deny':
                     allow = 'deny'
 
-                user = False
-                if matches[2]:
-                    user = True
-
-                path = matches[3].strip()
-                mode = matches[4]
-                nt_name = matches[6]
-                if nt_name:
-                    nt_name = nt_name.strip()
-
-                tmpmode = set()
-                if user:
-                    tmpmode = str_to_mode('%s::' % mode)
-                else:
-                    tmpmode = str_to_mode(mode)
+                mode = apparmor.aamode.AA_BARE_FILE_MODE
+                if not matches[2]:
+                    mode |= AA_OTHER(apparmor.aamode.AA_BARE_FILE_MODE)
 
-                if not write_prof_data[hat][allow]['path'][path].get('mode', set()) & tmpmode:
-                    correct = False
-
-                if nt_name and not write_prof_data[hat][allow]['path'][path].get('to', False) == nt_name:
-                    correct = False
-
-                if audit and not write_prof_data[hat][allow]['path'][path].get('audit', set()) & tmpmode:
-                    correct = False
+                audit = set()
+                if matches[0]:
+                    audit = mode
 
-                if correct:
+                path_rule = write_prof_data[profile][hat][allow]['path'][ALL]
+                if path_rule.get('mode', set()) & mode and \
+                   (not audit or path_rule.get('audit', set()) & audit) and \
+                   path_rule.get('file_prefix', set()):
                     if not segments['path'] and True in segments.values():
                         for segs in list(filter(lambda x: segments[x], segments.keys())):
                             depth = len(line) - len(line.lstrip())
@@ -4186,14 +4139,11 @@ def serialize_profile_from_old_profile(profile_data, name, options):
                             if write_prof_data[name]['deny'].get(segs, False):
                                 write_prof_data[name]['deny'].pop(segs)
                     segments['path'] = True
-                    write_prof_data[hat][allow]['path'].pop(path)
+                    write_prof_data[hat][allow]['path'].pop(ALL)
                     data.append(line)
-                else:
-                    #To-Do
-                    pass
 
-            elif RE_PROFILE_FILE_ENTRY.search(line):
-                matches = RE_PROFILE_FILE_ENTRY.search(line).groups()
+            elif RE_PROFILE_PATH_ENTRY.search(line):
+                matches = RE_PROFILE_PATH_ENTRY.search(line).groups()
                 audit = False
                 if matches[0]:
                     audit = True
@@ -4205,40 +4155,26 @@ def serialize_profile_from_old_profile(profile_data, name, options):
                 if matches[2]:
                     user = True
 
-                path = None
-                if matches[3]:
-                    path = matches[3].strip()
-
-                mode = None
-                if matches[4]:
-                    mode = matches[4].strip()
-
-                nt_name = None
-                if matches[6]:
-                    nt_name = matches[6].strip()
-
-                if not path and not mode and not nt_name:
-                    path = ALL
-                elif (path and not mode) or (nt_name and (not path or not mode)):
-                    correct = False
+                path = matches[4].strip()
+                mode = matches[5]
+                nt_name = matches[6]
+                if nt_name:
+                    nt_name = nt_name.strip()
 
                 tmpmode = set()
-                if mode:
-                    if user:
-                        tmpmode = str_to_mode('%s::' % mode)
-                    else:
-                        tmpmode = str_to_mode(mode)
+                if user:
+                    tmpmode = str_to_mode('%s::' % mode)
+                else:
+                    tmpmode = str_to_mode(mode)
 
                 if not write_prof_data[hat][allow]['path'][path].get('mode', set()) & tmpmode:
-                    if path != ALL:
-                        correct = False
+                    correct = False
 
                 if nt_name and not write_prof_data[hat][allow]['path'][path].get('to', False) == nt_name:
                     correct = False
 
                 if audit and not write_prof_data[hat][allow]['path'][path].get('audit', set()) & tmpmode:
-                    if path != ALL:
-                        correct = False
+                    correct = False
 
                 if correct:
                     if not segments['path'] and True in segments.values():
diff --git a/utils/apparmor/aamode.py b/utils/apparmor/aamode.py
index d0a5cb6..a761747 100644
--- a/utils/apparmor/aamode.py
+++ b/utils/apparmor/aamode.py
@@ -40,6 +40,7 @@ AA_EXEC_PROFILE = set('P')
 AA_EXEC_CHILD = set('C')
 AA_EXEC_NT = set('N')
 AA_LINK_SUBSET = set(['linksubset'])
+AA_BARE_FILE_MODE = set(['bare_file_mode'])
 #AA_OTHER_SHIFT = 14
 #AA_USER_MASK = 16384 - 1
 
diff --git a/utils/test/test-regex_matches.py b/utils/test/test-regex_matches.py
index 6d596bd..3726f73 100644
--- a/utils/test/test-regex_matches.py
+++ b/utils/test/test-regex_matches.py
@@ -187,7 +187,7 @@ class AARegexPath(unittest.TestCase):
         line = '   /tmp/foo r,'
         result = aa.RE_PROFILE_PATH_ENTRY.search(line)
         self.assertTrue(result, 'Couldn\'t find file rule in "%s"' % line)
-        mode = result.groups()[4].strip()
+        mode = result.groups()[5].strip()
         self.assertEqual(mode, 'r', 'Expected mode "r", got "%s"' % (mode))
 
     def test_simple_path_02(self):
@@ -198,7 +198,7 @@ class AARegexPath(unittest.TestCase):
         self.assertTrue(result, 'Couldn\'t find file rule in "%s"' % line)
         audit = result.groups()[0].strip()
         self.assertEqual(audit, 'audit', 'Couldn\t find audit modifier')
-        mode = result.groups()[4].strip()
+        mode = result.groups()[5].strip()
         self.assertEqual(mode, 'rw', 'Expected mode "rw", got "%s"' % (mode))
 
     def test_simple_path_03(self):
@@ -211,91 +211,67 @@ class AARegexPath(unittest.TestCase):
         self.assertEqual(audit, 'audit', 'Couldn\t find audit modifier')
         deny = result.groups()[1].strip()
         self.assertEqual(deny, 'deny', 'Couldn\t find deny modifier')
-        mode = result.groups()[4].strip()
+        mode = result.groups()[5].strip()
         self.assertEqual(mode, 'rw', 'Expected mode "rw", got "%s"' % (mode))
 
-    def test_simple_bad_path_01(self):
-        '''test '   file,' '''
+    def test_simple_path_04(self):
+        '''test '   file /tmp/foo rw,' '''
 
-        line = '   file,'
+        line = '   file /tmp/foo rw,'
         result = aa.RE_PROFILE_PATH_ENTRY.search(line)
-        self.assertFalse(result, 'RE_PROFILE_PATH_ENTRY unexpectedly matched "%s"' % line)
+        self.assertTrue(result, 'Couldn\'t find file rule in "%s"' % line)
+        path = result.groups()[4].strip()
+        self.assertEqual(path, "/tmp/foo", 'Couldn\'t find path')
+        mode = result.groups()[5].strip()
+        self.assertEqual(mode, 'rw', 'Expected mode "rw", got "%s"' % (mode))
 
-    def test_simple_bad_path_02(self):
-        '''test '   file /tmp/foo rw,' '''
+    def test_simple_bad_path_01(self):
+        '''test '   file,' '''
 
-        line = '   file /tmp/foo rw,'
+        line = '   file,'
         result = aa.RE_PROFILE_PATH_ENTRY.search(line)
         self.assertFalse(result, 'RE_PROFILE_PATH_ENTRY unexpectedly matched "%s"' % line)
 
-class AARegexFile(unittest.TestCase):
-    '''Tests for RE_PROFILE_FILE_ENTRY'''
+class AARegexBareFile(unittest.TestCase):
+    '''Tests for RE_PROFILE_BARE_FILE_ENTRY'''
 
     def _assertEqualStrings(self, str1, str2):
         self.assertEqual(str1, str2, 'Expected %s, got "%s"' % (str1, str2))
 
-    def test_simple_file_01(self):
-        '''test '   file /tmp/foo rw,' '''
-
-        path = '/tmp/foo'
-        mode = 'rw'
-        line = '   file %s %s,' % (path, mode)
-        result = aa.RE_PROFILE_FILE_ENTRY.search(line)
-        self.assertTrue(result, 'Couldn\'t find file rule in "%s"' % line)
-        self._assertEqualStrings(path, result.groups()[3].strip())
-        self._assertEqualStrings(mode, result.groups()[4].strip())
-
-    def test_simple_file_02(self):
+    def test_bare_file_01(self):
         '''test '   file,' '''
 
         line = '   file,'
-        result = aa.RE_PROFILE_FILE_ENTRY.search(line)
-        self.assertTrue(result, 'Couldn\'t find file rule in "%s"' % line)
-        path = result.groups()[3]
-        self.assertEqual(path, None, 'Unexpected path, got "%s"' % path)
-        mode = result.groups()[4]
-        self.assertEqual(mode, None, 'Unexpected mode, got "%s"' % (mode))
-
-    def test_simple_file_03(self):
-        '''test '   audit file,' '''
-
-        line = '   audit file,'
-        result = aa.RE_PROFILE_FILE_ENTRY.search(line)
+        result = aa.RE_PROFILE_BARE_FILE_ENTRY.search(line)
         self.assertTrue(result, 'Couldn\'t find file rule in "%s"' % line)
-        audit = result.groups()[0].strip()
-        self.assertEqual(audit, 'audit', 'Couldn\t find audit modifier')
-        path = result.groups()[3]
-        self.assertEqual(path, None, 'Unexpected path, got "%s"' % path)
-        mode = result.groups()[4]
-        self.assertEqual(mode, None, 'Unexpected mode, got "%s"' % (mode))
 
     def test_simple_bad_file_01(self):
         '''test '   dbus,' '''
 
         line = '   dbus,'
-        result = aa.RE_PROFILE_FILE_ENTRY.search(line)
-        self.assertFalse(result, 'RE_PROFILE_FILE_ENTRY unexpectedly matched "%s"' % line)
+        result = aa.RE_PROFILE_BARE_FILE_ENTRY.search(line)
+        self.assertFalse(result, 'RE_PROFILE_BARE_FILE_ENTRY unexpectedly matched "%s"' % line)
 
     def test_simple_bad_file_02(self):
-        '''test '   /tmp/foo rw,' '''
+        '''test '   file /tmp/foo rw,' '''
 
-        line = '   /tmp/foo rw,'
-        result = aa.RE_PROFILE_FILE_ENTRY.search(line)
-        self.assertFalse(result, 'RE_PROFILE_FILE_ENTRY unexpectedly matched "%s"' % line)
+        line = '   file /tmp/foo rw,'
+        result = aa.RE_PROFILE_BARE_FILE_ENTRY.search(line)
+        self.assertFalse(result, 'RE_PROFILE_BARE_FILE_ENTRY unexpectedly matched "%s"' % line)
 
     def test_simple_bad_file_03(self):
         '''test '   file /tmp/foo,' '''
 
         line = '   file /tmp/foo,'
-        result = aa.RE_PROFILE_FILE_ENTRY.search(line)
-        self.assertFalse(result, 'RE_PROFILE_FILE_ENTRY unexpectedly matched "%s"' % line)
+        result = aa.RE_PROFILE_BARE_FILE_ENTRY.search(line)
+        self.assertFalse(result, 'RE_PROFILE_BARE_FILE_ENTRY unexpectedly matched "%s"' % line)
 
     def test_simple_bad_file_04(self):
         '''test '   file r,' '''
 
         line = '   file r,'
-        result = aa.RE_PROFILE_FILE_ENTRY.search(line)
-        self.assertFalse(result, 'RE_PROFILE_FILE_ENTRY unexpectedly matched "%s"' % line)
+        result = aa.RE_PROFILE_BARE_FILE_ENTRY.search(line)
+        self.assertFalse(result, 'RE_PROFILE_BARE_FILE_ENTRY unexpectedly matched "%s"' % line)
 
 class AARegexSignal(unittest.TestCase):
     '''Tests for RE_PROFILE_SIGNAL'''
@@ -532,7 +508,7 @@ if __name__ == '__main__':
     test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexSplitComment))
     test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexCapability))
     test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexPath))
-    test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexFile))
+    test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexBareFile))
     test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexSignal))
     test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexPtrace))
     test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexPivotRoot))
-- 
1.9.1




More information about the AppArmor mailing list