From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43323) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YXSF1-0000Ou-67 for qemu-devel@nongnu.org; Mon, 16 Mar 2015 06:27:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YXSEx-0002sl-LT for qemu-devel@nongnu.org; Mon, 16 Mar 2015 06:27:23 -0400 Date: Mon, 16 Mar 2015 10:27:13 +0000 From: "Daniel P. Berrange" Message-ID: <20150316102701.GE10189@redhat.com> References: <1426277380-25665-1-git-send-email-armbru@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1426277380-25665-1-git-send-email-armbru@redhat.com> Subject: Re: [Qemu-devel] [PATCH] block: Deprecate QCOW/QCOW2 encryption Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, qemu-block@nongnu.org, qemu-devel@nongnu.org, stefanha@redhat.com On Fri, Mar 13, 2015 at 09:09:40PM +0100, Markus Armbruster wrote: > We've steered users away from QCOW/QCOW2 encryption for a while, > because it's a flawed design (commit 136cd19 Describe flaws in > qcow/qcow2 encryption in the docs). > > In addition to flawed crypto, we have comically bad usability, and > plain old bugs. Let me show you. > > = Example images = > > I'm going to use a raw image as backing file, and two QCOW2 images, > one encrypted, and one not: > > $ qemu-img create -f raw backing.img 4m > Formatting 'backing.img', fmt=raw size=4194304 > $ qemu-img create -f qcow2 -o encryption,backing_file=backing.img,backing_fmt=raw geheim.qcow2 4m > Formatting 'geheim.qcow2', fmt=qcow2 size=4194304 backing_file='backing.img' backing_fmt='raw' encryption=on cluster_size=65536 lazy_refcounts=off > $ qemu-img create -f qcow2 -o backing_file=backing.img,backing_fmt=raw normal.qcow2 4m > Formatting 'normal.qcow2', fmt=qcow2 size=4194304 backing_file='backing.img' backing_fmt='raw' encryption=off cluster_size=65536 lazy_refcounts=off > > = Usability issues = > > == Confusing startup == > > When no image is encrypted, and you don't give -S, QEMU starts the > guest immediately: > > $ qemu-system-x86_64 -nodefaults -display none -monitor stdio normal.qcow2 > QEMU 2.2.50 monitor - type 'help' for more information > (qemu) info status > VM status: running > > But as soon as there's an encrypted image in play, the guest is *not* > started, with no notification whatsoever: > > $ qemu-system-x86_64 -nodefaults -display none -monitor stdio geheim.qcow2 > QEMU 2.2.50 monitor - type 'help' for more information > (qemu) info status > VM status: paused (prelaunch) > > If the user figured out that he needs to type "cont" to enter his > keys, the confusion enters the next level: "cont" asks for at most > *one* key. If more are needed, it then silently does nothing. The > user has to type "cont" once per encrypted image: > > $ qemu-system-x86_64 -nodefaults -display none -monitor stdio -drive if=none,file=geheim.qcow2 -drive if=none,file=geheim.qcow2 > QEMU 2.2.50 monitor - type 'help' for more information > (qemu) info status > VM status: paused (prelaunch) > (qemu) c > none0 (geheim.qcow2) is encrypted. > Password: ****** > (qemu) info status > VM status: paused (prelaunch) > (qemu) c > none1 (geheim.qcow2) is encrypted. > Password: ****** > (qemu) info status > VM status: running > > == Incorrect passwords not caught == > > All existing encryption schemes give you the GIGO treatment: garbage > password in, garbage data out. Guests usually refuse to mount > garbage, but other usage is prone to data loss. > > == Need to stop the guest to add an encrypted image == > > $ qemu-system-x86_64 -nodefaults -display none -monitor stdio > QEMU 2.2.50 monitor - type 'help' for more information > (qemu) info status > VM status: running > (qemu) drive_add "" if=none,file=geheim.qcow2 > Guest must be stopped for opening of encrypted image > (qemu) stop > (qemu) drive_add "" if=none,file=geheim.qcow2 > OK > > Commit c3adb58 added this restriction. Before, we could expose images > lacking an encryption key to guests, with potentially catastrophic > results. See also "Use without key is not always caught". > > = Bugs = > > == Use without key is not always caught == > > Encrypted images can be in an intermediate state "opened, but no key". > The weird startup behavior and the need to stop the guest are there to > ensure the guest isn't exposed to that state. But other things still > are! > > * drive_backup > > $ qemu-system-x86_64 -nodefaults -display none -monitor stdio geheim.qcow2 > QEMU 2.2.50 monitor - type 'help' for more information > (qemu) drive_backup -f ide0-hd0 out.img raw > Formatting 'out.img', fmt=raw size=4194304 > > I guess this writes encrypted data to raw image out.img. Good luck > with figuring out how to decrypt that again. > > * commit > > $ qemu-system-x86_64 -nodefaults -display none -monitor stdio geheim.qcow2 > QEMU 2.2.50 monitor - type 'help' for more information > (qemu) commit ide0-hd0 > > I guess this writes encrypted data into the unencrypted raw backing > image, effectively destroying it. > > == QMP device_add of usb-storage fails when it shouldn't == > > When the image is encrypted, device_add creates the device, defers > actually attaching it to when the key becomes available, then fails. > This is wrong. device_add must either create the device and succeed, > or do nothing and fail. > > $ qemu-system-x86_64 -nodefaults -display none -usb -qmp stdio -drive if=none,id=foo,file=geheim.qcow2 > {"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": 2}, "package": ""}, "capabilities": []}} > { "execute": "qmp_capabilities" } > {"return": {}} > { "execute": "device_add", "arguments": { "driver": "usb-storage", "id": "bar", "drive": "foo" } } > {"error": {"class": "DeviceEncrypted", "desc": "'foo' (geheim.qcow2) is encrypted"}} > {"execute":"device_del","arguments": { "id": "bar" } } > {"timestamp": {"seconds": 1426003440, "microseconds": 237181}, "event": "DEVICE_DELETED", "data": {"path": "/machine/peripheral/bar/bar.0/legacy[0]"}} > {"timestamp": {"seconds": 1426003440, "microseconds": 238231}, "event": "DEVICE_DELETED", "data": {"device": "bar", "path": "/machine/peripheral/bar"}} > {"return": {}} > > This stuff is worse than useless, it's a trap for users. > > If people become sufficiently interested in encrypted images to > contribute a cryptographically sane implementation for QCOW2 (or > whatever other format), then rewriting the necessary support around it > from scratch will likely be easier and yield better results than > fixing up the existing mess. > > Let's deprecate the mess now, drop it after a grace period, and move > on. > > Signed-off-by: Markus Armbruster Reviewed-by: Daniel P. Berrange Looks good for this release, then we kill in next release. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|