[libvirt] RFC: Making storage vol upload/download more useful

[Daniel P. Berrange]

The virStorageVolUpload/Download APIs currently require that you upload
the physical disk image content, rather than the virtual disk image
content. ie the entire qcow2 file, not just its payload.

While this is certainly useful in some scenarios, it is rather unhelpful,
if not dangerous, in others.

For example, a qcow2 file can contain a backing file that points to
anything on the host filesystem and libvirt happily accepts it. The
best you can do is to query the volume XML again after upload and see
if it now reports a backing file. It would be highly desirable to
block this kind of thing before upload, because you don't want the
act of uploading to cause libvirt to open these arbitrary files.

In fact no matter what format you're storage volume currently is,
when you're uploading new content at a physical level, it can change
to any other format.

If uploading non-raw files, there's also no way to restrict the size
of the files uploaded. ie you can upload a 256 KB sized qcow2 file
whose logical disk size is 4 exabytes.  Again you can only detect
this after you've uploaded it, unless you're willing to run qemu-img
manually before upload, or write your own parser to validate upfront.
I would probably ok to just ignore this problem - checking after the
fact doesn't cause security problems, as the large logical size is
an issue until the disk is given to a guest for use.


Finally, you might not even have the file in the desired format. For
example, someone might have given you a raw file,but you want to store
everything as qcow2 files. Currently you would have to create a new
volume, upload the content to a new volume, delete the existing volume
and use virStorageVolCreateFrom to re-create it again with a forkmat
conversion, before finally deleting the temporary upload. It would be
very nice to be able to upload straight into the payload of an existing
file in one go - even if libvirt has to do a qemu-img conversion in the
process.


Some of these problems can be fixed with adding flags. For example I
would suggest

  - VIR_STORAGE_VOL_UPLOAD_VIRTUAL  => the stream provides the
    virtual disk image payload, not a physical disk image container

  - VIR_STORAGE_VOL_UPLOAD_FIXED_FORMAT => when VIRTUAL is not set,
    this indicates that the physical source format, must match the
    current storage vol format. ie don't allow the upload to change
    the image format by uploading a raw file into a qcow2 file or
    vica-verca

  - VIR_STORAGE_VOL_UPLOAD_NO_BACKING_FILE => when VIRUTAL is not
    set this indicates that the uploaded content must not be
    permitted to have any backing file set.


To allow on the fly conversion of data when using the VIRTUAL flag
though, we need a new API to expose the stream data format, otherwise
libvirt would have to probe stream format to know what to convert
from, and that is unsafe from a security POV:

    virStorageVolUploadConvert(virStorageVolPtr vol,
                               virStreamPtr stream,
                               int srcFormat,
                               unsigned long long offset,
                               unsigned long long length,
                               unsigned int flags);

The srcFormat parameter would map to virStorageFileFormat enum, which
would need to become public. It says what format the stream is providing
data in. In terms of implementation we would have to write the data into
a temporary file, and then use qemu-img convert to get it into the actual
file we want. We can't feed straight into qemu-img convert, since it
needs a seekable file as input & output.

If we had this functionality, then I think it would be possible for apps
to deal with storage uploads from end users, without having to run
qemu-img manually to validate data outside libvirt.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|