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>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1478
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit cd8b75abc0ca39ccad32c93ca615ed47abd3172e)
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>
(cherry picked from commit ff03702fdebb6c6c55532a6768b404706823c339)
Signed-off-by: John Johansen <john.johansen@canonical.com>
I had this message in my log
```
Dez 30 08:14:46 kernel: audit: type=1400 audit(1735542886.787:307): apparmor="DENIED" operation="open" class="file" profile="/usr/sbin/cupsd" name="/etc/paperspecs" pid=317509 comm="cupsd" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
```
If the second commit is bad, I can drop it.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1472
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit e5a960a68569e7f0cd9a87925c0324f149e8283b)
While cups itself writes to /etc the others require only read-only access
and might therefore live in /usr/etc.
(cherry picked from commit c3af6228fdf808c5013c27239c9ac73e2d6a355f)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Gtk applications like Firefox request write access to the file
`/run/user/1000/dconf/user`. The code in `dconf_shm_open` opens the file
with `O_RDWR | O_CREAT`.
4057f8c84f/shm/dconf-shm.c (L68)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1471
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 0eca26c6c2da9e8c3afb97915c9fba44667ae4de)
Gtk applications like Firefox request write access to the file
`/run/user/1000/dconf/user`. The code in `dconf_shm_open` opens the file
with `O_RDWR | O_CREAT`.
4057f8c84f/shm/dconf-shm.c (L68)
(cherry picked from commit 318fb3044619034c14a5b7dbc7e700b039b8d9ae)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Fix priority for file rules, and the ability to dump the dfa at different stages, and update and fix the equality tests.
This in particular adds the ability to better debug the equality tests. Instead of just piping the parser output into the hash it creates a tmp dir and drops the binary files there so they can be manually examined. It adds new options particularly the -r option making so the tests will exit on first failure to make it easier to isolate and examine a failure.
Eg.
```
./equality.sh -r -d -v
Equality Tests:
................................................................................................................................................................................................................................
Binary inequality 'priority=-1'x'priority=-1' change_hat rules automatically inserted
FAIL: Hash values match
parser: ./../apparmor_parser -QKSq --features-file=./features_files/features.all
known-good (ee4f926922ecd341f1389a79dd155879) == profile-under-test (ee4f926922ecd341f1389a79dd155879) for the following profiles:
known-good /t { priority=-1 owner /proc/[0-9]*/attr/{apparmor/,}current a, ^test { priority=-1 owner /proc/[0-9]*/attr/{apparmor/,}current a, /f r, }}
profile-under-test /t { priority=-1 owner /proc/[0-9]*/attr/{apparmor/,}current w, ^test { priority=-1 owner /proc/[0-9]*/attr/{apparmor/,}current w, /f r, }}
files retained in "/tmp/eq.3240859-deHu10/"
```
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1455
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 40e9b2a96147b9146b103b4143f8261dffd55f3a)
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>
(cherry picked from commit 027b508da8e12979ae893610f6336c31e6af2be5)
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>
(cherry picked from commit bf7b80c478071382c8ef9aef3905a32597ae9e1f)
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>
(cherry picked from commit 25f16b239d735023f2010d43f529fd79cd28326b)
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>
(cherry picked from commit 369029dc074cf292ebab61392a1bead2d2f78d08)
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>
(cherry picked from commit 84650beb2f06d90eb1e600f20cb7f1b7b3dcb8b2)
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>
(cherry picked from commit cca842b897f861eb6057f842c5b75d40345afd10)
Signed-off-by: John Johansen <john.johansen@canonical.com>
In preparation for some additional abilities wrap the current tests in
a function.
Signed-off-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit 05ddc61246888b9f5ccc0aaf848415afdaf6fb19)
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>
(cherry picked from commit 31e60baab22dd542005a55329d666e2e7e36f4ed)
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>
(cherry picked from commit 57c57f198ced77eb76e1e30252ea0c024ca1a42f)
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>
(cherry picked from commit 4b410b67f165aa1768975714b5fa8aabc8f7692c)
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>
(cherry picked from commit d275dfdd42a9195862b89cfe73fded85fb91726e)
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>
(cherry picked from commit fcee32a37e6aeb44aa8d541394a7e7014ba40e88)
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>
(cherry picked from commit 5d2a38e816eacd00bc76502aa8725502fd65eee7)
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>
(cherry picked from commit 9d5b86bc9d9f1fb31e1633145b1506975af0039c)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Starting with Python 3.8, you can use the PYTHONPYCACHEPREFIX environment
variable to define a cache directory for Python [1]. I think most people would set
this dir to @{HOME}/.cache/python/ , so the python abstraction should allow
writing to this location.
[1]: https://docs.python.org/3/using/cmdline.html#envvar-PYTHONPYCACHEPREFIX
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1467
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 8c799f4eec988c62e61fd1399d942b9213406326)
Starting with Python 3.8, you can use the PYTHONPYCACHEPREFIX environment
variable to define a cache directory for Python [1]. I think most people would set
this dir to @{HOME}/.cache/python/ , so the python abstraction should allow
writing to this location.
[1]: https://docs.python.org/3/using/cmdline.html#envvar-PYTHONPYCACHEPREFIX
(cherry picked from commit 03b5a29b05f6ed88ac1101d2cfb964b0fa2003a6)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Depending on the system, copying echo to the loop device fails because the echo binary is too large.
Especially on systems that have echo be just a symlink to coreutils (e.g. busybox) (as opposed to echo being its own binary) 16k is just not enough.
2M seems fine on my system, but this might need yet a higher value depending on what coreutils other people actually run.
The crash in question:
```
cp: error writing '/tmp/sdtest.3937422-31490-Bxvi6g/mount_target/echo': No space left on device
Fatal Error (file_unbindable_mount): Unexpected shell error. Run with -x to debug
rm: cannot remove '/tmp/sdtest.3937422-31490-Bxvi6g/mount_target': Device or resource busy
```
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1469
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 8e431ebcd915216a03ebc8d01e72b1741bb2f855)
Depending on the system, copying echo to the loop device fails because the echo binary is too large.
Especially on systems that have echo be just a symlink to coreutils (e.g. busybox) 16k is just not enough.
2M seems fine on my system, but this might need yet a higher value depending on what coreutils other people actually run.
The actual loop device needs to be larger to properly fit the allocated file size. Testing shows 4M is sufficient, but this is basically arbitrary.
(cherry picked from commit 1cc2a3bd86c5b6642f483c1da8da48276d694eab)
Signed-off-by: John Johansen <john.johansen@canonical.com>
This test, as is, emits an execname warning which is due to a bug in the `prologue.inc` infrastructure (see !1450 for a fix to this issue).
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1448
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit ba60bfff85ada5dbe511f0edeebb93411cf7bb61)
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
(cherry picked from commit fa58d3611a83371d0600e306b196fe303dc39f63)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
(cherry picked from commit c768a7dc79b76a338ab574a88bc46dbdecd833b7)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
(cherry picked from commit 049b35dff03731f2540a2fb1c4e3bf27ecb1a702)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
(cherry picked from commit f249c6d58fc7ae446bca0448aeeb1eab755074d8)
Signed-off-by: John Johansen <john.johansen@canonical.com>
- previously, aa-status --json --show profiles would return non-standard json
- adding the --pretty flag would crash completely
- closes#470
Things done:
- removed trailing ", " in json generation
- generate json seperator (", ") for each new json field
(profiles/processes) after the header if json is enabled
Tested on NixOS and apparmor 4.0.3 base, but should work on any version the patch applies on.
Closes#470
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1451
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit c489631770a0c38ce9075a024b710eff540703d6)
- previously, aa-status --json --show profiles would return non-standard json
- adding the --pretty flag would crash completely
- closes#470
Things done:
- removed trailing ", " in json generation
- generate json seperator (", ") for each new json field
(profiles/processes) after the header if json is enabled
Tested on NixOS and apparmor 4.0.3 base, but should work on any version the patch applies on.
(cherry picked from commit 4f006a660c57a61950e59172c511fef32ee1da63)
Signed-off-by: John Johansen <john.johansen@canonical.com>
This MR is meant to resolve warnings such as "Warning: execname '/home/username/Documents/apparmor/tests/regression/apparmor/file_unbindable_mount': no such file or directory" when running tests like the one in the current version of !1448.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1450
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 59957aa1d8485d924a3606a978e58c0ed29a9fb1)
When settest was called with two parameters, one for the test name and
the other for the test wrapper/binary, the profile created with
genprofile would show the test name, causing an error if the file
didn't exist.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
(cherry picked from commit b4adff2ce03e936cfac1742b655cdb74284dd211)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Some of the tests using the --stdin option of mkprofile.pl are adding
more than one profile at a time. Whenever a profile is created in the
test, its name is added to the file profile.names so the test
infrastructure can tell if the profile is loaded or removed when
appropriately. The issue is that the name of the second profile
created by --stdin is not added, so these checks are not applied.
This patch adds the option of appending a second profile (not rules).
The option --append was used instead of a short -A because the short
options are arguments of mkprofile.pl, which --append is not.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
(cherry picked from commit 0307619ed95d7fd932747b6743f62ba6f9fd4184)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Due to how the tests were implemented in the past, permissions could
be passed along with the image name, and the permission part would be
discarded. The issue is that permissions are usually separated by ':',
but namespaces also contain ':', which would cause a conflict.
Since permissions are no longer passed as part of the image name,
remove that description so profile names in namespaces can be
supported.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
(cherry picked from commit 9cc40e2dcaea75f094dd01be23d7b19f0dd87607)
Signed-off-by: John Johansen <john.johansen@canonical.com>
From LP: #2085377, when using ip netns to torrent traffic through a
VPN, attach_disconnected is needed by the policy because ip netns sets
up a mount namespace.
Fixes: https://bugs.launchpad.net/bugs/2085377
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1395
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 50f260df5144af9142eda4c7b03b49bcc0f64e09)
From LP: #2085377, when using ip netns to torrent traffic through a
VPN, attach_disconnected is needed by the policy because ip netns sets
up a mount namespace.
Fixes: https://bugs.launchpad.net/bugs/2085377
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
(cherry picked from commit f9edc7d4c19853f0502e9e4501f668b001fc656e)
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>
(cherry picked from commit 3ed5adb6656fde85937e6da3ab2628918a833108)
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
(cherry picked from commit 83270fcf68a1f1cd7180febc2370a16542302bdc)
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>
(cherry picked from commit 52babe8054c6428f83c5c028f15063c33ef88e3e)
Signed-off-by: John Johansen <john.johansen@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>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1465
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
(cherry picked from commit 67ee5f8b39811252c03f7e3af607aa7f08308248)
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>
(cherry picked from commit 96718ea4d15b0a4551fece1c36b8360c2e44fad3)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
(cherry picked from commit 5cd3362a81e5195d6364e3d5871a9608af6a30dd)
Signed-off-by: John Johansen <john.johansen@canonical.com>
As per !1462 it turns out that the swap regression test on btrfs also needs special casing in order to work properly. This is an analogous patch to check for btrfs.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1463
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
(cherry picked from commit 6d7b5df94757b0d93d195f8789e3eb81bf0fdf4e)
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
(cherry picked from commit 90c7af69c5e888b5de47f3b22eaf95d9b85e8993)
Signed-off-by: John Johansen <john.johansen@canonical.com>
