[libvirt PATCH 0/1] apparmor: Allow umount(/dev)
CC'ing AppArmor experts to get their input :) This is a farily big hammer, but unfortunately I don't think it's possible to tell AppArmor "let the driver use umount, but only if it's running inside a namespace". Andrea Bolognani (1): apparmor: Allow umount(/dev) src/security/apparmor/usr.sbin.libvirtd.in | 1 + src/security/apparmor/usr.sbin.virtqemud.in | 1 + 2 files changed, 2 insertions(+) -- 2.39.0
Commit 379c0ce4bfed introduced a call to umount(/dev) performed inside the namespace that we run QEMU in. As a result of this, on machines using AppArmor, VM startup now fails with internal error: Process exited prior to exec: libvirt: QEMU Driver error: failed to umount devfs on /dev: Permission denied The corresponding denial is AVC apparmor="DENIED" operation="umount" profile="libvirtd" name="/dev/" pid=70036 comm="rpc-libvirtd" Extend the AppArmor configuration for virtqemud and libvirtd so that this operation is allowed. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/security/apparmor/usr.sbin.libvirtd.in | 1 + src/security/apparmor/usr.sbin.virtqemud.in | 1 + 2 files changed, 2 insertions(+) diff --git a/src/security/apparmor/usr.sbin.libvirtd.in b/src/security/apparmor/usr.sbin.libvirtd.in index 886f1ad518..edb8dd8e26 100644 --- a/src/security/apparmor/usr.sbin.libvirtd.in +++ b/src/security/apparmor/usr.sbin.libvirtd.in @@ -35,6 +35,7 @@ profile libvirtd @sbindir@/libvirtd flags=(attach_disconnected) { mount options=(rw,rslave) -> /, mount options=(rw, nosuid) -> /{var/,}run/libvirt/qemu/*.dev/, umount /{var/,}run/libvirt/qemu/*.dev/, + umount /dev/, # libvirt provides any mounts under /dev to qemu namespaces mount options=(rw, move) /dev/ -> /{,var/}run/libvirt/qemu/*.dev/, diff --git a/src/security/apparmor/usr.sbin.virtqemud.in b/src/security/apparmor/usr.sbin.virtqemud.in index 3de03d49fc..f269c60809 100644 --- a/src/security/apparmor/usr.sbin.virtqemud.in +++ b/src/security/apparmor/usr.sbin.virtqemud.in @@ -35,6 +35,7 @@ profile virtqemud @sbindir@/virtqemud flags=(attach_disconnected) { mount options=(rw,rslave) -> /, mount options=(rw, nosuid) -> /{var/,}run/libvirt/qemu/*.dev/, umount /{var/,}run/libvirt/qemu/*.dev/, + umount /dev/, # libvirt provides any mounts under /dev to qemu namespaces mount options=(rw, move) /dev/ -> /{,var/}run/libvirt/qemu/*.dev/, -- 2.39.0
On 1/18/23 10:43, Andrea Bolognani wrote:
Commit 379c0ce4bfed introduced a call to umount(/dev) performed inside the namespace that we run QEMU in.
As a result of this, on machines using AppArmor, VM startup now fails with
internal error: Process exited prior to exec: libvirt: QEMU Driver error: failed to umount devfs on /dev: Permission denied
The corresponding denial is
AVC apparmor="DENIED" operation="umount" profile="libvirtd" name="/dev/" pid=70036 comm="rpc-libvirtd"
Extend the AppArmor configuration for virtqemud and libvirtd so that this operation is allowed.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/security/apparmor/usr.sbin.libvirtd.in | 1 + src/security/apparmor/usr.sbin.virtqemud.in | 1 + 2 files changed, 2 insertions(+)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> For more background on why umount is needed see my reply to Jim's question from earlier: https://listman.redhat.com/archives/libvir-list/2023-January/237149.html Michal
On Wed, Jan 18, 2023 at 11:00:33AM +0100, Michal Prívozník wrote:
On 1/18/23 10:43, Andrea Bolognani wrote:
Commit 379c0ce4bfed introduced a call to umount(/dev) performed inside the namespace that we run QEMU in.
As a result of this, on machines using AppArmor, VM startup now fails with
internal error: Process exited prior to exec: libvirt: QEMU Driver error: failed to umount devfs on /dev: Permission denied
The corresponding denial is
AVC apparmor="DENIED" operation="umount" profile="libvirtd" name="/dev/" pid=70036 comm="rpc-libvirtd"
Extend the AppArmor configuration for virtqemud and libvirtd so that this operation is allowed.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/security/apparmor/usr.sbin.libvirtd.in | 1 + src/security/apparmor/usr.sbin.virtqemud.in | 1 + 2 files changed, 2 insertions(+)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For more background on why umount is needed see my reply to Jim's question from earlier:
https://listman.redhat.com/archives/libvir-list/2023-January/237149.html
Welp, missed that one O:-) Jim, it looks like you came up with exactly the same solution as me, despite concerns about the size of the resulting hammer. Any other ideas, or should we just go ahead and merge this as-is? -- Andrea Bolognani / Red Hat / Virtualization
On 1/18/23 03:45, Andrea Bolognani wrote:
On Wed, Jan 18, 2023 at 11:00:33AM +0100, Michal Prívozník wrote:
On 1/18/23 10:43, Andrea Bolognani wrote:
Commit 379c0ce4bfed introduced a call to umount(/dev) performed inside the namespace that we run QEMU in.
As a result of this, on machines using AppArmor, VM startup now fails with
internal error: Process exited prior to exec: libvirt: QEMU Driver error: failed to umount devfs on /dev: Permission denied
The corresponding denial is
AVC apparmor="DENIED" operation="umount" profile="libvirtd" name="/dev/" pid=70036 comm="rpc-libvirtd"
Extend the AppArmor configuration for virtqemud and libvirtd so that this operation is allowed.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/security/apparmor/usr.sbin.libvirtd.in | 1 + src/security/apparmor/usr.sbin.virtqemud.in | 1 + 2 files changed, 2 insertions(+)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For more background on why umount is needed see my reply to Jim's question from earlier:
https://listman.redhat.com/archives/libvir-list/2023-January/237149.html
Welp, missed that one O:-)
Jim, it looks like you came up with exactly the same solution as me, despite concerns about the size of the resulting hammer. Any other ideas, or should we just go ahead and merge this as-is?
My apparmor skills are too weak to select a smaller tool, so I'd say merge as-is. It wasn't clear to me if/why the umount of /dev was actually needed, but Michal did an excellent job of describing why it is. Regards, Jim
On Wed, Jan 18, 2023 at 08:59:23AM -0700, Jim Fehlig wrote:
On 1/18/23 03:45, Andrea Bolognani wrote:
Jim, it looks like you came up with exactly the same solution as me, despite concerns about the size of the resulting hammer. Any other ideas, or should we just go ahead and merge this as-is?
My apparmor skills are too weak to select a smaller tool, so I'd say merge as-is. It wasn't clear to me if/why the umount of /dev was actually needed, but Michal did an excellent job of describing why it is.
Okay, pushed now. Does this warrant creating a maintenance branch / release? 9.0.0 is basically unusable out of the box on AppArmor hosts... On the other hand, package maintainers for Debian/Ubuntu and openSUSE are aware of the issue and know exactly which commit they need to backport. Are there other distros out there using AppArmor? -- Andrea Bolognani / Red Hat / Virtualization
On 1/18/23 10:07, Andrea Bolognani wrote:
On Wed, Jan 18, 2023 at 08:59:23AM -0700, Jim Fehlig wrote:
On 1/18/23 03:45, Andrea Bolognani wrote:
Jim, it looks like you came up with exactly the same solution as me, despite concerns about the size of the resulting hammer. Any other ideas, or should we just go ahead and merge this as-is?
My apparmor skills are too weak to select a smaller tool, so I'd say merge as-is. It wasn't clear to me if/why the umount of /dev was actually needed, but Michal did an excellent job of describing why it is.
Okay, pushed now.
Does this warrant creating a maintenance branch / release? 9.0.0 is basically unusable out of the box on AppArmor hosts...
There have been similar issues with past releases, e.g. a bug in the libxl driver preventing libvirt use with Xen.
On the other hand, package maintainers for Debian/Ubuntu and openSUSE are aware of the issue and know exactly which commit they need to backport.
Like the past cases, I'm fine backporting the commit.
Are there other distros out there using AppArmor?
Not that I'm aware of. Regards, Jim
On 1/18/23 02:43, Andrea Bolognani wrote:
CC'ing AppArmor experts to get their input :)
This is a farily big hammer, but unfortunately I don't think it's possible to tell AppArmor "let the driver use umount, but only if it's running inside a namespace".
Andrea Bolognani (1): apparmor: Allow umount(/dev)
src/security/apparmor/usr.sbin.libvirtd.in | 1 + src/security/apparmor/usr.sbin.virtqemud.in | 1 + 2 files changed, 2 insertions(+)
Reviewed-by: Jim Fehlig <jfehlig@suse.com> Regards, Jim
participants (3)
-
Andrea Bolognani -
Jim Fehlig -
Michal Prívozník