Max Reitz <mre...@redhat.com> writes: > On 2014-11-03 at 09:54, Markus Armbruster wrote: >> Kevin Wolf <kw...@redhat.com> 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 don't think making your hard disk a qcow2 image is a perfectly > normal operation, but perhaps that's just me. > > I still think writing to sector 0 is not a perfectly normal operation, > but rarely ever happens (at least for x86[1]), and if it does, there's > not that much variety to what is written there. > > [1] I'm open for arguments about how on other platforms there's not > necessarily a boot loader in sector 0 of an image and the host is > therefore actually completely free to write whatever it likes and > there's no telling of what it will be. > >>> It's with Markus's approach that we'll have to have code in many >>> different places as I showed. Its fundamental assumption that there is >>> always a filename string and the filename isn't passed in some QDict >>> option is simply wrong. Specifying the image is driver-dependent and >>> therefore you'd have to add functionality to each driver in order to get >>> the filename extension (or the information that there isn't anything >>> close enough to a filename). >> The RFC patch is incomplete. Can't say how much code recording the >> filename correctly will take until I've done it. >> >> I suspect the lack of an image filename already leads to rotten error >> messages in places. >> >>> The only argument brought up so far that I can reasonably buy is that >>> in the unlikely case of the restrictions applying it may be surprising >>> for the user to see requests failing. To address this, we could print >>> a warning when an image is opened in the "restricted raw" mode. This >>> way the user knows what's going on, >> Improves the virtual hardware from "crippled" to "openly crippled", and >> my opinion on it from "I hate it" to "I hate it slightly less" :) >> >>> and at the same time we still >>> effectively protect them instead of only printing a warning without real >>> protection. >> This isn't real protection, either. >> >> If the user runs with format=raw, the guest can write whatever it likes, >> and that's a feature. If the user forgets format=raw just once, he's >> p0wned. That's because your "protection" does not address the real >> problem (probing of untrusted images), > > Well, that's your definition of the real problem. I think making a raw > image effectively a non-raw image (according to what file(1) says) is > a real problem as well. Detecting an image as qcow2 when file(1) says > it is indeed qcow2 is not that much of a problem, in my very humble > opinion. > > Probably the real real problem is that qemu supports raw images at > all, in contrast to other VM software (which support it for CD and > floppy at the most). Hey, let's just add a flat mode to qcow2 and get > rid of raw altogether! (that was a joke) > >> but tries to prevent it by >> refusing to created "bad" untrusted images, but can do so only in >> special configurations, and not at all for images written by something >> else. >> >> Unlike your proposal, mine does attack the real problem, and thus *can* >> provide real protection. > > It's what you define to be the real problem. I for one think the > definition of what a raw image is is the real problem. When does a raw > image cease to be a raw image? My definition would be that it ceases > to be raw when sufficiently complex probing detects another > format. Your definition seems to be that it's always raw when the user > wants it to be raw, and sometimes the user doesn't know that he/she > wants it to be raw (because of legacy issues). > > Also, I think it's perfectly valid to prevent a guest from writing > things to the first sector which this sufficiently complex probing(TM) > detects to be some image format. You don't think so. That's why you > say probing is the real problem, whereas I say allowing the guest to > write a fake image header is the real problem (not the least because > it may fool other tools as well). > > So for me, your patch only mitigates the problem but does not fix > it. Mitigating it is better than doing nothing, however, which is why > I'm fine with it.
You're right, the *root* problem is the raw vs. other image formats ambiguity. This is an instance of the general raw data file vs. structured data file ambiguity. Whether a file contains raw data or not only the user can know. Users generally keep track of what's what by giving their files suitable names. The root problem becomes a QEMU security problem only because (a) we support both raw and image formats, (b) we second-guess the user by default, and (c) our guess can be controlled by a malicious guest. We can try to fix it by attacking one or more of the three conditions: (a) Outlaw raw images. Nobody is seriously suggesting that. (b) Make the second-guessing opt-in rather than opt-out. This is my approach. (c) Prevent the guest from affecting the second-guessing. This is Kevin's approach. I'll discuss these in more detail in a separate message. >> Not immediately because we want to avoid >> aprupt change, but eventually. Here's how: >> >> 1. Initially, we warn on insecure usage. This doesn't protect users, >> but it guides them towards protecting themselves. >> >> 2. Eventually, we make the warning an error. This *does* protect users, >> regardless of how they use or have used QEMU, *except* when they >> explicitly ask for insecure probing with format=insecure-probe (assuming >> we provide that option).
