Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it

[Jeff Cody <jcody@redhat.com>]

On Mon, Nov 10, 2014 at 11:30:25AM +0100, Markus Armbruster wrote:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Am 10.11.2014 um 09:12 hat Markus Armbruster geschrieben:
> >> Jeff Cody <jcody@redhat.com> writes:
> >> > So that would mean .img would always require format=, right?
> >> >
> >> > That also implies to me that the only extensions for raw that might
> >> > not require format= would be .iso and .raw.
> >> 
> >> .img means what we choose it to mean.
> >> 
> >> If we choose "can mean anything, including raw", then .img always
> >> requires an explicit format with this approach.
> >> 
> >> If we choose "means raw", then same as above, except you can omit
> >> format=raw, and you become prone to opening existing non-raw formats
> >> raw, which can be bad.
> >
> > My current thoughts about .img are that we need to consider that
> >
> > (a) it is occasionally used for multiple image formats and making it
> >     raw unconditionally is going to cause corruption.
> >
> > (b) looking at file extensions is absolutely useless if we exlucde
> >     .img from the automatic detection because it's still the main
> >     extension for raw.
> >
> > The common case could probably be covered by bringing probing back into
> > the game: If an .img file successfully probes for a non-raw format,
> > error out, avoiding the corruption. If it doesn't, assume raw.
> 
> I think this is the "refuse to use a format without an explicit format=
> when any other non-raw format probe accepts" idea, which should combine
> nicely with all the other ideas proposed so far.
> 
> Combining it with the hybrid approach we're discussing here gets us
> something like this, assuming .img means raw:
> 
> 1. Guess possible formats from trusted meta-data
> 
>    For foo.img, this yields { raw }.
> 
> 2. Probe to pick one
> 
>    Since { raw } has just one member, pick it without probing.
> 
> 3. Now probe all other non-raw formats, and error out if any accepts
> 
>    Protects users from opening a non-raw foo.img raw.
>

With the exception of fixed-size VHD.

But this problem exists today, so there isn't anything 'new' broken
there.  And we can fix that if we want by upgrading probing, to pass
both the first 512 bytes, and the last 512 bytes of the image file.

> 1+2 are the hybrid approach, 3 is the "refuse" idea.
> 
> Adding 3 is an improvement, from "this usage will now break at runtime,
> possibly corrupting data" to "this usage will now be rejected cleanly".
> 
> 3 should also help catch insufficiently selective probe methods.