For NAME_MAX
Fixes 322a98c8 ("Fix incorrect strnlen length in aa_load.c load_policy_dir")
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1666
Approved-by: Maxime Bélair <maxime.belair@canonical.com>
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: John Johansen <john@jjmx.net>
The internal permission accumulation is currently broken in that
the ordering of rules matter to whether deny is clearing accumulated
perms.
If a deny node comes before an allow node the deny bits will get set
but the following allow bits won't get cleared by the deny node.
This isn't currently an actual issue for mediation as the deny
bit will be applied at one of
1. apply_and_clear_deny
2. permission remapping
3. run time mediation
but it does result in the internal state having sometimes having both
allow and deny bits set, dependent on order of computation, resulting
in state machines with different sizes because minimization
partitioning is based on the internal permissions.
This means that dfa minimization may not result in a truly minimal
state machine, and even worse can cause inconsistenty and failure in
tests that rely on internal state like the equality and minimization
test, as seen in https://gitlab.com/apparmor/apparmor/-/issues/513
The failure was due to musl stl sets implementation producing a
different ordering of the nodes than glibc. So when the permissions
where accumulated the internal set of permissions were different.
Fix this by giving the different node classes their own internal priority.
This will ensure the bits are properly cleared for that priority before
accumulating.
Note: other ways of fixing.
1. Fixup internal accumulation to use accumulating perms of "higher"
priority as part of the mask (deny and allow mask prompt).
2. Do a hard masking apply at the end after all bits have been accumulated
(ie, in accept_perms after the for loop).
the priority route was chosen because it is a little smaller and
scales better if we get new Node types we have to deal with
(eg. planned complain node).
BugLink: https://gitlab.com/apparmor/apparmor/-/issues/513
Fixes: 1ebd99115 ("parser: change priority so that it accumulates based on permissions")
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1655
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The internal permission accumulation is currently broken in that
the ordering of rules matter to whether deny is clearing accumulated
perms.
If a deny node comes before an allow node the deny bits will get set
but the following allow bits won't get cleared by the deny node.
This isn't currently an actual issue for mediation as the deny
bit will be applied at one of
1. apply_and_clear_deny
2. permission remapping
3. run time mediation
but it does result in the internal state having sometimes having both
allow and deny bits set, dependent on order of computation, resulting
in state machines with different sizes because minimization
partitioning is based on the internal permissions.
This means that dfa minimization may not result in a truly minimal
state machine, and even worse can cause inconsistenty and failure in
tests that rely on internal state like the equality and minimization
test, as seen in https://gitlab.com/apparmor/apparmor/-/issues/513
The failure was due to musl stl sets implementation producing a
different ordering of the nodes than glibc. So when the permissions
where accumulated the internal set of permissions were different.
Fix this by giving the different node classes their own internal priority.
This will ensure the bits are properly cleared for that priority before
accumulating.
Note: other ways of fixing.
1. Fixup internal accumulation to use accumulating perms of "higher"
priority as part of the mask (deny and allow mask prompt).
2. Do a hard masking apply at the end after all bits have been accumulated
(ie, in accept_perms after the for loop).
the priority route was chosen because it is a little smaller and
scales better if we get new Node types we have to deal with
(eg. planned complain node).
BugLink: https://gitlab.com/apparmor/apparmor/-/issues/513
Fixes: 1ebd99115 ("parser: change priority so that it accumulates based on permissions")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add AA profile for ssh-keyscan. This profile has been tested on the latest plucky `ssh-agent` version `1:9.9p1-3ubuntu3`. In particular, this has been tested using the tests upstream and the archive in:
* `openssh-tests`
This is linked to the discussion in https://gitlab.com/apparmor/apparmor/-/merge_requests/1502, to add the profile deps of ssh-agent in as well.
Signed-off-by: Evan Caville <evan.caville@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1597
Approved-by: Ryan Lee <rlee287@yahoo.com>
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
This helps to limit the amount of rules skipped in the utils tests
(because the utils don't support the `unsafe` keyword)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1671
Approved-by: Maxime Bélair <maxime.belair@canonical.com>
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: Ryan Lee <rlee287@yahoo.com>
The documentation was wrong about how options=(list) and options in (list) are combined
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1674
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The lack of a space after $testtype is a syntax error and was causing the
equality tests on Ubuntu Xenial to be silently skipped and marked PASS.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1670
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
Add basic support for the priority rules prefix. This patch does not\
allow the utils to set or suggest priorities. It allows parsing and\
retaining of the priority prefix if it already exists on rules and\
checking if it's in the supported range.
Note that this MR is supposed to replace WIP MR https://gitlab.com/apparmor/apparmor/-/merge_requests/1531
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1636
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
Add basic support for the priority rules prefix. This patch does not
allow the utils to set or suggest priorities. It allows parsing and
retaining of the priority prefix if it already exists on rules and
checking if it's in the supported range.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Some classes don't support modifiers like audit and deny. Only rlimit
has been checking for the allow keyword, but the others shouldn't
support it as well. Since they all do the same check, refactor them
into a method from BaseRule in case more modifiers are added.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
The lack of a space after $testtype is a syntax error and was causing the
equality tests on Ubuntu Xenial to be silently skipped and marked PASS.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
A modeline must appear in the first and last five (by default) lines of a file
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1661
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
- set filetype, instead of syntax, in vim modelines
- replace filetype of subdomain with apparmor
- move modelines in the first or last five lines of each file so that
vim can recognize them
I'm working on a Rust library project that collects hardware and OS information from the local system and queries the Ubuntu Hardware Certification service to determine if the device model has been certified.
I'd like to add an AppArmor profile to ensure the `hwctl` program has access to the resources it needs.
Project source code: https://github.com/canonical/hardware-api/
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1658
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
We need to be able to dump the initial partition assignments, and then
the partitions after minimization but before remapping to be able to
check on what is being done by minimization.
Add these as part of -D dfa-minimize-partitions
Ideally we would rework the code so that the existing mininimization
dump could share the dump routine but, its interwined with computation
state and information is thrown away before reaching the end.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1651
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Improve the failure messages around getting policy information to
be less ambiguous about what went wrong.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1653
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
We need to be able to dump the initial partition assignments, and then
the partitions after minimization but before remapping to be able to
check on what is being done by minimization.
Add these as part of -D dfa-minimize-partitions
Ideally we would rework the code so that the existing mininimization
dump could share the dump routine but, its interwined with computation
state and information is thrown away before reaching the end.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Without this, arguments like -r would not work.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1659
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Unconfined delegates access to open file descriptors. Therefore when running a confined binary from unconfined, it will work even when the attachment path is not read-allowed.
However, as soon as these confined binaries are run from another confined process, this delegation is not permitted anymore and the program breaks.
This has been the cause of several bugs such as https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/2107455 or https://github.com/canonical/snapd/pull/15181 .
This MR makes sure every confining AppArmor profiles explicitly allow (at least) read access to their attachment path.
This Merge request:
- Introduce `test_profile.sh`, a helper script that ensures confining AppArmor profiles explicitly allow (at least) read access to their attachment path.
- Modifies a lot of profiles so that all profiles have r/mr access to their attachment path
- Extends `make check` to automatically ensure all AppArmor profile grant explicit read access to their attachment path, preventing future omissions.
- Modifies apparmor_parser to show attachment in --debug output
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1637
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This reverts commit 75959225b35cc3cd76e684f2db62e27ee4e81288.
Do not enable the verify attachment-path script as part of the CI.
1. The script itself has several bashisms, that need to be fixed before
we land it as part of the regular integration test.
2. The script is going to need to be extended to support the new
parser variables, before it can be turned on as part of the CI.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Improve the failure messages around getting policy information to
be less ambiguous about what went wrong.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Downstream report: https://github.com/NixOS/nixpkgs/issues/347490
Output of `aa-status` is nondescriptive if no profiles are loaded.
Worse yet, the json output isn't even valid json.
It would make sense to just return a json object with no entries,
instead of returning a non-json `eprint` to stderr.
**Ideally** output of non-json would be more descriptive.
Currently:
```
apparmor module is loaded.
Failed to get profiles: 2....
```
What i would prefer:
```
apparmor module is loaded.
Failed to get profiles: No policies loaded
```
However, i am unfamiliar with how the translation framework works,
and thus not confident to do this change blindly.
I am happy to add a commit doing so if i know where to change all that.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1652
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Downstream report: https://github.com/NixOS/nixpkgs/issues/347490
Output of `aa-status` is nondescriptive if no profiles are loaded.
Worse yet, the json output isn't even valid json.
It would make sense to just return a json object with no entries,
instead of returning a non-json `eprint` to stderr.
glibc defines bsd's rlimit ofile as nofile, however musl does not define
rlimit ofile at all.
Instead of just dropping ofile which would be bad for policy portability
make sure it is defined to be nofile.
This is a partial fie for
https://gitlab.com/apparmor/apparmor/-/issues/513
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1648
Approved-by: Steve Beattie <steve+gitlab@nxnw.org>
Merged-by: Steve Beattie <steve+gitlab@nxnw.org>
glibc defines bsd's rlimit ofile as nofile, however musl.
Instead of just dropping ofile which would be bad for policy portability
make sure it is defined to be nofile.
This is a partial fie for
https://gitlab.com/apparmor/apparmor/-/issues/513
Signed-off-by: John Johansen <john.johansen@canonical.com>
This reverts commit 27f5b623f726a84f8430825e2e2641043965af94, reversing
changes made to ee08bfbc905102380bfcaf64d5d84bced98c9360.
This causes the plasmashell profile to have a conflicting x modifiers
error. This breaks CI and compile/load of the plasmashell profile.
Revert until it can be fixed. Using priority.
Signed-off-by: John Johansen <john.johansen@canonical.com>
If kanidm is configured in nsswitch.conf(5), access to the kanidm-unixd
configuration is needed for applications to resolve entries.
For example:
```
type=AVC apparmor="DENIED" operation="open" class="file" profile="php-fpm"
name="/etc/kanidm/unixd" comm="php-fpm" requested_mask="r" denied_mask="r"
```
Signed-off-by: Georg Pfuetzenreuter <mail@georg-pfuetzenreuter.net>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1638
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: John Johansen <john@jjmx.net>
There is an unfortunate long kernel dev history as to why this currently
isn't the case, so we're stuck with documenting the facts for now.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1641
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
There is an unfortunate long kernel dev history as to why this currently
isn't the case, so we're stuck with documenting the facts for now.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
If kanidm is configured in nsswitch.conf(5), access to the kanidm-unixd
configuration is needed for applications to resolve entries.
For example:
```
type=AVC apparmor="DENIED" operation="open" class="file" profile="php-fpm"
name="/etc/kanidm/unixd" comm="php-fpm" requested_mask="r" denied_mask="r"
```
Signed-off-by: Georg Pfuetzenreuter <mail@georg-pfuetzenreuter.net>
Extend `make check` to automatically ensure every AppArmor profile grants
explicit read access to its attachment path, preventing future omissions.
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
Grant explicit read permission on each profile’s attachment path. This
avoid issues when running them from a confined environment and makes
test_profile.sh pass.
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
Unconfined delegates access to open file descriptors. Therefore when
running a confined binary from unconfined, it will work even when the
attachment path is not read-allowed.
However, as soon as these confined binaries are run from another
confined process, this delegation is not permitted anymore and the
program breaks.
This has been the cause of several bugs such as
https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/2107455 or
https://github.com/canonical/snapd/pull/15181 .
Introduce `test_profile.sh`, a helper script that ensures confining
AppArmor profiles explicitly allow (at least) read access to their
attachment path.
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
