[apparmor] [patch] utils: split out disable functionality in apparmor/tools.py
Seth Arnold
seth.arnold at canonical.com
Fri Feb 28 03:44:15 UTC 2014
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 <steve at nxnw.org>
Acked-by: Seth Arnold <seth.arnold at canonical.com>
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()
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20140227/506f0819/attachment.pgp>
More information about the AppArmor
mailing list