RE: RBD encryption support in libvirt

["Danny Harnik" <DANNYH@xxxxxxxxxx>]

"Daniel P. Berrangé" <berrange@xxxxxxxxxx>
wrote on 09/03/2021 10:34:48:

> From: "Daniel P. Berrangé" <berrange@xxxxxxxxxx>
> To: "Danny Harnik"
<DANNYH@xxxxxxxxxx>
> Cc: libvir-list@xxxxxxxxxx,
"Or Ozeri" <ORO@xxxxxxxxxx>, pkrempa@xxxxxxxxxx
> Date: 09/03/2021 10:35
> Subject: [EXTERNAL] Re: RBD
encryption support in libvirt
> 
> On Thu, Sep 02, 2021 at 09:26:27PM +0300, Danny Harnik wrote:
> > "Daniel P. Berrangé" <berrange@xxxxxxxxxx> wrote
on 09/02/2021 15:23:14:
> > 
> > > From: "Daniel P. Berrangé" <berrange@xxxxxxxxxx>
> > > To: "Or Ozeri" <ORO@xxxxxxxxxx>
> > > Cc: libvir-list@xxxxxxxxxx, pkrempa@xxxxxxxxxx, "Danny
Harnik" 
> > > <DANNYH@xxxxxxxxxx>
> > > Date: 09/02/2021 15:23
> > > Subject: [EXTERNAL] Re: RBD encryption support in libvirt
> > > 
> > > On Thu, Sep 02, 2021 at 05:55:20AM +0000, Or Ozeri wrote:
> > > > Hi,
> > > > 
> > > > 
> > > > I wanted to get your advice on a patch I'm preparing
for libvirt.
> > > > It touches the code-path that allows using LUKS encryption
on top 
> > > of an RBD image.
> > > > 
> > > > 
> > > > We recently added LUKS and LUKS2 encryption support
in Ceph's librbd.
> > > > We exposed this in qemu in a recent patch by adding
new optional
> > > > "encrypt" member to BlockdevOptionsRbd.
> > > > This patch was included in the recent release of qemu
6.1.
> > > > To enable libvirt users to use librbd encryption, we
need libvirt
> > > > to use this new "encrypt" when it builds
the blockdev options for RBD.
> > > > 
> > > > 
> > > > The interesting question is how to define the libvirt
XML syntax that
> > > > will trigger the use of librbd encryption.
> > > > My thought was to use the already existing <encryption>
tag.
> > > > In that case, we just need to add a new format 
> > > VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2 to the enum 
> > > virStorageEncryptionFormatType.
> > > > This type will be checked in qemuBlockStorageSourceGetRBDProps.
> > > 
> > > I don't think we want to switch impls based on luks1 vs
luks2,
> > > because that will create trouble in future when/if QEMU's
native
> > > impl gains Luksv2 support. So I think we need an explicit
way
> > > to request librbd encryption, even for luks2.
> > > 
> > > > The problem with this approach is that it only works
for LUKS2.
> > > > librbd encryption also supports LUKS1.
> > > > We want to allow the user to choose between the qemu
LUKS 
> > > implementation and the librbd one.
> > > > One reason to keep support both is that on the one
hand librbd 
> > > only supports XTS mode.
> > > > On the other hand, qemu implementation will not support
a chain of
> > > uniquely encrypted RBD images (each serving as a backing
store for 
> > > the previous one).
> > > 
> > > I think I asked this before on qemu-devel, but I can't find
the
> > > answer, but can you explain the RBB backing store chain
problem ?
> > > 
> > > QEMUs' LUKS driver can be layered on top of any QEMU block
protocol
> > > driver. So if there are multiple RBD layers in the qemu
-blockdev
> > > config, we can layer LUKS over each one independantly.
> > > 
> > RBD has an implementation of clones that is similar to backing
chains in 
> > QCOW2, One can create a thinly provisioned clone of a snapshot
using this 
> > mechanism. However the distinction between parent (source snapshot)
and 
> > child (new clone) volumes is internal to RBD and is not visible
to 
> > Qemu.Hence using Qemu LUKS on top of each layer is not a viable
solution. 
> > 
> > Instead, we implemented the ability to support varying encryption
keys 
> > between parent and child. Much like your qcow2-LUKS implementation
in 
> > QEMU. We took some different design choices (e.g. the LUKS header
is 
> > always part of the data payload), but the idea is very similar.
> 
> So you have a single passphrase that unlocks multiple headers, and
> each header gives a different master key ?
> 
Exactly. This is the second main
difference from the qcow2 design. 
While each volume in the chain has
a different "master key", each child also holds a wrapping of
the parent's master key, wrapped with its own master key. In effect, this
means that with one passphrase you can unlock the entire chain.
One benefit of this approach is that
we do not have to modify the qemu/libvirt APIs and need to provide only
a single passphrase even if we mount a chain with multiple master keys.
 
> 
> > > I don't think we'll ever do it, but conceptually libvirt
could even
> > > integrate with host kernel cryptsetup tools. Having an 'engine'
> > > attribute feels like a decent enough idea.
> > > 
> > > Maybe     engine='host|qemu|builtin'
> > > 
> > >   - host - the cryptosetup/kernel driver
> > >   - qemu - qemu's custom luks driver
> > >   - builtin - the librbd (or equiv) builtin driver
> > 
> > "builtin" is a bit general. Why not be explicit and
say "librbd"?
> >  I can envision scenarios in which you have two layers of
encryption that 
> > are both builtin... So being specific would resolve possible
confusion.
> 
> Yes, "librbd" is a possible alternative - I wasn't too sure
which
> way to go.
> 
> 
> Regards,
> Daniel
> -- 
> |: https://urldefense.proofpoint.com/v2/url?
> u=https-3A__berrange.com&d=DwIDaQ&c=jf_iaSHvJObTbx-
> siA1ZOg&r=FjFlELSjBSjSuiQkefrA36j3uFnZuT8td0N1TVKyCXU&m=_GOrA-
> Odqcq8ZEpp3wmF1nsx4YfPmbj9qqwuJb18MLQ&s=toMLWnzRUpurESkEvkBLL8kZLC1BqfwCJTJhC77BRbs&e=
> -o-    https://urldefense.proofpoint.com/v2/url?
> u=https-3A__www.flickr.com_photos_dberrange&d=DwIDaQ&c=jf_iaSHvJObTbx-
> siA1ZOg&r=FjFlELSjBSjSuiQkefrA36j3uFnZuT8td0N1TVKyCXU&m=_GOrA-
> Odqcq8ZEpp3wmF1nsx4YfPmbj9qqwuJb18MLQ&s=zmMJrnPXknU7heb60zqXipJREjc7_0ereSLe2Tj4z-
> A&e=  :|
> |: https://urldefense.proofpoint.com/v2/url?
> u=https-3A__libvirt.org&d=DwIDaQ&c=jf_iaSHvJObTbx-
> siA1ZOg&r=FjFlELSjBSjSuiQkefrA36j3uFnZuT8td0N1TVKyCXU&m=_GOrA-
> Odqcq8ZEpp3wmF1nsx4YfPmbj9qqwuJb18MLQ&s=teMVhlp58NrG9srfS6ncQNKdjdurvwIV6-
> s9Dx1H7CA&e=          -o-    
       https://
> urldefense.proofpoint.com/v2/url?
> u=https-3A__fstop138.berrange.com&d=DwIDaQ&c=jf_iaSHvJObTbx-
> siA1ZOg&r=FjFlELSjBSjSuiQkefrA36j3uFnZuT8td0N1TVKyCXU&m=_GOrA-
> Odqcq8ZEpp3wmF1nsx4YfPmbj9qqwuJb18MLQ&s=9iTT7RRgGeTXCkf_TsYVTI9JiUtPfpeDBUAKdjt9imk&e=
> :|
> |: https://urldefense.proofpoint.com/v2/url?
> u=https-3A__entangle-2Dphoto.org&d=DwIDaQ&c=jf_iaSHvJObTbx-
> siA1ZOg&r=FjFlELSjBSjSuiQkefrA36j3uFnZuT8td0N1TVKyCXU&m=_GOrA-
> Odqcq8ZEpp3wmF1nsx4YfPmbj9qqwuJb18MLQ&s=tWGoj1k3uG0sSe8_h33xxafK-
> Wqz2i-tOD9eP894J3s&e=     -o-    https://urldefense.proofpoint.com/
> v2/url?
> u=https-3A__www.instagram.com_dberrange&d=DwIDaQ&c=jf_iaSHvJObTbx-
> siA1ZOg&r=FjFlELSjBSjSuiQkefrA36j3uFnZuT8td0N1TVKyCXU&m=_GOrA-
> Odqcq8ZEpp3wmF1nsx4YfPmbj9qqwuJb18MLQ&s=cRiot3bn98LaVytIXQYr772gc7EilK5pPSQ-
> XDWm4Js&e=  :|
> 

Best, 
Danny