[apparmor] [patch] [14/38] Use FileRule and FileRuleset
Christian Boltz
apparmor at cboltz.de
Fri Aug 12 20:54:53 UTC 2016
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]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20160812/332c9a20/attachment-0001.pgp>
More information about the AppArmor
mailing list