[v2 4/4] block: Support detached LUKS header creation for blockdev-create

Hyman Huang posted 4 patches 11 months, 3 weeks ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[v2 4/4] block: Support detached LUKS header creation for blockdev-create
Posted by Hyman Huang 11 months, 3 weeks ago
Provide the "detached-mode" option for detached LUKS header
formatting.

To format the LUKS header on the pre-creating disk, example
as follows:

1. add a protocol blockdev node of LUKS header
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments":{"node-name":"libvirt-1-storage", "driver":"file",
> "filename":"/path/to/cipher.gluks" }}'

2. add the secret for encrypting the cipher stored in LUKS
   header above
$ virsh qemu-monitor-command vm '{"execute":"object-add",
> "arguments":{"qom-type": "secret", "id":
> "libvirt-1-storage-secret0", "data": "abc123"}}'

3. format the disk node
$ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> "arguments":{"job-id":"job0", "options":{"driver":"luks",
> "size":0, "file":"libvirt-1-storage", "detached-mode":true,
> "cipher-alg":"aes-256",
> "key-secret":"libvirt-3-storage-encryption-secret0"}}}'

Signed-off-by: Hyman Huang <yong.huang@smartx.com>
---
 block/crypto.c       | 8 +++++++-
 qapi/block-core.json | 5 ++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index 7d70349463..e77c49bd0c 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -667,10 +667,12 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
     BlockDriverState *bs = NULL;
     QCryptoBlockCreateOptions create_opts;
     PreallocMode preallocation = PREALLOC_MODE_OFF;
+    int64_t size;
     int ret;
 
     assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
     luks_opts = &create_options->u.luks;
+    size = luks_opts->size;
 
     bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
     if (bs == NULL) {
@@ -686,7 +688,11 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
         preallocation = luks_opts->preallocation;
     }
 
-    ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
+    if (luks_opts->detached_mode) {
+        size = 0;
+    }
+
+    ret = block_crypto_co_create_generic(bs, size, &create_opts,
                                          preallocation, errp);
     if (ret < 0) {
         goto fail;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 10be08d08f..1e7a7e1b05 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4952,13 +4952,16 @@
 # @preallocation: Preallocation mode for the new image (since: 4.2)
 #     (default: off; allowed values: off, metadata, falloc, full)
 #
+# @detached-mode: create a detached LUKS header. (since 9.0)
+#
 # Since: 2.12
 ##
 { 'struct': 'BlockdevCreateOptionsLUKS',
   'base': 'QCryptoBlockCreateOptionsLUKS',
   'data': { 'file':             'BlockdevRef',
             'size':             'size',
-            '*preallocation':   'PreallocMode' } }
+            '*preallocation':   'PreallocMode',
+            '*detached-mode':   'bool'}}
 
 ##
 # @BlockdevCreateOptionsNfs:
-- 
2.39.1
Re: [v2 4/4] block: Support detached LUKS header creation for blockdev-create
Posted by Daniel P. Berrangé 11 months, 2 weeks ago
On Thu, Dec 07, 2023 at 12:37:45AM +0800, Hyman Huang wrote:
> Provide the "detached-mode" option for detached LUKS header
> formatting.
> 
> To format the LUKS header on the pre-creating disk, example
> as follows:
> 
> 1. add a protocol blockdev node of LUKS header
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > "arguments":{"node-name":"libvirt-1-storage", "driver":"file",
> > "filename":"/path/to/cipher.gluks" }}'
> 
> 2. add the secret for encrypting the cipher stored in LUKS
>    header above
> $ virsh qemu-monitor-command vm '{"execute":"object-add",
> > "arguments":{"qom-type": "secret", "id":
> > "libvirt-1-storage-secret0", "data": "abc123"}}'
> 
> 3. format the disk node
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> > "arguments":{"job-id":"job0", "options":{"driver":"luks",
> > "size":0, "file":"libvirt-1-storage", "detached-mode":true,
> > "cipher-alg":"aes-256",
> > "key-secret":"libvirt-3-storage-encryption-secret0"}}}'
> 
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
>  block/crypto.c       | 8 +++++++-
>  qapi/block-core.json | 5 ++++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/block/crypto.c b/block/crypto.c
> index 7d70349463..e77c49bd0c 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -667,10 +667,12 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
>      BlockDriverState *bs = NULL;
>      QCryptoBlockCreateOptions create_opts;
>      PreallocMode preallocation = PREALLOC_MODE_OFF;
> +    int64_t size;
>      int ret;
>  
>      assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
>      luks_opts = &create_options->u.luks;
> +    size = luks_opts->size;
>  
>      bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
>      if (bs == NULL) {
> @@ -686,7 +688,11 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
>          preallocation = luks_opts->preallocation;
>      }
>  
> -    ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
> +    if (luks_opts->detached_mode) {
> +        size = 0;
> +    }
> +
> +    ret = block_crypto_co_create_generic(bs, size, &create_opts,
>                                           preallocation, errp);
>      if (ret < 0) {
>          goto fail;
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 10be08d08f..1e7a7e1b05 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -4952,13 +4952,16 @@
>  # @preallocation: Preallocation mode for the new image (since: 4.2)
>  #     (default: off; allowed values: off, metadata, falloc, full)
>  #
> +# @detached-mode: create a detached LUKS header. (since 9.0)
> +#
>  # Since: 2.12
>  ##
>  { 'struct': 'BlockdevCreateOptionsLUKS',
>    'base': 'QCryptoBlockCreateOptionsLUKS',
>    'data': { 'file':             'BlockdevRef',
>              'size':             'size',
> -            '*preallocation':   'PreallocMode' } }
> +            '*preallocation':   'PreallocMode',
> +            '*detached-mode':   'bool'}}

Using a bool flag here is insufficiently flexible. We need to be able to
honour preallocation of the payload device, while using a separate
header.

You need to make the existing 'file' optional, while also adding an
extra optional 'header' field. ie

  { 'struct': 'BlockdevCreateOptionsLUKS',
    'base': 'QCryptoBlockCreateOptionsLUKS',
    'data': { '*file':            'BlockdevRef',
              '*header':          'BlockdevRef',
              'size':             'size',
              '*preallocation':   'PreallocMode' } }


If 'preallocation' is requested, then we must enforce that 'file' is
non-NULL in the code.

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: [v2 4/4] block: Support detached LUKS header creation for blockdev-create
Posted by Yong Huang 11 months, 2 weeks ago
On Mon, Dec 18, 2023 at 7:19 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Thu, Dec 07, 2023 at 12:37:45AM +0800, Hyman Huang wrote:
> > Provide the "detached-mode" option for detached LUKS header
> > formatting.
> >
> > To format the LUKS header on the pre-creating disk, example
> > as follows:
> >
> > 1. add a protocol blockdev node of LUKS header
> > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > > "arguments":{"node-name":"libvirt-1-storage", "driver":"file",
> > > "filename":"/path/to/cipher.gluks" }}'
> >
> > 2. add the secret for encrypting the cipher stored in LUKS
> >    header above
> > $ virsh qemu-monitor-command vm '{"execute":"object-add",
> > > "arguments":{"qom-type": "secret", "id":
> > > "libvirt-1-storage-secret0", "data": "abc123"}}'
> >
> > 3. format the disk node
> > $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> > > "arguments":{"job-id":"job0", "options":{"driver":"luks",
> > > "size":0, "file":"libvirt-1-storage", "detached-mode":true,
> > > "cipher-alg":"aes-256",
> > > "key-secret":"libvirt-3-storage-encryption-secret0"}}}'
> >
> > Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> > ---
> >  block/crypto.c       | 8 +++++++-
> >  qapi/block-core.json | 5 ++++-
> >  2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/crypto.c b/block/crypto.c
> > index 7d70349463..e77c49bd0c 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -667,10 +667,12 @@ block_crypto_co_create_luks(BlockdevCreateOptions
> *create_options, Error **errp)
> >      BlockDriverState *bs = NULL;
> >      QCryptoBlockCreateOptions create_opts;
> >      PreallocMode preallocation = PREALLOC_MODE_OFF;
> > +    int64_t size;
> >      int ret;
> >
> >      assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
> >      luks_opts = &create_options->u.luks;
> > +    size = luks_opts->size;
> >
> >      bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
> >      if (bs == NULL) {
> > @@ -686,7 +688,11 @@ block_crypto_co_create_luks(BlockdevCreateOptions
> *create_options, Error **errp)
> >          preallocation = luks_opts->preallocation;
> >      }
> >
> > -    ret = block_crypto_co_create_generic(bs, luks_opts->size,
> &create_opts,
> > +    if (luks_opts->detached_mode) {
> > +        size = 0;
> > +    }
> > +
> > +    ret = block_crypto_co_create_generic(bs, size, &create_opts,
> >                                           preallocation, errp);
> >      if (ret < 0) {
> >          goto fail;
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 10be08d08f..1e7a7e1b05 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -4952,13 +4952,16 @@
> >  # @preallocation: Preallocation mode for the new image (since: 4.2)
> >  #     (default: off; allowed values: off, metadata, falloc, full)
> >  #
> > +# @detached-mode: create a detached LUKS header. (since 9.0)
> > +#
> >  # Since: 2.12
> >  ##
> >  { 'struct': 'BlockdevCreateOptionsLUKS',
> >    'base': 'QCryptoBlockCreateOptionsLUKS',
> >    'data': { 'file':             'BlockdevRef',
> >              'size':             'size',
> > -            '*preallocation':   'PreallocMode' } }
> > +            '*preallocation':   'PreallocMode',
> > +            '*detached-mode':   'bool'}}
>
> Using a bool flag here is insufficiently flexible. We need to be able to
> honour preallocation of the payload device, while using a separate
> header.
>
> You need to make the existing 'file' optional, while also adding an
> extra optional 'header' field. ie
>
>   { 'struct': 'BlockdevCreateOptionsLUKS',
>     'base': 'QCryptoBlockCreateOptionsLUKS',
>     'data': { '*file':            'BlockdevRef',
>               '*header':          'BlockdevRef',
>               'size':             'size',
>               '*preallocation':   'PreallocMode' } }
>
>
> If 'preallocation' is requested, then we must enforce that 'file' is
> non-NULL in the code.
>

Ok, thanks for the advice, I'll modify it in the next version.

>
> 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