by adding a warning flag that is disabled by default. This will enable
devs to find when and where #include is in use by adding the compile
flag
--warn=pound-include
and can even abort policy compiles by using
--warn=pound-include --Werror=pound-include
The resulting messages look like
Warning from /etc/apparmor.d/usr.sbin.cupsd (/etc/apparmor.d/usr.sbin.cupsd line 5): deprecated use of '#include'
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Time-out
This change tries to give us some room to deal with versioning issues
like we had in
145136f6 Fix 2.13 libapparmor so version
which revered the library version bump in
5c47e448 libapparmor: Bump revision in preparation for release.
which was done because of the symbol added by
351014c3 libapparmor: add _aa_asprintf to private symbols
unfortunately this collided with the 3.0 library symbol versioning
which had a different set of symbols with the same version numbering.
Leaving us no way to increase the version number to when fixing an
exported symbol version in a released library.
Ideally this would have been fixed with library version synchs but
in this case that option wasn't open as we were dealing with two
released versions of the library.
Bumping the current version by 5 on the larger releases gives us room
to deal with these kind of bugs in the future.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Time-out
Note: This was reported for /usr/libexec/libvirt_leaseshelper, but since
this is probably unrelated to the path or a path change, this commit
also adds r permissions for the previous path.
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1202161
I propose this patch for 3.0 and master (optionally also for 2.12 and 2.13 - please tell me if you want that after reviewing the patch, or just merge ;-)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/905
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Note: This was reported for /usr/libexec/libvirt_leaseshelper, but since
this is probably unrelated to the path or a path change, this commit
also adds r permissions for the previous path.
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1202161
profiles/Makefile:
Set PYTHON_DIST_BUILD_PATH using
libraries/libapparmor/swig/python/test/buildpath.py as it solves the
problem of setting the build directory generically.
libraries/libapparmor/swig/python/test/buildpath.py:
The changes introduced in
cc7f549665
targetted a wrong setuptools version (61.2).
The change in build directory naming has been introduced with 62.1
(1c23f5e1e4).
I've seen this test fail because "apparmor_parser -N" returned the expected
lines, but in a different order than what's expected (dirtest.out).
To fix this, sort both the expected and actual output.
This MR closes#239. In the temporary file commit, `tempfile.NamedTemporaryFile` is preferred over `tempfile.mkstemp` because it allows for simpler use of context managers and lets you choose what mode to open the file in. Also in this commit, note that in `aa.py` and `easyprof.py` destination files are now written directly, instead of writing to temp files and then renaming them.
Closes#239
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/898
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
Per the discussion in #243, this MR removes Python 2 compatibility. Namely, this merge request:
- removes code behind `sys` and `platform` interpreter version checks
- removes `unicode` vs. `str` handling
- removes unnecessary `__future__` imports
- removes unnecessary `object` inheritance
- removes unnecessary `super()` arguments
- uncomments commented-out code with "uncomment when python3 only" notes or some variant of that message
Regarding the `unicode` vs. `str` handling, it's arguably more Pythonic to check `isinstance(x, str)` as opposed to `type(x) is str`, but I didn't want to alter code behavior.
A change needs to be made to the `INCOMPLETE_COVERAGE` setting in `utils/test/Makefile` to get the pipeline to pass. I didn't get anywhere tweaking the setting myself, so someone else with more AppArmor experience will have to make that change.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/894
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
This is a follow-up on !812, which added a call to systemd-detect-virt.
Everywhere else we don't assume that program is present,
and first check if it's there before we run it.
Let's do the same here.
Tuples [are quite a bit faster than lists](https://stackoverflow.com/a/22140115). If you don't need mutability, a tuple is almost always the better collection to choose. This merge request changes lists to tuples where appropriate, and speeds up some list creations by changing `list()` calls into `[]` literals. It also changes a few function calls to use tuple unpacking, i.e. `func(*tup)`, as opposed to `func(tup[0], tup[1], ...)`. This yields yet another performance improvement, as well as a readability improvement.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/889
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
The inetutils syslogd implementation integrates the kmsg support in the
main server, it has support for syslog.d configuration fragment files.
and it uses a slightly different pid filename.
Signed-off-by: Guillem Jover <guillem@hadrons.org>
Abstractions should not generally include deny rules as this can unduly
constrain profiles which include them due to the precedent matching rules
between deny vs allow rules. Also as per the comment, this is not required
for exo-open to work, so simply omit it from the abstraction for
now. Finally, in Ubuntu, the evince profile includes the exo-open
abstraction and this deny rule causes evince to fail to initialise
correctly as it then assumes it cannot use gvfs.
Signed-off-by: Alex Murray <alex.murray@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/884
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Abstractions should not generally include deny rules as this can unduly
constrain profiles which include them due to the precedent matching rules
between deny vs allow rules. Also as per the comment, this is not required
for exo-open to work, so simply omit it from the abstraction for
now. Finally, in Ubuntu, the evince profile includes the exo-open
abstraction and this deny rule causes evince to fail to initialise
correctly as it then assumes it cannot use gvfs.
Signed-off-by: Alex Murray <alex.murray@canonical.com>
The inverse character set lists the characters it doesn't match. If
the inverse character set contains an oob then that is NOT considered
a match. So length should be one.
However because of oobs are handle not containing an oob doesn't mean
there is a match either. Currently the only way to match an oob is
via a positive express (no inverse matches are possible).
Signed-off-by: John Johansen <john.johansen@canonical.com>
I noticed that some apps return the following errors when launched:
```
kernel: audit: type=1400 audit(1651244478.255:5501): apparmor="DENIED" operation="open" profile="some_app" name="/sys/devices/pci0000:00/0000:00:02.0/revision" pid=1877976 comm="some_app" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
kernel: audit: type=1400 audit(1651244478.255:5502): apparmor="DENIED" operation="open" profile="some_app" name="/sys/devices/pci0000:00/0000:00:02.0/config" pid=1877976 comm="some_app" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
```
Blocking the files results in the following errors when the app is executed in a terminal:
```
MESA: error: Failed to query drm device.
libGL error: failed to create dri screen
libGL error: failed to load driver: crocus
MESA: error: Failed to query drm device.
libGL error: failed to create dri screen
libGL error: failed to load driver: crocus
```
Since they have something to do with MESA, I think the mesa abstraction should
be updated to fix the issue.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/879
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Without the change apparmor build fails on this week's gcc-13 snapshot as:
capability.h:66:6: error: variable or field '__debug_capabilities' declared void
66 | void __debug_capabilities(uint64_t capset, const char *name);
| ^~~~~~~~~~~~~~~~~~~~
capability.h:66:27: error: 'uint64_t' was not declared in this scope
66 | void __debug_capabilities(uint64_t capset, const char *name);
| ^~~~~~~~
capability.h:23:1: note: 'uint64_t' is defined in header '<cstdint>'; did you forget to '#include <cstdint>'?
22 | #include <linux/capability.h>
+++ |+#include <cstdint>
23 |
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/882
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Without the change apparmor build fails on this week's gcc-13 snapshot as:
capability.h:66:6: error: variable or field '__debug_capabilities' declared void
66 | void __debug_capabilities(uint64_t capset, const char *name);
| ^~~~~~~~~~~~~~~~~~~~
capability.h:66:27: error: 'uint64_t' was not declared in this scope
66 | void __debug_capabilities(uint64_t capset, const char *name);
| ^~~~~~~~
capability.h:23:1: note: 'uint64_t' is defined in header '<cstdint>'; did you forget to '#include <cstdint>'?
22 | #include <linux/capability.h>
+++ |+#include <cstdint>
23 |
aarch64 needs some additional rules on tumbleweed to handle for
example
apparmor="DENIED" operation="file_mmap" profile="samba-dcerpcd" name="/usr/lib64/samba/samba-dcerpcd" pid=897 comm="samba-dcerpcd" requested_mask="m" denied_mask="
The other new rpcd_* services exhibit similar errors
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1198309
Signed-off-by: Noel Power <noel.power@suse.com>
I noticed that some apps return the following errors when launched:
kernel: audit: type=1400 audit(1651244478.255:5501): apparmor="DENIED" operation="open" profile="some_app" name="/sys/devices/pci0000:00/0000:00:02.0/revision" pid=1877976 comm="some_app" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
kernel: audit: type=1400 audit(1651244478.255:5502): apparmor="DENIED" operation="open" profile="some_app" name="/sys/devices/pci0000:00/0000:00:02.0/config" pid=1877976 comm="some_app" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
Blocking the files results in the following errors when the app is executed in a terminal:
MESA: error: Failed to query drm device.
libGL error: failed to create dri screen
libGL error: failed to load driver: crocus
MESA: error: Failed to query drm device.
libGL error: failed to create dri screen
libGL error: failed to load driver: crocus
Since they have something to do with MESA, I think the mesa abstraction should
be updated to fix the issue.
The snap_browsers abstraction requires more permissions
due to updates on snaps.
Some of the permissions are not required in older versions of
Ubuntu that use 2.12 and 2.13, but are introduced for unification
and ease of maintenance purposes. These include:
```
all dbus permissions,
@{PROC}/sys/kernel/random/uuid r,
owner @{PROC}/@{pid}/cgroup r,
/var/lib/snapd/sequence/{chromium,firefox,opera}.json r,
```
I also propose a cherry-pick of this commit to 2.12, 2.13 and 3.0
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/877
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The snap_browsers abstraction requires more permissions
due to updates on snaps.
Some of the permissions are not required in older versions of
Ubuntu that use 2.12 and 2.13, but are introduced for unification
and ease of maintenance purposes. These include:
all dbus permissions,
@{PROC}/sys/kernel/random/uuid r,
owner @{PROC}/@{pid}/cgroup r,
/var/lib/snapd/sequence/{chromium,firefox,opera}.json r,
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
samba-4.16 has a completely new dcerpc subsystem, services that
used to be built into the smbd daemon itself (and deployed in forked
instances) are now hosted in standalone binaries. The following new
binaries now need new profiles
rpcd_classic
rpcd_epmapper
rpcd_fsrvp
rpcd_lsad
rpcd_mdssvc
rpcd_rpcecho
rpcd_spoolss
rpcd_winreg
samba-dcerpcd
Additionally smbd & winbindd need new entries because the exec
samba-dcerpcd
Signed-off-by: Noel Power <noel.power@suse.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/871
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
samba-4.16 has a completely new dcerpc subsystem, services that
used to be built into the smbd daemon itself (and deployed in forked
instances) are now hosted in standalone binaries. The following new
binaries now need new profiles
rpcd_classic
rpcd_epmapper
rpcd_fsrvp
rpcd_lsad
rpcd_mdssvc
rpcd_rpcecho
rpcd_spoolss
rpcd_winreg
samba-dcerpcd
Mostly these are captured in a single common profile 'samba-rpcd'
Additionally smbd & winbindd need new entries because they exec
samba-dcerpcd
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1198309
Signed-off-by: Noel Power <noel.power@suse.com>
This function is based on reload_profile() in tools.py, but also
replaces most of reload_base() in aa.py.
For bonus points, we get rid of shell=True when calling apparmor_parser.
Note: This slightly changes the behaviour of aa-logprof and aa-genprof -
if the parser errors out ($? > 0), the output no longer gets hidden.
However, this will not raise an exception, and aa-logprof and aa-genprof
won't abort on parser errors.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/855
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
systemd will attempt to force socket buffer size using setsockopt
and param SO_SNDBUFFORCE (which require net_admin cap) if it's previous
attempt to set size was clipped by kernel limit.
- Silence 'type=AVC msg=audit(1648725005.727:201): apparmor="DENIED" operation="capable" profile="smbd" pid=3054 comm="smbd" capability=12 capname="net_admin"'
type entries.
Signed-off-by: Noel Power <noel.power@suse.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/867
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
... if a test is expected to fail, but succeeds.
Also fix the copyright year - the test was created in 2022, not in 2013.
This fixes my comments on
bd78b6b292
systemd will attempt to force socket buffer size using setsockopt
and param SO_SNDBUFFORCE (which require net_admin cap) if it's previous
attempt to set size was clipped by kernel limit.
- Silence 'type=AVC msg=audit(1648725005.727:201): apparmor="DENIED" operation="capable" profile="smbd" pid=3054 comm="smbd" capability=12 capname="net_admin"'
type entries.
Signed-off-by: Noel Power <noel.power@suse.com>
similar to commit 2f9d172c64
we discovered that there was a service outage
when dovecot tried to send a usr1 signal
type=AVC msg=audit(1648024138.249:184964): apparmor="DENIED" operation="signal" profile="dovecot" pid=1690 comm="dovecot" requested_mask="send" denied_mask="send" signal=usr1 peer="dovecot-imap-login"
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/865
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
- add pki/blacklist and pki/blocklist
- add /usr/share/pki/ in adddition to /etc/pki/
pki/blocklist was suggested by @darix, the other changes are things I noticed while adding it.
I propose this patch for 3.0 and master. (`abstractions/ssl_certs` on 2.x branches is quite different and needs a manual backport (or more cherry-picks) if you want to backport this MR.)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/864
Approved-by: Jon Tourville <jon.tourville@canonical.com>
Merged-by: Christian Boltz <apparmor@cboltz.de>
similar to commit 2f9d172c64
we discovered that there was a service outage
when dovecot tried to send a usr1 signal
type=AVC msg=audit(1648024138.249:184964): apparmor="DENIED" operation="signal" profile="dovecot" pid=1690 comm="dovecot" requested_mask="send" denied_mask="send" signal=usr1 peer="dovecot-imap-login"
- add pki/blacklist and pki/blocklist
- add /usr/share/pki/ in adddition to /etc/pki/
pki/blocklist was suggested by @darix, the other changes are things I noticed while adding it.
libapparmor: fix handling of failed symlink traversal, fixed a couple
of directory walk issues that could cause failures. The test included
in this commit was supposed to be included in the previous commit,
but was accidentally dropped. Even worse the make file changes did
make it causing the previous commit to break the CI.
Fixes: MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/85
Signed-off-by: John Johansen <john.johansen@canonical.com>
Get rid of subprocess with shell=True
Simplify logmark used in syslog.
Instead of using `date | md5sum` and parsing the output to get the actual md5sum (without the stdin filename), use the current unixtime with a `logmark-` prefix.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/856
Acked-by: Seth Arnold <seth.arnold@gmail.com>
Merged-by: Christian Boltz <apparmor@cboltz.de>
Instead of using `date | md5sum` and parsing the output to get the
actual md5sum (without the stdin filename), use the current unixtime
with a `logmark-` prefix.
This function is based on reload_profile() in tools.py, but also
replaces most of reload_base() in aa.py.
For bonus points, we get rid of shell=True when calling apparmor_parser.
Note: This slightly changes the behaviour of aa-logprof and aa-genprof -
if the parser errors out ($? > 0), the output no longer gets hidden.
However, this will not raise an exception, and aa-logprof and aa-genprof
won't abort on parser errors.
Today, a normal user connected and did something (dunno what) that caused smbd to try to `/usr/share/samba/mdssvc/elasticsearch_mappings.json`:
Samba logs:
```
root@smb:~# journalctl -b0 -u smbd
-- Logs begin at Fri 2022-01-21 14:17:01 UTC, end at Thu 2022-02-17 23:56:02 UTC. --
Feb 17 14:01:20 smb systemd[1]: Starting Samba SMB Daemon...
Feb 17 14:01:26 smb smbd[113]: [2022/02/17 14:01:26.904865, 0] ../../lib/util/become_daemon.c:135(daemon_ready)
Feb 17 14:01:26 smb systemd[1]: Started Samba SMB Daemon.
Feb 17 14:01:26 smb smbd[113]: daemon_ready: daemon 'smbd' finished starting up and ready to serve connections
Feb 17 21:05:35 smb smbd[3084]: pam_unix(samba:session): session opened for user jdoe by (uid=0)
Feb 17 21:05:37 smb smbd[3084]: [2022/02/17 21:05:37.735182, 0] ../../source3/rpc_server/mdssvc/mdssvc_es.c:92(mdssvc_es_init)
Feb 17 21:05:37 smb smbd[3084]: mdssvc_es_init: Opening mapping file [/usr/share/samba/mdssvc/elasticsearch_mappings.json] failed: unable to open /usr/share/samba/mdssvc/elasticsearch_mappings.json: Permission denied
Feb 17 21:05:37 smb smbd[3084]: [2022/02/17 21:05:37.735436, 0] ../../source3/rpc_server/mdssvc/mdssvc.c:1490(mdssvc_init)
Feb 17 21:05:37 smb smbd[3084]: mdssvc_init: backend init failed
Feb 17 21:05:37 smb smbd[3084]: [2022/02/17 21:05:37.735562, 0] ../../source3/rpc_server/mdssvc/srv_mdssvc_nt.c:152(_mdssvc_open)
Feb 17 21:05:37 smb smbd[3084]: _mdssvc_open: Couldn't create policy handle for partage
Feb 17 23:56:02 smb smbd[3084]: pam_unix(samba:session): session closed for user jdoe
```
Since the 'smb' machine is a container, the Apparmor denial ended up in the host's log:
```
$ journalctl -o cat --grep samba -k --since today | cat
audit: type=1400 audit(1645131937.730:98): apparmor="DENIED" operation="open" namespace="root//lxd-smb_<var-snap-lxd-common-lxd>" profile="smbd" name="/usr/share/samba/mdssvc/elasticsearch_mappings.json" pid=35359 comm="smbd" requested_mask="r" denied_mask="r" fsuid=166549 ouid=165536
```
It is the first time it occurs in years of use but it seems legitimate as:
1) this file is installed by the package
2) `git grep -F elasticsearch_mappings` in Debian samba's source shows many hits:
```
$ git grep -F elasticsearch_mappings
debian/samba.install:usr/share/samba/mdssvc/elasticsearch_mappings.json
docs-xml/manpages/mdsearch.1.xml: <filename>/usr/share/samba/mdssvc/elasticsearch_mappings.json</filename>
docs-xml/smbdotconf/misc/elasticsearchmappings.xml: <value type="default">&pathconfig.SAMBA_DATADIR;/elasticsearch_mappings.json</value>
docs/manpages/mdfind.1:/usr/share/samba/mdssvc/elasticsearch_mappings\&.json
docs/manpages/smb.conf.5:\fI\fIelasticsearch:mappings\fR\fR\fI = \fR\fI${prefix}/var/samba/elasticsearch_mappings\&.json\fR\fI \fR
selftest/selftest.pl: elasticsearch:mappings = $srcdir_abs/source3/rpc_server/mdssvc/elasticsearch_mappings.json
selftest/target/Samba3.pm: elasticsearch:mappings = $srcdir_abs/source3/rpc_server/mdssvc/elasticsearch_mappings.json
source3/rpc_server/mdssvc/es_parser_test.c: "%s/mdssvc/elasticsearch_mappings.json",
source3/rpc_server/mdssvc/mdssvc_es.c: "%s/mdssvc/elasticsearch_mappings.json",
source3/rpc_server/wscript_build: 'mdssvc/elasticsearch_mappings.json')
```
While only the `mdssvc` sub-dir could be authorized, the whole dir content seemed OK for read access anyway:
```
root@smb:~# ll /usr/share/samba/
total 53
drwxr-xr-x 5 root root 10 Feb 1 14:08 ./
drwxr-xr-x 67 root root 67 Jun 22 2021 ../
-rwxr-xr-x 1 root root 1163 Jan 31 13:11 addshare.py*
drwxr-xr-x 3 root root 4 Feb 1 14:08 admx/
drwxr-xr-x 2 root root 3 Feb 1 14:08 mdssvc/
-rwxr-xr-x 1 root root 2059 Jan 31 13:11 panic-action*
-rwxr-xr-x 1 root root 1333 Jan 31 13:11 setoption.py*
drwxr-xr-x 5 root root 57 Feb 1 14:08 setup/
-rw-r--r-- 1 root root 8942 Jan 31 13:11 smb.conf
-rwxr-xr-x 1 root root 2682 Jan 31 13:11 update-apparmor-samba-profile*
```
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/853
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
It appears secret detection is failing if the master branch in the tree a merge request is being made from is too (some unknown value) far behind the branch of the tree it is being merged into.
This is problematic as it is not started practice to refresh the upstream branches of forked trees, but to keep multiple remotes in a single local tree, branch from mainline master, work on the branch and push to the fork for the merge request. This will require contributors to refresh their forked trees in secret-detection fails. Which may be problematic for some contributors, but since we don't know how bad this is going to be, for now re-enable secret detection.
This reverts commit 8b4344c17b.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/854
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: John Johansen <john@jjmx.net>
As shellcheck taught me
today (https://github.com/koalaman/shellcheck/wiki/SC2015),
"A && B || C is not if-then-else. C may run when A is true".
It does not matter here in practice, because worst case we would run "true" once
too many, but still.
In this case it does not matter, we're merely testing if we can actually
read from that file, but let's make this robust (and shellcheck happy)
for future's sake.
Reference: https://www.shellcheck.net/wiki/SC2162
In 73e124d4fb I've upstreamed the `is_container_with_internal_policy()` function, but so far it was not used anywhere upstream. This is the missing bit.
I could trace the history of that patch back to 2012 (2.7.102-0ubuntu3):
* debian/apparmor.init: do nothing in a container. This can be
removed once stacked profiles are supported and used by lxc.
(LP: #978297)
Context: I lack both knowledge and motivation to keep maintaining this as part of the Debian delta. I'd rather see upstream, and in particular folks more knowledgeable than me about LXC/LXD, or with external motivation factors to work on this part of the stack, take care of it.
Note: Debian has similar code in its [sysvinit script](https://salsa.debian.org/apparmor-team/apparmor/-/blob/debian/master/debian/apparmor.init). I'm not touching that one.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/840
Acked-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This MR depends on !843, mostly for convenience and to avoid having to rework it once !843 is merged. If this turns out to be a blocker, I can rebase it `--onto` master.
It's based on the draft from !584 and !716, but on top of copying'n'pasting the examples from the GitLab documentation, which was necessary but not sufficient, in this MR I tried my best to make these features work in our context: it actually passes CI, it does not clutter the CI UI with jobs that are not applicable here, and it yields a manageable amount of output (as opposed to hundreds of "OMG you're using format strings", that I don't think any of us is going to triage one by one any time soon).
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/844
Acked-by: Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This makes the pipeline run time about 30% shorter and, perhaps more importantly, this gives us more direct access to test failures: they are not hidden in the middle of the huge `test-all` log anymore.
As a bonus, this gives us much faster feedback for tests with a short duration.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/843
Acked-by: Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This allows distributions to start aa-notify automatically, should they wish so, by installing that file in a suitable location, such as `/etc/xdg/autostart`.
This file was introduced in Ubuntu 2.8.95~2430-0ubuntu3 package in 2014, replacing the `/etc/X11/Xsession.d` snippet that Ubuntu had added in 2010.
I'd like to stop having to care about this file as part of the Debian delta and to enable greater collaboration.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/839
Acked-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
libtool generates horrible shell code, you don't want to see the
shellcheck results for it ;-)
This is only relevant for local testing (not in CI) because these files
don't exist in git and therefore don't exist when the shellcheck CI job
runs.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/847
Acked-by: Approved-by: intrigeri <intrigeri@boum.org>
Merged-by: Christian Boltz <apparmor@cboltz.de>
It reports hundreds of issues, lots of them with critical severity.
The GitLab UI allows dismissing them one-by-one very quickly,
but I'm not a good person to do that.
Let's try to have a better signal/noise ratio for this first iteration.
According to
https://docs.gitlab.com/ee/user/application_security/dependency_scanning/,
"dependency scanning lets you know if your application uses an external (open
source) library that is known to be vulnerable".
AppArmor is not the kind of project that benefits from it: we don't link
statically against our dependencies, nor bundle them into released
artifacts.
libtool generates horrible shell code, you don't want to see the
shellcheck results for it ;-)
This is only relevant for local testing (not in CI) because these files
don't exist in git and therefore don't exist when the shellcheck CI job
runs.
- Assume /bin/sh has dash's features and ignore corresponding false positives
- Exclude parser/tst, tests and utils/test directories: they have tons
of shellcheck violations but they don't *directly* impact our users.
Let's first focus on code that runs on production systems.
- Exclude rc.apparmor.slackware: I don't know anything about Slackware's
/bin/sh.
The previous code happened to work only because we always pass either 0 or 1
arguments to these functions. If we ever passed them 2+ arguments,
unexpected things would happen.
For details, see https://www.shellcheck.net/wiki/SC2145
Even if there are Red Hat / Fedora systems that use AppArmor, chances are that
they use systemd, and not an initscript. And even if somehow they do use an
initscript, chances are that it's not this one, as last time it has seen
a non-cosmetic change was in 2007.
* Don't call aa_log_action_end after calling aa_log_failure_msg, because
a generic "failure" message will be outputted twice by the Red Hat and
Slackware init scripts.
* Don't append a space to the initial output from apparmor_stop, in line
with other usages of aa_log_daemon_msg.
Debian doesn't use the init script provided in parser/rc.apparmor.debian,
instead preferring to patch parser/rc.apparmor.functions and call its
functions directly in an init script they maintain themselves (something
they have done since 2006). Since this script is no longer used, and
currently doesn't work correctly anyway because it lacks definitions for
several functions that are relied upon in parser/rc.apparmor.functions,
it can be removed.
If `apparmor_parser -N` (in `profiles_names_list()`) fails,
`aa-remove-unknown` possibly gets an incomplete list of profiles in
`/etc/apparmor.d/` and therefore might remove more profiles than it
should.
Replace the `profiles_names_list()` call with a direct `apparmor_parser`
call, and abort aa-remove-unknown if it exits with $? != 0
Before:
```
aa-remove-unknown -n
AppArmor parser error for /etc/apparmor.d/broken in profile /etc/apparmor.d/broken at line 1: syntax error, unexpected TOK_ID, expecting TOK_OPEN
Would remove 'delete_me'
```
After:
```
./aa-remove-unknown -n
AppArmor parser error for /etc/apparmor.d in profile /etc/apparmor.d/zbroken at line 1: syntax error, unexpected TOK_ID, expecting TOK_OPEN
apparmor_parser exited with failure, aborting.
```
And of course, after fixing the broken profile:
```
./aa-remove-unknown -n
Would remove 'delete_me'
```
Also drop the now-unused profiles_names_list() from rc.apparmor.functions - the only user was aa-remove-unknown.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/836
Acked-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
If apparmor_parser -N (in profiles_names_list()) fails,
aa-remove-unknown possibly gets an incomplete list of profiles in
/etc/apparmor.d/ and therefore might remove more profiles than it
should.
Replace the profiles_names_list() call with a direct apparmor_parser
call, and abort aa-remove-unknown if it exits with $? != 0
Before:
```
aa-remove-unknown -n
AppArmor parser error for /etc/apparmor.d/broken in profile /etc/apparmor.d/broken at line 1: syntax error, unexpected TOK_ID, expecting TOK_OPEN
Would remove 'delete_me'
```
After:
```
./aa-remove-unknown -n
AppArmor parser error for /etc/apparmor.d in profile /etc/apparmor.d/zbroken at line 1: syntax error, unexpected TOK_ID, expecting TOK_OPEN
apparmor_parser exited with failure, aborting.
```
And of course, after fixing the broken profile:
```
./aa-remove-unknown -n
Would remove 'delete_me'
```
Library versioning requires we keep these changes in sync. Since the
3.0.4 release bumped revision we need to do it here as well.
Signed-off-by: John Johansen <john.johansen@canonical.com>
... which is used by aa-remove-unknown.
apparmor_parser can read a whole directory, therefore we don't need to
do the directory listing, excluding *.rpmnew etc. ourself.
Related to https://gitlab.com/apparmor/apparmor/-/issues/148
The AppArmor systemd scripts correctly detect the Windows Subsystem for Linux as a container, since all Linux distros executing under WSL 2 are containerized; however, unlike the majority of containers (since there is no accessible host distribution above them trying to set AppArmor policies that might interfere), AppArmor itself functions without problems in the WSL environment.
This patch adds WSL detection to the is_container_with_internal_policy function, allowing AppArmor to be started and operate normally in a WSL-plus-systemd environment.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/812
Acked-by: John Johansen <john@jjmx.net>
LP:1377338 <https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1377338>
has been fixed for quite awhile and we don't need to call xargs as
a fallback when loading policy fails.
In addition we really don't want to be doing this because we want to
be moving to atomic profile loads where if one profile fails the
whole load fails. This is a step in that direction.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
This test uses unix_fd_server to open a file and pass
its file descriptor to the attach_disconnected tests, which
then mounts, pivots root and then tries to open the file.
Since the server execs the client, this MR also inverts the order
of the parameters to allow the server to forward the client's args
along with the unix_socket path.
I'm also refactoring out the unix_fd_client logic into unix_fd_common,
so we can use this implementation when creating other clients, which is the case
for the test binary attach_disconnected
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/810
Acked-by: Approved-by: John Johansen <john@jjmx.net>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
This test uses unix_fd_server to open a file and pass
its file descriptor to the attach_disconnected tests, which
then mounts, pivots root and then tries to open the file.
Since the server execs the client, this commit also inverts the order
of the parameters to allow the server to forward the client's args
along with the unix_socket path.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/810
Acked-by: John Johansen <john.johansen@canonical.com>
Whenever the evince deb package tries to open a snap browser which was
selected as the default, we get the following denial:
audit[2110]: AVC apparmor="DENIED" operation="exec" profile="/usr/bin/evince" name="/usr/bin/snap" pid=2110 comm="env" requested_mask="x" denied_mask="x" fsuid=1000 ouid=0
As a short-term solution, we are adding a snap-browsers profile
which restricts what snaps opened by evince can do.
The long-term solution is currently not available, but could be
accomplished by using enhanced environment variable filtering/mediation
and delegation of open fds.
Bug: https://launchpad.net/bugs/1794064
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/806
Acked-by: John Johansen <john@jjmx.net>
GTK/QT apps need to read some config files in order to properly render their windows in a graphical session. There are some `qt5` abstractions already, but it looks like the `gtk` abstraction is missing.
The `*gtk*` rules are basically the location of the GTK config files I found on the internet when I was trying to compose this abstraction. Some of the paths are missing in Debian, but different distros can use (or were using) them. Since GTK apps use themes, the abstraction also include the whole `/usr/share/themes/` , `@{HOME}/.themes/` and `@{HOME}/.local/share/themes/` dirs. I'm not sure whether this should be tightened. The `.Xauthority` file is generally needed when you deal with GUI apps under Xserver. Also since all GUI apps redirect output/error to the `~/.xsession-errors` file, this file also was included here.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/65
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/168
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/825
Acked-by: John Johansen <john.johansen@canonical.com>
The Hack used to build the libapparmor swig interface for ruby fails
with ruby 3.1. Instead of trying to do black magic in ruby to rename
the generated Makefile to Makefile.new, just save off the Makefile
and restore after ruby's setup has been called.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/206
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
busybox xargs does not have -d nor long --max-procs options, instead use -0 (and separate arguments with printf "%s\\0") and -P which are more portable. While we are here, also add -r (--no-run-if-empty, which also has no long equivalent for busybox) as we likely don't want to run anything if no profile were found
This is useful for alpine systems where findutils is not installed by default, but busybox xargs is available.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/828
Acked-by: John Johansen <john.johansen@canoical.com>
busybox xargs does not have -d nor long --max-procs options,
instead use -0 (and separate arguments with printf "%s\0")
and -P which are more portable.
While we are here, also add -r (--no-run-if-empty, which also has
no long equivalent for busybox) as we likely don't want to run
anything if no profile were found
This is useful for alpine systems where findutils is not installed by
default, but busybox xargs is available.
profiles/apparmor.d: Fix file_mmap violation for bsc#1192336.
See merge request apparmor/apparmor!819
Acked-by: Christian Boltz <apparmor@cboltz.de> for 3.0 and master
If there's still some code left that tries to access an uninitialized
item in 'aa' (reading or writing), this will result in a very visible
crash instead of silently seeming to work.
Testing shows that we seem to correctly initialize each item in 'aa' (no
crashes), therefore let's hope the best ;-)
- allow reading *.so.*
- allow directory listings in .../site-packages/
- allow reading various metadata files
These additions are based on denials seen on openSUSE Leap 15.3 with
python 3.9.
This keeps all existing permissions, and adds a few that were out of
sync:
- /usr/lib*/python3.*/lib-dynload/*.so missed 3.1[0-9]
- /usr/lib/python3/... was missing, only /usr/local/python3/ was allowed
Both aarch64 and s390x have a bigger wtmp record size (16 bytes more than x86_64, 400 bytes total).
The byte position of the timestamp is also different on each architecture. To make things even more interesting, s390x is big endian.
Note that this MR includes more things, like
* moving `get_last_login_timestamp()` to the new `apparmor/notify.py` file
* add unit tests for it
* add wtmp example files from various architectures, including a hand-edited one claiming to be from 1999
* fixing a bug in `get_last_login_timestamp()` that unpacked `type` from too many bytes - which accidently worked on x86_64
* detecting from which architecture the wtmp file comes (luckily the timestamps are located at different locations)
See the individual commits for details.
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1181155
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/809
Acked-by: John Johansen <john.johansen@canonical.com>
Ensure that pre-2000 and post-2050 dates get rejected, and something in
between gets accepted.
This also extends coverage to 100% - before, the post-2050 branch was
not covered.
Both aarch64 and s390x have a bigger wtmp record size (16 bytes more
than x86_64, 400 bytes total).
The byte position of the timestamp is also different on each
architecture. To make things even more interesting, s390x is big endian.
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1181155
'type' is a short (see "ut_type" in wtmp(5)), therefore only read two
bytes and unpack them as short. Afterwards read two padding bytes to
/dev/null.
This accidently worked on x86_64 because it's little endian, but will
fail on big endian architectures.
Whenever the evince deb package tries to open a snap browser which was
selected as the default, we get the following denial:
audit[2110]: AVC apparmor="DENIED" operation="exec" profile="/usr/bin/evince" name="/usr/bin/snap" pid=2110 comm="env" requested_mask="x" denied_mask="x" fsuid=1000 ouid=0
As a short-term solution, we are adding a snap-browsers profile
which restricts what snaps opened by evince can do.
The long-term solution is currently not available, but could be
accomplished by using enhanced environment variable filtering/mediation
and delegation of open fds.
Bug: https://launchpad.net/bugs/1794064
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
... and some rules in the smbd profile to execute it and send it a term
signal.
samba-bgqd is (quoting its manpage) "an internal helper program
performing asynchronous printing-related jobs."
samba-bgqd was added in Samba 4.15.
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1191532
A small patch set to fix two issues with binutils/aa-features-abi:
1. The `aa-features-abi -f` short argument was not added to the
`getopt_long()` set of short arguments, resulting in the command
incorrectly failing when passed -f
2. Due to variable shadowing the file descriptor for the `--file`
argument was not being autoclosed.
- binutils/aa-features-abi: make -f short arg actually be accepted
- binutils/aa-features-abi: fix failure to close fd due to shadowed
var decl
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/804
The variable used to store the file descriptor for the --file ended up
being declared twice, resulting in the autoclose attribute attached to
the first declaration being removed by the shadowed second declaration.
Fix this by converting the second declaration to just be an assignment,
as was intended.
strace output before:
[...]
) = 1925
close(1) = 0
exit_group(0) = ?
+++ exited with 0 +++
strace output after removing shadow declaration:
) = 1925
close(1) = 0
close(3) = 0
exit_group(0) = ?
+++ exited with 0 +++
(File descriptor 3 is what is returned by the open() call on the
--file argument.)
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/804
The aa-features-abi -f short argument was not added to the
getopt_long() set of short arguments, resulting in the command
incorrectly failing like so:
$ ./aa-features-abi -f /etc/apparmor.d/abi/3.0
./aa-features-abi: invalid option -- 'f'
USAGE: ./aa-features-abi [OPTIONS] <SOURCE> [OUTPUT OPTIONS]
[...]
The long --file option works as expected.
Fix this by adding f to the set of short arguments passed to
getopt_long().
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/804
The parser is broken on RLIMIT parsing when receiving unexpected input
because the shared state for this specifies RLIMIT_MODEINCLUDE which
is an unknown start condition resulting in the following warning
parser_lex.l:745: undeclared start condition RLIMIT_MODEINCLUDE
and also means RLIMIT and INCLUDE are not properly handled
Signed-off-by: John Johansen <john.johansen@canonical.com>
If /proc/*/attr/apparmor/current exists, only read that - instead of
falling back to /proc/*/attr/current if a process is for example
unconfined so that read_proc_current returns None.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/199
Some STATUS log events trigger a crash in aa-notify because the log
line doesn't have operation=. Examples are:
type=AVC msg=audit(1630913351.586:4): apparmor="STATUS" info="AppArmor Filesystem Enabled" pid=1 comm="swapper/0"
type=AVC msg=audit(1630913352.610:6): apparmor="STATUS" info="AppArmor sha1 policy hashing enabled" pid=1 comm="swapper/0"
Fix this by not looking at log events without operation=
Also add one of the example events as libapparmor testcase.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/194
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/797
Acked-by: John Johansen <john.johansen@canonical.com>
The default log format for void linux is not handled by current log
parsing. The following example message results in an invalid record
error.
2021-09-11T20:57:41.91645 kern.notice: [ 469.180605] audit: type=1400 audit(1631392703.952:3): apparmor="ALLOWED" operation="mkdir" profile="/usr/bin/kak" name="/run/user/1000/kakoune/" pid=2545 comm="kak" requested_mask="c" denied_mask="c" fsuid=1000 ouid=1000
This log message fails on parsing
kern.notice:
which differs from the expect syslog format of
host_name kernel:
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/196
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/799
Signed-off-by: John Johansen <john.johansen@canonical.com>
When building with YYDEBUG=1 the following failure occurs
grammar.y:49:46: error: unknown type name ‘no_debug_unused_’; did you mean ‘debug_unused_’?
void aalogparse_error(unused_ void *scanner, no_debug_unused_ char const *s)
^~~~~~~~~~~~~~~~
debug_unused_
g
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/799
Signed-off-by: John Johansen <john.johansen@canonical.com>
Some STATUS log events trigger a crash in aa-notify because the log
line doesn't have operation=. Examples are:
type=AVC msg=audit(1630913351.586:4): apparmor="STATUS" info="AppArmor Filesystem Enabled" pid=1 comm="swapper/0"
type=AVC msg=audit(1630913352.610:6): apparmor="STATUS" info="AppArmor sha1 policy hashing enabled" pid=1 comm="swapper/0"
Fix this by not looking at log events without operation=
Also add one of the example events as libapparmor testcase.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/194
... instead of keeping an own version of it witht the exact same code
and a TODO note to use the one from common.
Also adjust the aa-easyprof tests to directly import AppArmorException
from apparmor.common.
* removes runtime dependency on which
* fixes aa-unconfined when ss is installed outside {/usr,}/bin
Signed-off-by: Michal Vasilek <michal.vasilek@nic.cz>
... instead of None.
This avoids the need to allow type changes (None vs. str).
Also adjust the tests accordingly.
While on it, simplify the tests for attachment.
attachment is always a str, therefore adjust the test to expect an empty
str ('') instead of None - and later converting that None to ''.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/786
Acked-by: John Johansen <john.johansen@canonical.com>
... using [] instead of {}
This should keep the order of checking (and therefore code coverage)
constant, and should fix the randomly appearing partial coverage in
severity.py handle_variable_rank(). In some random cases (depending in
which order the replacements were done and checked for their severity),
the coverage report indicated that the 'elif' condition was never false.
Note: This is only "coverage cosmetics". In "real users", it doesn't
matter in which order the variable replacements are checked because the
result doesn't depend on the ordering.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/790
Acked-by: John Johansen <john.johansen@canonical.com>
... using [] instead of {}
This should keep the order of checking (and therefore code coverage)
constant, and should fix the randomly appearing partial coverage in
severity.py handle_variable_rank(). In some random cases (depending in
which order the replacements were done and checked for their severity),
the coverage report indicated that the 'elif' condition was never false.
Note: This is only "coverage cosmetics". In "real users", it doesn't
matter in which order the variable replacements are checked because the
result doesn't depend on the ordering.
When using the system parser ${parser_config} will be empty and so if this
is quoted when passed as argument to the parser then this gets in
interpreted as the name of a file to be compiled and hence the parser just
prints:
File not found, skipping...
File not found, skipping...
File not found, skipping...
...
for all the aa_policy_cache tests - instead fix this by just not quoting
this argument as suggested by cboltz.
This fixes the regression tests to run to completion without error when
USE_SYSTEM=1 is set.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/788
Signed-off-by: Alex Murray <alex.murray@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Merge branch 'fix-policy-cache-regression-tests' into 'master'
When using the system parser ${parser_config} will be empty and so if this
is unconditionally passed as an argument to the parser then this gets in
interpreted as the name of a file to be compiled and hence the parser just
prints:
File not found, skipping...
File not found, skipping...
File not found, skipping...
...
for all the aa_policy_cache tests - instead fix this to pass a single args
argument to the parser which will only include parser_config if it is not
empty.
This fixes the regression tests to run to completion without error when
USE_SYSTEM=1 is set.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/782
Signed-off-by: Alex Murray <alex.murray@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
When using the system parser ${parser_config} will be empty and so if this
is unconditionally passed as an argument to the parser then this gets in
interpreted as the name of a file to be compiled and hence the parser just
prints:
File not found, skipping...
File not found, skipping...
File not found, skipping...
...
for all the aa_policy_cache tests - instead fix this to pass a single args
argument to the parser which will only include parser_config if it is not
empty.
This fixes the regression tests to run to completion without error when
USE_SYSTEM=1 is set.
Signed-off-by: Alex Murray <alex.murray@canonical.com>
libapparmor performs a test for the new stacking interface, however
how it does this test is problematic as it requires all confined
tasks to be given read access to the task introspection interface.
This results in tasks needing to be given read access to the interface
even if they don't need it. Making it possible for tasks to discover
their confinement even if they are not supposed to be able to.
Instead change the check to using state on the parent directory.
This will generate a getattr request instead of read and make it
on the directory instead of on any interface file that could be
used to obtain information.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/713
Signed-off-by: John Johansen john.johansen@canonical.com
Acked-by: Timeout
Adjust the interface check and fallback. Unfortunately there is no
solution that will fix all failure cases. Instead try to minimize
the failure cases and bias towards failures that don't cause a
regression under an old parser/policy.
Note: In cases where we absolutely know the interface should not
be accessed fail those accesses imediately instead of relying
on what ever LSM active to handle it.
While we are at it document the interfaces and failure cases.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/150
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/713
Signed-off-by: John Johansen <john.johansen@canonical.com>
The parameter that is landing upstream in "available" not
"private_enabled".
Also set the correct variable, as previously we were not.
Note: that skipping checking available for the private apparmor
proc interfaces is okay, as the dedicated apparmor interfaces will
fail correctly if available is False.
This just gives a clear way for userspace to query this info without
having to resort to error codes that access to the private interfaces
would return.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/150
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/713
Signed-off-by: John Johansen <john.johansen@canonical.com>
libapparmor performs a test for the new stacking interface, however
how it does this test is problematic as it requires all confined
tasks to be given read access to the task introspection interface.
This results in tasks needing to be given read access to the interface
even if they don't need it. Making it possible for tasks to discover
their confinement even if they are not supposed to be able to.
Instead change the check to using stat on the parent directory.
This will generate a getattr request instead of read and make it
on the directory instead of on any interface file that could be
used to obtain information.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/150
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/713
Signed-off-by: John Johansen <john.johansen@canonical.com>
This resolves an issue in the parser's job handling when running on a machine with >8 CPU cores. The test library was updated to resolve failures in the caching tests caused by the features directory entries being unsorted in the tests.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/775
Acked-by: John Johansen [john@jjmx.net](mailto:john@jjmx.net)
Currently for directory includes the directory timestamp is ignored.
This is wrong as operations like removing a file from the dir won't
be considered in the timestamp check.
Fix this by updating the timestamp check to include the included
directories timestamp.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/760
Signed-off-by: John Johansen <john@jjmx.net>
Acked-by: Georgia Garcia <georgia.garcia@canonical.com>
add abstractions/crypto, allowing reading @{etc_ro}/gcrypt/random.conf r, and move several rules around
See merge request apparmor/apparmor!772
Acked-by: John Johansen <john.johansen@canonical.com>
crypto allows reading /etc/gcrypt/random.conf, which is possibly needed
for all programs that use libgcrypt.
Reported by darix, he has seen it with vivaldi.
If a profile uses features not supported by the tools yet, add a
skiplist to (hopefully temporarily) exclude it from the tests.
This is meant to avoid blocking usage of new features in profiles.
When doing a release, the skip lists should be empty.
The added test makes sure that the python code can parse all profiles
shipped with AppArmor. If this fails, read_profiles() /
read_inactive_profiles() will raise an exception.
Checking for the number of read profiles is mostly done to ensure
*something* is read (to make sure an empty or non-existing directory
won't make the test useless).
We sometimes have random coverage changes that are not reproducible and
therefore hard to debug.
Generate html coverage as part of make coverage-regression, and keep the
resulting utils/test/htmlcov/ as artifact to make debugging easier.
coverage-html needs JS files from various libjs-* packages, install them
in before_script
The file query test on query_label.sh fails on kernels
before 4.4 because of the lack of support. Since there
is no feature file to examine for this feature, we
need to check for the kernel version.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
The regression tests are failing on some older kernels due to
commit 9f834ec18defc369d73ccf9e87a2790bfa05bf46 being cherry-picked
back to them without the corresponding apparmor patch
34c426acb75cc21bdf84685e106db0c1a3565057.
This means we can not rely on a simple features/flag check to determine
how the kernel is behaving with regard to mmap. Since this test is
not concerned with testing mmap, instead of adding a more complex
conditional simplify by always adding the m permission.
Fixes: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1830984
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Georgia Garcia <georgia.garcia@canonical.com>
ask_exec still uses aa[profile][hat], therefore
- use full_profile when accessing hashlog
- correctly split the merged profile name to profile and hat
- avoid accidently initializing non-existing aa[profile][hat]
This fixes a regression from converting lots of code to use flat
profile//hat array keys.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/763
Acked-by: John Johansen <john@jjmx.net>
See https://gitlab.com/redhat-crypto/fedora-crypto-policies for details.
Reported by darix and also my own audit.log - the actual denial was for
/usr/share/crypto-policies/DEFAULT/openssl.txt.
Also allow the /etc/crypto-policies/ counterpart.
(I'm aware that the crypto policies are not really certificates, but
since they are used by several crypto libraries, ssl_certs is probably
the best place for them even if the filename doesn't match.)
ask_exec still uses aa[profile][hat], therefore
- use full_profile when accessing hashlog
- correctly split the merged profile name to profile and hat
- avoid accidently initializing non-existing aa[profile][hat]
This fixes a regression from converting lots of code to use flat
profile//hat array keys.
The check-logprof test in the profiles Makefile specifies
the configuration directory as --configdir ../utils/test,
but when aa-logprof looks for severity.db in the configdir,
it cannot find it.
This fix points the configdir to utils. Note that the
logprof.conf on utils uses the configuration for files
created during the libapparmor installation on the system.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/177
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
All the calling code (directly or indirectly) uses write_flags=True,
therefore drop the parameter to simplify the code.
See the individual commits for details. Also, reviewing the individual commits is probably easier than reviewing the full diff (especially the test changes).
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/759
Acked-by: John Johansen <john@jjmx.net>
Drop unused write_flags parameter from AaTest_get_header and
AaTest_get_header_01. This is a cleanup for the previous commit.
While on it, add xattrs parameter to AaTest_get_header, and add two
tests with non-empty xattrs.
All the calling code (directly or indirectly) uses write_flags=True,
therefore drop the parameter to simplify the code.
A few tests called get_header() with write_flags=False. Adjust or drop
those tests.
Note: to keep the diff readable, the test changes are as small as
possible. The next commit will cleanup the now-superfluous write_flags
values in the tests.
... and into parse_profile_start_line() (which is used by
ProfileStorage.parse()).
With this change, the section handling RE_PROFILE_HAT_DEF in
parse_profile_data() becomes superfluous.
A nice side effect is that two simple_tests parse failures get
accidently ;-) fixed.
Also preserve 'hat' keyword in ProfileStorage instead of always writing hats as '^hat'.
When writing a profile, prepending '^' or 'hat' to a hat name moves from
aa.py write_piece() to ProfileStorage.get_header().
Finally, extend cleanprof_test.* with 'hat bar {...}'.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/756
Acked-by: John Johansen <john@jjmx.net>
... instead of always writing hats as '^hat'.
When writing a profile, prepending '^' or 'hat' to a hat name moves from
aa.py write_piece() to ProfileStorage.get_header().
Also extend cleanprof_test.* with 'hat bar {...}'.
... and into parse_profile_start_line() (which is used by
ProfileStorage.parse()).
With this change, the section handling RE_PROFILE_HAT_DEF in
parse_profile_data() becomes superfluous.
A nice side effect is that two simple_tests parse failures get
accidently ;-) fixed.
The 'profile' flag means "this profile is a profile or a child profile, but not a hat". Since that's true for most cases, rename the flag to 'is_hat'.
Note that `'profile' == True` translates to `'is_hat' == False`
Also adjust all code to switch from 'profile' to 'is_hat'.
Further down the commit list,
* move parse_profile_start{,_to_storage}() into ProfileStorage
* merge parse_profile_start() into parse()
* add some missing tests
See the individual commits for details.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/754
Acked-by: John Johansen <john@jjmx.net>
... and add some tests for other error conditions that don't imply
nested childs, so that the intended failure gets tested.
(This is probably a leftover of the `hat == profile` -> `hat = None`
(while not in a hat/child profile) change.)
... and make them class functions of ProfileStorage.
parse_profile_start_to_storage() gets renamed to parse().
Also move the tests for parse_profile_start() and
parse_profile_start_to_storage() to test-profile-storage.py.
The 'profile' flag means "this profile is a profile or a child profile,
but not a hat". Since that's true for most cases, rename the flag to
'is_hat'.
Note that `'profile' == True` translates to `'is_hat' == False`
Also adjust all code to switch from 'profile' to 'is_hat'.
This value is True if we are in a child profile (not: hat), but that's information we get "for free", so there's no need to hand it around. Besides that, it was wrongly set to False for main profiles (which are not hats).
Remove the pps_set_profile return value from parse_profile_start(), and always assume True unless we were parsing a hat. For completeness, explicitely set it to False when parsing a hat.
To make sure child profiles and hats don't get mixed up, add a child profile to cleanprof_test.{in,out}.
test-libapparmor-test_multi.py always interpreted foo//bar as being a hat, therefore explicitely mark them as such. (Technically not really needed since this is the default, but it helps to make things clear.)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/751
Acked-by: John Johansen <john@jjmx.net>
This value is True if we are in a child profile (not: hat), but that's
information we get "for free", so there's no need to hand it around.
Besides that, it was wrongly set to False for main profiles (which are
not hats).
Remove the pps_set_profile return value from parse_profile_start(), and
always assume True unless we were parsing a hat. For completeness,
explicitely set it to False when parsing a hat.
To make sure child profiles and hats don't get mixed up, add a child
profile to cleanprof_test.{in,out}.
test-libapparmor-test_multi.py always interpreted foo//bar as being
a hat, therefore explicitely mark them as such. (Technically not really
needed since this is the default, but it helps to make things clear.)
The name var is being improperly used in a warning. Not only is
it being used after it is freed, it also never had the correct value
as the "name" variable contained the value being used as the base
attachment.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: time out
Profile includes can be setup to loop and expand in a pathalogical manner that causes build failures. Fix this by caching which includes have already been seen in a given profile context.
In addition this can speed up some profile compiles, that end up re-including common abstractions. By not only deduping the files being included but skipping the need to reprocess and dedup the rules within the include.
Fixes: https://bugzilla.suse.com/show_bug.cgi?id=1184779
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/743
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Change the tools to use merged profile names (`var['foo//bar']`) instead of the profile/hat layout (`var[profile][hat]`) in many places. Also storage gets moved to ProfileList instead of using a hasher.
Already changed places (in this MR) are parsing profiles, writing profiles, handling and storing of extra profiles, log handling and asking the user about profile additions.
Remaining usage of the `var[profile][hat]` layout are the `aa` and `original_aa` hashers, they'll be replaced in a separate MR.
See the individual commits for details. I'd also recommend to do the review on the individual commits, because the big diff is probably unreadable ;-)
While this is a big chain of changes, each commit contains working code, converting between the two storage layouts with `split_to_merged()` and `merged_to_split()` as needed, with merged layout "bubbling up" in more and more functions.
The long-term goal of these changes is to enable support for nested child profiles in the tools, but - one step after the other ;-)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/736
Acked-by: John Johansen <john.johansen@canonical.com>
The function decides on the filename of a profile, therefore use
'filename' as variable name instead of the somewhat confusing 'profile'
and 'full_profilename'.
When building the parser with DEBUG=1 enabled the build fails with
the following error and warnings
In file included from parser_main.c:47:0:
parser_main.c: In function ‘void auto_tune_parameters()’:
parser_main.c:1421:35: error: ‘estimate_jobs’ was not declared in this scope
PDEBUG("Auto tune: --jobs=%d", estimate_jobs);
^
parser.h:201:37: note: in definition of macro ‘PDEBUG’
fprintf(stderr, "parser: " fmt, ## args); \
^~~~
parser_main.c:1421:35: note: suggested alternative: ‘estimated_jobs’
PDEBUG("Auto tune: --jobs=%d", estimate_jobs);
^
parser.h:201:37: note: in definition of macro ‘PDEBUG’
fprintf(stderr, "parser: " fmt, ## args); \
^~~~
parser.h:201:41: warning: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘long int’ [-Wformat=]
fprintf(stderr, "parser: " fmt, ## args); \
^
parser_main.c:1428:5: note: in expansion of macro ‘PDEBUG’
PDEBUG("Auto tune: --jobs=%d", jobs);
^~~~~~
Makefile:234: recipe for target 'parser_main.o' failed
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/745
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
If an include file includes itsself (for example if local/foo has
'#include <local/foo>'), print a warning instead of calling
load_include() again and again.
This fixes a crash when hitting such a case:
RecursionError: maximum recursion depth exceeded while calling a Python object
Fixes: https://bugzilla.suse.com/show_bug.cgi?id=1184779 for the tools.
The parser will also need a fix.
if a profile doesn't have an attachment specified and the profile name
starts with '/', set the attachment to the profile name. This allows to
have one add_profile() call instead of two very similar ones.
exit early if profile_data is empty (which means we did read an empty
file). This allows to simplify the if conditions to "if active_profile:"
and "else:".
... and adjust all callers and the tests.
For bonus points ;-) this also removes a hasher usage, and extends the
test to check that only the expected profile gets created.
... instead of the 'extras' hasher.
Also adjust all code that previously used 'extras' to use
'extra_profiles'. This affects get_profile() and read_profile().
Add a prof_storage parameter to add_profile() to hand over the actual
profile data/rules as ProfileStorage.
Also adjust several tests to hand over a (dummy) ProfileStorage object.
Note: For now, the parameter is optional because it needs some more changes
in aa.py to be really useable. This will change in a later commit.
... instead of converting log_dict to traditional [profile][hat] layout
in do_logprof_pass().
A nice side effect is that we get sorting the main profile before its
hats for free and can remove the sorting code.
Also update a comment in ask_rule_questions().
Finally, adjust aa-mergeprof so that it hands over a merged log_dict (using
split_to_merged())
... instead of the old [profile][hat] structure.
This needs changes in do_logprof_pass() when calling ask_the_questions()
(using merged_to_split() for now).
Also adjust test-libapparmor-test_multi.py logfile_to_profile() to
expect the merged structure.
... and convert them back to the [profile][hat] layout at the end so
that callers still get the expected result.
As a side effect, log_dict no longer needs to be a hasher().
... instead of the old [profile][hat] structure.
This needs changes in read_profile() (now using the merged profile name)
and attach_profile_data() (using merged_to_split() for now).
Also adjust test-aa.py to expect the merged structure.
Change parse_profile_data() to internally use merged profile names
(`foo//bar`) instead of separate profile and hat, and only split it up
again to the [profile][hat] layout at the very end with
merged_to_split().
A nice side effect is that we get rid of a hasher() usage.
parse_profile_data() also gets changed to use `hat = None` (instead of
`hat = profile`) if not inside a child profile. As a result,
parse_profile_start() and one of its tests need a small change.
Besides that small change, calling code should not see a difference, and
the tests also stay working.
the video abstraction currently it only contains the following rules:
@{sys}/class/video4linux r,
@{sys}/class/video4linux/** r,
Judging by the v4l path, this abstraction should be used whenever some
app wants to use for instance a webcam or other USB cameras to stream
video usually in chat apps. I was testing some apps, and it looks like
the following rules are needed to make the video streaming possible:
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/159
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/740
Signed-off-by: John Johansen <john.johansen@canonical.com>
... by adding some new tests, and by marking two lines as "pragma: no branch" because I didn't find a testcase that doesn't let them continue with the next line.
Finally, remove severity.py from the "not 100% covered" list in test/Makefile.
Also run severity tests with the official severity.db instead of the slightly outdated copy in test/.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/737
Acked-by: John Johansen <john.johansen@canonical.com>
... by adding some new tests, and by marking two lines as "pragma: no
branch" because I didn't find a testcase that doesn't let them continue
with the next line.
Finally, remove severity.py from the "not 100% covered" list in
test/Makefile.
Add tests with invalid type to ensure error handling works as expected.
Merge branch 'cboltz/cboltz-profile-storage-tests'
[Fixed conflict with prior change to utils/test/test-profile-storage.py]
Acked-by: Steve Beattie <steve@nxnw.org>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/735
Merge branch 'cboltz-move-write_header' into 'master'
As a preparation, change change_profile_flags() to use ProfileStorage,
and replace some regex usage with startswith() to make the code easier
to read and (probably) more performant.
See the individual commits for details.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/734
Acked-by: Steve Beattie <steve@nxnw.org>
Also adjust the calling code to use get_header() instead of
write_header().
Finally, move the tests to test-profile-storage.py and do a few
adjustments needed by the change. Part of these adjustments is to hand
over empty params with the correct type instead of just "None".
... instead of profile_data[profile][profile]['name'] which is not
always correct.
Note: setting 'name' will be fixed in a later commit, but don't hold
your breath for it ;-)
AUDIT events get skipped when parsing the log (they are not relevant for
updating a profile), therefore stop returning an always-empty AUDIT
section when reading the log.
dhclient wants to set its thread names to functional names for
introspection purposes. Eg.
$ pstree -at 3395
dhclient ens3
├─{isc-socket}
├─{isc-timer}
└─{isc-worker0000}
When denied this can result in dhclient breaking and failing to obtain
IPv4 addresses.
Fixes: https://bugs.launchpad.net/ubuntu/+source/isc-dhcp/+bug/1918410
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Doing so adds the $ac_tool_prefix during cross compilation and will end up using the correct, architecture-dependent python-config.
This is the second and last upstreamable change from https://bugs.debian.org/984582. It looks a little simpler here, because apparmor evolved upstream compared to the Debian version. Fortunately, it got a lot simpler in the process.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/729
Acked-by: John Johansen <john.johansen@canonical.com>
Doing so adds the $ac_tool_prefix during cross compilation and will end
up using the correct, architecture-dependent python-config.
Link: https://bugs.debian.org/984582
AC_CHECK_FILE is meant to check for host files and therefore fails hard during cross compilation unless one supplies a cached check result. Here we want to know about the presence of a build system file though, so AC_CHECK_FILE is the wrong tool.
Currently the wutmp abstraction has the following rules:
/var/log/lastlog rwk,
/var/log/wtmp wk,
@{run}/utmp rwk,
According to what I see in my apparmor profiles, just a few apps want
to interact with the files listed above, especially with the
/var/log/wtmp . But when the apps do this, they sometimes want the
read access to this file. An example could be the last command. Is
there any reason for not having the r in the rule? The second thing
is the file /var/log/btmp (which isn't included in the
abstracion). Whenever I see an app, which wants to access the
/var/log/wtmp file, it also tries to interact with the /var/log/btmp
file, for instance lightdm/sddm or su . Most of the time they need
just wk permissions, but sometimes apps need also r on this file, an
example could be the lastb command, which is just a link to last.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/152
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/724
Signed-off-by: John Johansen <john.johansen@canonical.com>
Rule downgrades are used to provide some confinement when a feature
is only partially supported by the kernel.
Eg. On a kernel that doesn't support fine grained af_unix mediation
but does support network mediation.
unix (connect, receive, send)
type=stream
peer=(addr="@/tmp/.ICE-unix/[0-9]*"),
will be downgraded to
network unix type=stream,
Which while more permissive still provides some mediation while
allowing the appication to still function. However making the rule
a deny rule result in tightening the profile.
Eg.
deny unix (connect, receive, send)
type=stream
peer=(addr="@/tmp/.ICE-unix/[0-9]*"),
will be downgraded to
deny network unix type=stream,
and that deny rule will take priority over any allow rule. Which means
that if the profile also had unix allow rules they will get blocked by
the downgraded deny rule, because deny rules have a higher priority,
and the application will break. Even worse there is no way to add the
functionality back to the profile without deleting the offending deny
rule.
To fix this we drop deny rules that can't be downgraded in a way that
won't break the application.
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1180766
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/700
Signed-off-by: John Johansen <john.johansen@canonical.com>
This is done by introducing a match_line_against_rule_classes() function to get of repeating code in parse_profile_data().
A side effect is a better separation of preamble and profile data, which needed changes at several places - especially include rules in the preamble were funny\[tm\].
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/719
Acked-by: John Johansen <john.johansen@canonical.com>
This needed several changes because so far data for all includes was
stored in include[]. However, preamble data for everything else gets
stored in active_profiles (and with this commit, preamble includes also
get stored in active_profiles).
The needed changes to store preamble includes in active_profiles are:
* include_list_recursive(): add and honor in_preamble flag
* add this flag at several places calling include_list_recursive() for
a preamble
* parse_profile_data(): call active_profiles.init_file() for all files.
Before, empty/comment-only files weren't (indirectly) added to
active_profiles because none of the add_$ruletype functions was
called, which could lead to KeyErrors for comment-only preamble include
files (prevented by the now-obsolete and removed check in
get_all_merged_variables()).
For this, we have to hand over in_preamble, and to do a slightly
different handling for preamble and profile rules.
For adding preamble rules to ProfileList, add a add_rule() function that
accepts the rule type as parameter. This is easier than using one of the
add_$type functions depending on the rule type.
With this change, match_line_against_rule_classes() handles nearly all
rule types that have a *Rule class, with the exception of include rules
which need some additional work.
in_preamble keeps track of the current parsing position.
It's True while parsing the preamble of a profile file, and when loading
an include file that is included in the preamble (typically tunables/*).
While inside a profile or parsing abstractions/*, it is False.
This commit only hands the information around and keeps in_preamble
updated, but it doesn't really get used yet.
Also adjust the tests to hand over the additional parameter to
parse_profile_data().
... to handle parsing of lines that are managed with a *Rule
This needs 'ruletypes' to also know about *Rule, not only about
*Ruleset.
Also switch over handling of most rules that live inside a profile from
parse_profile() to match_line_against_rule_classes() to make
parse_profile() more readable.
Exceptions are:
- include and abi, which can also exist in the preamble
- file rules, because they'd need to be handled later (after variable
definitions)
Update postfix profiles:
* cleanup postfix profiles - /etc/postfix/\*.db is covered by abstractions/postfix-common
* postfix: allow access to \*.lmdb files in addition to \*.db files. (openSUSE Tumbleweed now uses the lmdb format by default.)
* postfix-flush and -showq: add permissions needed with latest postfix as seen on openSUSE Tumbleweed
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/717
Acked-by: John Johansen <john.johansen@canonical.com>
Also split out parts of the function into loadincludes_dir() to keep the code readable.
Note: This change might affect the list of includes proposed by aa-logprof.
Also drop is_skippable_dir()
Since loadincludes() now only loads a specified list of subdirectories, we no longer need a directory blacklist.
The only possibly remaining part are .git subdirectories (for example tunables/.git or abstractions/.git). Since it's very unlikely that someone would have only a subdirectory of /etc/apparmor.d/ in git, drop that check.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/714
Acked-by: John Johansen <john.johansen@canonical.com>
Since loadincludes() now only loads a specified list of subdirectories,
we no longer need a directory blacklist.
The only possibly remaining part are .git subdirectories (for example
tunables/.git or abstractions/.git). Since it's very unlikely that
someone would have only a subdirectory of /etc/apparmor.d/ in git, drop
that check.
Also split out parts of the function into loadincludes_dir() to keep the
code readable.
Note: This change might affect the list of includes proposed by
aa-logprof.
The dynamic_cast operator is slow as it needs to look at RTTI information and even does some string comparisons, especially in deep hierarchies. Profiling with callgrind showed that dynamic_cast can eat a huge portion of the running time, as it takes most of the time that is spent in the simplify_tree() function. For some complex profiles, the number of calls to dynamic_cast can be in the range of millions.
This commit replaces the use of dynamic_cast in the Node hierarchy with a method called is_type(), which returns true if the pointer can be casted to the specified type. It works by looking at an Node object field that is an integer with bits set for each type up in the hierarchy. Therefore, dynamic_cast is replaced by a simple bits operation.
In my tests, for complex profiles the improvement in speed even made running apparmor_parser with "-O no-expr-simplify" slower that when simplifying, apparently because the smaller trees obtained after the expression simplification require less calls to DFA::update_state_transitions(), and that compensates the now significantly slower time spent in simplify_tree(). This opens the door to maybe avoid "-O no-expr-simplify" in the snapd daemon, thus allowing faster run-time checks in the kernel.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/711
Acked-by: John Johansen <john.johansen@canonical.com>
On some systems the build of the parser is spitting out
cc: fatal error: no input files
compilation terminated.
This is being caused by the REALLOCARRAY checkfailing due to cpp trying
to check for both input and output files and not correctly falling
back to stdin/stdout if infile and outfile aren't specified.
Fix this by being explicit that infile and outfile are supposed to
use stdin and stdout.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/712
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
The dynamic_cast operator is slow as it needs to look at RTTI
information and even does some string comparisons, especially in deep
hierarchies like the one for Node. Profiling with callgrind showed
that dynamic_cast can eat a huge portion of the running time, as it
takes most of the time that is spent in the simplify_tree()
function. For some complex profiles, the number of calls to
dynamic_cast can be in the range of millions.
This commit replaces the use of dynamic_cast in the Node hierarchy
with a method called is_type(), which returns true if the pointer can
be casted to the specified type. It works by looking at a Node object
field that is an integer with bits set for each type up in the
hierarchy. Therefore, dynamic_cast is replaced by a simple bits
operation.
This change can reduce the compilation times for some profiles more
that 50%, especially in arm/arm64 arch. This opens the door to maybe
avoid "-O no-expr-simplify" in the snapd daemon, as now that option
would make the compilation slower in almost all cases.
This is the example profile used in some of my tests, with this change
the run-time is around 1/3 of what it was before on an x86 laptop:
profile "test" (attach_disconnected,mediate_deleted) {
dbus send
bus={fcitx,session}
path=/inputcontext_[0-9]*
interface=org.fcitx.Fcitx.InputContext
member="{Close,Destroy,Enable}IC"
peer=(label=unconfined),
dbus send
bus={fcitx,session}
path=/inputcontext_[0-9]*
interface=org.fcitx.Fcitx.InputContext
member=Reset
peer=(label=unconfined),
dbus receive
bus=fcitx
peer=(label=unconfined),
dbus receive
bus=session
interface=org.fcitx.Fcitx.*
peer=(label=unconfined),
dbus send
bus={fcitx,session}
path=/inputcontext_[0-9]*
interface=org.fcitx.Fcitx.InputContext
member="Focus{In,Out}"
peer=(label=unconfined),
dbus send
bus={fcitx,session}
path=/inputcontext_[0-9]*
interface=org.fcitx.Fcitx.InputContext
member="{CommitPreedit,Set*}"
peer=(label=unconfined),
dbus send
bus={fcitx,session}
path=/inputcontext_[0-9]*
interface=org.fcitx.Fcitx.InputContext
member="{MouseEvent,ProcessKeyEvent}"
peer=(label=unconfined),
dbus send
bus={fcitx,session}
path=/inputcontext_[0-9]*
interface=org.freedesktop.DBus.Properties
member=GetAll
peer=(label=unconfined),
dbus (send)
bus=session
path=/org/a11y/bus
interface=org.a11y.Bus
member=GetAddress
peer=(label=unconfined),
dbus (send)
bus=session
path=/org/a11y/bus
interface=org.freedesktop.DBus.Properties
member=Get{,All}
peer=(label=unconfined),
dbus (receive, send)
bus=accessibility
path=/org/a11y/atspi/**
peer=(label=unconfined),
dbus (send)
bus=system
path=/org/freedesktop/Accounts
interface=org.freedesktop.DBus.Introspectable
member=Introspect
peer=(label=unconfined),
dbus (send)
bus=system
path=/org/freedesktop/Accounts
interface=org.freedesktop.Accounts
member=FindUserById
peer=(label=unconfined),
dbus (receive, send)
bus=system
path=/org/freedesktop/Accounts/User[0-9]*
interface=org.freedesktop.DBus.Properties
member={Get,PropertiesChanged}
peer=(label=unconfined),
dbus (send)
bus=session
interface=org.gtk.Actions
member=Changed
peer=(name=org.freedesktop.DBus, label=unconfined),
dbus (receive)
bus=session
interface=org.gtk.Actions
member={Activate,DescribeAll,SetState}
peer=(label=unconfined),
dbus (receive)
bus=session
interface=org.gtk.Menus
member={Start,End}
peer=(label=unconfined),
dbus (send)
bus=session
interface=org.gtk.Menus
member=Changed
peer=(name=org.freedesktop.DBus, label=unconfined),
dbus (send)
bus=session
path="/com/ubuntu/MenuRegistrar"
interface="com.ubuntu.MenuRegistrar"
member="{Register,Unregister}{App,Surface}Menu"
peer=(label=unconfined),
}
in_contained_hat is needed to know if we are already in a profile or
not. (Simply checking if we are in a hat doesn't work, because something
like "profile foo//bar" will set profile and hat at once, and later
(wrongfully) expect another "}".
However, the way how this variable was set became too complicated.
To simplify the code, set in_contained_hat directly in
parse_profile_data() RE_PROFILE_START instead of returning it via
parse_profile_start() and parse_profile_start_to_storage()
Since this change removes a return value from two functions, also adjust
the tests accordingly.
parse_profile_start_to_storage() converts the result of
parse_profile_start() into a ProfileStorage object.
No functional change, but parse_profile_data() becomes more readable.
Also extend tests to cover parse_profile_start_to_storage().
My main user account is managed by systemd-homed. When I enable
AppArmor and have nscd running, I get inconsistent behavior with my
user account - sometimes I can't log in, sometimes I can log in but
not use sudo, etc.
This is the output of getent passwd:
$ getent passwd
root❌0:0::/root:/usr/bin/zsh
bin❌1:1::/:/sbin/nologin
daemon❌2:2::/:/sbin/nologin
mail❌8:12::/var/spool/mail:/sbin/nologin
ftp❌14:11::/srv/ftp:/sbin/nologin
http❌33:33::/srv/http:/sbin/nologin
nobody❌65534:65534:Nobody:/:/sbin/nologin
dbus❌81:81:System Message Bus:/:/sbin/nologin
[...]
rose❌1000:1000:Rose Kunkel:/home/rose:/usr/bin/zsh
But getent passwd rose and getent passwd 1000 both return no output.
Stopping nscd.service fixes these problems. Checking the apparmor
logs, I noticed that nscd was denied access to
/etc/machine-id. Allowing access to that file seems to have fixed the
issue.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/707
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/145
Signed-off-by: John Johansen <john.johansen@canonical.com>
Ubuntu 18.04, Firefox 60.0.1+build2-0ubuntu0.18.04.1
Running firefix, then going to netflix.com and attempting to play a
movie. The widevinecdm plugin crashes, the following is found in
syslog:
Jun 15 19:13:22 xplt kernel: [301351.553043] audit: type=1400 audit(1529046802.585:246): apparmor="DENIED" operation="file_mmap" profile="/usr/lib/firefox/firefox{,*[^s][^h]}" name="/home/xav/.mozilla/firefox/wiavokxk.default-1510977878171/gmp-widevinecdm/1.4.8.1008/libwidevinecdm.so" pid=16118 comm="plugin-containe" requested_mask="m" denied_mask="m" fsuid=1000 ouid=1000
Jun 15 19:13:22 xplt kernel: [301351.553236] audit: type=1400 audit(1529046802.585:247): apparmor="DENIED" operation="ptrace" profile="/usr/lib/firefox/firefox{,*[^s][^h]}" pid=24714 comm="firefox" requested_mask="trace" denied_mask="trace" peer="/usr/lib/firefox/firefox{,*[^s][^h]}"
Jun 15 19:13:22 xplt kernel: [301351.553259] plugin-containe[16118]: segfault at 0 ip 00007fcdfdaa76af sp 00007ffc1ff03e28 error 6 in libxul.so[7fcdfb77a000+6111000]
Jun 15 19:13:22 xplt snmpd[2334]: error on subcontainer 'ia_addr' insert
...
Fixes: https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/1777070
Reported-by: Xav Paice <xav.paice@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/684
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
job scaling allows the parser to resample the number of cpus available
and increase the number of jobs that can be launched if cpu available
increases.
Unfortunately job scaling was being applied even when a fixed number
of jobs was specified. So
--jobs=2
doesn't actually clamp the compile at 2 jobs.
Instead job scaling should only be applied when --jobs=auto or when
jobs are set to a multiple of the cpus.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/703
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
The parsers default settings can OOM smaller special use systems
when building or loading policy. Use basic memory info and cpus to
tune the parser for lower resource environments.
Currently this just sets the jobs parameters if the default values
haven't been modified by user config or parameters. But in the
future this could add cache control and compile parameters.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/702
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
... to find regressions or improvements in the python code coverage.
`make coverage-regression` will error out if a file looses its 100% coverage, or if a file improved to 100% coverage.
Other coverage changes (for example 45% -> 47%) will be ignored.
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/697
Acked-by: John Johansen <john.johansen@canonical.com>
- On Arch Linux certificates are extracted to /etc/ca-certificates/ by the update-ca-trust script.
- /etc/libressl/ is used by Arch Linux's libressl package.
- Combine rules to reduce number of lines.
There is currently a case in which proc_attr_base won't get set when asprintf is able to generate the path, but the file doesn't exist, it will exit proc_attr_base_init_once() without proc_attr_base having been set as the fall-through if/else logic will get bypassed when asprintf is successful.
Without this fix, various commands like aa-status will not properly display which processes have an apparmor profile enforced because it proc_attr_base will always be NULL and therefore the proc attr path won't be able to be generated.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/701
Acked-by: John Johansen <john.johansen@canonical.com>
There is currently a case in which proc_attr_base won't get set when
asprintf is able to generate the path, but the file doesn't exist, it
will exit proc_attr_base_init_once() without proc_attr_base having been
set as the fall-through if/else logic will get bypassed when asprintf is
successful.
... to find regressions or improvements in the python code coverage.
`make coverage-regression` will error out if a file looses its 100%
coverage, or if a file improved to 100% coverage.
Other coverage changes (for example 45% -> 47%) will be ignored.
Add the BooleanRule and BooleanRuleset classes, add handling of boolean variable definitions in ProfileList and adjust `parse_profile_data()` to use BooleanRule. As usual, add tests for the added code.
See the individual commits for the details.
Note that this MR is also a bugfix - the previous code in (3.0 and master) saved boolean variables at a wrong place, and they were silently lost when writing the profile.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/693
Acked-by: John Johansen <john.johansen@canonical.com>
... and save rules at the right place (ProfileList) where they actually
get written when writing the profile.
This is also a bugfix - the previous code saved boolean variables at a
wrong place, and they were silently lost when writing the profile.
Extend cleanprof_test.{in,out} to ensure that this doesn't break again.
Also remove boolean_bad_[2-4] from the test-parser-simple-tests.py
exception_not_raised list because these test profiles now get correctly
detected as invalid.
These two classes are meant to handle the definition of boolean rules
like `$foo = true`.
Also extend RE_PROFILE_BOOLEAN to provide named matches.
As usual, add tests for the new classes.
libraries/libapparmor/swig/python/Makefile.am: Add global LDFLAGS when building the python library. When only applying the custom PYTHON_LDFLAGS (which are in fact `python-config --ldflags`) distributions are unable to build the library with e.g. full RELRO.
Closes#129
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/689
Acked-by: John Johansen <john.johansen@canonical.com>
libraries/libapparmor/swig/python/Makefile.am:
Add global LDFLAGS when building the python library.
When only applying the custom PYTHON_LDFLAGS (which are in fact
`python-config --ldflags`) distributions are unable to build the library
with e.g. full RELRO.
Fixes#129
Related to #138
libapparmor on startup does detection of whether the new stacking
proc interfaces are available and then store a var for which interface
should be used. This avoids libapparmor needing to detect which interface
to use on each proc based api call.
Unfortunately if the domain is changed on the task via change_hat or
change_profile and the proc interface is used after the domain change
it is possible that access to the interface will be denied by policy.
This is not a problem in and of it self except policy may have been
created assuming the old interface.
Fix this by adding a fallback that tries the old interface if we
are using the new interface by default and the failure was due to
an EACCES denial (policy based).
Also refactor the code a bit so this retry is isolated to one function
instead of adding it in two places.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/131
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/681
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
If aa-notify races file rotation it may crash with a trace back to
the log file being removed before the new one is moved into place.
Traceback (most recent call last):
File "/usr/sbin/aa-notify", line 570, in <module>
main()
File "/usr/sbin/aa-notify", line 533, in main
for message in notify_about_new_entries(logfile, args.wait):
File "/usr/sbin/aa-notify", line 145, in notify_about_new_entries
for event in follow_apparmor_events(logfile, wait):
File "/usr/sbin/aa-notify", line 236, in follow_apparmor_events
if os.stat(logfile).st_ino != log_inode:
FileNotFoundError: [Errno 2] No such file or directory: '/var/log/audit/audit.log'
If we hit this situation sleep and then retry opening the logfile.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/130
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/688
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
The generated files are exactly the same, but the code is a bit more readable.
Additional differences:
* added test_gen_list() to verify the result of gen_list()
* null_target has a non-empty value to avoid that it gets skipped in loops as empty value
* invert_save has an additional entry for ''
* copyright header added (based on git log of gen-xtrans.pl)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/673
Acked-by: John Johansen <john.johansen@canonical.com>
Since this option is mostly meant for testing, it will not show up in `--help`.
`aa-notify` was the only tool that honored the `__AA_CONFDIR` env variable. Drop it in favor of the `--configdir` option.
Note: Since we now pass `confdir=` to `init_aa()` (in most cases `None`), setting the default needs to be moved inside the function.
Also use `--configdir` in the tests.
See the individual commits for details.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/670
Acked-by: John Johansen <john.johansen@canonical.com>
So far, aa-autodep "accidently" loaded the abstractions when parsing the
existing profiles. Obviously, this only worked if there is at least one
profile in the active or extra profile directory.
Without any existing profiles, aa-autodep crashed with
KeyError: '/tmp/apparmor.d/abstractions/base'
Prevent this crash by explicitely loading the abstractions on start.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=1178527#c1 [1]
This is needed to catch conflicts between uppercase and lowercase hotkeys of the same letter, as seen with `(B)enannt` and `A(b)lehnen` in the german utils translations.
Also fix conflicting hotkeys in utils de.po, id.po and sv.po.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/675
Acked-by: John Johansen <john.johansen@canonical.com>
This is needed to catch conflicts between uppercase and lowercase
hotkeys of the same letter, as seen with `(B)enannt` and `A(b)lehnen` in
the german utils translations.
Since this option is mostly meant for testing, it will not show up in
--help.
aa-notify was the only tool that honored the __AA_CONFDIR env variable.
It still does if --configdir is not given.
Note: Since we now pass confdir= to init_aa() (in most cases None),
setting the default needs to be moved inside the function.
The generated files are exactly the same, but the code is a bit more
readable.
Additional differences:
- added test_gen_list() to verify the result of gen_list()
- null_target has a non-empty value to avoid that it gets skipped in
loops as empty value
- invert_save has an additional entry for ''
- copyright header added (based on git log of gen-xtrans.pl)
Dovecot is hit with this denial on Debian 10 (buster):
```
type=AVC msg=audit(1603647096.369:24514): apparmor="DENIED"
operation="open" profile="dovecot" name="/usr/share/dovecot/dh.pem"
pid=28774 comm="doveconf" requested_mask="r" denied_mask="r" fsuid=0
ouid=0
```
This results in fatal error:
```
Oct 25 19:31:36 dovecot[28774]: doveconf: Fatal: Error in configuration
file /etc/dovecot/conf.d/10-ssl.conf line 50: ssl_dh: Can't open file
/usr/share/dovecot/dh.pem: Permission denied
```
Add rule to allow reading dh.pem.
[Fontmatrix](https://github.com/fontmatrix/fontmatrix) [adds \~/.Fontmatrix/Activated to fonts.conf](https://github.com/fontmatrix/fontmatrix/blob/75552e2/src/typotek.cpp#L1081-L1088). This causes programs which use [Fontconfig](https://gitlab.freedesktop.org/fontconfig/fontconfig) (directly or indirectly through libraries such as [Pango](https://pango.gnome.org/)) to include that directory in their font search path, which causes errors such as:
```
audit: type=1400 audit(1602678958.525:53): apparmor="DENIED" operation="open" profile="fr.emersion.Mako" name="/home/username/.Fontmatrix/Activated/.uuid" pid=48553 comm="mako" requested_mask="r" denied_mask="r" fsuid=1000 ouid=1000
audit: type=1400 audit(1602678958.525:54): apparmor="DENIED" operation="open" profile="fr.emersion.Mako" name="/home/username/.Fontmatrix/Activated/" pid=48553 comm="mako" requested_mask="r" denied_mask="r" fsuid=1000 ouid=1000
```
if the program does not explicitly include this directory in its AppArmor profile. As with other common font locations, add `~/.Fontmatrix/Activated` to the fonts abstraction for read-only access.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/657
Acked-by: John Johansen <john.johansen@canonical.com>
Unfortunately in apparmor sockets need rw access. Currently x11 can only work if abstract socket is available and used instead so those restrictions won't trigger. This partially reverts c7b8368216
On arch
make -C profiles check-logprof
fails with
*** Checking profiles from ./apparmor.d against logprof
ERROR: Can't find AppArmor profiles in /etc/apparmor.d
make: *** [Makefile:113: check-logprof] Error 1
make: Leaving directory '/build/apparmor/src/apparmor-2.13.3/profiles'
because /etc/apparmor.d/ is not available in the build environment
and aa-logprofs --dir argument, is not being passed to init_aa()
but used to update profiles_dir after the fact.
Fix this by passing profiledir as an argument to init_aa()
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/36
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/663
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
When run with the -p flag, aa-notify works fine for 100 seconds and then it exits.
I suspect that the issue arises from the following check on line 259 in utils/aa-notify
if debug_logger.debug_level <= 10 and int(time.time()) - start_time > 100:
debug_logger.debug('Debug mode detected: aborting notification emitter after 100 seconds.')
sys.exit(0)
together with line 301 in utils/apparmor/common.py which initializes debug_logger.debug_level to logging.DEBUG which has the numerical value 10.
A simple solution might be to just remove the check as I'm not quit sure why one would want aa-notify to exit when run in debug mode in the first place.
Alternatively, one could check against debug_logger.debugging (initialized to False) or change the initialization of debug_logger.debug_level to something else, but I don't know how that would affect other consumers of utils/apparmor/common.py.
For now just add dbugger_logger.debugging as an additional check as the
reason for timing out after 100s during debugging are unclear.
Suggested-by: vicvbcun
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/126
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/660
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Otto Kekäläinen <otto@kekalainen.net>
This reverts merge request !628. My reason for this proposal is that commit 88c142c6 already provided this change, something I must have missed when I opened the initial merge request. This resulted in duplicate entries in the profile, something that is also potentially confusing to users or other contributors.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/659
Acked-by: John Johansen <john.johansen@canonical.com>
[Fontmatrix] [adds ~/.Fontmatrix/Activated to fonts.conf]. This causes
programs which use [Fontconfig] (directly or indirectly through
libraries such as [pango]) to include that directory in their font
search path, which causes errors such as:
audit: type=1400 audit(1602678958.525:53): apparmor="DENIED" operation="open" profile="fr.emersion.Mako" name="/home/username/.Fontmatrix/Activated/.uuid" pid=48553 comm="mako" requested_mask="r" denied_mask="r" fsuid=1000 ouid=1000
audit: type=1400 audit(1602678958.525:54): apparmor="DENIED" operation="open" profile="fr.emersion.Mako" name="/home/username/.Fontmatrix/Activated/" pid=48553 comm="mako" requested_mask="r" denied_mask="r" fsuid=1000 ouid=1000
if the program does not explicitly include this directory in its
AppArmor profile. As with other common font locations, add
~/.Fontmatrix/Activated to the fonts abstraction for read-only access.
[Fontconfig]: https://gitlab.freedesktop.org/fontconfig/fontconfig
[Fontmatrix]: https://github.com/fontmatrix/fontmatrix
[Pango]: https://pango.gnome.org/
[adds ~/.Fontmatrix/Activated to fonts.conf]: https://github.com/fontmatrix/fontmatrix/blob/75552e2/src/typotek.cpp#L1081-L1088
Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
When using the in tree parser we should not be using the system parser.conf file, as if the system apparmor is newer than the tree being tested the parser.conf file could contain options not understood by the in tree apparmor_parser.
Use --config-file to specify the default in tree parser.conf
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/653
Acked-by: Steve Beattie <steve.beattie@canonical.com>
When using the in tree parser we should not be using the system
parser.conf file, as if the system apparmor is newer than the
tree being tested the parser.conf file could contain options not
understood by the in tree apparmor_parser.
Use --config-file to specify the default in tree parser.conf
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/653
Signed-off-by: John Johansen <john.johansen@canonical.com>
In cross build environments, using the hosts cpp gives incorrect
detection of reallocarray. Change cpp to a variable.
fixes:
parser_misc.c: In function 'int capable_add_cap(const char*, int, unsigned int, capability_flags)':
| parser_misc.c:297:37: error: 'reallocarray' was not declared in this scope
| 297 | tmp = (struct capability_table *) reallocarray(cap_table, sizeof(struct capability_table), cap_table_size+1);
Signed-off-by: Armin Kuster <akuster808@gmail.com>
add limits.h
aa_status.c:269:22: error: 'PATH_MAX' undeclared (first use in this function); did you mean 'AF_MAX'?
| 269 | real_exe = calloc(PATH_MAX + 1, sizeof(char));
Signed-off-by: Armin Kuster <akuster808@gmail.com>
It trys to create a symlink before the man pages are installed.
ln -sf aa-status.8 /(path}/apparmor/3.0-r0/image/usr/share/man/man8/apparmor_status.8
| ln: failed to create symbolic link '{path}/apparmor/3.0-r0/image/usr/share/man/man8/apparmor_status.8': No such file or directory
...
install -d /{path}/apparmor/3.0-r0/image/usr/share/man/man8 ; install -m 644 aa-status.8 /{path}/apparmor/3.0-r0/image/usr/share/man/man8;
Signed-off-by: Armin Kuster <akuster808@gmail.com>
By using assertIn, we test if a given message is contained in the parser error message. This can (and actually does) hide errors if the error message changes outside the checked part.
Change the test to assertEqual to test the full error message, and add '\\n' to all expected error messages to make them still match.
Note: test_modefail() has an outdated error message which went unnoticed so far, and should cause the (updated) tests to fail.
I'll add another commit that adjusts the expected test_modefail() error message as soon as we have an official test failure.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/632
Acked-by: John Johansen <john.johansen@canonical.com>
By using assertIn, we test if a given message is contained in the parser
error message. This can (and actually does) hide errors if the error
message changes outside the checked part.
Change the test to assertEqual to test the full error message, and add
'\n' to all expected error messages to make them still match.
Depending on the kernel version and patches, there can be an additional
message
Cache read/write disabled: interface file missing. (Kernel needs AppArmor 2.4 compatibility patch.)
which will be ignored by the check.
While `include/sys/apparmor.h` makes use of `socklen_t`, it doesn't include the `<sys/socket.h>` header to make its declaration available. While this works on systems using glibc via transitive includes, it breaks compilation on musl libc.
Fix the issue by including the header.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/642
Acked-by: John Johansen <john.johansen@canonical.com>
While `_aa_asprintf` is supposed to be of private visibility, it's used
by apparmor_parser and thus required to be visible when linking. This
commit thus adds it to the list of private symbols to make it available
for linking in apparmor_parser.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
With AppArmor release 3.0, a new function `aa_features_new_from_file`
was added, but not added to the list of public symbols. As a result,
it's not possible to make use of this function when linking against
libapparmor.so.
Fix the issue by adding it to the symbol map.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
While `include/sys/apparmor.h` makes use of `socklen_t`, it doesn't
include the `<sys/socket.h>` header to make its declaration available.
While this works on systems using glibc via transitive includes, it
breaks compilation on musl libc.
Fix the issue by including the header.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
utils/test/test-aa-notify.py:
Change `AANotifyTest.test_entries_since_login()` to be decorated by a
`skipUnless()` checking for existence of **/var/log/wtmp** (similar to
`AANotifyTest.test_entries_since_login_verbose()`).
The test otherwise fails trying to access /var/log/wtmp in environments
where the file is not available.
Fixes#120
2020-10-02 23:58:53 +02:00
376 changed files with 12418 additions and 9050 deletions
B<UNQUOTED FILEGLOB> = (must start with '/' (after variable expansion), B<AARE> have special meanings; see below. May include I<VARIABLE>. Rules with embedded spaces or tabs must be quoted. Rules must end with '/' to apply to directories.)
B<AARE> = B<?*[]{}^>
See section "Globbing (AARE)" below for meanings.
B<ACCESS> = ( 'r' | 'w' | 'a' | 'l' | 'k' | 'm' | I<EXEC TRANSITION> )+ (not all combinations are allowed; see below.)
yyerror(_("failed to find features abi '%s': %m"), filename);
}
if (policy_features) {
if (!aa_features_is_equal(tmp_features, policy_features)) {
pwarn(WARN_ABI, _("%s: %s features abi '%s' differs from policy declared feature abi, using the features abi declared in policy\n"), progname, current_filename, filename);
if (tmp_features) {
if (!aa_features_is_equal(tmp_features, policy_features)) {
pwarn(WARN_ABI, _("%s: %s features abi '%s' differs from policy declared feature abi, using the features abi declared in policy\n"), progname, current_filename, filename);
}
aa_features_unref(tmp_features);
}
aa_features_unref(tmp_features);
} else if (!tmp_features) {
/* skipped reinclude, but features not set */
yyerror(_("failed features abi not set but include cache skipped\n"));
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
