When doing permission merging in the dfa minimization phase the information
about whether a rule is dominant or not has been lost so the merge of
xtransitions can not be handled correctly.
When two conflicting x transitions are merged the results are unpredicitable
and not currently detected. So default dfa minimization to set up its
initial partitions with permission hashing, this ensures that dfa states
that have different xtransitions in the minimization stage will never
be merged thus will not result in a conflict.
x permission checking is still enforced at the dfa creation phase where
the originial information is available to check whether the conflicting
permissions came from exact match or re rules so that conflict resolution
can be properly applied.
The end result is that dfa minimization does not result in a truely minimal
dfa (the minimization phase is also slightly faster).
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently apparmor provides the unsafe keyword to indicate an xtransition
is not scrubbing its environment variables. This can be used to be
explicit about which transition are unsafe instead of relying on people
remembering which of px Px is safe or unsafe.
Add the orthogonal keyword safe to allow specifying a transition is
safe.
Signed-off-by: John Johansen <john.johansen@canonical.com>
x Permissions when specified as a the start of the rule had a differnt
meaning than when they appeared at the tail of a rule.
Specifically px,cx,ux were not treated as unsafe when they appeared at
the start of the rule.
px /foo,
instead of at the tail of the rule
/foo px,
the keyword unsafe had to be used to force the rule to cause the x transitio
to be its unsafe variant.
Fix leading permissions so that they are consistent with file rules that
use trailing permissions.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Test the leading permission form of an xrule against its trailing permission
form, to verify that they are generating the same xtransition and thus
don't conflict (assumes xtransition conflict checking is working).
eg.
px /foo,
/foo px,
should generate the same rule and thus not result in any conflicts
Signed-off-by: John Johansen <john.johansen@canonical.com>
All the combiniation of xtransition conflics where not well represented in
the regression test suite. Instead of relying on multiple static test
files, automatically generate all possible conflicts.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The is_merged_x_consistend macro was incorrect in that is tested for
USER_EXEC_TYPE to determine if there was an x transition. This fails
for unconfined execs so an unconfined exec would not correctly conflict
with another exec type.
The dfa match flag table for xtransitions was not large enough and not
indexed properly for pux, and cux transitions. The index calculation did
not take into account the pux flag so that pux and px aliased to the same
location and cux and cx aliased to the same location.
This would result in the first rule being processed defining what the
transition type was for all following rules of the type following. So
if a px transition was processed first all pux, transitions in the profile
would be treated pux.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The dfa engine uses the defines from immunix.h for permission conflict
checking, so make the build depend on it.
Signed-off-by: John Johansen <john.johansen@canonical.com>
During some of the dfa cleanups, the checks for conflicting xtransition
was removed. This adds the conflict checking back in and makes it part
of dfa creation.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Several of the x-trans tests where failing because of the include file was
bad. This kept the test from testing what it was supposed as the test
was expected to fail. Thus hidding a bug :(
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add the ability to specify the name and attachment of the profile
separately. It does not allow for the attachment specification to
begin with a variable however since variables in profile names is not
currently support this shouldn't be and issue.
Signed-off-by: John Johansen <john.johansen@canonical.com>
clean up profile parsing by merging profile and :namespace:profile parsing
into a single rule.
This also fixes a bug where the profile keyword was not allowed to proceed
profiles with a namespace declaration.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Short summary: Unloading of profiles with a space in the name fails,
therefore "rcapparmor stop" (or restart) causes a funny message - and
the profile is still loaded.
Thanks to Christian Boltz <apparmor@cboltz.de>
parser that interact with the regex DFA generation library, and thus
need to be recompiled when the header file changes.
(This patch isn't particularly of interest to distros, as they
typically won't be doing incremental compilation.)
The other changes have made it so that using a macro really isn't justified
so rework the code to get rid of the hiddeous update_for_nodes macro.
Signed-off-by: John Johansen <john.johansen@canonical.com>
With the addition of the nodes field to the state we can make the work
queue, be based off of the state instead of the node, and avoid doing
the node to map lookup to get back to the state.
This means that the NodeMap is now only used for duplicate elimination.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Factoring the updating of the state transitions doesn't save on any code
but it provides a nice logical seperation and makes the dfa work_queue
loop and the updating of the state transitions easier to understand as
units.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The match_count variable is a sum of the number of duplicates node sets
that have been encountered and discarded. Rename it to better reflect what
it is doing.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Embedding the nodes are part of the state gives fast back reference from
the state to the nodes that created it. This is useful for the state to
nodes mapping dump as it lets us output the states in order. It will also
let us avoid certain nodemap lookup in the future.
Overlay the nodes field (used only in dfa construction) with the partition
field which is only used during dfa minimization to avoid making the state
any larger.
Signed-off-by: John Johansen <john.johansen@canonical.com>
commits were made (as well as a few other minor warnings elsewhere).
The Makefile change is to avoid passing -Wstrict-prototypes and
-Wnested-externs to the C++ compiler, which the compiler yells about and
then ignores.
Since we compile with -Wmissing-field-initializers I dropped the
unreferenced zero-width fields in the header structs, and then explicitly
initialized the remaining fields.
I tagged several unused function parameters to silence those warnings.
And finally, I dropped the unused filter_escapes() too.
Embedding the the partition mapping into the State structure significantly
speeds up dfa minimization, by converting rbtree finds to straight direct
references when checking for same mappings.
The overall time improvement is small but it can half the time spent in
minimization.
The nodemap.size() increases by one with each node added, every time we
add a state we label it so this provides the proper labeling without needing
a separate variable.
add short options to turn on all stats, and all progress indicators,
also allow adding "no-" prefix to dump options to allow subtracting
individual options when short options are used.
eg.
-D stats -D no-expr-simplify
