On Tue, Dec 5, 2023 at 1:44 AM Daniel P. Berrangé <berra...@redhat.com> wrote:
> On Tue, Dec 05, 2023 at 01:32:51AM +0800, Yong Huang wrote: > > On Tue, Dec 5, 2023 at 12:51 AM Daniel P. Berrangé <berra...@redhat.com> > > wrote: > > > > > On Tue, Dec 05, 2023 at 12:41:16AM +0800, Yong Huang wrote: > > > > On Tue, Dec 5, 2023 at 12:24 AM Daniel P. Berrangé < > berra...@redhat.com> > > > > wrote: > > > > > > > > > On Tue, Dec 05, 2023 at 12:06:17AM +0800, Hyman Huang wrote: > > > > > > This functionality was motivated by the following to-do list seen > > > > > > in crypto documents: > > > > > > https://wiki.qemu.org/Features/Block/Crypto > > > > > > > > > > > > The last chapter says we should "separate header volume": > > > > > > > > > > > > The LUKS format has ability to store the header in a separate > volume > > > > > > from the payload. We should extend the LUKS driver in QEMU to > support > > > > > > this use case. > > > > > > > > > > > > As a proof-of-concept, I've created this patchset, which I've > named > > > > > > the Gluks: generic luks. As their name suggests, they offer > > > encryption > > > > > > for any format that QEMU theoretically supports. > > > > > > > > > > I don't see the point in creating a new driver. > > > > > > > > > > I would expect detached header support to be implemented via an > > > > > optional new 'header' field in the existing driver. ie > > > > > > > > > > diff --git a/qapi/block-core.json b/qapi/block-core.json > > > > > index ca390c5700..48d1f2a974 100644 > > > > > --- a/qapi/block-core.json > > > > > +++ b/qapi/block-core.json > > > > > @@ -3352,11 +3352,15 @@ > > > > > # decryption key (since 2.6). Mandatory except when doing a > > > > > # metadata-only probe of the image. > > > > > # > > > > > +# @header: optional reference to the location of a blockdev > > > > > +# storing a detached LUKS heaer > > > > > +# > > > > > # Since: 2.9 > > > > > ## > > > > > { 'struct': 'BlockdevOptionsLUKS', > > > > > 'base': 'BlockdevOptionsGenericFormat', > > > > > - 'data': { '*key-secret': 'str' } } > > > > > + 'data': { '*key-secret': 'str', > > > > > + "*header-file': 'BlockdevRef'} } > > > > > > > > > > ## > > > > > # @BlockdevOptionsGenericCOWFormat: > > > > > @@ -4941,9 +4945,18 @@ > > > > > # > > > > > # Driver specific image creation options for LUKS. > > > > > # > > > > > -# @file: Node to create the image format on > > > > > +# @file: Node to create the image format on. Mandatory > > > > > +# unless a detached header file is specified using > > > > > +# @header. > > > > > # > > > > > -# @size: Size of the virtual disk in bytes > > > > > +# @size: Size of the virtual disk in bytes. Mandatory > > > > > +# unless a detached header file is specified using > > > > > +# @header. > > > > > +# > > > > > +# @header: optional reference to the location of a blockdev > > > > > +# storing a detached LUKS heaer. The @file option is > > > > > +# is optional when this is given, unless it is desired > > > > > +# to perform pre-allocation > > > > > # > > > > > # @preallocation: Preallocation mode for the new image (since: > 4.2) > > > > > # (default: off; allowed values: off, metadata, falloc, full) > > > > > @@ -4952,8 +4965,9 @@ > > > > > ## > > > > > { 'struct': 'BlockdevCreateOptionsLUKS', > > > > > 'base': 'QCryptoBlockCreateOptionsLUKS', > > > > > - 'data': { 'file': 'BlockdevRef', > > > > > - 'size': 'size', > > > > > + 'data': { '*file': 'BlockdevRef', > > > > > + '*size': 'size', > > > > > + '*header': 'BlockdevRef' > > > > > '*preallocation': 'PreallocMode' } } > > > > > > > > > > ## > > > > > > > > > > It ends up giving basicallly the same workflow as you outline, > > > > > without needing the new block driver > > > > > > > > > > > > > How about the design and usage, could it be simpler? Any advice? :) > > > > > > > > > > > > As you can see below, the Gluks format block layer driver's design is > > > > quite simple. > > > > > > > > virtio-blk/vhost-user-blk...(front-end device) > > > > ^ > > > > | > > > > Gluks (format-like disk node) > > > > / \ > > > > file header (blockdev reference) > > > > / \ > > > > file file (protocol node) > > > > | | > > > > disk data Luks data > > > > > > What I suggested above ends up with the exact same block driver > > > graph, unless I'm missing something. > > > > > > > I could overlook something or fail to adequately convey the goal of the > > patchset. :( > > > > Indeed, utilizing the same block driver might be effective if our only > goal > > is to divide the header volume, giving us an additional way to use Luks. > > > > While supporting encryption for any disk format that QEMU is capable of > > supporting is another feature of this patchset. This implies that we > might > > link the Luks header to other blockdev references, which might alter how > > the Luks are used and make them incompatible with it. It's not > > user-friendly in my opinion, and I'm not aware of a more elegant > solution. > > That existing LUKS driver can already be used in combination with > any QEMU block driver, and in the case of disk formats, can be used > either above or below the format, depending on whether you want to > encrypt just the image payload, or the payload and metadata ie. > > We can do > > luks -> qcow2 -> file (qcow2 header and qcow2 payload protected) > OK, I prefer this solution so that we can support any format implemented by the QEMU block driver. > > or > > qcow2 -> luks -> file (only qcow2 payload protected) > > And in the qcow2 case, we also have support for LUKS integrated natively > into the qcow2 format, which is similar to the 2nd case, with the > benefit that we're explicit that the image requires encryption. > > 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 :| > > Let me make a conclusion about our discussion, if i misunderstand something, point that out please: 1. To achieve the generic encryption, extending the pre-existing LUKS QEMU block driver is suggested in practice. 2. Introducing a optional field called "header-file" that store the LUKS header independently, and once "header-file" was specified, we should use it to encrypt/decrypt the blockdev node referenced by the "file" field. The blockdev tree is like: virtio-blk/vhost-user-blk...(frontend device) ^ | LUKS / \ file header-file / \ data protocol node LUKS header protocol node 3. The usage of the generic LUKS is like: Take the qcow2 as an example: 1. add a protocol blockdev node of data disk $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", "arguments":{"node-name": "libvirt-1-storage", "driver": "file", "filename": "/path/to/test_disk.qcow2"}}' 2. add a protocol blockdev node of Luks header as above. $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", "arguments":{"node-name": "libvirt-2-storage", "driver": "file", "filename": "/path/to/cipher.gluks" }}' 3. add the secret for decrypting the cipher stored in Gluks header as above $ virsh qemu-monitor-command vm '{"execute":"object-add", "arguments":{"qom-type": "secret", "id": "libvirt-2-storage-secret0", "data": "abc123"}}' 4. add the qcow2-drived blockdev format node: $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", "arguments":{"node-name": "libvirt-1-format", "driver": "qcow2", "file": "libvirt-1-storage"}}' 5. add the luks-drived blockdev to connect the qcow2 disk with Luks header by specifying the field "header-file" $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", "arguments":{"node-name": "libvirt-2-format", "driver": "luks", "file": "libvirt-1-format", "header-file": "libvirt-2-storage", "key-secret": "libvirt-2-format-secret0"}}' 6. add the device finally $ virsh qemu-monitor-command vm '{"execute":"device_add", "arguments": {"num-queues": "1", "driver": "virtio-blk-pci", "scsi": "off", "drive": "libvirt-2-format", "id": "virtio-disk2"}}' Anyway, thanks for the comments. Yong. -- Best regards
