Hello, Am Montag, 26. September 2016, 14:45:34 CEST schrieb Steve Beattie: > On Fri, Aug 12, 2016 at 11:03:09PM +0200, Christian Boltz wrote: > > when matching an AARE against another AARE, most AARE objects don't > > contain orig_regex (only AARE instances originating from a log event > > contain orig_regex). > > > > In this case, match() will use is_equal() to error out on the safe > > side. Unfortunately this also means that there are lots of false > > negative cases where match() returns False errornously. > > > > With this patch, match() checks the given AARE regex and, if it > > doesn't contain any special characters (wildcards, alternations or > > variables), handles it as plain path. This avoids most of the false > > negatives. > > > > Also extend the AARE tests to check a bunch of plain path regexes > > using AARE matching instead of only str matching. > > > > [ 28-aare-plain-path.diff ] > > Acked-by: Steve Beattie <[email protected]>, though I'm not crazy about > commingling the plain checks with the regex checks in the same > function, as I suspect it will make figuring out what's failing when > something goes wrong more difficult (in answering "What's being > tested and why?").
Please allow me to disagree ;-)
In most cases (except when reading from audit.log, where the input is
guaranteed not to be a regex), we check one regex against another regex.
Even if a profile contains a plain path like "/bin/foo", it's technically
a regex because profiles by definition contain "path regexes", not "plain
paths". ["regex" means AARE syntax here, as described in the apparmor.d
manpage. Don't confuse this with the AARE class.]
Unfortunately checking one regex against another is terribly difficult [1]
so this patch avoids this nightmare in cases we can easily avoid it.
By checking if a given regex is "boring" (as in: only contains
characters that do not have a special meaning, so plain path == regex),
we can cover lots of cases without too much complexity.
Yes, this is an "error out on the safe side" attemp [2] - but I still
consider this better than relying on code I don't really understand ;-)
If someone can explain all details mentioned in [1] and/or provides a
patch to improve the situation, this is more than welcome!
TL;DR: AARE still does only "plain" checks when matching against another
AARE - it just became more tolerant on what to accept as "plain" ;-)
Regards,
Christian Boltz
PS: non-random sig
[1] I'd _guess_ matchliteral() in aa.py does something like that, but I
have to admit that I don't fully understand this function and
convert_regexp() in common.py which it uses, and I'm especially not
sure if it does what I think it does. (Needless to say: no tests.)
Yes, there are some tests for convert_regexp(), but if you look at
the resulting regex, you might understand why I don't really
understand it ;-) (at least for the more complex cases)
TL;DR: Welcome to regex hell...
On the positive side, I have _the answer[tm]_ - and, what an irony,
this answer will be patch 42 ;-)
After looking at the code again, I found out that matchliteral() is
now unused :-) (see patch 42 for details, it was an interesting
hunt) so the best thing I can do is to drop it.
[2] worst thing that can happen: a profile ends up with an additional,
superfluous rule because the tools didn't recognize that it's covered
by another rule
--
LGTM? There is a special place in hell for regular expressions.
[ewindisch on https://github.com/docker/docker/pull/19069]
signature.asc
Description: This is a digitally signed message part.
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
