From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60151) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XlJJf-00075s-Qm for qemu-devel@nongnu.org; Mon, 03 Nov 2014 10:13:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XlJJZ-0004g2-LF for qemu-devel@nongnu.org; Mon, 03 Nov 2014 10:13:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60877) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XlJJZ-0004fy-Et for qemu-devel@nongnu.org; Mon, 03 Nov 2014 10:13:05 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sA3FD4XY011324 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Mon, 3 Nov 2014 10:13:04 -0500 Message-ID: <54579B7E.4000605@redhat.com> Date: Mon, 03 Nov 2014 16:13:02 +0100 From: Max Reitz MIME-Version: 1.0 References: <1414512220-19058-1-git-send-email-armbru@redhat.com> <1414512220-19058-3-git-send-email-armbru@redhat.com> <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> In-Reply-To: <20141103150533.GC4609@stefanha-thinkpad.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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: Stefan Hajnoczi , Kevin Wolf Cc: jcody@redhat.com, Markus Armbruster , qemu-devel@nongnu.org On 2014-11-03 at 16:05, Stefan Hajnoczi wrote: > On Mon, Nov 03, 2014 at 11:25:10AM +0100, Kevin Wolf wrote: >> Am 03.11.2014 um 09:54 hat Markus Armbruster geschrieben: >>> Kevin Wolf writes: >>> >>>> Am 31.10.2014 um 12:24 hat Stefan Hajnoczi geschrieben: >>>>> On Thu, Oct 30, 2014 at 10:36:35AM +0100, Kevin Wolf wrote: >>>>>> Am 30.10.2014 um 10:27 hat Stefan Hajnoczi geschrieben: >>>>>>> The guest may legitimately use raw devices that contain image format >>>>>>> data. Imagine tools similar to libguestfs. >>>>>>> >>>>>>> It's perfectly okay for them to lay out image format data onto a raw >>>>>>> device. >>>>>>> >>>>>>> Probing is the problem, not putting image format data onto a raw device. >>>>>> Agreed, that's why any restrictions only apply when probing was used to >>>>>> detect a raw image. If you want to do anything exotic like storing a >>>>>> qcow2 image for nested virt on a disk that is a raw image in the host, >>>>>> then making sure to pass format=raw shouldn't be too much. >>>>> Because at that point the solution is way over-engineered. >>>>> >>>>> Probing checks should be in the QEMU command-line code, not sprinkled >>>>> across the codebase and even at run-time. >>>>> >>>>> Isn't Markus approach much simpler and cleaner? >>>> I don't think so. My code isn't "sprinkled across the codebase", it has >>>> the checks right where the problem arises, in the raw block driver. >>> Actually, that's not where the problem is. >>> >>> The guest writing funny things to its disks is *not* the problem, it's >>> perfectly normal operation. Probing untrusted images is the problem! >> I think we all know it, but let's clearly state the real problem once >> again to remind us what we're trying to do on a high level. The security >> problem we're talking about is that some image formats can contain file >> names, and the content of these files can be exposed (in some cases even >> r/w) to the guest. > That's too specific. There are other cases: > > Imagine you are paying for a 10 GB disk KVM virtual server with a > hosting provider. > > The hosting provider did not configure KVM correctly and they use > probing. You can write a qcow2 header with a 100 GB disk size to the > raw disk and reboot the VM. > > Suddenly your guest has access to 100 GB of disk space even though you > are only paying for 10 GB! > >> Therefore not probing, but using untrusted images is the root problem - >> but it's not one that we can fix. If the users downloads (or otherwise >> obtains) an untrusted image with a bad backing file name, and of course >> the correct file extension, we don't have a chance to protect them. > This is driving the discussion into the wrong direction. You are right > that QEMU cannot solve that but it's also a well-known problem that > OpenStack and co solved a long time ago. > >> What we can do something about is a special case of this, where a >> malicious guest tries to turn a (trusted) image into something that >> isn't trustworthy any more. In my book, the bad action is modifying the >> image so that it looks like a different format with a bad header, it's >> not treating an image file like what it looks like. If you disable >> probing in qemu, you still allow qemu guests to write bad headers that >> can fool other tools. >> >> The problem really isn't all the non-raw formats that have a header >> which can contain file names. The problem is with raw which doesn't have >> a header and allows the guest to turn its image file into any other file >> type it wants and can potentially trick the user. raw is really a bad >> format (or actually, it's a non-format), and if I could choose I >> wouldn't support it, or at least only r/o. We should instead have used a >> format with a header and then the raw file shifted by some offset. It's >> too late for that, though. >> >> But coming back to your specific statement: Probing images isn't the >> problem. The guest writing funny things isn't the problem either. Using >> a format that can't reliably represent those funny things is the >> problem. And if the format can't reliably represent something, it should >> return an error when you try to do it anyway. If you need the feature of >> storing funny things, use a format that supports it reliably. (And >> calling explicit format=raw reliable isn't quite right either, but it's >> a compromise I'd be willing to make.) > I don't follow this logic. You end up with "raw is bad because it > doesn't have a header". That doesn't get us anywhere. It's true anyway. Just because an argument is not constructive does not mean it's bad, as long as it's true. > (There are other formats that can also be confused. For example, you > can create a qcow2 file that is also a vpc file because vpc allows files > that have only a footer.) Then vpc is bad, too. > Fact remains that you must not probe untrusted guest disk images. Nobody wants to completely abolish probing. Markus only wants to use the extension as a second source of information, basically. > 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. Apropos "the cloud provider did not configure KVM correctly"? > 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. To me it does, yes. Max