6:27 p.m.
and other improvements. V2 of
https://listman.redhat.com/archives/libvir-list/2021-June/msg00456.html
Changes since V1:
Removed many unneeded capabilities. I used the 'audit' qualifier as suggested
by cboltz to verify which capabilities were actually used. It's a difficult
task though, as it is nearly impossible for one person to exercise a driver
in all the ways thousands of users will push it :-). I was able to whittle
the virtxend profile quite a bit since xen doesn't need a lot in the way of
host capabilities.
Removed patch containing the virtlxcd profile since I'm unable to start any
lxc domains with virtlxcd.
Added patches to squelch denial messages from the virt-aa-helper profile.
Jim Fehlig (4):
Apparmor: Add profile for virtqemud
Apparmor: Add profile for virtxend
Apparmor: Allow reading libnl's classid file
Apparmor: Allow reading /etc/ssl/openssl.cnf
src/security/apparmor/libvirt-qemu | 5 +
src/security/apparmor/meson.build | 2 +
.../usr.lib.libvirt.virt-aa-helper.in | 4 +-
src/security/apparmor/usr.sbin.virtqemud.in | 135 ++++++++++++++++++
src/security/apparmor/usr.sbin.virtxend.in | 53 +++++++
5 files changed, 198 insertions(+), 1 deletion(-)
create mode 100644 src/security/apparmor/usr.sbin.virtqemud.in
create mode 100644 src/security/apparmor/usr.sbin.virtxend.in
--
2.31.1
6:27 p.m.
New subject: [PATCH V2 1/4] Apparmor: Add profile for virtqemud
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(a)suse.com>
---
src/security/apparmor/libvirt-qemu | 3 +
src/security/apparmor/meson.build | 1 +
src/security/apparmor/usr.sbin.virtqemud.in | 135 ++++++++++++++++++++
3 files changed, 139 insertions(+)
diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu
index 85c9e61d6c..3e31ed4981 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,
@@ -221,6 +223,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..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
12:43 a.m.
New subject: [PATCH V2 1/4] Apparmor: Add profile for virtqemud
On Wed, Jun 23, 2021 at 1:27 AM Jim Fehlig <jfehlig(a)suse.com> wrote:
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(a)suse.com>
Thanks for your work on this, but since in the split daemon mode
virtqemud will do the
majority of the tasks I wonder if along this change (or later) we
should consider
removing rules from the libvirtd profile.
It should now have less tasks and therefore need less permissions.
Have you had the chance to take a look into that already?
There is a bonus-problem though, as long as we provide the option to build
non-split daemons we would effectively need two profiles.
One for the monolithic libvirtd and a reduced one for the split kind.
This shall not stall this patch series, it is just a related thought
that I wanted
to bring to your attention.
---
src/security/apparmor/libvirt-qemu | 3 +
src/security/apparmor/meson.build | 1 +
src/security/apparmor/usr.sbin.virtqemud.in | 135 ++++++++++++++++++++
3 files changed, 139 insertions(+)
On the patch itself while I had no chance to test it myself it looks
exactly as I'd have expected a virtqemud profile.
Reviewed-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu
index 85c9e61d6c..3e31ed4981 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,
@@ -221,6 +223,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..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
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
9:24 a.m.
New subject: [PATCH V2 1/4] Apparmor: Add profile for virtqemud
On 6/23/21 11:43 PM, Christian Ehrhardt wrote:
On Wed, Jun 23, 2021 at 1:27 AM Jim Fehlig <jfehlig(a)suse.com>
wrote:
>
> 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(a)suse.com>
Thanks for your work on this, but since in the split daemon mode
virtqemud will do the
majority of the tasks I wonder if along this change (or later) we
should consider
removing rules from the libvirtd profile.
AFAIK (at least in theory), the modular and monolithic daemons are mutually
exclusive. Either you run the desired modular daemon(s) or the monolithic
libvirtd. So the libvirtd rules need to stay IMO.
And IIRC, Daniel has long-term plans to remove the monolithic daemon, at which
point the libvirtd profile can be dropped too.
It should now have less tasks and therefore need less permissions.
Have you had the chance to take a look into that already?
There is a bonus-problem though, as long as we provide the option to build
non-split daemons we would effectively need two profiles.
One for the monolithic libvirtd and a reduced one for the split kind.
Agreed. We'll need both as long as we have the modular and monolithic daemons.
This shall not stall this patch series, it is just a related thought
that I wanted
to bring to your attention.
> ---
> src/security/apparmor/libvirt-qemu | 3 +
> src/security/apparmor/meson.build | 1 +
> src/security/apparmor/usr.sbin.virtqemud.in | 135 ++++++++++++++++++++
> 3 files changed, 139 insertions(+)
On the patch itself while I had no chance to test it myself it looks
exactly as I'd have expected a virtqemud profile.
Reviewed-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
Thanks! I did notice another problem while doing more testing yesterday. See below.
> diff --git a/src/security/apparmor/libvirt-qemu
b/src/security/apparmor/libvirt-qemu
> index 85c9e61d6c..3e31ed4981 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,
> @@ -221,6 +223,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..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),
s/libvirtd/virtqemud/
> + signal (receive) set=("term")
peer=/usr/sbin/libvirtd,
Not needed.
> + signal (receive) set=("term") peer=libvirtd,
s/libvirtd/virtqemud/
I'll include these changes in V3.
Regards,
Jim
> +
> + /dev/net/tun rw,
> + /etc/qemu/** r,
> + owner @{PROC}/*/status r,
> +
> + /usr/{lib,lib64,lib/qemu,libexec}/qemu-bridge-helper rmix,
> + }
> +}
> --
> 2.31.1
>
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
9:42 a.m.
New subject: [PATCH V2 1/4] Apparmor: Add profile for virtqemud
On Thu, Jun 24, 2021 at 08:24:05AM -0600, Jim Fehlig wrote:
On 6/23/21 11:43 PM, Christian Ehrhardt wrote:
> On Wed, Jun 23, 2021 at 1:27 AM Jim Fehlig <jfehlig(a)suse.com> wrote:
> >
> > 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(a)suse.com>
>
> Thanks for your work on this, but since in the split daemon mode
> virtqemud will do the
> majority of the tasks I wonder if along this change (or later) we
> should consider
> removing rules from the libvirtd profile.
AFAIK (at least in theory), the modular and monolithic daemons are mutually
exclusive. Either you run the desired modular daemon(s) or the monolithic
libvirtd. So the libvirtd rules need to stay IMO.
And IIRC, Daniel has long-term plans to remove the monolithic daemon, at
which point the libvirtd profile can be dropped too.
> It should now have less tasks and therefore need less permissions.
> Have you had the chance to take a look into that already?
>
> There is a bonus-problem though, as long as we provide the option to build
> non-split daemons we would effectively need two profiles.
> One for the monolithic libvirtd and a reduced one for the split kind.
Agreed. We'll need both as long as we have the modular and monolithic daemons.
FWIW, I when making the Fedora feature proposal[1] I stated that we intend
to keep the monolithic libvirtd upstream for /at least/ 1 year, starting
from when a major Linux distro has a release that defaults to the modular
daemons. So that's going to be at least late 2022 before we talk about
deleting libvirtd.
Regards,
Daniel
[1] https://fedoraproject.org/wiki/Changes/LibvirtModularDaemons
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
6:27 p.m.
New subject: [PATCH V2 2/4] Apparmor: Add profile for virtxend
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(a)suse.com>
---
src/security/apparmor/meson.build | 1 +
src/security/apparmor/usr.sbin.virtxend.in | 53 ++++++++++++++++++++++
2 files changed, 54 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..37c31bb104
--- /dev/null
+++ b/src/security/apparmor/usr.sbin.virtxend.in
@@ -0,0 +1,53 @@
+#include <tunables/global>
+
+profile virtxend @sbindir@/virtxend flags=(attach_disconnected) {
+ #include <abstractions/base>
+ #include <abstractions/dbus>
+
+ capability kill,
+ capability setgid,
+ capability setuid,
+
+ 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
6:27 p.m.
New subject: [PATCH V2 3/4] Apparmor: Allow reading libnl's classid file
I noticed the following denial messages from apparmor in audit.log when
starting confined VMs via the QEMU driver
type=AVC msg=audit(1623864006.370:837): apparmor="DENIED"
operation="open" \
profile="virt-aa-helper" name="/etc/libnl/classid" pid=11265 \
comm="virt-aa-helper" requested_mask="r" denied_mask="r"
fsuid=0 ouid=0
type=AVC msg=audit(1623864006.582:849): apparmor="DENIED"
operation="open" \
profile="libvirt-0ca2720d-6cff-48bb-86c2-61ab9a79b6e9" \
name="/etc/libnl/classid" pid=11270 comm="qemu-system-x86" \
requested_mask="r" denied_mask="r" fsuid=107 ouid=0
It is possible for site admins to assign names to classids in this file,
which are then used by all libnl tools, possibly those used by libvirt.
To be on the safe side, allow read access to the file in the virt-aa-helper
profile and the libvirt-qemu abstraction.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/security/apparmor/libvirt-qemu | 2 ++
src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in | 3 ++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu
index 3e31ed4981..4156428163 100644
--- a/src/security/apparmor/libvirt-qemu
+++ b/src/security/apparmor/libvirt-qemu
@@ -37,6 +37,8 @@
@{PROC}/sys/vm/overcommit_memory r,
# detect hardware capabilities via qemu_getauxval
owner @{PROC}/*/auxv r,
+ # allow reading libnl's classid file
+ /etc/libnl{,-3}/classid r,
# For hostdev access. The actual devices will be added dynamically
/sys/bus/usb/devices/ r,
diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
index dd18c8ab89..8ebb47596a 100644
--- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
+++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
@@ -19,7 +19,8 @@ profile virt-aa-helper @libexecdir@/virt-aa-helper {
# Used when internally running another command (namely apparmor_parser)
@{PROC}/@{pid}/fd/ r,
- @sysconfdir@/libnl-3/classid r,
+ # allow reading libnl's classid file
+ @sysconfdir@/libnl{,-3}/classid r,
# for gl enabled graphics
/dev/dri/{,*} r,
--
2.31.1
12:43 a.m.
New subject: [PATCH V2 3/4] Apparmor: Allow reading libnl's classid file
On Wed, Jun 23, 2021 at 1:28 AM Jim Fehlig <jfehlig(a)suse.com> wrote:
I noticed the following denial messages from apparmor in audit.log when
starting confined VMs via the QEMU driver
type=AVC msg=audit(1623864006.370:837): apparmor="DENIED"
operation="open" \
profile="virt-aa-helper" name="/etc/libnl/classid" pid=11265 \
comm="virt-aa-helper" requested_mask="r" denied_mask="r"
fsuid=0 ouid=0
type=AVC msg=audit(1623864006.582:849): apparmor="DENIED"
operation="open" \
profile="libvirt-0ca2720d-6cff-48bb-86c2-61ab9a79b6e9" \
name="/etc/libnl/classid" pid=11270 comm="qemu-system-x86" \
requested_mask="r" denied_mask="r" fsuid=107 ouid=0
It is possible for site admins to assign names to classids in this file,
which are then used by all libnl tools, possibly those used by libvirt.
To be on the safe side, allow read access to the file in the virt-aa-helper
profile and the libvirt-qemu abstraction.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
While this particular rule would be covered in
abstractions/nameservice that would allow much more.
I agree if we really only need libnl and nothing else then
adapting/adding the existing rule should be better.
Reviewed-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
---
src/security/apparmor/libvirt-qemu | 2 ++
src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in | 3 ++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu
index 3e31ed4981..4156428163 100644
--- a/src/security/apparmor/libvirt-qemu
+++ b/src/security/apparmor/libvirt-qemu
@@ -37,6 +37,8 @@
@{PROC}/sys/vm/overcommit_memory r,
# detect hardware capabilities via qemu_getauxval
owner @{PROC}/*/auxv r,
+ # allow reading libnl's classid file
+ /etc/libnl{,-3}/classid r,
# For hostdev access. The actual devices will be added dynamically
/sys/bus/usb/devices/ r,
diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
index dd18c8ab89..8ebb47596a 100644
--- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
+++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
@@ -19,7 +19,8 @@ profile virt-aa-helper @libexecdir@/virt-aa-helper {
# Used when internally running another command (namely apparmor_parser)
@{PROC}/@{pid}/fd/ r,
- @sysconfdir@/libnl-3/classid r,
+ # allow reading libnl's classid file
+ @sysconfdir@/libnl{,-3}/classid r,
# for gl enabled graphics
/dev/dri/{,*} r,
--
2.31.1
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
5:51 p.m.
New subject: [PATCH V2 3/4] Apparmor: Allow reading libnl's classid file
On 6/23/21 11:43 PM, Christian Ehrhardt wrote:
On Wed, Jun 23, 2021 at 1:28 AM Jim Fehlig <jfehlig(a)suse.com>
wrote:
>
> I noticed the following denial messages from apparmor in audit.log when
> starting confined VMs via the QEMU driver
>
> type=AVC msg=audit(1623864006.370:837): apparmor="DENIED"
operation="open" \
> profile="virt-aa-helper" name="/etc/libnl/classid" pid=11265 \
> comm="virt-aa-helper" requested_mask="r"
denied_mask="r" fsuid=0 ouid=0
>
> type=AVC msg=audit(1623864006.582:849): apparmor="DENIED"
operation="open" \
> profile="libvirt-0ca2720d-6cff-48bb-86c2-61ab9a79b6e9" \
> name="/etc/libnl/classid" pid=11270 comm="qemu-system-x86" \
> requested_mask="r" denied_mask="r" fsuid=107 ouid=0
>
> It is possible for site admins to assign names to classids in this file,
> which are then used by all libnl tools, possibly those used by libvirt.
> To be on the safe side, allow read access to the file in the virt-aa-helper
> profile and the libvirt-qemu abstraction.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
While this particular rule would be covered in
abstractions/nameservice that would allow much more.
Christian B. mentioned that in V1, and also discouraged its use for the single file.
I agree if we really only need libnl and nothing else then
adapting/adding the existing rule should be better.
Reviewed-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
Thanks! I've pushed 3 and 4, and after making a few more tweaks sent a V3 of the
others.
Regards,
Jim
6:27 p.m.
New subject: [PATCH V2 4/4] Apparmor: Allow reading /etc/ssl/openssl.cnf
I noticed the following denial when running confined VMs with the QEMU
driver
type=AVC msg=audit(1623865089.263:865): apparmor="DENIED"
operation="open" \
profile="virt-aa-helper" name="/etc/ssl/openssl.cnf" pid=12503 \
comm="virt-aa-helper" requested_mask="r" denied_mask="r"
fsuid=0 ouid=0
Allow reading the file by including the openssl abstraction in the
virt-aa-helper profile.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
index 8ebb47596a..ff1d46bebe 100644
--- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
+++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
@@ -2,6 +2,7 @@
profile virt-aa-helper @libexecdir@/virt-aa-helper {
#include <abstractions/base>
+ #include <abstractions/openssl>
# needed for searching directories
capability dac_override,
--
2.31.1
12:43 a.m.
New subject: [PATCH V2 4/4] Apparmor: Allow reading /etc/ssl/openssl.cnf
On Wed, Jun 23, 2021 at 1:28 AM Jim Fehlig <jfehlig(a)suse.com> wrote:
I noticed the following denial when running confined VMs with the QEMU
driver
type=AVC msg=audit(1623865089.263:865): apparmor="DENIED"
operation="open" \
profile="virt-aa-helper" name="/etc/ssl/openssl.cnf" pid=12503 \
comm="virt-aa-helper" requested_mask="r" denied_mask="r"
fsuid=0 ouid=0
Allow reading the file by including the openssl abstraction in the
virt-aa-helper profile.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
While I don't immediately see which configuration makes virt-aa-helper
need openssl it is an abstraction that isn't allowing a lot, so IMHO
that should be ok to add.
Reviewed-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
---
src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
index 8ebb47596a..ff1d46bebe 100644
--- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
+++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
@@ -2,6 +2,7 @@
profile virt-aa-helper @libexecdir@/virt-aa-helper {
#include <abstractions/base>
+ #include <abstractions/openssl>
# needed for searching directories
capability dac_override,
--
2.31.1
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
2:17 p.m.
Hello,
[I'm not subscribed to libvir-list - please CC me in replies]
Am Mittwoch, 23. Juni 2021, 01:27:43 CEST schrieb Jim Fehlig:
and other improvements. V2 of
https://listman.redhat.com/archives/libvir-list/2021-June/msg00456.htm
Changes since V1:
Removed many unneeded capabilities. I used the 'audit' qualifier as
suggested by cboltz to verify which capabilities were actually used.
It's a difficult task though, as it is nearly impossible for one
person to exercise a driver in all the ways thousands of users will
push it :-). I was able to whittle the virtxend profile quite a bit
since xen doesn't need a lot in the way of host capabilities.
Your updated patches look good :-)
There's one thing I missed in the first review, but that might be worth
a separate patch instead of updating this patchset:
Starting with AppArmor 3.0 userspace, profiles should have an abi rule
in their preamble (as the first line):
abi <abi/3.0>,
Without this abi rule, network, dbus and unix will not be enforced.
Note that even without the abi/3.0 rule, (open)SUSE kernels support and
enforce network rules since years, and Ubuntu kernels support all rule
types.
Older AppArmor versions will ignore the abi line.
Adding the abi rule might mean that you'll have to add some network,
dbus or unix rules to the profiles, therefore please do some testing
instead of blindly adding the abi rule ;-)
Regards,
Christian Boltz
--
The Consultant's Curse:
When the customer has beaten upon you long enough, give him
what he asks for, instead of what he needs. This is very strong
medicine, and is normally only required once.
5:45 p.m.
On 6/23/21 1:17 PM, Christian Boltz wrote:
Hello,
[I'm not subscribed to libvir-list - please CC me in replies]
Am Mittwoch, 23. Juni 2021, 01:27:43 CEST schrieb Jim Fehlig:
> and other improvements. V2 of
> https://listman.redhat.com/archives/libvir-list/2021-June/msg00456.htm
>
> Changes since V1:
> Removed many unneeded capabilities. I used the 'audit' qualifier as
> suggested by cboltz to verify which capabilities were actually used.
> It's a difficult task though, as it is nearly impossible for one
> person to exercise a driver in all the ways thousands of users will
> push it :-). I was able to whittle the virtxend profile quite a bit
> since xen doesn't need a lot in the way of host capabilities.
Your updated patches look good :-)
Thanks. The V3 I sent earlier contains a few more incremental improvements and
can likely be merged IMO.
There's one thing I missed in the first review, but that might be
worth
a separate patch instead of updating this patchset:
Starting with AppArmor 3.0 userspace, profiles should have an abi rule
in their preamble (as the first line):
abi <abi/3.0>,
Definitely sounds like something for a separate patch.
Without this abi rule, network, dbus and unix will not be enforced.
Note that even without the abi/3.0 rule, (open)SUSE kernels support and
enforce network rules since years, and Ubuntu kernels support all rule
types.
Older AppArmor versions will ignore the abi line.
Adding the abi rule might mean that you'll have to add some network,
dbus or unix rules to the profiles, therefore please do some testing
instead of blindly adding the abi rule ;-)
My relationship with apparmor is complicated. Even the slightest changes call
for a fair bit of testing :-).
Regards,
Jim
1247
days inactive
1249
days old
12 comments
4 participants
participants (4)
-
Christian Boltz -
Christian Ehrhardt -
Daniel P. Berrangé -
Jim Fehlig