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>
This commit adds support for EXPECT_DENIALS in profile tests. Any test
that sets the EXPECT_DENIALS environment variable is expected to trigger
AppArmor denials and will fail if none was generated.
This allows to test that problematic behaviors are correctly blocked.
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1515
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
Introduce the EXPECT_DENIALS environment variable for profile tests.
Each line of EXPECT_DENIALS is a regex that must match an AppArmor
denial for the corresponding test, and conversely.
This ensures that problematic behaviors are correctly blocked and logged.
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
tinyproxy does not need all of nameservice, nameservice-strict is
sufficient. Thanks to @cboltz for the suggestion.
Signed-off-by: Alex Murray <alex.murray@canonical.com>
Add comments to the profile to explain the use of the local override if the
default configuration is changed. As suggested by @rlee287.
Signed-off-by: Alex Murray <alex.murray@canonical.com>
Add rules to allow tinyproxy to bind to privileged ports and access files even
when run as unprivileged/privileged users when using non-standard
configurations. As suggested by @rlee287.
Signed-off-by: Alex Murray <alex.murray@canonical.com>
This was tested using the test-tinyproxy.py script from qa-regression-testing as
well as by running the upstream test suite with a brief hack to ensure it
invokes tinyproxy with aa-exec -p tinyproxy first.
Signed-off-by: Alex Murray <alex.murray@canonical.com>
Profile for `tar` package.
In order to test this, I've diffed the output of the `tar`'s testsuite with and without the profile:
```
sudo apt build-dep tar
apt source tar
cd tar-*/
./configure
cd tests/
./testsuite > without_profile.log
apparmor_parser ~/tar
./testsuite > with_profile.log
diff without_profile.log with_profile.log # should not output anything
echo $? # should be zero
```
Additionally, [the testsuite available on QRT](https://git.launchpad.net/qa-regression-testing/tree/scripts/test-tar.py) for the `tar` package should continue to pass after loading the profile.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1453
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
We had some mixture of indent styles.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1510
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Zygmunt Krynicki <me@zygoon.pl>
We were not building or caching the .seed.iso target, causing make to re-create
the image, as seen in the make --debug --dry-run output:
```
Updating goal targets....
File ubuntu-cloud-24.04.user-data does not exist.
Must remake target ubuntu-cloud-24.04.user-data.
echo "${USER_DATA}" | tee ubuntu-cloud-24.04.user-data
Successfully remade target file ubuntu-cloud-24.04.user-data.
File ubuntu-cloud-24.04.meta-data does not exist.
Must remake target ubuntu-cloud-24.04.meta-data.
echo "${META_DATA}" | tee ubuntu-cloud-24.04.meta-data
Successfully remade target file ubuntu-cloud-24.04.meta-data.
Prerequisite ubuntu-cloud-24.04.user-data is newer than target ubuntu-cloud-24.04.seed.iso.
Prerequisite ubuntu-cloud-24.04.meta-data is newer than target ubuntu-cloud-24.04.seed.iso.
Must remake target ubuntu-cloud-24.04.seed.iso.
/usr/bin/genisoimage \
-input-charset utf-8 \
-output ubuntu-cloud-24.04.seed.iso \
-volid CIDATA \
-joliet \
-rock \
-graft-points \
user-data=ubuntu-cloud-24.04.user-data \
meta-data=ubuntu-cloud-24.04.meta-data
Successfully remade target file ubuntu-cloud-24.04.seed.iso.
Prerequisite ubuntu-cloud-24.04.seed.iso is newer than target ubuntu-cloud-24.04.x86_64.qcow2.
```
Build and cache the cloud-init seed iso to prevent that.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
We are seeing images cached and then re-constructed as if something had
changed in the meanitime. Debug image construction with make --dry-run --debug.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
This way there's somewhat less repetition and the flow of job definitions is,
at least to me, easier to read.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Our cache is rather compressed already, so this should help
a little with wall-clock time.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
A new explicit, non-parallel job is injected when the .image-garden.mk or
.spread.yaml file changes. This job warms up the cache for the subsequent
parallel testing jobs.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
As a security measure, GitLab splits cache into two broad pools: protected and
non-protected. Any job running in a protected branch has access to the
protected cache pool. All other jobs run in the non-protected cache pool.
This effectively forces us to push to cache in non-protected branches, like all
the merge requests, in order to actually use the cache.
Ideally we'd disable this protection and only push from the default branch and
pull otherwise, as changes to dependency set is rather rare.
[1] https://docs.gitlab.com/ee/ci/caching/#use-the-same-cache-for-all-branches
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
This job is now redundant since the same operation is done when spread
builds and runs regression tests.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
This requires a runner with the tags: linux, x86_64, kvm. One needs to
be provisioned for the AppArmor project for the pipeline to function.
It is possible to run the same tests on SAAS runners offered by GitLab
but due to issue gitlab-org/gitlab-runner#6208 there is no way to expose
/dev/kvm on the host to the guest. Without this feature emulation works
but is rather slow as to be impractical.
Note that there's some overlap between the build-all job and spread that
might be avoided in the future. At present this is made more difficult
by the fact that the path where build-all job builds libapparmor is
stored internally by autotools. This prevents us from using GitLab
artifacts from moving the built files across to the spread testing jobs
without extra work.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
There's contention between running spread across many nodes, in chunks,
in a CI/CD pipeline, and running spread on one machine, across many
instances at the same time. The case with CI/CD needs one worker, as
parallelism is provided by GitLab. The case with local spread needs many
workers as parallelism is provided locally by spread allocating new
instances.
At present we need to focus on the CI/CD case. I have a plan on how to
avoid the problem entirely down the line, by running multiple copies of
spread locally, as if everything was done in a CI/CD pipeline.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
A number of tests are failing and since spread does not contain a native
XFAIL facility, we have to maintain a silent-failure feature code
ourselves. A few of those have been fixed since the first iteration of
this patch. The remaining known failures are being fixed.
Later on I would like to separate XFAIL from SKIP so that if a test is
known to exercise kernel feature unavailable on the given system, the
test is just not executed.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1483
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Python 3.13 changes the formatting of long-short option pairs that use a
meta-variable. Up until 3.13 the meta-variable was repeated. Since
Python change [1] the meta-var is only printed once.
[1] https://github.com/python/cpython/pull/103372
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1495
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Zygmunt Krynicki <me@zygoon.pl>
