On Mon, Feb 24, 2014 at 12:16:49PM -0800, Steve Beattie wrote: > This patch splits out the disable functionality from the > apparmor/tools.py:act() method into a separate cmd_disable() > method. The intent is to unwind the logic in act() into smaller, more > digestible chunks, while sharing commonality via helper functions > (e.g. the added get_next_to_profile() function). > > A secondary driver of this change is that the tools fail when used > against the trunk profiles, due to act() forcing all the profiles to > be read and the tools not understanding the recently added dbus rules > (they were intentionally ignored as part of scoping the rewrite). > Unfortunately, this is not a solution for aa-enforce, aa-complain, etc. > as they are expected to know enough about profiles to understand and > update profile flags.
Yeah, these do need to work reliably regardless of the rest of the profile's contents. This looks like a good step. > I should note that one side effect is that this patch effectively > neuters the -r (revert) option for aa-disable. I don't really like that > option (I'd rather point people at using aa-enforce to undo aa-disable). > I can submit a patch that either removes the option or adds the > functionality if we desire it. When I was looking at this program a few days ago I thought the revert option looked out of place; removing revert felt like the right thing to do but I didn't take the time then to do it myself. > (The patch is a little larger than I'd hoped, to deal with the removal > of the 'p' variable due to lifting get_next_to_profile() into a separate > function.) Ah, sure, but the final program is better off for it. > Signed-off-by: Steve Beattie <[email protected]> Acked-by: Seth Arnold <[email protected]> Thanks! > --- > utils/aa-disable | 4 +-- > utils/apparmor/tools.py | 56 > +++++++++++++++++++++++++++++------------------- > 2 files changed, 36 insertions(+), 24 deletions(-) > > Index: b/utils/apparmor/tools.py > =================================================================== > --- a/utils/apparmor/tools.py > +++ b/utils/apparmor/tools.py > @@ -55,12 +55,12 @@ class aa_tools: > if not os.path.isdir(self.disabledir): > raise apparmor.AppArmorException("Can't find AppArmor disable > directory %s" % self.disabledir) > > - def act(self): > + def get_next_to_profile(self): > for p in self.profiling: > if not p: > continue > > - program = None > + program = p > if os.path.exists(p): > program = apparmor.get_full_path(p).strip() > else: > @@ -68,16 +68,18 @@ class aa_tools: > if which: > program = apparmor.get_full_path(which) > > + yield program > + > + def act(self): > + for program in self.get_next_to_profile(): > + > apparmor.read_profiles() > - #If program does not exists on the system but its profile does > - if not program and apparmor.profile_exists(p): > - program = p > > if not program or not(os.path.exists(program) or > apparmor.profile_exists(program)): > if program and not program.startswith('/'): > program = aaui.UI_GetString(_('The given program cannot > be found, please try with the fully qualified path name of the program: '), > '') > else: > - aaui.UI_Info(_("%s does not exist, please double-check > the path.") % p) > + aaui.UI_Info(_("%s does not exist, please double-check > the path.") % program) > sys.exit(1) > > if self.name == 'autodep' and program and > os.path.exists(program): > @@ -85,21 +87,13 @@ class aa_tools: > > elif program and apparmor.profile_exists(program): > if self.name == 'cleanprof': > - self.clean_profile(program, p) > + self.clean_profile(program) > > else: > filename = apparmor.get_profile_filename(program) > > if not os.path.isfile(filename) or > apparmor.is_skippable_file(filename): > - aaui.UI_Info(_('Profile for %s not found, skipping') > % p) > - > - elif self.name == 'disable': > - if not self.revert: > - aaui.UI_Info(_('Disabling %s.') % program) > - self.disable_profile(filename) > - else: > - aaui.UI_Info(_('Enabling %s.') % program) > - self.enable_profile(filename) > + aaui.UI_Info(_('Profile for %s not found, skipping') > % program) > > elif self.name == 'audit': > if not self.remove: > @@ -124,13 +118,32 @@ class aa_tools: > raise apparmor.AppArmorException(cmd_info[1]) > > else: > - if '/' not in p: > - aaui.UI_Info(_("Can't find %s in the system path list. > If the name of the application\nis correct, please run 'which %s' as a user > with correct PATH\nenvironment set up in order to find the fully-qualified > path and\nuse the full path as parameter.") % (p, p)) > + if '/' not in program: > + aaui.UI_Info(_("Can't find %s in the system path list. > If the name of the application\nis correct, please run 'which %s' as a user > with correct PATH\nenvironment set up in order to find the fully-qualified > path and\nuse the full path as parameter.") % (program, program)) > else: > - aaui.UI_Info(_("%s does not exist, please double-check > the path.") % p) > + aaui.UI_Info(_("%s does not exist, please double-check > the path.") % program) > sys.exit(1) > > - def clean_profile(self, program, p): > + def cmd_disable(self): > + for program in self.get_next_to_profile(): > + filename = apparmor.get_profile_filename(program) > + print('profile %s: filename is %s' % (program, filename)) > + > + if not os.path.isfile(filename) or > apparmor.is_skippable_file(filename): > + aaui.UI_Info(_('Profile for %s not found, skipping') % > program) > + continue > + > + aaui.UI_Info(_('Disabling %s.') % program) > + self.disable_profile(filename) > + > + # FIXME: this should be a profile_remove function/method > + # FIXME: should ensure profile is loaded before unloading > + cmd_info = cmd([apparmor.parser, '-I%s' % apparmor.profile_dir, > '-R', filename]) > + > + if cmd_info[0] != 0: > + raise apparmor.AppArmorException(cmd_info[1]) > + > + def clean_profile(self, program): > filename = apparmor.get_profile_filename(program) > import apparmor.cleanprofile as cleanprofile > prof = cleanprofile.Prof(filename) > @@ -149,7 +162,6 @@ class aa_tools: > q['default'] = 'CMD_VIEW_CHANGES' > q['options'] = [] > q['selected'] = 0 > - p = None > ans = '' > arg = None > while ans != 'CMD_SAVE_CHANGES': > @@ -165,7 +177,7 @@ class aa_tools: > apparmor.write_profile_ui_feedback(program) > apparmor.reload_base(program) > else: > - raise apparmor.AppArmorException(_('The profile for %s does not > exists. Nothing to clean.') % p) > + raise apparmor.AppArmorException(_('The profile for %s does not > exists. Nothing to clean.') % program) > > def use_autodep(self, program): > apparmor.check_qualifiers(program) > Index: b/utils/aa-disable > =================================================================== > --- a/utils/aa-disable > +++ b/utils/aa-disable > @@ -26,7 +26,7 @@ parser.add_argument('-r', '--revert', ac > parser.add_argument('program', type=str, nargs='+', help=_('name of > program')) > args = parser.parse_args() > > -disable = apparmor.tools.aa_tools('disable', args) > +tool = apparmor.tools.aa_tools('disable', args) > > -disable.act() > +tool.cmd_disable() > >
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
