Re: [apparmor] [PATCH] Set flags for profiles represented by a glob
Hello,
Am Mittwoch, 11. April 2018, 18:32:20 CEST schrieb Goldwyn Rodrigues:
> On 04/08/2018 01:09 PM, Christian Boltz wrote:
> > 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.
Ah, now I understand the problem, and just confirmed that renaming
bin.ping to usr.bin.ping indeed fixes the issue.
aa-logprof can handle "wrong" filenames - basically it reads all
profiles first, and while doing that, also builds a list of profile <->
filename mappings which it then uses instead of the "default" filenames.
Doing the same for aa-complain etc. should be possible, but I'll have to
check the code before I know how hard it is or if it causes side
effects. My *guess* is that it isn't really hard, but it needs parsing
of all profiles and therefore might make aa-complain a bit slower. (That
said - aa-complain already seems to parse all profiles because it errors
out if one of them has a syntax error. Sounds like this will be an
interesting bug hunt ;-)
I'd say ignore this for now and submit your patch. We can always submit
another patch on top to fix this detail ;-)
> I so wish aa.py was written a bit better.
No objections on this. I celebrate every line of code I can remove from
it, and my long-term goal is rm aa.py ;-)
> > 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.
Maybe it's a good idea to check against both ;-) Assume someone runs
aa-complain ping and we have profile ping /{usr/,}bin/ping then
"ping" would be an exact match on the profile name and /bin/ping or
/usr/bin/ping would match the attachment.
(This doesn't need to be in this patch - not making the given parameter
a full path to be able to check against the profile name sounds like a
bigger change.)
> > (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.
:-)
Regards,
Christian Boltz
--
Unix: Alles ist ein File, und was kein File ist, hat sich gefaelligst
als ein solches zu tarnen. [Wolfgang Weisselberg in linux-liste]
signature.asc
Description: This is a digitally signed message part.
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] Set flags for profiles represented by a glob
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
>>
>> 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
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] Set flags for profiles represented by a glob
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
>
> 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.
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.
(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.
Regards,
Christian Boltz
--
> Manfred, Du solltest so spaet keine Emails mehr schreiben :-)
Danke für die Berichtigung, werd mir den Tipp hinter die Ohren schreiben
und nur noch Mailen, wenn ich die Augen zumindestens zu einem drittel
aufkriege. [> Thomas Hertweck und Manfred Tremmel in suse-linux]
signature.asc
Description: This is a digitally signed message part.
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
