Re: [PATCH v4 5/7] block: Support detached LUKS header creation using qemu-img

[Daniel P . Berrangé]

On Tue, Jan 30, 2024 at 01:37:23PM +0800, yong.huang@smartx.com wrote:
> From: Hyman Huang <yong.huang@smartx.com>
> 
> Even though a LUKS header might be created with cryptsetup,
> qemu-img should be enhanced to accommodate it as well.
> 
> Add the 'detached-header' option to specify the creation of
> a detached LUKS header. This is how it is used:
> $ qemu-img create --object secret,id=sec0,data=abc123 -f luks
> > -o cipher-alg=aes-256,cipher-mode=xts -o key-secret=sec0
> > -o detached-header=true header.luks
> 
> Using qemu-img or cryptsetup tools to query information of
> an LUKS header image as follows:
> 
> Assume a detached LUKS header image has been created by:
> $ dd if=/dev/zero of=test-header.img bs=1M count=32
> $ dd if=/dev/zero of=test-payload.img bs=1M count=1000
> $ cryptsetup luksFormat --header test-header.img test-payload.img
> > --force-password --type luks1
> 
> Header image information could be queried using cryptsetup:
> $ cryptsetup luksDump test-header.img
> 
> or qemu-img:
> $ qemu-img info 'json:{"driver":"luks","file":{"filename":
> > "test-payload.img"},"header":{"filename":"test-header.img"}}'
> 
> When using qemu-img, keep in mind that the entire disk
> information specified by the JSON-format string above must be
> supplied on the commandline; if not, an overlay check will reveal
> a problem with the LUKS volume check logic.
> 
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
>  block.c          |  5 ++++-
>  block/crypto.c   | 10 +++++++++-
>  block/crypto.h   |  8 ++++++++
>  qapi/crypto.json |  5 ++++-
>  4 files changed, 25 insertions(+), 3 deletions(-)


> diff --git a/block/crypto.c b/block/crypto.c
> index 8e7ee5e9ac..65426d3a16 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -791,6 +792,9 @@ block_crypto_co_create_opts_luks(BlockDriver *drv, const 
> char *filename,
>      PreallocMode prealloc;
>      char *buf = NULL;
>      int64_t size;
> +    bool detached_hdr =
> +        qemu_opt_get_bool(opts, "detached-header", false);
> +    unsigned int cflags = 0;
>      int ret;
>      Error *local_err = NULL;
>  
> @@ -830,6 +834,10 @@ block_crypto_co_create_opts_luks(BlockDriver *drv, const 
> char *filename,
>          goto fail;
>      }
>  
> +    if (detached_hdr) {
> +        cflags |= QCRYPTO_BLOCK_CREATE_DETACHED;
> +    }
> +

We're setting cflags but not using it ever.

>      /* Create format layer */
>      ret = block_crypto_co_create_generic(bs, size, create_opts,
>                                           prealloc, 0, errp);

This '0' here should be replaced by 'cflags', since you're
checking for QCRYPTO_BLOCK_CREATE_DETACHED inside the
block_crypto_co_create_generic method.

I'll make this change when I merge this, so no need to resend.


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