Hi, On Fri, Feb 27, 2015 at 03:21:43PM +0100, Christian Boltz wrote: > to keep the number of pending patches stable: > > This patch cleans up the aa-disable handling in tools.py. > > Initially, I wanted to move the aa-disable specific code from __init__() > and the two lines in check_disable_dir() to cmd_disable(). > > Then I followed the code a bit and found out that create_symlink() in > aa.py (called in disable_profile()) creates the directory if it doesn't > exist. > > Long story short: The patch removes the check if the disable directory > exists. If it's really missing, it will be auto-created by > create_symlink(), so we automagically fix things instead of annoying the > user with an error message ;-) > > > I propose this for trunk and 2.9, but trunk only would also be ok. > > > [ tools-cleanup-disable.diff ]
Acked-by: Steve Beattie <st...@nxnw.org> for both trunk and 2.9.
In order to help you in your mission to keep a stable number of
patches pending review, I noticed a couple of quirks in behavior while
testing your patch:
1) If the profile is not currently loaded in the kernel when
aa-disable is run, aa-disable throws a python traceback due to
the parser returning the error code that it was unsuccessful
in unloading the profile. This doesn't seem to me to warrant
aa-disable crashing, perhaps we can handle this situation
better here?
(Though, aa-disable may not be able to distinguish from the
results returned by the parser "removing the profile failed due
to the profile not existing in the kernel's set of loaded policy"
and "removing the profile failed due to some other kernel error,
incompatibility between the parser and kernel, etc." Moving the
policy load to libapparmor and getting the utils to make use
of it could potentially get us a richer set of failure state
information.)
2) As a follow up to the above, we don't appear to document the
return codes of the parser in the apparmor_parser(8) man page. :(
3) This is more of a discussion point than anything, but my test
environment as passed to the utils via --dir had an outdated
tunables/ dir and didn't have an abstractions/ directory. This
caused the parser to abort when trying to operate on my test
profile not because it couldn't find the abstractions included
within it, but because the system-wide abstractions that it fell
back to referenced a policy variable that my test tree's tunables/
did not define. This was not what I was expecting, but I'm not
sure that we've defined what we mean by -d/--dir with the utils;
my expectation was that it would be the equivalent to setting
the --base option in the parser, but maybe that was misguided.
My bigger question is: are the utils when passed --dir in sync
with parser in terms of where relative includes will be pulled
from?
--
Steve Beattie
<sbeat...@ubuntu.com>
http://NxNW.org/~steve/
signature.asc
Description: Digital signature
-- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
