Re: [RFC 0/8] Support generic Luks encryption
On Tue, Dec 05, 2023 at 09:51:21AM +0800, Yong Huang wrote: > 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. yes > > 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. Yes. > > The blockdev tree is like: > virtio-blk/vhost-user-blk...(frontend device) >^ > | > LUKS > / \ > file header-file > / \ > data protocol nodeLUKS header protocol node That is one possible blockdev tree, but there are many possible variations. > 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"}}' Yes, that looks like a valid sequence of steps. 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 :|
Re: [RFC 0/8] Support generic Luks encryption
Am 04.12.2023 um 18:43 hat Daniel P. Berrangé geschrieben: > 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é > > 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é > > > > 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) > > > > / \ > > > > filefile (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 > >
Re: [RFC 0/8] Support generic Luks encryption
On Tue, Dec 5, 2023 at 1:44 AM Daniel P. Berrangé 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é > > 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) > > > > / \ > > > > filefile (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
Re: [RFC 0/8] Support generic Luks encryption
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é > 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é > > > 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) > > > / \ > > > filefile (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
Re: [RFC 0/8] Support generic Luks encryption
On Tue, Dec 5, 2023 at 12:51 AM Daniel P. Berrangé 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é > > 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) > > / \ > > filefile (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. > 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 :| > > -- Best regards
Re: [RFC 0/8] Support generic Luks encryption
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é > 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) > / \ > filefile (protocol node) >| | >disk data Luks data What I suggested above ends up with the exact same block driver graph, unless I'm missing something. 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 :|
Re: [RFC 0/8] Support generic Luks encryption
On Tue, Dec 5, 2023 at 12:24 AM Daniel P. Berrangé 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) / \ filefile (protocol node) | | disk data Luks data > > 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 :| > > -- Best regards
Re: [RFC 0/8] Support generic Luks encryption
On Tue, Dec 5, 2023 at 12:24 AM Daniel P. Berrangé 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. > Indeed, this definitely makes things simple. The next version would do that ! > > 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 > Yes, most of the logic reuse the pre-existing Luks driver. > 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 :| > > -- Best regards
Re: [RFC 0/8] Support generic Luks encryption
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 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 :|