Currently states are added to the reachable set when they are popped
from the workqueue. This however can result in states being
added to the work queue multiple times and reprocessed.
Eg. If state 2 has the transitions, and 9 is not in the reachable set
a -> 9
b -> 9
c -> 9
d -> 9
e -> 3
then 9 will get pushed onto the work 4 times. Even worse other states
on the workqueue may also add state 9 to the workqueue because it has
not been added to the reachable set.
Instead add states to the reachable set when they are added to the
workqueue. The first encounter with a state will result in it being
reachable and all other encounters will see that it already in the set
and not add it to the workqueue.
Signed-off-by: John Johansen <john.johansen@canonical.com>
There is a general industry wide effort to move off of md5 and even
sha1 (see recent kernel changes). While in this particular use case it
doesn't make a difference (besides slightly lowering the chance of a
collision) switch to sha256sum to make sure our code doesn't depend on
tools that are deprecated and there is an effort to remove.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Similar to the deny x permission tests, the tests that test carving
out r permissions need to be updated to be conditional on what
priority is being used on the rule.
Signed-off-by: John Johansen <john.johansen@canonical.com>
With priority rules, deny does not carve out permissions from the
higher priority rule. Technically it doesn't from lower priority either
as it completely overrides them, but that case already results in
an inequality so does not cause the tests to fail.
Signed-off-by: John Johansen <john.johansen@canonical.com>
cx rules using a specified profile transition, may be emulated by
using px and a hierarchical profile name. That is
cx -> b
may be transformed into
px -> profile//b
which will generate an xtable entry of
profile//b
which means the previous patch using
pivot_root -> b,
to reliably add b to the xtable will not cover this case.
transition to using two pivot_root rules to provide the xtable entries
pivot_root /a -> b,
pivot_root /c -> /t//b,
the paths /a and /c are irrelavent as long as they don't have an
overlap with the generic globbing expression in the test, Two table
entries will be generated. We guarantee no overlap by converting the
/** to /f**
Also the xtable reserving rules are moved to the end of the profile so
the table order can be reliably created. A follow on MR around xtable
improvements should add reliability to xtable order.
Signed-off-by: John Johansen <john.johansen@canonical.com>
exec rules that specify an specific target profile generate an entry
in the xtable. The test entries containing " -> b" are an example of
this.
Currently the parser allocates the xtable entry before priorities are
applied in the backend, or minimization is done. Further more the
parser does not ref count the xtable entry to know what it is no
longer referenced.
The equality tests generate rules that are designed to completely
override and remove a lower priority rule, and remove it. Eg.
/t { priority=1 /* ux, /f px -> b, }
and then compares the generated profile to the functionaly equivalent
profile eg.
/t { priority=1 /* ux, }
To verify the overridden rule has been completely removed.
Unfortunately the compilation is not removing the unused xtable entry
for the specified transition, causing the equality comparison to fail.
Ideally the parser should be fixed so unused xtable entries are removed,
but that should be done in a different MR, and have its own test.
To fix the current tests, and another rule that adds an xtable entry
to the same target that can not be overriden by the x rule using
pivot_root. The parser will dedup the xtable entry resulting in the
known and test profile both having the same xtable. So the test will
pass and meet the original goal of verifying the x rule being overriden
and eliminated.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Failed equality tests can be hard to debug. The profiles aren't always
enough to figure out what is going on. Add several options that will
help in debugging, and developing new tests.
Add switches and arg parsing.
Add the ability to run tests individually
Add a -r flag to allow retaining the test and output
similar to the regression tests, so the exact output from the
tests can be examined.
Add a -d flag to dump dfa build information.
Allow overriding the parser, features, and description for a given
test run.
Signed-off-by: John Johansen <john.johansen@canonical.com>
printf of failure/error info should be going to stderr. Unfortunately
the test has a mix of 2>&1 and 1>&2. Having a mix is just wrong, we
could standardize on either but since the info is error info 1>&2
seems to be the better choice.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The test was passing because the file priority was always zero bug
resulting in the priority rule always being correctly combined
with the specific match x rule, instead of overriding it.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The test was passing because the file priority always being zero bug,
the supplied rule always had the same priority as the implied
rule. Resulting in binary_equality always passing even though the
specified priority should have resulted in a failure.
Fix this by checking if the priorities are equal to the implied
rule other wise it should result in an inequality.
Signed-off-by: John Johansen <john.johansen@canonical.com>
When there is a failure output the exact call info used to invoke the
parser. To facilitate manually recreating the test.
Signed-off-by: John Johansen <john.johansen@canonical.com>
With the file priority fix the xequality (expected equal but known
failure) tests are now passing. So convert them to regular equality
tests.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The dfa goes through several stages during the build. Allow dumping it
at the various stages instead of only at the end.
Signed-off-by: John Johansen <john.johansen@canonical.com>
File rules could drop priority info when rule matched a rule
that was the same except for having different priority. For now
fix this by treating them as a different rule.
The priority was also be dropped when add_prefix was used to
add the priority during the parse resulting in file rules always
getting a default priority of 0.
Signed-off-by: John Johansen <john.johansen@canonical.com>
While the mount syscall documentation disallows this, the kernel silently
ignores make-* flags when doing a remount, and real applications were
passing this conflicting set of flags. Because changing the kernel to
reject this combination would break userspace, we should allow them
instead.
For an example: see https://bugs.launchpad.net/apparmor/+bug/2091424.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1466
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
While the mount syscall documentation disallows this, the kernel silently
ignores make-* flags when doing a remount, and real applications were
passing this conflicting set of flags. Because changing the kernel to
reject this combination would break userspace, we should allow them
instead.
For an example: see https://bugs.launchpad.net/apparmor/+bug/2091424.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
The previous code would concatenate all of them together without spacing.
While dump_flags and the corresponding operator<< function aren't currently used,
this will help for when dump_flags is used to debug parser problems.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MS_SYNC is a flag for msync(2) while MS_SYNCHRONOUS is a flag for mount(2).
The header used to define MS_SYNC but IMO this is confusing since that's an
unrelated flag.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
currently the equality tests require the tests to PASS as known equality
or inequality. Add the ability to add tests that are a known problem
and are expected to fail the equality, or inequality test.
This is done by using
verify_binary_xequality
verify_binary_xinequality
This allows new tests to be added to document a known issue, without
having to develop the fix for the issue. The use of this facility
is expected to be temporary, so any test marked as xequality or
xinequality will be noisy but not fail the other tests until they
are fixed, at which point they will cause the tests to fail to
force them to be updated to the correct equality or inequality
test.
Signed-off-by: John Johansen <john.johansen@canonical.com>
This also includes renaming SIGTSTP "stp" to "tstp" while preserving backwards compatibility.
Analogous to !1420.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1425
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
On a test system without bison installed, make setup fails with:
/bin/sh: 1: bison: not found
/bin/sh: 1: test: -ge: unexpected operator
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Because this is used in parsing profiles, we keep backwards compatibility by including
both names and mapping them to the same underlying signal number.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Because sig_names is only used to dump parsed signals for debugging purposes,
renaming SIGTSTP "stp" to "tstp" is not a breaking change.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Match Flags convert output to hex but don't restore after outputting
the flag resulting in following numbers being hex encoded. This
results in dumps that can be confusing eg.
rule: \d2 -> \x2 priority=1001 (0x4/0)< 0x4>
rule: \d7 -> \a priority=3e9 (0x4/0)< 0x4>
rule: \d10 -> \n priority=3e9 (0x4/0)< 0x4>
rule: \d9 -> \t priority=3e9 (0x4/0)< 0x4>
rule: \d14 -> \xe priority=1001 (0x4/0)< 0x4>
where priority=3e9 is the hex encoded priority 1001.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1419
Approved-by: Maxime Bélair <maxime.belair@canonical.com>
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The old code implicitly initialized it to 0 by overwriting a
zero-initialized array terminator. Now that we construct the new entry
from scratch, we need to do this manually.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Fix libapparmor_re/Makefile so it works correctly with rebuilds and improve state machine dump information, to aid with debugging of permission handling during the compile.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1410
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Unfortunately, meaningfully parallelizing parser testing is a giant task:
- Parser equality testing is a shell script based framework where adding parallelism would be a major rework.
- Parser testing using Python’s unittest framework also needs a different test runner to enable parallelism.
- Parser testing using Perl’s prove framework already supports parallelism, but adding -j to Prove does not result in speedups. Thus, I suspect most of the overhead is in spawning the processes, and that speeding this part up will require making the parser a library and testing it that way.
The commit in this MR passes a `-j` parallelism flag to Perl's prove framework, but local testing has shown that this does not create speedups, and Gitlab CI has a very modest improvement of 11 minutes 16 seconds for the parser testing stage without `-j $(nproc)` vs 10 minutes 51 seconds with `-j $(nproc)`. Instead of passing `-j $(nproc)`, pass a fixed `-j 2` to gain some speedups, as the overhead of `-j $(nproc)` on a system with more than 2 cores eats up any time gains that parallelism would have brought.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1416
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This function was broken all this time: instead of duplicating each entry in the list, it would duplicate the first entry n times. Since this function is currently not used anywhere, delete it instead of fixing it.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1421
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
Besides of transitioning towards C++, this also eliminates the linear scan search that the functions using these arrays did.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1420
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This function was broken all this time: instead of duplicating each entry
in the list, it would duplicate the first entry n times. Since this
function is currently not used anywhere, delete it instead of fixing it.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
This is simple enough to fix even if weld_file_to_policy isn't used in practice
with the compat layer that uses it being a target for deletion
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
There is a null check before storing invflags into inv, but not before initializing the value at inv to 0.
Assuming the null check is needed, it should be there in both places.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Match Flags convert output to hex but don't restore after outputting
the flag resulting in following numbers being hex encoded. This
results in dumps that can be confusing eg.
rule: \d2 -> \x2 priority=1001 (0x4/0)< 0x4>
rule: \d7 -> \a priority=3e9 (0x4/0)< 0x4>
rule: \d10 -> \n priority=3e9 (0x4/0)< 0x4>
rule: \d9 -> \t priority=3e9 (0x4/0)< 0x4>
rule: \d14 -> \xe priority=1001 (0x4/0)< 0x4>
where priority=3e9 is the hex encoded priority 1001.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The mapping of AA_CONT_MATCH was being dropped resulting in the
tcp tests failing because they would only match up to the first conditional
match check in the layout.
Bug: https://gitlab.com/apparmor/apparmor/-/issues/462
Fixes: e29f5ce5f ("parser: if extended perms are supported by the kernel build a permstable")
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1409
Approved-by: Ryan Lee <rlee287@yahoo.com>
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The parser recently changed how/where deny information is applied.
commit 1fa45b7c1 ("parser: dfa minimization prepare for extended
permissions") removed the implicit filtering of explicit denies during
the minimization pass. The implicit clear allowed the explicit
information to be carried into the minimization pass and merged with
implicit denies. The end result being a minimized dfa with the explicit
deny information available to be applied post minimization, and
then dropped later at permission encoding in the accept entries.
Extended permission however enable carrying explicit deny information
into the kernel to fix certain bugs like complain mode not being
able to distinguish between implicit and explicit deny rules (ie.
deny rules get ignored in complain mode). However keeping explicit
deny information when unnecessary result in a larger state machine
than necessary and slower compiles.
commit 179c1c1ba ("parser: fix minimization check for filtering_deny")
Moved the explicit apply_and_clear_deny() pass to before minimization
to restore mnimization's ability to create a minimized dfa with
explicit and implicit deny information merged but this also cleared
the explicit deny information that used to be carried through
minimization. This meant that when the deny information was applied
post minimization it resulted in the audit and quiet information
being cleared.
This resulted in the query_label tests failing as they are checking
for the expected audit infomation in the permissions.
Fixes: 179c1c1ba ("parser: fix minimization check for filtering_deny")
Bug: https://gitlab.com/apparmor/apparmor/-/issues/461
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1408
Approved-by: Ryan Lee <rlee287@yahoo.com>
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Besides of transitioning towards C++ this also eliminates the linear scan search that the functions using these arrays did.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Because the main overhead of the parser_sanity test suite is process
spawning, parallelizing too much could end up hurting performance instead
of helping it. Thus, use a fixed value of 2 instead of $(nproc).
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
The mapping of AA_CONT_MATCH was being dropped resulting in the
tcp tests failing because they would only match up to the first conditional
match check in the layout.
Bug: https://gitlab.com/apparmor/apparmor/-/issues/462
Fixes: e29f5ce5f ("parser: if extended perms are supported by the kernel build a permstable")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Instead of encoding permissions in the accept and accept2 tables
extended perms uses a permissions table and accept becomes an index
into the table.
Add the ability to dump the permissions table so that it can be
compared and debugged.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The chfa dump is missing information about the accept2 entry. The
accept2 information is necessary to help with debugging state machine
builds as accept2 is used to store quiet and audit information in the
old format or conditional information in the extended perms format.
Signed-off-by: John Johansen <john.johansen@canonical.com>
