Re: [PATCH v3 1/4] block: Add trivial backing_fmt support to qcow, sheepdog, vmdk

[Eric Blake]

On 3/9/20 10:44 AM, Daniel P. Berrangé wrote:

> Consider the user creates an image with "-F raw". We can validate the backing
> image is raw, and so our check succeeds.  Later the malicious <something> can
> write a qcow header into this raw file and QEMU will thereafter probe the
> image as qcow, not raw.
>
> IOW, in the case of "-F raw", even if we immediately check the format, we're
> still not offering the protection promised by the "-F" flag, because that
> promise refers to the runtime behaviour of the QEMU emulator, not the
> immediate qemu-img cmd.
>
> We could support "-F ..." and validate any non-raw formats, while raising a
> runtime error in the case of "-F raw", as only the "raw" backing format has
> the probing security risk.
>
> Users who need  to use qcow, with a backing file, without a format can
> just not pass "-F" and in doing so will be insecure.

And the warning will remind them of that.

> We could take this opportunity to deprecate 'qcow' perhaps, declare it
> a read-only format, restricted to qemu-img/qemu-io for purpose of data
> liberation ?

I'm fine with that, although it makes for a bigger task.

> For sheepdog, if it is something we genuinely still care about, then
> adding a backing file format record seems neccessary, unless we either
> forbid use of raw backing files, or forbid use of non-raw backing files,
> either way would be safe.

I concur - as long as you either have ONLY non-raw (in which case 
probing is safe), or ONLY raw (in which case no probing is necessary), 
then not recording the backing format is safe.  It is only for formats 
that allow both raw and non-raw backing, but which do not have space in 
the image to document which of the two backing formats is expected, 
where we have problems.

> > I'm guessing that qcow works with either raw or qcow as backing format (and
> > anything else is odd - a qcow2 backing to a qcow is unusual, and would be
> > better to reject).  I'm not sure if sheepdog can be backed by anything but
> > another sheepdog, similarly, I'm not sure if a vmdk can be backed by
> > anything but another vmdk.  If so, it should be simple enough to do a v4 of
> > this patch which requires -F to be a known-acceptable probe type for these
> > images.
> >
> > Still, the point of this patch is that I want to add -F into all the
> > iotests, and without something along the lines of this patch, all of those
> > iotests are broken for these image formats.  Patch 2 is a lot harder to
> > write if we have to make our use of -F conditional on the image format in
> > question.
> >
> > --
> > Eric Blake, Principal Software Engineer
> > Red Hat, Inc.           +1-919-301-3226
> > Virtualization:  qemu.org | libvirt.org
>
> Regards,
> Daniel

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org