On 2/14/20 1:14 PM, Christian Ehrhardt wrote:
>
>
> On Fri, Feb 14, 2020 at 6:00 PM Jim Fehlig <jfehlig@suse.com
> <mailto:jfehlig@suse.com>> wrote:
>
>     On 2/13/20 4:32 AM, Christian Ehrhardt wrote:
>      > Configuring vhost-user-gpu like:
>      >      <video>
>      >        <driver name='vhostuser'/>
>      >        <model type='virtio' heads='1'/>
>      >      </video>
>      > Triggers an apparmor denial like:
>      >      apparmor="DENIED" operation="exec" profile="libvirtd"
>      >      name="/usr/lib/qemu/vhost-user-gpu" pid=888257 comm="libvirtd"
>      >      requested_mask="x" denied_mask="x" fsuid=0 ouid=0
>      >
>      > This helper is provided by qemu for vhost-user-gpu and thereby being
>      > in the same path as qemu_bridge_helper. Due to that adding a rule allowing
>      > to call uses the same path list.
>
>     Does the vhost-usr-gpu helper need a profile to restrict its access, similar to
>     the bridge helper?
>
> Hi Jim,
> Yes - we can later on add one, as soon as someone did the work to trace all the
> things that will be needed.
> I had no full setup - and I'm not sure about the multitude of potential
> configurations - so I didn't go that far.
> I didn't have that yet, but if anyone has please just add a follow on patch.
>
> The P in PUx allows that someone defines an external profile to guard it - and
> it would be used, but without one existing the U allows it to fall back to
> unconfined.

Nod. That's reasonable behavior in the absence of an internal profile.

> If/Once we add an internal profile like we do for bridge helper P can be changed
> to C, but as I said I have no useful profile and no full setup to test&train one
> at the moment.

With a bit of searching I could probably find a setup, but getting access is
another thing. In the meantime your patch is a fine alternative IMO

Reviewed-by: Jim Fehlig <jfehlig@suse.com>

Regards,
Jim

>      >
>      > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com
>     <mailto:christian.ehrhardt@canonical.com>>
>      > ---
>      >   src/security/apparmor/usr.sbin.libvirtd.in
>     <http://usr.sbin.libvirtd.in> | 1 +
>      >   1 file changed, 1 insertion(+)
>      >
>      > diff --git a/src/security/apparmor/usr.sbin.libvirtd.in
>     <http://usr.sbin.libvirtd.in> b/src/security/apparmor/usr.sbin.libvirtd.in
>     <http://usr.sbin.libvirtd.in>
>      > index b384b7213b..1e137039e9 100644
>      > --- a/src/security/apparmor/usr.sbin.libvirtd.in
>     <http://usr.sbin.libvirtd.in>
>      > +++ b/src/security/apparmor/usr.sbin.libvirtd.in
>     <http://usr.sbin.libvirtd.in>
>      > @@ -86,6 +86,7 @@ profile libvirtd @sbindir@/libvirtd
>     flags=(attach_disconnected) {
>      >     /usr/{lib,lib64}/xen-common/bin/xen-toolstack PUx,
>      >     /usr/{lib,lib64}/xen/bin/* Ux,
>      >     /usr/lib/xen-*/bin/libxl-save-helper PUx,
>      > +  /usr/{lib,lib64,lib/qemu,libexec}/vhost-user-gpu PUx,
>      >
>      >     # Required by nwfilter_ebiptables_driver.c:ebiptablesWriteToTempFile() to
>      >     # read and run an ebtables script.
>      >
>
>
>
> --
> Christian Ehrhardt
> Staff Engineer, Ubuntu Server
> Canonical Ltd