From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40169) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaKL9-0007e6-In for qemu-devel@nongnu.org; Tue, 24 Mar 2015 04:37:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YaKL1-0001m7-Se for qemu-devel@nongnu.org; Tue, 24 Mar 2015 04:37:35 -0400 Sender: Paolo Bonzini Message-ID: <55112241.7000300@redhat.com> Date: Tue, 24 Mar 2015 09:37:21 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1426856744-18750-1-git-send-email-armbru@redhat.com> <1426856744-18750-2-git-send-email-armbru@redhat.com> <550C21DB.8000607@redhat.com> <87h9tf7pbi.fsf@blackfin.pond.sub.org> <550C2577.2010201@redhat.com> <550C26FE.6010807@redhat.com> <87384z4uq4.fsf@blackfin.pond.sub.org> <55104C17.80407@redhat.com> <551051E1.2030603@redhat.com> <55105283.9070105@redhat.com> <87sicvfov1.fsf@blackfin.pond.sub.org> In-Reply-To: <87sicvfov1.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH RFC for-2.3 1/1] block: New command line option --no-format-probing List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: stefanha@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org, Max Reitz On 23/03/2015 21:19, Markus Armbruster wrote: > Paolo Bonzini 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? :) > >