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

[Ilya Dryomov]

On Wed, Nov 16, 2022 at 12:15 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> 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'.

Yeah, I think we may need to reconsider the intent to deprecate
LUKS1 and LUKS2 options for loading encryption in librbd in favor
of a generic LUKS(-ANY) option.  But, just on its own, LUKS(-ANY)
is definitely a thing and having it exposed in QEMU seems natural.

Thanks,

                Ilya