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
