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>
---
 src/security/apparmor/libvirt-qemu          |   6 +
 src/security/apparmor/meson.build           |   1 +
 src/security/apparmor/usr.sbin.virtqemud.in | 135 ++++++++++++++++++++
 3 files changed, 142 insertions(+)

diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu
index 85c9e61d6c..990bb0b2ba 100644
--- a/src/security/apparmor/libvirt-qemu
+++ b/src/security/apparmor/libvirt-qemu
@@ -16,9 +16,13 @@
 
   ptrace (readby, tracedby) peer=libvirtd,
   ptrace (readby, tracedby) peer=/usr/sbin/libvirtd,
+  ptrace (readby, tracedby) peer=virtqemud,
+  ptrace (readby, tracedby) peer=/usr/sbin/virtqemud,
 
   signal (receive) peer=libvirtd,
   signal (receive) peer=/usr/sbin/libvirtd,
+  signal (receive) peer=virtqemud,
+  signal (receive) peer=/usr/sbin/virtqemud,
 
   /dev/kvm rw,
   /dev/net/tun rw,
@@ -221,6 +225,8 @@
   # 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),
+  unix (send, receive) type=stream addr=none peer=(label=/usr/sbin/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..b986241c74
--- /dev/null
+++ b/src/security/apparmor/usr.sbin.virtqemud.in
@@ -0,0 +1,135 @@
+#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 libvirtd
+   unix (send, receive) type=stream addr=none peer=(label=libvirtd),
+   signal (receive) set=("term") peer=/usr/sbin/libvirtd,
+   signal (receive) set=("term") peer=libvirtd,
+
+   /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,

[I'm not subscribed to the libvirt list, please CC me in replies]

Am Mittwoch, 16. Juni 2021, 05:41:02 CEST schrieb Jim Fehlig:
> diff --git a/src/security/apparmor/libvirt-qemu
> b/src/security/apparmor/libvirt-qemu index 85c9e61d6c..990bb0b2ba
> 100644
> --- a/src/security/apparmor/libvirt-qemu
> +++ b/src/security/apparmor/libvirt-qemu
[...]

You only need to add

> +  ptrace (readby, tracedby) peer=virtqemud,

The following rule

> +  ptrace (readby, tracedby) peer=/usr/sbin/virtqemud,

is superfluous and can be removed.

Technical background: the reason why there are rules for libvirtd and 
/usr/sbin/libvirtd is backwards compability to the old
  /usr/sbin/libvirtd {
profile before it became
  profile libvirtd /usr/sbin/libvirtd {

You don't need that for a new profile that is
  profile virtqumud /usr/sbin/virtquemud {
from the beginning.

This also applies to your 2/3 and 3/3 patches.

>    signal (receive) peer=libvirtd,
>    signal (receive) peer=/usr/sbin/libvirtd,
> +  signal (receive) peer=virtqemud,
> +  signal (receive) peer=/usr/sbin/virtqemud,

Same here - the rule with peer=/usr/sbin/virtquemud is superfluous.

[...]
> +  unix (send, receive) type=stream addr=none peer=(label=virtqemud), 
> +  unix (send, receive) type=stream addr=none peer=(label=/usr/sbin/
virtqemud),

And again ;-)

[...]
> diff --git a/src/security/apparmor/usr.sbin.virtqemud.in
> b/src/security/apparmor/usr.sbin.virtqemud.in new file mode 100644
> index 0000000000..b986241c74
> --- /dev/null
> +++ b/src/security/apparmor/usr.sbin.virtqemud.in
> @@ -0,0 +1,135 @@
> +#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,
[...]

Just wondering - do the new profiles (in all 3 patches) reallly need 
all the capabilities and the other broad rules?
(See my 0/3 reply how to find out ;-)


Regards,

Christian Boltz
-- 
Let's hope the best and praise the Gecko!
[Hans-Peter Jansen in opensuse-factory]

On 6/16/21 11:23 AM, Christian Boltz wrote:
> Hello,
> 
> [I'm not subscribed to the libvirt list, please CC me in replies]
> 
> Am Mittwoch, 16. Juni 2021, 05:41:02 CEST schrieb Jim Fehlig:
>> diff --git a/src/security/apparmor/libvirt-qemu
>> b/src/security/apparmor/libvirt-qemu index 85c9e61d6c..990bb0b2ba
>> 100644
>> --- a/src/security/apparmor/libvirt-qemu
>> +++ b/src/security/apparmor/libvirt-qemu
> [...]
> 
> You only need to add
> 
>> +  ptrace (readby, tracedby) peer=virtqemud,
> 
> The following rule
> 
>> +  ptrace (readby, tracedby) peer=/usr/sbin/virtqemud,
> 
> is superfluous and can be removed.
> 
> Technical background: the reason why there are rules for libvirtd and
> /usr/sbin/libvirtd is backwards compability to the old
>    /usr/sbin/libvirtd {
> profile before it became
>    profile libvirtd /usr/sbin/libvirtd {
> 
> You don't need that for a new profile that is
>    profile virtqumud /usr/sbin/virtquemud {
> from the beginning.

Understood.

> This also applies to your 2/3 and 3/3 patches.

Will fix in V2.

>>     signal (receive) peer=libvirtd,
>>     signal (receive) peer=/usr/sbin/libvirtd,
>> +  signal (receive) peer=virtqemud,
>> +  signal (receive) peer=/usr/sbin/virtqemud,
> 
> Same here - the rule with peer=/usr/sbin/virtquemud is superfluous.
> 
> [...]
>> +  unix (send, receive) type=stream addr=none peer=(label=virtqemud),
>> +  unix (send, receive) type=stream addr=none peer=(label=/usr/sbin/
> virtqemud),
> 
> And again ;-)
> 
> [...]
>> diff --git a/src/security/apparmor/usr.sbin.virtqemud.in
>> b/src/security/apparmor/usr.sbin.virtqemud.in new file mode 100644
>> index 0000000000..b986241c74
>> --- /dev/null
>> +++ b/src/security/apparmor/usr.sbin.virtqemud.in
>> @@ -0,0 +1,135 @@
>> +#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,
> [...]
> 
> Just wondering - do the new profiles (in all 3 patches) reallly need
> all the capabilities and the other broad rules?

I'll try to figure that out before posting V2

> (See my 0/3 reply how to find out ;-)

... using your tips!

Regards,
Jim