[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Image probing: how it can be insecure, and what we coul
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it |
Date: |
Thu, 6 Nov 2014 14:02:41 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 06.11.2014 um 13:26 hat Markus Armbruster geschrieben:
> >> * Reuse the image *without* specifying the raw format. QEMU guesses the
> >> format based on untrusted image contents. Now QEMU guesses a format
> >> chosen by the guest, with meta-data chosen by the guest. By
> >> controlling image meta-data, the malicious guest can access arbitrary
> >> files as QEMU, enlarge its storage, and more. A non-malicious guest
> >> can accidentally DoS itself, by writing a pattern probing recognizes.
> >
> > Thank you for bringing that to my attention. This means that I'm even
> > more in favor of using Kevin's patches because in fact they don't
> > break anything.
>
> They break things differently. The difference may or may not matter.
>
> Example: innocent guest writes a recognized pattern.
>
> Now: next restart fails, guest DoSed itself until host operator gets
> around to adding format=raw to the configuration. Consequence:
> downtime (probably lengthy), but no data corruption.
Depends.
Another possible outcome is that the guest doesn't DoS itself, but
writes a valid image header. You've argued before that a guest could be
keeping a qcow2 image on a whole disk for whatever odd reason. In this
case, the qemu restart will present the nested image instead of the
top-level one to the guest, which can probably be labelled corruption.
> With Kevin's patch: write fails, guest may or may not handle the
> failure gracefully. Consequences can range from "guest complains to
> its logs (who cares)" via "guest stops whatever it's doing and refuses
> to continue until its hardware gets fixed (downtime as above)" to
> "data corruption".
>
> Example: innocent guest first writes a recognized pattern, then
> overwrites it with a non-recognized pattern.
>
> Now: works.
>
> With Kevin's patch: as above.
>
> Again, I'm not claiming the differences are serious in practice, only
> that they exist.
Yes, the failure scenario is different. The point still stands that if
it were relevant in practice, we would likely have heard of it before.
> > You actually want to completely abolish probing? I thought we just
> > wanted to use the filename as a second source of information and warn
> > if the contents and the extension don't seem to match; and in the
> > future, maybe make it an error (but we don't have to discuss that
> > second part now, I think).
>
> Yes, I propose to ditch it completely, after a suitable grace period. I
> tried to make that quite clear in my PATCH RFC 2/2.
>
> Here's why.
>
> Now: 1. probe
> 4. open, error out if meta-data is bad
>
> With my proposed patch:
> 1. probe
> 2. guess from trusted meta-data
> 3. warn unless 1 and 2 match
> 4. open, error out if meta-data is bad
>
> Now make the warning an error:
> 1. probe
> 2. guess from trusted meta-data
> 3. error out unless 1 and 2 match
> 4. open, error out if meta-data is bad
>
> I figure the following is equivalent, but simpler:
>
> 2. guess from trusted meta-data
> 4. open, error out if meta-data is bad
>
> because open should detect all the errors the previous step 3 caught.
> If not, things are broken with explicit format=.
Not entirely true, see below.
> > My conclusion: Don't ditch probing. It increases entropy, why would
> > you ditch probing? Just combine it with the extension and if both
> > don't seem to match, that's an error.
>
> How does probing add value?
>
> Compare
>
> 1. probe
> 2. guess from trusted meta-data
> 3. error out unless 1 and 2 match
> 4. open, error out if meta-data is bad
>
> to just
>
> 2. guess from trusted meta-data
> 4. open, error out if meta-data is bad
>
> Let P be the driver recommended by probe, and G be the driver
> recommended by guess.
P = qcow2, G = raw
> If P == G, same result: we open with the same driver.
No, they are not the same.
> Else, if open with G fails, equivalent result: error out in step 3
> vs. error out in step 4.
No, raw accepts the image.
> Else, we have an odd case: one driver's probe accepts (P's), yet a
> different driver's open succeeds (G's).
>
> If G's probe rejects: is this a bug? Shouldn't open always fail
> when probe rejects?
No, raw's probe doesn't reject, it just has a very low score.
> If G's probe accepts, then probing chose P over G. Maybe it
> shouldn't have. Or maybe the probing functions are imprecise.
> Anyway, keeping probing around makes this an error. Should it be
> one?
Yes, raw being the fallback for everything is imprecise. It's the only
way we have for probing raw.
> Am I missing something?
This is the safety measure that was missing in your proposal, against
corruption caused by a qcow2 image stored in foo.img that is now
unintentionally opened as raw.
Same thing probably for qcow2 stored on LVs etc.
Kevin
- Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it, (continued)
- Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it, Kevin Wolf, 2014/11/05
- Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it, Markus Armbruster, 2014/11/06
- Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it, Max Reitz, 2014/11/06
- Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it, Jeff Cody, 2014/11/06
- Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it, Max Reitz, 2014/11/06
- Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it, Markus Armbruster, 2014/11/07
- Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it, Max Reitz, 2014/11/07
- Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it, Markus Armbruster, 2014/11/10
- Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it, Markus Armbruster, 2014/11/07
- Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it,
Kevin Wolf <=
- Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it, Markus Armbruster, 2014/11/07
Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it, Gerd Hoffmann, 2014/11/05
Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it, Eric Blake, 2014/11/05
Re: [Qemu-devel] Image probing: how it can be insecure, and what we could do about it, Kevin Wolf, 2014/11/05