qemu: caps: Don't try to ask for CAP_DAC_OVERRIDE if non-root

[libvirt] [PATCH RFC] qemu: caps: Don't try to ask for CAP_DAC_OVERRIDE if non-root

Posted by Peter Krempa 5 years, 2 months ago

It will not work. This breaks qemu capabilities probing as a user.
---
 src/qemu/qemu_capabilities.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 7ed5f94803..81ef0357e7 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4524,7 +4524,8 @@ virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
 #if WITH_CAPNG
     /* QEMU might run into permission issues, e.g. /dev/sev (0600), override
      * them just for the purpose of probing */
-    virCommandAllowCap(cmd->cmd, CAP_DAC_OVERRIDE);
+    if (geteuid() == 0)
+        virCommandAllowCap(cmd->cmd, CAP_DAC_OVERRIDE);
 #endif

     virCommandSetGID(cmd->cmd, cmd->runGid);
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH RFC] qemu: caps: Don't try to ask for CAP_DAC_OVERRIDE if non-root

Posted by Peter Krempa 5 years, 2 months ago

On Mon, Feb 04, 2019 at 16:26:02 +0100, Peter Krempa wrote:
> It will not work. This breaks qemu capabilities probing as a user.
> ---
>  src/qemu/qemu_capabilities.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 7ed5f94803..81ef0357e7 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -4524,7 +4524,8 @@ virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
>  #if WITH_CAPNG
>      /* QEMU might run into permission issues, e.g. /dev/sev (0600), override
>       * them just for the purpose of probing */
> -    virCommandAllowCap(cmd->cmd, CAP_DAC_OVERRIDE);
> +    if (geteuid() == 0)

Possibly replacing this with driver->privileged for the qemu driver but
that is not easily accessible.

> +        virCommandAllowCap(cmd->cmd, CAP_DAC_OVERRIDE);
>  #endif
> 
>      virCommandSetGID(cmd->cmd, cmd->runGid);
> -- 
> 2.20.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH RFC] qemu: caps: Don't try to ask for CAP_DAC_OVERRIDE if non-root

Posted by Daniel P. Berrangé 5 years, 2 months ago

On Mon, Feb 04, 2019 at 04:32:52PM +0100, Peter Krempa wrote:
> On Mon, Feb 04, 2019 at 16:26:02 +0100, Peter Krempa wrote:
> > It will not work. This breaks qemu capabilities probing as a user.
> > ---
> >  src/qemu/qemu_capabilities.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index 7ed5f94803..81ef0357e7 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -4524,7 +4524,8 @@ virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
> >  #if WITH_CAPNG
> >      /* QEMU might run into permission issues, e.g. /dev/sev (0600), override
> >       * them just for the purpose of probing */
> > -    virCommandAllowCap(cmd->cmd, CAP_DAC_OVERRIDE);
> > +    if (geteuid() == 0)
> 
> Possibly replacing this with driver->privileged for the qemu driver but
> that is not easily accessible.

It doesn't really matter as  privileged == (geteuid() == 0)  since we purged
the hack for Solaris that tried to run privileged libvirtd as non-root for
Xen.  We've got many other cases using geteuid() == 0 where they cannot
access the 'privileged' flag alrady.

Regards,
Daniel
-- 
|: 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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH RFC] qemu: caps: Don't try to ask for CAP_DAC_OVERRIDE if non-root

Posted by Erik Skultety 5 years, 2 months ago

On Mon, Feb 04, 2019 at 04:26:02PM +0100, Peter Krempa wrote:
> It will not work. This breaks qemu capabilities probing as a user.
> ---
>  src/qemu/qemu_capabilities.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 7ed5f94803..81ef0357e7 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -4524,7 +4524,8 @@ virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
>  #if WITH_CAPNG
>      /* QEMU might run into permission issues, e.g. /dev/sev (0600), override
>       * them just for the purpose of probing */
> -    virCommandAllowCap(cmd->cmd, CAP_DAC_OVERRIDE);
> +    if (geteuid() == 0)
> +        virCommandAllowCap(cmd->cmd, CAP_DAC_OVERRIDE);

This should be enough of a fix with the current code base as it won't clash with
the other two CAP_SYS_RAWIO and CAP_NET_ADMIN which should still fail when used
with the session daemon as expected.

Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list