Mount has regressed in two ways. That are affecting snapd confinement, since landing the mount fixes for CVE-2016-1585 in 3.1.4 and the fix for the mount change type regression in 3.1.5
Bug Reports:
https://bugs.launchpad.net/apparmor/+bug/2023814https://bugzilla.opensuse.org/show_bug.cgi?id=1211989
Issue 1:
Denial of Mount
```
[ 808.531909] audit: type=1400 audit(1686759578.010:158): apparmor="DENIED" operation="mount" class="mount" info="failed mntpnt match" error=-13 profile="snap-update-ns.test-snapd-lp-1803535" name="/tmp/.snap/etc/" pid=14529 comm="5" srcname="/etc/" flags="rw, rbind"
```
when the profile contains a rule that should match
```
mount options=(rw, rbind) "/etc/" -> "/tmp/.snap/etc/",
```
Issue 2: change_type failure.
Denial of Mount in log
```
type=AVC msg=audit(1686977968.399:763): apparmor="DENIED" operation="mount" class="mount" info="failed flags match" error=-13 profile="snap-update-ns.authy" name="/var/cache/fontconfig/" pid=26702 comm="5" srcname="/var/lib/snapd/hostfs/var/cache/fontconfig/" flags="rw, bind"
...
```
snapd error
```
- Run configure hook of "chromium" snap if present (run hook "configure":
-----
update.go:85: cannot change mount namespace according to change mount (/var/lib/snapd/hostfs/usr/share/doc /usr/share/doc none bind,ro 0 0): permission denied
update.go:85: cannot change mount namespace according to change mount (/var/lib/snapd/hostfs/usr/share/fonts /usr/share/fonts none bind,ro 0 0): permission denied
update.go:85: cannot change mount namespace according to change mount (/var/snap/cups/common/run /var/cups none bind,rw 0 0): permission denied
cannot update snap namespace: cannot create writable mimic over "/snap/chromium/2475": permission denied
snap-update-ns failed with code 1
```
and NO mount rules in the profiles.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1054
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1048
made it so rules like
mount slave /snap/bin/** -> /**,
mount /snap/bin/** -> /**,
would get passed into change_mount_type rule generation when they
shouldn't have been. This would result in two different errors.
1. If kernel mount flags were present on the rule. The error would
be caught causing an error to be returned, causing profile compilation
to fail.
2. If the rule did not contain explicit flags then rule would generate
change_mount_type permissions based on souly the mount point. And
the implied set of flags. However this is incorrect as it should
not generate change_mount permissions for this type of rule. Not
only does it ignore the source/device type condition but it
generates permissions that were never intended.
When used in combination with a deny prefix this overly broad
rule can result in almost all mount rules being denied, as the
denial takes priority over the allow mount rules.
Fixes: https://bugs.launchpad.net/apparmor/+bug/2023814
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1211989
Fixes: 9d3f8c6cc ("parser: fix parsing of source as mount point for propagation type flags")
Fixes: MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1048
Signed-off-by: John Johansen <john.johansen@canonical.com>
Found in testing a slimmed-down `usr.sbin.sshd` profile:
```
Jun 8 21:09:38 testvm kernel: [ 54.847014] audit: type=1400 audit(1686272978.009:68): apparmor="DENIED" operation="open" profile="/usr/sbin/sshd" name="/sys/kernel/mm/transparent_hugepage/hpage_pmd_size" pid=1035 comm="sshd" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
```
Not sure what glibc/system call uses this, but it seems pretty broadly applicable, and read access is presumably harmless. [THP reference](https://www.kernel.org/doc/html/latest/admin-guide/mm/transhuge.html)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1050
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit ad3750058d)
Signed-off-by: John Johansen <john.johansen@canonical.com>
/etc/ld-musl-*.path is required to perform dynamic linking on musl libc.
The wildcard is to match all CPU architectures, like x86_64.
type=AVC msg=audit(1686087677.497:67): apparmor="DENIED" operation="open" class="file" profile="syslog-ng" name="/etc/ld-musl-x86_64.path" pid=25866 comm="syslog-ng" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
Closes#333
Signed-off-by: Nikita Romaniuk <kelvium@yahoo.com>
Closes#333
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1047
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 6e0d776f65)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Before 300889c3a, mount rules would compile policy when using source
as mount point for rules that contain propagation type flags, such as
unbindable, runbindable, private, rprivate, slave, rslave, shared, and
rshared. Even though it compiled, the rule generated would not work as
expected.
This commit fixes both issues. It allows the usage of source as mount
point for the specified flags, albeit with a deprecation warning, and
it correctly generates the mount rule.
The policy fails to load when both source and mount point are
specified, keeping the original behavior (reference
parser/tst/simple_tests/mount/bad_opt_10.sd for example).
Fixes: https://bugs.launchpad.net/bugs/1648245
Fixes: https://bugs.launchpad.net/bugs/2023025
It should be backported to versions 2.13, 3.0, 3.1.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1048
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 1e0d7bcbb7)
Signed-off-by: John Johansen <john.johansen@canonical.com>
When opening snap browsers with evince using the snap_browsers
abstraction, we get the following AppArmor denials which prevent the
browsers from opening
audit: type=1400 audit(1685996894.479:225): apparmor="DENIED" operation="open" class="file" profile="/usr/bin/evince//snap_browsers" name="/var/lib/snapd/inhibit/firefox.lock" pid=13282 comm="snap" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
audit: type=1400 audit(1685997517.142:259): apparmor="DENIED" operation="file_lock" class="file" profile="/usr/bin/evince//snap_browsers" name="/var/lib/snapd/inhibit/firefox.lock" pid=14200 comm="snap" requested_mask="k" denied_mask="k" fsuid=1000 ouid=0
This MR should be cherry-picked into 2.13, 3.0, 3.1
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1045
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
(cherry picked from commit a00ece5b6e)
daec4bc8 profiles: add lock file permission to snap browsers
`unscd` is a drop-in replacement for `nscd` that uses the same binary location (`/usr/sbin/nscd`) and config file (`/etc/nscd.conf`). The `usr.sbin.nscd` profile only needs one additional permission to support it.
```
May 9 18:07:42 darkstar kernel: [ 2706.138823] audit: type=1400
audit(1683670062.580:839): apparmor="DENIED" operation="sendmsg"
profile="nscd" name="/run/systemd/notify" pid=4343 comm="nscd"
requested_mask="w" denied_mask="w" fsuid=125 ouid=0
```
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1031
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
(cherry picked from commit dec3815f07)
bd0d401b nscd: add permission to allow supporting unscd
This patch adds the following mount options: 'nostrictatime',
'lazytime', and 'nolazytime'.
The MS_STRICTATIME mount flag already existed, and 'nostrictatime' was
listed along with 'strictatime' in the comments of parser/mount.cc, so
this patch adds a mapping for 'nostrictatime' to clear MS_STRICTATIME.
Additionally, the Linux kernel includes the 'lazytime' option with
MS_LAZYTIME mapping to (1<<25), so this patch adds MS_LAZYTIME to
parser/mount.h and the corresponding mappings in parser/mount.cc for
'lazytime' and 'nolazytime'.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1005
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit c37be61d17)
Signed-off-by: Jon Tourville <jon.tourville@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1026
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Jon Tourville <jon.tourville@canonical.com>
This patch adds the following mount options: 'nostrictatime',
'lazytime', and 'nolazytime'.
The MS_STRICTATIME mount flag already existed, and 'nostrictatime' was
listed along with 'strictatime' in the comments of parser/mount.cc, so
this patch adds a mapping for 'nostrictatime' to clear MS_STRICTATIME.
Additionally, the Linux kernel includes the 'lazytime' option with
MS_LAZYTIME mapping to (1<<25), so this patch adds MS_LAZYTIME to
parser/mount.h and the corresponding mappings in parser/mount.cc for
'lazytime' and 'nolazytime'.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1005
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit c37be61d17)
Signed-off-by: Jon Tourville <jon.tourville@canonical.com>
The mount options MS_LAZYTIME and MS_NOSYMFOLLOW were added in
kernels 4.0 and 5.10, respectively. Update the mount test script
and helper to skip testing those options if they are not available.
Signed-off-by: Jon Tourville <jon.tourville@canonical.com>
(cherry picked from commit 9a760def8d)
Signed-off-by: Jon Tourville <jon.tourville@canonical.com>
This allows regression tests to generate profiles that use rule qualifiers,
such as allow, deny, and audit. Qualifiers can be specified for a rule by
prepending 'qual=', followed by a comma-separated list of rule qualifiers,
then a ':', then the rule itself.
Signed-off-by: Jon Tourville <jon.tourville@canonical.com>
(cherry picked from commit f6bfd141bd)
Signed-off-by: Jon Tourville <jon.tourville@canonical.com>
The unix network tests are not being run on a v8 network capable kernel. Under v8 there needs to be some adjustments to the tests because unix rules get downgraded to the socket rule ```network unix,``` which does not have the same set of conditionals or fine grained permissions, meaning some tests that would fail under af_unix (like missing permission tests) will pass under v8 network rules.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/893
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 59b4109a8b)
Signed-off-by: John Johansen <john.johansen@canonical.com>
This MR is similar to !1009, only it targets the `apparmor-3.1` branch.
There are also three commits here, but the first one only updates comments, and the last one is much smaller. After merging, `git diff apparmor-3.1 master -- profiles` will likewise produce no output.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1010
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
(Don't remove rules from the usr.sbin.{dnsmasq,nmbd,winbindd} profiles,
however, as these have been transferred to abstractions which might not
be correctly updated on a user system)
This is used by various applications including libreoffice etc so it may as well
be added to the base abstraction along with the existing zoneinfo DB access.
AVC apparmor="DENIED" operation="open" class="file" profile="snap.libreoffice.calc" name="/usr/share/zoneinfo-icu/44/le/zoneinfo64.res" pid=44742 comm="soffice.bin" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
Signed-off-by: Alex Murray <alex.murray@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1007
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Christian Boltz <apparmor@cboltz.de>
(cherry picked from commit 8d9985ac0b)
c691b11d abstractions/base: allow reading tzdata ICU zoneinfo DB
Probably thanks to O_MAYEXEC, denials for file access can now contain a
mix of x (exec) and other file permissions.
The actual exec should appear in a separate "exec" log event, therefore
ignore 'x' in file events for now if it's mixed with other permissions.
Note that file events ("open", "link" etc.) that contain denied_mask="x"
without another permission will still cause an error. (So far, this
hasn't been seen in the wild.)
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/303
Also add the log line from the bugreport and the (for now) expected
result as test_multi testcase.
I propose this patch for all branches.
Closes#303
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1001
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit cf6539b217)
Signed-off-by: John Johansen <john.johansen@canonical.com>
This is a partial fix for CVE-2016-1585, it address the frontend rule encoding problems particularly
- Permissions being given that shouldn't happen
- Multiple option conditionals in a single rule resulting in wider permission instead of multiple rules
- optional flags not being handled correctly
- multiple backend rules being created out of one frontend rule when they shouldn't be
it does not address the backend issue of short cut permissions not being correctly updated when deny rules carve out permissions on an allow rule that has a short cut permission in the encoding.
Thanks to the additional work by Alexander Mikhalitsyn for beating this MR into shape so we can land it
Alexander Changelog:
- rebased to an actual tree
- addressed review comments from @wbumiller and @setharnold
- fixed compiler warnings about class_mount_hdr is uninitialized
- infinite loop fix
- MS_MAKE_CMDS bitmask value fixed
- fixed condition in `gen_flag_rules` to cover cases like `mount options in (bind) /d -> /4,` when flags are empty and only opt_flags are present
- marked some tests as a FAIL case behavior was changed after `parser: add conflicting flags check for options= conditionals` commit
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/333
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit c1a1a3a923)
Signed-off-by: John Johansen <john.johansen@canonical.com>
For example winbindd when configured on a samba system using
sssd can trigger
apparmor="DENIED" operation="file_mmap" profile="/usr/sbin/winbindd" name="/usr/lib64/krb5/plugins/authdata /sssd_pac_plugin.so" pid=2798 comm="winbindd" requested_mask="m" denied_mask="m" fsuid=52311 ouid=0
Signed-off-by: Noel Power <noel.power@suse.com>
(cherry picked from commit 6e94794c68)
samba-dcerpcd requires access to `/var/cache/samba/names.tdb`.
audit: type=1400 audit(1676835286.187:62): apparmor="DENIED" operation="open" profile="samba-dcerpcd" name="/var/cache/samba/names.tdb" pid=6948 comm="samba-dcerpcd" requested_mask="wrc" denied_mask="wrc" fsuid=0 ouid=0
See also https://bbs.archlinux.org/viewtopic.php?id=281411
Since `usr.sbin.winbindd` already has a rule for it, and `usr.sbin.nmbd`
has similar ones, simply add `/var/cache/samba/*.tdb rwk` to
`abstractions/samba`.
(cherry picked from commit 763c4ecd23,
with cleanup of now-superfluous rules in usr.sbin.nmbd and
usr.sbin.winbindd dropped)
Also allow access to samba pid files directly in /run/
This is a backport of !987, with the cleanup of now-superfluous rules removed.
I propose this patch for 3.x (also for 2.13 if it cleanly applies)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/988
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
On Arch Linux, `samba-dcerpcd.pid` is in `/run/`, not `/run/samba/`.
apparmor="DENIED" operation="mknod" profile="samba-dcerpcd" name="/run/samba-dcerpcd.pid" pid=80920 comm="samba-dcerpcd" requested_mask="c" denied_mask="c" fsuid=0 ouid=0
The same is true for `nmbd.pid`, `smbd.pid` and probably others too.
(cherry picked from commit 6f0d2ef7fe)
samba-dcerpcd requires access to `/var/cache/samba/names.tdb`.
audit: type=1400 audit(1676835286.187:62): apparmor="DENIED" operation="open" profile="samba-dcerpcd" name="/var/cache/samba/names.tdb" pid=6948 comm="samba-dcerpcd" requested_mask="wrc" denied_mask="wrc" fsuid=0 ouid=0
See also https://bbs.archlinux.org/viewtopic.php?id=281411
Since `usr.sbin.winbindd` already has a rule for it, and `usr.sbin.nmbd`
has similar ones, simply add `/var/cache/samba/*.tdb rwk` to
`abstractions/samba`.
(cherry picked from commit 763c4ecd23,
with cleanup of now-superfluous rules in usr.sbin.nmbd and
usr.sbin.winbindd dropped)
In order to decrease the number of characters in the audit logs
from the kernel, we will drop the "_mask" from the fields
"requested_mask" and "denied_mask".
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
(cherry picked from commit a05c9483f3)
Signed-off-by: John Johansen <john.johansen@canonical.com>
dbus-broker requires some modification of the test suite. In summary:
* refactor to support starting and stopping both dbus and dbus-broker.
* Make it so we can run the tests on each, where appropriate
* skip unrequested reply and eavesdrop tests for dbus broker because they are not supported.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/965
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 223036d952)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Resolves#292.
This fix is the same as !830 but for Brave.
Opening links in Brave now works as intended.
Note that now a separate denial is caused, related to WidevineCDM, is produced:
```
[ERROR:content_main_runner_impl.cc(415)] Unable to load CDM /home/username/.config/BraveSoftware/Brave-Browser/WidevineCdm/4.10.2557.0/_platform_specific/linux_x64/libwidevinecdm.so (error: /home/username/.config/BraveSoftware/Brave-Browser/WidevineCdm/4.10.2557.0/_platform_specific/linux_x64/libwidevinecdm.so: failed to map segment from shared object)
```
In the syslog:
```
audit: type=1400 audit(1671108748.090:117): apparmor="DENIED" operation="file_mmap" profile="/usr/bin/evince//sanitized_helper" name="/home/username/.config/BraveSoftware/Brave-Browser/WidevineCdm/4.10.2557.0/_platform_specific/linux_x64/libwidevinecdm.so" pid=65765 comm="brave" requested_mask="m" denied_mask="m" fsuid=1000 ouid=100
```
I'm not sure if granting permission(s) for this is desirable. In either case, the potential relevant changes are out of the scope of this MR.
If I disable WidevineCDM in Brave, I get the following denial on cap sys_admin:
```
audit: type=1400 audit(1671112807.666:174): apparmor="DENIED" operation="capable" profile="/usr/bin/evince//sanitized_helper" pid=112098 comm="brave" capability=21 capname="sys_admin"
```
which is fine, as mentioned by @jjohansen [here](https://gitlab.com/apparmor/apparmor/-/merge_requests/830#note_831915024).
Closes#292
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/957
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 5fd8c25745)
Signed-off-by: John Johansen <john.johansen@canonical.com>
- ssl_certs: /{etc,usr/share}/pki/trust/ has more than the 'anchors' subdirectory
- crypoto: allow reading /etc/gcrypt/hwf.deny
I propose this patch for 3.0..master (2.13 doesn't have abstractions/crypto).
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/961
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit bb30df7843)
d15bfa99 Extend crypto and ssl_certs abstractions
Glibc in 2.36 and later will [1] access sysfs at
/sys/devices/system/cpu/possible when usig sysconf
for _SC_NPROCESSORS_CONF.
That will make a lot of different code, for example
anything linked against libnuma, trigger this apparmor
denial.
apparmor="DENIED" operation="open" class="file" ...
name="/sys/devices/system/cpu/possible" ...
requested_mask="r" denied_mask="r" fsuid=0 ouid=0

This entry seems rather safe, and it follows others
that are already in place. Instead of fixing each
software individually this should go into the base
profile as well.
Initially reported via
https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1989073
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/267
MR: none - ML
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit c159d0925a)
Signed-off-by: John Johansen <john.johansen@canonical.com>
When WSL automatically generates a resolv.conf for an instance, the /etc/resolv.conf file is a symlink to /mnt/wsl/resolv.conf. This patch adds an entry for this to the other policies to handle such management.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/935
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 8fa77e9e6c)
5232eaa2 Adds /mnt/wsl/resolv.conf to nameservices.
If audit.log contains entries for a profile that doesn't exist (for
example when working with a log file from another system), skip these
log entries instead of crashing.
Reproducer (crashes without this patch):
aa-logprof -f <(echo 'type=AVC msg=audit(1661739121.578:77893): apparmor="DENIED" operation="open" profile="no_such_profile" name="/run/" pid=33099 comm="no" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0')
I propose this patch for 3.1 and master. (3.0 and older are not affected and do not need this fix.)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/919
Approved-by: Jon Tourville <jon.tourville@canonical.com>
Merged-by: Christian Boltz <apparmor@cboltz.de>
(cherry picked from commit f5594fbb7c)
94c7c79c Prevent crash on log entries for non-existing profile
In commit 7c7224004 ("Prepare for AppArmor 3.1 release"), as preperation
for the AppArmor 3.1.0 release, the SO versioning information was
adjusted, using a more significant bump to give prior AppArmor releases
room to address bugs in libapparmor without ending up with conflicting
SO versions. Unfortunately, that process was untested and because
AA_LIB_AGE was not incremented by the same amount as AA_LIB_CURRENT,
this resulted in an accidental major SO versions bump with the library
SO version being:
libapparmor.so.4.9.0
This commit increments AA_LIB_AGE by the same amount, resulting in a
library versioned as:
libapparmor.so.1.12.0
and adds a note to mention that AA_LIB_AGE needs to be incremented
in the same way as AA_LIB_CURRENT. This fix is intended to address
this for the 3.1 branch; I'd like to find a better approach for
the development branch that can be used in future AppArmor primary
releases.
In general, thanks to symbol versioning (see
`libraries/libapparmor/src/libapparmor.map`) we should not need to
ever bump the SO version except in an extreme case.
Fixes: 7c7224004 ("Prepare for AppArmor 3.1 release")
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Bug: https://gitlab.com/apparmor/apparmor/-/issues/266
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/913
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
In commit 7c7224004 ("Prepare for AppArmor 3.1 release"), as preperation
for the AppArmor 3.1.0 release, the SO versioning information was
adjusted, using a more significant bump to give prior AppArmor releases
room to address bugs in libapparmor without ending up with conflicting
SO versions. Unfortunately, that process was untested and because
AA_LIB_AGE was not incremented by the same amount as AA_LIB_CURRENT,
this resulted in an accidental major SO versions bump with the library
SO version being:
libapparmor.so.4.9.0
This commit increments AA_LIB_AGE by the same amount, resulting in a
library versioned as:
libapparmor.so.1.12.0
and adds a note to mention that AA_LIB_AGE needs to be incremented
in the same way as AA_LIB_CURRENT. This fix is intended to address
this for the 3.1 branch; I'd like to find a better approach for
the development branch that can be used in future AppArmor primary
releases.
In general, thanks to symbol versioning (see
`libraries/libapparmor/src/libapparmor.map`) we should not need to
ever bump the SO version except in an extreme case.
Fixes: 7c7224004 ("Prepare for AppArmor 3.1 release")
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Bug: https://gitlab.com/apparmor/apparmor/-/issues/266
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
bison change the default text past to yerror in bison 3.6, this
breaks make check as some tests are comparing against the error
output
======================================================================
FAIL: test_modefail (__main__.AAErrorTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/jj/apparmor.git/parser/tst/testlib.py", line 50, in new_unittest_func
return unittest_func(self)
File "./errors.py", line 58, in test_modefail
self._run_test(
File "./errors.py", line 40, in _run_test
self.assertIn(message, outerr, report)
AssertionError: 'AppArmor parser error for errors/modefail.sd in profile errors/modefail.sd at line 6: syntax error, unexpected TOK_ID, expecting TOK_MODE' not found in 'AppArmor parser error for errors/modefail.sd in profile errors/modefail.sd at line 6: syntax error\n' :
Command: ../apparmor_parser --config-file=./parser.conf -S -I errors errors/modefail.sd
Exit value:1
STDERR
AppArmor parser error for errors/modefail.sd in profile errors/modefail.sd at line 6: syntax error
To fix this we need to add
define parse.error=verbose
to bison. Unfortunately define parse.error was only added in bison 3.0
and and older versions of bison will break if that is defined in
parser_yacc.y
Instead test for the version of bison available and set define parse.error
as a build flag if supported by the version of bison being called.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/640
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
The parser is not handling variable expansion/rule conditionals
correctly in some cases. Eg. NOT treating unix addr as a path and
filtering slashes after variable expansion. This can lead to errors
where
@{foo}=/a/ unix bind addr=@{foo}/bar,
will always fail because addr is being matched as /a//bar instead of
/a/bar.
Note: this is a first pass and may not fix all cases. Eg. There is a
known issue with profile names not being able to begin with a
variable.
Signed-off-by: John Johansen john.johansen@canonical.com
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Attempt to get clarity on what is valid syntax for mount options and
fstype options.
Note that simple_tests/mount/bad_opt_27.sd is marked TODO, as the
parser accepts it but should not.
Also mark the tests as expecting to fail to raise an exception by the
python utils.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/607
Signed-off-by: John Johansen <john.johansen@canonical.com>
This adds support for the new v8 network mediation in the 4.17 and later kernels.
It requires that policy either be tagged with an abi rule that support the v8 network, or the policy be pinned with --policy-features= with a feature abit that supports v8 network.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/521
Acked-by: Steve Beattie <steve.beattie@canonical.com>
af_unix allows for sockets to be bound to a name that is autogenerated.
Currently this type of binding is only supported by a very generic
rule.
unix (bind) type=dgram,
but this allows both sockets with specified names and anonymous
sockets. Extend unix rule syntax to support specifying just an
auto bind socket by specifying addr=auto
eg.
unix (bind) addr=auto,
It is important to note that addr=auto only works for the bind
permission as once the socket is bound to an autogenerated address,
the addr with have a valid unique value that can be matched against
with a regular
addr=@name
expression
Fixes: https://bugs.launchpad.net/apparmor/+bug/1867216
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/521
Signed-off-by: John Johansen <john.johansen@canonical.com>
v8 network permissions extend into the range used by exec mapping
so it is important to not blindly do execmapping on both the
file dfa and policydb dfa any more.
Track what type of dfa and its permissions we are building so
we can properly apply exec mapping only when building the
file dfa.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/521
Signed-off-by: John Johansen <john.johansen@canonical.com>
If an exec gets denied in enforce mode, there are no log events for the target binary/profile. Therefore, trying to set the final_name for the target will crash with a KeyError.
Check for the existence of hashlog\[aamode\]\[target_profile\] in all exec options to prevent this crash.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/634
Acked-by: John Johansen <john.johansen@canonical.com>
Add a basic tool for manipulating the apparmor features abi via
libapparmor. This serves as a basic tool and as an example of using
the library api.
Currently its function is limited to extracting the kernel feature
abi and loading a feature abi from a file and then outputing it.
In the future it will pickup the ability to verify the feature
abi, and merge feature abis.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/613
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
... and allow dovecot to Px to it.
The profile is based on a profile I received in a bugreport, with the
actual script factored out. Note that you'll _need_ to add a rule to the
local/ include to allow executing the actual script, but since the
script to execute is configurable, it's basically impossible to add a
default rule for it.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=1166007
If an exec gets denied in enforce mode, there are no log events for the
target binary/profile. Therefore, trying to set the final_name for the
target will crash with a KeyError.
Check for the existence of hashlog[aamode][target_profile] in all exec
options to prevent this crash.
Note that the log doesn't include enough information for EXEC MODE and EXEC COND, therefore aa-logprof will always propose ALL as EXEC COND (comm= might give a hint about EXEC COND, but isn't good enough).
With the added support in aa-logprof, remove the changeprofile tests from the known-failing list in test-libapparmor-test_multi.py.
Also add another test log (from darix) / expected profile to the libapparmor testsuite.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/631
Acked-by: John Johansen <john.johansen@canonical.com>
When switching to complain or enforce mode (for example with aa-complain and aa-enforce), remove conflicting flags like kill and unconfined.
As ground work, change add_or_remove_flag() to allow to add or remove multiple flags. Multiple flags can be given as string (will be split) or as array.
Also add some tests confirming that everything works as expected.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/633
Acked-by: John Johansen <john.johansen@canonical.com>
Snapd now loads its own policy via its own systemd unit
https://github.com/snapcore/snapd/pull/8467
If A distro is not using snapd systemd unit then dropping snapd policy
from the apparmor unit is a breaking change, distros will either need
to use the snapd systemd unit or revert
0164fd05 init: stop loading snap policy
Signed-off-by: John Johansen <john.johansen@canonical.com>
Note that the log doesn't include enough information for EXEC MODE and
EXEC COND, therefore aa-logprof will always propose ALL as EXEC COND
(comm= might give a hint about EXEC COND, but isn't good enough).
With the added support in aa-logprof, remove the changeprofile tests
from the known-failing list in test-libapparmor-test_multi.py.
Also add another test log (from darix) / expected profile to the
libapparmor testsuite.
... when reading /proc/$pid/attr/{apparmor/,}current
Also add a comment about _not_ adding support for the 'unconfined'
profile mode, because that would give a quite confusing output.
This means moving the code that reads the 'current' file into a new
function read_proc_current()Then call that function for both
/proc/$pid/attr/apparmor/current (preferred) and /proc/$pid/attr/current
(fallback).
New kernels provide an alternative proc attr interface for apparmor
which is needed for LSM stacking.
Update the remaining profiles that use the old interface to
include access to the new interface.
Signed-off-by: John Johansen <john.johansen@canonical.com>
As of [48755ebf], dnsmasq iterates through directory entries in
/proc/self/fd/ to find which file descriptors are open to avoid the
potentially costly operation of closing all possible file descriptors.
[48755ebf]: https://www.openhub.net/p/dnsmasq/commits/1679009518
MR625 fixed hats not emitting the rule to access the proc interface needed for change_hat, but it broke the rule being emitted for the parent (which used to work).
The proc attr access rule should be emitted for any profile that is a hat OR any profile that contains hats.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/626
Acked-by: Steve Beattie <steve.beattie@canonical.com>
This add a test to ensure that the parser is inserting rules to allow
access to the proc interface for change_hat.
Unfortunately the rule the parser inserts is a bare owner write that
we can't replicate in policy as policy write perm maps to create,
append and write.
So to test equality compare profiles using rules granting access to
the proc attr interface except one uses the append permission and
the other uses write. They will differ in permissions unless the
parser inserts the proc attr write rule for change_hat in which
case the permissions will get merged and we have equivalence.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/626
Signed-off-by: John Johansen <john.johansen@canonical.com>
LSM stacking adds new interfaces
/proc/<pid>/attr/apparmor/*
that should be used in preference of the old interface files in
/proc/<pid>/attr/*
The library has already been updated to use the new path but the
abstraction for change_profile has not so accessing the new interface
in a confined application fails.
Fix this by making the abstraction cover the old and new interfaces.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/626
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR625 fixed hats not emitting the rule to access the proc interface
needed for change_hat, but it broke the rule being emitted for the
parent (which used to work).
The proc attr access rule should be emitted for any profile that
is a hat OR any profile that contains hats.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/626
Signed-off-by: John Johansen <john.johansen@canonical.com>
The parser is supposed to add a rule to profiles if they are a hat
or contain hats granting write access to the kernel interfaces
used to perform the change_hat operation.
Unfortunately the check is broken and currently won't add the
rule to hats (it does add it for the parent).
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/625
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Acked-by: Steve Beattie <steve.beattie@canonical.com>
serialize_profile() assumes that active_profiles has the /etc/apparmor.d/ filename of a profile initialized.
This patch makes sure this is true even when using an extra profile by initializing it in get_profile().
Ideally serialize_profile() shouldn't always use active_profiles, but that will be part of a bigger change.
Reported by zt1024 including a proposed patch on https://gitlab.com/apparmor/apparmor/-/merge_requests/604 but of course ;-) this patch is better because it selectively does the initialization only in the case that needs it.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/623
Acked-by: John Johansen <john.johansen@canonical.com>
serialize_profile() assumes that active_profiles has the
/etc/apparmor.d/ filename of a profile initialized.
This patch makes sure this is true even when using an extra profile by
initializing it in get_profile().
Ideally serialize_profile() shouldn't always use active_profiles, but
that will be part of a bigger change.
Reported by zt1024 including a proposed patch on
https://gitlab.com/apparmor/apparmor/-/merge_requests/604
but of course ;-) this patch is better because it selectively does the
initialization only in the case that needs it.
php-fpm: fix profile name in php-worker
This is analogous to 9892a032. It allows php-fpm to manage the worker threads by allowing the threads to receive signals from php-fpm (rather than the nonexistent profile `/usr/sbin/php-fpm*`).
See merge request apparmor/apparmor!619
Acked-by: Christian Boltz <apparmor@cboltz.de>
A debug message in reset_parser() gives developers more data about how
the parser is behaving. In addition, it provides much needed context to
the relatively vague debug message in clear_cap_flag().
Another solution might be to pass the profile name into
clear_cap_flag(), however, clear_cap_flag() does not need the profile
name, except potentially for debugging purposes.
Signed-off-by: Mike Salvatore <mike.salvatore@canonical.com>
A copy/paste error leads to a build failure due to an undefined variable
"name" in parser_misc.c:clear_cap_flag().
Signed-off-by: Mike Salvatore <mike.salvatore@canonical.com>
Add flags to be able to control parser warnings, and convert warnings into errors.
Flags can be enabled by using `--warn` followed by a single flag. If multiple flags need to be set --warn can be specified multiple times.
Eg.
```
--warn=deprecated --warn=rule-downgrade
```
Flags can be disabled no prefix the flag name with `no-`
Eg.
```
--warn=no-config
```
The set of currently enabled flags can be seen by specifying
```
--warn=show
```
and the set of supported flags can be seen by specifying
```
--help=warn
```
The default set of warning flags that are enabled are
```
WARN_CONFIG - config warnings
WARN_CACHE - cache warnings that were not hidden behind debug-cache
WARN_JOBS - warnings around job failures
WARN_UNEXPECTED - warnings about internal consistency checks against what is expected from kernel
WARN_OVERRIDE - warnings about overriding some option, currently limited to forcing an override on a namespace.
```
Warnings can be converted into errors by specifying `-Werror`. It supports all the same options as \`\`\`--warn\`. A warning must be turned on before the warning can be converted into an error message.
```
--warn=deprecated --Werror=all
```
Will create error messages for deprecation warnings but not for rule-downgrades. -Werror is disabled by default.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/600
Acked-by: Steve Beattie <steve.beattie@canonical.com>
--config-file is processed early in a separate argument processing
pass. Adjust --warn and --Werror processing so they are done in
both the early and late arg processing pass.
--warn and --Werror must be run in both argument processing passes
so that
1. They can be used with --config-file as long as they are specified
before --config-file (early pass)
2. They are not overriden by any flags set in the config file, as
command line options take priority over what is in the config
file (hence the need for reprocessing in the second pass)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/600
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add basic ability to treat a warning as an error and abort the compile
by specifying the new option --Werror.
--Werror
will turn all warnings into errors. Where if an warning type is
specified only that type of warning will be turned into an error.
--Werror=deprecated.
The full list of supported warning types can be found by using
apparmor_parser --help=warn
or
apparmor_parser --help=Werror
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/600
Signed-off-by: John Johansen <john.johansen@canonical.com>
Make all warnings that go through pwarn() controllable by warning
flags. This adds several new warning control flags, documented in
--help=warn
Convert --debug-cache to be unified with warning flags. So it can be
set by either
--debug-cache
or
--warn=debug-cache
Also add an "all" option to be able to turn on all warnings.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/600
Signed-off-by: John Johansen <john.johansen@canonical.com>
Host files accessed by tcp_wrapper can reference other files, from man
5 hosts.allow
```
A string that begins with a '/' character is treated as a file name. A host name or address is matched if it matches any host name or address pattern listed in the named file. The file format is zero or more lines with zero or more host name or address patterns separated by whitespace. A file name pattern can be used anywhere a host name or address pattern can be used.
```
This means adding a file to hosts requires updating multiple profiles
Add a hosts abstraction so users only have to modify a single location.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/605
Fixes: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1864466
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Fix /usr/lib/postfix binary paths in postfix+dovecot profiles
See merge request apparmor/apparmor!602
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.12..master
Provide example rules in parser.conf to pin pre 3.0 policy and
appropriate abi files.
abis for vanilla upstream kernels and outoftree network patched
kernels are provided. With both ABIs dropping v8 support from
advertised by the kernel as 2.x policy/userspace did not support it.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/598
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
prologue.inc:settest() in same cases invokes a sed command that uses
'~' as a pattern separator, on things that can contain filesystem
paths. However, in the debian/ubuntu world, '~' can be used in version
strings, particularly for pre-release versions, and when this happens
and the version is embedded in the path, the sed command breaks
because of the extraneous separator. Fix this by using '#' as a
separator, which has the benefit of being considered a comment if
accidentally interpreted by a shell.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/599
Merge branch 'parser-fix_cap_list_check' into 'master'
The sed command to manipulate the known cap list (`base_cap_names.h`) into
a format to match the generated_cap_names.h was buggy because the
trailing `}` would never match anything, leading to failures when built
against 5.8 kernel headers, due to it not raplcing the base capabilities
correctly.
Fix this by removing the trailing `}` match and instead match the third
comma-delimited field that matches a capability name, and replace that.
See merge request apparmor/apparmor!596
The sed command to manipulate the known cap list (base_cap_names.h) into
a format to match the generated_cap_names.h was buggy because the
trailing '}' would never match anything, leading to failures when built
against 5.8 kernel headers, due to it not replacing the base capabilities
correctly.
Fix this by removing the trailing '}" match and instead match the third
comma-delimited field that matches a capability name, and replace that.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/596
fix bug that some rules will be added to the profile repeatedly
The ask_exec() function may cause some rules to be added to the profile repeatedly.
See merge request apparmor/apparmor!593
Acked-by: Christian Boltz <apparmor@cboltz.de>
In AppArmor 2 distros could pin the feature file being used by setting
the feature-file option in the config file.
With AppArmor 3 policy is now explicitly tagged with an abi rule.
The problem is the interaction on systems that have a mixture of
AppArmor 2 and AppArmor 3 policy and use feature pinning.
The feature pinning is required to make the apparmor 2 policy behave
as expected but it also overrides the abi rules that are explicitly
set as part of the policy. This means we either have the apparmor 2
pinned policy working as desired or the apparmor 3 policy, but not
both.
To fix this make setting the flag in the config file have lower
priority than the feature abi rule. But still allow the config option
specified on the command line to override the config file and the
feature abi rule.
The Priority ordering to determine the policy abi to use is
1. Use abi rules if present
2. if no abi rule use command line option
3. if no abi rule or command line option use config setting
4. if none of the above use the default abi
v2:
add priority ordering to documentation
reorder priority so existing config/command line options behave as expected, make ABI rules highest priority
add new option --override-policy-ABI to allow overriding of abi rules
move arg processing from numbers to defines
v3
update patch description around how the abi pinning problem is being fixed
add MR: tags to patches
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/579
I screwed up adding the last option, yet again because
EARLY_ARG_CONFIG_FILE was define out of order, and adding the new
option seems to skip a number, ...
Switch to defines to make it easier to update, and keep all these
define numbers together in order.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/579
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add an option to allow setting/pinning the feature ABI and overriding
of ABI rules if they exist.
--override-policy-abi
This option is primarily for profile development and testing without
allowing adjusting feature abis temporarily without modifying the
profile.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/579
Signed-off-by: John Johansen <john.johansen@canonical.com>
In AppArmor 2 distros could pin the feature file being used by setting
the feature-file option in the config file.
With AppArmor 3 policy is now explicitly tagged with an abi rule.
The problem is the interaction on systems that have a mixture of
AppArmor 2 and AppArmor 3 policy and use feature pinning.
The feature pinning is required to make the apparmor 2 policy behave
as expected but it also overrides the abi rules that are explicitly
set as part of the policy. This means we either have the apparmor 2
pinned policy working as desired or the apparmor 3 policy, but not
both.
To fix this make setting the flag on command line or in config file
lower priority than an abi rule specified in policy. The ability
to override abi rules will be added in a separate patch.
The Priority ordering to determine the policy abi to use is
1. Use abi rules if present
2. if no abi rule use command line option
3. if no abi rule or command line option use config setting
4. if none of the above use the default abi
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/579
Signed-off-by: John Johansen <john.johansen@canonical.com>
utils `make check_severity_db` will fail the build if a (probably new)
capability in not listed in severity.db. This also means it should print
out an ERROR, not a warning.
This is a follow-up of lp#1890547 and
https://gitlab.com/apparmor/apparmor/-/merge_requests/589
The warn_once() function is duplicated in 6 different places. A common,
reusable version has been added to parser_common.c.
Signed-off-by: Mike Salvatore <mike.salvatore@canonical.com>
Nobody told the tools that log events with operation="symlink" exist.
Add this keyword to the list of file or network operations (I don't
expect network symlinks ;-) but keeping everything in that list makes
things easier than special-casing it.)
Also add the log sample and expected result to the libapparmor tests.
Fixes https://gitlab.com/apparmor/apparmor/-/issues/107
The fixes that landed for #98 are incomplete: they only added a check to
ensure that the parser is built, and not libapparmor as well, and also
didn't adjust the invocation of aa-logprof to actually make use of the
built libapparmor or its swig python library.
Furthermore, neither it nor the tests in utils/ add the location of the
swig libapparmor C library to the library search path, meaning that if
the system one does not exist, it will not be found and the test will
fail.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Bug: https://gitlab.com/apparmor/apparmor/-/issues/98
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/586
The commit c8b6d8b393 ("profiles: Update 'make check' to select tools
based on USE_SYSTEM") set a bunch of variables but neglected to apply
them when invoking aa-logprof. This commit addresses this by:
* correcting the PYTHONPATH used with aa-logprof
* setting LD_LIBRARY_PATH when invoking aa-logprof
* adjusting LD_LIBRARY_PATH to include both the directory location
of libapparmor but also the swig libapparmor library needed for
python tools to function.
* adjusts the test for the presence of libapparmor to not use
LD_LIBRARY_PATH but instead a libapparmor specific variable
LIBAPPARMOR_PATH
Bug: https://gitlab.com/apparmor/apparmor/-/issues/98
Fixes: c8b6d8b393
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/586
The authentication, base and nameservice abstraction used /{usr/,}etc/
in several rules. Switch that to the more readable (and tunable)
@{etc_ro} variable.
Add a couple of bitmap renderings of the logo for convenience
128x128 pixel rendering of the logo
700x700 pixel rendering of the logo
Signed-off-by: John Johansen <john.johansen@canonical.com>
The profiles dirs make check is not always using the correct tools.
Update it to be similar to other Makefiles where the var USE_SYSTEM
make check USE_SYSTEM=1
is used to indicated that the system installed tools should be used
and
make check
is used to run the tests against the in tree tools
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/580
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
The hashing of the featue set is wrong because it is hashing the
whole feature structure instead of just the feature string.
This results in the refcount and hash field becoming part of the
hash and the feature string not being completely hashed as the
bytes of the refcount and hash field are being counted in the
as part of the string length when the hash is taken.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/583
Reported-by: Samuele Pedroni <samuele.pedroni@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
This adds support for the new PERFMON, and BPF capabilities.
In addition this updates AppArmor's handling in several ways.
* Move to a static capability list that is compared to the automatically generated capability list, and fail the build if new capabilities are detected. This is required to ensure backwards mapping of capabilities is properly handled.
* allow an already built apparmor to dynamically detect any new additions of capabilities so a release patch doesn't need to be made to support new capabilities. This however will be missing support for backwards mapping of capabilities
* support backwards mapping of new capabilities that have been split of existing capabilities for older kernels (eg. CAP_PERFMON was split off of CAP_SYS_ADMIN)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/578
Acked-by: Seth Arnold <seth.arnold@canonical.com>
If a capability is known in policy but not by the kernel, check to see if it has
a backwards mapping to a different capability and use that instead.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The kernel and policy abis can be used to detect and support new
capabilities without having to update base_cap_names.h and and
rebuilding the compiler.
This is not perfect however in that the does not provide any backwards
compatibility mappings, so we still need to keep the internal
capability table.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently features doesn't provide a way to query a features
value. So add an api to extract the value string of a feature.
The value string returned is a raw text value and may contain
leading spaces, etc that the caller may need to be aware of.
Signed-off-by: John Johansen <john.johansen@canonical.com>
We need to be able to dynamically add capabilities to the capability
list so switch to using a dynamically allocated table that we can
extend.
Signed-off-by: John Johansen <john.johansen@canonical.com>
We need a set of flags to track where a capability can from so we know
how to processes it for policy compatibility purposes.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Some capabilities like perfmon and bpf have been split out from
another capability, likely cap sys_admin. Add this backmapping
infomation so that the parser can take advantage of it to support
policy on older kernels that don't support the new capabilities.
Signed-off-by: John Johansen <john.johansen@canonical.com>
There are currently two capability name tables the one that is
autogenerated and an internal hardcoded name table.
Now that the autogenerated table has been converted to a base
static table we can drop the internal static table. This
removes the chance of getting the tables getting out of sync.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The static cap sys_log declaration is no longer needed as
base_cap_names.h contains it and ensures that it will always be
present.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The auto-generated cap_names.h has problems when the parser if the
parser is built against a kernel with a smaller capability list than
the kernel policy is being compiled for.
Moving to a pre-generated list lets us support all capabilities even
when we build against older kernels. However we don't want to only use
the pre-generated list as that would make it too easy to miss when a
new capability has been added.
Keep auto generating the caps list and compare it to the pre-generated
caps list so we can detect when new capabilities are added, and fail
the build so that the pre-generated list can be updated. We screen the
diff for only additions so that the parser can continue to build on
older kernels that don't have the full capability list without errors.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Mikhail Morfikov has discovered [0] that some font packages in Debian
ships font files in /usr/bin/fonts-foo-bar (like
/usr/share/fonts-font-awesome/ for example). This produces denials for
GUI applications.
Update fonts abstraction to allow reading /usr/bin/fonts-* directories.
Also, refactor abstraction to aggregate two old rules into one.
Closes#94
[0] https://gitlab.com/apparmor/apparmor/-/issues/94
This fixes a regression introduced by the abi patches for policy that
is not tagged with an abi rule.
Specifically if the current apparmor (apparmor 3) is used with a
kernel that supports unix rules, and policy has network rules but has
not been updated to use abi rules, without this patch the policy unix
rules will stop working and unix mediation will not be enforced.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/568
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
The lexer is broken and passing echo input to stdout that it doesn't match in some states ignoring the error.
First add `%option nodefault` to guarantee the lexer won't ever echo the unknown input to stdout, this will cause the parser to error out with
```
flex scanner jammed
```
and $?=2 if a profile contains unknown/invalid parts. That's not really a helpful error message, but still better than ignoring errors.
Next improve the error message output,
```
AppArmor parser error for tst/simple_tests//vars/vars_simple_assignment_14.sd line 8: Lexer found unexpected character: '
' (0xa) in state: SUB_ID_WS
```
using flex's error output only if there is an mistake made when introducing new states.
Finally fix bugs that are found.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569
Acked-by: Steve Beattie <steve.beattie@canonical.com>
* allow reading my.cnf in dovecot-dict profile (seen with the mail users in a mysql database)
* allow /proc/\*/attr/current in dovecot imap and lmtp (needed when using the "apparmor" plugin which means dovecot switches to user-specific hats)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/566
Acked-by: John Johansen <john.johansen@canonical.com>
Newlines should generally be treated as whitespace. Expand the list
of states using the newline rule to include almost all rules that
eat WS.
There are two exceptions assign and comment which have special handling
of newlines.
this fixes the failures
not ok 71543 - ./simple_tests//vars/vars_simple_assignment_13.sd: quoted commas should not trigger an error
not ok 71544 - ./simple_tests//vars/vars_simple_assignment_14.sd: quoted commas should not trigger an error
found by introducing nodefault
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
There were several states missing from the default rule which catches
unexpected input in a state.
Update the default rule to catch all input including newlines and
update its error message to include information about which state the
failure occured in. Also update the comment about what to do when
adding new states.
While the lexer now has the "nodefault" option set, it doesn't provide
as much information as the default rule does, so we prefer states
to use our provided default rule.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
... (using `%option nodefault`) instead of echoing the unknown parts to
stdout, and ignoring the error.
This will cause the parser to error out with
flex scanner jammed
and $?=2 if a profile contains unknown/invalid parts. That's not really
a helpful error message, but still better than ignoring errors.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569
Signed-off-by: Christian Boltz <apparmor@cboltz.de>
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
This fixes a regression due on older policy due to the abi patches.
Specifically if the current apparmor is used with a kernel that
supports v7 networking, and policy has network rules but has not been
updated to use abi rules, without this patch the policy network rules
will stop working and network mediation will be unenforced.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/564
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Acked-by: Steve Beattie <sbeattie@ubuntu.com>
Add profile flags enforce, kill and unconfined.
enforce - is the default mode and is not required but this allows the mode reported by introspection to be used in the profile.
kill - a modified enforce mode that kills tasks instead of returning a denial.
unconfined - allow a named profile to behave as if it is unconfined.
Eg.
profile example flags=(enforce) { ... }
profile example flags=(kill) { ... }
profile example flags=(unconfined) { ... }
Closes#7
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/440
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
This is not a real change - since some commits, include_dir_filelist()
gets only called with absolute paths.
Add a check to ensure this, drop the now superfluous get_include_path()
call, and replace usage of include_name_abs with include_name (which are
the same now).
Also drop the superfluous profile_dir parameter, and adjust the only
caller accordingly.
With the check that incfile starts with a '/', incfile and incfile_abs
(as returned by get_include_path()) are always the same.
Drop the get_include_path() call and setting incfile_abs, and replace
usage of incfile_abs with incfile.
This is just a cleanup, no behaviour change.
... so that the include rules proposed by aa-logprof continue to be
relative to the profile directory.
This fixes the behaviour change introduced in the previous commit.
This removes the need to remove profile_dir from include paths at
various places.
A side effect is that aa-logprof / match_includes() now propose more
include rules, for example matching local/ files.
Another side effect is that proposals for include rules
(match_includes() again) now come with the full path.
Both side effects will be fixed in the next commits.
This is needed for running the tests, because test/logprof.conf contains
a relative path, and tests only "manually" set the profile_dir if they
need/have a modified copy of the profiles.
chfa.cc:348:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 16));
^~
chfa.cc:349:3: note: here
case 2:
^~~~
chfa.cc:350:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 8));
^~
chfa.cc:351:3: note: here
case 1:
^~~~
chfa.cc: In function ‘void write_flex_table(std::ostream&, int, Iter, Iter) [with Iter = __gnu_cxx::__normal_iterator<unsigned int*, std::vector<unsigned int> >]’:
chfa.cc:348:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 16));
^~
chfa.cc:349:3: note: here
case 2:
^~~~
chfa.cc:350:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 8));
^~
chfa.cc:351:3: note: here
case 1:
^~~~
chfa.cc: In function ‘void write_flex_table(std::ostream&, int, Iter, Iter) [with Iter = __gnu_cxx::__normal_iterator<short unsigned int*, std::vector<short unsigned int> >]’:
chfa.cc:348:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 16));
^~
chfa.cc:349:3: note: here
case 2:
^~~~
chfa.cc:350:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 8));
^~
chfa.cc:351:3: note: here
case 1:
^~~~
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
private.c: In function ‘_aa_is_blacklisted’:
private.c:140:35: warning: comparison of integer expressions of different signedness: ‘long int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
140 | found - name + suffix->len == name_len ) {
| ^~
private.c: In function ‘readdirfd’:
private.c:352:16: warning: comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘ssize_t’ {aka ‘long int’} [-Wsign-compare]
352 | for (i = 0; i < n; ) {
| ^
private.c:378:17: warning: comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘ssize_t’ {aka ‘long int’} [-Wsign-compare]
378 | for (i = 0; i < n; i++)
| ^
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
The len parameter returns a value that correlates to a getsockopt
parameter which is typed to socklen_t which is an unsigned int.
This technically changes the fn() api but old code using this is
already broken if the getsockopt parameter is large enough to overflow
the value.
In reality what is returned shouldn't ever be negative and the value
should never be large enough to trip the overflow. This is just
cleaning up a corner case.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561 Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
kernel.c: In function ‘aa_getpeercon_raw’:
kernel.c:823:14: warning: comparison of integer expressions of different signedness: ‘socklen_t’ {aka ‘unsigned int’} and ‘int’ [-Wsign-compare]
823 | if (optlen < *len) {
| ^
kernel.c: In function ‘query_label’:
kernel.c:966:10: warning: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
966 | if (ret != size) {
| ^~
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561 Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
This fixes lots of warnings like
```
*** Checking abstractions from ./apparmor.d/abstractions against apparmor_parser
Warning from stdin (stdin line 1): ../parser/apparmor_parser: File 'stdin' missing feature abi, falling back to default policy feature abi
Warning from stdin ([...]/profiles/./apparmor.d/abstractions/apparmor_api/change_profile line 9): ../parser/apparmor_parser: [...]/profiles/./apparmor.d/abstractions/apparmor_api/change_profile features abi 'abi/3.0' differes from policy declared feature abi, using the features abi declared in policy
```
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/554
Acked-by: John Johansen <john.johansen@canonical.com>
```
libaalogparse.c: In function 'hex_to_string':
libaalogparse.c:144:16: warning: comparison of integer expressions of different signedness: 'int' and 'size_t' {aka 'long unsigned int'} [-Wsign-compare]
144 | for (i = 0; i < len; i++) {
| ^
```
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/558
Acked-by: John Johansen <john.johansen@canonical.com>
```
libaalogparse.c: In function 'hex_to_string':
libaalogparse.c:144:16: warning: comparison of integer expressions of different signedness: 'int' and 'size_t' {aka 'long unsigned int'} [-Wsign-compare]
144 | for (i = 0; i < len; i++) {
| ^
```
`i` gets used/changed as counter variable in the for loop and only gets
increased (starting at 0), so making it an (unsigned) size_t should be
safe.
This fixes lots of warnings like
```
*** Checking abstractions from ./apparmor.d/abstractions against apparmor_parser
Warning from stdin (stdin line 1): ../parser/apparmor_parser: File 'stdin' missing feature abi, falling back to default policy feature abi
Warning from stdin ([...]/profiles/./apparmor.d/abstractions/apparmor_api/change_profile line 9): ../parser/apparmor_parser: [...]/profiles/./apparmor.d/abstractions/apparmor_api/change_profile features abi 'abi/3.0' differes from policy declared feature abi, using the features abi declared in policy
```
Tag profiles and abstractions with abi information.
Tagging abstractions is not strictly necessary but allows the parser
to detect when their is a mismatch and that policy will need an
update for abi.
We do not currently tag the tunables because variable declarations
are not currently affected by abi.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
Add basic support for policy to specify a feature abi. Under the
current implementation the first feature abi specified will be
used as the policy abi for the entire profile.
If no feature abi is defined before rules are processed then the
default policy abi will be used.
If multiple feature abi rules are encountered and the specified
abi is different then a warning will be issued, and the initial abi
will continue to be used. The ability to support multiple policy
feature abis during a compile will be added in a future patch.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
The features abi adds the ability to track the policy abi separate
from the kernel. This allow the compiler to determine whether policy
was developed with a certain feature in mind, eg. unix rules.
This allows the compiler to know whether it should tell the kernel to
enforce the feature if the kernel supports the rule but the policy
doesn't use it.
To find if a feature is supported we take the intersection of what is
supported by the policy and what is supported by the kernel.
Policy encoding features like whether to diff_encode policy are not
influenced by policy so these remain kernel only features.
In addition to adding the above intersection of policy rename
--compile-features to --policy-features as better represents what it
represents. --compile-features is left as a hidden item for backwards
compatibility.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
Define EXTRA_WARNINGS in the common/Make.rules helper so that adding
additional warnings can be done in one(-ish) location, and replace
locally defined C compiler warning flags with EXTRA_WARNINGS in most
locations in the build tree.
v2: issue a warning for any compiler option that the compiler does not
support
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
... instead of relying on the filesystem(!) ordering, which will look
random to both users and unittests.
Also partially revert the test changes from
c5a7bcd50e /
https://gitlab.com/apparmor/apparmor/-/merge_requests/548 -
sorting the result only in the tests is a bad idea.
This fixes cases when two aliases with the same left side were
configured - instead of "last one wins" in the dict, AliasRuleset now
keeps both.
ProfileList add_alias() changes its parameters and now expects an
AliasRule object. Adjust all callers to that.
Drop the no longer needed write_alias().
Also adjust the tests to use AliasRule and add a dedup test promised in
an earlier patch series.
Add VariableRule and VariableRuleset and use it for variable handling
Besides the usual advantages of switching to classes, we finally get rid of the `filelist` hasher.
While on it, also fix some bugs around variable handling, including https://bugs.launchpad.net/apparmor/+bug/1331856 and some that maybe nobody noticed before.
As usual, see the individual commits for details.
See merge request apparmor/apparmor!544
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Trailing commas in variable values are not allowed (unless they are
quoted). Fix the regex to avoid "eating" the comma, and add a check to
detect invalid commas.
As usual, add some tests, and remove some testcases from the
exception_not_raised list.
... and add a test to ensure that everything works as expected.
Note that broken variable names like '@{foo' match the (quite
permissive) regex, but are invalid nevertheless.
... by calling active_profiles.get_all_merged_variables()
Also remove vars/vars_bad_add_assignment_1.sd from the
exception_not_raised list again - now it raises an exception as
expected.
Add set_variables() to severity.py to set the variables for severity
rating. It typically gets the data from the get_all_merged_variables()
result.
This replaces the slightly broken load_variables() that parsed profile
files for variables. (For example, parsing "@{foo} = /bar" resulted
in a variable name "@{foo} " with trailing space.)
Also adjust aa.py and the severity tests to use set_variables() (with
get_all_merged_variables()) instead of load_variables().
This also re-adds the checks that were removed in the "Store variables
in active_profiles (ProfileList)" commit earlier, while still fixing
lp:1331856.
With this change, unload_variables() becomes useless (the variables get
overwritten in set_variables() anyway), drop it and its calls.
Note that load_variables() silently ignored non-existing files while the
get_all_merged_variables() call only works for existing files that are
known to active_profiles. Since the input of ask_the_questions() and
ask_exec() comes from log_dict (= audit.log or a profile to merge), add
a check if that profile actually exists in the set of active profiles.
Also adjust the severity tests to use set_variables().
Finally, drop the tests that check for handling non-existing include
files, redefining and adding to non-existing variables - all these
things get now handled in include_list_recursive() and
get_all_merged_variables() and their tests.
Fixes: https://bugs.launchpad.net/apparmor/+bug/1331856
This function returns a dict with all variables and their values for a
given profile file. It also checks for redefined variables, and adding
to non-existing variables, and errors out in both cases. Note that it
does not check the include order and is therefore more forgiving than
apparmor_parser.
Also add some tests for get_all_merged_variables().
Note: the tests are based on reading "real" profiles, therefore we need
to initialize apparmor.aa and call some of its functions.
The alternative would be to construct ProfileList objects for some
profiles and includes manually, but doing that in a way that can replace
the tests with "real" profiles would be quite some work, and I'm not
bored enough for that ;-)
Everything handled in 'filelist' gets handled in active_profiles now.
Note: the 'elif' branch in delete_all_duplicates() was probably never
hit because `if include.get(...)` always matched. The only possible
exception might be non-existing include files, but those cause a 'file
not found' error anyway.
... instead of filelist[file]['lvar'], and also write them from there.
Also fix detection of variable definitions inside a profile, which is
not allowed.
Note that ProfileList has a different write order than the old code -
first includes, then variable definitions. This makes more sense because
typical profiles first include tunables/global, and then define
additonal variables (that might use variables from tunables/global) or
extend variables defined in tunables/global.
This change also fixes some problems with the simple_test test profiles.
The "adding to non-existing variable" check currently doesn't exist,
which "fixes" lp:1331856.
OTOH this also means that such cases are not detected, therefore add
vars_bad_add_assignment_1.sd to the exception_not_raised list.
The check will be re-added in a later commit
in get_all_merged_variables().
strip_quotes() assumed its parameter is at least one character long, and errored out on an empty string.
It also converted a string consisting of a single quote to an empty string because that single quote had a quote as first and last char.
This commit fixes these two bugs.
Also rewrite TestStripQuotes to use tests\[\], and add some test for an empty string, a one-char path (just a slash) and a single quote.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/545
Acked-by: John Johansen <john.johansen@canonical.com>
At least on Debian, when DNSSEC validation is enabled, dnsmasq needs to read
/usr/share/dnsmasq-base/trust-anchors.conf
Bug-Debian: https://bugs.debian.org/934869
strip_quotes() assumed its parameter is at least one character long, and
errored out on an empty string.
It also converted a string consisting of a single quote to an empty
string because that single quote had a quote as first and last char.
This commit fixes these two bugs.
Also rewrite TestStripQuotes to use tests[], and add some test for an empty
string, a one-char path (just a slash) and a single quote.
If multiple -p/--profile or -n/--namespace parameters are given to aa-exec, they are handled on a "last one wins" base.
This probably isn't expected behaviour, and erroring out in case of conflicting parameters probably makes more sense.
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/540
Acked-by: John Johansen <john.johansen@canonical.com>
If multiple -p/--profile or -n/--namespace parameters are given to
aa-exec, they are handled on a "last one wins" base.
This probably isn't expected behaviour, and erroring out in case of
conflicting parameters probably makes more sense.
This patch series moves include rule handling away from the `filelist` hasher to using IncludeRule and IncludeRuleset. This means that only variable handling is left in `filelist`.
As usual, check the individual commits for details.
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/537
Acked-by: John Johansen <john.johansen@canonical.com>
remove_duplicate_rules() composed the 'includes' list from a) the
include rules in the profile and b) the file_includes (preamble
includes), and then checked the profile includes against this
combination of profile and preamble includes.
Checking profile includes against preamble includes is wrong, and the
only reason why this went unnoticed for years is that preamble include
files (like tunables) won't work inside a profile and therefore never
appear there.
In theory this might be a backport candidate, even if this shouldn't
cause a real-world bug.
This function returns a list of all includes in a profile and its
included files.
It's split off from is_known_rule() and get_file_perms() which shared
lots of common code to recursively get the include list. These functions
now use include_list_recursive().
... or calling is_known_rule() on events for non-existing hats.
It's the usual hasher() "fun" again - accessing a non-existing element
will create its parent.
In theory this commit might be worth a backport. In practise, it doesn't cause
any visible problem.
However, starting with the next commit, it will cause lots of test errors.
Also add a missing is_known_rule() call for dbus rules, which might have
caused similar hasher() "fun".
rule_obj.get_full_paths() handles directories and non-existing files
(depending on 'if exists'), so we can simply feed the resulting list
into load_include()
This function returns a list of paths for an include rule. This can be
- a single path if the include file is a file and exists
- a single path if the include file doesn't exist, but doesn't have
'if exists' (this will cause a 'file not found' error when used)
- a list of paths if the include is a directory
- an empty list if the rule has 'if exists' and the file doesn't exist
Also add some tests for get_full_paths()
IncludeRuleset needs to import it (next commit), which is impossible
when it lives in aa.py (would cause recursive import loop because aa.py
imports IncludeRuleset)
... because this is now done via IncludeRule, and keeping the separate
code would mean asking twice.
Note that the user interface changes slightly.
The old workflow was
1 - #include <foo>
2 - #include <bar>
3 - #include <baz>
[select 2 and (A)dd, then get the next question for the remainder]
1 - #include <foo>
2 - #include <baz>
[(A)dd another one, or (I)gnore the remainder]
The new workflow will ask separately for each abstraction, and you can
(A)dd or (I)gnore it. This is probably easier to understand because it's
basically a yes/no question.
... and write them (only) from 'inc_ie' (IncludeRuleset), which can
handle both "include" and "include if exists" rules.
This duplicates storage of include rules because 'include' is still used
and needed at various places that work on/with the include rules.
With this, we get removal of duplicate include lines insinde a profile
in aa-cleanprof "for free" - extend cleanprof_test.in to confirm this.
aa-notify does not need to load the policy includes for its current
features, so drop the unneeded overhead.
Signed-off-by: John Johansen <john.johansen@canonical.com>
alid_include() checks if the given include file exists or is whitelisted in cfg\['settings'\]\['custom_includes'\].
The check if that include file is already part of the profile is unrelated to that. Move it to match_includes() where it fits much better (and drop the now superfluous profile parameter from valid_include())
In theory is_known_rule() should prevent that case from ever happening, but let's restrict this commit to moving the code around and keep this check just to be sure.
While on it, add some documentation to both functions.
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/534
Acked-by: John Johansen <john.johansen@canonical.com>
Also rename aa.py delete_duplicates() and make ruletypes a parameter.
See the commit messages for details.
This reduces usage of global variables.
The final change is that aa-genprof now asks about preamble rules that are \*Rule classes (currently `abi` and `include if exists`).
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/532
Acked-by: John Johansen <john.johansen@canonical.com>
valid_include() checks if the given include file exists or is
whitelisted in cfg['settings']['custom_includes'].
The check if that include file is already part of the profile is
unrelated to that. Move it to match_includes() where it fits much
better (and drop the now superfluous profile parameter from
valid_include())
In theory is_known_rule() should prevent that case from ever happening,
but let's restrict this commit to moving the code around and keep this
check just to be sure.
While on it, add some documentation to both functions.
For now, that means aa-mergeprof will ask for `abi` and `include if
exists` rules (currently hardcoded).
This needs storing of `active_profiles` in the Prof object - the
preamble `abi` and `include if exists` rules are stored there.
Since several functions expect an `include` dict, add an empty one to
ProfileList to prevent lots of errors and breakage. It can be removed
again when handling of `include` rules gets moved to IncludeRule.
Rename delete_duplicates() to delete_all_duplicates() to make the
function easier grep-able - the *rule classes have delete_duplicates()
which might be confused with the old name.
Also hand over 'ruletypes' as parameter to delete_all_duplicates()
instead of using the global variable.
ask_the_questions() stays the "main" function, loops over all profiles,
and calls ask_rule_questions() for each profile.
ask_rule_questions() asks the questions for all events in a specific
profile or hat.
This reduces the usage of global variables in ask_rule_questions().
The TODO in write_rules() was long solved - remove the safety net type
check, it's no longer needed.
Without this, the function becomes a simple .get_rules_clean() call
which can easily be inlined into write_piece() at the two places where
it's called.
Replace usage of filelist\[file_name\]\['profiles'\] with the `active_profiles` ProfileList which already has the same data.
Add `profiles_in_file()` to ProfileList as a nice interface to get the list of profiles in a file.
For more details, check the individual commits.
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/523
Avked-by: John Johansen <john.johansen>
... but not for abi rules, which (according to the simple_tests
profiles) do not share these bugs)
For unquoted paths, make sure that the path doesn't include whitespace.
Don't commingle stderr and stdout by default, and provide a
function that provides the exit value, stderr, and stdout as a
tuple. Also, expect UTF-8 output by default from the commands (with
universal_newlines), rather than bytes.
(Nifty commands like subprocess.run() weren't added to python until
python 3.5.)
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/522
... because after the previous three commits, nothing reads/needs this
anymore
Note: file_name in ask_exec() was only used in the (dropped) filelist
usage.
Instead of checking filelist[file]['profiles'] for duplicate hats, check
profile_data[profile][hat].
With this, the duplicate hat check is done in the same way as the check
for duplicate profiles and child profiles.
Also add tests for duplicate child profiles and duplicate hats.
Using a list as parameter for subprocess.call is more secure, and avoids
funny problems if the filename to display contains spaces or other funny
characters.
When aa-genprof proposes a local inactive profile, it had a hardcoded
call to 'less' to display that profile.
Unsurprisingly, this doesn't work in JSON mode and breaks YaST (luckily
it's only a case of "the button doesn't work").
References: https://bugzilla.opensuse.org/show_bug.cgi?id=1171315
... which can mean "too late" in some special cases (if a profile
already exists in /etc/apparmor.d/$non_default_filename).
However, the main reason is that without this change
- the new profile will be added to (otherwise empty) active_profiles
- the first do_logprof_pass() will read all profiles, including the new
one, and add them to active_profiles - which unsurprisingly results in
an error like `ERROR: Profile /usr/sbin/vsftpd exists in
/etc/apparmor.d/usr.sbin.vsftpd and /etc/apparmor.d/usr.sbin.vsftpd`
To fix this,
- change do_logprof_pass to never call read_profiles() (and get rid of
the 'passno' parameter)
- adjust its callers (aa-logprof and aa-genprof) to call read_profiles()
themself
- move printing the 'Updating AppArmor profiles in $directory.' message
to read_profiles(), but only display it if requested (to keep the
current UI behaviour)
Since 4b7108f1e5, aa-genprof and
aa-autodep crashed with
apparmor.common.AppArmorBug: /etc/apparmor.d/usr.sbin.vsftpd not listed in ProfileList files
because the profile was not added to active_profiles.
Add the profile to active_profiles to fix this regression, and to have
all the needed data in active_profiles.
The profile repo is dead since years and most likely won't come back, so
there's no point in keeping and maintaining the code for uploading and
downloading profiles.
The parser tests were missing include tests for include entries in the
preamble section of a profile. This commit adds both #include and
include variants, as well as include if exists variants.
Also added is an exception list for the utils tests -- though it should
be noted that the utils silently drop the "#include if exists" format if
it's in the preamble without raising an exception.
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/509
- add_inc_ie() stores include and include if exists rules
- get_clean() and get_raw() return the profile preamble (currently only
the include rules)
Also add tests for the new functions.
This is similar to get_clean(), but keeps the original rule order
instead of sorting them.
This is useful for include rules in the preamble, where the order might
be relevant - for example if the first include defines a variable that
is then used or extended in the second include file.
With the addition of commits to make libapparmor LSM stacking aware, several tests were failing due to:
- aa_getpeercon() having an added aa_enabled() check
- aa_getprocattr() looking in /proc/pid/attr/apparmor/current first
Fix the specific failing testcases with additional permissions, and grant default profiles and hat generated by mkprofile.pl write access to /proc/*/attr/apparmor/current.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/504
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Because of the need to be stacking LSM aware, aa_getpeercon() calls
aa_enable to ensure that apparmor is enabled. Without the permission,
aa_getpeercon() fails, causing test failures.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
While xattr attachments exist without outofband transitions, they are
broken when values contain the null character. The addition of
outofband transitions fixes this problem.
While we could make the test requirement for outofband transitions
specific to a few tests, since the parser currently requires outofband
transitions to generate xattr attachment conditionals, for a first
pass just make the whole test suite require them.
Revisit this when/if the parser will allow xattr attachments without
outofband transitions.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
For now, only 'include if exists' rules will be handled by IncludeRule.
Using it also for 'include' rules needs some more code changes so that
included files still get checked etc.
Also remove some testcases from test-parser-simple-tests.py unknown_line
which no longer fail.
These classes are meant to handle 'include' and 'include if exists'
rules.
Due to restrictions in re_match_include_parse(), some cases in
is_covered_localvars() and is_equal_localvars() can't be reached in the
unittests.
Also, IncludeRule isn't used in aa-logprof (yet?), which means
logprof_header_localvars() result format isn't decided yet, and
therefore not tested.
This means test coverage for the new classes isn't 100% this time ;-)
LSM stacking adds the new /proc/<pid>/attr/apparmor directory
dedicated to apparmor so that it won't collide with other LSMs.
The library has already been switched to using this interface by
default, but the parser need to add permissions to access this
location when hats and/or change_profile rules are defined.
Also make sure to update the apache abstraction to support the new
interface as well.
Signed-off-by: John Johansen <john.johansen@canonical.com
Acked-by: Seth Arnold <seth.arnold@canonical.com>
- extend RE_INCLUDE to also match "include if exists"
- rename re_match_include() to re_match_include_parse() and extend it to
also support "include if exists" rules. The return value also includes
"ismagic" now to avoid another future change, but that's not used yet.
- add re_match_include() which is now a wrapper around
re_match_include_parse() and behaves exactly as the old
re_match_include()
AppArmor 3 exposes kernel support for the kill and unconfined profile
modes. Make sure aa-status has basic support for these modes.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Processes that are confined by multiple profiles in a stack can have
more than one profile mode applied. Allow aa-status to report
processes that are in a mixed profile confinement mode.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Switch aa-status over to using the libapparmor proc accessor. This
will ensure that aa-status works correctly under LSM stacking kernels,
and if other new interfaces are added in the future.
Also at the same time switch over the splitting of context's to
the library provided aa_splitcon() fn.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Make aa-enabled able to distinguish being enabled and enabled without
access to the shared LSM iterfaces.
Signed-off-by: John Johansen <john.johansen@canonical.com>
With LSM stacking apparmor may be available even if it is not
enabled. If apparmor is available but not enabled it does not own
the /proc/<pid>/attr/* interface and should not use it as they
will not provide apparmor context.
Note: this also applies to the SO_PEERSEC interface, currently we
only fail is apparmor does not own the interface. Once the new interface
command is finalized supprt to use that will be added.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
LSM stacking makes the old /proc/<pid>/attr/* interfaces potentially
be shared such that the values returned might be for a different LSM
than apparmor.
Use the new apparmor dedicated interface at /proc/<pid>/attr/apparmor/*
if it is present.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
libapparmor and the pyton tools now require python3 but on systems
where python2 is the default the build will fail unless the PYTHON=
env var is set to the systems python3.
Instead just force autoconf to search for python3 as the python
version to use.
Signed-off-by: John Johansen <john.johansen@canonical.com>
When libintl is available on the system, we need to link against the
libintl library too.
Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
In order to have all the prerequisite folders before actually installing
os dependent configuration files, we need to guarantee the correct
install sequence.
Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
If Swig builds the library against python 2 and the utils/tests
run using python3, parts of the library are in accesible with
failures like
Traceback (most recent call last):
File "test-pivot_root_parse.py", line 12, in <module>
import apparmor.aa as aa
File "/home/jj/apparmor/apparmor-jjohansen.git/utils/apparmor/aa.py", line 28, in <module>
import apparmor.logparser
File "/home/jj/apparmor/apparmor-jjohansen.git/utils/apparmor/logparser.py", line 19, in <module>
import LibAppArmor
ImportError: No module named LibAppArmor
Signed-off-by: John Johansen <john.johansen@canonical.com>
$ make check
will fail unless PYFLAKES=pyflakes3 is passed as an environment
variable. Fix it so make check will work by default.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The apparmor python tools are written assuming python3. But the tests
are still using python2. This can result in failures of the test suite
on some systems, depending on how libapparmor is built.
Eg. on Ubuntu 18.04
apparmor.git/utils$ PYTHON=/usr/bin/python3 make check PYFLAKES=/usr/bin/pyflakes3
LANG=C podchecker -warning -warning *.pod
aa-audit.pod pod syntax OK.
aa-autodep.pod pod syntax OK.
aa-cleanprof.pod pod syntax OK.
aa-complain.pod pod syntax OK.
aa-decode.pod pod syntax OK.
aa-disable.pod pod syntax OK.
aa-easyprof.pod pod syntax OK.
aa-enforce.pod pod syntax OK.
aa-genprof.pod pod syntax OK.
aa-logprof.pod pod syntax OK.
aa-mergeprof.pod pod syntax OK.
aa-notify.pod pod syntax OK.
aa-remove-unknown.pod pod syntax OK.
aa-sandbox.pod pod syntax OK.
aa-status.pod pod syntax OK.
aa-unconfined.pod pod syntax OK.
logprof.conf.pod pod syntax OK.
aa-notify syntax OK
Checking aa-easyprof
Checking aa-genprof
Checking aa-logprof
Checking aa-cleanprof
Checking aa-mergeprof
Checking aa-autodep
Checking aa-audit
Checking aa-complain
Checking aa-enforce
Checking aa-disable
Checking aa-status
Checking aa-unconfined
Checking apparmor
Checking test/common_test.py
Checking test/minitools_test.py
Checking test/test-aa-decode.py
Checking test/test-aa-easyprof.py
Checking test/test-aa.py
Checking test/test-aamode.py
Checking test/test-aare.py
Checking test/test-baserule.py
Checking test/test-capability.py
Checking test/test-change_profile.py
Checking test/test-common.py
Checking test/test-config.py
Checking test/test-dbus.py
Checking test/test-example.py
Checking test/test-file.py
Checking test/test-libapparmor-test_multi.py
Checking test/test-logparser.py
Checking test/test-mount_parse.py
Checking test/test-network.py
Checking test/test-parser-simple-tests.py
Checking test/test-pivot_root_parse.py
Checking test/test-profile-list.py
Checking test/test-profile-storage.py
Checking test/test-ptrace.py
Checking test/test-regex_matches.py
Checking test/test-rlimit.py
Checking test/test-severity.py
Checking test/test-signal.py
Checking test/test-translations.py
Checking test/test-unix_parse.py
make[1]: Entering directory '/home/jj/apparmor/apparmor-jjohansen.git/utils/test'
=== test-example.py ===
.......
----------------------------------------------------------------------
Ran 7 tests in 0.000s
OK
.......
----------------------------------------------------------------------
Ran 7 tests in 0.000s
OK
=== test-pivot_root_parse.py ===
Traceback (most recent call last):
File "test-pivot_root_parse.py", line 12, in <module>
import apparmor.aa as aa
File "/home/jj/apparmor/apparmor-jjohansen.git/utils/apparmor/aa.py", line 28, in <module>
import apparmor.logparser
File "/home/jj/apparmor/apparmor-jjohansen.git/utils/apparmor/logparser.py", line 19, in <module>
import LibAppArmor
ImportError: No module named LibAppArmor
Makefile:81: recipe for target 'check' failed
make[1]: *** [check] Error 1
make[1]: Leaving directory '/home/jj/apparmor/apparmor-jjohansen.git/utils/test'
Makefile:95: recipe for target 'check' failed
make: *** [check] Error 2
Signed-off-by: John Johansen <john.johansen@canonical.com>
This function was introduced as a temporary (ahem...) solution in 95404bb2f3 but was never really correct. It checked against other_value.regex (as a string!) and, while this was somewhat generous in the results, could have unintended side effects.
Better error out on the safe side and add/keep a few superfluous rules than having a wrong match in is_covered() and miss to add/keep a rule that would be needed.
The perfect solution would be to really compare one AARE against the other as the parser does. I'm not too keen to implement this in python, and will wait until someone provides this function (which the parser already has) via libapparmor ;-)
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/478
Acked-by: John Johansen <john.johansen@canonical.com>
Only bool and str keys may be overwritten.
Other key types (list, dict, *Ruleset) can have their content modified
(not __setitem__()s job), but replacing the whole collection should
never happen.
This function was introduced as a temporary (ahem...) solution in
95404bb2f3 but was never really correct.
It checked against other_value.regex (as a string!) and, while this was
somewhat generous in the results, could have unintended side effects.
Better error out on the safe side and add/keep a few superfluous rules
than having a wrong match in is_covered() and miss to add/keep a rule
that would be needed.
The perfect solution would be to really compare one AARE against the
other as the parser does. I'm not too keen to implement this in python,
and will wait until someone provides this function (which the parser
already has) via libapparmor ;-)
If create-apparmor.vim.py fails, an empty apparmor.vim gets created. The
next "make" run will assume that apparmor.vim was already created (the
file exists and has a new-enough timestamp) and will therefore skip the
create-apparmor.vim.py run, keeping the broken apparmor.vim forever.
Adjust the Makefile to delete apparmor.vim if the script fails. This
ensures that make tries again in the next run.
In focal users of mdns get denials in apparmor confined applications.
An exampel can be found in the original bug below.
It seems it is a common pattern, see
https://github.com/lathiat/nss-mdns#etcmdnsallow
Therefore I'm asking to add
/etc/mdns.allow r,
to the file
/etc/apparmor.d/abstractions/mdns"
by default.
--- original bug ---
Many repetitions of
audit: type=1400 audit(1585517168.705:63): apparmor="DENIED" operation="open" profile="/usr/sbin/chronyd" name="/etc/mdns.allow" pid=1983815 comm="chronyd" requested_mask="r" denied_mask="r" fsuid=123 ouid=0
in log. I use libnss-mdns for .local name resolution, so /etc/nsswitch.conf contains
hosts: files mdns [NOTFOUND=return] myhostname dns
and /etc/mnds.allow contains the domains to resolve with mDNS (in may case, "local." and "local"; see /usr/share/doc/libnss-mdns/README.html.)
Presumably cronyd calls a gethostbyX() somewhere, thus eventually trickling down through the name service switch and opening /etc/mdns.allow, which the AppArmor profile in the chrony package does not allow.
Fixes: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1869629
Signed-off-by: John Johansen <john.johansen@canonical.com>
abstractions/mesa: allow checking if the kernel supports the i915 perf interface
See merge request apparmor/apparmor!464
Acked-by: Vincas Dargis <vindrg@gmail.com>
Acked-by: Christian Boltz <apparmor@cboltz.de> for master and 2.13
The error:
type=AVC msg=audit(1585403559.846:34317577): apparmor="DENIED" operation="exec" profile="/usr/sbin/dnsmasq" name="/usr/libexec/libvirt_leaseshelper" pid=7162 comm="sh" requested_mas
k="x" denied_mask="x" fsuid=0 ouid=0
type=AVC msg=audit(1585403559.846:34317578): apparmor="DENIED" operation="open" profile="/usr/sbin/dnsmasq" name="/usr/libexec/libvirt_leaseshelper" pid=7162 comm="sh" requested_mas
k="r" denied_mask="r" fsuid=0 ouid=0
Looks like the path to libvirt_leasehelper is incorrect usr.sbin.dnsmasq, at least in gentoo. Patching the file fixes the problem:
issue: https://gitlab.com/apparmor/apparmor/-/issues/87
Signed-off-by: John Johansen <john.johansen@canonical.com>
This patch fixes a couple of nitpicks that I encountered packaging apparmor for buildroot:
1. In a cross-compiling environment, python executable cannot be trusted for getting the python settings because it is generally compiled for the host. For this reason, we should rely on target python-config.
1. Setup.py for libapparmor swig bindings is always called without taking into account the discovered settings from ac_python_devel.m4
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/462
When crosscompiling, setupy.py should be called passing the settings
discovered by ac_python_devel.m4 and not using the default system
settings.
Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
In a crosscompiling environment it's common to have a python executable
running for the host system with a python-config reporting the host
configuration and a second python-config reporting the target configuration.
In such cases, relying on the default oython-config is wrong and breaks
the cross compilation.
This patch adds a PYTHON_CONFIG variable that can be pointed to the second
python-config and fixes the rest of the m4 accordingly.
Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
Update usr.sbin.winbindd profile to allow krb5 rcache files locking
See merge request apparmor/apparmor!460
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.11..master
This way we could generate the capabilities in a way that works with
every version of make.
Changes to list_capabilities are intended to exactly replicate the old
behavior.
The library version must be consistent across releases. Since 2.13.4
and master use the same library version it needs to be updated.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Implement set of abstractions to handle opening uris via xdg-open and similar helpers used on different desktop environments.
Abstractions are intended to be included into child profile, together with bundle abstractions such as ubuntu-browsers, ubuntu-email and others, for fine-grained control on what confined application can actually open via xdg-open and similar helpers.
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/404
Acked-by: John Johansen <john.johansen@canonical.com>
Fixes two resource leaks. https://scan.coverity.com/projects/apparmor
I don't actually know how to link to the individual reports but the
first one comes from an early return. The second comes from an iterator
potentially being empty.
/run/uuidd/request is hardcoded in libuuid from util-linux and uuidd
listens on this socket to provide random and time-based UUIDs in a
secure manner (man 8 uuidd). Some applications (eg, python's uuid)
prefer to use this socket, falling back to getrandom(), /dev/urandom,
etc. Eg:
$ strace -f aa-exec -p test -- \
python3 -c 'import uuid ; print("%s\n" % str(uuid.uuid1()))'
...
socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = -1 EACCES (Permission denied)
getrandom("\x8e\x89\xa5\xe7\x39\x1b", 6, GRND_NONBLOCK) = 6
...
uuidd itself produces random numbers using getrandom() and
/dev/{,u}random (falling back to time-based if not), which are already
allowed in the base abstraction. The uuidd daemon, when available, runs
unprivileged under a dedicated user, so allowing read-only access to
/run/uuidd/request is reasonable.
Make kde-open5 abstraction more conservative by removing gstreamer
support by default. Update usage example to suggest conditionally including
gstreamer abstraction if required.
Update kde-open5 abstraction to not include accessibility abstraction by
default.
Update documentation to suggest adding it manually in child profile when
using kde-open5 if accessibility access is required by profile author.
Update exo-open abstraction to not include accessibility abstraction by
default.
Update documentation to suggest adding it manually in child profile when
using exo-open if accessibility access is required by profile author.
In case of error (for example failing to open provided uri) message box
is show, and alert sound is played. Currently `exo-open` abstraction
does not allow access to audio subsystems in result producing denials in
system logs.
Update abstraction to include `abstractions/audio`.
Implement set of abstractions to handle opening uris via xdg-open and similar
helpers used on different desktop environments.
Abstractions are intended to be included into child profile, together
with bundle abstractions such as ubuntu-browsers, ubuntu-email and others, for
fine-grained control on what confined application can actually open via xdg-open
and similar helpers.
This helped a lot to debug the CI failure caused by 3db14e8e / !430 (merged), and won't add any noise as long as configure succeeds.
Note that this MR is based on master without !433 (merged) which means it will fail in CI (and display config.log). After !433 (merged) got merged, I'll rebase it to show that it won't add additional noise as long as everything works.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/434
Acked-by: John Johansen <john.johansen@canonical.com>
Allow /usr/etc/ in abstractions/authentication
openSUSE (and hopefully some other distributions) work on moving shipped
config files from /etc/ to /usr/etc/ so that /etc/ only contains files
written by the admin of each system.
See https://en.opensuse.org/openSUSE:Packaging_UsrEtc for details and
the first moved files.
Updating abstractions/authentication is the first step, and also fixes
bugzilla.opensuse.org/show_bug.cgi?id=1153162
See merge request apparmor/apparmor!426
Acked-by: John Johansen <john.johansen@canonical.com> for 2.12..master
Some applications queries network configuration (using QNetworkConfigurationManager class in Qt and similar), and that produces DBus denials under AppArmor confinement when NetworkManager backend is used.
Add abstraction that allows most common read-only DBus queries for getting current network configuration from NetworkManager backend.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/409
Acked-by: John Johansen <john.johansen@canonical.com>
Requires NPAPI which no major browsers support anymore: Mozplugger npviewer / nspluginwrapper
The following have been discontinued Google Talk (and plugin) f-spot conkeror galeon Mozilla Prism rekonq Adobe Reader for Linux
Other gnome-codec-install is not in Ubuntu anymore gstreamer0.10 was replaced by 1.0
PR: https://gitlab.com/apparmor/apparmor/merge_requests/417
Acked-by: John Johansen <john.johansen@canonical.com>
Add support for out of band transitions that be to separate elements that can contain any input character. Out of band transitions can only be triggered by explicit transitions in the code (instruction stream). They function the same as the null transition except they work for all input and when writing expressions there is no need to not match them.
The out of band transitions show up in the dfa as negative offsets from the base. Currently only a single out of band transition of -1 is supported, but it is fairly trivial to add new ones if needed. The negative offset for out of band transitions were chosen instead of positive offsets that are out of input range to leave open the option of extending the input range in the future.
kernel patches: apparmor-kernel!6
v2.
fix spacing error identified by cboltz
update commit messages and signed-off-bys
allow globbing in xattrs to match NULL character
free leaked xattr cond_entry_list
make xattr generation conditional on kernel support
v3.
fix crash in commit: parser: make sure xattr cond_entry_list is not leaked
port eric's patch from !362 "support matching xattr keys but not values" to work with out of band transitions, and not a separate value array
v4.
double free fix crash of xattr.name in parser: make sure xattr cond_entry_list is not leaked
remove xattr/bad_XX tests that are no longer bad because attr names do not have to have a value specified
v5
fix bug in append_rule where the out of band transition was being converted to character 255
v6
fix bug in writing of dfa header flags value
v7
fix bug in out of band transitions where they could in certain cases be matched by an expression and hence not function as a separator.
fix xattr encoding so xattr presence can be determined distinctly from xattr value
fix xattr encoding so xattrs are not treated as optional
fix xattr encoding so that permissions are properly encoded at the end of each segment match
update xattr regression tests to make failures unique and reflect that nul character values are now allowed
v8
update transchar comment
v9
update tranchar comment
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Time Out
Update the xattr_profile.sh tests so that
- each test can be uniquely identified
- the embedded nul tests are updated to the now supported/passing state
Signed-off-by: John Johansen <john.johansen@canonical.com>
The current encoding makes every xattr optional and uses this to
propogate the permission from the tail to the individual rule match
points.
This however is wrong. Instead change the encoding so that an xattr
(unless optional) is required to be matched before allowing moving
onto the next xattr match.
The permission is carried on the end on each rule portion file match,
xattr 1, xattr 2, ...
Signed-off-by: John Johansen <john.johansen@canonical.com>
Make sure we can support optional xattrs distinct from optional xattr
values in the encoding.
Currently all xattrs specified are required to be present even
if there value is not specified. However under the old encoding there
was no way to distinguish if the presence of the xattr vs. the
xattr having a null length value.
Fix this so that if we decide to support optional xattrs it is possible
without having to change the abi.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Support profiles that choose to match the presence of an extended
attribute without validating its value. This lets AppArmor target xattrs
with binary data, such as security.ima and security.evm values. For
example, it's now possible to write a profile such as:
profile signed_binaries /** xattrs=(security.ima) {
# ...
}
Both presence and value matches can be used in the same profile. To
match a signed xattr, target both the xattr and the security.ima value:
profile python_script /** xattrs=(
security.evm
security.apparmor="python"
) {
# ...
}
Updated to work using out of band matching instead of separate data
array.
Signed-off-by: Eric Chiang <ericchiang@google.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
xattrs are a byte string that can contain all input characters including
the null character. Allow * ** and ? glob patterns to match the null
character while retaining their apparmor characteristics for '/'.
That is * and ? won't traverse a '/' treating it as a path element.
While ** will match anything.
Signed-off-by: John Johansen <john.johansen@canonical.com>
xattrs can contain NULL characters in their values which means we can
not user regular NULL transitions to separate values. To fix this
use out of band transition instead.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently the NULL character is used as an out of band transition
for string/path elements. This works for them as the NULL character
is not valid for this data. However this does not work for binary
data that can contain a NULL character.
So far we have only dealt with fixed length fields of binary data
making the NULL separator either unnecessary.
However binary data like in the xattr match and mount data field are
variable length and can contain NULL characters. To deal with this
add the ability to specify out of band transitions, that can only
be triggered by code not input data.
The out of band transition can be used to separate variable length
data fields just as the NULL transition has been used to separate
variable length strings.
In the compressed hfa out of band transitions are expressed as a
negative offset from the states base. This leaves us room to expand
the character match range in the future if desired and on average
makes the range between the out of band transition and the input
transitions smaller than would be had if the out of band transition
had been stored after the valid input transitions.
Out of band transitions in the dfa will not break old kernels
that don't know about them, but they won't be able to trigger
the out of band transition match. So they should not be used unless
the kernel indicates that it supports them.
It should be noted that this patch only adds support for a single
out of band transition. If multiple out of band transitions are
required. It is trivial to extend.
- Add a tag indicating support in the kernel
- add a oob max range field to the dfa header so the kernel knows
what the max range that needs verifying is.
- extend oob generation fns to generate oob based on value instead
of a fixed -1.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently the parser is not correctly setting the dfa flag value
and it hasn't been caught because base policy uses a flag value
of 0.
Signed-off-by: John Johansen <john.johansen@canonical.com>
As a step in preparing for out of band transitions and double walk
transitions rework the backend from using a char index to a class
with an larger range than char.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add nnp tests around the unconfined exception.
An unconfined task is allowed changing to a profile under nnp restrictions. However this has implications when the task has stacked confinement.
profile//&unconfined
profile//&:ns:unconfined
...
will not be caught by the unconfined test. Instead the unconfined exception needs to be tested as part of the subset test. Add tests to ensure we can catch regressions around a stacked unconfined profile.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/424
Signed-off-by: John Johansen <john.johansen@canonical.com>
pyflakes 2.x nowadays complains about
aa-genprof:60: undefined name 'PermissionError'
We already deprecated py2 support in 2.11 (and obviously nobody tests
with py2 anymore), therefore recommending pyflakes3 makes sense.
Maybe we should also change our Makefiles to use py3 and pyflakes3 by
default, but that will be another (master-only) commit.
'localinclude' is/was meant to have 'local/*' includes separate, but
it's write-only and never used, which makes it useless.
Additionally, it causes a crash in the aa-* tools which gets fixed by
removing all the 'localinclude'-related code (what a big word for two
lines ;-)
References: https://bugs.launchpad.net/apparmor/+bug/1848227
openSUSE (and hopefully some other distributions) work on moving shipped
config files from /etc/ to /usr/etc/ so that /etc/ only contains files
written by the admin of each system.
See https://en.opensuse.org/openSUSE:Packaging_UsrEtc for details and
the first moved files.
Updating abstractions/authentication is the first step, and also fixes
bugzilla.opensuse.org/show_bug.cgi?id=1153162
The unconfined exception needs to be applied even when a stack is
being used. When a stack is in use it prevents the unconfined
test from being used and instead it must be done as part of the
subset test.
Signed-off-by: John Johansen <john.johansen@canonical.com>
drop deprecated perl code
* drop repair_obsolete_profiles (updated profiles for abstraction renames pre-2006)
* drop ancient perl aa-* tools and their modules (rewritten to python in 2.9)
* drop rc.aaeventd.{redhat,suse} initscripts (aaeventd was dropped long ago, therefore we don't need to keep its initscripts)
See merge request apparmor/apparmor!423
Acked-by: John Johansen <john.johansen@canonical.com>
Applications running under Xwayland in a GNOME+Wayland session need read access to this file since:
a8984a81c2
… that was first included in mutter 3.33.3.
This rule is presumably only needed for GNOME+Wayland sessions, so one could argue that it should live in abstractions/wayland instead, but Jamie argued that it should be in the X abstraction because Xwayland is a X server.
MR: https://gitlab.com/apparmor/apparmor/merge_requests/419
Bug-Debian: https://bugs.debian.org/935058
Acked-by: John Johansen <john.johansen@canonical.com>
879531b36ec3dfc7f9b72475c68c30e4f4b7b6af changed access for
@{HOME}/.{,cache/}fontconfig/** to include 'w'rite. Fontconfig has been
a source of CVEs. Confined applications should absolutely have read
access, but write access could lead to breaking out of the sandbox if a
confined application can write a malformed font cache file since
unconfined applications could then pick them up and be controlled via
the malformed cache. The breakout is dependent on the fontconfig
vulnerability, but this is the sort of thing AppArmor is meant to help
guard against.
Bug-Debian: https://bugs.debian.org/935058
Applications running under Xwayland in a GNOME+Wayland session need read access
to this file since:
a8984a81c2
… that was first included in mutter 3.33.3.
This rule is presumably only needed for GNOME+Wayland sessions, so one could
argue that it should live in abstractions/wayland instead, but Jamie argued that
it should be in the X abstraction because Xwayland is a X server.
Requires NPAPI which no major browsers support anymore:
Mozplugger
npviewer / nspluginwrapper
The following have been discontinued
Google Talk (and plugin)
f-spot
conkeror
galeon
Mozilla Prism
rekonq
Adobe Reader for Linux
Other
gnome-codec-install is not in Ubuntu anymore
gstreamer0.10 was replaced by 1.0
With the effort to remove perl, it makes no sense to keep the depracted
perl code around for new releases.
Signed-off-by: John Johansen <john.johansen@canonical.com>
aa-status crashed if a profile name contains an opening parenthesis
because the regex enforces (simplified) '^[^(]* \(.*\)' when reading
/sys/kernel/security/apparmor/profiles
This obviously doesn't match if a profile name contains '(' which is
rare and strange, but still allowed, and the match result "None" then
crashes aa-status.
Adjust the regex to allow all chars instead of all except '(' to handle
these corner cases.
Note that '(enforce)' and '(complain)' still get read correctly because
the regex ends with '\((\w+)\)$' and therefore enforces matching
"something inside parenthesis at the end of the line".
This bug exists since aa-status was rewritten into python, and even
existed in the perl version before. However, in the perl version, the
regex matching was protected with an if so profile names with '(' were
skipped and hidden from the aa-status output.
Fixes: https://gitlab.com/apparmor/apparmor/issues/51
When a filesystem is mounted with the option *user*, the file selection
dialogue, e.g. in *Evince*, triggers an access of */run/mount/utab*, which
comes from *libmount* and should be allowed.
Winbind requires access to /var/cache/samba/msg.lock/*.
Move msg.lock/ to abstractions/samba.
On Arch Linux Winbind's pid is set to /run/winbindd.pid.
Signed-off-by: nl6720 <nl6720@gmail.com>
As discussed in https://gitlab.com/apparmor/apparmor/merge_requests/395
RE_LOG_ALL should be more broad so that it doesn't accidently
overlook/ignore valid log events.
Instead of adding more and more known log formats to logparser.py,
simplify the regex to do only a basic check.
If we really hand over a line to libapparmor that isn't an AppArmor log
event, worst thing that can happen is that we waste a few milliseconds
for handing over that line to libapparmor, and get AA_RECORD_INVALID as a
result.
Some applications queries network configuration (using
QNetworkConfigurationManager class in Qt and similar), and that produces
DBus denials under AppArmor confinement when NetworkManager backend is
used.
Add abstraction that allows most common read-only DBus queries for
getting current network configuration from NetworkManager backend.
Improve readability by being explicit about the value to use when the
conditional evaluates to true.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Suggested-by: John Johansen <john.johansen@canonical.com>
Commit ad81ea0e67 ("tests: Add option to dump policy cache dir with
the libapparmor wrapper") modified aa_policy_cache.c to call
aa_policy_cache_dir_path_preview(). That added a hard dependency on
libapparmor >= 2.13, which is the first version to add
policy_cache_dir_path_preview() to libapparmor. The dependency makes it
impossible to build and run the upstream regression tests against an
installed libapparmor older than 2.13 due to aa_policy_cache.c failing
to build.
Add a compatible aa_policy_cache_dir_path_preview() when building
aa_policy_cache.c against a libapparmor older than 2.13 and newer than
or equal to 2.10.
Fixes: ad81ea0e67 ("tests: Add option to dump policy cache dir with the libapparmor wrapper")
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Correct the long option used to print the cache directory.
Fixes: e9d9395f91 ("parser: Add option to print the cache directory")
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Test the profile transition limits imposed by NO_NEW_PRIVS to ensure
that behavior doesn't unexpectedly change.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
commit 7297e2f6a8 assumed that exec events
always have a "target=...", but this is only true for events in complain
mode. In enforce mode, the log line doesn't include "target=...".
This commits sets an empty target instead of erroring out on every exec
event in enforce mode.
Also adjust ask_exec() in aa.py to only update
hashlog[aamode][target_profile]['final_name'] if target_profile is set
(hashlog[aamode][''] doesn't exist and trying to write to it would error
out)
- in complain mode, exec events include target=...
- in enforce mode, the log message doesn't include target=...
Note that the utils tests skip exec events (they'd be interactive),
therefore exec*.profile doesn't contain an exec rule.
convert_regexp() needs to escape '(' and ')' in filenames taken from a
logfile to get rid of their special meaning, and to avoid a crash on
unbalanced parenthesis (which makes the regex invalid if they are not
escaped).
Note: The added tests include an example log line, but the tests itsself
don't/didn't trigger the crash because they don't call convert_regexp().
Fixes: https://bugs.launchpad.net/bugs/1835311
v2:
- parse partial log line broken at \n
- add testcase_dbus_10.* for partial log line
- remove quotes from testcasw_dbus_09.profile
The following log format has been seen in the wild, and currently results
in a RECORD_INVALID
[4835959.046111] audit: type=1107 audit(1561053426.749:186): pid=640 uid=103 auid=4294967295 ses=4294967295 msg='apparmor="ALLOWED" operation="dbus_method_call" bus="system" path="/org/freedesktop/systemd1" interface="org.freedesktop.systemd1.Manager" member="LookupDynamicUserByName" mask="send" name="org.freedesktop.systemd1" pid=20596 label="/usr/sbin/sshd" peer_pid=1 peer_label="unconfined"
exe="/usr/bin/dbus-daemon" sauid=103 hostname=? addr=? terminal=?'
Test parsing the above message with and without the \n embedded between
peer_label= and exec=
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add for Certbot on openSUSE Leap
The default path is `/etc/certbot/archive/{some domain}/{file name}.pem`
See merge request apparmor/apparmor!397
Acked-by: Christian Boltz <apparmor@cboltz.de>
The mult_mount test creates a small disk image, formats it, and mounts
it in multiple locations in preparation for the tests. However, the
created raw file (80KB) is too small to make a working file system if
4K blocks are used by mkfs. In Ubuntu 19.10, the default was recently
changed for mkfs to default to always using 4K blocks, causing the
script to fail.
We could force mkfs to use 1K blocks, but instead, in case some future
version of mkfs decides not to support 1K blocks at all, we bump up the
size of the disk image to 512KB; large enough to work with 4K blocks
yet small enough to be workable in small scale test environments.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Bug: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1834192
MR: https://gitlab.com/apparmor/apparmor/merge_requests/396
Instead of setting SFS_MOUNTPOINT in is_apparmor_loaded() (which is
called in most cases) and in is_container_with_internal_policy() (which
covers/fixes the remaining cases), set it globally.
This also fixes a bug in is_container_with_internal_policy() (introduced
in f10e72a14f) where the variable
definition tried to use the no longer existing $MODULE variable and
therefore got a wrong path for $SFS_MOUNTPOINT.
Besides this bug, there's a minor behaviour change / improvement if
securityfs isn't mounted - "file not found" error messages will now
contain the full/correct path ;-)
This change/cleanup is a follow-up of
https://gitlab.com/apparmor/apparmor/merge_requests/363 and some IRC
discussions 2019-04-16.
When dc010bc034 was
backported to the apparmor-2.13 branch (in commit
75236d62e2), it did not take into
account cb8c3377ba, which creates the
common/list_af_names.sh script as used in the test case, was not also
backported to the apparmor-2.13 branch.
Change the test case to get the list of network AF names via the same
make invocation taken by the utils/vim/create-apparmor.vim.py script
before the common/list_af_names.sh existed.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/391
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
update postfix profiles
On openSUSE Leap 15.1, the postfix binaries live in /usr/lib/postfix/bin/ which was not covered in the postfix.\* attachment and mrix rules.
Also add several permissions to the postfix.\* profiles needed on openSUSE Leap 15.1.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/380
Signed-off-by: John Johansen <john.johansen@canonical.com>
is_container_with_internal_policy() is called independently of
apparmor_*() in the systemd unit and potentially other consumers of
rc.apparmor.functions. When the unit and rc.apparmor.functions functions
were rewritten, they were written so that SFS_MOUNTPOINT was only set in
is_apparmor_loaded(), but this is only called in apparmor_start(),
remove_profiles(), apparmor_kill(), apparmor_restart(), apparmor_try_restart()
and apparmor_status() and not is_container_with_internal_policy().
While it is clear that is_container_with_internal_policy() is meant to
be called before apparmor_start(), is is unclear why SFS_MOUNTPOINT is
only defined in is_apparmor_loaded(). There are several ways to fix
this:
1. update is_container_with_internal_policy() to call is_apparmor_loaded()
2. identify the callers of is_container_with_internal_policy() and have
them call is_apparmor_loaded()
3. reorganize the code to remove duplicate calls and assignments
4. define SFS_MOUNTPOINT along with SECURITYFS and MODULE, at the top
level
5. also define SFS_MOUNTPOINT in is_container_with_internal_policy()
'1' would result in redundant calls in many common cases since the
systemd unit would call is_apparmor_loaded() both in
is_container_with_internal_policy() and prior to other calls.
'2' would like break consumers of rc.apparmor.funcions, like
Debian/Ubuntu's profile-load.
'3' is perhaps ok, but requires more effort and is regression-prone.
'4' seems the simplest, most correct fix
'5' is what this patch implements, which is as simple as '4' but tries
to maintain the original author's intent of when to set SFS_MOUNTPOINT.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/363
Signed-off-by: Jamie Strandboge <jamie@strandboge.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add a target to install build dependencies for the apparmor regression
tests. Currently supports Ubuntu and Debian distros.
Signed-off-by: Mike Salvatore <mike.salvatore@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
The translated action character for Deny conflicted with the
untranslated action character for Finish in the Swedish translation.
Remote it, and hope for more action translations.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
The parser currently skips the cache if optimizations are specified
because it can not determine if the cached policy was compiled
with the specified optimization. However this causes cache misses
even if policy is cached with those options, and distros are setting
some optimizations by default.
Instead of skipping reading the cache if optimizations are set, users
can force overwriting the cache if needed, until the parser can
store aditional meta info in the cache.
BugLink: http://bugs.launchpad.net/bugs/1820068
Signed-off-by: John Johansen <john.johansen@canonical.com>
finishing was always false, resulting in always returning 'NORMAL'.
Remove the variable, remove the unused condition - and change the only
place (aa-genprof) that expected a return value from do_logprof_pass()
to not expect it anymore.
found by Coverity, CID 198637
Rewrite log handling
logparser.py puts each log event on a big "stack" in self.pid. Later,
handle_children() in aa.py then converts that (named 'log' in aa.py) to
the prelog hasher.
This commit changes logparser.py to create a prelog-like structure itsself
(named hashlog), which
- removes one level of indirection
- probably saves some memory because the hashlog automatically
de-duplicates events
In aa.py, collapse_log() gets updated to work with hashlog. (There's
also a handle_hashlog() function in this patch series, but it didn't
survive the final patches ;-)
OTOH, the now superfluous code handling capability, network etc.
events gets removed from handle_children(). The remaining parts of
this function get split into ask_exec() and ask_addhat().
logparser.py gets a new function init_hashlog() to initialize hashlog
for each profile. It also gets changed to store capability, network etc.
events into hashlog instead of storing them in self.pid.
hashlog uses the full profile name as key, which is the first baby step
to support nested child profiles. (for now, collapse_log() still
splits the profile name into profile and hat.)
There are many more details, see the individual commits ;-)
See merge request apparmor/apparmor!377
Acked-by: Seth Arnold <seth.arnold@canonical.com>
looping through the first 16 loop devices to find a free device will
fail if those mount devices are taken, and unfortunately there are
now services that use an excessive amount of loop devices causing
the regression test to fail.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
If final_name still includes null-*, that's most likely caused by nested
execs which aren't supported by the tools yet. Ignoring them is better
than creating a useless null-* hat.
Note: The tools always had this restriction, so this is not a regression ;-)
Also note that test-libapparmor-test_multi expects that null-* hats get
created (which makes sense because the one-line log sniplets don't have
any exec indication), therefore add an optional parameter to keep this
behaviour for the tests.
Now that all log events arrive in hashlog, having a separate 'prelog' no
longer makes sense. Changing collapse_log() to accept 'hashlog' directly
removes that level of indirection.
It's only used by two functions:
- handle_hashlog() - writes to prelog, and now returns it
- collapse_log() - reads prelog, and now gets it as parameter
'final_name' by default is the profile name, but ask_exec() will change
it for the target profile (which is a null-* profile at this stage)
based on exec mode choice. ask_addhat() will also change it based on the
chosen hat.
Choosing "deny" or "unconfined" will result in an empty final_name and
ignoring these log events.
All other choices set final_name to the full profile name ("foo" for Px,
"foo//bar" for Cx, current profile for ix).
Also fix the order of handling log events - since ask_exec() changes the
hashlog final_name, it has to run first so that ask_addhat() (which
"only" adjusts the hat name in final_name) and handle_hashlog() can work
with the updated profile name.
Finally, update test-libapparmor-test_multi.py to ignore final_name when
checking if hashlog is empty, and fix the call order of ask_exec() etc.
profile and hat don't get used in the following lines, and later get
overwritten in the next round of the for loop.
The deleted code was last useful for setting the (dropped)
profile_changes array.
... and self.pid which is also unused.
This simple change also means to adjust all the code that uses ReadLog.
We get rid of log_pid in aa.py, and have to change lots of test-*
After all the changes in this branch, parse_event_for_tree always
returns None, which makes the "if event is not None" branch dead code.
This branch was the only place where add_to_tree() was called, therefore
remove this function.
This also makes self.pid and self.log unused. They'll be removed with
separate commits.
The only remaining job of handle_children() was to handle exec events.
(And recursively calling itsself if it hits nested log events, but
logparser.py never created such a log structure.)
Therefore:
- drop the dead code handling nested log (type != str)
- rename the remaining function to ask_exec()
- drop checks for typ = 'exec' (now done as part of the for loop
- drop the "else" branch for unknown event types
- change 'return' to 'continue' because ask_exec handles all exec events
in a loop instead of being called multiple times
- oh, and of course switch over to using hashlog
Finally, change do_logprof_pass() and the tests to call ask_exec()
instead of handle_children().
While on it, update a comment in test-translations.py which held the
last reference to handle_children().
The 'hat' and 'prog' variables are no longer needed, drop them.
Also no longer include denied_mask in the event - operation='exec' means
an exec event, no need to additionally hand over 'x' permissions.
Note: This commit introduces a "brain split", which means exec handling
is temporarily broken. Later commits will fix this.
parse_event_for_tree() always sets prog = 'HINT' and to_name = '' for
exec events.
The only exception is in the 'mandatory profile missing' case where
prog = aamode and to_name = target_profile, but I've never seen that in
practise.
This means the prog != 'HINT' branch in exec handling never gets used
and can be dropped.
Erroring out "if to_name:" also never gets used (to_name is always ''),
therefore drop it as well.
- replace/merge 'uhat' with 'hat'
- adjust ProfileStorage comment to the new function name
- initialize default hat if it gets chosen and doesn't exist yet
- don't exit the function if "Deny" gets chosen. Instead, continue with
the next hat to (possibly) add
Adjust logparser.py to store change_hat events in hashlog.
In aa.py,
- split off ask_addhat() from handle_children()
- change ask_addhat() to use hashlog
- call ask_addhat() from do_logprof_pass()
Also call ask_addhat() in test-libapparmor-test_multi.py to keep it in
sync with do_logprof_pass().
All the variables initialized at the start of the function always get
overwritten in the loop, therefore it's superfluous to initialize them.
Also inline 'entries' to the only place that uses it.
According to John, this isn't supported since 10 years.
In case you wonder why this commit removes the "fork" handling in
handle_children() - logparser.py names clone events "fork" on the event
stack.
In logparser parse_event_for_tree() path event handling, drop mapping
permissions for request_mask because request_mask never gets used.
Also drop the validate_log_mode() call because the function has its own,
more strict check since the last commit.
In aamode.py, drop the now unused validate_log_mode() and
hide_log_mode() functions and the LOG_MODE_RE regex.
Finally, drop the validate_log_mode() tests from test-aamode.py
In logparser.py parse_event_for_tree, convert path handling to hashlog.
While on it, include 'owner' as part of hashlog so that aa.py doesn't
need to guess.
Also switch to a simple for loop instead of using log_str_to_mode() from
aamode.py to convert denied_mask to hasher keys (which would have been
needed to allow merging of several log events for the same path anyway).
Note that the check for 'mrawlk' (intentionally without 'x') is more
strict than the validate_log_mode(), but it should still cover all file
permissions. (validate_log_mode() also allows things like 'Px', which
we'll never hit in a logfile.)
In aa.py collapse_log() update the handling of path events to match the
additional [owner] key in hashlog/prelog. This makes the owner detection
in collapse_log() superfluous.
In aa.py handle_children(), remove 'path' handling from the 'path' or
'exec' section, and add an 'if True:' to avoid lots of whitespace
changes.
In aamode.py, drop the now unused split_mode() function, and
AA_OTHER_REMOVE() that was only used by split_mode().
Finally, remove sample log events with null-* hats from the list of
known failures in test-libapparmor-test_multi.py (we no longer filter
out null-* hats), and fix whitespace in two expected profiles.
According to John, 'inode_permission' wasn't used for 10 years.
This little change also means that we now have a clear separation
between "exec" and "normal" (mrwlk) file events.
Also drop the now superfluous makes peek_at_next_log_entry() and
throw_away_next_log_entry() functions.
logparser.py puts each log event on a big "stack" in self.pid. Later,
handle_children() in aa.py then converts that (named 'log' in aa.py) to
the prelog hasher.
This commit changes logparser.py to create the prelog structure itsself
(named hashlog), which
- removes one level of indirection
- probably saves some memory because the hashlog automatically
de-duplicates events
This commit does this for capability, network and signal events, and
adds the infrastructure needed for all event/rule types.
In aa.py, the new function handle_hashlog() copies the hashlog content
to prelog. OTOH, the now superfluous code handling capability, network
and signal events gets removed from handle_children().
Long-term, hashlog will replace log in aa.py. When this is done,
handle_hashlog() will be replaced by a simple prelog = hashlog.
logparser.py gets a new function init_hashlog() to initialize hashlog
for each profile. It also gets changed to store capability, network and
signal events into hashlog instead of storing them in self.pid.
hashlog uses the full profile name as key, which is the first baby step
to support nested child profiles. (for now, handle_hashlog() still
splits the profile name into profile and hat.)
Known issue: The new implementation doesn't handle exec yet, which means
that events get lost at the exec boundary (= in cases aa-logprof asks
which execute mode to use). This will be fixed in a later commit.
This function was meant to set a process running under a null-* profile
to its "real" profile after deciding about the exec mode/target.
However, this is not supported in the kernel.
"Luckily" the function was also broken and exited early, which
successfully prevented erroring out.
All that means set_process() is useless and we can drop it.
logparser cleanup
- drop old commented out code
- inline two small functions into the places calling them to make the callstack smaller and easier to understand
- rename 'netdomain' log events to 'network' to match rule name
- handle_children: raise exception on unknown event type
As usual, reading the individual commits makes the changes easier to understand.
See merge request apparmor/apparmor!374
Acked-by: Seth Arnold <seth.arnold@canonical.com>
parser/rc.apparmor.functions: fix minor issues detected by shellcheck/shellharden
See merge request apparmor/apparmor!370
Acked-by: Christian Boltz <apparmor@cboltz.de>
The apparmor.d manpage listed 'to' as an alternative for '->' in link
rules.
However, the parser doesn't accept 'to', none of our examples and tests
include it, and nobody ever complained about it. Therefore I'll call
this a documentation bug ;-) and simply adjust the manpage to only list
'->' as valid syntax.
Fix error 'KeyError: 'logfiles'' when no logprof.conf exists
See merge request apparmor/apparmor!365
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.12..master
- Code layout based on aa-genprof example
- Extend Python dependencies to cover new need by aa-notify
- Update documentation after aa-notify is no longer in Perl
Related to #22.
Fixes error message:
Traceback (most recent call last):
File "./aa-notify", line 523, in <module>
main()
File "./aa-notify", line 399, in main
aa.set_logfile(args.file)
File "/home/otto/koodia/apparmor/utils/apparmor/aa.py", line 1762,
in set_logfile
print(conf.find_first_file(cfg['settings']['logfiles']))
File "/usr/lib/python3.6/configparser.py", line 1233, in __getitem__
raise KeyError(key)
KeyError: 'logfiles'
Add tests for shared aa library functions used in command line scripts
See merge request apparmor/apparmor!328
Acked-by: Christian Boltz <apparmor@cboltz.de>
Add some tests with the complex profile name (including alternations and
wildcards) to ensure we don't break such cases in the future.
These tests are based on the log from the (invalid) bugreport
https://gitlab.com/apparmor/apparmor/issues/26
Since !345 the set of permissions that are granted (get_file_perms_2)
or suggested (propose_file_rules) has changed. These new sets are
expected due to the changes brought by this MR, so let's adjust
the test suite accordingly.
base abstraction: allow mr on *.so* in common library paths.
For example, VirtualBox guests have /usr/lib/VBoxOGL.so.
Without this changes, in a VirtualBox VM with VBoxVGA graphics,
at least one Qt5 application (OnionShare) won't start and display:
ImportError: libGL.so.1: failed to map segment from shared object
… and the system logs have:
apparmor="DENIED" operation="file_mmap" profile="/usr/bin/onionshare-gui" name="/usr/lib/VBoxOGL.so" pid=11415 comm="onionshare-gui" requested_mask="m" denied_mask="m" fsuid=1000 ouid=0
While this works fine with VBoxSVGA and VMSVGA when 3D acceleration is enabled.
So let's not assume all libraries have a name that starts with "lib".
Bug-Tails: https://redmine.tails.boum.org/code/issues/16414
Candidate for master and 2.13.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/345
Signed-off-by: John Johansen <john.johansen@canonical.com>
For example, VirtualBox guests have /usr/lib/VBoxOGL.so.
Without this changes, in a VirtualBox VM with VBoxVGA graphics,
at least one Qt5 application (OnionShare) won't start and display:
ImportError: libGL.so.1: failed to map segment from shared object
… and the system logs have:
apparmor="DENIED" operation="file_mmap" profile="/usr/bin/onionshare-gui" name="/usr/lib/VBoxOGL.so" pid=11415 comm="onionshare-gui" requested_mask="m" denied_mask="m" fsuid=1000 ouid=0
While this works fine with VBoxSVGA and VMSVGA when 3D acceleration is enabled.
So let's not assume all libraries have a name that starts with "lib".
Add userland support for matching based on extended file attributes. This
leverages DFA based matching already in the kernel:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=8e51f908https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=73f488cd
Matching is exposed via flags on the profile:
/usr/bin/* xattrs=(user.foo=bar user.bar=foo) {
# ...
}
xattr values are appended to the existing xmatch via a null transition.
$ echo '/usr/bin/* xattrs=(user.foo=foo user.bar=bar) {}' | \
./parser/apparmor_parser -QT -D expr-tree
DFA: Expression Tree
/usr/bin/[^\0000/]([^\0000/])*(\0000bar)?(\0000foo)?< 0x1>
DFA: Expression Tree
(\a|(\n|(\0002|\t)))< 0x4>
Tested manually on a 4.19 kernel via QEMU+KVM.
TODO:
* ~~Add regression tests~~ (EDIT: done)
* ~~EDIT: add support in the tools~~ (EDIT: done)
Questions for reviewers:
* ~~parser/libapparmor: regex construction probably needs cleaning up~~ (EDIT: done)
* ~~parser/parser_regex.c: confused what xmatch length is for~~ (EDIT: done)
/cc @mjg59
PR: https://gitlab.com/apparmor/apparmor/merge_requests/270
Signed-off-by: John Johansen <john.johansen@canonical.com>
Compiling the parser currently prints a deprecation warning. Remove
throw(int) annotations from function signatures. These aren't required
to catch exceptions. This gets us closer to possibly enabling '-Werror'
in the future.
For example, the following program catches the exception without a
throw(int) annotation:
#include <iostream>
void throw_an_error()
{
throw 3;
return;
}
int main ()
{
try
{
throw_an_error();
}
catch (int e)
{
std::cout << "caught exception " << e << '\n';
}
return 0;
}
This program prints:
$ g++ -o error error.cc
$ ./error
caught exception 3
PR: https://gitlab.com/apparmor/apparmor/merge_requests/356
Signed-off-by: Eric Chiang <ericchiang@google.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Note that $PARSER_OPTS has to stay without quotes because it can
sometimes be empty, and would (if quoted) be interpreted as empty
filename by apparmor_parser
Compiling the parser currently prints a deprecation warning. Remove
throw(int) annotations from function signatures. These aren't required
to catch exceptions.
For example, the following program catches the exception without a
throw(int) annotation:
#include <iostream>
void throw_an_error()
{
throw 3;
return;
}
int main ()
{
try
{
throw_an_error();
}
catch (int e)
{
std::cout << "caught exception " << e << '\n';
}
return 0;
}
This program prints:
$ g++ -o error error.cc
$ ./error
caught exception 3
Signed-off-by: Eric Chiang <ericchiang@google.com>
The tools also have a list of network keywords, update it:
- add xdp and qipcrtr
- move ib and mpls to match the kernel order
Also add a test to ensure that (at least) the keywords provided by the
running kernel are listed in network_domain_keywords.
Extend the subshell so that the actual (possibly non-zero) value of
$retval gets returned. Before, the changed value was lost at "done"
(= leaving the subshell), and the initial $retval=0 was returned.
(found with shellcheck)
The calling function and the merge() macro both make use of a variable
named "i" but this causes problems when merging. The aa_policy_cache.sh
test script experiences "bad file descriptor" failures due to the merged
list containing invalid fd values (-1).
Fix this by renaming merge()'s index variables from i and j to y and z
to hopefully prevent future overlap. The better fix here would be to
convert merge() to a function but that's a more intrusive change and I
don't have an easy way to test the overlay feature.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
qsort()'s _size_ parameter is used to indicate the size of the elements
in the _base_ array parameter. Adjust the third argument to qsort() to
indicate that we're dealing with an array of struct dirent pointers
rather than an array of struct dirent.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Adjust aa_policy_cache.sh to handle the additional layer in the
directory hierarchy when determining where the policy cache binaries are
stored. This is needed due to the multicache changes that allow multiple
policy caches to exist on a single system.
Differentiate between the cache location (the top level directory
containing all caches) and the cache directory (the directory used to
store the cached policies).
Use the libapparmor wrapper to get the cache directory for the given
cache location and the features of the currently running kernel.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Print the policy cache directory path for the features of the currently
running kernel to stdout so that the aa_policy_cache.sh regression test
script can make use of it when writing out binary policy files.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
parser: determine xmatch priority based on smallest DFA match
The length of a xmatch is used to prioritize multiple profiles that
match the same path, with the intent that the more specific match wins.
Currently, the length of a xmatch is computed by the position of the
first regex character.
While trying to work around issues with no_new_privs by combining
profiles, we noticed that the xmatch length computation doesn't work as
expected for multiple regexs. Consider the following two profiles:
profile all /** { }
profile bins /{,usr/,usr/local/}bin/** { }
xmatch_len is currently computed as "1" for both profiles, even though
"bins" is clearly more specific.
When determining the length of a regex, compute the smallest possible
match and use that for xmatch priority instead of the position of the
first regex character.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/326
Signed-off-by: John Johansen <john.johansen@canonical.com>
profiles/gnome: add @{HOME}/.cache/gtk-3.0
Found this path is used by gtk_compose_hash_get_cache_path() in
gtkcomposetable.c.
See merge request apparmor/apparmor!342
Acked-by: Jamie Strandboge <jamie@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Replace deprecated assertEquals with assertEqual
assertEquals is deprecated since Python 2.7 and 3.2.
See merge request apparmor/apparmor!347
Acked-by: Eric Chiang <ericchiang@google.com>
Update the indetation of work_spawn to correct for the changes made in
cb43e57d27 ("parser: Fix parser failing to handle errors when setting up work")
the indetation was not updated in that patch to make the changes made
easier to review and see in diffs.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Those 3 login daemons should have similiar needs and thus similar
profiles. IMAP is likely the most tested one so let's align the
other 2 with it. Unix and TCP sockets rules were added to pop3-login
after the removal of abstractions/nameservice that included them
implicitly.
Signed-off-by: Simon Deziel <simon@sdeziel.info>
When using passdb/userdb not requiring root (!= /etc/shadow access)
it is recommended to run the auth processes as non root and chroot'ed
Signed-off-by: Simon Deziel <simon@sdeziel.info>
Debian and Ubuntu have releases coming out with usr-merge in place. For these systems, /bin and /sbin are symlinks to their respective /usr directories. This breaks a few tests in the python utils and in the regression tests. This patch series fixes them, mostly by performing realpath() calls when necessary. For the ptrace regression test, it copies the called /bin/true binary into the created temporary directory and executes it from there. (Good for other reasons, too.)
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
These patches should be safe to backport to the supported releases (but I have not attempted that, yet).
See merge request apparmor/apparmor!331
In a usr-merge ubuntu/debian environment /bin is a symlink to
/usr/bin, which causes invalid apparmor policy to be generated for
/bin/true. Instead, copy /bin/true to the per test temporary directory
and execute it from there.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
When run locally on a development machine or in production, the full test
is likely to run. However inside a CI system container 'last' might fail
to show last login or there might not be access to kern.log and the test
will automatically skip those without failing the whole test suite.
This will help ensure the future rewrite of aa-notify from Perl to Python
is less likely to introduce regressions. Tests run the command line utility
via a subprocess so it does not matter that the tests are in Python but
the aa-notify utility is in Perl (for now).
The length of a xmatch is used to prioritize multiple profiles that
match the same path, with the intent that the more specific match wins.
Currently, the length of a xmatch is computed by the position of the
first regex character.
While trying to work around issues with no_new_privs by combining
profiles, we noticed that the xmatch length computation doesn't work as
expected for multiple regexs. Consider the following two profiles:
profile all /** { }
profile bins /{,usr/,usr/local/}bin/** { }
xmatch_len is currently computed as "1" for both profiles, even though
"bins" is clearly more specific.
When determining the length of a regex, compute the smallest possible
match and use that for xmatch priority instead of the position of the
first regex character.
This makes it possible for e.g. command line tools to have the --debug
option and when invoked print the existing debug messages directly to
stderr so the user running the command can see them.
kde: fix global settings access for Kubuntu and openSUSE
See merge request apparmor/apparmor!322
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.12..master
Defining 'stderr = subprocess.STDOUT' as a default value for function
did not work and the 'stderr' was always empty, thus also 'outerr' was
always empty and not standard error contents was ever considered in any
way.
Best in fact was to remove excess function arguments as they were not even
used and replace it with a simpler and less error prone structure.
Even after reading 'stderr' correctly it did not help much as all tests
used 'assertIn' which ignored excess output. Better replace the normal
output with the error output if there ever was something, since stderr
is most likely a serious thing and tests should stop on it.
- Add documentation stub for what the UI module is and how JSON works
- Make indentation consistent
- Use pythonic 'if' clauses
- Add two spaces after function definitions (Python style)
Exceptions are
- ubuntu-browsers (because we already have ubuntu-browsers.d with
different usage)
- ubuntu-helpers (which includes the sanitized_helper subprofile, so
adding something in the global area wouldn't make much sense)
Also adjust abstractions/postfix-common to use the style all
abstractions use.
Commit b5be596460 added ability to read
/usr/share/drirc.d/ directory to mesa abstraction.
This seems to be a mistake, as it was noted that not all GUI
applications, that need access to drirc.d, also need whole mesa-related
rules (including writing caches).
Move /usr/share/drirc.d/ access to different abstraction. This is not a
breaking change, because any GUI application will have included X
abstraction already, and in the end result - dri-common abstraction too.
Fix typo introduced when resolving a merge conflict for d9ab83281b
("Add support for local additions to abstractions")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Local policy may want to extend or override abstractions, so add support for including local updates to them.
Acked-by: Christian Boltz <apparmor@cboltz.de>
Acked-by: intrigeri <intrigeri@boum.org>
Signed-off-by: John Johansen <john.johansen@canonical.com>
aa-logprof errors out if it hits a log event for a non-existing profile
while a profile file with the default filename for that non-existing
profile exists. This can for example happen after adding a profile name
to a profile if audit.log still contains events for the attachment-based
profile name.
Since we ignore log events for non-existing profiles in general, drop
the code for the special case "but a file matching the default filename
for that non-existing profile exists" and also silently ignore events
for this very special non-existing profile.
Also drop the now unused function get_profile_filename()
References: https://bugzilla.opensuse.org/show_bug.cgi?id=1120472
PR: https://gitlab.com/apparmor/apparmor/merge_requests/296
Signed-off-by: John Johansen <john.johansen@canonical.com>
This updates the mysqld to what I use on my servers nowadays.
Note: my profile also has capability sys_resource,, but I'm not sure why I had to add this and therefore didn't include it in this merge request.
Speaking about "why I had added $whatever" - these changes were collected over the last years and of course ;-) I don't remember any details.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/310
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
This change updates parser/Makefile to respect target dependencies and
not rebuild apparmor_parser if nothing's changed. The goal is to allow
cross-compiled tests #17 to run on a target system without the tests
attempting to rebuild the parser.
Two changes were made:
* Generate af_names.h in a script so the script timestamp is compared.
* Use FORCE instead of PHONY for libapparmor_re/libapparmor_re.a
Changes to list_af_names are intended to exactly replicate the old
behavior.
Signed-off-by: Eric Chiang <ericchiang@google.com>
The /usr/sbin/libvirtd profile will get a profile name ("libvirtd").
This patch adjusts the dnsmasq profile to support the named profile in
addition to the "old" path-based profile name.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=1118952#c3
*Add anonymous shared memory access rule for QtSettings.conf file
itself.
* Reduce strictness of anonymous shared memory file names - numbers in
them can be smaller or bigger.
* Fix consistency - require anonymous shared memory file names to end
with digit in all rules.
parser/apparmor.systemd: fix minor issues detected by shellcheck
See merge request apparmor/apparmor!293
Acked-by: Christian Boltz <apparmor@cboltz.de> for master and 2.13
abstractions/ssl_{certs,keys}: dehydrated uses /var/lib on Debian
See merge request apparmor/apparmor!299
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.10..master
aa-logprof errors out if it hits a log event for a non-existing profile
while a profile file with the default filename for that non-existing
profile exists. This can for example happen after adding a profile name
to a profile if audit.log still contains events for the attachment-based
profile name.
Since we ignore log events for non-existing profiles in general, drop
the code for the special case "but a file matching the default filename
for that non-existing profile exists" and also silently ignore events
for this very special non-existing profile.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=1120472
Commit 0d5ab43d59 removed support for loading
modules and introduced a caller, in apparmor_start(), that passes no argument to
is_apparmor_present(), which breaks that function when /bin/sh → /bin/dash.
Passing a module name as argument does not make sense since we dropped support
for the long obsolete "subdomain" module, so let's simplify
is_apparmor_present() and adjust its callers accordingly.
Bug-Debian: https://bugs.debian.org/917874
/var/lib/dhcp/dhcpd6.leases path for lease file on IPv6 has been in
dhcp repository since 2008, now visible in commit 8dea7ba7 ("Add DHCPv6
files in configure").
Signed-off-by: Petr Vorel <pvorel@suse.cz>
/var/lib/dhcp/dhclient6.leases path for lease file on IPv6 has been in
dhcp repository since 2008, now visible in commit 8dea7ba7 ("Add DHCPv6
files in configure").
Adding it via extending dhclient-*.leases pattern.
Not sure whether /var/lib/dhcp6/dhclient.leases was ever used on Linux,
but keep in for backward compatibility.
Signed-off-by: Petr Vorel <pvorel@suse.cz>
Since 04eb2fe3, __parse_profiles_dir can only return 0 or 1, so $STATUS can only
be 0 or 1, so trying to reset this variable to 0 when its value is 2 can only
cause confusion.
Split out RE_FLAGS
... instead of having it duplicated in RE_PROFILE_HAT_DEF and RE_PROFILE_START.
Note that the flags=... handling in RE_PROFILE_HAT_DEF was more/too
strict (for example it didn't allow whitespace around the "="), so this
change also qualifies as a little bugfix.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/287
Acked-by: John Johansen <john.johansen@canonical.com>
dovecot: allow reading /proc/sys/fs/suid_dumpable
This is needed if a dovecot child process segfaults - in this case, dovecot provides a helpful error message like
dovecot[6179]: auth-worker: Fatal: master: service(auth-worker): child 8103 killed with signal 11 (core not dumped - https://dovecot.or /bugreport.html#coredumps - set /proc/sys/fs/suid_dumpable to 2)
which involves reading the current value in suid_dumpable.
I propose this fix for 2.10..master.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/286
Acked-by: John Johansen <john.johansen@canonical.com>
Ignore *.orig and *.rej files when loading profiles
This was "accidently" reported by Ralph on the opensuse-support
mailinglist.
I propose this for 2.10..master (I verified that 2.10 tools and libapparmor have *.orig and *.rej in the ignore list)
PR: https://gitlab.com/apparmor/apparmor/merge_requests/282
Acked-by: John Johansen <john.johansen@canonical.com>
dnsmasq profile updates by Petr Vorel
This merge request includes two dnsmasq profiles Petr Vorel sent to the mailinglist:
dnsmasq: Add pid file used by NetworkManager
dnsmasq: Adjust pattern for log files to comply SELinux
I propose these patches for 2.11..master.
I'm not against also backporting to 2.10, but the profile in 2.10 doesn't allow to write anything in /var/log/, so either we apply/backport the changes manually, or we rely on the fact that we didn't get any bugreports for it ;-) Oh, and since I'm only forwarding these patches, I'll already add
Acked-by: Christian Boltz apparmor@cboltz.de for 2.11..master
Acked-by: John Johansen <john.johansen@canonical.com>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/288
i.e. move '*' from beginning to before suffix.
Commit 025c7dc6 ("dnsmasq: Add permission to open log files") added
pattern, which is not compatible with SELinux. As this pattern has been
in SELinux since 2011 (with recent change to accept '.log' suffix +
logrotate patterns which are not relevant to AppArmor) IMHO it's better
to adjust our profile.
Fixes: 025c7dc6 ("dnsmasq: Add permission to open log files")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
... instead of having it duplicated in RE_PROFILE_HAT_DEF and
RE_PROFILE_START.
Note that the flags=... handling in RE_PROFILE_HAT_DEF was more/too
strict (for example it didn't allow whitespace around the "="), so this
change also qualifies as a little bugfix.
This is needed if a dovecot child process segfaults - in this case,
dovecot provides a helpful error message like
dovecot[6179]: auth-worker: Fatal: master: service(auth-worker): child 8103 killed with signal 11 (core not dumped - https://dovecot.org/bugreport.html#coredumps - set /proc/sys/fs/suid_dumpable to 2)
which involves reading the current value in suid_dumpable.
In fed101920b, the coverity build process
was modified to split out the build logs into separate files, instead of
having one log file that gets overwritten repeatedly, making failures
hard to debug.
However, the coverity service gets upset if there is no file named with
the expected build log name. Therefore, instead, we'll capture the
python bits first, and then capture all the compilation bits in one
cov-build command.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/273
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
`/etc/letsencrypt/live/` contains symlinks to
`/etc/letsencrypt/archive/` which contains the keys. Add the
certs to ssl_certs and the private keys to ssl_keys.
Grant the ability to communicate with the postfix named child profiles
via signals and unix sockets. Include the path-based match names as
a fallback on upgrades.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Convert all the postfix subprocesses to using named profiles instead of
path match profiles, and adjust exec paths for newer debian/ubuntu
releses. Rename profiles to match profile names.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Convert postfix's master profile to use a named profile
(postfix-master) rather than the exec path match pattern. Adjust
postfix-common abstraction to take this into account. Rename profile
name in the profiles/apparmor/profiles/extras/ directory to match
the profile name.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
aa-genprof checks if one of the profiles in the extra profile dir
matches the binary, and proposes to use that profile as a starting
point.
Since 4d722f1839 the "(V)iew profile"
option to display the proposed profile was broken.
The easiest fix is to remember the filename in the extras directory, and
display the file from there.
Sidenote: when choosing to use the extra profile, it gets written to
disk without any problems, so this bug really only affected "(V)iew
profile" to preview the proposed extra profile.
parse_profile_data(): Ensure last line in a profile is valid
'lastline' gets merged into 'line' (and reset to None) when reading the
next line. If 'lastline' isn't empty after reading the whole profile,
this means there's something unparseable at the end of the profile,
therefore parse_profile_data() should error out.
Also remove some simple_tests testcases from the 'exception_not_raised'
list - they only didn't raise the exception because the invalid rule was
the last line in the affected profile.
Thanks to Eric Chiang for accidently (and maybe even unnoticedly ;-)
discovering this bug while adding some xattr testcases that surprisingly
didn't fail in the tools.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/271
Acked-by: John Johansen <john.johansen@canonical.com>
'lastline' gets merged into 'line' (and reset to None) when reading the
next line. If 'lastline' isn't empty after reading the whole profile,
this means there's something unparseable at the end of the profile,
therefore parse_profile_data() should error out.
Also remove some simple_tests testcases from the 'exception_not_raised'
list - they only didn't raise the exception because the invalid rule was
the last line in the affected profile.
Thanks to Eric Chiang for accidently (and maybe even unnoticedly ;-)
discovering this bug while adding some xattr testcases that surprisingly
didn't fail in the tools.
Expr tree simplification makes multiple passes at simplifying the
expression tree trying to use fatoring rules and heuristics to achieve
the minimum tree, so that dfa construction has fewer nodes to deal
with.
Unfortunately expr tree simplification can slow some policy compiles,
dependent on the type of expressions generated, down, and even worse
is currently subject to never terminating on some expressions as the
left and right passes keep undoing each others work.
Limiting the number of passes that expr tree simplification does can
provide most of its benefits (later passes generally have diminishing
returns), reduces the overhead it has on simple policy where it is of
little benefit, and insures that simplifications can not get stuck in
an infinite loop due to the left and right passes ping-ponging on each
others factoring.
Note: This also results in a performance improvement in evince
compiles, and general policy compiles because it achieves a better
balance between time spent on simplifying the tree to remove nodes and
time the dfa build requires to build with extra nodes and then
eliminate with minimization.
$ time apparmor_parser -QT /etc/apparmor.d/usr.bin.evince
real 0m2.744s
user 0m2.714s
sys 0m0.028s
vs.
$ time apparmor_parser -QT /etc/apparmor.d/usr.bin.evince
real 0m2.992s
user 0m2.979s
sys 0m0.012s
and
$ time apparmor_parser -QT /etc/apparmor.d/
real 0m3.568s
user 0m14.529s
sys 0m0.152s
vs.
$ time apparmor_parser -QT /etc/apparmor.d/
real 0m3.741s
user 0m15.400s
sys 0m0.179s
PR: https://gitlab.com/apparmor/apparmor/merge_requests/246
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Remove traces of aa-eventd
aa-eventd and its initscripts have been moved to deprecated/ in 2014 and didn't get any serious updates for several more years, so it's most probably useless and/or broken nowadays.
This also means we don't need to keep the AA_EV_BIN and AA_EV_PIDFILE variables in rc.apparmor.functions anymore.
(In theory I could move these variables to deprecated/rc.aaeventd.* - but in practise that sounds more than superfluous ;-)
PR: https://gitlab.com/apparmor/apparmor/merge_requests/263
Acked-by: John Johansen <john.johansen@canonical.com>
aa-eventd and its initscripts have been moved to deprecated/ in 2014 and
didn't get any serious updates for several more years, so it's most
probably useless and/or broken nowadays.
This also means we don't need to keep the AA_EV_BIN and AA_EV_PIDFILE
variables in rc.apparmor.functions anymore.
The apparmor kernel "module" has not been a loadable module for more
than a decade, it must be built into the kernel and due configuration
requirements it will never go back to being a loadable module.
Remove the long unfunctioning load_module support from the init script.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/257
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: seth.arnold@canonical.com
Use @{sys} tunable in profiles and abstractions
Commit aa065287 made @{sys} tunable available by default.
Update profiles and abstractions to actually use @{sys} tunable for better confinement in the future (when @{sys} becomes kernel var).
Closes LP#1728551
PR: https://gitlab.com/apparmor/apparmor/merge_requests/262
Acked-by: John Johansen <john.johansen@canonical.com>
Commit aa06528790 made @{sys} tunable
available by default.
Update profiles and abstractions to actually use @{sys} tunable for
better confinement in the future (when @{sys} becomes kernel var).
Closes LP#1728551
Elaborate in class comment of firstpos, lastpos, followpos, and nullable
fields beyond just referencing the Dragon book. Also add the section of
the book these are explained in.
If a test is marked as TODO, but matches its EXRESULT, this means the
TODO is superfluous and (probably) a change fixed what the TODO was for.
Instead of more or less ignoring such superfluous TODOs, error out to
make the change visible instantly.
Both result in "superfluous TODO" (for unknown reason), but fail after
removing the TODO.
Disable the tests until we find out why they have this strange
behaviour, to unblock merging the "error out on superfluous TODO" patch.
Imported from the Debian/Ubuntu packaging. We need this function so that
Debian/Ubuntu can switch to using this shell library instead of their own code.
Technical stuff first:
Replace existing_profiles (a dict with the filenames for both active and
inactive profiles) with active_profiles and extra_profiles which are
ProfileList()s and store the active profiles and those in the extra
directory separately. Thanks to ProfileList, now also the relation
between attachments and filenames is easily available.
Also replace all usage of existing_profiles with active_profiles and
extra_profiles, and adjust it to the ProfileList syntax everywhere.
With this change, several bugs in aa-complain and the other minitools
get fixed:
- aa-complain etc. never found profiles that have a profile name
(the attachment wasn't checked)
- even if the profile name was given as parameter to aa-complain, it
first did "which $parameter" so it never matched on named profiles
- profile names with alternations (without attachment specification)
also never matched because the old code didn't use AARE.
References: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=882047#92
(search for "As usual" ;-)
Just for completeness - the matching still doesn't honor/expand
variables in the profile name.
ProfileList is meant to store the list of profiles (both name and
attachment) and in which files they live.
Also add unittests to make sure everything works as expected.
parse_profile_data() returns the parsed profiles, but writes to
existing_profiles directly.
read_profiles() calls parse_profile_data() and already handles adding
the parsed profiles to aa, original_aa or extras, which means updating
existing_profiles there is a much better place.
This commit also includes a hidden change: Previously, when parsing
include files, they were also added to existing_profiles. This is
superfluous, only real profiles need to be stored there.
Split get_profile_filename() into
- get_profile_filename_from_profile_name() (parameter: a profile name)
- get_profile_filename_from_attachment() (parameter: an attachment)
Currently both functions call get_profile_filename_orig() (formerly
get_profile_filename()) so the behaviour doesn't change yet.
The most important part of this commit is changing all
get_profile_filename() calls to use one of the new functions to make
clear if they specify a profile or an attachment/executable as
parameter.
As promised, the is_attachment parameter starts to get used in this
patch ;-)
Note: The get_new parameter (which I'll explain in the patch actually
using it) is set to True in all calls to the new functions.
The long term plan is to get rid of it in most cases (hence defaulting
to False), but that will need more testing.
The minitools call write_profile(), write_profile_feedback_ui() and
serialize_profile() with the _attachment_ as parameter.
However, aa-logprof etc. call them with the _profile name_ as parameter.
This patch adds an is_attachment parameter to write_profile() and
write_profile_feedback_ui(). It also passes it through to
serialize_profile() via the options parameter.
If is_attachment is True, the parameter will be handled as attachment,
otherwise it is expected to be a profile name.
tools.py gets changed to set is_attachment to True when calling the
functions listed above to make clear that the parameter is an attachment.
Note: This patch only adds the is_attachment parameter/option, but
doesn't change any behaviour. That will happen in the next patch.
profiles/Makefile: test abstractions against apparmor_parser
See merge request apparmor/apparmor!237
Acked-by: Christian Boltz <apparmor@cboltz.de> for trunk and 2.13.
Pre-acked for 2.10..2.12 after removing the --config-file option which is not supported in these branches.
aa-notify man page: update user's configuration file path
See merge request apparmor/apparmor!239
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.10..master
Also adjust the signal rules in the dovecot-common and apache2-common
abstractions to match the profile names, and to really do that
(peer=...{bin,sbin}... didn't work, the correct syntax would have been
peer=...\{bin,sbin\}...)
This fixes the regression introduced by !149 / commit
4200932d8f
Add most abi/bad_*.sd tests to "exception not raised" list
Interestingly, abi/bad_6.sd is detected as invalid, and therefore not
added to the list.
I propose this for all branches that g[eo]t support for abi rules and the abi testcases.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/238
Acked-by: John Johansen <john.johansen@canonical.com>
AppArmor 3.0 requires policy to use a feature abi rule for access to
new features. However some policy may start using abi rules even if
they don't have rules that require new features. This is especially
true for out of tree policy being shipped in other packages.
Add enough support to older releases that the parser will ignore the
abi rule and warn that it is falling back to the apparmor 2.x
technique of using the system abi.
If the profile contains rules that the older parser does not
understand it will fail policy compilation at the unknown rule instead
of the abi rule.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/196
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
We can reduce the INCLUDE/INCLUDE_EXISTS code dup by using a
variable for whether the name was enclosed by '<' and using
processid() to handle the whether the id is quoted or not.
In addition using processid allows include names to contain
escaoe sequences like \n and have them handled correctly.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/196
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Currently if stdin is used the warning
apparmor_parser: cannot use or update cache, disable, or force-complain via stdin
is always displayed but if caching has been disabled there is no need for
this message.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Hasher causes some fun in aa-mergeprof: If the profile in
/etc/apparmor.d/ has a hat or subprofile that doesn't exist in the
to-be-merged profile, aa-mergeprof crashes. This is caused by reading
self.other.aa[program][hat]['include'] which accidently "creates" that
profile inside the aa hasher as empty hasher (instead of ProfileStorage).
Later, the code loops over self.other.aa[profile].keys(), expects
everything to be ProfileStorage, and explodes [1] when for example
trying to run .delete_duplicates on the hasher (which obviously doesn't
provide this method).
This patch adds checks to all self.other.aa accesses in
CleanProf.remove_duplicate_rules() to avoid accidently creating new keys
in the hasher.
Interestingly this bug survived unnoticed for years (at least since
2.11).
[1] last lines of the backtrace:
File ".../utils/apparmor/cleanprofile.py", line 42, in compare_profiles
deleted += self.remove_duplicate_rules(profile)
File ".../utils/apparmor/cleanprofile.py", line 65, in remove_duplicate_rules
deleted += apparmor.delete_duplicates(self.other.aa[program][hat], inc)
File ".../utils/apparmor/aa.py", line 1680, in delete_duplicates
deleted += profile[rule_type].delete_duplicates(include[incname][incname][rule_type])
AttributeError: 'collections.defaultdict' object has no attribute 'delete_duplicates'
.gitignore profiles/apparmor.d/local/* except README
The old patter . doesn't match lsb_release and nvidia_modprobe, and
the only file we ship in local is a README. This patch adjusts the
pattern to ignore everything except README.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/227
Acked-by: John Johansen <john.johansen@canonical.com>
The old patter *.* doesn't match lsb_release and nvidia_modprobe, and
the only file we ship in local is a README. This patch adjusts the
pattern to ignore everything except README.
commit 94dfe15b28 attempted to remove
LD_RUN_PATH unfortunately
But all it actually does is cause the Makefile.perl to embed the rpath
"" instead. Which is still an rpath, only I guess an even worse one.
--
Eli Schwartz
Arch Linux Bug Wrangler and Trusted User
This is because it cleared the setting of the variable LD_RUN_PATH
which was expanded in the command
$(INST_DYNAMIC) : $(OBJECT) $(MYEXTLIB) $(INST_ARCHAUTODIR)$(DFSEP).exists $(EXPORT_LIST) $(PERL_ARCHIVEDEP) $(PERL_ARCHIVE_AFTER) $(INST_DYNAMIC_DEP)
$(RM_F) $@
LD_RUN_PATH="$(LD_RUN_PATH)" $(LD) $(LDDLFLAGS) $(LDFROM) $(OTHERLDFLAGS) -o $@ $(MYEXTLIB) \
$(PERL_ARCHIVE) $(LDLOADLIBS) $(PERL_ARCHIVE_AFTER) $(EXPORT_LIST) \
$(INST_DYNAMIC_FIX)
$(CHMOD) $(PERM_RWX) $@
resulting in LD_RUN_PATH="" being passed to the command.
Finish removing LD_RUN_PATH from Makefile.perl by removing it from
the command invocation if it is present.
Note: we use \x24 instead of $ in the regex as there seems to be a bug
and no level of escaping $ would allow it to be used.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Move @{sys} into tunables/kernelvars to make it readily available for
all profiles by default. In order to avoid duplicate variable
declaration, other tunable includes where modified accordingly.
Additionally, apparmor_api/is_enable has updated comment with regards to
@{sys} availability. tunables/sys is now marked as deprecated and should
not be used as it is noop, left only for backwards compatibility.
This is preparation for fixing bug:
https://bugs.launchpad.net/apparmor/+bug/1728551
Thanks to Christian Boltz for original idea to implement this in
non-breaking manner.
--log-facility option needs to have permission to open files.
Use '*' to allow using more files (for using more dnsmasq instances).
Signed-off-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Jamie Strandboge <jamie@canonical.com>
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
We ship newer versions of these profiles in apparmor.d, so there's no
reason to keep the outdated ones (no serious change since > 10 years) in
the extra directory.
parser: fix Makefile hardcoded paths to flex and bison
Closes#4
See merge request apparmor/apparmor!224
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.10..master
The hardcoded paths for flex and python can break builds on systems
where those tools are stored in an alternate location. Use which
to lookup where flex and bison are available.
This fixes issue #4
Signed-off-by: John Johansen <john.johansen@canonical.com>
Use nvidia_modprobe named profile inside opencl-nvidia abstraction
Commit 8f9bd5b0 rightfully removed PUx
transition into nvidia-modprobe executable due to security concerns. To
overcome this, commit 327420b1 added
named nvidia_modprobe profile, which allows to use this abstraction
without requiring additional rules to make OpenCL work with NVIDIA
drivers.
Add rule to allow Px transition into nvidia_modprobe profile for
nvidia-modprobe executable.
I propose this for 2.13..master (opencl* are only available in 2.13...). Please check if nvidia_modprobe is already backported.
https://gitlab.com/apparmor/apparmor/merge_requests/219
Acked-by: John Johansen <john.johansen@canonical.com>
aa-notify: Read user's configuration file from XDG_CONFIG_HOME
Legacy path ~/.apparmor/notify.conf is preferred if it exists, otherwise $XDG_CONFIG_HOME/apparmor/notify.conf, with fallback to ~/.config/apparmor/notify.conf, is used.
See merge request https://gitlab.com/apparmor/apparmor/merge_requests/215
Signed-off-by: nl6720 nl6720@gmail.com
Acked-by: John Johansen <john.johansen@canonical.com>
Add missing paths to usr.sbin.nmbd, usr.sbin.smbd and abstractions/samba
See merge request apparmor/apparmor!210
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.10..master
Commit 8f9bd5b0e3 rightfully removed PUx
transition into nvidia-modprobe executable due to security concerns. To
overcome this, commit 327420b151 added
named nvidia_modprobe profile, which allows to use this abstraction
without requiring additional rules to make OpenCL work with NVIDIA
drivers.
Add rule to allow Px transition into nvidia_modprobe profile for
nvidia-modprobe executable.
Legacy path ~/.apparmor/notify.conf is preferred if it exists, otherwise
$XDG_CONFIG_HOME/apparmor/notify.conf, with fallback to
~/.config/apparmor/notify.conf, is used.
Signed-off-by: nl6720 <nl6720@gmail.com>
Add basic support for abi rules to the tools
Add basic support for abi rules to the tools
Add basic "understand and keep" support for abi rules, where "understand" means to not error out when seeing an abi rule, and "keep" simply means to keep the original abi rule when serializing a profile.
On the long term, abi rules should be parsed (similar to include rules), but for now, this patch is the smallest possible changeset and easy to backport.
Note that the only added test is via cleanprof_test.* which is used by minitools_test.py - and does not run if you do a 'make check'. Oh, and of course the simple_tests/abi/ files also get parsed by test-parser-simple-tests.py.
Also note that serialize_profile_from_old_profile() (which no longer exists in master, "only" in <= 2.13) would in theory also need support for abi rules. In practise, making this another case of "serialize_profile_from_old_profile() has known issues" is probably fine, but we should at least test that "(V)iew changes" doesn't break if an abi rule is present.
I propose this patch for 2.10..master.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/202
Acked-by: John Johansen <john.johansen@canonical.com>
nvidia-modprobe is setuid executable is used to create various device
files and load the the NVIDIA kernel module
(https://github.com/NVIDIA/nvidia-modprobe).
Add named profile to be used in application profiles for confining
potentially risky setuid application.
Add a check to logfile_to_profile() that checks the parsed log against a
list of input logs (log_to_profile_known_empty_log) that produce an
empty output.
The function was messing up its use of fds, it could get away with
it because the cb_dirfd passed to fdopendir was still valid until
closedir was called but if code was moved around, or fdopendir
code changed behavior it could easily break.
Also the check for dup failing was wrong fix it.
Reference: coverity #187003
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Time-out
Include qt5 into kde abstraction
See merge request apparmor/apparmor!209
Acked-by: Christian Boltz <apparmor@cboltz.de> for master and backporting to 2.10..2.13 (as soon as the qt5 abstraction is backported)
Currently, kde abstraction has rules relevant to Qt 3 and Qt 4
libraries, but are missing rules against latest Qt 5.
Include read-only Qt 5 abstraction to fix styling and similar issues for
software running on KDE 5 desktop.
Fixes https://bugs.launchpad.net/apparmor/+bug/1787201
Add uid and uids kernel var placeholders
Add @{uid} and @{uids} variables to allow migrating profiles in advance while awaiting path mediation implementation, based on current user id, in kernel side.
See merge request apparmor/apparmor!208
Acked-by: John Johansen <john.johansen@canonical.com>
Add @{uid} and @{uids} variables to allow migrating profiles in advance
while awaiting path mediation implementation, based on current user id,
in kernel side.
gio-launch-desktop helper tries to execute /usr/bin/thunderbird wrapper
script, not the /usr/lib/thunderbird... directly.
Add rule allowing to execute /usr/bin/thunderbird.
Harden abstractions
Harden abstractions
remove antiquated abstractions/launchpad-integration
abstractions/opencl-nvidia: don't allow PUx on nvidia-modprobe
abstractions/private-files-strict: disallow access to the dirs of private files
abstractions/private-files: disallow writes to thumbnailer dir (LP: #1788929)
ubuntu-browsers.d/user-files: disallow access to the dirs of private files
Nominating launchpad-integration and opencl-nvidia for 2.13. Nominating private-files-strict, private-files and user-files for 2.10 and higher
See merge request apparmor/apparmor!203
Acked-by: John Johansen <john.johansen@canonical.com>
Add basic "understand and keep" support for abi rules, where
"understand" means to not error out when seeing an abi rule, and "keep"
simply means to keep the original abi rule when serializing a profile.
On the long term, abi rules should be parsed (similar to include rules),
but for now, this patch is the smallest possible changeset and easy to
backport.
Note that the only added test is via cleanprof_test.* which is used by
minitools_test.py - and does _not_ run if you do a 'make check'.
Oh, and of course the simple_tests/abi/ files also get parsed by
test-parser-simple-tests.py.
Also note that serialize_profile_from_old_profile() (which no longer
exists in master, "only" in <= 2.13) would in theory also need support
for abi rules. In practise, making this another case of
"serialize_profile_from_old_profile() has known issues" is probably
fine, but we should at least test that "(V)iew changes" doesn't break if
an abi rule is present.
This reverts commit ab7459ed40.
This commit was meant to go to a branch to setup a merge request in
gitlab. Revert until the commit has gone through the regular review
process.
Signed-off-by: John Johansen <john.johansen@canonical.com>
make simple.pl / parser make check output more useful
Instead of printing the (useless) numbers of no-longer-failing tests
marked as TODO, let prove print out the actual test names.
A side effect is that really unfixed TODOs and disabled tests get
printed (lists about 10 tests), but that's much better than having to
hunt down the no-longer-failing TODOs.
This change will print out lines like (the first one is still a TODO,
the second one got fixed at some time)
not ok 71447 - TODO: ./simple_tests//vars/vars_profile_name_13.sd: reference variables that are the profile name and attachment # TODO Unfixed testcase.
ok 71448 - TODO: ./simple_tests//vars/vars_profile_name_14.sd: reference variables in rules that also have alternations # TODO Unfixed testcase.
before printing the test summary.
I propose this patch for master and (optionally) the maintained branches.
See merge request apparmor/apparmor!194
Acked-by: John Johansen <john.johansen@canonical.com>
Instead of printing the (useless) numbers of no-longer-failing tests
marked as TODO, let `prove` print out the actual test names.
A side effect is that really unfixed TODOs and disabled tests get
printed (lists about 10 tests), but that's much better than having to
hunt down the no-longer-failing TODOs.
This change will print out lines like (the first one is still a TODO,
the second one got fixed at some time)
```
not ok 71447 - TODO: ./simple_tests//vars/vars_profile_name_13.sd: reference variables that are the profile name and attachment # TODO Unfixed testcase.
ok 71448 - TODO: ./simple_tests//vars/vars_profile_name_14.sd: reference variables in rules that also have alternations # TODO Unfixed testcase.
```
before printing the test summary.
usr.sbin.dnsmasq: add paths for NetworkManager connection sharing
dnsmasq needs to access additional paths when used for connection sharing by NetworkManager.
Additionally it needs read permissions to /usr/share/dnsmasq/trust-anchors.conf which contains the DNSSEC trust anchors.
See merge request apparmor/apparmor!193
Acked-by: John Johansen <john.johansen@canonical.com>
Add qt5 writing abstractions
Qt-based applications stores QFileDialog (latest browsed directory) and
other shared user settings inside ~/.config/QtProject.conf. Currently
available qt abstraction only allows to read it (by design), so this
patch introduces abstraction that grants permissions for writing.
Relevant denies discovered with KDE Dragon Player:
/var/log/audit/audit.log.1:type=AVC msg=audit(1533485161.999:981): apparmor="DENIED" operation="mknod" profile="/usr/bin/dragon" name="/home/vincas/.config/QtProject.conf.lock" pid=29911 comm="dragon" requested_mask="c" denied_mask="c" fsuid=1000 ouid=1000
type=AVC msg=audit(1533486419.266:1141): apparmor="DENIED" operation="file_lock" profile="/usr/bin/dragon" name="/home/vincas/.config/QtProject.conf.lock" pid=30406 comm="dragon" requested_mask="k" denied_mask="k" fsuid=1000 ouid=1000
/var/log/audit/audit.log.1:type=AVC msg=audit(1533485206.575:1006): apparmor="DENIED" operation="link" profile="/usr/bin/dragon" name="/home/vincas/.config/QtProject.conf.Gflpds" pid=29946 comm="dragon" requested_mask="l" denied_mask="l" fsuid=1000 ouid=1000 target="/home/vincas/.config/#12982163"
In addition, added new qt-compose-cache-write abstraction as some applications wants to write compose cache. qt5 abstraction is appended with read-only rule (that's enough for LibreOffice using KDE file dialog).
See merge request apparmor/apparmor!159
Acked-by: John Johansen <john.johansen@canonical.com>
use empty parser/tst/parser.conf in all parser and profile tests
See merge request apparmor/apparmor!191
Without this, the system-wide parser.conf gets used, which causes test
failures if for example caching is enabled and the cache dir isn't
writeable for the user running the tests.
I propose this for all branches where the parser understands --config-file.
Acked-by: John Johansen <john.johansen@canonical.com>
Without this, the system-wide parser.conf gets used, which causes test
failures if for example caching is enabled and the cache dir isn't
writeable for the user running the tests.
add python3.7 to logprof.conf
See merge request apparmor/apparmor!190
add python3.7 to logprof.conf
I propose this patch for at least 2.12..master, maybe also 2.10 and 2.11 (even if it's unlikely that someone uses the latest python3 on a distro with old AppArmor)
Acked-by: John Johansen <john.johansen@canonical.com>
The URL redirect ends up at a page in the new wiki that doesn't exist.
We have to link directly to the gitlab URL here since the current URL
redirect doesn't let us use a wiki.apparmor.net URL and still reach the
expected Profiles page.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
The open-coded readdirfd fn used to replace scandirat skipped
checks for memory allocation failures and cleaning on faulures,
fix this.
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
The `scandirat` function is a nonstandard GNU extension, which opens a
directory relative to a file descriptor. musl libc does not implement
that function and thus cannot be used to compile libapparmor.
All our uses of `scandirat` directly scan the directory the file
descriptor is referring to, not any directory beneath the FD. Implement
a function `readdirfd()`, which gets as arguments the directory FD, the
location where to put the list of directory entries as well as a
function pointer to a comparing function. `readdirfd` will then scan all
directory entries except "." and ".." and return them via an allocated
array. The array is sorted in case the comparing function is set.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/107
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Using stdin with --write-cache set results in
# apparmor_parser --show-cache --write-cache
Cache: added primary location '/var/cache/apparmor'
Warnung aus stdin (Zeile 1): Cache: added readonly location '/usr/share/apparmor/cache'
Warnung aus stdin (Zeile 1): apparmor_parser: cannot use or update cache, disable, or force-complain via stdin
Cache miss: stdin
Wrote cache: /var/cache/apparmor/9b2cd0d0.0/(null)
The "Wrote cache:" message is referencing a null value and should not
be displayed.
BugLink: http://bugs.launchpad.net/bugs/1787717
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Timeout
abstractions/php: allow ICU (unicode support) data tables
See merge request apparmor/apparmor!184
abstractions/php: allow ICU (unicode support) data tables
Reported by darix on IRC, and also something I noticed in my own usage of PHP.
I propose this addition for 2.10..master.
Acked-by: John Johansen <john.johansen@canonical.com>
profiles: support void-specific binary names for openntp and traceroute
See merge request apparmor/apparmor!183
Acked-by: Christian Boltz <apparmor@cboltz.de>
Fix concatenated-multiple-lines due to lacking Markdown's linebreak markup
by wrapping it with <pre>.
Signed-off-by: 林博仁(Buo-ren, Lin) <Buo.Ren.Lin@gmail.com>
Requiring --config-file to be first in the option list is not user
friendly fix the option parsing so that --config-file can be specified
anywhere in the option list.
This also fixes a bug where even when the --config-file option is
first the option parsing fails because the detection logic is broken
for some option cases.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/175
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
The compiler is spitting out the warning
parser_main.c:1291:16: warning: ISO C++ forbids converting a string constant to 'char*' [-Wwrite-strings]
char *tmp = "/var/cache/apparmor";
fix this by constifying the cacheloc array.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Qt GUI applications that uses "platforminputcontexts"-class of plugins
might need reading and/or writing compose cache. Add read-only rule in
qt5 abstraction and create new writing dedicated for compose cache
writing.
Qt-based applications stores QFileDialog (latest browsed directory) and
other shared user settings inside ~/.config/QtProject.conf. Currently
available qt abstraction only allows to read it (by design), so this
patch introduces abstraction that grants permissions for writing.
The parser config file can affect the parsers behavior during tests.
Allow overriding the default location with the option
--config-file=
the option must be the first option in the commands argument list.
Also provile a
--print-config-file
option to display what the parser is using for a config file.
BugLink: http://bugs.launchpad.net/bugs/1277711
Acked-by: Christian Boltz <apparmor@cboltz.de>
Signed-off-by: John Johansen <john.johansen@canonical.com>
KIconLoader uses ~/.cache/icon-cache.kcache, and it is opened in
read-write mode. Because access to it does not seem to be critical, and
read-only mode is not used, rules for accessing this cache is added to
it's own new "write" abstraction, instead of making kde abstraction more
permissive by default.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently, kde abstraction only allows reading
~/.config/klanguageoverridesrc file (by design). Some KDE applications
has option to change language for it's interface, and this needs write
access. This is fixed by introducing new abstraction.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently, kde abstraction only allows reading ~/.config/kdeglobals (by
design), though some applications might need to update it's contents
such as KFileDialog settings. This patch fixes it by introducing new
abstraction.
Signed-off-by: John Johansen <john.johansen@canonical.com>
prevent that aa-complain etc. overwrites flags in child profiles if they differ from the main profile
See merge request apparmor/apparmor!150
Acked-by: John Johansen <john.johansen@canonical.com>
When running aa-genprof in a lxd instance, printk_ratelimit is readonly
and writing to it fails. Instead of crashing with a backtrace, only
print a warning.
References: https://bugs.launchpad.net/apparmor/+bug/1785391
Let's not store a bunch of automatically generated binary files in /etc.
AppArmor 3.0 will store the cache in /var/cache and most distros
(openSUSE, Debian, and soon Ubuntu) moved it there already.
Bug-Debian: https://bugs.debian.org/904637
This will allow removal of the lsb_release sub-profile from the
chromium, firefox and murmurd profiles, and consolidation of the rules
for /usr/bin/lsb_release in a single file.
The integration changes are taken from the patch at
apparmor-2.12/debian/patches/debian/add-debian-integration-to-lighttpd.patch
and are necessary so that lighttpd doesn't serve everything as
application/octet-stream.
As Simon McVittie wrote, "if a specification or library creates extra caches, or
has .desktop files in a subdirectory, or anything like that, then I don't see
why we wouldn't want to allow reading those too".
As Simon McVittie <smcv@collabora.com> wrote on
https://bugs.debian.org/865206 and on the AppArmor mailing list:
"Anything in /var/lib/flatpak/exports/share or
~/.local/share/flatpak/exports/share is essentially equivalent to
the corresponding path in /usr/{local/,}share, and is something
that has deliberately been "exported" to the rest of the system by a
Flatpak-confined app.
The only reason to prevent reading those directories would be if you do
not want the AppArmor-confined app to be able to enumerate the other
software you have installed on your system, as an anti-fingerprinting
mechanism.".
Bug-Debian: https://bugs.debian.org/865206
... instead of overwriting them with the flags of the main profile.
This fixes a longstanding issue with aa-complain, aa-enforce and
aa-audit which broke the flags of child profiles and hats if they
differed from the main profile.
It also fixes several issues documented in the tests (which obviously
need adjustment to match the fixed behaviour).
Also change the "no profile found" cases to AppArmorException - errors
in a profile are not worth triggering AppArmorBug ;-)
All callers call change_profile_flags(), so it makes sense to test this
function instead of set_profile_flags().
Besides that, set_profile_flags() will be merged into
change_profile_flags() in the next commit ;-)
Note that this commit adds some '# XXX' notes to the tests. These will
be addressed in later commits.
If the old flags are given as str (or None), call split_flags() to
convert them to a list.
This allows to simplify change_profile_flags() which now doesn't need to
call split_flags() on its own.
Also add some tests with a str for the old flags
... instead of set_profile_flags() to keep possibly existing flags like
attach_disconnected.
Note that this function is unused (meant to be used with the
no-longer-existing profile repo), therefore nobody noticed that
set_profile_flags() was called with the wrong number of parameters ;-)
Each coverity command writes its debugging output to
cov-int/build-log.txt, which means that multiple runs of cov-build
overwrite previous logs, resulting in only the last invocation's output
remaining at the end of the build, making debugging why failures to
capture coverity output difficult. Fix this by renaming the build-log to
per-directory log files.
(This would still be an issue even if we had a single build command
for the entire tree, as capturing python and other interpreted
files requires a second invocation of cov-build to scan for those
file types.)
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/145
'case OPTION_OFILE' missed the 'break', which means if did fallthrough
to the default case.
Adding the 'break' means no longer executing another PERROR, and no
longer executing the 'exit(1)' in the default branch.
References: coverity #55994
Add kernel patches that will NEVER be sent upstream. These provide abi
compatibility with the v2.x network and af_unix rules.
The 4.17 network mediation pull request deliberately broke abi
compatibility with the v2.x rules, and these are provided so that
distros who shipped the v2.x compatible patches can provide new
kernels on older releases that require v2.x network support.
Signed-off-by: John Johansen <john.johansen@canonical.com>
When creating a new profile with aa-genprof, get_profile() searches for
an inactive ("extra") profile and, if it finds one, removes the filename
from that profile so that it gets stored in /etc/apparmor.d/ later.
However, it used .pop() to remove the filename, which explodes since
ProfileStorage is a class now.
This patch fixes this (tested manually).
Note that NO_FLAGS was an inverse option, therefore
- NO_FLAGS was changed to FLAGS (also in sync_profile() which is the
only caller that sets FLAGS)
- the default for include_flags (if FLAGS is not set) is True
Add mesa abstraction to allow writing to the Mesa-specific cache
locations and listing devices. Abstraction is needed for applications
utilizing OpenGL API with Mesa implementation available on the system.
The tools can't handle nested child profiles yet. Instead of failing
in funny[tm] ways (parse_profile_start() only returned the first two
segments of the profile name) better error out with a clear message.
The initial apparmor.service assumed the profile cache is in
/var/lib/apparmor/ which was an openSUSE-specific path that was chosen
for historical reasons, but was far from perfect.
In the meantime, the profile cache moved to /var/cache/apparmor/ and the
read-only/packaged version was added in /usr/share/apparmor/cache/
This commit adjusts apparmor.service to use these paths.
These tests verify that
- _is_equal_aare() really raises an exception when it sees an invalid
combination of other_value and other_all
- BaseRuleset.__repr__() works as expected
As a side effect, this commit pushes the test coverage of
apparmor/rule/__init__.py to 100% ;-)
... which is unused since the last commit.
Also delete several functions that were only used by this function:
- write_change_profile()
- write_rlimits()
- write_capabilities()
- write_netdomain()
- write_dbus()
- write_signal()
- write_ptrace()
- write_file()
Finally, no longer import some functions from profile_storage:
- write_links
- write_mount
- write_pivot_root
- write_unix
BTW: Deleting these 460 lines improves test coverage of aa.py from
38% to 44%, and total test coverage from 63% to 66% :-)
- rename prof_data parameter to ref
- drop empty allow, prefix and tail, and their usage in the format
string
- inline sep into the format string
- replace usage of fn with the actual function name (var_transform)
This is needed to get a reproducible output.
Also adjust the tests in test-profile-storage.py and add some example
variable to cleanprof.in and cleanprof.out
The aa-exec tool was indicating, in an error message, that a profile
didn't exist in cases where aa_change_profile() failed with errno set to
ENOENT or EACCES. However, the EACCES error means that changing to the
target profile is not allowed. This patch fixes the error message in the
cause of EACCES:
$ ./aa-exec -p dne -- true
aa-exec: ERROR: profile 'dne' does not exist
$ ./aa-exec -p /usr/sbin/tcpdump -- ./aa-exec -p unconfined -- true
aa-exec: ERROR: insufficient permissions to change to the profile 'unconfined'
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Add the pid of the process to the error, debug, and verbose output. This
is useful for debugging.
For example,
$ aa-exec -v -- whoami
[30389] exec whoami
tyhicks
$ aa-exec -p dne -- true
[30390] aa-exec: ERROR: profile 'dne' does not exist
$ aa-exec -d -p unconfined -- true
[30409] aa-exec: DEBUG: 0 = aa_change_onexec("unconfined")
It can also help when multiple aa-exec invocations are used across a
fork and exec. Here's a contrived example:
$ aa-exec -v -- \
sh -c 'aa-exec -vp /usr/sbin/tcpdump -- aa-exec -p unconfined -- true'
[3424] exec sh -c aa-exec -vp /usr/sbin/tcpdump -- aa-exec -p unconfined -- true
[3425] aa_change_onexec("/usr/sbin/tcpdump")
[3425] exec aa-exec -p unconfined -- true
[3425] aa-exec: ERROR: profile 'unconfined' does not exist
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
This commit adds an initial gitlab-ci.yml file to perform test builds
and run tests on each commit.
v2: add liblocale-gettext-perl dependency for parser simple test
v3:
- set noninteractive prompt to avoid debconf queries when installing
packages
- disable profiles test against aa-logprof; even if library and python
path issues are resolved, aa-logprof early aborts due to being
unable to find /sbin/apparmor_parser
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: Tyler Hicks <tyler.hicks@canonical.com>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/101
If a log line contains a denial for a child profile, log_dict will
(obviously) only contain the child profile. However, serialize_profile()
expects that the parent profile is also initialized as ProfileStorage.
This patch makes sure the parent profile gets initialized.
It also removes 26 of the 37 reasons in the TODO note in aa.py :-)
ProfileStorage.get_rules_clean() returns all rules in a profile
(withouth the profile header or the closing '}')
Also change aa.py write_rules() to use get_rules_clean()
ProfileStorage() stores the content of a profile, so it makes sense to
also have the functions to write those rules (including helper functions
used by these functions) in the same file.
Note that I only moved the functions for rule types that are not handled
by *Ruleset classes.
The functions for writing rules stored in a *Ruleset class will
hopefully be superfluous sooner or later (probably later because
serialize_parse_profile_start() depends on them, and rewriting it won't
be easy)
Also move the test for var_transform() to test-profile-storage.py.
The POSIX standard never defines the typedefs `comparison_fn_t` and
`__free_fn_t`, but they are provided by glibc and user in the parsing
code. Provide the typedefs ourselves to fix compiling on musl based
systems.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
The `secure_getenv` function is a non-POSIX compliant extension of
glibc. In contrast to the POSIX `getenv`, `secure_getenv` will return
`NULL` for all environment variables when the program is run with
escalated privileges due to an SUID or SGID bit. Some strictly
POSIX-compliant libc libraries, most notably musl libc, do not have this
function and do not wish to implement it. Thus, AppArmor cannot be
compiled on such systems.
In libapparmor, `secure_getenv` is only used to determine whether the
environment variable DEBUG_ENV_VAR has been set to enable debugging. In
case an unprivileged user runs a SUID/SGID executable linked against
libapparmor, we do not want that user to be able to get additional
information via debug output.
The fix here is to produce an error only in case where debug output is
enabled by defining ENABLE_DEBUG_OUTPUT. Otherwise, we simply define
`secure_getenv` to `NULL` to completely disable the debug output.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
While the parser makes use of the `aa_policy_cache_add_ro_dir` function,
it is not being declared as a global function in the libapparmor.map
file. Due to this, dynamic linking of apparmor_parser with
libapparmor.so is not possible.
[Fixed up to use 2.13.1 symbol section as when the
`aa_policy_cache_add_ro_dir` was introduced -- @smb]
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/107
This is especially handy if your distro doesn't split sbin and bin
and only wants to install into bin (so that the sbin directory doesn't
clash with the sbin -> bin symlink)
[Per feedback, added USR_SBINDIR as a toggle for the install location
of aa-teardown -- @smb]
Signed-off-by: Rasmus Thomsen <cogitri@exherbo.org>
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/111/
Instead of calling write_pair() (which is quite complex because it needs
to handle multiple rule types), write the alias rules directly in
write_alias().
This comes with minor code duplication, but makes the code much more
readable (3 instead of 7 %s)
write_pair() ignored the 'tail' parameter, which resulted in writing
invalid alias rules (without the trailing comma).
Also add an alias to test/cleanprof.* to ensure it doesn't break again.
test-libapparmor-test_multi.py converts the libapparmor test_multi log
examples to profiles.
This patch allows to call test-libapparmor-test-multi.py with a logfile
(containing a single log line) as parameter. It will then print the
resulting profile.
Example:
# python3 test-libapparmor-test_multi.py /path/to/libraries/libapparmor/testsuite/test_multi/testcase_dbus_01.in
/tmp/apparmor-2.8.0/tests/regression/apparmor/dbus_service {
dbus send bus=session path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=Hello peer=(label=unconfined),
}
Writing a "link subset" rule missed a space, which resulted in something
like
link subset/foo -> /bar,
Also add a test rule to tests/cleanprof.* to ensure this doesn't break
again.
write_piece() has some funny code that converts the result of
write_header() and write_rules() (which is a list) to... a list.
Needless to say that this is superfluous ;-)
match_includes() and is_known_rule() have safety nets to avoid troube if
include[incname][incname] is not a valid ProfileStorage object.
However, this situation shouldn't happen in practise anymore, so let's
drop these now superfluous safety nets.
I use this patch locally since months without problems.
Commit 63b7cb0660 (libapparmor: convert
multicache from using djb2 hashing to murmur3 hash) mistakenly added
PmurHash.h to the list of files generated by the build process and thus
should be removed when the 'maintainerclean' make target is invoked.
This fixes the issue by removing PmurHash.h from the list of generated
files.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/112
When running the swap regression tests, the swapon command would issue
a warning about insecure permissions on the swapfile being used as
part of the test:
mkswap: /tmp/sdtest.9698-822-2BL034/swapfile: insecure permissions 0644, 0600 suggested.
Fix this by setting the permissions after the swapfile is created.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/108
This excludes the /etc/apparmor.d/cache.d/ directory from aa-logprof
parsing because parsing the binary cache, well, takes a while :-/
Reported on the opensuse-factory mailinglist by Frank Krüger and
confirmed by others.
Split the features file into compile features and kernel features
which is needed for policy versioning and the new caching scheme.
A new flag --kernel-features was added to set the kernel features but
unfortunately -M, --features-file was setup to only specify the
compile features, when it used to effectively specify both the
compile and kernel features.
This broke existing uses of -M.
Fix this by having -M specify both the compile and kernel features,
and a new flag --compile-features that can be used to specify the
compile fature set separate from the kernel feature set.
sbeattie> fixed up error message to refer to compile features when
--compile-features argument fails.
Fixes: 9e48a5da5e ("parser: split kernel features from compile features.")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/104
The parser currently uses a fork model to do job processing. For
consistency even when the number of jobs is set to 1 a single
work process is forked. However this makes using gdb more difficult
and can be even worse for other debugging tools.
Make -j 0 disable all job spawning so all processing happens in the
main process.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/105
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The recently added overlay cache directory support added to libapparmor
makes use of reallocarray(3) to resize memory allocations; however,
reallocarray() was only included in glibc 2.26. This commit adds a
configure check for reallocarray() and if it's not available, provides
it as a wrapper around realloc(3).
PR: https://gitlab.com/apparmor/apparmor/merge_requests/100
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Security issues can be filed in GitLab by opening up a new [issue](https://gitlab.com/apparmor/apparmor/-/issues) and selecting the tick box ```This issue is confidential and should only be visible to team members with at least Reporter access.``` or directed to `security@apparmor.net`. Additional details can be found
in the [wiki](https://gitlab.com/apparmor/apparmor/wikis/home#reporting-security-vulnerabilities).
--------------
Privacy Policy
--------------
The AppArmor security project respects users privacy and data and does not collect data from or on its users beyond what is required for a given component to function.
The AppArmor kernel security module will log violations to the audit subsystem, and those will be logged/forwarded/recorded on the user's system(s) according to how the administrator has logging configured. Again this is not forwarded to or collected by the AppArmor project.
The AppArmor userspace tools do not collect information on the system user beyond the logs and information needed to interact with the user. This is not forwarded to, nor collected by the AppArmor project.
Users may submit information as part of an email, bug report or merge request, etc. and that will be recorded as part of the mailing list, bug/issue tracker, or code repository but only as part of a user initiated action.
The AppArmor project does not collect information from contributors beyond their interactions with the AppArmor project, code, and community. However contributors are subject to the terms and conditions and privacy policy of the individual platforms (currently GitLab) should they choose to contribute through those platforms. And those platforms may collect data on the user that the AppArmor project does not.
Currently GitLab requires a user account to submit patches or report bugs and issues. If a contributor does not wish to create an account for these platforms the mailing list is available. Membership in the list is not required. Content from non-list members will be sent to moderation, to ensure that it is on topic, so there may be a delay in choosing to interact in this way.
-------------
Source Layout
@@ -33,6 +68,7 @@ Source Layout
AppArmor consists of several different parts:
```
binutils/ source for basic utilities written in compiled languages
changehat/ source for using changehat with Apache, PAM and Tomcat
common/ common makefile rules
@@ -43,6 +79,7 @@ parser/ source for parser/loader and corresponding documentation
profiles/ configuration files, reference profiles and abstractions
tests/ regression and stress testsuites
utils/ high-level utilities for working with AppArmor
```
--------------------------------------
Important note on AppArmor kernel code
@@ -67,63 +104,82 @@ the following order. Some systems may need to export various python-related
environment variables to complete the build. For example, before building
anything on these systems, use something along the lines of:
/* When compiling for windows, we specify a specific calling convention to avoid issues where we are being called from a project with a different default calling convention. For windows you have 3 define options:
CJSON_HIDE_SYMBOLS - Define this in the case where you don't want to ever dllexport symbols
CJSON_EXPORT_SYMBOLS - Define this on library build when you want to dllexport symbols (default)
CJSON_IMPORT_SYMBOLS - Define this if you want to dllimport symbol
For *nix builds that support visibility attribute, you can define similar behavior by
setting default visibility to hidden by adding
-fvisibility=hidden (for gcc)
or
-xldscope=hidden (for sun cc)
to CFLAGS
then using the CJSON_API_VISIBILITY flag to "export" the same symbols the way CJSON_EXPORT_SYMBOLS does
*/
#define CJSON_CDECL __cdecl
#define CJSON_STDCALL __stdcall
/* export symbols by default, this is necessary for copy pasting the C and header file */
#define CJSON_PUBLIC(type) __declspec(dllexport) type CJSON_STDCALL
#elif defined(CJSON_IMPORT_SYMBOLS)
#define CJSON_PUBLIC(type) __declspec(dllimport) type CJSON_STDCALL
#endif
#else /* !__WINDOWS__ */
#define CJSON_CDECL
#define CJSON_STDCALL
#if (defined(__GNUC__) || defined(__SUNPRO_CC) || defined (__SUNPRO_C)) && defined(CJSON_API_VISIBILITY)
#define CJSON_PUBLIC(type) __attribute__((visibility("default"))) type
#else
#define CJSON_PUBLIC(type) type
#endif
#endif
/* project version */
#define CJSON_VERSION_MAJOR 1
#define CJSON_VERSION_MINOR 7
#define CJSON_VERSION_PATCH 13
#include<stddef.h>
/* cJSON Types: */
#define cJSON_Invalid (0)
#define cJSON_False (1 << 0)
#define cJSON_True (1 << 1)
#define cJSON_NULL (1 << 2)
#define cJSON_Number (1 << 3)
#define cJSON_String (1 << 4)
#define cJSON_Array (1 << 5)
#define cJSON_Object (1 << 6)
#define cJSON_Raw (1 << 7) /* raw json */
#define cJSON_IsReference 256
#define cJSON_StringIsConst 512
/* The cJSON structure: */
typedefstructcJSON
{
/* next/prev allow you to walk array/object chains. Alternatively, use GetArraySize/GetArrayItem/GetObjectItem */
structcJSON*next;
structcJSON*prev;
/* An array or object item will have a child pointer pointing to a chain of the items in the array/object. */
structcJSON*child;
/* The type of the item, as above. */
inttype;
/* The item's string, if type==cJSON_String and type == cJSON_Raw */
char*valuestring;
/* writing to valueint is DEPRECATED, use cJSON_SetNumberValue instead */
intvalueint;
/* The item's number, if type==cJSON_Number */
doublevaluedouble;
/* The item's name string, if this item is the child of, or is in the list of subitems of an object. */
char*string;
}cJSON;
typedefstructcJSON_Hooks
{
/* malloc/free are CDECL on Windows regardless of the default calling convention of the compiler, so ensure the hooks allow passing those functions directly. */
void*(CJSON_CDECL*malloc_fn)(size_tsz);
void(CJSON_CDECL*free_fn)(void*ptr);
}cJSON_Hooks;
typedefintcJSON_bool;
/* Limits how deeply nested arrays/objects can be before cJSON rejects to parse them.
* This is to prevent stack overflows. */
#ifndef CJSON_NESTING_LIMIT
#define CJSON_NESTING_LIMIT 1000
#endif
/* returns the version of cJSON as a string */
CJSON_PUBLIC(constchar*)cJSON_Version(void);
/* Supply malloc, realloc and free functions to cJSON */
/* Memory Management: the caller is always responsible to free the results from all variants of cJSON_Parse (with cJSON_Delete) and cJSON_Print (with stdlib free, cJSON_Hooks.free_fn, or cJSON_free as appropriate). The exception is cJSON_PrintPreallocated, where the caller has full responsibility of the buffer. */
/* Supply a block of JSON, and this returns a cJSON object you can interrogate. */
/* ParseWithOpts allows you to require (and check) that the JSON is null terminated, and to retrieve the pointer to the final byte parsed. */
/* If you supply a ptr in return_parse_end and parsing fails, then return_parse_end will contain a pointer to the error so will match cJSON_GetErrorPtr(). */
/* Render a cJSON entity to text using a buffered strategy. prebuffer is a guess at the final size. guessing well reduces reallocation. fmt=0 gives unformatted, =1 gives formatted */
/* For analysing failed parses. This returns a pointer to the parse error. You'll probably need to look a few chars back to make sense of it. Defined when cJSON_Parse() returns 0. 0 when cJSON_Parse() succeeds. */
/* Append reference to item to the specified array/object. Use this when you want to add an existing cJSON to a new cJSON, but don't want to corrupt your existing cJSON. */
UI_Info(sprintf(gettext('Can\'t find %s in the system path list. If the name of the application is correct, please run \'which %s\' as a user with the correct PATH environment set up in order to find the fully-qualified path.'), $profiling, $profiling));
exit 1;
} else {
UI_Info(sprintf(gettext('%s does not exist, please double-check the path.'), $profiling));
exit 1;
}
}
}
exit 0;
sub usage {
UI_Info(sprintf(gettext("usage: \%s [ -d /path/to/profiles ] [ program to switch to audit mode ]"), $0));
UI_Important(sprintf(gettext('Can\'t find AppArmor profiles in %s.'), $profiledir));
exit 1;
}
# what are we profiling?
my @profiling = @ARGV;
unless (@profiling) {
@profiling = (UI_GetString(gettext("Please enter the program to create a profile for: "), ""));
}
for my $profiling (@profiling) {
next unless $profiling;
my $fqdbin;
if (-e $profiling) {
$fqdbin = get_full_path($profiling);
chomp($fqdbin);
} else {
if ($profiling !~ /\//) {
my $which = which($profiling);
if ($which) {
$fqdbin = get_full_path($which);
}
}
}
# make sure that the app they're requesting to profile is not marked as
# not allowed to have it's own profile
if ($qualifiers{$fqdbin}) {
unless ($qualifiers{$fqdbin} =~ /p/) {
UI_Info(sprintf(gettext('%s is currently marked as a program that should not have it\'s own profile. Usually, programs are marked this way if creating a profile for them is likely to break the rest of the system. If you know what you\'re doing and are certain you want to create a profile for this program, edit the corresponding entry in the [qualifiers] section in /etc/apparmor/logprof.conf.'), $fqdbin));
exit 1;
}
}
if (-e $fqdbin) {
if (-e getprofilefilename($fqdbin) && !$force) {
UI_Info(sprintf(gettext('Profile for %s already exists - skipping.'), $fqdbin));
} else {
autodep($fqdbin);
reload($fqdbin) if $sd_mountpoint;
}
} else {
if ($profiling =~ /^[^\/]+$/) {
UI_Info(sprintf(gettext('Can\'t find %s in the system path list. If the name of the application is correct, please run \'which %s\' as a user with the correct PATH environment set up in order to find the fully-qualified path.'), $profiling, $profiling));
exit 1;
} else {
UI_Info(sprintf(gettext('%s does not exist, please double-check the path.'), $profiling));
UI_Info(sprintf(gettext('Can\'t find %s in the system path list. If the name of the application is correct, please run \'which %s\' as a user with the correct PATH environment set up in order to find the fully-qualified path.'), $profiling, $profiling));
exit 1;
} else {
UI_Info(sprintf(gettext('%s does not exist, please double-check the path.'), $profiling));
exit 1;
}
}
}
exit 0;
sub usage {
UI_Info(sprintf(gettext("usage: \%s [ -d /path/to/profiles ] [ program to switch to complain mode ]"), $0));
UI_Info(sprintf(gettext('Can\'t find %s in the system path list. If the name of the application is correct, please run \'which %s\' as a user with the correct PATH environment set up in order to find the fully-qualified path.'), $profiling, $profiling));
exit 1;
} else {
UI_Info(sprintf(gettext('%s does not exist, please double-check the path.'), $profiling));
exit 1;
}
}
}
exit 0;
sub usage {
UI_Info(sprintf(gettext("usage: \%s [ -d /path/to/profiles ] [ program to have profile disabled ]"), $0));
UI_Info(sprintf(gettext('Can\'t find %s in the system path list. If the name of the application is correct, please run \'which %s\' as a user with the correct PATH environment set up in order to find the fully-qualified path.'), $profiling, $profiling));
exit 1;
} else {
UI_Info(sprintf(gettext('%s does not exist, please double-check the path.'), $profiling));
exit 1;
}
}
}
exit 0;
sub usage {
UI_Info(sprintf(gettext("usage: \%s [ -d /path/to/profiles ] [ program to switch to enforce mode ]"), $0));
fatal_error(gettext("AppArmor does not appear to be started. Please enable AppArmor and try again."));
}
# let's convert it to full path...
$profiledir = get_full_path($profiledir);
unless (-d $profiledir) {
fatal_error "Can't find AppArmor profiles in $profiledir.";
}
# what are we profiling?
my $profiling = shift;
unless ($profiling) {
$profiling = UI_GetString(gettext("Please enter the program to profile: "), "")
|| exit 0;
}
my $fqdbin;
if (-e $profiling) {
$fqdbin = get_full_path($profiling);
chomp($fqdbin);
} else {
if ($profiling !~ /\//) {
my $which = which($profiling);
if ($which) {
$fqdbin = get_full_path($which);
}
}
}
unless ($fqdbin && -e $fqdbin) {
if ($profiling =~ /^[^\/]+$/) {
fatal_error(sprintf(gettext('Can\'t find %s in the system path list. If the name of the application is correct, please run \'which %s\' in the other window in order to find the fully-qualified path.'), $profiling, $profiling));
} else {
fatal_error(sprintf(gettext('%s does not exist, please double-check the path.'), $profiling));
}
}
# make sure that the app they're requesting to profile is not marked as
# not allowed to have it's own profile
check_qualifiers($fqdbin);
# load all the include files
loadincludes();
my $profilefilename = getprofilefilename($fqdbin);
# When reading from syslog, it is possible to hit the default kernel
# printk ratelimit. This will result in audit entries getting skipped,
# making profile generation inaccurate. When using genprof, disable
# the printk ratelimit, and restore it on exit.
my $ratelimit_sysctl = "/proc/sys/kernel/printk_ratelimit";
my $ratelimit_saved = sysctl_read($ratelimit_sysctl);
END { sysctl_write($ratelimit_sysctl, $ratelimit_saved); }
sysctl_write($ratelimit_sysctl, 0);
UI_Info(gettext("\nBefore you begin, you may wish to check if a\nprofile already exists for the application you\nwish to confine. See the following wiki page for\nmore information:\nhttp://wiki.apparmor.net/index.php/Profiles"));
UI_Important(gettext("Please start the application to be profiled in \nanother window and exercise its functionality now.\n\nOnce completed, select the \"Scan\" button below in \norder to scan the system logs for AppArmor events. \n\nFor each AppArmor event, you will be given the \nopportunity to choose whether the access should be \nallowed or denied."));
UI_Info(gettext("Reloaded AppArmor profiles in enforce mode."));
UI_Info(gettext("\nPlease consider contributing your new profile! See\nthe following wiki page for more information:\nhttp://wiki.apparmor.net/index.php/Profiles\n"));
UI_Info(sprintf(gettext('Finished generating profile for %s.'), $fqdbin));
exit 0;
sub usage {
UI_Info(sprintf(gettext("usage: \%s [ -d /path/to/profiles ] [ -f /path/to/logfile ] [ program to profile ]"), $0));
test -f ./.libs/$(EXPECTED_SO_NAME)||{echo'*** unexpected .so name/number for libapparmor (expected $(EXPECTED_SO_NAME), the actual filename is shown below) ***'; ls -l ./.libs/libapparmor.so.*.* ;exit 1;}
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.
