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

[Stefan Hajnoczi]

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 <address@hidden> 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.

(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.)

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

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.

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.

Stefan

pgpfWByNhXmQV.pgp
Description: PGP signature