Re: [PATCH v3] block/rbd: Add support for layered encryption

[Daniel P . Berrangé]

On Wed, Nov 16, 2022 at 10:23:52AM +0000, Daniel P. Berrangé wrote:
> On Wed, Nov 16, 2022 at 09:03:31AM +0000, Or Ozeri wrote:
> > > -----Original Message-----
> > > From: Daniel P. Berrangé <berrange@redhat.com>
> > > Sent: 15 November 2022 19:47
> > > To: Or Ozeri <ORO@il.ibm.com>
> > > Cc: qemu-devel@nongnu.org; qemu-block@nongnu.org; Danny Harnik
> > > <DANNYH@il.ibm.com>; idryomov@gmail.com
> > > Subject: [EXTERNAL] Re: [PATCH v3] block/rbd: Add support for layered
> > > encryption
> > > 
> > > AFAICT, supporting layered encryption shouldn't require anything other 
> > > than
> > > the 'parent' addition.
> > > 
> > 
> > Since the layered encryption API is new in librbd, we don't have to
> > support "luks" and "luks2" at all.
> > In librbd we are actually deprecating the use of "luks" and "luks2",
> > and instead ask users to use "luks-any".
> 
> Deprecating that is a bad idea. The security characteristics and
> feature set of LUKSv1 and LUKSv2 can be quite different. If a mgmt
> app is expecting the volume to be protected with LUKSv2, it should
> be stating that explicitly and not permit a silent downgrade if
> the volume was unexpectedly using LUKSv1.
> 
> > If we don't add "luks-any" here, we will need to implement
> > explicit cases for "luks" and "luks2" in the qemu_rbd_encryption_load2.
> > This looks like a kind of wasteful coding that won't be actually used
> > by users of the rbd driver in qemu.
> 
> It isn't wasteful - supporting the formats explicitly is desirable
> to prevent format downgrades.
> 
> > Anyhow, we need the "luks-any" option for our use-case, so if you
> > insist, I will first submit a patch to add "luks-any", before this
> > patch.
> 
> I'm pretty wary of any kind of automatic encryption format detection
> in QEMU. The automatic block driver format probing has been a long
> standing source of CVEs in QEMU and every single mgmt app above QEMU.

Having said that, normal linux LUKS tools like cryptsetup or systemd
LUKS integration will auto-detect  luks1 vs luks2. All cryptsetup
commands also have an option to explicitly specify the format version.

So with that precedent I guess it is ok to add 'luks-any'.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|