| Message ID | cover.1701705003.git.yong.huang@smartx.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Support generic Luks encryption | expand |
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
On Tue, Dec 5, 2023 at 12:24 AM Daniel P. Berrangé <berrange@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. > 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 :| > >
On Tue, Dec 5, 2023 at 12:24 AM Daniel P. Berrangé <berrange@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 > > 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 :| > >
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é <berrange@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. With regards, Daniel
On Tue, Dec 5, 2023 at 12:51 AM Daniel P. Berrangé <berrange@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é <berrange@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. > 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 :| > >
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é <berrange@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é <berrange@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) 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
On Tue, Dec 5, 2023 at 1:44 AM Daniel P. Berrangé <berrange@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é <berrange@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é < > berrange@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.
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é <berrange@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é <berrange@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) > > or > > qcow2 -> luks -> file (only qcow2 payload protected) The other way around actually. If you want to have qcow2 metadata encrypted, you need to put luks between qcow2 and file so that it sees the metadata I/O done by the qcow2 driver. If you have luks above qcow2, then qcow2 will see only encrypted data, but will access the metadata as usual without going through encryption. Kevin > 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 :| >
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 node LUKS 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
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. 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 We don't need to create a new disk format in order to use the Gluks to encrypt the disk; all we need to do is construct a Luks header, which we will refer to as the "Gluk" because it only contains Luks header data and no user data. The creation command, for instance, is nearly identical to Luks image: $ qemu-img create --object secret,id=sec0,data=abc123 -f gluks -o cipher-alg=aes-256,cipher-mode=xts -o key-secret=sec0 cipher.gluks As previously mentioned, the "size" option is not accepted during the generation of the Gluks format because it only contains the Luks header data. To hot-add a raw disk with Gluks encryption, see the following steps: 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.raw"}}' 2. add a protocol blockdev node of Luks header $ 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 $ virsh qemu-monitor-command c81_node1 '{"execute":"object-add", "arguments":{"qom-type": "secret", "id": "libvirt-2-storage-secret0", "data": "abc123"}}' 4. add the Gluks-drived blockdev to connect the user disk with Luks header, QEMU will use the cipher in the Luks header to encrypt/decrypt the disk data $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", "arguments":{"node-name": "libvirt-1-format", "driver": "gluks", "file": "libvirt-1-storage", "header": "libvirt-2-storage", "key-secret": "libvirt-2-storage-secret0"}}' 5. add the device finally $ virsh qemu-monitor-command vm '{"execute":"device_add", "arguments": {"num-queues": "1", "driver": "virtio-blk-pci", "scsi": "off", "drive": "libvirt-1-format", "id": "virtio-disk1"}}' Do the reverse to hot-del the raw disk. To hot-add a qcow2 disk with Gluks encryption: 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. block ref: libvirt-2-storage 3. add the secret for decrypting the cipher stored in Gluks header as above too secret ref: libvirt-2-storage-secret0 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 Gluks-drived blockdev to connect the qcow2 disk with Luks header $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", "arguments":{"node-name": "libvirt-2-format", "driver": "gluks", "file": "libvirt-1-format", "header": "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"}}' In a virtual machine, several disk nodes are allowed to share a single Gluks header. This patchset, as previously said, is a proof-of-concept; additional work may be required before productization. As the title suggests, we have uploaded it solely for comments. Additionally, a thorough test would be performed on the following version. Any ideas and comments about this feature would be appreciated. Thanks, Yong Best regared ! Hyman Huang (8): crypto: Export util functions and structures crypto: Introduce payload offset set function Gluks: Add the basic framework Gluks: Introduce Gluks options qapi: Introduce Gluks types to qapi crypto: Provide the Luks crypto driver to Gluks Gluks: Implement the fundamental block layer driver hooks. block: Support Gluks format image creation using qemu-img block.c | 5 + block/crypto.c | 20 +--- block/crypto.h | 23 ++++ block/generic-luks.c | 250 +++++++++++++++++++++++++++++++++++++++++ block/generic-luks.h | 29 +++++ block/meson.build | 1 + crypto/block.c | 5 + include/crypto/block.h | 1 + qapi/block-core.json | 22 +++- qapi/crypto.json | 10 +- 10 files changed, 348 insertions(+), 18 deletions(-) create mode 100644 block/generic-luks.c create mode 100644 block/generic-luks.h