[PATCH V3 0/2] Apparmor: Add profiles for hypervisor daemons

V2: https://listman.redhat.com/archives/libvir-list/2021-June/msg00676.html V1: https://listman.redhat.com/archives/libvir-list/2021-June/msg00456.html Changes since V2: Patches 3 and 4 ACKed and pushed since they are bug fixes independent of modular vs monolithic daemons. The qemu_bridge_helper subprofile in patch 1 was adjusted for communication with virtqemud instead of libvirtd. After snooping through git history, I found a few capabilities explicitly added for xen that have been added back to the virtxend profile. Note: The profile for virtlxcd will have to wait until the following issue is fixed https://gitlab.com/libvirt/libvirt/-/issues/181 Jim Fehlig (2): Apparmor: Add profile for virtqemud Apparmor: Add profile for virtxend src/security/apparmor/libvirt-qemu | 3 + src/security/apparmor/meson.build | 2 + src/security/apparmor/usr.sbin.virtqemud.in | 134 ++++++++++++++++++++ src/security/apparmor/usr.sbin.virtxend.in | 55 ++++++++ 4 files changed, 194 insertions(+) create mode 100644 src/security/apparmor/usr.sbin.virtqemud.in create mode 100644 src/security/apparmor/usr.sbin.virtxend.in -- 2.31.1

A new apparmor profile derived from the libvirtd profile, with non-QEMU related rules removed. Adopt the libvirt-qemu abstraction to work with the new profile. Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- I pursued a suggestion to use qemu's `make check-acceptance` as an additional driver of qemu for verifying if all the host capabilities in virtqemud profile are actually needed. That allowed me to shrink the list of unverified capabilities to net_raw, sys_chroot, sys_ptrace, setpcap, and sys_pacct. All but the last were added by the original libvirtd profile commit 624a7927f07. sys_pacct was added in 2015 by commit b61fb8e8af1. The capability was needed for xen, but from the description sounds like it could be applicable to qemu as well "Allow CAP_SYS_PACCT, which is required when resetting some multi-port Broadcom cards by writting to the PCI config space" As for the others, I'm hesitant to remove them even though I've been unable to verify the capabilities are required. The paranoid side of me says to keep them to avoid some unforeseen breakage. src/security/apparmor/libvirt-qemu | 3 + src/security/apparmor/meson.build | 1 + src/security/apparmor/usr.sbin.virtqemud.in | 134 ++++++++++++++++++++ 3 files changed, 138 insertions(+) diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index 6275b6e95b..4156428163 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -16,9 +16,11 @@ ptrace (readby, tracedby) peer=libvirtd, ptrace (readby, tracedby) peer=/usr/sbin/libvirtd, + ptrace (readby, tracedby) peer=virtqemud, signal (receive) peer=libvirtd, signal (receive) peer=/usr/sbin/libvirtd, + signal (receive) peer=virtqemud, /dev/kvm rw, /dev/net/tun rw, @@ -223,6 +225,7 @@ # allow connect with openGraphicsFD to work unix (send, receive) type=stream addr=none peer=(label=libvirtd), unix (send, receive) type=stream addr=none peer=(label=/usr/sbin/libvirtd), + unix (send, receive) type=stream addr=none peer=(label=virtqemud), # for gathering information about available host resources /sys/devices/system/cpu/ r, diff --git a/src/security/apparmor/meson.build b/src/security/apparmor/meson.build index af43780211..56f308bf3a 100644 --- a/src/security/apparmor/meson.build +++ b/src/security/apparmor/meson.build @@ -1,6 +1,7 @@ apparmor_gen_profiles = [ 'usr.lib.libvirt.virt-aa-helper', 'usr.sbin.libvirtd', + 'usr.sbin.virtqemud', ] apparmor_gen_profiles_conf = configuration_data() diff --git a/src/security/apparmor/usr.sbin.virtqemud.in b/src/security/apparmor/usr.sbin.virtqemud.in new file mode 100644 index 0000000000..2d16ea821d --- /dev/null +++ b/src/security/apparmor/usr.sbin.virtqemud.in @@ -0,0 +1,134 @@ +#include <tunables/global> +@{LIBVIRT}="libvirt" + +profile virtqemud @sbindir@/virtqemud flags=(attach_disconnected) { + #include <abstractions/base> + #include <abstractions/dbus> + + capability kill, + capability net_admin, + capability net_raw, + capability setgid, + capability sys_admin, + capability sys_module, + capability sys_ptrace, + capability sys_pacct, + capability sys_nice, + capability sys_chroot, + capability setuid, + capability dac_override, + capability dac_read_search, + capability fowner, + capability chown, + capability setpcap, + capability mknod, + capability fsetid, + capability audit_write, + capability ipc_lock, + capability sys_rawio, + capability bpf, + capability perfmon, + + # Needed for vfio + capability sys_resource, + + mount options=(rw,rslave) -> /, + mount options=(rw, nosuid) -> /{var/,}run/libvirt/qemu/*.dev/, + umount /{var/,}run/libvirt/qemu/*.dev/, + + # libvirt provides any mounts under /dev to qemu namespaces + mount options=(rw, move) /dev/ -> /{,var/}run/libvirt/qemu/*.dev/, + mount options=(rw, move) /dev/** -> /{,var/}run/libvirt/qemu/*{,/}, + mount options=(rw, move) /{,var/}run/libvirt/qemu/*.dev/ -> /dev/, + mount options=(rw, move) /{,var/}run/libvirt/qemu/*{,/} -> /dev/**, + + network inet stream, + network inet dgram, + network inet6 stream, + network inet6 dgram, + network netlink raw, + network packet dgram, + network packet raw, + + # for --p2p migrations + unix (send, receive) type=stream addr=none peer=(label=unconfined addr=none), + + ptrace (read,trace) peer=unconfined, + ptrace (read,trace) peer=@{profile_name}, + ptrace (read,trace) peer=dnsmasq, + ptrace (read,trace) peer=/usr/sbin/dnsmasq, + ptrace (read,trace) peer=libvirt-*, + + signal (send) peer=dnsmasq, + signal (send) peer=/usr/sbin/dnsmasq, + signal (read, send) peer=libvirt-*, + signal (send) set=("kill", "term") peer=unconfined, + + # For communication/control to qemu-bridge-helper + unix (send, receive) type=stream addr=none peer=(label=libvirtd//qemu_bridge_helper), + signal (send) set=("term") peer=libvirtd//qemu_bridge_helper, + + # allow connect with openGraphicsFD, direction reversed in newer versions + unix (send, receive) type=stream addr=none peer=(label=libvirt-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*), + # unconfined also required if guests run without security module + unix (send, receive) type=stream addr=none peer=(label=unconfined), + + # required if guests run unconfined seclabel type='none' but libvirtd is confined + signal (read, send) peer=unconfined, + + # Very lenient profile for libvirtd since we want to first focus on confining + # the guests. Guests will have a very restricted profile. + / r, + /** rwmkl, + + /bin/* PUx, + /sbin/* PUx, + /usr/bin/* PUx, + @sbindir@/virtlogd pix, + @sbindir@/* PUx, + /{usr/,}lib/udev/scsi_id PUx, + /usr/{lib,lib64,lib/qemu,libexec}/vhost-user-gpu PUx, + /usr/{lib,lib64,lib/qemu,libexec}/virtiofsd PUx, + + # Required by nwfilter_ebiptables_driver.c:ebiptablesWriteToTempFile() to + # read and run an ebtables script. + /var/lib/libvirt/virtd* ixr, + + # force the use of virt-aa-helper + audit deny /{usr/,}sbin/apparmor_parser rwxl, + audit deny /etc/apparmor.d/libvirt/** wxl, + audit deny /sys/kernel/security/apparmor/features rwxl, + audit deny /sys/kernel/security/apparmor/matching rwxl, + audit deny /sys/kernel/security/apparmor/.* rwxl, + /sys/kernel/security/apparmor/profiles r, + @libexecdir@/* PUxr, + @libexecdir@/libvirt_parthelper ix, + @libexecdir@/libvirt_iohelper ix, + /etc/libvirt/hooks/** rmix, + + # allow changing to our UUID-based named profiles + change_profile -> @{LIBVIRT}-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*, + + /usr/{lib,lib64,lib/qemu,libexec}/qemu-bridge-helper Cx -> qemu_bridge_helper, + # child profile for bridge helper process + profile qemu_bridge_helper { + #include <abstractions/base> + + capability setuid, + capability setgid, + capability setpcap, + capability net_admin, + + network inet stream, + + # For communication/control from virtqemud + unix (send, receive) type=stream addr=none peer=(label=virtqemud), + signal (receive) set=("term") peer=virtqemud, + + /dev/net/tun rw, + /etc/qemu/** r, + owner @{PROC}/*/status r, + + /usr/{lib,lib64,lib/qemu,libexec}/qemu-bridge-helper rmix, + } +} -- 2.31.1

Hello, [please CC me in replies] Your updated patches still look good, I just noticed something that is probably minor nitpicking: Am Donnerstag, 24. Juni 2021, 22:48:58 CEST schrieb Jim Fehlig: [...]
+ signal (send) set=("kill", "term") peer=unconfined, [...] + signal (send) set=("term") peer=libvirtd//qemu_bridge_helper,
The quotes around the signal names are superfluous. You can simply use set=(kill, term) set=(term) Actually the parenthesis are optional if there's only a single signal mentioned, so you could also use set=term (keeping the parenthesis for consistency with other rules is also fine) There are several signal rules with superfluous quotes in this patch, and also one in the 2/2 patch. (There's no need to re-send the patch for such a minor change IMHO.) Regards, Christian Boltz -- I'm root - if you see me laughing you better have a backup!

On 6/25/21 5:19 AM, Christian Boltz wrote:
Hello,
[please CC me in replies]
Your updated patches still look good, I just noticed something that is probably minor nitpicking:
Am Donnerstag, 24. Juni 2021, 22:48:58 CEST schrieb Jim Fehlig: [...]
+ signal (send) set=("kill", "term") peer=unconfined, [...] + signal (send) set=("term") peer=libvirtd//qemu_bridge_helper,
The quotes around the signal names are superfluous. You can simply use set=(kill, term) set=(term)
Actually the parenthesis are optional if there's only a single signal mentioned, so you could also use set=term (keeping the parenthesis for consistency with other rules is also fine)
There are several signal rules with superfluous quotes in this patch, and also one in the 2/2 patch.
(There's no need to re-send the patch for such a minor change IMHO.)
Thanks. I've squashed the below diff into my local branch (along with a similar change to the one instance in 2/2). Regards, Jim diff --git a/src/security/apparmor/usr.sbin.virtqemud.in b/src/security/apparmor/usr.sbin.virtqemud.in index 2d16ea821d..3de03d49fc 100644 --- a/src/security/apparmor/usr.sbin.virtqemud.in +++ b/src/security/apparmor/usr.sbin.virtqemud.in @@ -62,11 +62,11 @@ profile virtqemud @sbindir@/virtqemud flags=(attach_disconnected) { signal (send) peer=dnsmasq, signal (send) peer=/usr/sbin/dnsmasq, signal (read, send) peer=libvirt-*, - signal (send) set=("kill", "term") peer=unconfined, + signal (send) set=(kill, term) peer=unconfined, # For communication/control to qemu-bridge-helper unix (send, receive) type=stream addr=none peer=(label=libvirtd//qemu_bridge_helper), - signal (send) set=("term") peer=libvirtd//qemu_bridge_helper, + signal (send) set=(term) peer=libvirtd//qemu_bridge_helper, # allow connect with openGraphicsFD, direction reversed in newer versions unix (send, receive) type=stream addr=none peer=(label=libvirt-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*), @@ -123,7 +123,7 @@ profile virtqemud @sbindir@/virtqemud flags=(attach_disconnected) { # For communication/control from virtqemud unix (send, receive) type=stream addr=none peer=(label=virtqemud), - signal (receive) set=("term") peer=virtqemud, + signal (receive) set=(term) peer=virtqemud, /dev/net/tun rw, /etc/qemu/** r,

A new apparmor profile initially derived from the libvirtd profile. All rules were prefixed with the 'audit' qualifier to verify they are actually used by virtxend. It turns out that several, beyond the obvious ones, can be dropped in the resulting virtxend profile. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V3: Added back a few more capabilities to the virtxend profile after checking git history. src/security/apparmor/meson.build | 1 + src/security/apparmor/usr.sbin.virtxend.in | 55 ++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/src/security/apparmor/meson.build b/src/security/apparmor/meson.build index 56f308bf3a..990f00b4f3 100644 --- a/src/security/apparmor/meson.build +++ b/src/security/apparmor/meson.build @@ -2,6 +2,7 @@ apparmor_gen_profiles = [ 'usr.lib.libvirt.virt-aa-helper', 'usr.sbin.libvirtd', 'usr.sbin.virtqemud', + 'usr.sbin.virtxend', ] apparmor_gen_profiles_conf = configuration_data() diff --git a/src/security/apparmor/usr.sbin.virtxend.in b/src/security/apparmor/usr.sbin.virtxend.in new file mode 100644 index 0000000000..0f6b825f47 --- /dev/null +++ b/src/security/apparmor/usr.sbin.virtxend.in @@ -0,0 +1,55 @@ +#include <tunables/global> + +profile virtxend @sbindir@/virtxend flags=(attach_disconnected) { + #include <abstractions/base> + #include <abstractions/dbus> + + capability kill, + capability setgid, + capability setuid, + capability sys_pacct, + capability ipc_lock, + + network inet stream, + network inet dgram, + network inet6 stream, + network inet6 dgram, + network netlink raw, + network packet dgram, + network packet raw, + + # for --p2p migrations + unix (send, receive) type=stream addr=none peer=(label=unconfined addr=none), + + ptrace (read,trace) peer=unconfined, + + signal (send) set=("kill", "term", "hup") peer=unconfined, + + # Very lenient profile for virtxend + / r, + /** rwmkl, + + /bin/* PUx, + /sbin/* PUx, + /usr/bin/* PUx, + @sbindir@/virtlogd pix, + @sbindir@/* PUx, + /{usr/,}lib/udev/scsi_id PUx, + /usr/{lib,lib64}/xen-common/bin/xen-toolstack PUx, + /usr/{lib,lib64}/xen/bin/* Ux, + /usr/{lib,libexec}/xen-*/bin/libxl-save-helper PUx, + /usr/{lib,libexec}/xen-*/bin/pygrub PUx, + + # force the use of virt-aa-helper + audit deny /{usr/,}sbin/apparmor_parser rwxl, + audit deny /etc/apparmor.d/libvirt/** wxl, + audit deny /sys/kernel/security/apparmor/features rwxl, + audit deny /sys/kernel/security/apparmor/matching rwxl, + audit deny /sys/kernel/security/apparmor/.* rwxl, + /sys/kernel/security/apparmor/profiles r, + @libexecdir@/* PUxr, + @libexecdir@/libvirt_parthelper ix, + @libexecdir@/libvirt_iohelper ix, + /etc/libvirt/hooks/** rmix, + /etc/xen/scripts/** rmix, +} -- 2.31.1

Hi All, Is it fine to push these patches now that the release is out? Christian E. has ACKed 1/2 and Christian B. has 'LGTM' both. Anyone brave enough to ACK 2/2, or have further comments? I'd like to get these in since modular daemons are now prime-time. Regards, Jim On 6/24/21 2:48 PM, Jim Fehlig wrote:
V2: https://listman.redhat.com/archives/libvir-list/2021-June/msg00676.html V1: https://listman.redhat.com/archives/libvir-list/2021-June/msg00456.html
Changes since V2: Patches 3 and 4 ACKed and pushed since they are bug fixes independent of modular vs monolithic daemons.
The qemu_bridge_helper subprofile in patch 1 was adjusted for communication with virtqemud instead of libvirtd.
After snooping through git history, I found a few capabilities explicitly added for xen that have been added back to the virtxend profile.
Note: The profile for virtlxcd will have to wait until the following issue is fixed
https://gitlab.com/libvirt/libvirt/-/issues/181
Jim Fehlig (2): Apparmor: Add profile for virtqemud Apparmor: Add profile for virtxend
src/security/apparmor/libvirt-qemu | 3 + src/security/apparmor/meson.build | 2 + src/security/apparmor/usr.sbin.virtqemud.in | 134 ++++++++++++++++++++ src/security/apparmor/usr.sbin.virtxend.in | 55 ++++++++ 4 files changed, 194 insertions(+) create mode 100644 src/security/apparmor/usr.sbin.virtqemud.in create mode 100644 src/security/apparmor/usr.sbin.virtxend.in

On 7/6/21 4:37 PM, Jim Fehlig wrote:
Hi All,
Is it fine to push these patches now that the release is out? Christian E. has ACKed 1/2 and Christian B. has 'LGTM' both. Anyone brave enough to ACK 2/2, or have further comments? I'd like to get these in since modular daemons are now prime-time.
Yes, it is fine to push these. Michal

On Thu, Jun 24, 2021 at 4:49 PM Jim Fehlig <jfehlig@suse.com> wrote:
V2: https://listman.redhat.com/archives/libvir-list/2021-June/msg00676.html V1: https://listman.redhat.com/archives/libvir-list/2021-June/msg00456.html
Changes since V2: Patches 3 and 4 ACKed and pushed since they are bug fixes independent of modular vs monolithic daemons.
The qemu_bridge_helper subprofile in patch 1 was adjusted for communication with virtqemud instead of libvirtd.
After snooping through git history, I found a few capabilities explicitly added for xen that have been added back to the virtxend profile.
Note: The profile for virtlxcd will have to wait until the following issue is fixed
https://gitlab.com/libvirt/libvirt/-/issues/181
Jim Fehlig (2): Apparmor: Add profile for virtqemud Apparmor: Add profile for virtxend
src/security/apparmor/libvirt-qemu | 3 + src/security/apparmor/meson.build | 2 + src/security/apparmor/usr.sbin.virtqemud.in | 134 ++++++++++++++++++++ src/security/apparmor/usr.sbin.virtxend.in | 55 ++++++++ 4 files changed, 194 insertions(+) create mode 100644 src/security/apparmor/usr.sbin.virtqemud.in create mode 100644 src/security/apparmor/usr.sbin.virtxend.in
-- 2.31.1
Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ!/ Always, there's only one truth!
participants (4)
-
Christian Boltz
-
Jim Fehlig
-
Michal Prívozník
-
Neal Gompa