When a test fails because of an unexpected success (XFAIL), do not display the empty error log as that may confuse the reader just as it had confused the author.
In addition, when something legitimately fails then display tail of trace log as that may show some useful information.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1548
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 8711c7754b)
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
(cherry picked from commit c268e5d11b)
Signed-off-by: John Johansen <john.johansen@canonical.com>
This makes no sense since the test has passed and there's nothing to look at in the log.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
(cherry picked from commit 473e791e4e)
Signed-off-by: John Johansen <john.johansen@canonical.com>
The the `attach_disconnectd` test is now passing on Ubuntu 24.04+.
The `posix_ipc` is passing everywhere.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1547
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 84bf3dee2d)
There is no mqueue in Makefile TESTS anywhere. This is a red herring.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
(cherry picked from commit c56cbad5ea)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
(cherry picked from commit 5f8863c7ca)
Signed-off-by: John Johansen <john.johansen@canonical.com>
The test used to fail on some versions of Ubuntu but it now passes
everywhere.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
(cherry picked from commit 083dc9652b)
Signed-off-by: John Johansen <john.johansen@canonical.com>
The test is now passing on Ubuntu 24.04+
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
(cherry picked from commit 3987bf0f33)
Signed-off-by: John Johansen <john.johansen@canonical.com>
We should be using apparmor controlled domains for these files.
Rename the template file from
com.ubuntu.pkexec.aa-notify.policy
to
net.apparmor.pkexec.aa-notify.policy
And update the template file and the install file so that the files
that are generated use net.apparmor instead of com.ubuntu
Signed-off-by: John Johansen <john.johansen@canonical.com>
Closes#486
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1541
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit e085a23b40)
Signed-off-by: John Johansen <john.johansen@canonical.com>
We should be using apparmor controlled domains for these files.
Rename the template file from
com.ubuntu.pkexec.aa-notify.policy
to
net.apparmor.pkexec.aa-notify.policy
And update the template file and the install file so that the files
that are generated use net.apparmor instead of com.ubuntu
Signed-off-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit a410f347a3)
Signed-off-by: John Johansen <john.johansen@canonical.com>
The install of the polkit action files for aa-notify leaks build root
information.
From OBS
apparmor-utils.noarch: E: file-contains-buildroot (Badness: 10000) /usr/share/polkit-1/actions/com.ubuntu.pkexec.aa-notify.policy
this is present on Ubuntu as well
<annotate key="org.freedesktop.policykit.exec.path">/build/apparmor-ZUzkoL/apparmor-4.1.0~beta4/debian/tmp/usr/lib/python3/dist-packages/apparmor/update_profile.py</annotate>
this occurs because the {LIB_PATH} template variable is being replaced
with the self.install_lib. Make sure we strip the build prefix if
we are generating the files in a build environment instead of doing
a direct install.
Closes: https://gitlab.com/apparmor/apparmor/-/issues/486
Co-Author: Ryan Lee <ryan.lee@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit b4e6f0449b)
Signed-off-by: John Johansen <john.johansen@canonical.com>
The install of the polkit action files for aa-notify leaks build root
information.
From OBS
apparmor-utils.noarch: E: file-contains-buildroot (Badness: 10000) /usr/share/polkit-1/actions/com.ubuntu.pkexec.aa-notify.policy
this is present on Ubuntu as well
<annotate key="org.freedesktop.policykit.exec.path">/build/apparmor-ZUzkoL/apparmor-4.1.0~beta4/debian/tmp/usr/lib/python3/dist-packages/apparmor/update_profile.py</annotate>
this occurs because the {LIB_PATH} template variable is being replaced
with the self.install_lib. Make sure we strip the build prefix if
we are generating the files in a build environment instead of doing
a direct install.
Closes: https://gitlab.com/apparmor/apparmor/-/issues/486
Signed-off-by: John Johansen <john.johansen@canonical.com>
Closes#486
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1540
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 697e53d752)
Follow up from !1544 with the other basic variables.
Variables such as `@{rand6}` and `@{word6}` are very commonly used as they allow us to restrict access from rules such as: `/tmp/*`, `/tmp/??????`
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1546
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit b5ff20b5f1)
This PR only adds the digit `@{d}` and integer `@{int}` variables.
It provides two improvements from the use of the `[0-9]*` glob:
- security: the glob means "a digit followed by anything but `/`", whereas `@{int}` means "up to 10 digits"
Next to the
- stability: using glob in path with `x` can expose to path conflict, removing the glob fixed a lot of issues.
These variables are used by a lot of abstractions that could be upstream here from apparmor.d (PR will follow). It is an import from 33681e14f2/apparmor.d/tunables/multiarch.d/system where other similar variables are in use: `@{hex}`, `@{rand}`, `@{word}`, `@{u8}`, `@{u16}`, `@{u64}`, `@{int2}...@{int64}` ...
They also all could be upstreamed here.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1544
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 783f012074)
On certain lxc containers, when aa-genprof tries to set
printk_ratelimit, it fails with the OSError exception, with the
message "OSError: [Errno 30] Read-only file system" instead of
PermissionError.
Since PermissionError is a subclass of OSError, replace it by broader
OSError exception to include both cases in which running aa-genprof
fails.
Reported-by: Paulo Flabiano Smorigo <paulo.smorigo@canonical.com>
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1539
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 226ab5f050)
On certain lxc containers, when aa-genprof tries to set
printk_ratelimit, it fails with the OSError exception, with the
message "OSError: [Errno 30] Read-only file system" instead of
PermissionError.
Since PermissionError is a subclass of OSError, replace it by broader
OSError exception to include both cases in which running aa-genprof
fails.
Reported-by: Paulo Flabiano Smorigo <paulo.smorigo@canonical.com>
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
(cherry picked from commit e1ae6fa81c)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Instead of setting those variables unconditionally, set them if they
aren't externally set by environment variables. This will allow for usages
like DESTDIR=/some/other/dir make install in the utils directory.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1542
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 49bc2d855f)
Instead of setting those variables unconditionally, set them if they
aren't externally set by environment variables. This will allow for usages
like DESTDIR=/some/other/dir make install in the utils directory.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
(cherry picked from commit 2747013d9b)
Signed-off-by: John Johansen <john.johansen@canonical.com>
ttkthemees may not be installed on some systems, and if not present
will cause aa-notify to fail. Instead of making ttkthemes a required
dependency, make its use conditional on it being present.
Backport by: Christian Boltz <apparmor@cboltz.de>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Changes include:
- using `long` instead of `intmax_t` for `pid_t` typemap (32-bit build failure); see commit message for more details
- specifying messages for `static_assert` declarations (required up until C23, was accepted as a compiler extension on the systems I had tested this on previously)
- removing label-followed-by-declaration instance (also a C23 feature supported as extension)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1536
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 55889ef783)
The message being optional is apparently a C23 thing that was available as an extension on the systems I tested on previously
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
(cherry picked from commit 87b60e4e94)
Signed-off-by: John Johansen <john.johansen@canonical.com>
The previous code using intmax_t failed to build on armhf because
intmax_t was long long int instead of long int on that platform.
As to shrinking down to a long: not only does SWIG lack a
SWIG_AsVal_intmax_t, but aalogparse also assumes PIDs fit in a long
by storing them as unsigned longs in aa_log_record. Thus, we can
assume that sizeof(pid_t) <= sizeof(long) right now and deal with
the big headache that a change to pid_t would cause if it becomes
larger than a long in the future.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
(cherry picked from commit c5016e1227)
Signed-off-by: John Johansen <john.johansen@canonical.com>
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>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1532
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 0c4e452b46)
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>
(cherry picked from commit cce5bd6e95)
Signed-off-by: John Johansen <john.johansen@canonical.com>
The unshare-userns-restrict profile contained a cx transition to
transition to a profile that allows most things while denying
capabilities:
audit allow cx /** -> unpriv,
However, this transition does not stack the unshare//unpriv profile
against any other profile the target binary might have had. As a result,
the lack of stacking resulted in a non-namespace-related sandboxing
bypass in which attachments of other profiles that should have confined
the target binary do not get applied. Instead, we adopt a stack similar
to the one in bwrap-userns-restrict, with the exception that unshare
does not use no-new-privs and therefore only needs a two-layer stack
instead of a three-layer stack.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1533
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 8e586e5492)
The unshare-userns-restrict profile contained a cx transition to
transition to a profile that allows most things while denying
capabilities:
audit allow cx /** -> unpriv,
However, this transition does not stack the unshare//unpriv profile
against any other profile the target binary might have had. As a result,
the lack of stacking resulted in a non-namespace-related sandboxing
bypass in which attachments of other profiles that should have confined
the target binary do not get applied. Instead, we adopt a stack similar
to the one in bwrap-userns-restrict, with the exception that unshare
does not use no-new-privs and therefore only needs a two-layer stack
instead of a three-layer stack.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
(cherry picked from commit ab3ca1a93f)
Signed-off-by: John Johansen <john.johansen@canonical.com>
There are minor tweak around the lib, constify some vars etc. That
don't justify a large bump. Bumpt there have been changes to swig
such that we want to force it to be linked against the new lib.
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>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1522
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 0ab4fc0580)
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>
(cherry picked from commit 1ebd991155)
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>
(cherry picked from commit e56dbc2084)
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>
(cherry picked from commit cc31a0da22)
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>
(cherry picked from commit 9221d291ec)
Signed-off-by: John Johansen <john.johansen@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.
In addition to adding the spread job, remove test-build-regression job.
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>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1512
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 12787648a7)
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
(cherry picked from commit 5a44cbe661)
Signed-off-by: John Johansen <john.johansen@canonical.com>
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>
(cherry picked from commit 4cfeb4a9ad)
Signed-off-by: John Johansen <john.johansen@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>
(cherry picked from commit b3ce87af23)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
(cherry picked from commit 62f93b400e)
Signed-off-by: John Johansen <john.johansen@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>
(cherry picked from commit bcf8c968db)
Signed-off-by: John Johansen <john.johansen@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>
(cherry picked from commit ebb82952bc)
Signed-off-by: John Johansen <john.johansen@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>
(cherry picked from commit 14ceb92ca0)
Signed-off-by: John Johansen <john.johansen@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>
(cherry picked from commit a0adb01631)
Signed-off-by: John Johansen <john.johansen@canonical.com>
