On 23/03/2015 21:19, Markus Armbruster wrote:
Paolo Bonzini pbonz...@redhat.com writes:
On 23/03/2015 18:48, Eric Blake wrote:
Why can't libvirt just add ,format=raw instead of leaving out the
format key altogether?
Libvirt DOES add format=raw. This patch is an extra insurance
policy to guarantee that libvirt does not have any code paths that
omit the explicit format (as we have had a couple of CVEs in
libvirt over the years where that was the case).
And where's the extra insurance policy to guarantee that QEMU does not
have any code paths that ignore the new command line option?
Right here (in the non-RFC patch, not this one):
@@ -751,6 +752,11 @@ static int find_image_format(BlockDriverState *bs, const
char *filename,
return ret;
}
+if (bdrv_image_probing_disabled) {
+error_setg(errp, Format not specified and image probing disabled);
+return -EINVAL;
+}
+
ret = bdrv_pread(bs, 0, buf, sizeof(buf));
if (ret 0) {
error_setg_errno(errp, -ret, Could not read image for determining
its
The option sets bdrv_image_probing_disabled in a straightforward manner,
and bdrv_image_probing_disabled guards the probing code in an equally
straightforward manner.
But what about migration from newer to older QEMU? Libvirt even
supports QEMU versions where the only way to specify disks is -hda
XYZ, so it is _impossible_ to honor the format=raw specifier.
Also, libvirt can start qemu-nbd and doesn't force format=raw in that
case. So the protection is far from complete. This reinforces my
opinion that the false sense of safety provided by this patch is worse
than the insurance against future CVEs (also, have there been any
actual libvirt CVEs about this after 2010? near misses don't count IMHO).
How to get p0wned anyway:
1. Use your raw image with format=raw. Malicious guest writes qcow2
header.
2. Use the image again without a format. Probe returns qcow2, no
warning is printed.
Plausible attack? Maybe not. Worth stopping cold anyway? I think so,
as long as it's easy.
My new option is a different kind of mitigation. It's for users who
want more airtight protection, prefer writes to sector 0 just work,
funny or not, and are willing to always specify the format. At least
one such user exists: libvirt.
Without this patch:
* Alternate use of raw images with and without format is still insecure;
Kevin's mitigation can't protect you then.
That's PEBKAC.
Paolo
* If you somehow miss specifying a format, and probing detects raw, you
get a warning on stderr. If your guest writes something funny to
sector 0, the write fails.
With this patch:
* If you somehow miss specifying a format, open fails. This is what
libvirt wants.
There certainly hasn't been enough discussion
for this to get into 2.3.
Isn't that what were doing now? :)
