parser: determine xmatch priority based on smallest DFA match
The length of a xmatch is used to prioritize multiple profiles that
match the same path, with the intent that the more specific match wins.
Currently, the length of a xmatch is computed by the position of the
first regex character.
While trying to work around issues with no_new_privs by combining
profiles, we noticed that the xmatch length computation doesn't work as
expected for multiple regexs. Consider the following two profiles:
profile all /** { }
profile bins /{,usr/,usr/local/}bin/** { }
xmatch_len is currently computed as "1" for both profiles, even though
"bins" is clearly more specific.
When determining the length of a regex, compute the smallest possible
match and use that for xmatch priority instead of the position of the
first regex character.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/326
Signed-off-by: John Johansen <john.johansen@canonical.com>
Update the indetation of work_spawn to correct for the changes made in
cb43e57d2796 ("parser: Fix parser failing to handle errors when setting up work")
the indetation was not updated in that patch to make the changes made
easier to review and see in diffs.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The parser is not correctly handling some error conditions when
dealing with work units. Failure to spawn work, access files, etc
should be returned where appropriate, and be able to abort processing
if abort_on_error is set.
In addition some errors are leading to a direct exit without checking
for abort_on_error.
BugLink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=921866
BugLink: http://bugs.launchpad.net/bugs/1815294
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Eric Chiang <ericchiang@google.com>
The length of a xmatch is used to prioritize multiple profiles that
match the same path, with the intent that the more specific match wins.
Currently, the length of a xmatch is computed by the position of the
first regex character.
While trying to work around issues with no_new_privs by combining
profiles, we noticed that the xmatch length computation doesn't work as
expected for multiple regexs. Consider the following two profiles:
profile all /** { }
profile bins /{,usr/,usr/local/}bin/** { }
xmatch_len is currently computed as "1" for both profiles, even though
"bins" is clearly more specific.
When determining the length of a regex, compute the smallest possible
match and use that for xmatch priority instead of the position of the
first regex character.
This change updates parser/Makefile to respect target dependencies and
not rebuild apparmor_parser if nothing's changed. The goal is to allow
cross-compiled tests #17 to run on a target system without the tests
attempting to rebuild the parser.
Two changes were made:
* Generate af_names.h in a script so the script timestamp is compared.
* Use FORCE instead of PHONY for libapparmor_re/libapparmor_re.a
Changes to list_af_names are intended to exactly replicate the old
behavior.
Signed-off-by: Eric Chiang <ericchiang@google.com>
Commit 0d5ab43d592245d011b2614e6e20fc7cb851c53c removed support for loading
modules and introduced a caller, in apparmor_start(), that passes no argument to
is_apparmor_present(), which breaks that function when /bin/sh → /bin/dash.
Passing a module name as argument does not make sense since we dropped support
for the long obsolete "subdomain" module, so let's simplify
is_apparmor_present() and adjust its callers accordingly.
Bug-Debian: https://bugs.debian.org/917874
Since 04eb2fe3, __parse_profiles_dir can only return 0 or 1, so $STATUS can only
be 0 or 1, so trying to reset this variable to 0 when its value is 2 can only
cause confusion.
Expr tree simplification makes multiple passes at simplifying the
expression tree trying to use fatoring rules and heuristics to achieve
the minimum tree, so that dfa construction has fewer nodes to deal
with.
Unfortunately expr tree simplification can slow some policy compiles,
dependent on the type of expressions generated, down, and even worse
is currently subject to never terminating on some expressions as the
left and right passes keep undoing each others work.
Limiting the number of passes that expr tree simplification does can
provide most of its benefits (later passes generally have diminishing
returns), reduces the overhead it has on simple policy where it is of
little benefit, and insures that simplifications can not get stuck in
an infinite loop due to the left and right passes ping-ponging on each
others factoring.
Note: This also results in a performance improvement in evince
compiles, and general policy compiles because it achieves a better
balance between time spent on simplifying the tree to remove nodes and
time the dfa build requires to build with extra nodes and then
eliminate with minimization.
$ time apparmor_parser -QT /etc/apparmor.d/usr.bin.evince
real 0m2.744s
user 0m2.714s
sys 0m0.028s
vs.
$ time apparmor_parser -QT /etc/apparmor.d/usr.bin.evince
real 0m2.992s
user 0m2.979s
sys 0m0.012s
and
$ time apparmor_parser -QT /etc/apparmor.d/
real 0m3.568s
user 0m14.529s
sys 0m0.152s
vs.
$ time apparmor_parser -QT /etc/apparmor.d/
real 0m3.741s
user 0m15.400s
sys 0m0.179s
PR: https://gitlab.com/apparmor/apparmor/merge_requests/246
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
aa-eventd and its initscripts have been moved to deprecated/ in 2014 and
didn't get any serious updates for several more years, so it's most
probably useless and/or broken nowadays.
This also means we don't need to keep the AA_EV_BIN and AA_EV_PIDFILE
variables in rc.apparmor.functions anymore.
The apparmor kernel "module" has not been a loadable module for more
than a decade, it must be built into the kernel and due configuration
requirements it will never go back to being a loadable module.
Remove the long unfunctioning load_module support from the init script.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/257
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: seth.arnold@canonical.com
Elaborate in class comment of firstpos, lastpos, followpos, and nullable
fields beyond just referencing the Dragon book. Also add the section of
the book these are explained in.
If a test is marked as TODO, but matches its EXRESULT, this means the
TODO is superfluous and (probably) a change fixed what the TODO was for.
Instead of more or less ignoring such superfluous TODOs, error out to
make the change visible instantly.
Both result in "superfluous TODO" (for unknown reason), but fail after
removing the TODO.
Disable the tests until we find out why they have this strange
behaviour, to unblock merging the "error out on superfluous TODO" patch.
Remove TODO notes from no-longer-failing tests
See merge request apparmor/apparmor!180
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: intrigeri <intrigeri@debian.org>
Imported from the Debian/Ubuntu packaging. We need this function so that
Debian/Ubuntu can switch to using this shell library instead of their own code.
AppArmor 3.0 requires policy to use a feature abi rule for access to
new features. However some policy may start using abi rules even if
they don't have rules that require new features. This is especially
true for out of tree policy being shipped in other packages.
Add enough support to older releases that the parser will ignore the
abi rule and warn that it is falling back to the apparmor 2.x
technique of using the system abi.
If the profile contains rules that the older parser does not
understand it will fail policy compilation at the unknown rule instead
of the abi rule.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/196
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The handling for quotedid checked for the first quote but failed
to ensure the trailing quote was present.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/196
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
We can reduce the INCLUDE/INCLUDE_EXISTS code dup by using a
variable for whether the name was enclosed by '<' and using
processid() to handle the whether the id is quoted or not.
In addition using processid allows include names to contain
escaoe sequences like \n and have them handled correctly.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/196
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
We can reduce code duplication by checking the current state to
determine the single parameter difference between include and
include if exists
PR: https://gitlab.com/apparmor/apparmor/merge_requests/196
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Currently if stdin is used the warning
apparmor_parser: cannot use or update cache, disable, or force-complain via stdin
is always displayed but if caching has been disabled there is no need for
this message.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
The hardcoded paths for flex and python can break builds on systems
where those tools are stored in an alternate location. Use which
to lookup where flex and bison are available.
This fixes issue #4
Signed-off-by: John Johansen <john.johansen@canonical.com>
Instead of printing the (useless) numbers of no-longer-failing tests
marked as TODO, let `prove` print out the actual test names.
A side effect is that really unfixed TODOs and disabled tests get
printed (lists about 10 tests), but that's much better than having to
hunt down the no-longer-failing TODOs.
This change will print out lines like (the first one is still a TODO,
the second one got fixed at some time)
```
not ok 71447 - TODO: ./simple_tests//vars/vars_profile_name_13.sd: reference variables that are the profile name and attachment # TODO Unfixed testcase.
ok 71448 - TODO: ./simple_tests//vars/vars_profile_name_14.sd: reference variables in rules that also have alternations # TODO Unfixed testcase.
```
before printing the test summary.
Without this, the system-wide parser.conf gets used, which causes test
failures if for example caching is enabled and the cache dir isn't
writeable for the user running the tests.
