Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing

[Max Reitz <mreitz@redhat.com>]

On 2014-11-03 at 16:05, Stefan Hajnoczi wrote:
> On Mon, Nov 03, 2014 at 11:25:10AM +0100, Kevin Wolf wrote:
>> Am 03.11.2014 um 09:54 hat Markus Armbruster geschrieben:
>>> Kevin Wolf <kwolf@redhat.com> writes:
>>>
>>>> Am 31.10.2014 um 12:24 hat Stefan Hajnoczi geschrieben:
>>>>> On Thu, Oct 30, 2014 at 10:36:35AM +0100, Kevin Wolf wrote:
>>>>>> Am 30.10.2014 um 10:27 hat Stefan Hajnoczi geschrieben:
>>>>>>> The guest may legitimately use raw devices that contain image format
>>>>>>> data.  Imagine tools similar to libguestfs.
>>>>>>>
>>>>>>> It's perfectly okay for them to lay out image format data onto a raw
>>>>>>> device.
>>>>>>>
>>>>>>> Probing is the problem, not putting image format data onto a raw device.
>>>>>> Agreed, that's why any restrictions only apply when probing was used to
>>>>>> detect a raw image. If you want to do anything exotic like storing a
>>>>>> qcow2 image for nested virt on a disk that is a raw image in the host,
>>>>>> then making sure to pass format=raw shouldn't be too much.
>>>>> Because at that point the solution is way over-engineered.
>>>>>
>>>>> Probing checks should be in the QEMU command-line code, not sprinkled
>>>>> across the codebase and even at run-time.
>>>>>
>>>>> Isn't Markus approach much simpler and cleaner?
>>>> I don't think so. My code isn't "sprinkled across the codebase", it has
>>>> the checks right where the problem arises, in the raw block driver.
>>> Actually, that's not where the problem is.
>>>
>>> The guest writing funny things to its disks is *not* the problem, it's
>>> perfectly normal operation.  Probing untrusted images is the problem!
>> I think we all know it, but let's clearly state the real problem once
>> again to remind us what we're trying to do on a high level. The security
>> problem we're talking about is that some image formats can contain file
>> names, and the content of these files can be exposed (in some cases even
>> r/w) to the guest.
> That's too specific.  There are other cases:
>
> Imagine you are paying for a 10 GB disk KVM virtual server with a
> hosting provider.
>
> The hosting provider did not configure KVM correctly and they use
> probing.  You can write a qcow2 header with a 100 GB disk size to the
> raw disk and reboot the VM.
>
> Suddenly your guest has access to 100 GB of disk space even though you
> are only paying for 10 GB!
>
>> Therefore not probing, but using untrusted images is the root problem -
>> but it's not one that we can fix. If the users downloads (or otherwise
>> obtains) an untrusted image with a bad backing file name, and of course
>> the correct file extension, we don't have a chance to protect them.
> This is driving the discussion into the wrong direction.  You are right
> that QEMU cannot solve that but it's also a well-known problem that
> OpenStack and co solved a long time ago.
>
>> What we can do something about is a special case of this, where a
>> malicious guest tries to turn a (trusted) image into something that
>> isn't trustworthy any more. In my book, the bad action is modifying the
>> image so that it looks like a different format with a bad header, it's
>> not treating an image file like what it looks like. If you disable
>> probing in qemu, you still allow qemu guests to write bad headers that
>> can fool other tools.
>>
>> The problem really isn't all the non-raw formats that have a header
>> which can contain file names. The problem is with raw which doesn't have
>> a header and allows the guest to turn its image file into any other file
>> type it wants and can potentially trick the user. raw is really a bad
>> format (or actually, it's a non-format), and if I could choose I
>> wouldn't support it, or at least only r/o. We should instead have used a
>> format with a header and then the raw file shifted by some offset. It's
>> too late for that, though.
>>
>> But coming back to your specific statement: Probing images isn't the
>> problem. The guest writing funny things isn't the problem either. Using
>> a format that can't reliably represent those funny things is the
>> problem. And if the format can't reliably represent something, it should
>> return an error when you try to do it anyway. If you need the feature of
>> storing funny things, use a format that supports it reliably. (And
>> calling explicit format=raw reliable isn't quite right either, but it's
>> a compromise I'd be willing to make.)
> I don't follow this logic.  You end up with "raw is bad because it
> doesn't have a header".  That doesn't get us anywhere.

It's true anyway. Just because an argument is not constructive does not 
mean it's bad, as long as it's true.

> (There are other formats that can also be confused.  For example, you
> can create a qcow2 file that is also a vpc file because vpc allows files
> that have only a footer.)

Then vpc is bad, too.

> Fact remains that you must not probe untrusted guest disk images.

Nobody wants to completely abolish probing. Markus only wants to use the 
extension as a second source of information, basically.

> The argument that there might not be a traditional filename doesn't make
> sense to me.  When there is no filename the command-line is already
> sufficiently complex and usage is fancy enough that probing adds no
> convenience, the user can just specify the format.

Apropos "the cloud provider did not configure KVM correctly"?

> Anyway, does this sound reasonable:
>
> In QEMU 3.0, require the format= option for -drive.  Keep probing the
> way it is for non-drive options because they are used for convenience by
> local users.

To me it does, yes.

Max