On 10/28/2014 12:29 PM, Jeff Cody wrote: >>> This patch is RFC because of open questions: >>> >>> * Should tools warn, too? Probing isn't insecure there, but a "this >>> may pick a different format in the future" warning may be >>> appropriate. >> >> Yes. For precedent, libvirt can be considered a tool on images for >> certain operations, and libvirt has been warning about probing since 2010. >> > > I think at least the invocation 'qemu-img info' should be exempt from > the warning; doing a format probe is arguably part of its intended > usage. > >> Also, while I agree that any tool that operates on ONLY one layer of an >> image, without ever trying to chase backing chains, can't be tricked >> into opening wrong files, I'm not sure I agree with the claim that >> "probing isn't insecure" - >> > > Maybe we should draw the distinction at tools that write data? > Without a guest running, a tool that simply reads files should be safe > to probe.
Misprobing a top-level raw file as qcow2 can result in opening and reading a backing file, even when the top-level file was opened with read-only intent. If the guest can stick some sort of /proc filesystem name as a qcow2 backing file for interpretation for a bogus probe of a raw file, you can result in hanging the process in trying to read the backing file. Even if you aren't leaking data about what was read, this could still possibly constitute a denial of service attack. I was about to propose these two rules as something I'd still feel more comfortable with: if it is the top-level file, then warn for read-write access doing a probe where the probe differed from filename heuristics, be silent for read-only access doing a probe (whether or not the file claims to have a backing image) if it is chasing the backing chain (necessarily read-only access of the backing), then warn if the backing format was not specified and the probe differs from filename heuristics But that still has the drawback that if the backing file is some /proc name that will cause the process to hang, you don't want to print the message until after you read the file to discover that the probe differed from heuristics, but it is doing the open/read that determines the hang. So I don't see an elegant way to break the chicken-and-egg problem. >> What happens if more than one format tends to pick the same extension? >> For example, would you consider '.qcow' a typical extension for qcow2 >> files, even though it would probably match the older qcow driver first?... >> > > I think this could arguably end up being the case for VHD and VHDX > (i.e., both using .vhd). > > I guess the question is, in the case of common extensions, should the > priority be on the probe, or on the extension? With the way the code > is written, the priority is all going to depend on the order the > driver is registered, so it may or may not warn. Technically, don't we correctly probe both VHD and VHDX files? It is only files that start out raw and later get mis-probed as non-raw where we have an issue, so I'd rather treat the probe as accurate if it matches a common extension for that format, and NOT treat the extension as dictating a single required format. > > Currently, the code does a format probe, does an independent extension > lookup, and checks if the two agree. Instead, would it be sufficient > to do the format probe, and then just verify the detected driver has a > compatible extension name? Yes, that was what I was thinking as well. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature