=== modified file 'apparmor/aa.py' --- apparmor/aa.py 2013-07-27 10:02:12 +0000 +++ apparmor/aa.py 2013-07-28 02:59:59 +0000 @@ -449,7 +451,7 @@ if interpreter == 'perl': local_profile[localfile]['include']['abstractions/perl'] = True - elif interpreter == 'python': + elif re.search('python([23]|[23]\.[0-9])?$', interpreter): ### ^python would be better - even if I think nobody uses monty_python as interpreter ;-) ### maybe also use [0-9]+ to also match 3.10 @ -612,8 +614,8 @@ """Reads the old profile file and updates the flags accordingly""" regex_bin_flag = re.compile('^(\s*)(("??\/.+?"??)|(profile\s+("??.+?"??)))\s+(flags=\(.+\)\s+)*\{\s*$/') ### 2013-07-23 #apparmor ### [21:47:19] in that line I was trying to incorporate the regex for comments too. I'll remove the over-write. ### (done in r28) ### [21:48:36] 611 will probably fail with inline comments ### [21:48:53] you should at least copy over the comment part from 613 ;-) ### [21:49:47] will fix that. ### [21:50:24] while you are on it - the regex also has (flags=...)* ### [21:50:43] it should be (flags...)? because flags can only appear once ### [21:51:14] (with multiple flags specified in (...) ) ### [21:51:54] oh, and flags=() is also allowed IIRC, so .+ could be wrong ;-) @@ -1945,10 +1960,10 @@ if cfg['settings']['custom_includes']: for incn in cfg['settings']['custom_includes'].split(): - if incn in incname: + if incn == incname: include_valid = True - if 'abstraction' in incname: + if incname.startswith('abstractions/'): include_valid = True ### better, but I'd still recommend to test if the file exists ;-) @ -2148,20 +2167,28 @@ elif ans == 'CMD_GLOB': newpath = options[selected].strip() if not re_match_include(newpath): if newpath[-1] == '/': if newpath[-4:] == '/**/' or newpath[-3:] == '/*/': # /foo/**/ and /foo/*/ => /**/ newpath = re.sub('/[^/]+/\*{1,2}/$', '/**/', newpath) #re.sub('/[^/]+/\*{1,2}$/', '/\*\*/', newpath) + # /foo**/ => /**/ ### this comment should be placed below the elif, not above + elif re.search('/[^/]+\*\*/$', newpath): + newpath = re.sub('/[^/]+\*\*/$', '/**/', newpath) ### two other corner cases: ### /foo**bar/ => /**/ ### /**bar/ => /**/ ### (be careful - using [^/]*\*\*[^/]*/$ in the regex would also match /**/) else: - newpath = re.sub('/[^/]+/$', '/\*/', newpath) + newpath = re.sub('/[^/]+/$', '/*/', newpath) else: + # /foo/** and /foo/* => /** ### this comment should be placed below the if if newpath[-3:] == '/**' or newpath[-2:] == '/*': newpath = re.sub('/[^/]+/\*{1,2}$', '/**', newpath) + # /**foo => /** ### this comment should be placed below the elif elif re.search('/\*\*[^/]+$', newpath): newpath = re.sub('/\*\*[^/]+$', '/**', newpath) + # /foo** => /** ### this comment should be placed below the elif + elif re.search('/[^/]+\*\*$', newpath): + newpath = re.sub('/[^/]+\*\*$', '/**', newpath) ### corner case: /foo**bar => /** is not covered @@ -2169,13 +2196,20 @@ elif ans == 'CMD_GLOBEXT': newpath = options[selected].strip() if not re_match_include(newpath): + # match /**.ext and /*.ext - match = re.search('/\*{1,2}(\.[^/]+)$', newpath) + match = re.search('/\*{1,2}(\.[^/]+)$', newpath) ### superfluous trailing whitespace if match: + # /foo/**.ext and /foo/*.ext => /**.ext - newpath = re.sub('/[^/]+/\*{1,2}\.[^/]+$', '/\*\*'+match.group()[0], newpath) + newpath = re.sub('/[^/]+/\*{1,2}\.[^/]+$', '/**'+match.group()[0], newpath) + # /foo**.ext => /**.ext ### this comment should be placed below the elif + elif re.search('/[^/]+\*\*\.[^/]+$'): + match = re.search('/[^/]+\*\*(\.[^/]+)$') + newpath = re.sub('/[^/]+\*\*\.[^/]+$', '/**'+match.groups()[0], newpath) ### corner cases not covered: ### /foo**bar.ext ### /**bar.ext @@ -2264,3 +2302,678 @@ +def match_net_include(incname, family, type): + includelist = incname[:] + checked = [] + name = None + if includelist: + name = includelist.pop(0) + while name: + checked.append(name) + if netrules_access_check(include[name][name]['allow']['netdomain'], family, type): + return True + + if include[name][name]['include'].keys() and name not in checked: + includelist += include[name][name]['include'].keys() + + if len(includelist): + name = includelist.pop(0) + else: + name = False ### missing return statement? +def match_cap_includes(profile, cap): + newincludes = [] + includevalid = False + for incname in include.keys(): + includevalid = False + if profile['include'].get(incname, False): + continue + + if cfg['settings']['custom_includes']: + for incm in cfg['settings']['custom_includes'].split(): + if incm in incname: + includevalid = True + + if 'abstractions' in incname: ### better use incname.startswith('abstractions/') ### besides that, this is the 3rd place checking for valid abstraction paths -> worth another function "validate_include(incname) ;-) ### (my other comments about validating abstraction paths also apply here) + includevalid = True + + if not includevalid: + continue + if include[incname][incname]['allow']['capability'][cap].get('set', False) == 1: + newincludes.append(incname) + + return newincludes + +def re_match_include(path): + """Matches the path for include and returns the include path""" + regex_include = re.compile('^\s*#?include\s*<(\.*)>') ### looks good, but doesn't detect #include lines with trailing garbage, for example ### #include garbage ### proposal: add (\s*#.*)?$ to the regex to allow comments and make the regex $-anchored +def match_net_includes(profile, family, nettype): + newincludes = [] + includevalid = False + for incname in include.keys(): + includevalid = False + + if profile['include'].get(incname, False): + continue + + if cfg['settings']['custom_includes']: + for incm in cfg['settings']['custom_includes'].split(): + if incm == incname: + includevalid = True + + if incname.startswith('abstractions/'): + includevalid = True ### another function that could call validate_include ;-) ### this would shorten this function to a validate_include call and the following 3 lines: + if includevalid and match_net_include(incname, family, type): + newincludes.append(incname) + + return newincludes +def save_profiles(): + while ans != 'CMD_SAVE_CHANGES': + ans, arg = UI_PromptUser(q) + if ans == 'CMD_VIEW_CHANGES': + which = changed[arg] + oldprofile = serialize_profile(original_aa[which], which) + newprofile = serialize_profile(aa[which], which) + + display_changes(oldprofile, newprofile) + + for profile_name in changed_list: + writeprofile_ui_feedback(profile_name) + reload_base(profile_name) ### just an idea - does it make sense to allow saving only some of the changed profiles? +def generate_diff(oldprofile, newprofile): + oldtemp = tempfile.NamedTemporaryFile('wr', delete=False) + + oldtemp.write(oldprofile) + oldtemp.flush() + + newtemp = tempfile.NamedTemporaryFile('wr', delete=False) + newtemp.write(newprofile) + newtemp.flush() + + difftemp = tempfile.NamedTemporaryFile('wr', deleted=False) ### delete_d_ ? + subprocess.call('diff -u %s %s > %s' %(oldtemp.name, newtemp.name, difftemp.name), shell=True) ### diff -u -p please (to include the profile or hat name in the @@ lines) + oldtemp.delete = True + oldtemp.close() + newtemp.delete = True + newtemp.close() ### If I understand the tempfile documentation right, it should also work if you do not use "delete=False" when creating the tempfile ### It will then be deleted at close() ### (you'll need to test if this also works for difftemp, or if the file is auto-closed when the function ends) + return difftemp +def get_profile_diff(oldprofile, newprofile): + difftemp = generate_diff(oldprofile, newprofile) + diff = [] + with open_file_read(difftemp.name) as f_in: + for line in f_in: + if not (line.startswith('---') and line .startswith('+++') and re.search('^\@\@.*\@\@$', line)): ### the @@ regex will fail if you switch to diff -u -p - remove the $ or just use line.startswith('@@') +def set_process(pid, profile): + # If process running don't do anything + if os.path.exists('/proc/%s/attr/current' % pid): + return None ### so this returns None if the process is running + process = None + try: + process = open_file_read('/proc/%s/attr/current') ### missing % pid ? + except IOError: + return None ### and this returns None if the process is not running (aka /proc/%s/attr/current does not exist) ### in other words: we are out of this function in any case, and the code below is probably unused. ### I doubt this is intentional ;-) ### (from the remaining code in this function, I'd guess the goal is "exit function if the process is NOT running") + current = process.readline().strip() + process.close() + + if not re.search('null(-complain)*-profile', current): ### the regex should have ^...$ + return None ### so the function only survives (if it's still alive, see above) if the process is in the null-complain-profile === modified file 'apparmor/ui.py' --- apparmor/ui.py 2013-07-17 23:59:54 +0000 +++ apparmor/ui.py 2013-07-28 02:53:46 +0000 @@ -225,7 +225,8 @@ 'CMD_NET_FAMILY': 'Allow Network Fa(m)ily', 'CMD_OVERWRITE': '(O)verwrite Profile', 'CMD_KEEP': '(K)eep Profile', - 'CMD_CONTINUE': '(C)ontinue' + 'CMD_CONTINUE': '(C)ontinue', + 'CMD_IGNORE_ENTRY': '(I)gnore Entry' ### I'd just call it '(I)gnore'