[PATCH v2 0/7] Couple of apparmor fixes

v2 of: https://www.redhat.com/archives/libvir-list/2020-January/msg01068.html diff to v1: - Keep old paths to virt-aa-helper in profiles as SUSE still uses it. - patch 7/7 is new Michal Prívozník (7): apparmor: Fix parthelper, iohelper and virt-aa-helper paths in profiles apparmor: Allow libvirt to spawn virt-aa-helper and libvirt_lxc docs: Fix virt-aa-helper location apparmor: Rename virt-aa-helper profile apparmor: Sort paths in blocks in libvirt-qemu profile apparmor: Allow some more BIOS/UEFI paths apparmor: Drop 'Last modified' comment from profiles docs/drvqemu.html.in | 2 +- src/security/Makefile.inc.am | 10 +-- src/security/apparmor/libvirt-lxc | 2 - src/security/apparmor/libvirt-qemu | 80 +++++++++---------- ...t-aa-helper => usr.libexec.virt-aa-helper} | 7 +- src/security/apparmor/usr.sbin.libvirtd | 7 +- 6 files changed, 53 insertions(+), 55 deletions(-) rename src/security/apparmor/{usr.lib.libvirt.virt-aa-helper => usr.libexec.virt-aa-helper} (88%) -- 2.24.1

These helper binaries are installed under libexec dir not lib dir. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/apparmor/usr.lib.libvirt.virt-aa-helper | 2 +- src/security/apparmor/usr.sbin.libvirtd | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper index 11e9c039ca..ca1f6ca083 100644 --- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper @@ -39,7 +39,7 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { deny /dev/mapper/ r, deny /dev/mapper/* r, - /usr/{lib,lib64}/libvirt/virt-aa-helper mr, + /usr/{lib,lib64,libexec}/libvirt/virt-aa-helper mr, /{usr/,}sbin/apparmor_parser Ux, /etc/apparmor.d/libvirt/* r, diff --git a/src/security/apparmor/usr.sbin.libvirtd b/src/security/apparmor/usr.sbin.libvirtd index 29f9936ad9..2089ba1b3e 100644 --- a/src/security/apparmor/usr.sbin.libvirtd +++ b/src/security/apparmor/usr.sbin.libvirtd @@ -100,8 +100,8 @@ profile libvirtd /usr/sbin/libvirtd flags=(attach_disconnected) { audit deny /sys/kernel/security/apparmor/.* rwxl, /sys/kernel/security/apparmor/profiles r, /usr/{lib,lib64}/libvirt/* PUxr, - /usr/{lib,lib64}/libvirt/libvirt_parthelper ix, - /usr/{lib,lib64}/libvirt/libvirt_iohelper ix, + /usr/libexec/libvirt_parthelper ix, + /usr/libexec/libvirt_iohelper ix, /etc/libvirt/hooks/** rmix, /etc/xen/scripts/** rmix, -- 2.24.1

On Thu, Jan 30, 2020 at 8:05 AM Michal Privoznik <mprivozn@redhat.com> wrote:
These helper binaries are installed under libexec dir not lib dir.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/apparmor/usr.lib.libvirt.virt-aa-helper | 2 +- src/security/apparmor/usr.sbin.libvirtd | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper index 11e9c039ca..ca1f6ca083 100644 --- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper @@ -39,7 +39,7 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { deny /dev/mapper/ r, deny /dev/mapper/* r,
- /usr/{lib,lib64}/libvirt/virt-aa-helper mr, + /usr/{lib,lib64,libexec}/libvirt/virt-aa-helper mr, /{usr/,}sbin/apparmor_parser Ux,
/etc/apparmor.d/libvirt/* r, diff --git a/src/security/apparmor/usr.sbin.libvirtd b/src/security/apparmor/usr.sbin.libvirtd index 29f9936ad9..2089ba1b3e 100644 --- a/src/security/apparmor/usr.sbin.libvirtd +++ b/src/security/apparmor/usr.sbin.libvirtd @@ -100,8 +100,8 @@ profile libvirtd /usr/sbin/libvirtd flags=(attach_disconnected) { audit deny /sys/kernel/security/apparmor/.* rwxl, /sys/kernel/security/apparmor/profiles r, /usr/{lib,lib64}/libvirt/* PUxr, - /usr/{lib,lib64}/libvirt/libvirt_parthelper ix, - /usr/{lib,lib64}/libvirt/libvirt_iohelper ix, + /usr/libexec/libvirt_parthelper ix, + /usr/libexec/libvirt_iohelper ix,
This needs the same {lib,lib64,libexec} treatment. E.g. on Debian/Ubuntu this is in: /usr/lib/libvirt/libvirt_parthelper Suse most likely again has lib64 here. As I suggested in one of the patches I think we either want full dir listings for all common cases here or make it dependent to the --libexecdir configure option.
/etc/libvirt/hooks/** rmix, /etc/xen/scripts/** rmix,
-- 2.24.1
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd

Both of these binaries are spawn by libvirt. Add a rule to the default profile to allow that. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/apparmor/usr.sbin.libvirtd | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/security/apparmor/usr.sbin.libvirtd b/src/security/apparmor/usr.sbin.libvirtd index 2089ba1b3e..27314b1512 100644 --- a/src/security/apparmor/usr.sbin.libvirtd +++ b/src/security/apparmor/usr.sbin.libvirtd @@ -100,6 +100,8 @@ profile libvirtd /usr/sbin/libvirtd flags=(attach_disconnected) { audit deny /sys/kernel/security/apparmor/.* rwxl, /sys/kernel/security/apparmor/profiles r, /usr/{lib,lib64}/libvirt/* PUxr, + /usr/libexec/virt-aa-helper PUxr, + /usr/libexec/libvirt_lxc PUxr, /usr/libexec/libvirt_parthelper ix, /usr/libexec/libvirt_iohelper ix, /etc/libvirt/hooks/** rmix, -- 2.24.1

On Thu, Jan 30, 2020 at 8:05 AM Michal Privoznik <mprivozn@redhat.com> wrote:
Both of these binaries are spawn by libvirt. Add a rule to the default profile to allow that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/apparmor/usr.sbin.libvirtd | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/security/apparmor/usr.sbin.libvirtd b/src/security/apparmor/usr.sbin.libvirtd index 2089ba1b3e..27314b1512 100644 --- a/src/security/apparmor/usr.sbin.libvirtd +++ b/src/security/apparmor/usr.sbin.libvirtd @@ -100,6 +100,8 @@ profile libvirtd /usr/sbin/libvirtd flags=(attach_disconnected) { audit deny /sys/kernel/security/apparmor/.* rwxl, /sys/kernel/security/apparmor/profiles r, /usr/{lib,lib64}/libvirt/* PUxr, + /usr/libexec/virt-aa-helper PUxr, + /usr/libexec/libvirt_lxc PUxr,
Again - I'd appreciate if we could here use generated paths based on --libexecdir configure option.
/usr/libexec/libvirt_parthelper ix, /usr/libexec/libvirt_iohelper ix, /etc/libvirt/hooks/** rmix, -- 2.24.1
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd

The location of virt-aa-helper shown in the docs is incorrect. The helper binary is installed under libexec dir. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/drvqemu.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/drvqemu.html.in b/docs/drvqemu.html.in index 87542afd27..93a7e6e7df 100644 --- a/docs/drvqemu.html.in +++ b/docs/drvqemu.html.in @@ -439,7 +439,7 @@ chmod o+x /path/to/directory <p> While users can define their own AppArmor profile scheme, a typical configuration will include a profile for <code>/usr/sbin/libvirtd</code>, - <code>/usr/lib/libvirt/virt-aa-helper</code> (a helper program which the + <code>/usr/libexec/virt-aa-helper</code> (a helper program which the libvirtd daemon uses instead of manipulating AppArmor directly), and an abstraction to be included by <code>/etc/apparmor.d/libvirt/TEMPLATE</code> (typically <code>/etc/apparmor.d/abstractions/libvirt-qemu</code>). -- 2.24.1

On Thu, Jan 30, 2020 at 8:06 AM Michal Privoznik <mprivozn@redhat.com> wrote:
The location of virt-aa-helper shown in the docs is incorrect. The helper binary is installed under libexec dir.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/drvqemu.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/drvqemu.html.in b/docs/drvqemu.html.in index 87542afd27..93a7e6e7df 100644 --- a/docs/drvqemu.html.in +++ b/docs/drvqemu.html.in @@ -439,7 +439,7 @@ chmod o+x /path/to/directory <p> While users can define their own AppArmor profile scheme, a typical configuration will include a profile for <code>/usr/sbin/libvirtd</code>, - <code>/usr/lib/libvirt/virt-aa-helper</code> (a helper program which the + <code>/usr/libexec/virt-aa-helper</code> (a helper program which the
Don't get me wrong please the changes in this series seem correct to me to have filenames/doc match the rest of the code. But as it became clear in the discussion on v1 - the two Distributions that actually run apparmor usually (Suse and Ubuntu) have the helper in different places. We seem to agree on the rules to list a multitude of paths. But I wonder if in docs and makefiles this should be part of the .html.in -> .html conversion to set the right paths for each. Otherwise Suse and Ubuntu will have to carry reverts or modifications of these changes forever. For example the path above is in fact the correct path on Ubuntu - that might also be the history how it became that way in the src. Essentially this comes through config option --libexecdir=/usr/lib/libvirt, so we would not even have to add a new argument for this. What do others think about this (for all the patches of the series it applies to)?
libvirtd daemon uses instead of manipulating AppArmor directly), and an abstraction to be included by <code>/etc/apparmor.d/libvirt/TEMPLATE</code> (typically <code>/etc/apparmor.d/abstractions/libvirt-qemu</code>). -- 2.24.1
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd

On 1/30/20 8:39 AM, Christian Ehrhardt wrote:
On Thu, Jan 30, 2020 at 8:06 AM Michal Privoznik <mprivozn@redhat.com <mailto:mprivozn@redhat.com>> wrote:
The location of virt-aa-helper shown in the docs is incorrect. The helper binary is installed under libexec dir.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com <mailto:mprivozn@redhat.com>> --- docs/drvqemu.html.in <http://drvqemu.html.in> | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/drvqemu.html.in <http://drvqemu.html.in> b/docs/drvqemu.html.in <http://drvqemu.html.in> index 87542afd27..93a7e6e7df 100644 --- a/docs/drvqemu.html.in <http://drvqemu.html.in> +++ b/docs/drvqemu.html.in <http://drvqemu.html.in> @@ -439,7 +439,7 @@ chmod o+x /path/to/directory <p> While users can define their own AppArmor profile scheme, a typical configuration will include a profile for <code>/usr/sbin/libvirtd</code>, - <code>/usr/lib/libvirt/virt-aa-helper</code> (a helper program which the + <code>/usr/libexec/virt-aa-helper</code> (a helper program which the
Don't get me wrong please the changes in this series seem correct to me to have filenames/doc match the rest of the code.
But as it became clear in the discussion on v1 - the two Distributions that actually run apparmor usually (Suse and Ubuntu) have the helper in different places. We seem to agree on the rules to list a multitude of paths.
But I wonder if in docs and makefiles this should be part of the .html.in <http://html.in> -> .html conversion to set the right paths for each. Otherwise Suse and Ubuntu will have to carry reverts or modifications of these changes forever. For example the path above is in fact the correct path on Ubuntu - that might also be the history how it became that way in the src.
Essentially this comes through config option --libexecdir=/usr/lib/libvirt, so we would not even have to add a new argument for this. What do others think about this (for all the patches of the series it applies to)?
Aha! This is the missing part. I didn't realize that one can specify libexecdir independently. Alright, let me see if I can get these generated. In order to do that I would probably need to move existing files to XXX.in and have a simple sed to replace LIBEXEC with its actual value. Meanwhile, I will merge the three patches you ACKed since they are independent of the rest. Thanks, Michal

The profile name should reflect the path under which the binary it describes is installed. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/Makefile.inc.am | 10 +++++----- ...bvirt.virt-aa-helper => usr.libexec.virt-aa-helper} | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) rename src/security/apparmor/{usr.lib.libvirt.virt-aa-helper => usr.libexec.virt-aa-helper} (93%) diff --git a/src/security/Makefile.inc.am b/src/security/Makefile.inc.am index 6fe9d50f29..02efefd6d6 100644 --- a/src/security/Makefile.inc.am +++ b/src/security/Makefile.inc.am @@ -38,7 +38,7 @@ EXTRA_DIST += \ security/apparmor/TEMPLATE.lxc \ security/apparmor/libvirt-qemu \ security/apparmor/libvirt-lxc \ - security/apparmor/usr.lib.libvirt.virt-aa-helper \ + security/apparmor/usr.libexec.virt-aa-helper \ security/apparmor/usr.sbin.libvirtd \ $(NULL) @@ -91,7 +91,7 @@ endif WITH_SECDRIVER_APPARMOR if WITH_APPARMOR_PROFILES apparmordir = $(sysconfdir)/apparmor.d/ apparmor_DATA = \ - security/apparmor/usr.lib.libvirt.virt-aa-helper \ + security/apparmor/usr.libexec.virt-aa-helper \ security/apparmor/usr.sbin.libvirtd \ $(NULL) @@ -111,11 +111,11 @@ APPARMOR_LOCAL_DIR = "$(DESTDIR)$(apparmordir)/local" install-apparmor-local: $(MKDIR_P) "$(APPARMOR_LOCAL_DIR)" echo "# Site-specific additions and overrides for \ - 'usr.lib.libvirt.virt-aa-helper'" \ - >"$(APPARMOR_LOCAL_DIR)/usr.lib.libvirt.virt-aa-helper" + 'usr.libexec.virt-aa-helper'" \ + >"$(APPARMOR_LOCAL_DIR)/usr.libexec.virt-aa-helper" uninstall-apparmor-local: - rm -f "$(APPARMOR_LOCAL_DIR)/usr.lib.libvirt.virt-aa-helper" + rm -f "$(APPARMOR_LOCAL_DIR)/usr.libexec.virt-aa-helper" rmdir "$(APPARMOR_LOCAL_DIR)" || : INSTALL_DATA_LOCAL += install-apparmor-local diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper b/src/security/apparmor/usr.libexec.virt-aa-helper similarity index 93% rename from src/security/apparmor/usr.lib.libvirt.virt-aa-helper rename to src/security/apparmor/usr.libexec.virt-aa-helper index ca1f6ca083..72a2fecebe 100644 --- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/src/security/apparmor/usr.libexec.virt-aa-helper @@ -1,7 +1,7 @@ # Last Modified: Mon Apr 5 15:10:27 2010 #include <tunables/global> -profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { +profile virt-aa-helper /usr/{lib,lib64,libexec}/libvirt/virt-aa-helper { #include <abstractions/base> # needed for searching directories @@ -70,5 +70,5 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { /**.[iI][sS][oO] r, /**/disk{,.*} r, - #include <local/usr.lib.libvirt.virt-aa-helper> + #include <local/usr.libexec.virt-aa-helper> } -- 2.24.1

On Thu, Jan 30, 2020 at 8:06 AM Michal Privoznik <mprivozn@redhat.com> wrote:
The profile name should reflect the path under which the binary it describes is installed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/Makefile.inc.am | 10 +++++----- ...bvirt.virt-aa-helper => usr.libexec.virt-aa-helper} | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) rename src/security/apparmor/{usr.lib.libvirt.virt-aa-helper => usr.libexec.virt-aa-helper} (93%)
diff --git a/src/security/Makefile.inc.am b/src/security/Makefile.inc.am index 6fe9d50f29..02efefd6d6 100644 --- a/src/security/Makefile.inc.am +++ b/src/security/Makefile.inc.am @@ -38,7 +38,7 @@ EXTRA_DIST += \ security/apparmor/TEMPLATE.lxc \ security/apparmor/libvirt-qemu \ security/apparmor/libvirt-lxc \ - security/apparmor/usr.lib.libvirt.virt-aa-helper \ + security/apparmor/usr.libexec.virt-aa-helper \
Again - probably better to make it dependent on --libexecdir configure option. The old path matches the real Ubuntu path, so "for me" that would be a regression making me carry a revert.
security/apparmor/usr.sbin.libvirtd \ $(NULL)
@@ -91,7 +91,7 @@ endif WITH_SECDRIVER_APPARMOR if WITH_APPARMOR_PROFILES apparmordir = $(sysconfdir)/apparmor.d/ apparmor_DATA = \ - security/apparmor/usr.lib.libvirt.virt-aa-helper \ + security/apparmor/usr.libexec.virt-aa-helper \ security/apparmor/usr.sbin.libvirtd \ $(NULL)
@@ -111,11 +111,11 @@ APPARMOR_LOCAL_DIR = "$(DESTDIR)$(apparmordir)/local" install-apparmor-local: $(MKDIR_P) "$(APPARMOR_LOCAL_DIR)" echo "# Site-specific additions and overrides for \ - 'usr.lib.libvirt.virt-aa-helper'" \ - >"$(APPARMOR_LOCAL_DIR)/usr.lib.libvirt.virt-aa-helper" + 'usr.libexec.virt-aa-helper'" \ + >"$(APPARMOR_LOCAL_DIR)/usr.libexec.virt-aa-helper"
uninstall-apparmor-local: - rm -f "$(APPARMOR_LOCAL_DIR)/usr.lib.libvirt.virt-aa-helper" + rm -f "$(APPARMOR_LOCAL_DIR)/usr.libexec.virt-aa-helper" rmdir "$(APPARMOR_LOCAL_DIR)" || :
INSTALL_DATA_LOCAL += install-apparmor-local diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper b/src/security/apparmor/usr.libexec.virt-aa-helper similarity index 93% rename from src/security/apparmor/usr.lib.libvirt.virt-aa-helper rename to src/security/apparmor/usr.libexec.virt-aa-helper index ca1f6ca083..72a2fecebe 100644 --- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/src/security/apparmor/usr.libexec.virt-aa-helper @@ -1,7 +1,7 @@ # Last Modified: Mon Apr 5 15:10:27 2010 #include <tunables/global>
-profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { +profile virt-aa-helper /usr/{lib,lib64,libexec}/libvirt/virt-aa-helper { #include <abstractions/base>
# needed for searching directories @@ -70,5 +70,5 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { /**.[iI][sS][oO] r, /**/disk{,.*} r,
- #include <local/usr.lib.libvirt.virt-aa-helper> + #include <local/usr.libexec.virt-aa-helper> } -- 2.24.1
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd

Even though we construct a domain specific profile for each domain we start (which should cover domain specific paths), there is also another file that is included from the profile and which contains domain agnostic paths (e.g. to cover libraries that qemu links with). The paths in the file are split into blocks divided by comments. Sort the paths in each block individually (ignoring case sensitivity). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/apparmor/libvirt-qemu | 76 +++++++++++++++--------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index d33348aa05..2291829270 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -22,8 +22,8 @@ signal (receive) peer=libvirtd, signal (receive) peer=/usr/sbin/libvirtd, - /dev/net/tun rw, /dev/kvm rw, + /dev/net/tun rw, /dev/ptmx rw, @{PROC}/*/status r, # When qemu is signaled to terminate, it will read cmdline of signaling @@ -39,19 +39,19 @@ /sys/bus/usb/devices/ r, /sys/devices/**/usb[0-9]*/** r, # libusb needs udev data about usb devices (~equal to content of lsusb -v) + /run/udev/data/+usb* r, /run/udev/data/c16[6,7]* r, /run/udev/data/c18[0,8,9]* r, - /run/udev/data/+usb* r, # WARNING: this gives the guest direct access to host hardware and specific # portions of shared memory. This is required for sound using ALSA with kvm, # but may constitute a security risk. If your environment does not require # the use of sound in your VMs, feel free to comment out or prepend 'deny' to # the rules for files in /dev. + /dev/snd/* rw, /{dev,run}/shm r, /{dev,run}/shmpulse-shm* r, /{dev,run}/shmpulse-shm* rwk, - /dev/snd/* rw, capability ipc_lock, # spice owner /{dev,run}/shm/spice.* rw, @@ -73,21 +73,21 @@ /var/lib/dbus/machine-id r, # access to firmware's etc - /usr/share/kvm/** r, - /usr/share/qemu/** r, - /usr/share/qemu-kvm/** r, + /usr/share/AAVMF/** r, /usr/share/bochs/** r, + /usr/share/kvm/** r, + /usr/share/misc/sgabios.bin r, /usr/share/openbios/** r, /usr/share/openhackware/** r, - /usr/share/proll/** r, - /usr/share/vgabios/** r, - /usr/share/seabios/** r, - /usr/share/misc/sgabios.bin r, - /usr/share/ovmf/** r, /usr/share/OVMF/** r, - /usr/share/AAVMF/** r, + /usr/share/ovmf/** r, + /usr/share/proll/** r, /usr/share/qemu-efi/** r, + /usr/share/qemu-kvm/** r, + /usr/share/qemu/** r, + /usr/share/seabios/** r, /usr/share/slof/** r, + /usr/share/vgabios/** r, # pki for libvirt-vnc and libvirt-spice (LP: #901272, #1690140) /etc/pki/CA/ r, @@ -98,7 +98,33 @@ # the various binaries /usr/bin/kvm rmix, /usr/bin/qemu rmix, + /usr/bin/qemu-aarch64 rmix, + /usr/bin/qemu-alpha rmix, + /usr/bin/qemu-arm rmix, + /usr/bin/qemu-armeb rmix, + /usr/bin/qemu-cris rmix, + /usr/bin/qemu-i386 rmix, /usr/bin/qemu-kvm rmix, + /usr/bin/qemu-m68k rmix, + /usr/bin/qemu-microblaze rmix, + /usr/bin/qemu-microblazeel rmix, + /usr/bin/qemu-mips rmix, + /usr/bin/qemu-mips64 rmix, + /usr/bin/qemu-mips64el rmix, + /usr/bin/qemu-mipsel rmix, + /usr/bin/qemu-mipsn32 rmix, + /usr/bin/qemu-mipsn32el rmix, + /usr/bin/qemu-or32 rmix, + /usr/bin/qemu-ppc rmix, + /usr/bin/qemu-ppc64 rmix, + /usr/bin/qemu-ppc64abi32 rmix, + /usr/bin/qemu-ppc64le rmix, + /usr/bin/qemu-s390x rmix, + /usr/bin/qemu-sh4 rmix, + /usr/bin/qemu-sh4eb rmix, + /usr/bin/qemu-sparc rmix, + /usr/bin/qemu-sparc32plus rmix, + /usr/bin/qemu-sparc64 rmix, /usr/bin/qemu-system-aarch64 rmix, /usr/bin/qemu-system-alpha rmix, /usr/bin/qemu-system-arm rmix, @@ -132,32 +158,6 @@ /usr/bin/qemu-system-x86_64 rmix, /usr/bin/qemu-system-xtensa rmix, /usr/bin/qemu-system-xtensaeb rmix, - /usr/bin/qemu-aarch64 rmix, - /usr/bin/qemu-alpha rmix, - /usr/bin/qemu-arm rmix, - /usr/bin/qemu-armeb rmix, - /usr/bin/qemu-cris rmix, - /usr/bin/qemu-i386 rmix, - /usr/bin/qemu-m68k rmix, - /usr/bin/qemu-microblaze rmix, - /usr/bin/qemu-microblazeel rmix, - /usr/bin/qemu-mips rmix, - /usr/bin/qemu-mips64 rmix, - /usr/bin/qemu-mips64el rmix, - /usr/bin/qemu-mipsel rmix, - /usr/bin/qemu-mipsn32 rmix, - /usr/bin/qemu-mipsn32el rmix, - /usr/bin/qemu-or32 rmix, - /usr/bin/qemu-ppc rmix, - /usr/bin/qemu-ppc64 rmix, - /usr/bin/qemu-ppc64abi32 rmix, - /usr/bin/qemu-ppc64le rmix, - /usr/bin/qemu-s390x rmix, - /usr/bin/qemu-sh4 rmix, - /usr/bin/qemu-sh4eb rmix, - /usr/bin/qemu-sparc rmix, - /usr/bin/qemu-sparc32plus rmix, - /usr/bin/qemu-sparc64 rmix, /usr/bin/qemu-unicore32 rmix, /usr/bin/qemu-x86_64 rmix, # for Debian/Ubuntu qemu-block-extra / RPMs qemu-block-* (LP: #1554761) -- 2.24.1

On Thu, Jan 30, 2020 at 8:06 AM Michal Privoznik <mprivozn@redhat.com> wrote:
Even though we construct a domain specific profile for each domain we start (which should cover domain specific paths), there is also another file that is included from the profile and which contains domain agnostic paths (e.g. to cover libraries that qemu links with). The paths in the file are split into blocks divided by comments. Sort the paths in each block individually (ignoring case sensitivity).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
--- src/security/apparmor/libvirt-qemu | 76 +++++++++++++++--------------- 1 file changed, 38 insertions(+), 38 deletions(-)
diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index d33348aa05..2291829270 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -22,8 +22,8 @@ signal (receive) peer=libvirtd, signal (receive) peer=/usr/sbin/libvirtd,
- /dev/net/tun rw, /dev/kvm rw, + /dev/net/tun rw, /dev/ptmx rw, @{PROC}/*/status r, # When qemu is signaled to terminate, it will read cmdline of signaling @@ -39,19 +39,19 @@ /sys/bus/usb/devices/ r, /sys/devices/**/usb[0-9]*/** r, # libusb needs udev data about usb devices (~equal to content of lsusb -v) + /run/udev/data/+usb* r, /run/udev/data/c16[6,7]* r, /run/udev/data/c18[0,8,9]* r, - /run/udev/data/+usb* r,
# WARNING: this gives the guest direct access to host hardware and specific # portions of shared memory. This is required for sound using ALSA with kvm, # but may constitute a security risk. If your environment does not require # the use of sound in your VMs, feel free to comment out or prepend 'deny' to # the rules for files in /dev. + /dev/snd/* rw, /{dev,run}/shm r, /{dev,run}/shmpulse-shm* r, /{dev,run}/shmpulse-shm* rwk, - /dev/snd/* rw, capability ipc_lock, # spice owner /{dev,run}/shm/spice.* rw, @@ -73,21 +73,21 @@ /var/lib/dbus/machine-id r,
# access to firmware's etc - /usr/share/kvm/** r, - /usr/share/qemu/** r, - /usr/share/qemu-kvm/** r, + /usr/share/AAVMF/** r, /usr/share/bochs/** r, + /usr/share/kvm/** r, + /usr/share/misc/sgabios.bin r, /usr/share/openbios/** r, /usr/share/openhackware/** r, - /usr/share/proll/** r, - /usr/share/vgabios/** r, - /usr/share/seabios/** r, - /usr/share/misc/sgabios.bin r, - /usr/share/ovmf/** r, /usr/share/OVMF/** r, - /usr/share/AAVMF/** r, + /usr/share/ovmf/** r, + /usr/share/proll/** r, /usr/share/qemu-efi/** r, + /usr/share/qemu-kvm/** r, + /usr/share/qemu/** r, + /usr/share/seabios/** r, /usr/share/slof/** r, + /usr/share/vgabios/** r,
# pki for libvirt-vnc and libvirt-spice (LP: #901272, #1690140) /etc/pki/CA/ r, @@ -98,7 +98,33 @@ # the various binaries /usr/bin/kvm rmix, /usr/bin/qemu rmix, + /usr/bin/qemu-aarch64 rmix, + /usr/bin/qemu-alpha rmix, + /usr/bin/qemu-arm rmix, + /usr/bin/qemu-armeb rmix, + /usr/bin/qemu-cris rmix, + /usr/bin/qemu-i386 rmix, /usr/bin/qemu-kvm rmix, + /usr/bin/qemu-m68k rmix, + /usr/bin/qemu-microblaze rmix, + /usr/bin/qemu-microblazeel rmix, + /usr/bin/qemu-mips rmix, + /usr/bin/qemu-mips64 rmix, + /usr/bin/qemu-mips64el rmix, + /usr/bin/qemu-mipsel rmix, + /usr/bin/qemu-mipsn32 rmix, + /usr/bin/qemu-mipsn32el rmix, + /usr/bin/qemu-or32 rmix, + /usr/bin/qemu-ppc rmix, + /usr/bin/qemu-ppc64 rmix, + /usr/bin/qemu-ppc64abi32 rmix, + /usr/bin/qemu-ppc64le rmix, + /usr/bin/qemu-s390x rmix, + /usr/bin/qemu-sh4 rmix, + /usr/bin/qemu-sh4eb rmix, + /usr/bin/qemu-sparc rmix, + /usr/bin/qemu-sparc32plus rmix, + /usr/bin/qemu-sparc64 rmix, /usr/bin/qemu-system-aarch64 rmix, /usr/bin/qemu-system-alpha rmix, /usr/bin/qemu-system-arm rmix, @@ -132,32 +158,6 @@ /usr/bin/qemu-system-x86_64 rmix, /usr/bin/qemu-system-xtensa rmix, /usr/bin/qemu-system-xtensaeb rmix, - /usr/bin/qemu-aarch64 rmix, - /usr/bin/qemu-alpha rmix, - /usr/bin/qemu-arm rmix, - /usr/bin/qemu-armeb rmix, - /usr/bin/qemu-cris rmix, - /usr/bin/qemu-i386 rmix, - /usr/bin/qemu-m68k rmix, - /usr/bin/qemu-microblaze rmix, - /usr/bin/qemu-microblazeel rmix, - /usr/bin/qemu-mips rmix, - /usr/bin/qemu-mips64 rmix, - /usr/bin/qemu-mips64el rmix, - /usr/bin/qemu-mipsel rmix, - /usr/bin/qemu-mipsn32 rmix, - /usr/bin/qemu-mipsn32el rmix, - /usr/bin/qemu-or32 rmix, - /usr/bin/qemu-ppc rmix, - /usr/bin/qemu-ppc64 rmix, - /usr/bin/qemu-ppc64abi32 rmix, - /usr/bin/qemu-ppc64le rmix, - /usr/bin/qemu-s390x rmix, - /usr/bin/qemu-sh4 rmix, - /usr/bin/qemu-sh4eb rmix, - /usr/bin/qemu-sparc rmix, - /usr/bin/qemu-sparc32plus rmix, - /usr/bin/qemu-sparc64 rmix, /usr/bin/qemu-unicore32 rmix, /usr/bin/qemu-x86_64 rmix, # for Debian/Ubuntu qemu-block-extra / RPMs qemu-block-* (LP: #1554761) -- 2.24.1
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd

There are two more paths that we are missing in the default domain profile: /usr/share/edk2-ovmf/ and /usr/share/sgabios/. These exist on my Gentoo box and contain UEFI and BIOS images respectively. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/apparmor/libvirt-qemu | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index 2291829270..6942b83969 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -75,6 +75,7 @@ # access to firmware's etc /usr/share/AAVMF/** r, /usr/share/bochs/** r, + /usr/share/edk2-ovmf/** r, /usr/share/kvm/** r, /usr/share/misc/sgabios.bin r, /usr/share/openbios/** r, @@ -86,6 +87,7 @@ /usr/share/qemu-kvm/** r, /usr/share/qemu/** r, /usr/share/seabios/** r, + /usr/share/sgabios/** r, /usr/share/slof/** r, /usr/share/vgabios/** r, -- 2.24.1

On Thu, Jan 30, 2020 at 8:05 AM Michal Privoznik <mprivozn@redhat.com> wrote:
There are two more paths that we are missing in the default domain profile: /usr/share/edk2-ovmf/ and /usr/share/sgabios/. These exist on my Gentoo box and contain UEFI and BIOS images respectively.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
--- src/security/apparmor/libvirt-qemu | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index 2291829270..6942b83969 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -75,6 +75,7 @@ # access to firmware's etc /usr/share/AAVMF/** r, /usr/share/bochs/** r, + /usr/share/edk2-ovmf/** r, /usr/share/kvm/** r, /usr/share/misc/sgabios.bin r, /usr/share/openbios/** r, @@ -86,6 +87,7 @@ /usr/share/qemu-kvm/** r, /usr/share/qemu/** r, /usr/share/seabios/** r, + /usr/share/sgabios/** r, /usr/share/slof/** r, /usr/share/vgabios/** r,
-- 2.24.1
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd

On Thu, Jan 30, 2020 at 8:05 AM Michal Privoznik <mprivozn@redhat.com> wrote:
There are two more paths that we are missing in the default domain profile: /usr/share/edk2-ovmf/ and /usr/share/sgabios/. These exist on my Gentoo box and contain UEFI and BIOS images respectively.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/apparmor/libvirt-qemu | 2 ++ 1 file changed, 2 insertions(+)
Hi Michal, You might already have abandoned this as I've seen other parts of the series land (thanks for the dynamic paths now).But revisiting this I found that they seem not needed.
diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index 2291829270..6942b83969 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -75,6 +75,7 @@ # access to firmware's etc /usr/share/AAVMF/** r, /usr/share/bochs/** r, + /usr/share/edk2-ovmf/** r,
At least on Debian/Ubuntu the multiple edk2 related cases are already covered by 85342a3771b (Guido Günther 2014-04-07 12:15:02 +0200) /usr/share/ovmf/** r, f9803f59148 (Guido Günther 2017-07-06 11:04:21 +0200) /usr/share/OVMF/** r, f9803f59148 (Guido Günther 2017-07-06 11:04:21 +0200) /usr/share/AAVMF/** r, f9803f59148 (Guido Günther 2017-07-06 11:04:21 +0200) /usr/share/qemu-efi/** r,
/usr/share/kvm/** r, /usr/share/misc/sgabios.bin r, /usr/share/openbios/** r, @@ -86,6 +87,7 @@ /usr/share/qemu-kvm/** r, /usr/share/qemu/** r, /usr/share/seabios/** r, + /usr/share/sgabios/** r,
Again for Debian/Ubuntu this is already covered by: 987d1fdc535 (Guido Günther 2018-01-15 09:44:37 +0100) /usr/share/misc/sgabios.bin r, I guess Suse would have mentioned if the paths would not have worked for them. Did you have another Distro which uses the paths that try to add here?
/usr/share/slof/** r, /usr/share/vgabios/** r,
--
2.24.1
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd

On 2/12/20 11:38 AM, Christian Ehrhardt wrote:
On Thu, Jan 30, 2020 at 8:05 AM Michal Privoznik <mprivozn@redhat.com <mailto:mprivozn@redhat.com>> wrote:
There are two more paths that we are missing in the default domain profile: /usr/share/edk2-ovmf/ and /usr/share/sgabios/. These exist on my Gentoo box and contain UEFI and BIOS images respectively.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com <mailto:mprivozn@redhat.com>> --- src/security/apparmor/libvirt-qemu | 2 ++ 1 file changed, 2 insertions(+)
Hi Michal, You might already have abandoned this as I've seen other parts of the series land (thanks for the dynamic paths now).But revisiting this I found that they seem not needed.
diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index 2291829270..6942b83969 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -75,6 +75,7 @@ # access to firmware's etc /usr/share/AAVMF/** r, /usr/share/bochs/** r, + /usr/share/edk2-ovmf/** r,
At least on Debian/Ubuntu the multiple edk2 related cases are already covered by
85342a3771b (Guido Günther 2014-04-07 12:15:02 +0200) /usr/share/ovmf/** r, f9803f59148 (Guido Günther 2017-07-06 11:04:21 +0200) /usr/share/OVMF/** r, f9803f59148 (Guido Günther 2017-07-06 11:04:21 +0200) /usr/share/AAVMF/** r, f9803f59148 (Guido Günther 2017-07-06 11:04:21 +0200) /usr/share/qemu-efi/** r,
/usr/share/kvm/** r, /usr/share/misc/sgabios.bin r, /usr/share/openbios/** r, @@ -86,6 +87,7 @@ /usr/share/qemu-kvm/** r, /usr/share/qemu/** r, /usr/share/seabios/** r, + /usr/share/sgabios/** r,
Again for Debian/Ubuntu this is already covered by: 987d1fdc535 (Guido Günther 2018-01-15 09:44:37 +0100) /usr/share/misc/sgabios.bin r,
I guess Suse would have mentioned if the paths would not have worked for them. Did you have another Distro which uses the paths that try to add here?
I'm not sure what you mean. The commit message says that Gentoo uses these paths. And indeed it does: # qlist sys-firmware/edk2-ovmf | grep OVMF_CODE.fd /usr/share/edk2-ovmf/OVMF_CODE.fd # qlist sys-firmware/sgabios /usr/share/sgabios/sgabios.bin Are you saying that we should remove some other, pre-existing paths from the file? Michal

At the beginning of each profile we have a comment that says when the profile was last updated. In theory, it makes sense because one can see immediately if they are using an outdated profile. However, we don't do a good job in keeping the comments in sync with reality and also sysadmins should rather use their package manager to find out libvirt version which installed the profiles. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/apparmor/libvirt-lxc | 2 -- src/security/apparmor/libvirt-qemu | 2 -- src/security/apparmor/usr.libexec.virt-aa-helper | 1 - src/security/apparmor/usr.sbin.libvirtd | 1 - 4 files changed, 6 deletions(-) diff --git a/src/security/apparmor/libvirt-lxc b/src/security/apparmor/libvirt-lxc index 4bfb503aa5..e556f2a7bd 100644 --- a/src/security/apparmor/libvirt-lxc +++ b/src/security/apparmor/libvirt-lxc @@ -1,5 +1,3 @@ -# Last Modified: Fri Feb 7 13:01:36 2014 - #include <abstractions/base> umount, diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index 6942b83969..80986aec61 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -1,5 +1,3 @@ -# Last Modified: Wed Sep 3 21:52:03 2014 - #include <abstractions/base> #include <abstractions/consoles> #include <abstractions/nameservice> diff --git a/src/security/apparmor/usr.libexec.virt-aa-helper b/src/security/apparmor/usr.libexec.virt-aa-helper index 72a2fecebe..e037ee7e26 100644 --- a/src/security/apparmor/usr.libexec.virt-aa-helper +++ b/src/security/apparmor/usr.libexec.virt-aa-helper @@ -1,4 +1,3 @@ -# Last Modified: Mon Apr 5 15:10:27 2010 #include <tunables/global> profile virt-aa-helper /usr/{lib,lib64,libexec}/libvirt/virt-aa-helper { diff --git a/src/security/apparmor/usr.sbin.libvirtd b/src/security/apparmor/usr.sbin.libvirtd index 27314b1512..a7bdf4d2fe 100644 --- a/src/security/apparmor/usr.sbin.libvirtd +++ b/src/security/apparmor/usr.sbin.libvirtd @@ -1,4 +1,3 @@ -# Last Modified: Mon Apr 5 15:03:58 2010 #include <tunables/global> @{LIBVIRT}="libvirt" -- 2.24.1

On Thu, Jan 30, 2020 at 8:05 AM Michal Privoznik <mprivozn@redhat.com> wrote:
At the beginning of each profile we have a comment that says when the profile was last updated. In theory, it makes sense because one can see immediately if they are using an outdated profile. However, we don't do a good job in keeping the comments in sync with reality and also sysadmins should rather use their package manager to find out libvirt version which installed the profiles.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
--- src/security/apparmor/libvirt-lxc | 2 -- src/security/apparmor/libvirt-qemu | 2 -- src/security/apparmor/usr.libexec.virt-aa-helper | 1 - src/security/apparmor/usr.sbin.libvirtd | 1 - 4 files changed, 6 deletions(-)
diff --git a/src/security/apparmor/libvirt-lxc b/src/security/apparmor/libvirt-lxc index 4bfb503aa5..e556f2a7bd 100644 --- a/src/security/apparmor/libvirt-lxc +++ b/src/security/apparmor/libvirt-lxc @@ -1,5 +1,3 @@ -# Last Modified: Fri Feb 7 13:01:36 2014 - #include <abstractions/base>
umount, diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index 6942b83969..80986aec61 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -1,5 +1,3 @@ -# Last Modified: Wed Sep 3 21:52:03 2014 - #include <abstractions/base> #include <abstractions/consoles> #include <abstractions/nameservice> diff --git a/src/security/apparmor/usr.libexec.virt-aa-helper b/src/security/apparmor/usr.libexec.virt-aa-helper index 72a2fecebe..e037ee7e26 100644 --- a/src/security/apparmor/usr.libexec.virt-aa-helper +++ b/src/security/apparmor/usr.libexec.virt-aa-helper @@ -1,4 +1,3 @@ -# Last Modified: Mon Apr 5 15:10:27 2010 #include <tunables/global>
profile virt-aa-helper /usr/{lib,lib64,libexec}/libvirt/virt-aa-helper { diff --git a/src/security/apparmor/usr.sbin.libvirtd b/src/security/apparmor/usr.sbin.libvirtd index 27314b1512..a7bdf4d2fe 100644 --- a/src/security/apparmor/usr.sbin.libvirtd +++ b/src/security/apparmor/usr.sbin.libvirtd @@ -1,4 +1,3 @@ -# Last Modified: Mon Apr 5 15:03:58 2010 #include <tunables/global> @{LIBVIRT}="libvirt"
-- 2.24.1
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd
participants (2)
-
Christian Ehrhardt
-
Michal Privoznik