From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42842) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xlw7W-0006Qv-60 for qemu-devel@nongnu.org; Wed, 05 Nov 2014 03:39:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xlw7P-0004uQ-KJ for qemu-devel@nongnu.org; Wed, 05 Nov 2014 03:39:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43085) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xlw7P-0004uK-DQ for qemu-devel@nongnu.org; Wed, 05 Nov 2014 03:39:07 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sA58d6uj025643 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 5 Nov 2014 03:39:06 -0500 From: Markus Armbruster References: <5452001E.9070907@redhat.com> <20141030092722.GB30746@stefanha-thinkpad.redhat.com> <20141030093635.GB9097@noname.str.redhat.com> <20141031112423.GE10332@stefanha-thinkpad.redhat.com> <20141031115639.GD4496@noname.str.redhat.com> <878ujs1x6y.fsf@blackfin.pond.sub.org> <20141103102510.GB4437@noname.str.redhat.com> <20141103150533.GC4609@stefanha-thinkpad.redhat.com> <20141104101133.GB4119@noname.redhat.com> <20141104152544.GA28330@stefanha-thinkpad.redhat.com> <20141104153715.GG4119@noname.redhat.com> Date: Wed, 05 Nov 2014 09:39:03 +0100 In-Reply-To: <20141104153715.GG4119@noname.redhat.com> (Kevin Wolf's message of "Tue, 4 Nov 2014 16:37:15 +0100") Message-ID: <87d292ujmg.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: jcody@redhat.com, qemu-devel@nongnu.org, Stefan Hajnoczi , Max Reitz Kevin Wolf writes: > Am 04.11.2014 um 16:25 hat Stefan Hajnoczi geschrieben: >> On Tue, Nov 04, 2014 at 11:11:33AM +0100, Kevin Wolf wrote: >> > Am 03.11.2014 um 16:05 hat Stefan Hajnoczi geschrieben: >> > > The argument that there might not be a traditional filename doesn't make >> > > sense to me. When there is no filename the command-line is already >> > > sufficiently complex and usage is fancy enough that probing adds no >> > > convenience, the user can just specify the format. >> > >> > -hda nbd://localhost >> > -drive file=nbd://localhost,format=raw >> > >> > Almost double the length, and I don't see anything fancy in the first >> > line. >> > >> > > Anyway, does this sound reasonable: >> > > >> > > In QEMU 3.0, require the format= option for -drive. Keep probing the >> > > way it is for non-drive options because they are used for convenience by >> > > local users. >> > >> > And being hacked while using -hda is better in which way? >> >> Markus is proposing that we look at the filename extension. In that >> case QEMU cannot be tricked by the contents of a raw image. >> >> That makes -hda perfectly safe although there are cases where QEMU >> doesn't know what to do and requires format=. > > Wait, by "keep probing the way it is" you mean implementing one of the > other proposals? So you're only suggesting being stricter on -drive as > an additional measure? > >> I do worry that changing QEMU's probing behavior drastically can lead to >> consistencies where libvirt does its own probing :(. Haven't thought >> through the bug scenarios but that could be a security problem in >> itself. > > Hm... In which cases does libvirt probe the image format? And is it even > consistent with qemu today? I had a quick look at the source. Eric, please correct misunderstandings. Enumation type virStorageFileProbeFormat enumerates supported formats. It has pseudo-formats VIR_STORAGE_FILE_AUTO, VIR_STORAGE_FILE_AUTO_SAFE. I don't understand VIR_STORAGE_FILE_AUTO_SAFE offhand. VIR_STORAGE_FILE_AUTO means probing. Its use appears to be deprecated. Actual probing happens in virStorageFileProbeFormatFromBuf(): For all formats: if magic and version match, pick this format If some magic matched, but not the version: warn For all formats: if file name extension matches, pick this format Pick raw. The formats' magic, version and extension are defined in fileTypeInfo[]. If I remember correctly, libvirt has its own probing because running an external program just to probe is too slow. Another reason for having its own probing might be providing a secure replacement for QEMU's insecure probing. > If you can get libvirt to explicitly pass the wrong format=... option > because it did its own probing, we have a problem no matter what we > change in qemu. Yes, but that would be a libvirt problem. No excuse for us to ignore our own problems.