[libvirt] [PATCH RFC 00/16] Add support for LUKS encrypted devices

[Daniel P. Berrange]

On Tue, Jun 07, 2016 at 02:34:39PM -0400, John Ferlan wrote:
> 
> 
> On 06/07/2016 12:01 PM, Daniel P. Berrange wrote:
> > On Tue, Jun 07, 2016 at 10:45:29AM -0400, John Ferlan wrote:
> >> Patches 1-3 were posted separately:
> >>
> >> http://www.redhat.com/archives/libvir-list/2016-June/msg00256.html
> >>
> >> But perhaps seeing the final direction will make things more clear as
> >> to why a "real" flag system wasn't used and keeping the current paradigm
> >> of constant value returns still works just fine.
> >>
> >> Patches 4-5 were posted separately:
> >>
> >> http://www.redhat.com/archives/libvir-list/2016-June/msg00091.html  (4)
> >> http://www.redhat.com/archives/libvir-list/2016-June/msg00094.html  (5)
> >>
> >> Although at one point patch 4 had an ACK:
> >>
> >> http://www.redhat.com/archives/libvir-list/2016-May/msg02115.html
> >>
> >> It wasn't clear if the more recent review rescinded that, so it still
> >> remains "in the list". I understand the concern about adding secret to
> >> cfg.mk checking, but without a better idea of how to handle - I left
> >> things as they were.
> >>
> >> Patches 6-16 are all new. Some parts are separable, but rather than continue
> >> piecemeal I just figured going with an RFC will at least 
> >>
> >> Patch 6 is only there to "prove" that using the current encryption paradigm
> >> XML still works, although if I've read the tea leaves correctly, the qemu
> >> support isn't working as desired/expected.
> >>
> >> Patch 7 adds "usage" as an XML attribute for encryption and the associated
> >> tests with that. I've chosen to "reuse" the <encryption> XML element rather
> >> than inventing something new.  I'm not opposed to something new, but let's
> >> decide up a name quickly...
> >>
> >> Patch 8-9 adds the ability for the storage backend to create/recognize a
> >> luks volume
> >>
> >> Patches 10-13 adds support for luks encryption in the storage backend.
> >> The new "<secret>" format uses "luks" as the usage type and "<key>" as
> >> the 'name'. If those names cause angst, I'm fine with changing, but just
> >> give a better suggestion!  Adding <cipher> and <ivgen> were a result of
> >> using qemu constructs from qemu commit id '3e308f20'. Since we are parsing
> >> something new, I figure failing in the domain parse code for this new type
> >> was acceptible as opposed to some post processing check.
> >>
> >> Patches 14-16 adds support for luks encryption to the domain using
> >> <encryption type='luks'... <secret format='key' usage/uuid='xxx'>>
> >>
> >> I've tested using a "good" and "bad" password and got the expected results
> >> for starting a domain.  I did not add 'virsh vol-create-as' support just
> >> yet. I figured that would be less to go back and redo if the names of
> >> elements changes.  I've also run the changes through Coverity with no
> >> new issues detected.
> >>
> >> The whole series is a result of the following bz:
> >>
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1301021
> > 
> > BTW, one of the core reasons for the LUKS support in QEMU is to facilitate
> > use of LUKS with non-local-file based disks. eg, LUKS over RBD, or LUKS
> > over NBD or LUKS over iSCSI, etc.
> > 
> > The diffstat for the files shows you've wired up luks-over-plain-file
> > ok, but I'm wondering if the code posted here is also dealing with the
> > broader support, or if that's something to be addressed later.
> > 
> > No big deal either way, particularly since qemu-img can't actually
> > create LUKS volumes on anything other than plain files/block devs
> > yet.
> > 
> 
> Hmm, right, sigh, but I had to start somewhere. So rather than be
> either/or in qemuDomainSecretDiskPrepare and qemuBuildDriveStr, there
> would need to be a way to have multiple 'secinfo' structs per disk or
> maybe one for <auth> and one for <encrypt>. Can we predict the future
> and go with a list or just two secinfo's...  Any preference?

I'd suggest just two secinfos. Then again if we have a chain of backing
files that are encrpted or need authentication - of course I don't think
our XML lets us deal with that yet anyway.

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 :|