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

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Am 04.11.2014 um 10:36 hat Markus Armbruster geschrieben:
>> Kevin Wolf <address@hidden> writes:
>> 
>> > Am 31.10.2014 um 23:45 hat Eric Blake geschrieben:
>> >> On 10/30/2014 06:49 AM, Markus Armbruster wrote:
>> >> 
>> >> > You either have to prevent *any* writing of the first 2048 bytes (the
>> >> > part that can be examined by a bdrv_probe() method, or your have to
>> >> > prevent writing anything a probe recognizes, or the user has to specify
>> >> > the format explicitly.
>> >> > 
>> >> > If you do the former, you're way outside the realm of "theoretical".
>> >> > 
>> >> > If you do the latter, the degree of "theoreticalness" depends on the
>> >> > union of the patterns you prevent.  Issues:
>> >> > 
>> >> > 1. Anthony's method of checking a few known signatures is fragile.  The
>> >> > only obviously robust way to test "is probing going to find something
>> >> > other than 'raw'?" is running the probes.  Feasible.
>> >> > 
>> >> > 2. Whether the union of patterns qualifies as "theoretical" for all our
>> >> > targets is not obvious, and whether it'll remain "theoretical" for all
>> >> > future formats and target machines even less.
>> >> 
>> >> This one scares me.  The proof of concept patch you posted tests whether
>> >> a write to the first sector would result in the sector matching a
>> >> _currently known probe_ for the file formats that were compiled in at
>> >> configure time to the currently running binary.  But this is NOT the set
>> >> of all possible binary formats that may be introduced in the future.  By
>> >> extrapolation, if we pursue write blocking, then the only SAFE way to is
>> >> to reject ALL writes to the first sector, because we can't know which
>> >> writes will match some theoretical future probe - but by the time you
>> >> get to that point, then we no longer match bare metal (rejecting ALL
>> >> writes to the first sector is ridiculous).
>> >
>> > There is no absolute security without forbidding raw. Who says that the
>> > next format doesn't have its magic in sector 42?
>> 
>> Correct.
>> 
>> > You are right that if an attacker knows which new format supporting
>> > backing files we'll have in the next version, and the user uses probed
>> > raw despite the warning (which means they are not using libvirt), the
>> > attacker can write the header of the new image format now and hope that
>> > the user leaves it alone until the update happens at some point in the
>> > future. Then the malicious guest can access that one file, but not
>> > obtain access to the next one (because the new checks are in place
>> > then).
>> >
>> > I don't think this is a relevant attack vector. It's probably much
>> > easier to get the user to run an untrusted image than converting a
>> > trusted image into a time bomb and waiting potentially for months or
>> > years for it to explode.
>> 
>> The threat from using untrusted images with embedded filenames is real.
>> 
>> Regardless, I wouldn't discount the (different) threat from guests
>> exploiting "future" probes so cavalierly.  If your host is running a
>> stable distro, its future is easy to know.  Even the point of time when
>> it gets upgraded can be predictable.
>> 
>> In general, I recommend against leaving security holes in place just
>> because you think nobody could be bothered to exploit them :)
>> 
>> Security is not an absolute that cannot be trumped by any other
>> consideration.  I'm perfectly happy to consider usability and
>> compatibility issues, as well as implementation complexity.  I'm much
>> less willing to accept a "come on, nobody is going to try *that*"
>> argument.
>
> No, I didn't mean to make a "come on, nobody is going to try *that*"
> argument.
>
> I just can think of more attacks that we won't avoid with either
> approach. For example, what if another program on the host is
> exploitable and you just need a file with the right content to attack
> it? A raw image gives you what you need. Nobody is going to try *that*
> then, if you're content with allowing this?

When I give a raw image to an untrusted guest, it's entire contents
becomes untrusted.  I should treat it with the same caution as any other
untrusted data.

This is not the only way to put files of untrusted data on your system.
That it is one may be a bit more surprising than for downloading tools,
though.

We can't stop users from using untrusted data carelessly.

What we *can* do is making it easy for users to treat untrusted data
with the proper care as far as the software we're producing is
concerned.

The most obvious and simple way to use images with our software does not
treat untrusted data with the proper care.  This is also the way our
documentation advertises.

Worse, exercising proper care *consistently* is hard.  I couldn't use
raw images securely in all contexts myself, not without a lot more
staring at our source code.  And even then I wouldn't bet my own money
on it.  Libvirt has had a hard time doing it, with multiple failures,
including recent ones.

> The only full solution is forbidding raw. We have already agreed that
> this isn't an option. Now we have to draw a line somewhere.

I'm drawing the line around the software we actually maintain.  Make it
treat raw image contents as untrusted unless told otherwise by the user.
If somebody else's software doesn't, then that's regrettable, but we
can't fix that in QEMU.

Since you've been attacking this line vigorously, I'm drawing a second
line behind it, in bright red: secure usage of our software should not
be hard.

You can make me sacrifice my "secure by default" requirement (against my
better judgement), but you can't make me compromise on my "secure should
not be hard" requirement.