[apparmor] [PATCH] Set flags for profiles represented by a glob

[Goldwyn Rodrigues]

On 04/08/2018 01:09 PM, Christian Boltz wrote:
> Hello,
> 
> Am Freitag, 23. März 2018, 02:28:12 CEST schrieb Goldwyn Rodrigues:
>> Getting and Setting profile represented by a glob does not work
>> correctly because they are checked for equality. Use a glob match to
>> check for them. Also, add a warning stating that the profile being
>> set represents multiple programs.
>>
>> traceroute is an example whose profile name is represented as
>> /usr/{sbin/traceroute,bin/traceroute.db} and exhibits the issue:
>>
>> # aa-enforce /usr/sbin/traceroute
>> Setting /usr/sbin/traceroute to enforce mode.
>>
>> ERROR: /etc/apparmor.d/usr.sbin.traceroute contains no profile
>>
>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn at suse.com>
>>
>> diff --git a/utils/apparmor/aa.py b/utils/apparmor/aa.py
>> index 1e7f4bba..262c96f1 100644
>> --- a/utils/apparmor/aa.py
>> +++ b/utils/apparmor/aa.py
>> @@ -613,8 +613,9 @@ def get_profile_flags(filename, program):
>>              if RE_PROFILE_START.search(line):
>>                  matches = parse_profile_start_line(line, filename)
>>                  profile = matches['profile']
>> +                profile_glob = AARE(profile, True)
>>                  flags = matches['flags']
>> -                if profile == program or program is None:
>> +                if (program is not None and
>> profile_glob.match(program)) or program is None: return flags
>>
>>      raise AppArmorException(_('%s contains no profile') % filename)
>> @@ -667,8 +668,11 @@ def set_profile_flags(prof_filename, program,
>> newflags): space = matches['leadingspace'] or ''
>>                      profile = matches['profile']
>>
>> -                    if profile == program or program is None:
>> +                    profile_glob = AARE(profile, True)
>> +                    if (program is not None and
>> profile_glob.match(program)) or program is None: found = True
>> +                        if program is not None and program !=
>> profile: 
>> +                          aaui.UI_Info('Warning: profile %s
>> represents multiple programs' % profile) 
> 
> I finally had some time to test your patch. A minor issue is that the 
> warning should be translateable, so please wrap it with   _('...')
> (and, while on it, add a dot at the end of the message)
> 
>     aaui.UI_Info(_('Warning: profile %s represents multiple programs.') % profile) 
> 
> 
> That said - you picked a good example profile ;-)
> aa-complain /usr/sbin/traceroute   works :-)
> 
> Unfortunately it still doesn't work with all profiles - for example,
> aa-complain fails for the ping profile
>     profile ping /{usr/,}bin/ping {
> and also if I change it to
>     /{usr/,}bin/ping {
> 
> The failure for both is the old one:
>     Profile for /usr/bin/ping not found, skipping
> 
> I verified that AARE matching works as expected, so there must be a bug
> somewhere else.
> 


This is because of profile to filename conversion. We are finding the
program associated with it and converting the filename replacing with
dots. So, the profile (ping) which evaluates (/usr/bin/ping) does not
find the profile named /etc/apparmor.d/usr.bin.ping.  There is no easy
solution to this as of now.

I so wish aa.py was written a bit better.

> 
> Bonus bug: if I change the traceroute profile to
>     profile traceroute /usr/{sbin/traceroute,bin/traceroute.db} {
> 
>     # aa-enforce traceroute
>     Setting /usr/sbin/traceroute to enforce mode.
> 
>     ERROR: Path doesn't start with / or variable: traceroute
> 
> which indicates that the match is done against the profile name instead
> of the attachment.

Okay, I fixed that. It needs to check against the right name.

> 
> (You can also trigger similar errors by simply running "make check".)
> 
> 
> BTW: We moved development to gitlab.com, merge requests are always 
> welcome ;-)  - but if you prefer to send patches by mail, that's of 
> course still possible.

Okay I will send the merge request there.

-- 
-- 
Goldwyn