[libvirt] [PATCH] qemudDomainMemoryPeek: chown temporary file to qemu.qemu.

[Daniel P. Berrange]

On Fri, May 20, 2011 at 03:28:14PM +0100, Richard W.M. Jones wrote:
> 
> This attached patch resolves a fairly obvious problem which stops
> qemuDomainMemoryPeek from working at all.
> 
> However, it's not the whole story.  Read on ...
> 
> (1) qemu still fails unless I manually set the mode of
> /var/cache/libvirt to 0755 (it is normally 0700).
> 
>                            Owner      Fails   Works
>   /var/cache/libvirt       root.root  0700    0755
>   /var/cache/libvirt/qemu  qemu.qemu  0750    0750
> 
> qemu is doing:
> 
>   open("/var/cache/libvirt/qemu/qemu.mem.fdVCod", O_WRONLY|O_CREAT|O_TRUNC, 0666)
> 
> It's kinda annoying that /etc/libvirt and /var/{cache,lib}/libvirt are
> unreadable by other users.  Is there some deep reason to do this?

Both /etc/libvirt & /var/lib/libvirt stores files which may
contain passwords. /var/cache/libvirt contains data such
as these memory dumps which potentially have sensitive data
in them. Unprivileges users on the host must be prevented
from seeing any of this data unless authorized.

I think we likely need  /var/cache/libvirt to be 0711 so that
QEMU can access directories below it, but not actually read it.

> (2) After applying this patch, using virDomainMemoryPeek causes
> libvirtd to generate the following serious-looking warning.  It
> doesn't appear to crash or fail in any way that I can tell:
> 
>   15:17:09.982: 7784: error : virDomainFree:2122 : invalid domain pointer in virDomainFree
> 
> I don't know where this is coming from.  It only appears when my
> program actually does virDomainMemoryPeek, not when I have the same
> code with virDomainMemoryPeek commented out.

Oh, there is a bogus  'if (dom) virDomainFree(dom)' call in the
remote dispatcher remoteDispatchDomainMemoryPeek

> >From 1a2ba0c1b58142a06602722c6bb0995ef6e8b347 Mon Sep 17 00:00:00 2001
> From: Richard W.M. Jones <rjones at redhat.com>
> Date: Fri, 20 May 2011 13:56:46 +0100
> Subject: [PATCH] qemudDomainMemoryPeek: chown temporary file to qemu.qemu.
> 
> Otherwise qemu is unable to write to it, with the error:
> 
> libvir: QEMU error : internal error unable to execute QEMU command 'memsave': Could not open '/var/cache/libvirt/qemu/qemu.mem.RRNvLv'
> ---
>  src/qemu/qemu_driver.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 44acc6a..08d2549 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5536,6 +5536,14 @@ qemudDomainMemoryPeek (virDomainPtr dom,
>          goto endjob;
>      }
>  
> +    if (qemu_driver->privileged &&
> +        chown(tmp, qemu_driver->user, qemu_driver->group) < 0) {
> +        virReportSystemError(errno,
> +                             _("unable to set ownership on %s to %d:%d"),
> +                             tmp, qemu_driver->user, qemu_driver->group);
> +        goto endjob;
> +    }

We will also need to set the SELinux context on the file. So instead
of directly using chown,  we need to call

virSecurityManagerSetSavedStateLabel(qemu_driver->securityManager, vm, tmp);

and after the monitor command completes, run

virSecurityManagerRestoreSavedStateLabel(qemu_driver->securityManager, vm, tmp);

to revoke QEMU's permission again

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|