These helper binaries are installed under libexec dir not lib dir. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- 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..504c70e0ce 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/libexec/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 1/27/20 5:30 PM, Jamie Strandboge wrote: > On Sat, 25 Jan 2020, Michal Privoznik wrote: > > > These helper binaries are installed under libexec dir not lib > > dir. > > > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > > --- > > 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..504c70e0ce 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/libexec/virt-aa-helper mr, > > I suggest you use this instead here and in the rest of the patch series: > > /usr/{lib,lib64,libexec}/libvirt/virt-aa-helper mr, > > since it will let existing installs to continue to work. You mean some downstream has installed virt-aa-helper into /usr/lib or /usr/lib64? Because the upstream install rule says /usr/libexec/.

On 1/29/20 11:53 AM, Jamie Strandboge wrote: > On Wed, 29 Jan 2020, Michal Privoznik wrote: > >> On 1/27/20 5:30 PM, Jamie Strandboge wrote: >>> On Sat, 25 Jan 2020, Michal Privoznik wrote: >>> >>>> These helper binaries are installed under libexec dir not lib >>>> dir. >>>> >>>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >>>> --- >>>> 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..504c70e0ce 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/libexec/virt-aa-helper mr, >>> >>> I suggest you use this instead here and in the rest of the patch series: >>> >>> /usr/{lib,lib64,libexec}/libvirt/virt-aa-helper mr, >>> >>> since it will let existing installs to continue to work. >> >> You mean some downstream has installed virt-aa-helper into /usr/lib or >> /usr/lib64? Because the upstream install rule says /usr/libexec/. > > Well, I was thinking the rule was what it was for a reason, so with my > distro hat on, changing it to just libexec sounded like a potential > pain point for upgraders. I also understand that the policy is intended > as example policy that distros can adjust as needed, so perhaps it is ok > to cut straight to libexec in this patchset... I don't have objections > if you prefer to keep it as is. With my downstream hat on I also prefer your suggestion. For better or worse, virt-aa-helper is in /usr/lib64/libvirt/ on SUSE distros.

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(a)redhat.com&gt; --- 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 8beb28655c..60d4352556 100644 --- a/docs/drvqemu.html.in +++ b/docs/drvqemu.html.in @@ -340,7 +340,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>).

The profile name should reflect the path under which the binary it describes is installed. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- 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 504c70e0ce..25754037e1 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/libexec/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

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(a)redhat.com&gt; --- 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)

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(a)redhat.com&gt; --- 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,