"Daniel P. Berrange" <berra...@redhat.com> writes: > On Tue, Mar 10, 2015 at 06:26:38PM +0100, Markus Armbruster wrote: >> RFC because the series only covers open [PATCH 1], but not create. >> Also missing: make qemu-img print a warning when it creates an >> encrypted image. Finally, some of the material in the cover letter >> should be worked into the commit messages. >> >> 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. > > >> = Usability issues = > >> == Confusing startup == >> == Incorrect passwords not caught == >> == Need to stop the guest to add an encrypted image == >> == Use without key is not always caught == >> == QMP device_add of usb-storage fails when it shouldn't == > > So there's really two separate root cuase problems we're facing > here. One of the usability issues is inherant artifact of the > qcow design. The other 4 issues are all due to the badly designed > block driver / monitor key management approach.
Yup. The latter is fixable, but not worth fixing as long as the underlying crypto isn't worth using. >> 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 drop the mess and move on. Keep qemu-img convert working, of >> course, to let users rescue their data. > > Once I've got through the current work i'm doing on TLS support > for migrate/nbd/chardev/etc, my intention is to work on adding > support for the LUKS format to QEMU. We really need this natively > in OpenStack since we're increasingly using the QEMU native client > for nbd, iscsi, nfs, etc but at the same time don't want to sacrifice > encryption which we currently do via LUKS. It is probably a good > 4-6 months though before I get on to working on this. Great! > I agree with all your points about the usability being fubar. This > clearly needs to be fixed for encryption support to be viable in > QEMU, regardless of the actual encryption format used. > > I guess my question is whether it is worth trying to fix the blockdev > integration part of things now, or to rip it out now and reimplement > it from scratch later ? I think I probably agree with killing it > now, since it might actually make doing a sensible impl easier later > on. In your shoes, I'd certainly prefer to drop the current mess and start over. Not just because I think it'll make the work easier, but also because I'd want to break out of the back-compat strait-jacket, to be free to create the best user interface I can. > And lets assume we do eventually have a fixed blockdev layer and a > sane LUKS encryption driver, would we still want to kill off qcow2 > encryption ? Given the way subpar encryption is being actively > attacked by everyone & their dog, I think mandatory retirement of > qcow2 encryption is a good idea sooner rather than later. I strongly believe retiring it is a public service. Except for qemu-img. I'm prepared to keep it there for a long time, maybe forever. > My only concern here is whether we've given users enough prior > warning. While we added that doc change a year ago, what are the > odds that anyone has actually read those docs & noticed the warning. > Should we have one major release where we log a deprecation warning > on stderr, informing users of an explicit timeframe for its removal, > before we actually use the big hammer of disabling it permanently ? I'm fine with that. In fact, I could agree to pretty much any deprecation schedule, as long as we start it *now*. > FWIW, I could see an improved interaction scheme working as follows > > First, introduce a new monitor command for setting named passwords, > > add_key mykey1 SECRETDATA > > Now, extend the blockdev_add so that you can provide key names > by adding > > 'keyname': 'mykey1' > > as a parameter in the json args. Can you explain why that's better than sticking 'key': SECRETDATA right into blockdev-add's arguments? > If an attempt is made to add a blockdev without having provided a > key, the attempt should just fail. This avoids all the insanity > around delayed opening of files, as well as avoiding need to stop > the guest to add devices. Yes, we need to exterminate the awkward and dangerous intermediate state "image opened, but lacks decryption key". This series avoids it (except in qemu-img, but there it's safely confined to img_open()). Future work should not bring it back. > For cold plug, have a command line arg '--add-keys prompt' to > indicate the user should be prompted on TTY to enter keys, which > is good for interactive usage. For managed usage we could allow > '--add-keys fd=FDNUM' and just read keys from the file descriptor. I guess sugared command line like "-hda geheim.qcow2" should simply prompt for the key right in the tty. None of this nonsense with not starting the guest, and having "cont" prompt in the monitor.
