On 25/03/2015 09:10, Markus Armbruster wrote:
>> Based on my reading of the code, libvirt will actually ignore the
>> allow_disk_format_probing setting, and not do anything about the format
>> when driving such an old QEMU.  By contrast, if you specify a format and
>> libvirt invokes an old qemu-nbd without --format, libvirt fails hard.
>> That's already CVE worthy, isn't it?
>>
>> So I think an option like this is premature.  libvirt should _first of
>> all_ ensure that it completely abides by the allow_disk_format_probing
>> setting, including refusing to drive old QEMUs when format probing is
>> disabled.  Once libvirt is consistent within itself, we can talk about
>> what help QEMU can provide.
> 
> If we had a "no format probing" option in qemu-nbd (not in my patch, but
> that's a flaw in the patch, not the idea), then libvirt developers
> would've put it to use right away, and then their insecure usage
> would've failed hard, making it immediately obvious.

It wouldn't have fixed the bug in _old QEMU without -drive_ (and thus
without the option!), where allow_disk_format_probing does nothing.
Which is a vulnerability!  I noticed now that Eric is not CCed, adding him.

qemu-nbd invocations always require an explicit disk format in libvirt,
and fail hard on old qemu-nbd.

In the end we have:

- one bug for old QEMUs, so libvirt would not use the option

- one case where libvirt is stricter and requires a format, so the
option does not add anything

> I agree secure usage is currently is too hard for casual command line
> use.  Unfortunately, it has proven too hard even for sophisticated
> programmatic users like libvirt.

I disagree.  Libvirt has been doing well.  Hard---yes; but not too hard.

> When we discussed security issues with probing last year, my first line
> of defense was "the default should be secure".  Overwhelming opposition
> from the backward compatibility camp forced me to retreat from that line
> to my second line of defense "secure shouldn't be hard".  That line
> needs to be held at all costs :)

I agree.  Making the default secure is difficult because the old-style
options (-hda, -cdrom, etc.) are still good for casual use.  When I'm
boot-testing 20 different QEMUs to check that a series is bisectable,
it's generally okay to use IDE disks and cache=writeback, and it also
shouldn't matter if my image is raw or qcow2.

And making "secure" as easy as "--security on" is a worthwhile goal indeed.

That said, I'm not sure it's attainable.  For example, most downloadable
images are provided in qcow2 format.  The question then is: if users
can't be bothered to use -drive format=raw all the time, why would they
be bothered to use -drive format=qcow2 instead?

Users want security _and_ DWIM.  Anvil, meet hammer.

>>                                                         But I still
>> maintain that for libvirt this is basically security theater, and the
>> priorities are others.
> 
> To be honest, I find your use of "security theater" offensive.
> 
> I'd readily accept the moniker if the feature would provide libvirt
> developers an excuse not to fix their bugs, or reduce the incentives to
> fix their bugs.

I say it again: libvirt should first be consistent within itself.
Either it should drop support for old QEMUs, or it should be audited to
ensure that old QEMUs are invoked securely.

Once old QEMUs are deemed okay, we can add smarties to new QEMUs.  If
you don't do it in this order, the smarties indeed reduce the incentive
to fix bugs.

Also, I'm calling it security theater because on the surface the option
seems to provide a mitigation strategy, but in the end doesn't provide
much.  Because...

> But the feature in fact does the opposite: it makes such bugs blatantly
> obvious and release-blocker-painful.

... they are only obvious inasmuch as there is a testcase for "no
explicitly specified format".  Otherwise, you have a hidden failure for
new QEMU and a vulnerability for old QEMU, so you don't gain anything.

Problem is, the same person write code and testcases.  If you are smart
enough to add testcases that catch possible vulnerability, you probably
are already catching it in the code.  (And the testcase is a way of
praising yourself for your forethought. :)).

Paolo

Reply via email to