Hello, the review for r93..95 of the new profile tools is attached.
Your commits fixed several bugs, but there are also cases where you replaced one bug with another ;-) I also answered your question about trace handling in the aa-* tools, even if my answer where to handle it won't be too surprising ;-) Regards, Christian Boltz -- Well, I guess, Stephan knows very well, what the fuzz is about: it's about hundreds of patches, which will have to be regenerated, done as an employment-creation measure for this lazy gang of packagers. [Hans-Peter Jansen in opensuse-packaging]
------------------------------------------------------------ revno: 93 committer: Kshitij Gupta <kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Sat 2014-02-01 06:14:05 +0530 message: Some bugfixes for UIYesNo to deny invalid keys, fix autodep when creating new profiles === modified file 'Tools/aa-audit' --- Tools/aa-audit 2013-10-21 21:36:23 +0000 +++ Tools/aa-audit 2014-02-01 00:44:05 +0000 #[merged review for aa-audit r93..95] # # r95 message: # Fixed the sample --trace feature. Opinions on using it? and should it be implemented in every tool separately? # not shocking users with a backtrace is a good idea. # maybe you should only "hide" AppArmor exceptions (they could be called "expected", and the error message in them is usually enough) # and still display other ("unexpected") exceptions (because we want to know where something fails unexpectedly) # This should of course ;-) be handled in a central place (tools.py?) instead of adding it to every aa-* === modified file 'apparmor/aa.py' --- apparmor/aa.py 2013-12-19 21:42:58 +0000 +++ apparmor/aa.py 2014-02-01 00:44:05 +0000 @ -418,7 +418,8 @@ local_profile[hat]['flags'] = 'complain' created.append(localfile) - + changed.append(localfile) # looks good, but... # python Tools/aa-genprof ~cb/linuxtag/apparmor/scripts/hello # Traceback (most recent call last): # File "Tools/aa-genprof", line 102, in <module> # apparmor.autodep(program) # File "/usr/lib/python2.7/site-packages/apparmor/aa.py", line 569, in autodep # profile_data = create_new_profile(pname) # File "/usr/lib/python2.7/site-packages/apparmor/aa.py", line 421, in create_new_profile # changed.append(localfile) # AttributeError: 'dict' object has no attribute 'append' # hint (aa.py around line 90) # changed = dict() # created = [] @@ -865,34 +869,42 @@ def build_x_functions(default, options, exec_toggle): ret_list = [] + fallback_toggle = False if exec_toggle: if 'i' in options: ret_list.append('CMD_ix') if 'p' in options: ret_list.append('CMD_pix') - ret_list.append('CMD_EXEC_IX_OFF') + fallback_toggle = True - elif 'c' in options: + if 'c' in options: ret_list.append('CMD_cix') - ret_list.append('CMD_EXEC_IX_OFF') + fallback_toggle = True - elif 'n' in options: + if 'n' in options: ret_list.append('CMD_nix') + fallback_toggle = True + if fallback_toggle: ret_list.append('CMD_EXEC_IX_OFF') # placing CMD_EXEC_IX_OFF here has a small disadvantage - it will be offered left of CMD_ux (but should be right of it) - elif 'u' in options: + if 'u' in options: ret_list.append('CMD_ux') === modified file 'apparmor/common.py' --- apparmor/common.py 2013-12-29 09:42:30 +0000 +++ apparmor/common.py 2014-02-01 00:44:05 +0000 @@ -201,9 +201,16 @@ +def user_perm(prof_dir): # the function name is not too helpful # what about "is_writeable()"? # (or "must_be_writeable()" and let it raise an exception if prof_dir is not writeable, see below) + if not os.access(prof_dir, os.R_OK): # [should check for W_OK, already fixed in r94] + sys.stdout.write("Cannot write to profile directory.\n" + + "Please run as a user with appropriate permissions." ) # please make the error message translatable # I'd also mention prof_dir in the message. # To make the function useable for everything (not just profile dir), change the message to "Cannot write to %s" class DebugLogger(object): def __init__(self, module_name=__name__): ... + try: + logging.basicConfig(filename=self.logfile, level=self.debug_level, + format='%(asctime)s - %(name)s - %(message)s\n') + except OSError: # looks good, but fails with py2 # -> use "except IOError:" instead, that works with py2 and py3 + # Unable to open the default logfile, so create a temporary logfile and tell use about it # ... tell use_r_ about it + import tempfile + templog = tempfile.NamedTemporaryFile('w', prefix='apparmor', suffix='.log' ,delete=False) + sys.stdout.write("\nCould not open: %s\nLogging to: %s\n"%(self.logfile, templog.name)) # printing another \n would be nice === modified file 'apparmor/tools.py' --- apparmor/tools.py 2013-12-29 09:42:30 +0000 +++ apparmor/tools.py 2014-02-01 00:44:05 +0000 @@ -41,6 +43,9 @@ if not os.path.isdir(apparmor.profile_dir): raise apparmor.AppArmorException("%s is not a directory." %self.profiledir) + if not user_perm(apparmor.profile_dir): + raise apparmor.AppArmorException("Cannot write to profile directory: %s." %(apparmor.profile_dir)) # user_perm() already prints an error message, so you'll have two at the end... # you should decide where to print the error message, and maybe raise the exception at the same place that prints the error message # if you decide that user_perm() should only check the permission without printing any error message, # this might be one of the rare cases where a separate function doesn't make sense ;-) vim:ft=diff
-- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor