mir/apparmor
2
0
Fork 0
mirror of https://gitlab.com/apparmor/apparmor synced 2025-08-22 10:07:12 +00:00
Code Issues Releases Wiki Activity

update python tools to support includes with absolute paths

Browse Source 
For now we only allow quoted absolute paths without spaces in the name
due to:
- 1738877: include rules don't handle files with spaces in the name
- 1738879: include rules don't handle absolute paths without quotes in
  some versions of parser
- 1738880: include rules don't handle relative paths in some versions of
  the parser
This commit is contained in:
Jamie Strandboge 2017-12-18 19:37:35 +00:00
parent ebf0cfe838
commit 9bbef8e307
6 changed files with 196 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
utils/aa-mergeprof
View File

@ -165,7 +165,10 @@ class Merge(object):
options = [] options = []
for inc in other.filelist[other.filename]['include'].keys(): for inc in other.filelist[other.filename]['include'].keys():
if not inc in self.user.filelist[self.user.filename]['include'].keys(): if not inc in self.user.filelist[self.user.filename]['include'].keys():
options.append('#include <%s>' %inc) if inc.startswith('/'):
options.append('#include "%s"' %inc)
else:
options.append('#include <%s>' %inc)
default_option = 1 default_option = 1

67
utils/apparmor/aa.py
View File

@ -1432,7 +1432,10 @@ def ask_the_questions(log_dict):
options = [] options = []
for inc in log_dict[aamode][profile][hat]['include'].keys(): for inc in log_dict[aamode][profile][hat]['include'].keys():
if not inc in aa[profile][hat]['include'].keys(): if not inc in aa[profile][hat]['include'].keys():
options.append('#include <%s>' %inc) if inc.startswith('/'):
options.append('#include "%s"' %inc)
else:
options.append('#include <%s>' %inc)
default_option = 1 default_option = 1
@ -1719,6 +1722,8 @@ def valid_include(profile, incname):
if incname.startswith('abstractions/') and os.path.isfile(profile_dir + '/' + incname): if incname.startswith('abstractions/') and os.path.isfile(profile_dir + '/' + incname):
return True return True
elif incname.startswith('/') and os.path.isfile(incname):
return True
return False return False
@ -2298,7 +2303,11 @@ def parse_profile_data(data, file, do_include):
filelist[file] = hasher() filelist[file] = hasher()
filelist[file]['include'][include_name] = True filelist[file]['include'][include_name] = True
# If include is a directory # If include is a directory
if os.path.isdir(profile_dir + '/' + include_name): if include_name.startswith('/'):
dir = include_name
else:
dir = profile_dir + '/' + include_name
if os.path.isdir(dir):
for file_name in include_dir_filelist(profile_dir, include_name): for file_name in include_dir_filelist(profile_dir, include_name):
if not include.get(file_name, False): if not include.get(file_name, False):
load_include(file_name) load_include(file_name)
@ -2568,7 +2577,13 @@ def write_single(prof_data, depth, allow, name, prefix, tail):
if ref.get(name, False): if ref.get(name, False):
for key in sorted(ref[name].keys()): for key in sorted(ref[name].keys()):
qkey = quote_if_needed(key) if name == 'include':
if key.startswith('/'):
qkey = '"%s"' % key
else:
qkey = '<%s>' % quote_if_needed(key)
else:
qkey = quote_if_needed(key)
data.append('%s%s%s%s%s' % (pre, allow, prefix, qkey, tail)) data.append('%s%s%s%s%s' % (pre, allow, prefix, qkey, tail))
if ref[name].keys(): if ref[name].keys():
data.append('') data.append('')
@ -2607,7 +2622,7 @@ def write_pair(prof_data, depth, allow, name, prefix, sep, tail, fn):
return data return data
def write_includes(prof_data, depth): def write_includes(prof_data, depth):
return write_single(prof_data, depth, '', 'include', '#include <', '>') return write_single(prof_data, depth, '', 'include', '#include ', '')
def write_change_profile(prof_data, depth): def write_change_profile(prof_data, depth):
data = [] data = []
@ -3334,7 +3349,11 @@ def is_known_rule(profile, rule_type, rule_obj):
incname = includelist.pop(0) incname = includelist.pop(0)
checked.append(incname) checked.append(incname)
if os.path.isdir(profile_dir + '/' + incname): if incname.startswith('/'):
dir = incname
else:
dir = profile_dir + '/' + incname
if os.path.isdir(dir):
includelist += include_dir_filelist(profile_dir, incname) includelist += include_dir_filelist(profile_dir, incname)
else: else:
if include[incname][incname].get(rule_type, False): if include[incname][incname].get(rule_type, False):
@ -3362,7 +3381,11 @@ def get_file_perms(profile, path, audit, deny):
continue continue
checked.append(incname) checked.append(incname)
if os.path.isdir(profile_dir + '/' + incname): if incname.startswith('/'):
dir = incname
else:
dir = profile_dir + '/' + incname
if os.path.isdir(dir):
includelist += include_dir_filelist(profile_dir, incname) includelist += include_dir_filelist(profile_dir, incname)
else: else:
incperms = include[incname][incname]['file'].get_perms_for_path(path, audit, deny) incperms = include[incname][incname]['file'].get_perms_for_path(path, audit, deny)
@ -3443,7 +3466,8 @@ def reload(bin_path):
def get_include_data(filename): def get_include_data(filename):
data = [] data = []
filename = profile_dir + '/' + filename if not filename.startswith('/'):
filename = profile_dir + '/' + filename
if os.path.exists(filename): if os.path.exists(filename):
with open_file_read(filename) as f_in: with open_file_read(filename) as f_in:
data = f_in.readlines() data = f_in.readlines()
@ -3452,15 +3476,24 @@ def get_include_data(filename):
return data return data
def include_dir_filelist(profile_dir, include_name): def include_dir_filelist(profile_dir, include_name):
'''returns a list of files in the given profile_dir/include_name directory, except skippable files''' '''returns a list of files in the given profile_dir/include_name directory,
except skippable files. If include_name is an absolute path, ignore
profile_dir.
'''
files = [] files = []
for path in os.listdir(profile_dir + '/' + include_name): if include_name.startswith('/'):
dir = include_name
else:
dir = profile_dir + '/' + include_name
for path in os.listdir(dir):
path = path.strip() path = path.strip()
if is_skippable_file(path): if is_skippable_file(path):
continue continue
if os.path.isfile(profile_dir + '/' + include_name + '/' + path): if os.path.isfile(dir + '/' + path):
file_name = include_name + '/' + path file_name = include_name + '/' + path
file_name = file_name.replace(profile_dir + '/', '') # strip off profile_dir for non-absolute paths
if not include_name.startswith('/'):
file_name = file_name.replace(profile_dir + '/', '')
files.append(file_name) files.append(file_name)
return files return files
@ -3469,17 +3502,20 @@ def load_include(incname):
load_includeslist = [incname] load_includeslist = [incname]
while load_includeslist: while load_includeslist:
incfile = load_includeslist.pop(0) incfile = load_includeslist.pop(0)
incfile_abs = incfile
if not incfile.startswith('/'):
incfile_abs = profile_dir + '/' + incfile
if include.get(incfile, {}).get(incfile, False): if include.get(incfile, {}).get(incfile, False):
pass # already read, do nothing pass # already read, do nothing
elif os.path.isfile(profile_dir + '/' + incfile): elif os.path.isfile(incfile_abs):
data = get_include_data(incfile) data = get_include_data(incfile_abs)
incdata = parse_profile_data(data, incfile, True) incdata = parse_profile_data(data, incfile, True)
attach_profile_data(include, incdata) attach_profile_data(include, incdata)
#If the include is a directory means include all subfiles #If the include is a directory means include all subfiles
elif os.path.isdir(profile_dir + '/' + incfile): elif os.path.isdir(incfile_abs):
load_includeslist += include_dir_filelist(profile_dir, incfile) load_includeslist += include_dir_filelist(profile_dir, incfile)
else: else:
raise AppArmorException("Include file %s not found" % (profile_dir + '/' + incfile) ) raise AppArmorException("Include file %s not found" % (incfile_abs))
return 0 return 0
@ -3577,4 +3613,3 @@ def init_aa(confdir="/etc/apparmor"):
parser = conf.find_first_file(cfg['settings'].get('parser')) or '/sbin/apparmor_parser' parser = conf.find_first_file(cfg['settings'].get('parser')) or '/sbin/apparmor_parser'
if not os.path.isfile(parser) or not os.access(parser, os.EX_OK): if not os.path.isfile(parser) or not os.access(parser, os.EX_OK):
raise AppArmorException('Can\'t find apparmor_parser at %s' % (parser)) raise AppArmorException('Can\'t find apparmor_parser at %s' % (parser))

25
utils/apparmor/regex.py
View File

@ -133,7 +133,7 @@ def parse_profile_start_line(line, filename):
return result return result
RE_INCLUDE = re.compile('^\s*#?include\s*<(?P<magicpath>.*)>' + RE_EOL) RE_INCLUDE = re.compile('^\s*#?include\s*(<(?P<magicpath>.*)>|"(?P<quotedpath>.*)"|(?P<unquotedpath>[^<>"]*))' + RE_EOL)
def re_match_include(line): def re_match_include(line):
"""Matches the path for include and returns the include path""" """Matches the path for include and returns the include path"""
@ -142,10 +142,29 @@ def re_match_include(line):
if not matches: if not matches:
return None return None
if not matches.group('magicpath').strip(): path = None
if matches.group('magicpath'):
path = matches.group('magicpath').strip()
elif matches.group('unquotedpath'):
# LP: #1738879 - parser doesn't handle unquoted paths everywhere
# path = matches.group('unquotedpath').strip()
raise AppArmorException(_('Syntax error: required quotes missing for #include'))
elif matches.group('quotedpath'):
path = matches.group('quotedpath').strip()
# LP: 1738880 - parser doesn't handle relative paths everywhere, and
# neither do we (see aa.py)
if len(path) > 0 and path[0] != '/':
raise AppArmorException(_('Syntax error: #include cannot be relative path'))
# if path is empty or the empty string
if path is None or path == "":
raise AppArmorException(_('Syntax error: #include rule with empty filename')) raise AppArmorException(_('Syntax error: #include rule with empty filename'))
return matches.group('magicpath') # LP: #1738877 - parser doesn't handle files with spaces in the name
if re.search('\s', path):
raise AppArmorException(_('Syntax error: #include rule filename cannot contain spaces'))
return path
def strip_parenthesis(data): def strip_parenthesis(data):
'''strips parenthesis from the given string and returns the strip()ped result. '''strips parenthesis from the given string and returns the strip()ped result.

4
utils/apparmor/severity.py
View File

@ -185,7 +185,9 @@ class Severity(object):
# If any includes, load variables from them first # If any includes, load variables from them first
match = re_match_include(line) match = re_match_include(line)
if match: if match:
new_path = self.PROF_DIR + '/' + match new_path = match
if not new_path.startswith('/'):
new_path = self.PROF_DIR + '/' + match
self.load_variables(new_path) self.load_variables(new_path)
else: else:
# Remove any comments # Remove any comments

53
utils/test/test-aa.py
View File

@ -859,6 +859,59 @@ class AaTest_propose_file_rules(AATest):
proposals = propose_file_rules(profile, rule_obj) proposals = propose_file_rules(profile, rule_obj)
self.assertEqual(proposals, expected) self.assertEqual(proposals, expected)
class AaTest_propose_file_rules_with_absolute_includes(AATest):
tests = [
# log event path and perms expected proposals
(['/not/found/anywhere', 'r'], ['/not/found/anywhere r,']),
(['/dev/null', 'w'], ['/dev/null rw,']),
(['/some/random/include', 'r'], ['/some/random/include rw,']),
(['/some/other/include', 'w'], ['/some/other/* rw,', '/some/other/inc* rw,', '/some/other/include rw,']),
]
def _run_test(self, params, expected):
self.createTmpdir()
#copy the local profiles to the test directory
self.profile_dir = '%s/profiles' % self.tmpdir
shutil.copytree('../../profiles/apparmor.d/', self.profile_dir, symlinks=True)
# load the abstractions we need in the test
apparmor.aa.profiledir = self.profile_dir
apparmor.aa.load_include('abstractions/base')
abs_include1 = write_file(self.tmpdir, 'test-abs1', "/some/random/include rw,")
apparmor.aa.load_include(abs_include1)
abs_include2 = write_file(self.tmpdir, 'test-abs2', "/some/other/* rw,")
apparmor.aa.load_include(abs_include2)
abs_include3 = write_file(self.tmpdir, 'test-abs3', "/some/other/inc* rw,")
apparmor.aa.load_include(abs_include3)
profile = apparmor.aa.ProfileStorage('/test', '/test', 'test-aa.py')
profile['include']['abstractions/base'] = False
profile['include'][abs_include1] = False
profile['include'][abs_include2] = False
profile['include'][abs_include3] = False
rule_obj = FileRule(params[0], params[1], None, FileRule.ALL, owner=False, log_event=True)
proposals = propose_file_rules(profile, rule_obj)
self.assertEqual(proposals, expected)
class AaTest_nonexistent_includes(AATest):
def test_bad_includes(self):
tests = [
"/nonexistent/absolute/path",
"nonexistent/relative/path",
]
for i in tests:
with self.assertRaises(AppArmorException):
apparmor.aa.load_include(i)
setup_aa(apparmor.aa) setup_aa(apparmor.aa)
setup_all_loops(__name__) setup_all_loops(__name__)
if __name__ == '__main__': if __name__ == '__main__':

71
utils/test/test-regex_matches.py
View File

@ -437,17 +437,27 @@ class TestInvalid_parse_profile_start_line(AATest):
class Test_re_match_include(AATest): class Test_re_match_include(AATest):
tests = [ tests = [
('#include <abstractions/base>', 'abstractions/base' ), ('#include <abstractions/base>', 'abstractions/base' ), # magic path
('#include <abstractions/base> # comment', 'abstractions/base' ), ('#include <abstractions/base> # comment', 'abstractions/base' ),
('#include<abstractions/base>#comment', 'abstractions/base' ), ('#include<abstractions/base>#comment', 'abstractions/base' ),
(' #include <abstractions/base> ', 'abstractions/base' ), (' #include <abstractions/base> ', 'abstractions/base' ),
('include <abstractions/base>', 'abstractions/base' ), # not supported by parser ('#include "/foo/bar"', '/foo/bar' ), # absolute path
# ('include foo', 'foo' ), # XXX not supported in tools yet ('#include "/foo/bar" # comment', '/foo/bar' ),
# ('include /foo/bar', '/foo/bar' ), # XXX not supported in tools yet ('#include "/foo/bar"#comment', '/foo/bar' ),
# ('include "foo"', 'foo' ), # XXX not supported in tools yet (' #include "/foo/bar" ', '/foo/bar' ),
# ('include "/foo/bar"', '/foo/bar' ), # XXX not supported in tools yet ('include <abstractions/base>', 'abstractions/base' ), # magic path
(' some #include <abstractions/base>', None, ), ('include <abstractions/base> # comment', 'abstractions/base' ),
('include<abstractions/base>#comment', 'abstractions/base' ),
(' include <abstractions/base> ', 'abstractions/base' ),
('include "/foo/bar"', '/foo/bar' ), # absolute path
('include "/foo/bar" # comment', '/foo/bar' ),
('include "/foo/bar"#comment', '/foo/bar' ),
(' include "/foo/bar" ', '/foo/bar' ),
(' some #include <abstractions/base>', None, ), # non-matching
(' /etc/fstab r,', None, ), (' /etc/fstab r,', None, ),
('/usr/include r,', None, ),
('/include r,', None, ),
] ]
def _run_test(self, params, expected): def _run_test(self, params, expected):
@ -455,8 +465,53 @@ class Test_re_match_include(AATest):
class TestInvalid_re_match_include(AATest): class TestInvalid_re_match_include(AATest):
tests = [ tests = [
('#include <>', AppArmorException ), ('#include <>', AppArmorException ), # '#include'
('#include < >', AppArmorException ), ('#include < >', AppArmorException ),
('#include ""', AppArmorException ),
('#include " "', AppArmorException ),
('#include', AppArmorException ),
('#include ', AppArmorException ),
('#include "foo"', AppArmorException ), # LP: 1738880 (relative)
('#include "foo" # comment', AppArmorException ),
('#include "foo"#comment', AppArmorException ),
(' #include "foo" ', AppArmorException ),
('#include "foo/bar"', AppArmorException ),
('#include "foo/bar" # comment', AppArmorException ),
('#include "foo/bar"#comment', AppArmorException ),
(' #include "foo/bar" ', AppArmorException ),
('#include foo', AppArmorException ), # LP: 1738879 (no quotes)
('#include foo/bar', AppArmorException ),
('#include /foo/bar', AppArmorException ),
('#include foo bar', AppArmorException ), # LP: 1738877 (space in name)
('#include foo bar/baz', AppArmorException ),
('#include "foo bar"', AppArmorException ),
('#include /foo bar', AppArmorException ),
('#include "/foo bar"', AppArmorException ),
('#include "foo bar/baz"', AppArmorException ),
('include <>', AppArmorException ), # 'include'
('include < >', AppArmorException ),
('include ""', AppArmorException ),
('include " "', AppArmorException ),
('include', AppArmorException ),
('include ', AppArmorException ),
('include "foo"', AppArmorException ), # LP: 1738880 (relative)
('include "foo" # comment', AppArmorException ),
('include "foo"#comment', AppArmorException ),
(' include "foo" ', AppArmorException ),
('include "foo/bar"', AppArmorException ),
('include "foo/bar" # comment', AppArmorException ),
('include "foo/bar"#comment', AppArmorException ),
(' include "foo/bar" ', AppArmorException ),
('include foo', AppArmorException ), # LP: 1738879 (no quotes)
('include foo/bar', AppArmorException ),
('include /foo/bar', AppArmorException ),
('include foo bar', AppArmorException ), # LP: 1738877 (space in name)
('include foo bar/baz', AppArmorException ),
('include "foo bar"', AppArmorException ),
('include /foo bar', AppArmorException ),
('include "/foo bar"', AppArmorException ),
('include "foo bar/baz"', AppArmorException ),
] ]
def _run_test(self, params, expected): def _run_test(self, params, expected):