1. Patchew
  2. Libvirt
  3. View series

[PATCH] apparmor: allow to call vhost-user-gpu

Christian Ehrhardt posted 1 patch 4 years, 2 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200213113236.24358-1-christian.ehrhardt@canonical.com
src/security/apparmor/usr.sbin.libvirtd.in | 1 +
1 file changed, 1 insertion(+)
[PATCH] apparmor: allow to call vhost-user-gpu
Posted by Christian Ehrhardt 4 years, 2 months ago 
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.

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
 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 b384b7213b..1e137039e9 100644
--- a/src/security/apparmor/usr.sbin.libvirtd.in
+++ b/src/security/apparmor/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.
-- 
2.25.0
Re: [PATCH] apparmor: allow to call vhost-user-gpu
Posted by Jim Fehlig 4 years, 2 months ago 
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?

Regards,
Jim

> 
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> ---
>   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 b384b7213b..1e137039e9 100644
> --- a/src/security/apparmor/usr.sbin.libvirtd.in
> +++ b/src/security/apparmor/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.
>
Re: [PATCH] apparmor: allow to call vhost-user-gpu
Posted by Christian Ehrhardt 4 years, 2 months ago 
On Fri, Feb 14, 2020 at 6:00 PM Jim Fehlig <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.
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.

Regards,
> Jim
>
> >
> > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > ---
> >   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 b384b7213b..1e137039e9 100644
> > --- a/src/security/apparmor/usr.sbin.libvirtd.in
> > +++ b/src/security/apparmor/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
Re: [PATCH] apparmor: allow to call vhost-user-gpu
Posted by Jim Fehlig 4 years, 2 months ago 
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
Re: [PATCH] apparmor: allow to call vhost-user-gpu
Posted by Christian Ehrhardt 4 years, 1 month ago 
On Fri, Feb 14, 2020 at 10:19 PM Jim Fehlig <jfehlig@suse.com> wrote:

> 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>
>

Thanks, pushed with the Reviewed-by added.


> 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
>
>

-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.