[libvirt] (resend) Problems with virt-manager checking access on virtual images.

[Daniel P. Berrange]

On Fri, Jan 30, 2009 at 09:06:38AM -0500, Daniel J Walsh wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Daniel P. Berrange wrote:
> > 
> > I don't particularly like the idea of running another program to check
> > this because SELinux context isn't the only thing which will potentially
> > prevent QEMU accessing the disk image. CGroups device policy make prevent
> > it. A setuid() call in QEMU to drop privileges, may prevent it. It may
> > be asked to open it read-write, and only be able to open it read-only .
> > It may want to take a lock on the image, but it already be locked, and
> > so on. QEMU does already report pretty much the same error message as 
> > the qemuaccess program does when it is unable to access a disk image
> > 
> > # virsh start demo
> > libvir: QEMU error : internal error unable to start guest: qemu: could not open disk image /home/berrange/Fedora-9-i686-Live.iso
> > 
> > IMHO, this all stems from a mistake I made when designing the original
> > virt-manager  UI. Namely, I should never have used a generic file 
> > dialog which allows selection of disk images anywhere on the host.  It
> > was wrong for general disk images, in just the same way that its wrong
> > for ISO images. And absolutely useless for remote provisioning.
> > 
> > With the storage pool APIs we have the opportunity to fix this in a way
> > helps not only the SELinux case, but also the app in general. Instead of
> > showing a generic file dialog, we should just show a dialog displaying
> > the configured storage pools, and allow the user to pick an ISO out of
> > one of the the pools. virt-manager should offer to remember which storage
> > pool contains installable ISO images, and which should be used by default
> > for disk images. We already have /var/lib/libvirt/images by default for
> > disk images, and should add /var/lib/libvirt/isos too. If a user downloads
> > an ISO to their home directory, we could easily have a option in the UI
> > for 'Import an ISO file to the pool' which would move it into the correct
> > location. 
> > 
> > NB, here I am talking about use of the system-wide QEMU driver instance
> > of 'qemu:///system', which runs privileged on the host. This is really
> > intended at server deployments of virt, but we have been happening to 
> > use it for general ad-hoc desktop usage too in Fedora.
> > 
> > There is also a per-user 'qemu://session' UI where both libvirtd and the
> > QEMU guests run unprivileged as that user's UID. In that scenario the
> > storage pools should use something like $HOME/VirtualMachines/images and
> > "$HOME/VirtualMachines/ISOs'as the default pools for disk & ISOs
> > respectively. I would like to see Fedora use qemu:///session by default
> > for generic desktop virt usage, in which case everything runs as that
> > UID, and keeps everything in $HOME. 
> > 
> We discussed this also.  The only problem I see here is the additional
> disk usage, since you would permanently keeping the iso, even though you
> might only be using it once.  What about removable media, cdrom and usb
> key isos.  Do you want to copy them to the /var/lib/libvirt/isos
> directory also?

If think if someone downloads a large ISO its fairly likely they'll want
to keep it around for some amount of time, rather than risk having to
download it again. That said, our storage pools APIs can trivally be
used to delete it if it is no longer required.

For removable media, we're just directly attaching the block device,
so we can't avoid the need to re-label from fixed_dev_t (or equiva)
to virt_image_t - though having a virt_image_ro_t for readonly access
would be preferrable for cases where we're explicitly attaching it to
the guest in read-only mode. 

USB keys I'm not so sure about - in the common case they'll be FAT
filesystem which doesn't support labelling at all. So we'd either have
to ensure its mounted with a suitable context= mount option, or get
the policy to  to allow read-only access to the default mount context
for FAT filesystems. The /var/lib/libvirt/isos directly is just a 
plain directory based storage pool. We also have concept of filesystem
based storage pools (eg a local device, mounted in some directory)
which can we directly manage. Likewise NFS mounts we can handle
directly, mounting a remote directoy from a server in a specific
place, so we can ensure a context= mount option is used.

So I think a combination of factors

 - Default directories for local host stored file based images & ISO
   which get correct context automatically
 - libvirt has to set contenxt on any block device nodes which need
   to be assigned
 - Setting mount context for removable disks / changing policy to
   allow read-only access to mounted removable disks.
 

> 
> Are the isos in this directory going to be Read Only or can some qemus
> read/write them?

Any files used to back the QEMU CDROM device should be restricted to
be read-only, since we need ability to safely assign the same ISO to
multiple guests concurrently - for concurrent installs. So a new disk
type virt_image_ro_t might be desirable  here.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|