On Wed, Mar 26, 2014 at 09:02:47PM -0500, Tyler Hicks wrote: > Bug: https://bugs.launchpad.net/bugs/1295346 > > Add the ability to read and write path rules containing the file prefix. > This also includes bare "file," rules. > > The ALL global is updated to include a preceding NUL char to eliminate > possibilities of a real file path colliding with the ALL global. > > Signed-off-by: Tyler Hicks <[email protected]>
Nack.
Unfortunately, the RE_PROFILE_PATH_ENTRY end up matching other bareword
keywords, like so:
$ sudo sh -c 'PYTHONPATH=. python3 ./aa-enforce ~/tmp/true '
Traceback (most recent call last):
File "./aa-enforce", line 30, in <module>
tool.cmd_enforce()
File "/home/steve/bzr/apparmor/utils/apparmor/tools.py", line 153, in
cmd_enforce
apparmor.read_profiles()
File "/home/steve/bzr/apparmor/utils/apparmor/aa.py", line 2564, in
read_profiles
read_profile(profile_dir + '/' + file, True)
File "/home/steve/bzr/apparmor/utils/apparmor/aa.py", line 2590, in
read_profile
profile_data = parse_profile_data(data, file, 0)
File "/home/steve/bzr/apparmor/utils/apparmor/aa.py", line 2944, in
parse_profile_data
load_include(include_name)
File "/home/steve/bzr/apparmor/utils/apparmor/aa.py", line 4283, in
load_include
incdata = parse_profile_data(data, incfile, True)
File "/home/steve/bzr/apparmor/utils/apparmor/aa.py", line 2944, in
parse_profile_data
load_include(include_name)
File "/home/steve/bzr/apparmor/utils/apparmor/aa.py", line 4283, in
load_include
incdata = parse_profile_data(data, incfile, True)
File "/home/steve/bzr/apparmor/utils/apparmor/aa.py", line 2888, in
parse_profile_data
raise AppArmorException(_('Syntax Error: Invalid path entry found in
file: %s line: %s') % (file, lineno + 1))
apparmor.common.AppArmorException: 'Syntax Error: Invalid path entry found in
file: abstractions/ubuntu-helpers line: 42'
Line 42 of abstractions/ubuntu-helpers contains simply:
dbus,
Also unfortunate, the added test cases were not being exercised and
were pointing at the wrong regular expression. (Yes, I'm aware of the
difficulty of running the tests on ubuntu trusty right now due to some
python difficulties.)
I've attached the updated patch with the testcase issues fixed + plus
an additional test case -- and yes, test_simple_bad_file_01() fails. The
following is the diff between the two versions:
diff -u b/utils/test/test-regex_matches.py b/utils/test/test-regex_matches.py
--- b/utils/test/test-regex_matches.py
+++ b/utils/test/test-regex_matches.py
@@ -164,7 +164,7 @@
'''test ' /tmp/foo r,' '''
line = ' /tmp/foo r,'
- result = aa.RE_PROFILE_CAP.search(line)
+ result = aa.RE_PROFILE_PATH_ENTRY.search(line)
self.assertTrue(result, 'Couldn\'t find file rule in "%s"' % line)
mode = result.groups()[5].strip()
self.assertEqual(mode, 'r', 'Expected mode "r", got "%s"' % (mode))
@@ -173,7 +173,7 @@
'''test ' audit /tmp/foo rw,' '''
line = ' audit /tmp/foo rw,'
- result = aa.RE_PROFILE_CAP.search(line)
+ result = aa.RE_PROFILE_PATH_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')
@@ -184,7 +184,7 @@
'''test ' audit deny /tmp/foo rw,' '''
line = ' audit deny /tmp/foo rw,'
- result = aa.RE_PROFILE_CAP.search(line)
+ result = aa.RE_PROFILE_PATH_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')
@@ -197,7 +197,7 @@
'''test ' file /tmp/foo rw,' '''
line = ' file /tmp/foo rw,'
- result = aa.RE_PROFILE_CAP.search(line)
+ result = aa.RE_PROFILE_PATH_ENTRY.search(line)
self.assertTrue(result, 'Couldn\'t find file rule in "%s"' % line)
file_prefix = result.groups()[3].strip()
self.assertEqual(file_prefix, 'file', 'Couldn\t find file prefix')
@@ -207,14 +207,21 @@
def test_simple_file_02(self):
'''test ' file,' '''
- line = ' file /tmp/foo rw,'
- result = aa.RE_PROFILE_CAP.search(line)
+ line = ' file,'
+ result = aa.RE_PROFILE_PATH_ENTRY.search(line)
self.assertTrue(result, 'Couldn\'t find file rule in "%s"' % line)
file_prefix = result.groups()[3].strip()
self.assertEqual(file_prefix, 'file', 'Couldn\t find file prefix')
mode = result.groups()[5]
self.assertEqual(mode, None, 'Unexpected mode, got "%s"' % (mode))
+ def test_simple_bad_file_01(self):
+ '''test ' dbus,' '''
+
+ line = ' dbus,'
+ result = aa.RE_PROFILE_PATH_ENTRY.search(line)
+ self.assertFalse(result, 'RE_PROFILE_PATH_ENTRY unexpectedly matched
"%s"' % line)
+
if __name__ == '__main__':
verbosity = 2
@@ -225,6 +232,7 @@
test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexHasComma))
test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexSplitComment))
test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexCapability))
+
test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexPath))
result = unittest.TextTestRunner(verbosity=verbosity).run(test_suite)
if not result.wasSuccessful():
exit(1)
--
Steve Beattie
<[email protected]>
http://NxNW.org/~steve/
Bug: https://bugs.launchpad.net/bugs/1295346 Add the ability to read and write path rules containing the file prefix. This also includes bare "file," rules. The ALL global is updated to include a preceding NUL char to eliminate possibilities of a real file path colliding with the ALL global. Signed-off-by: Tyler Hicks <[email protected]> --- utils/apparmor/aa.py | 109 ++++++++++++++++++++++++++++----------- utils/test/test-regex_matches.py | 69 ++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 29 deletions(-) Index: b/utils/apparmor/aa.py =================================================================== --- a/utils/apparmor/aa.py +++ b/utils/apparmor/aa.py @@ -79,7 +79,7 @@ seen_events = 0 # was our user_globs = [] # The key for representing bare rules such as "capability," or "file," -ALL = '_ALL' +ALL = '\0ALL' ## Variables used under logprof ### Were our @@ -2615,7 +2615,7 @@ RE_PROFILE_VARIABLE = re.compile('^\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_PATH_ENTRY = re.compile('^\s*(audit\s+)?(allow\s+|deny\s+)?(owner\s+)?([\"@/].*?)\s+(\S+)(\s+->\s*(.*?))?\s*,\s*(#.*)?$') +RE_PROFILE_PATH_ENTRY = re.compile('^\s*(audit\s+)?(allow\s+|deny\s+)?(owner\s+)?(file\s+|file(?=,))?([\"@/].*?\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*(#.*)?$') @@ -2865,11 +2865,27 @@ def parse_profile_data(data, file, do_in if matches[2]: user = True - path = matches[3].strip() - mode = matches[4] - nt_name = matches[6] - if nt_name: - nt_name = nt_name.strip() + file_prefix = False + if matches[3]: + file_prefix = True + + path = None + if matches[4]: + path = matches[4].strip() + + mode = None + if matches[5]: + mode = matches[5] + + nt_name = None + if matches[7]: + nt_name = matches[7].strip() + + if file_prefix and not path and not mode and not nt_name: + path = ALL + elif (file_prefix and path and not mode) or \ + (not file_prefix and (not path or not mode)): + raise AppArmorException(_('Syntax Error: Invalid path entry found in file: %s line: %s') % (file, lineno + 1)) p_re = convert_regexp(path) try: @@ -2877,17 +2893,21 @@ def parse_profile_data(data, file, do_in except: raise AppArmorException(_('Syntax Error: Invalid Regex %s in file: %s line: %s') % (path, file, lineno + 1)) - if not validate_profile_mode(mode, allow, nt_name): - raise AppArmorException(_('Invalid mode %s in file: %s line: %s') % (mode, file, lineno + 1)) - tmpmode = set() - if user: - tmpmode = str_to_mode('%s::' % mode) - else: - tmpmode = str_to_mode(mode) + 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 + 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 @@ -3359,13 +3379,19 @@ def write_path_rules(prof_data, depth, a if prof_data[allow].get('path', False): for path in sorted(prof_data[allow]['path'].keys()): + filestr = '' + if prof_data[allow]['path'][path].get('file_prefix', False): + 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, other = split_mode(mode) - user_audit, other_audit = split_mode(audit) + user = None + other = None + if mode or audit: + user, other = split_mode(mode) + user_audit, other_audit = split_mode(audit) while user or other: ownerstr = '' @@ -3393,13 +3419,19 @@ def write_path_rules(prof_data, depth, a if tmpmode & tmpaudit: modestr = mode_to_str(tmpmode & tmpaudit) path = quote_if_needed(path) - data.append('%saudit %s%s%s %s%s,' % (pre, allowstr, ownerstr, path, modestr, tail)) + 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) path = quote_if_needed(path) - data.append('%s%s%s%s %s%s,' % (pre, allowstr, ownerstr, path, modestr, tail)) + 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)) data.append('') return data @@ -3931,26 +3963,45 @@ def serialize_profile_from_old_profile(p if matches[2]: user = True - path = matches[3].strip() - mode = matches[4] - nt_name = matches[6] - if nt_name: - nt_name = nt_name.strip() + file_prefix = False + if matches[3]: + file_prefix = True + + path = None + if matches[4]: + path = matches[4].strip() + + mode = None + if matches[5]: + mode = matches[5].strip() + + nt_name = None + if matches[7]: + nt_name = matches[7].strip() + + if file_prefix and not path and not mode and not nt_name: + path = ALL + elif (file_prefix and path and not mode) or \ + (not file_prefix and (not path or not mode)): + correct = False tmpmode = set() - if user: - tmpmode = str_to_mode('%s::' % mode) - else: - tmpmode = str_to_mode(mode) + if 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: - correct = False + if path != ALL: + 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 + if path != ALL: + correct = False if correct: if not segments['path'] and True in segments.values(): Index: b/utils/test/test-regex_matches.py =================================================================== --- a/utils/test/test-regex_matches.py +++ b/utils/test/test-regex_matches.py @@ -96,6 +96,9 @@ regex_split_comment_testcases = [ ('dbus send member=no_comment, ', False), ('audit "/tmp/foo, # bar" rw', False), ('audit "/tmp/foo, # bar" rw # comment', ('audit "/tmp/foo, # bar" rw ', '# comment')), + ('file,', False), + ('file, # bare', ('file, ', '# bare')), + ('file /tmp/foo rw, # read-write', ('file /tmp/foo rw, ', '# read-write')), ] def setup_split_comment_testcases(): @@ -154,6 +157,71 @@ class AARegexCapability(unittest.TestCas result = aa.RE_PROFILE_CAP.search(line) self.assertFalse(result, 'Found unexpected capability rule in "%s"' % line) +class AARegexPath(unittest.TestCase): + '''Tests for RE_PROFILE_PATH_ENTRY''' + + def test_simple_path_01(self): + '''test ' /tmp/foo r,' ''' + + 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()[5].strip() + self.assertEqual(mode, 'r', 'Expected mode "r", got "%s"' % (mode)) + + def test_simple_path_02(self): + '''test ' audit /tmp/foo rw,' ''' + + line = ' audit /tmp/foo rw,' + result = aa.RE_PROFILE_PATH_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') + mode = result.groups()[5].strip() + self.assertEqual(mode, 'rw', 'Expected mode "rw", got "%s"' % (mode)) + + def test_simple_path_03(self): + '''test ' audit deny /tmp/foo rw,' ''' + + line = ' audit deny /tmp/foo rw,' + result = aa.RE_PROFILE_PATH_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') + deny = result.groups()[1].strip() + self.assertEqual(deny, 'deny', 'Couldn\t find deny modifier') + mode = result.groups()[5].strip() + self.assertEqual(mode, 'rw', 'Expected mode "rw", got "%s"' % (mode)) + + def test_simple_file_01(self): + '''test ' file /tmp/foo rw,' ''' + + line = ' file /tmp/foo rw,' + result = aa.RE_PROFILE_PATH_ENTRY.search(line) + self.assertTrue(result, 'Couldn\'t find file rule in "%s"' % line) + file_prefix = result.groups()[3].strip() + self.assertEqual(file_prefix, 'file', 'Couldn\t find file prefix') + mode = result.groups()[5].strip() + self.assertEqual(mode, 'rw', 'Expected mode "rw", got "%s"' % (mode)) + + def test_simple_file_02(self): + '''test ' file,' ''' + + line = ' file,' + result = aa.RE_PROFILE_PATH_ENTRY.search(line) + self.assertTrue(result, 'Couldn\'t find file rule in "%s"' % line) + file_prefix = result.groups()[3].strip() + self.assertEqual(file_prefix, 'file', 'Couldn\t find file prefix') + mode = result.groups()[5] + self.assertEqual(mode, None, 'Unexpected mode, got "%s"' % (mode)) + + def test_simple_bad_file_01(self): + '''test ' dbus,' ''' + + line = ' dbus,' + result = aa.RE_PROFILE_PATH_ENTRY.search(line) + self.assertFalse(result, 'RE_PROFILE_PATH_ENTRY unexpectedly matched "%s"' % line) + if __name__ == '__main__': verbosity = 2 @@ -164,6 +232,7 @@ if __name__ == '__main__': test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexHasComma)) test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexSplitComment)) test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexCapability)) + test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AARegexPath)) result = unittest.TextTestRunner(verbosity=verbosity).run(test_suite) if not result.wasSuccessful(): exit(1)
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
