[PATCH v2] apparmor: allow libvirtd to call virtiofsd

[Christian Ehrhardt]

On Tue, Aug 25, 2020 at 3:31 PM Kevin Locke <kevin at kevinlocke.name> wrote:
>
> When using [virtiofs], libvirtd must launch [virtiofsd] to provide
> filesystem access on the host.  When a guest is configured with
> virtiofs, such as:
>
>     <filesystem type='mount' accessmode='passthrough'>
>       <driver type='virtiofs'/>
>       <source dir='/path'/>
>       <target dir='mount_tag'/>
>     </filesystem>
>
> Attempting to start the guest fails with:
>
>     internal error: virtiofsd died unexpectedly
>
> /var/log/libvirt/qemu/$name-fs0-virtiofsd.log contains (as a single
> line, wrapped below):
>
>     libvirt:  error : cannot execute binary /usr/lib/qemu/virtiofsd:
>     Permission denied
>
> dmesg contains (as a single line, wrapped below):
>
>     audit: type=1400 audit(1598229295.959:73): apparmor="DENIED"
>     operation="exec" profile="libvirtd" name="/usr/lib/qemu/virtiofsd"
>     pid=46007 comm="rpc-worker" requested_mask="x" denied_mask="x"
>     fsuid=0 ouid=0
>
> To avoid this, allow execution of virtiofsd from the libvirtd AppArmor
> profile.
>
> [virtiofs]: https://libvirt.org/kbase/virtiofs.html
> [virtiofsd]: https://www.qemu.org/docs/master/interop/virtiofsd.html
>
> Signed-off-by: Kevin Locke <kevin at kevinlocke.name>
> Reviewed-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>

Thank you Kevin for the v2!
I've now also had the chance to test it and can confirm the reported issues
as well as the change fixing it.
With review and test in place I'll commit this apparmor change before
the 6.7.0 freeze happens.

But long term we should think about adding a profile for virtiofsd itself.
I have started some work but it is yet imperfect, it has open TODOs.
I'll reply with a RFC patch to this mail how that sub-profile could look
like and hope for a good discussion there from everyone.

In that RFC are questions for everyone (expected paths to agree on) as
well as apparmor specialists (I hope for Jamie) around pivot_root.

@Kevin - if you want you could continue your experiments with that
subprofile and let me know of the rough bumps that you find with it.

> ---
>
> Changes in v2:
> - Wrap log and dmesg messages, as requested by Christian Ehrhardt.
>
>  src/security/apparmor/usr.sbin.libvirtd.in | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/security/apparmor/usr.sbin.libvirtd.in b/src/security/apparmor/usr.sbin.libvirtd.in
> index 4518e8f865..f2030764cd 100644
> --- a/src/security/apparmor/usr.sbin.libvirtd.in
> +++ b/src/security/apparmor/usr.sbin.libvirtd.in
> @@ -89,6 +89,7 @@ profile libvirtd @sbindir@/libvirtd flags=(attach_disconnected) {
>    /usr/lib/xen-*/bin/libxl-save-helper PUx,
>    /usr/lib/xen-*/bin/pygrub 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.
> --
> 2.28.0
>

--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd