Hello, this patch changes aa.py to use FileRule and FileRuleset for parsing and saving profiles.
In detail, this means: - add 'file' to the list of rule classes to enable it at various places - store file rules in aa[profile][hat]['file'] (not 'path' as before) to be consistent with the FileRule name - drop the no longer needed delete_path_duplicates() - this is now handled by FileRuleset like in all other rule classes. (same change in cleanprofile.py) - replace usage of RE_PROFILE_BARE_FILE_ENTRY and RE_PROFILE_PATH_ENTRY with FileRule.match() - drop write_path_rules() and write_paths() and replace them with the new write_file() function. - adjust several code sections to use write_file and 'file' instead of 'path' FileRule doesn't drop optional keywords ('allow' and 'file'), therefore adjust cleanprof_test.out to the changed behaviour. (If someone insists on dropping optional keywords in aa-cleanprof, that's something for a future patch.) Also adjust the list of known failures in test-parser-simple-tests.py - switching to FileRule avoids several test failures (and introduces a few new ones ;-) IMPORTANT: This patch introduces a "brain split" which means - parsing and writing the profile and aa-cleanprof use the new location (aa[profile][hat]['file']) - aa-logprof and aa-genprof still save data to the old location (aa[profile][hat]['allow']['path']) and probably ask superfluous questions because there are no rules existing in the old location TL;DR: don't try aa-logprof or aa-genprof with only this patch applied. I know this isn't ideal, but still better than an even bigger and totally unreadable patch ;-) [ 14-switch-to-FileRule.diff ] === modified file ./utils/apparmor/aa.py --- utils/apparmor/aa.py 2016-02-04 17:51:59.552181829 +0100 +++ utils/apparmor/aa.py 2016-02-04 20:19:14.734161664 +0100 @@ -36,7 +36,7 @@ import apparmor.ui as aaui -from apparmor.aamode import (str_to_mode, mode_to_str, split_mode, +from apparmor.aamode import (str_to_mode, mode_to_str, mode_to_str_user, mode_contains, AA_OTHER, flatten_mode, owner_flatten_mode) @@ -44,7 +44,6 @@ RE_PROFILE_ALIAS, RE_PROFILE_BOOLEAN, RE_PROFILE_VARIABLE, RE_PROFILE_CONDITIONAL, RE_PROFILE_CONDITIONAL_VARIABLE, RE_PROFILE_CONDITIONAL_BOOLEAN, - RE_PROFILE_BARE_FILE_ENTRY, RE_PROFILE_PATH_ENTRY, RE_PROFILE_CHANGE_HAT, RE_PROFILE_HAT_DEF, RE_PROFILE_MOUNT, RE_PROFILE_PIVOT_ROOT, @@ -56,13 +55,14 @@ from apparmor.rule.capability import CapabilityRuleset, CapabilityRule from apparmor.rule.change_profile import ChangeProfileRuleset, ChangeProfileRule from apparmor.rule.dbus import DbusRuleset, DbusRule +from apparmor.rule.file import FileRuleset, FileRule from apparmor.rule.network import NetworkRuleset, NetworkRule from apparmor.rule.ptrace import PtraceRuleset, PtraceRule from apparmor.rule.rlimit import RlimitRuleset, RlimitRule from apparmor.rule.signal import SignalRuleset, SignalRule -from apparmor.rule import parse_modifiers, quote_if_needed +from apparmor.rule import quote_if_needed -ruletypes = ['capability', 'change_profile', 'dbus', 'network', 'ptrace', 'rlimit', 'signal'] +ruletypes = ['capability', 'change_profile', 'dbus', 'file', 'network', 'ptrace', 'rlimit', 'signal'] from apparmor.yasti import SendDataToYast, GetDataFromYast, shutdown_yast @@ -457,6 +464,7 @@ profile['capability'] = CapabilityRuleset() profile['dbus'] = DbusRuleset() + profile['file'] = FileRuleset() profile['change_profile'] = ChangeProfileRuleset() profile['network'] = NetworkRuleset() profile['ptrace'] = PtraceRuleset() @@ -2016,22 +2014,6 @@ newpath = re.sub('/[^/]+(\.[^/]+)$', '/*' + match.groups()[0], newpath) return newpath -def delete_path_duplicates(profile, incname, allow): - deleted = [] - for entry in profile[allow]['path'].keys(): - if entry == '#include <%s>' % incname: - continue - # XXX Make this code smart enough to know that bare file rules - # makes some path rules unnecessary. For example, "/dev/random r," - # would no longer be needed if "file," was present. - cm, am, m = match_include_to_path(incname, allow, entry) - if cm and mode_contains(cm, profile[allow]['path'][entry]['mode']) and mode_contains(am, profile[allow]['path'][entry]['audit']): - deleted.append(entry) - - for entry in deleted: - profile[allow]['path'].pop(entry) - return len(deleted) - def delete_duplicates(profile, incname): deleted = 0 # Allow rules covered by denied rules shouldn't be deleted @@ -2041,16 +2023,10 @@ for rule_type in ruletypes: deleted += profile[rule_type].delete_duplicates(include[incname][incname][rule_type]) - deleted += delete_path_duplicates(profile, incname, 'allow') - deleted += delete_path_duplicates(profile, incname, 'deny') - elif filelist.get(incname, False): for rule_type in ruletypes: deleted += profile[rule_type].delete_duplicates(filelist[incname][incname][rule_type]) - deleted += delete_path_duplicates(profile, incname, 'allow') - deleted += delete_path_duplicates(profile, incname, 'deny') - return deleted def match_includes(profile, rule_type, rule_obj): @@ -2735,86 +2734,6 @@ # Conditional Boolean defined pass - elif RE_PROFILE_BARE_FILE_ENTRY.search(line): - matches = RE_PROFILE_BARE_FILE_ENTRY.search(line) - - if not profile: - raise AppArmorException(_('Syntax Error: Unexpected bare file rule found in file: %(file)s line: %(line)s') % { 'file': file, 'line': lineno + 1 }) - - audit, deny, allow_keyword, comment = parse_modifiers(matches) - # TODO: honor allow_keyword and comment - if deny: - allow = 'deny' - else: - allow = 'allow' - - mode = apparmor.aamode.AA_BARE_FILE_MODE - if not matches.group('owner'): - mode |= AA_OTHER(apparmor.aamode.AA_BARE_FILE_MODE) - - path_rule = profile_data[profile][hat][allow]['path'][ALL] - path_rule['mode'] = mode - path_rule['audit'] = set() - if audit: - path_rule['audit'] = mode - path_rule['file_prefix'] = True - - elif RE_PROFILE_PATH_ENTRY.search(line): - matches = RE_PROFILE_PATH_ENTRY.search(line).groups() - - if not profile: - raise AppArmorException(_('Syntax Error: Unexpected path entry found in file: %(file)s line: %(line)s') % { 'file': file, 'line': 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 - - file_prefix = False - if matches[3]: - file_prefix = True - - path = strip_quotes(matches[4].strip()) - mode = matches[5] - nt_name = matches[7] - if nt_name: - nt_name = nt_name.strip() - - p_re = convert_regexp(path) - try: - re.compile(p_re) - except: - raise AppArmorException(_('Syntax Error: Invalid Regex %(path)s in file: %(file)s line: %(line)s') % { 'path': path, 'file': file, 'line': lineno + 1 }) - - if not validate_profile_mode(mode, allow, nt_name): - raise AppArmorException(_('Invalid mode %(mode)s in file: %(file)s line: %(line)s') % {'mode': mode, 'file': file, 'line': lineno + 1 }) - - tmpmode = set() - 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 - - 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_match_include(line): # Include files include_name = re_match_include(line) @@ -2985,6 +2904,13 @@ else: initial_comment = initial_comment + line + '\n' + elif FileRule.match(line): + # leading permissions could look like a keyword, therefore handle file rules after everything else + if not profile: + raise AppArmorException(_('Syntax Error: Unexpected path entry found in file: %(file)s line: %(line)s') % { 'file': file, 'line': lineno + 1 }) + + profile_data[profile][hat]['file'].add(FileRule.parse(line)) + elif not RE_RULE_HAS_COMMA.search(line): # Bah, line continues on to the next line if RE_HAS_COMMENT_SPLIT.search(line): @@ -3282,76 +3208,10 @@ return data -def write_path_rules(prof_data, depth, allow): - pre = ' ' * depth +def write_file(prof_data, depth): data = [] - allowstr = set_allow_str(allow) - - 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) - - while user or other: - ownerstr = '' - tmpmode = 0 - tmpaudit = False - if user - other: - # if no other mode set - ownerstr = 'owner ' - tmpmode = user - other - tmpaudit = user_audit - user = user - tmpmode - else: - if user_audit - other_audit & user: - ownerstr = 'owner ' - tmpaudit = user_audit - other_audit & user - tmpmode = user & tmpaudit - user = user - tmpmode - else: - ownerstr = '' - tmpmode = user | other - tmpaudit = user_audit | other_audit - 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) - 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) - 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 - -def write_paths(prof_data, depth): - data = write_path_rules(prof_data, depth, 'deny') - data += write_path_rules(prof_data, depth, 'allow') - + if prof_data.get('file', False): + data = prof_data['file'].get_clean(depth) return data def write_rules(prof_data, depth): @@ -3368,7 +3228,7 @@ data += write_pivot_root(prof_data, depth) data += write_unix(prof_data, depth) data += write_links(prof_data, depth) - data += write_paths(prof_data, depth) + data += write_file(prof_data, depth) data += write_change_profile(prof_data, depth) return data @@ -3530,7 +3390,7 @@ 'pivot_root': write_pivot_root, 'unix': write_unix, 'link': write_links, - 'path': write_paths, + 'file': write_file, 'change_profile': write_change_profile, } default_write_order = [ 'alias', @@ -3546,7 +3406,7 @@ 'pivot_root', 'unix', 'link', - 'path', + 'file', 'change_profile', ] # prof_correct = True # XXX correct? @@ -3563,7 +3423,7 @@ 'pivot_root': True, # not handled otherwise yet 'unix': True, # not handled otherwise yet 'link': False, - 'path': False, + 'file': False, 'change_profile': False, 'include_local_started': False, # unused } @@ -3813,78 +3673,6 @@ #To-Do pass - 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].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 = write_prof_data[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(): - data += write_prior_segments(write_prof_data[name], segments, line) - segments['path'] = True - write_prof_data[hat][allow]['path'].pop(ALL) - data.append(line) - - elif RE_PROFILE_PATH_ENTRY.search(line): - matches = RE_PROFILE_PATH_ENTRY.search(line).groups() - audit = False - if matches[0]: - audit = True - allow = 'allow' - if matches[1] and matches[1].split() == 'deny': - allow = 'deny' - - user = False - if matches[2]: - user = True - - path = strip_quotes(matches[4].strip()) - mode = matches[5] - nt_name = matches[7] - if nt_name: - nt_name = nt_name.strip() - - tmpmode = set() - if user: - tmpmode = str_to_mode('%s::' % mode) - else: - tmpmode = str_to_mode(mode) - - if not write_prof_data[hat][allow]['path'].get(path): - correct = False - else: - 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 - - if correct: - if not segments['path'] and True in segments.values(): - data += write_prior_segments(write_prof_data[name], segments, line) - segments['path'] = True - write_prof_data[hat][allow]['path'].pop(path) - data.append(line) - else: - #To-Do - pass - elif re_match_include(line): include_name = re_match_include(line) if profile: @@ -3928,6 +3716,20 @@ else: #To-Do pass + elif FileRule.match(line): + # leading permissions could look like a keyword, therefore handle file rules after everything else + file_obj = FileRule.parse(line) + + if write_prof_data[hat]['file'].is_covered(file_obj, True, True): + if not segments['file'] and True in segments.values(): + data += write_prior_segments(write_prof_data[name], segments, line) + segments['file'] = True + write_prof_data[hat]['file'].delete(file_obj) + data.append(line) + else: + #To-Do + pass + else: if correct: data.append(line) === modified file ./utils/apparmor/cleanprofile.py --- utils/apparmor/cleanprofile.py 2015-12-03 22:04:36.782275414 +0100 +++ utils/apparmor/cleanprofile.py 2016-02-04 20:15:13.227661220 +0100 @@ -13,7 +13,6 @@ # # ---------------------------------------------------------------------- import apparmor.aa as apparmor -from apparmor.regex import re_match_include class Prof(object): def __init__(self, filename): @@ -71,39 +71,4 @@ else: deleted += self.other.aa[program][hat][ruletype].delete_duplicates(None) - #Clean the duplicates of path in other profile - deleted += delete_path_duplicates(self.profile.aa[program][hat], self.other.aa[program][hat], 'allow', self.same_file) - deleted += delete_path_duplicates(self.profile.aa[program][hat], self.other.aa[program][hat], 'deny', self.same_file) - return deleted - -def delete_path_duplicates(profile, profile_other, allow, same_profile=True): - deleted = [] - # Check if any individual rule makes any rule superfluous - for rule in profile[allow]['path'].keys(): - for entry in profile_other[allow]['path'].keys(): - if rule == entry: - # Check the modes - cm = profile[allow]['path'][rule]['mode'] - am = profile[allow]['path'][rule]['audit'] - # If modes of rule are a superset of rules implied by entry we can safely remove it - if apparmor.mode_contains(cm, profile_other[allow]['path'][entry]['mode']) and apparmor.mode_contains(am, profile_other[allow]['path'][entry]['audit']): - if not same_profile: - deleted.append(entry) - continue - if re_match_include(rule) or re_match_include(entry): - continue - # Check if the rule implies entry - if apparmor.matchliteral(rule, entry): - # Check the modes - cm = profile[allow]['path'][rule]['mode'] - am = profile[allow]['path'][rule]['audit'] - # If modes of rule are a superset of rules implied by entry we can safely remove it - if apparmor.mode_contains(cm, profile_other[allow]['path'][entry]['mode']) and apparmor.mode_contains(am, profile_other[allow]['path'][entry]['audit']): - deleted.append(entry) - - for entry in deleted: - profile_other[allow]['path'].pop(entry) - - return len(deleted) - === modified file ./utils/test/cleanprof_test.out --- utils/test/cleanprof_test.out 2016-02-01 21:31:56.419302952 +0100 +++ utils/test/cleanprof_test.out 2016-02-01 18:15:00.895533604 +0100 @@ -20,8 +20,8 @@ unix (receive) type=dgram, - /home/*/** r, - /home/foo/** w, + allow /home/*/** r, + allow /home/foo/** w, change_profile, @@ -34,7 +34,7 @@ } } /usr/bin/other/cleanprof/test/profile { - /home/*/** rw, - /home/foo/bar r, + allow /home/*/** rw, + allow /home/foo/bar r, } === modified file ./utils/test/test-parser-simple-tests.py --- utils/test/test-parser-simple-tests.py 2016-02-01 21:31:56.415302976 +0100 +++ utils/test/test-parser-simple-tests.py 2016-01-26 22:22:31.505637218 +0100 @@ -53,17 +53,9 @@ 'dbus/bad_regex_04.sd', 'dbus/bad_regex_05.sd', 'dbus/bad_regex_06.sd', - 'file/bad_append_1.sd', - 'file/bad_append_2.sd', - 'file/bad_embedded_spaces_1.sd', + 'file/bad_re_brace_1.sd', 'file/bad_re_brace_2.sd', 'file/bad_re_brace_3.sd', - 'file/file/bad_append_1.sd', - 'file/file/bad_embedded_spaces_1.sd', - 'file/file/owner/bad_3.sd', - 'file/file/owner/bad_5.sd', - 'file/owner/bad_3.sd', - 'file/owner/bad_5.sd', 'mount/bad_opt_10.sd', 'mount/bad_opt_11.sd', 'mount/bad_opt_12.sd', @@ -188,17 +180,10 @@ 'xtrans/simple_bad_conflicting_x_11.sd', 'xtrans/simple_bad_conflicting_x_12.sd', 'xtrans/simple_bad_conflicting_x_13.sd', - 'xtrans/simple_bad_conflicting_x_14.sd', - 'xtrans/simple_bad_conflicting_x_15.sd', - 'xtrans/simple_bad_conflicting_x_1.sd', 'xtrans/simple_bad_conflicting_x_2.sd', - 'xtrans/simple_bad_conflicting_x_3.sd', 'xtrans/simple_bad_conflicting_x_4.sd', - 'xtrans/simple_bad_conflicting_x_5.sd', 'xtrans/simple_bad_conflicting_x_6.sd', - 'xtrans/simple_bad_conflicting_x_7.sd', 'xtrans/simple_bad_conflicting_x_8.sd', - 'xtrans/simple_bad_conflicting_x_9.sd', 'xtrans/x-conflict.sd', ] @@ -211,11 +196,9 @@ 'file/ok_other_2.sd', 'file/ok_other_3.sd', - # permissions before path + # 'unsafe' keyword 'file/file/front_perms_ok_1.sd', 'file/front_perms_ok_1.sd', - 'profile/local/local_named_profile_ok1.sd', - 'profile/local/local_profile_ok1.sd', 'xtrans/simple_ok_cx_1.sd', # permissions before path and owner / audit {...} blocks @@ -267,6 +250,9 @@ 'file/ok_5.sd', # Invalid mode UX 'file/ok_2.sd', # Invalid mode RWM 'file/ok_4.sd', # Invalid mode iX + 'file/ok_embedded_spaces_4.sd', # \-escaped space + 'file/file/ok_embedded_spaces_4.sd', # \-escaped space + 'file/ok_quoted_4.sd', # quoted string including \" 'xtrans/simple_ok_pix_1.sd', # Invalid mode pIx 'xtrans/simple_ok_pux_1.sd', # Invalid mode rPux Regards, Christian Boltz -- > Einfach mal nach Maengelexemplaren suchen (z.B. bei Amazon- > Marketplace). Habe mir dort gerade den Latex-Begleiter ich wusste garnicht das Amazon auch eine SM-Abteilung hat: Latex-Begleiter und das auch noch zu kaufen ... ;-) [> Heinz W. Pahlke und Rolf Masfelder in suse-linux]
signature.asc
Description: This is a digitally signed message part.
-- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor