[apparmor] [patch] Let aa-audit print a warning if a profile is disabled

[Christian Boltz]

Hello,

Am Sonntag, 31. Mai 2015 schrieb Kshitij Gupta:
> On Mon, May 25, 2015 at 9:36 PM, Christian Boltz wrote:
> > this patch lets aa-audit print a warning if a profile is disabled.
> > 
> > Users might expect that setting a profile into audit mode also
> > activates it (which shouldn't happen IMHO because the audit flag is
> > not part of the enforce/complain/disable triple), so we should at
> > least tell them.
> > 
> > References: https://bugs.launchpad.net/apparmor/+bug/1429448
> > 
> > 
> > I propose this patch for trunk and 2.9.
> > 
> > 
> > [ 37-aa-audit-warn-about-disabled-profiles.diff ]
> > 
> > === modified file utils/apparmor/tools.py
> > --- utils/apparmor/tools.py     2015-05-25 17:29:05.067517743 +0200
> > +++ utils/apparmor/tools.py     2015-05-25 17:59:05.837870272 +0200
> > @@ -186,6 +186,11 @@
> > 
> >                  aaui.UI_Info(_('Removing audit mode from %s.') %
> > 
> > output_name)
> > 
> >              apparmor.change_profile_flags(profile, program,
> >              'audit', not
> > 
> > self.remove)
> > 
> > +            disable_link = '%s/disable/%s' % (apparmor.profile_dir,
> > os.path.basename(profile))
> > +
> 
> The aa.py code uses: re.sub('^%s' % profile_dir, '%s/%s' %
> (profile_dir, subdir), path)
> to generate disable_link.
> 
> I'd suggest we should change that code (which I think is probably a
> waste of an re.sub call) to follow the above style? or you could do
> vice versa.
> 
> For fun a comparison of speed of the two methods[1]:
> Python 2.7: re=2.87s cboltz=0.56s
> Python 3.4: re=2.39s cboltz=1.09s

That's a good argument to use the non-re code ;-)  (even if the 
difference is not really big when running it once)

In the end I'd even say we should split out a symlink_name() function to 
avoid we have 3 places that calculate the symlink path.

(Added to the TODO list, but I won't complain if someone else grabs this 
and sends a patch ;-)

BTW: I'm surprised that the non-re code takes twice the time with py3 
than with py2 - any idea what could cause this?

> +            if os.path.exists(disable_link):
> > +                aaui.UI_Info(_('\nWarning: the profile %s is
> > disabled. Use aa-enforce or aa-complain to enable it.') %
> > os.path.basename(profile)) +
> 
> Also, the message should probably tell the profile for which _program_
> is disabled rather than give the filename? Giving program name would
> also be useful for a user who would like to run aa-complain or
> aa-enforce thereafter.

Well, printing the program might be difficult ;-) - for example, a file 
could contain multiple profiles or a named profile with or without an 
attachment specified.

Besides that, you can easily call aa-audit with a profile file as 
parameter:
    aa-audit /etc/apparmor.d/bin.ping

> With the warning message modified suitably.
> 
> Acked-by: Kshitij Gupta <kgupta8592 at gmail.com>.

Does the Ack also stand for the unmodified patch with the explanation 
above?


Regards,

Christian Boltz
-- 
In its default setup, Windows XP on the Internet amounts to a car parked
in a bad part of town, with the doors unlocked, the key in the ignition
and a Post-It note on the dashboard saying, "Please don't steal this".
[Washington Post, 23.8.2003]