[apparmor] GSoC review r48..51
Christian Boltz
apparmor at cboltz.de
Thu Aug 22 20:39:54 UTC 2013
Hello,
Am Donnerstag, 22. August 2013 schrieb Christian Boltz:
> the review for r48, 49, 50 and 51 is attached.
... or not :-/
Let me try a second time ;-)
Regards,
Christian Boltz
--
# GO AWAY !
# YOU DO NOT WANT TO SEE THIS SCRIPT !!!
[from /opt/kde3/share/apps/krpmview/setup_temp_source]
-------------- next part --------------
merged review for:
------------------------------------------------------------
revno: 51
First set of tools in their alpha release, logprof and genprof are pre-bleeding edge so dont hurt yourself or worse your distro.
------------------------------------------------------------
revno: 50
Semmingly working writer from old profile
------------------------------------------------------------
revno: 49
first partially working iteration of new profile writer from old profile
------------------------------------------------------------
revno: 48
working commit prior to writer code alterations
=== added file 'Tools/aa-audit.py'
--- Tools/aa-audit.py 1970-01-01 00:00:00 +0000
+++ Tools/aa-audit.py 2013-08-21 05:56:09 +0000
@@ -0,0 +1,54 @@
+#!/usr/bin/python
+import sys
+import os
+import argparse
+
+import apparmor.aa as apparmor
+
+parser = argparse.ArgumentParser(description='Switch the given programs to audit mode')
+parser.add_argument('-d', type=str, help='path to profiles')
+parser.add_argument('program', type=str, nargs='+', help='name of program to hswitch to audit mode')
# typo: switch (not _h_switch)
+args = parser.parse_args()
+
+profiling = args.program
+profiledir = args.d
+
+if profiledir:
+ apparmor.profile_dir = apparmor.get_full_path(profiledir)
+ if not os.path.isdir(apparmor.profile_dir):
+ raise apparmor.AppArmorException("Can't find AppArmor profiles in %s." %profiledir)
# wrong error message, should be "%s is not a directory"
+ if os.path.exists(program):
+ apparmor.read_profiles()
+ filename = apparmor.get_profile_filename(program)
+
+ if not os.path.isfile(filename) or apparmor.is_skippable_file(filename):
+ continue
+
+ sys.stdout.write(_('Setting %s to audit mode.\n')%program)
+
+ apparmor.set_profile_flags(filename, 'audit')
# does this mean that existing flags (for example 'complain') are lost?
# (if yes, then complain() and enforce() in aa.py share the same bug)
# (hint: a function add_profile_flag() and remove_profile_flag() might be useful)
+ cmd_info = apparmor.cmd(['cat', filename, '|', parser, '-I%s'%apparmor.profile_dir, '-R 2>&1', '1>/dev/null'])
# The parser can read the file itsself - no need to use "cat"
+ if cmd_info[0] != 0:
+ raise apparmor.AppArmorException(cmd_info[1])
+ else:
+ if '/' not in p:
+ apparmor.UI_Info(_("Can't find %s in the system path list. If the name of the application is correct, please run 'which %s' as a user with correct PATH environment set up in order to find the fully-qualified path.")%(p, p))
# maybe add something like "Please give the full path as parameter"?
# I also have a feature request - add a commandline option to remove the audit flag
# (basically "aa-noaudit", but I doubt we need to make it a separate command)
# another feature request - aa-disable should get a similar option (basically aa-enable)
=== added file 'Tools/aa-autodep.py'
--- Tools/aa-autodep.py 1970-01-01 00:00:00 +0000
+++ Tools/aa-autodep.py 2013-08-21 05:56:09 +0000
@@ -0,0 +1,53 @@
+parser = argparse.ArgumentParser(description='Disable the profile for the given programs')
# wrong description ;-)
+parser.add_argument('-d', type=str, help='path to profiles')
+parser.add_argument('program', type=str, nargs='+', help='name of program to have profile disabled')
+args = parser.parse_args()
+
+profiledir = args.d
+profiling = args.program
+
+if profiledir:
+ apparmor.profile_dir = apparmor.get_full_path(profiledir)
+ if not os.path.isdir(apparmor.profile_dir):
+ raise apparmor.AppArmorException("Can't find AppArmor profiles in %s." %profiledir)
# wrong error message - should be "%s is not a directory"
+for p in profiling:
+ else:
+ if '/' not in p:
+ apparmor.UI_Info(_("Can't find %s in the system path list. If the name of the application is correct, please run 'which %s' as a user with correct PATH environment set up in order to find the fully-qualified path.")%(p, p))
# maybe add "and use the full path as parameter for aa-genprof" or something like that?
=== added file 'Tools/aa-complain.py'
--- Tools/aa-complain.py 1970-01-01 00:00:00 +0000
+++ Tools/aa-complain.py 2013-08-21 05:56:09 +0000
# exactly the same code as aa-audit (with s/audit/complain)
# and therefore the same notes apply
# This means you have multiple copies of the same code, with only 5 lines differing.
# The better solution would be to make aa-complain, aa-enforce, aa-audit etc. one-liners calling
# a common module that does the handling
# (or make a symlink aa-complain -> aa-audit and decide behaviour based on $0 - but I'd prefer using a common module)
=== added file 'Tools/aa-disable.py'
--- Tools/aa-disable.py 1970-01-01 00:00:00 +0000
+++ Tools/aa-disable.py 2013-08-21 05:56:09 +0000
# see aa-audit and aa-complain for the general notes.
+disabledir = apparmor.profile_dir+'/disable'
+if not os.path.isdir(disabledir):
+ raise apparmor.AppArmorException("Can't find AppArmor disable directorys %s." %disabledir)
# typo: "directory" (not "directory_s_")
+ bname = os.path.basename(filename)
+ if not bname:
+ apparmor.AppArmorException(_('Unable to find basename for %s.')%filename)
# missing "raise"?
=== added file 'Tools/aa-enforce.py'
--- Tools/aa-enforce.py 1970-01-01 00:00:00 +0000
+++ Tools/aa-enforce.py 2013-08-21 05:56:09 +0000
# see aa-audit for comments
=== added file 'Tools/aa-genprof.py'
--- Tools/aa-genprof.py 1970-01-01 00:00:00 +0000
+++ Tools/aa-genprof.py 2013-08-21 05:56:09 +0000
@@ -0,0 +1,148 @@
+def last_audit_entry_time():
+ out = subprocess.check_output(['tail', '-1', '/var/log/audit/audit.log'], shell=True)
# hey, we have a '-f' argument ;-)
+parser.add_argument('-f', type=str, help='path to logfile')
...
+filename = args.f
# filename is not used anywhere
+aa_mountpoint = apparmor.check_for_apparmor()
+if not aa_mountpoint:
+ raise apparmor.AppArmorException(_('AppArmor seems to have not been started. Please enable AppArmor and try again.'))
# the first sentence looks a bit complicated. I'm not a native english speaker, but what about "It seems AppArmor was not started."?
# (should be the same as in aa-logprof)
+if profiledir:
+ apparmor.profile_dir = apparmor.get_full_path(profiledir)
+ if not os.path.isdir(apparmor.profile_dir):
+ raise apparmor.AppArmorException("Can't find AppArmor profiles in %s." %profiledir)
# wrong error message - should be "not a directory"
...
+if not program or not os.path.exists(program):
+ if '/' not in profiling:
+ raise apparmor.AppArmorException(_("Can't find %s in the system path list. If the name of the application is correct, please run 'which %s' in another window in order to find the fully-qualified path.") %(profiling, profiling))
# maybe add "and use the full path as parameter for aa-genprof" or something like that?
+syslog = True
+logmark = ''
+done_profiling = False
+
+if os.path.exists('/var/log/audit/audit.log'):
+ syslog = False
# not sure if "audit.log exists" is the best way to choose the logfile
# but I have to admit that I don't have a better method ;-)
# @John: any better ideas?
+for p in sorted(apparmor.helpers.keys()):
# just curious - why sorted() ?
+ if apparmor.helpers[p] == 'enforce':
+ enforce(p)
+ reload(p)
+
+apparmor.UI_Info(_('\nReloaded AppArmor profiles in enforce mode.'))
+apparmor.UI_Info(_('\nPlease consider contributing your new profile!\nSee the following wiki page for more information:\nhttp://wiki.apparmor.net/index.php/Profiles\n'))
+apparmor.UI_Info(_('Finished generating profile for %s.')%program)
+sys.exit(0)
=== modified file 'Tools/aa-logprof.py'
--- Tools/aa-logprof.py 2013-08-11 09:52:07 +0000
+++ Tools/aa-logprof.py 2013-08-21 05:56:09 +0000
@@ -1,15 +1,30 @@
-sys.path.append('../')
# just an information for everybody reading this diff and trying the new tools:
# it's finally time to symlink apparmor/ into /usr/lib/python2.7/site-packages and /usr/lib64/python3.3/site-packages ;-)
+parser.add_argument('-f', type=str, help='path to logfile')
...
+filename = args.f
# nice, but it would be even better if you use that filename later ;-)
# (currently there's no code to handle it)
+aa_mountpoint = apparmor.check_for_apparmor()
+if not aa_mountpoint:
+ raise apparmor.AppArmorException(_('AppArmor seems to have not been started. Please enable AppArmor and try again.'))
# the first sentence looks a bit complicated. I'm not a native english speaker, but what about "It seems AppArmor was not started."?
+if profiledir:
+ apparmor.profiledir = apparmor.get_full_path(profiledir)
+ if not os.path.isdir(apparmor.profiledir):
+ raise apparmor.AppArmorException("Can't find AppArmor profiles in %s." %profiledir)
# This error message is wrong - it should just say "%s is not a directory"
...
+sys.exit(0)
# not sure if exitcode 0 is always correct ;-)
=== added file 'Tools/aa-unconfined.py'
--- Tools/aa-unconfined.py 1970-01-01 00:00:00 +0000
+++ Tools/aa-unconfined.py 2013-08-21 05:56:09 +0000
+aa_mountpoint = apparmor.check_for_apparmor()
+if not aa_mountpoint:
+ raise apparmor.AppArmorException(_('AppArmor seems to have not been started. Please enable AppArmor and try again.'))
# the first sentence looks a bit complicated. I'm not a native english speaker, but what about "It seems AppArmor was not started."?
# (should be the same as in aa-logprof)
+ cmdline = apparmor.cmd(['cat', '/proc/%s/cmdline'%pid])[1]
# looks like a useless use of cat - what about just reading from that file?
+ pname = cmdline.split('\0')[0]
+ if '/' in pname and pname != prog:
+ pname = '(%s)'%pname
+ else:
+ pname = ''
+ if not attr:
+ if re.search('^(/usr)?/bin/(python|perl|bash)', prog):
# this list looks incomplete - what about sh etc?
+ cmdline = re.sub('\0', ' ', cmdline)
+ cmdline = re.sub('\s+$', '', cmdline).strip()
+ sys.stdout.write(_('%s %s (%s) not confined\n')%(pid, prog, cmdline))
+ else:
+ if pname and pname[-1] == ')':
+ pname += ' '
+ sys.stdout.write(_('%s %s %snot confined\n')%(pid, prog, pname))
+ else:
+ if re.search('^(/usr)?/bin/(python|perl|bash)', prog):
# this list looks incomplete - what about sh etc?
=== modified file 'apparmor/aa.py'
--- apparmor/aa.py 2013-08-12 19:13:20 +0000
+++ apparmor/aa.py 2013-08-21 05:56:09 +0000
@@ -516,6 +513,16 @@
+def get_profile_flags(filename):
+ flags = 'enforce'
+ with open_file_read(filename) as f_in:
+ for line in f_in:
+ if RE_PROFILE_START.search(line):
+ flags = RE_PROFILE_START.search(line).groups()[6]
+ return flags
# to make things more interesting: in theory, a file can contain more than one profile...
+ return flags
# instead of returning 'enforce' as default, you should raise an exception if the file does not contain RE_PROFILE_START
@ -1@@ -1720,15 +1736,17 @@
if matchregexp(path, entry):
if mode_contains(mode, aa[profile][hat]['allow']['path'][entry]['mode']):
- aa[profile][hat]['allow']['path'].pop(entry)
- deleted += 1
+ deleted.append(entry)
+ for entry in deleted:
+ aa[profile][hat]['allow']['path'].pop(entry)
+ deleted = len(deleted)
# deleted contains a nice diff, but you are throwing it away in the last line
@@ -1951,26 +1969,28 @@
def delete_cap_duplicates(profilecaps, inccaps):
...
+ for capname in deleted:
+ profilecaps.pop(capname)
+
+ return len(deleted)
# deleted contains a nice diff, but you are throwing it away in the last line
def delete_path_duplicates(profile, incname, allow):
...
+ for entry in deleted:
+ profile[allow]['path'].pop(entry)
+ return len(deleted)
# deleted contains a nice diff, but you are throwing it away in the last line
@@ -2223,6 +2262,26 @@
+def display_changes_with_comments(oldprofile, newprofile):
+ """Compare the new profile with the existing profile inclusive of all the comments"""
+ if not os.path.exists(oldprofile):
+ raise AppArmorException("Can't find existing profile %s to compare changes." %oldprofile)
+ if UI_mode == 'yast':
+ #To-Do
+ pass
# in yast mode, you'll also need to create the diff - only displaying it is different
+ else:
+ newtemp = tempfile.NamedTemporaryFile('w')
+ newtemp.write(newprofile)
+ newtemp.flush()
+
+ difftemp = tempfile.NamedTemporaryFile('w')
+
+ subprocess.call('diff -u -p %s %s > %s' %(oldprofile, newtemp.name, difftemp.name), shell=True)
+
+ newtemp.close()
+ subprocess.call('less %s' %difftemp.name, shell=True)
# this "less" call is probably the only thing that will be different in yast mode
+ difftemp.close()
@@ -3157,6 +3215,535 @@
+def serialize_profile_from_old_profile(profile_data, name, options):
+ with open_file_read(prof_filename) as f_in:
+ profile = None
+ hat = None
+ write_methods = {'alias': write_alias,
+ 'lvar': write_list_vars,
+ 'include': write_includes,
+ 'rlimit': write_rlimits,
+ 'capability': write_capabilities,
+ 'netdomain': write_netdomain,
+ 'link': write_links,
+ 'path': write_paths,
+ 'change_profile': write_change_profile,
+ }
+ prof_correct = True
+ segments = {
+ 'alias': False,
+ 'lvar': False,
+ 'include': False,
+ 'rlimit': False,
+ 'capability': False,
+ 'netdomain': False,
+ 'link': False,
+ 'path': False,
+ 'change_profile': False,
+ 'include_local_started': False,
+ }
+ #data.append('reading prof')
+ for line in f_in:
+ correct = True
+ line = line.rstrip('\n')
+ #data.append(' ')#data.append('read: '+line)
+ if RE_PROFILE_START.search(line):
+ elif RE_PROFILE_END.search(line):
# just for the records: } can also be the end of a hat or end of a rule block
# you'll need to handle that
+ # DUMP REMAINDER OF PROFILE
+ if profile:
+ data += write_alias(write_prof_data[name], depth)
+ data += write_list_vars(write_prof_data[name], depth)
+ data += write_includes(write_prof_data[name], depth)
+ data += write_rlimits(write_prof_data, depth)
+ data += write_capabilities(write_prof_data[name], depth)
+ data += write_netdomain(write_prof_data[name], depth)
+ data += write_links(write_prof_data[name], depth)
+ data += write_paths(write_prof_data[name], depth)
+ data += write_change_profile(write_prof_data[name], depth)
# can't you just loop over write_methods?
...
+
+ 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 = matches[2]
# will this also work for a line just containing "capability,"?
+ 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 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())
+ data += write_methods[segs](write_prof_data[name], int(depth/2))
+ segments[segs] = False
+ if write_prof_data[name]['allow'].get(segs, False): write_prof_data[name]['allow'].pop(segs)
+ if write_prof_data[name]['deny'].get(segs, False): write_prof_data[name]['deny'].pop(segs)
+ segments['capability'] = True
+ write_prof_data[hat][allow]['capability'].pop(capability)
+ data.append(line)
+
+ #write_prof_data[hat][allow]['capability'][capability].pop(audit)
+
+ #Remove this line
;-)
+ else:
+ # To-Do
# a warning (maybe even an error?) would be nice (for all rule types, not just for capabilities)
+ elif RE_PROFILE_HAT_DEF.search(line):
+ matches = RE_PROFILE_HAT_DEF.search(line).groups()
+ in_contained_hat = True
+ hat = matches[0]
+ hat = strip_quotes(hat)
+ flags = matches[3]
+ if not write_prof_data[hat]['flags'] == flags:
+ correct = False
+ if not write_prof_data[hat]['declared'] == False:
+ correct = False
+ if not write_filelist['profile'][profile][hat]:
+ correct = False
+ if correct:
+ data.append(line)
+ else:
+ #To-Do
+ pass
+ else:
# This "else" basically means "unknown line"
# that's ok for comments and whitespace-only lines
# but everything else should raise an error
+ if correct:
+ data.append(line)
+ else:
+ #To-Do
+ pass
=== modified file 'apparmor/ui.py'
--- apparmor/ui.py 2013-08-12 19:13:20 +0000
+++ apparmor/ui.py 2013-08-21 05:56:09 +0000
@@ -261,7 +292,7 @@
def confirm_and_finish():
- sys.stdout.stdout('FINISHING\n')
+ sys.stdout.write('FINISHING..\n')
# typically there are 3 dots - "FINISHING...\n"
=== added file 'apparmor/writeprofile.py'
# that's an empty file...
vim:ft=diff
More information about the AppArmor
mailing list