[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Daniel P. Berrange wrote:
> 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

New file context in
/var/lib/libvirt/images(/.*)?
gen_context(system_u:object_r:virt_image_t,s0)
/var/lib/libvirt/isos(/.*)?
gen_context(system_u:object_r:virt_content_t,s0)

HOME_DIR/VirtualMachines(/.*)?
gen_context(system_u:object_r:virt_image_t,s0)
HOME_DIR/VirtualMachines/isos(/.*)?
gen_context(system_u:object_r:virt_content_t,s0)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAkmDGvQACgkQrlYvE4MpobNqnwCfXvE6ZrPZ05igprlDxCNOXc24
c84AoIjZKuCdczTj2p29l1+zV3ze1Q/6
=pin6
-----END PGP SIGNATURE-----


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]