... and adjust the tools to raise an exception if such a rule is found.
While this is not nice, it's better than the previous behaviour where
only the last 'options' was kept, and the others were lost when writing
the rule.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1617
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
MR: 1561 Added the ability to specify special a keyword to allow
detached mounts. Unfortunately it updated remount to use the device
and devbuffer when remounts current encoding doesn't support it.
This caused the mount.sh regression test to fail in the following
way.
```
$ sudo bash mount.sh
[sudo] password for jj:
using mount rules ...
Error: mount failed. Test 'MOUNT (confined cap bind mount remount rprivate conflict)' was expected to 'pass'. Reason for failure 'FAIL: mount /tmp/sdtest.358520-12403-ASaOnn/mountpoint2 on /tmp/sdtest.358520-12403-ASaOnn/mountpoint failed - Permission denied'
not supported by parser - skipping mount options=(nodirsync),
Error: mount failed. Test 'MOUNT (confined cap mount remount option)' was expected to 'pass'. Reason for failure 'FAIL: mount /dev/loop40 on /tmp/sdtest.358520-12403-ASaOnn/mountpoint failed - Permission denied'
Error: mount failed. Test 'MOUNT (confined cap mount remount)' was expected to 'pass'. Reason for failure 'FAIL: mount /dev/loop40 on /tmp/sdtest.358520-12403-ASaOnn/mountpoint failed - Permission denied'
Error: mount passed. Test 'MOUNT (confined cap mount remount deny option)' was expected to 'fail'
```
Revert the change to remount. This fixes the regression failure.
fa0746f2e parser: add special casing for detached move mounts
Signed-off-by: John Johansen <john.johansen@canonical.com>
... and adjust the tools to raise an exception if such a rule is found.
While this is not nice, it's better than the previous behaviour where
only the last 'fstype' was kept, and the others were lost when writing
the rule.
... and adjust the tools to raise an exception if such a rule is found.
While this is not nice, it's better than the previous behaviour where
only the last 'options' was kept, and the others were lost when writing
the rule.
upsteam move_mount mediation now allows for a detached (disconnected)
mount to be move mounted into a namespace.
Add support for this by detecting 'detached' as a keyword for the
source/device and using it to create a null match. Because existing
mount encoding using a null separator between the mount terms null
match followed by the null seperator will separate detached mounts
within the existing encoding.
Eg.
mount detached -> /destination,
mount options=(ro) fstype=ext4 detached -> /destination,
This is functionally equivalent to using
mount "" -> /destination,
However using "" does not provide any context that about what the rule is allowing or why so the 'detached' form is preferred.
This is not a perfect solution, but is what can be currently supported
by the kernel without more LSM hooks.
On kernels that don't support detached mount detection, rules using
the detached souce conditional will be ignored (never matched).
This encoding also allows the existing
mount,
mount options=(move),
mount options=(move) -> /destination,
to continue to work with both detached and regular mounts on kernels
that support the move_mount() syscall.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Fixes https://bugs.launchpad.net/apparmor/+bug/2106033
Improve the validation of AARE file paths by introducing a new regex
that supports paths starting with '{' (e.g. '{/,/org/freedesktop/DBus}').
These paths are notably used in snap.lxd.* profiles.
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1607
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The original location in the middle of the priority function helper was
completely nonsensical. We can instead do this check just once after
running all the tests.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Fixes https://bugs.launchpad.net/apparmor/+bug/2106033
Improve the validation of AARE file paths by introducing a new regex
that supports paths starting with '{' (e.g. '{/,/org/freedesktop/DBus}').
These paths are notably used in snap.lxd.* profiles.
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
Followup that replaces !1576.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1581
Approved-by: Maxime Bélair <maxime.belair@canonical.com>
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: John Johansen <john@jjmx.net>
The attach_disconnected.ipc flag allows the use of disconnected paths
on posix mqueues. This flag is a subset of attach_disconnected, and it
does not allow disconnected paths for all files.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Similarly to apparmor/apparmor!403, we don't really need to pass these flags
here, but if we don't, blhc raises a false positive, and I don't want to get
used to ignoring blhc failures on Debian's GitLab CI.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1558
Approved-by: Alex Murray <alex.murray@canonical.com>
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: Ryan Lee <rlee287@yahoo.com>
This imports translations from launchpad up to commit
revno: 2523
committer: Launchpad Translations on behalf of apparmor-dev
branch nick: apparmor
timestamp: Fri 2025-02-21 09:32:26 +0000
message:
Launchpad automatic translations update.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Similarly to apparmor/apparmor!403, we don't really need to pass these flags
here, but if we don't, blhc raises a false positive, and I don't want to get
used to ignoring blhc failures on Debian's GitLab CI.
In the following policy, "ptrace" would be dropped during merging:
```
$FOO=true
/bin/true {
if $FOO {
ptrace,
}
}
```
Current behavior:
```
----- Debugging built structures -----
Name: /bin/true
Local To: <NULL>
Mode:
```
With patch:
```
----- Debugging built structures -----
Name: /bin/true
Local To: <NULL>
Mode:
ptrace,
```
I am quite new to the AA code base, so please let me know if I'm missing something obvious and this is intended behavior :)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1551
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
In the following policy, "ptrace" would be dropped during merging:
$FOO=true
/bin/true {
if $FOO {
ptrace,
}
}
Current behavior:
----- Debugging built structures -----
Name: /bin/true
Local To: <NULL>
Mode:
With patch:
----- Debugging built structures -----
Name: /bin/true
Local To: <NULL>
Mode:
ptrace,
The documentation was missing information about path sanitization, and
why you shouldn't do a leading @{VAR} on path rules. While the example
doing this was fixed, actual information about why you shouldn't do
this was missing.
Document how apparmor will collapse consecutive / characters into a
single character for paths, except when this occurs at the start of
the path.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The current behavior of priority rules can be non-intuitive with
higher priority rules completely overriding lower priority rules even in
permissions not held in common. This behavior does have use cases but
its can be very confusing, and does not normal policy behavior
Eg.
priority=0 allow r /**,
priority=1 deny w /**,
will result in no allowed permissions even though the deny rule is
only removing the w permission, beause the higher priority rule
completely over ride lower priority permissions sets (including
none shared permissions).
Instead move to tracking the priority at a per permission level. This
allows the w permission to still override at priority 1, while the
read permission is allowed at priority 0.
The final constructed state will still drop priority for the final
permission set on the state.
Note: this patch updates the equality tests for the cases where
the complete override behavior was being tested for.
The complete override behavior will be reintroduced in a future
patch with a keyword extension, enabling that behavior to be used
for ordered blocks etc.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The original patch adding priority to the set of prefixes failed to
update the prefix dump to include the priority priority field.
Fixes: e3fca60d1 ("parser: add the ability to specify a priority prefix to rules")
Signed-off-by: John Johansen <john.johansen@canonical.com>
The priority field is only used during state construction, and can
even prevent later optimizations like minimization. The parser already
explcitily clears the states priority field as part of the last thing
done during construction so it doesn't prevent minimization
optimizations.
This means the state priority not only wastes storage because it is
unused post construction but if used it could introduce regressions,
or other issues.
The change to the minimization tests just removes looking for the
priority field that is no longer reported.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Like was done for the other MatchFlags switch to using a node type
instead of dynamic_cast as this will result in a performance
improvement.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add a basic overview of the ordering of the backend of the compiler
and which stages specific dump info lines up with.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1470
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Add a basic overview of the ordering of the backend of the compiler
and which stages specific dump info lines up with.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Construction of the chfa can reorder states from what the numbering
given during the hfa constuctions because of reordering for better
compression, dead state removal to ensure better packing etc.
This however means the dfa dump is difficult (it is possible using
multiple dumpes) to match up to the chfa that the kernel is
using. Make this easier by making the dfa dump be able to take
the remapping as input, and provide an option to dump the
chfa equivalent hfa.
Renumbered states will show up as {new <== {orig}} in the dump
Eg.
```
--D dfa-states
{1} <== priority (allow/deny/prompt/audit/quiet)
{5} 0 (0x 4/0//0/0/0)
{1} perms: none
0x2 -> {5} 0 (0x 4/0//0/0/0)
0x4 -> {5} 0 (0x 4/0//0/0/0)
\a 0x7 -> {5} 0 (0x 4/0//0/0/0)
\t 0x9 -> {5} 0 (0x 4/0//0/0/0)
\n 0xa -> {5} 0 (0x 4/0//0/0/0)
\ 0x20 -> {5} 0 (0x 4/0//0/0/0)
4 0x34 -> {3}
{3} perms: none
0x0 -> {6}
{6} perms: none
1 0x31 -> {5} 0 (0x 4/0//0/0/0)
```
```
-D dfa-compressed-states
{1} <== priority (allow/deny/prompt/audit/quiet)
{2 == {5}} 0 (0x 4/0//0/0/0)
{1} perms: none
0x2 -> {2 == {5}} 0 (0x 4/0//0/0/0)
0x4 -> {2 == {5}} 0 (0x 4/0//0/0/0)
\a 0x7 -> {2 == {5}} 0 (0x 4/0//0/0/0)
\t 0x9 -> {2 == {5}} 0 (0x 4/0//0/0/0)
\n 0xa -> {2 == {5}} 0 (0x 4/0//0/0/0)
\ 0x20 -> {2 == {5}} 0 (0x 4/0//0/0/0)
4 0x34 -> {3}
{3} perms: none
0x0 -> {4 == {6}}
{4 == {6}} perms: none
1 0x31 -> {2 == {5}} 0 (0x 4/0//0/0/0)
```
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1474
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
As reported in https://gitlab.com/apparmor/apparmor/-/merge_requests/1475
uint requires the inclusion of sys/types.h for use in musl libc.
Including that would be fine but since it is only used for the
cast for the owner type comparison, just convert to use a more
standard type.
Reported-by: @fossd <fossdd@pwned.life>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Construction of the chfa can reorder states from what the numbering
given during the hfa constuctions because of reordering for better
compression, dead state removal to ensure better packing etc.
This however means the dfa dump is difficult (it is possible using
multiple dumpes) to match up to the chfa that the kernel is
using. Make this easier by making the dfa dump be able to take the
emapping as input, and provide an option to dump the chfa equivalent
hfa.
Renumbered states will show up as {new <== {orig}} in the dump
Eg.
--D dfa-states
{1} <== priority (allow/deny/prompt/audit/quiet)
{5} 0 (0x 4/0//0/0/0)
{1} perms: none
0x2 -> {5} 0 (0x 4/0//0/0/0)
0x4 -> {5} 0 (0x 4/0//0/0/0)
\a 0x7 -> {5} 0 (0x 4/0//0/0/0)
\t 0x9 -> {5} 0 (0x 4/0//0/0/0)
\n 0xa -> {5} 0 (0x 4/0//0/0/0)
\ 0x20 -> {5} 0 (0x 4/0//0/0/0)
4 0x34 -> {3}
{3} perms: none
0x0 -> {6}
{6} perms: none
1 0x31 -> {5} 0 (0x 4/0//0/0/0)
-D dfa-compressed-states
{1} <== priority (allow/deny/prompt/audit/quiet)
{2 == {5}} 0 (0x 4/0//0/0/0)
{1} perms: none
0x2 -> {2 == {5}} 0 (0x 4/0//0/0/0)
0x4 -> {2 == {5}} 0 (0x 4/0//0/0/0)
\a 0x7 -> {2 == {5}} 0 (0x 4/0//0/0/0)
\t 0x9 -> {2 == {5}} 0 (0x 4/0//0/0/0)
\n 0xa -> {2 == {5}} 0 (0x 4/0//0/0/0)
\ 0x20 -> {2 == {5}} 0 (0x 4/0//0/0/0)
4 0x34 -> {3}
{3} perms: none
0x0 -> {4 == {6}}
{4 == {6}} perms: none
1 0x31 -> {2 == {5}} 0 (0x 4/0//0/0/0)
Signed-off-by: John Johansen <john.johansen@canonical.com>
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>
