Hello,
this patch changes aa.py and cleanprof.py to use the new capabiliy rule
class.
The most important details in the change are:
- the capability rules are stored in
aa[profile][hat]['capability'] instead of
aa[profile][hat]['allow']['capability'] and
aa[profile][hat]['deny']['capability']
(allow/deny is handled inside the capability_rules class)
- profile_known_capability() now just returns True or False. Before, it
returned different values for deny and allow, but the calling code
doesn't care about this detail anyway.
Some things are still a bit ugly (and commented as such). On the long
term, I plan to change logparser so that it returns a set of *_rule
classes instead of the current array with parsed events. This will
remove some of the ugly tricks I had to add.
We should also come up with a function that initializes the structure
for each profile in aa[profile][hat] - with that, we could drop several
safety checks I had to add to avoid problems with profiles that don't
contain a capability rule.
I also had to add several
+ if write_prof_data[name].get(segs, False):
+ write_prof_data[name][segs].delete_all_rules()
in serialize_profile_from_old_profile(). That's needed to avoid writing
rules twice. On the positive side, when we have converted everything to
rule classes, we can delete the four lines above each of these added
lines :-)
The patch also contains a small bugfix - when choosing "deny" for a
capability, the audit flag was ignored. Maybe it also fixes some other
small bugs we didn't even notice before ;-)
Let me finalize this patch series with the line count statistics:
1-add-base-and-capability-rule-class.diff - 329 lines added, 0 removed
2-add-capability-rule-test.diff - 657 lines added, 0 removed
3-use-capability-rule-class.diff - 62 lines added, 107 removed
That all said - I'm looking forward for feedback ;-)
Enjoy reviewing the patches!
Regards,
Christian Boltz
--
PHP bietet einige Möglicheiten etwas falsch zu machen.
Die phpBB Entwickler haben wohl viele dieser Möglichkeiten genutzt.
[Kommentar auf http://www.pro-linux.de/news/2007/12106.html]
=== modified file 'utils/apparmor/aa.py'
--- utils/apparmor/aa.py 2014-11-15 11:51:24 +0000
+++ utils/apparmor/aa.py 2014-11-15 21:14:56 +0000
@@ -52,6 +52,8 @@
import apparmor.rules as aarules
+from apparmor.rule.capability import capability_rules, capability_rule
+
from apparmor.yasti import SendDataToYast, GetDataFromYast, shutdown_yast
# setup module translations
@@ -89,7 +91,7 @@
# To store the globs entered by users so they can be provided again
user_globs = []
-# The key for representing bare rules such as "capability," or "file,"
+# The key for representing bare "file," rules
ALL = '\0ALL'
## Variables used under logprof
@@ -1628,16 +1630,16 @@
if deleted:
aaui.UI_Info(_('Deleted %s previous matching profile entries.') % deleted)
- aa[profile][hat]['allow']['capability'][capability]['set'] = True
- aa[profile][hat]['allow']['capability'][capability]['audit'] = audit_toggle
+ else:
+ aa[profile][hat]['capability'].add_raw('%s capability %s,'% (audit, capability)) # XXX yes, ugly ;-)
+ aaui.UI_Info(_('Adding capability %s to profile.') % capability)
changed[profile] = True
- aaui.UI_Info(_('Adding capability %s to profile.') % capability)
done = True
elif ans == 'CMD_DENY':
- aa[profile][hat]['deny']['capability'][capability]['set'] = True
+ aa[profile][hat]['capability'].add_raw('%s deny capability %s,'% (audit, capability)) # XXX yes, ugly ;-)
changed[profile] = True
aaui.UI_Info(_('Denying capability %s to profile.') % capability)
@@ -2121,20 +2123,6 @@
deleted += 1
return deleted
-def delete_cap_duplicates(profilecaps, inccaps):
- deleted = []
- if profilecaps and inccaps:
- for capname in profilecaps.keys():
- # XXX The presence of a bare capability rule ("capability,") should
- # cause more specific capability rules
- # ("capability audit_control,") to be deleted
- if inccaps[capname].get('set', False) == 1:
- deleted.append(capname)
- for capname in deleted:
- profilecaps.pop(capname)
-
- return len(deleted)
-
def delete_path_duplicates(profile, incname, allow):
deleted = []
for entry in profile[allow]['path'].keys():
@@ -2161,9 +2149,7 @@
deleted += delete_net_duplicates(profile['deny']['netdomain'], include[incname][incname]['deny']['netdomain'])
- deleted += delete_cap_duplicates(profile['allow']['capability'], include[incname][incname]['allow']['capability'])
-
- deleted += delete_cap_duplicates(profile['deny']['capability'], include[incname][incname]['deny']['capability'])
+ deleted += profile['capability'].delete_duplicates(include[incname][incname]['capability'])
deleted += delete_path_duplicates(profile, incname, 'allow')
deleted += delete_path_duplicates(profile, incname, 'deny')
@@ -2173,9 +2159,7 @@
deleted += delete_net_duplicates(profile['deny']['netdomain'], filelist[incname][incname]['deny']['netdomain'])
- deleted += delete_cap_duplicates(profile['allow']['capability'], filelist[incname][incname]['allow']['capability'])
-
- deleted += delete_cap_duplicates(profile['deny']['capability'], filelist[incname][incname]['deny']['capability'])
+ deleted += profile['capability'].delete_duplicates(filelist[incname][incname]['capability'])
deleted += delete_path_duplicates(profile, incname, 'allow')
deleted += delete_path_duplicates(profile, incname, 'deny')
@@ -2206,7 +2190,9 @@
def match_cap_includes(profile, cap):
newincludes = []
for incname in include.keys():
- if valid_include(profile, incname) and include[incname][incname]['allow']['capability'][cap].get('set', False) == 1:
+ # XXX type check should go away once we init all profiles correctly
+ # XXX using a capability_rule object instead of covered_raw() might be nice/faster
+ if valid_include(profile, incname) and type(include[incname][incname]['capability']) == capability_rules and include[incname][incname]['capability'].covered_raw("capability %s," % cap):
newincludes.append(incname)
return newincludes
@@ -2516,7 +2502,8 @@
for capability in prelog[aamode][profile][hat]['capability'].keys():
# If capability not already in profile
- if not aa[profile][hat]['allow']['capability'][capability].get('set', False):
+ # XXX remove first check when we have proper profile initialisation
+ if aa[profile][hat].get('capability', False) and not aa[profile][hat]['capability'].covered_raw("capability %s," % capability):
log_dict[aamode][profile][hat]['capability'][capability] = True
nd = prelog[aamode][profile][hat]['netdomain']
@@ -2704,6 +2691,10 @@
profile_data[profile][profile]['repo']['url'] = repo_data['url']
profile_data[profile][profile]['repo']['user'] = repo_data['user']
+ # init rule classes (if not done yet)
+ if not profile_data[profile][hat].get('capability', False):
+ profile_data[profile][hat]['capability'] = capability_rules()
+
elif RE_PROFILE_END.search(line):
# If profile ends and we're not in one
if not profile:
@@ -2719,21 +2710,14 @@
initial_comment = ''
elif RE_PROFILE_CAP.search(line):
- matches = RE_PROFILE_CAP.search(line)
-
if not profile:
raise AppArmorException(_('Syntax Error: Unexpected capability entry found in file: %(file)s line: %(line)s') % { 'file': file, 'line': lineno + 1 })
- audit, allow, allow_keyword, comment = parse_audit_allow(matches)
- # TODO: honor allow_keyword and comment
-
- capability = ALL
- if matches.group('capability'):
- capability = matches.group('capability').strip()
- # TODO: can contain more than one capability- split it?
-
- profile_data[profile][hat][allow]['capability'][capability]['set'] = True
- profile_data[profile][hat][allow]['capability'][capability]['audit'] = audit
+ # init rule class (if not done yet)
+ if not profile_data[profile][hat].get('capability', False):
+ profile_data[profile][hat]['capability'] = capability_rules()
+
+ profile_data[profile][hat]['capability'].add_raw(line)
elif RE_PROFILE_LINK.search(line):
matches = RE_PROFILE_LINK.search(line).groups()
@@ -3373,29 +3357,8 @@
def write_list_vars(prof_data, depth):
return write_pair(prof_data, depth, '', 'lvar', '', ' = ', '', var_transform)
-def write_cap_rules(prof_data, depth, allow):
- pre = ' ' * depth
- data = []
- allowstr = set_allow_str(allow)
-
- if prof_data[allow].get('capability', False):
- for cap in sorted(prof_data[allow]['capability'].keys()):
- audit = ''
- if prof_data[allow]['capability'][cap].get('audit', False):
- audit = 'audit '
- if prof_data[allow]['capability'][cap].get('set', False):
- if cap == ALL:
- data.append('%s%s%scapability,' % (pre, audit, allowstr))
- else:
- data.append('%s%s%scapability %s,' % (pre, audit, allowstr, cap))
- data.append('')
-
- return data
-
def write_capabilities(prof_data, depth):
- #data = write_single(prof_data, depth, '', 'set_capability', 'set capability ', ',')
- data = write_cap_rules(prof_data, depth, 'deny')
- data += write_cap_rules(prof_data, depth, 'allow')
+ data = prof_data['capability'].get_clean(depth)
return data
def write_net_rules(prof_data, depth, allow):
@@ -3886,6 +3849,8 @@
write_prof_data[name]['allow'].pop(segs)
if write_prof_data[name]['deny'].get(segs, False):
write_prof_data[name]['deny'].pop(segs)
+ if write_prof_data[name].get(segs, False):
+ write_prof_data[name][segs].delete_all_rules()
# then write everything else
for segs in default_write_order:
@@ -3927,25 +3892,7 @@
profile = None
elif RE_PROFILE_CAP.search(line):
- matches = RE_PROFILE_CAP.search(line).groups()
- audit = False
- if matches[0]:
- audit = matches[0]
-
- allow = 'allow'
- if matches[1] and matches[1].strip() == 'deny':
- allow = 'deny'
-
- capability = ALL
- if matches[2]:
- capability = matches[2].strip()
-
- if not write_prof_data[hat][allow]['capability'][capability].get('set', False):
- correct = False
- if not write_prof_data[hat][allow]['capability'][capability].get(audit, False) == audit:
- correct = False
-
- if correct:
+ if write_prof_data[hat]['capability'].covered_raw(line, True, True):
if not segments['capability'] and True in segments.values():
for segs in list(filter(lambda x: segments[x], segments.keys())):
depth = len(line) - len(line.lstrip())
@@ -3955,13 +3902,11 @@
write_prof_data[name]['allow'].pop(segs)
if write_prof_data[name]['deny'].get(segs, False):
write_prof_data[name]['deny'].pop(segs)
+ if write_prof_data[name].get(segs, False):
+ write_prof_data[name][segs].delete_all_rules()
segments['capability'] = True
- write_prof_data[hat][allow]['capability'].pop(capability)
+ write_prof_data[hat]['capability'].delete_raw(line)
data.append(line)
-
- #write_prof_data[hat][allow]['capability'][capability].pop(audit)
-
- #Remove this line
else:
# To-Do
pass
@@ -3996,6 +3941,8 @@
write_prof_data[name]['allow'].pop(segs)
if write_prof_data[name]['deny'].get(segs, False):
write_prof_data[name]['deny'].pop(segs)
+ if write_prof_data[name].get(segs, False):
+ write_prof_data[name][segs].delete_all_rules()
segments['link'] = True
write_prof_data[hat][allow]['link'].pop(link)
data.append(line)
@@ -4020,6 +3967,8 @@
write_prof_data[name]['allow'].pop(segs)
if write_prof_data[name]['deny'].get(segs, False):
write_prof_data[name]['deny'].pop(segs)
+ if write_prof_data[name].get(segs, False):
+ write_prof_data[name][segs].delete_all_rules()
segments['change_profile'] = True
write_prof_data[hat]['change_profile'].pop(cp)
data.append(line)
@@ -4050,6 +3999,8 @@
write_prof_data[name]['allow'].pop(segs)
if write_prof_data[name]['deny'].get(segs, False):
write_prof_data[name]['deny'].pop(segs)
+ if write_prof_data[name].get(segs, False):
+ write_prof_data[name][segs].delete_all_rules()
segments['alias'] = True
if profile:
write_prof_data[hat]['alias'].pop(from_name)
@@ -4079,6 +4030,8 @@
write_prof_data[name]['allow'].pop(segs)
if write_prof_data[name]['deny'].get(segs, False):
write_prof_data[name]['deny'].pop(segs)
+ if write_prof_data[name].get(segs, False):
+ write_prof_data[name][segs].delete_all_rules()
segments['rlimit'] = True
write_prof_data[hat]['rlimit'].pop(from_name)
data.append(line)
@@ -4104,6 +4057,8 @@
write_prof_data[name]['allow'].pop(segs)
if write_prof_data[name]['deny'].get(segs, False):
write_prof_data[name]['deny'].pop(segs)
+ if write_prof_data[name].get(segs, False):
+ write_prof_data[name][segs].delete_all_rules()
segments['lvar'] = True
write_prof_data[hat]['lvar'].pop(bool_var)
data.append(line)
@@ -4135,6 +4090,8 @@
write_prof_data[name]['allow'].pop(segs)
if write_prof_data[name]['deny'].get(segs, False):
write_prof_data[name]['deny'].pop(segs)
+ if write_prof_data[name].get(segs, False):
+ write_prof_data[name][segs].delete_all_rules()
segments['lvar'] = True
if profile:
write_prof_data[hat]['lvar'].pop(list_var)
@@ -4173,6 +4130,8 @@
write_prof_data[name]['allow'].pop(segs)
if write_prof_data[name]['deny'].get(segs, False):
write_prof_data[name]['deny'].pop(segs)
+ if write_prof_data[name].get(segs, False):
+ write_prof_data[name][segs].delete_all_rules()
segments['path'] = True
write_prof_data[hat][allow]['path'].pop(ALL)
data.append(line)
@@ -4221,6 +4180,8 @@
write_prof_data[name]['allow'].pop(segs)
if write_prof_data[name]['deny'].get(segs, False):
write_prof_data[name]['deny'].pop(segs)
+ if write_prof_data[name].get(segs, False):
+ write_prof_data[name][segs].delete_all_rules()
segments['path'] = True
write_prof_data[hat][allow]['path'].pop(path)
data.append(line)
@@ -4241,6 +4202,8 @@
write_prof_data[name]['allow'].pop(segs)
if write_prof_data[name]['deny'].get(segs, False):
write_prof_data[name]['deny'].pop(segs)
+ if write_prof_data[name].get(segs, False):
+ write_prof_data[name][segs].delete_all_rules()
segments['include'] = True
write_prof_data[hat]['include'].pop(include_name)
data.append(line)
@@ -4294,6 +4257,8 @@
write_prof_data[name]['allow'].pop(segs)
if write_prof_data[name]['deny'].get(segs, False):
write_prof_data[name]['deny'].pop(segs)
+ if write_prof_data[name].get(segs, False):
+ write_prof_data[name][segs].delete_all_rules()
segments['netdomain'] = True
elif RE_PROFILE_CHANGE_HAT.search(line):
@@ -4408,19 +4373,20 @@
return 0
def profile_known_capability(profile, capname):
- if profile['deny']['capability'][capname].get('set', False):
- return -1
+ rule = capability_rule()
+ rule.set_raw('capability %s,' % capname) # having a capability_rule object as parameter would be even better ;-)
- if profile['allow']['capability'][capname].get('set', False):
- return 1
+ # XXX get rid of type checks after we have a proper function to initialize a profile
+ if type(profile['capability']) == capability_rules:
+ if profile['capability'].covered_obj(rule, False):
+ return True
for incname in profile['include'].keys():
- if include[incname][incname]['deny']['capability'][capname].get('set', False):
- return -1
- if include[incname][incname]['allow']['capability'][capname].get('set', False):
- return 1
+ if type(include[incname][incname]['capability']) == capability_rules:
+ if include[incname][incname]['capability'].covered_obj(rule, False):
+ return True
- return 0
+ return False
def profile_known_network(profile, family, sock_type):
if netrules_access_check(profile['deny']['netdomain'], family, sock_type):
=== modified file 'utils/apparmor/cleanprofile.py'
--- utils/apparmor/cleanprofile.py 2014-09-05 21:21:00 +0000
+++ utils/apparmor/cleanprofile.py 2014-11-15 18:48:23 +0000
@@ -65,8 +65,8 @@
deleted += apparmor.aa.delete_duplicates(self.other.aa[program][hat], inc)
#Clean the duplicates of caps in other profile
- deleted += delete_cap_duplicates(self.profile.aa[program][hat]['allow']['capability'], self.other.aa[program][hat]['allow']['capability'], self.same_file)
- deleted += delete_cap_duplicates(self.profile.aa[program][hat]['deny']['capability'], self.other.aa[program][hat]['deny']['capability'], self.same_file)
+ if self.same_file:
+ deleted += self.other.aa[program][hat]['capability'].delete_duplicates(self.profile.aa[program][hat]['capability'])
#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)
@@ -108,17 +108,6 @@
return len(deleted)
-def delete_cap_duplicates(profilecaps, profilecaps_other, same_profile=True):
- deleted = []
- if profilecaps and profilecaps_other and not same_profile:
- for capname in profilecaps.keys():
- if profilecaps_other[capname].get('set', False):
- deleted.append(capname)
- for capname in deleted:
- profilecaps_other.pop(capname)
-
- return len(deleted)
-
def delete_net_duplicates(netrules, netrules_other, same_profile=True):
deleted = 0
hasher_obj = apparmor.aa.hasher()
--
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
