[PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create

yong.huang@smartx.com posted 7 patches 10 months 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>, Hyman Huang <yong.huang@smartx.com>
[PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create
Posted by yong.huang@smartx.com 10 months ago
From: Hyman Huang <yong.huang@smartx.com>

Firstly, enable the ability to choose the block device containing
a detachable LUKS header by adding the 'header' parameter to
BlockdevCreateOptionsLUKS.

Secondly, when formatting the LUKS volume with a detachable header,
truncate the payload volume to length without a header size.

Using the qmp blockdev command, create the LUKS volume with a
detachable header as follows:

1. add the secret to lock/unlock the cipher stored in the
   detached LUKS header
$ virsh qemu-monitor-command vm '{"execute":"object-add",
> "arguments":{"qom-type": "secret", "id": "sec0", "data": "foo"}}'

2. create a header img with 0 size
$ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> "arguments":{"job-id":"job0", "options":{"driver":"file",
> "filename":"/path/to/detached_luks_header.img", "size":0 }}}'

3. add protocol blockdev node for header
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments": {"driver":"file", "filename":
> "/path/to/detached_luks_header.img", "node-name":
> "detached-luks-header-storage"}}'

4. create a payload img with 0 size
$ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> "arguments":{"job-id":"job1", "options":{"driver":"file",
> "filename":"/path/to/detached_luks_payload_raw.img", "size":0}}}'

5. add protocol blockdev node for payload
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments": {"driver":"file", "filename":
> "/path/to/detached_luks_payload_raw.img", "node-name":
> "luks-payload-raw-storage"}}'

6. do the formatting with 128M size
$ virsh qemu-monitor-command c81_node1 '{"execute":"blockdev-create",
> "arguments":{"job-id":"job2", "options":{"driver":"luks", "header":
> "detached-luks-header-storage", "file":"luks-payload-raw-storage",
> "size":134217728, "preallocation":"full", "key-secret":"sec0" }}}'

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

diff --git a/block/crypto.c b/block/crypto.c
index 1b3f87922a..8e7ee5e9ac 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -162,6 +162,48 @@ error:
     return ret;
 }
 
+static int coroutine_fn GRAPH_UNLOCKED
+block_crypto_co_format_luks_payload(BlockdevCreateOptionsLUKS *luks_opts,
+                                    Error **errp)
+{
+    BlockDriverState *bs = NULL;
+    BlockBackend *blk = NULL;
+    Error *local_error = NULL;
+    int ret;
+
+    if (luks_opts->size > INT64_MAX) {
+        return -EFBIG;
+    }
+
+    bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
+    if (bs == NULL) {
+        return -EIO;
+    }
+
+    blk = blk_co_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE,
+                             BLK_PERM_ALL, errp);
+    if (!blk) {
+        ret = -EPERM;
+        goto fail;
+    }
+
+    ret = blk_truncate(blk, luks_opts->size, true,
+                       luks_opts->preallocation, 0, &local_error);
+    if (ret < 0) {
+        if (ret == -EFBIG) {
+            /* Replace the error message with a better one */
+            error_free(local_error);
+            error_setg(errp, "The requested file size is too large");
+        }
+        goto fail;
+    }
+
+    ret = 0;
+
+fail:
+    bdrv_co_unref(bs);
+    return ret;
+}
 
 static QemuOptsList block_crypto_runtime_opts_luks = {
     .name = "crypto",
@@ -341,7 +383,9 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
 static int coroutine_fn GRAPH_UNLOCKED
 block_crypto_co_create_generic(BlockDriverState *bs, int64_t size,
                                QCryptoBlockCreateOptions *opts,
-                               PreallocMode prealloc, Error **errp)
+                               PreallocMode prealloc,
+                               unsigned int flags,
+                               Error **errp)
 {
     int ret;
     BlockBackend *blk;
@@ -369,7 +413,7 @@ block_crypto_co_create_generic(BlockDriverState *bs, int64_t size,
                                   block_crypto_create_init_func,
                                   block_crypto_create_write_func,
                                   &data,
-                                  0,
+                                  flags,
                                   errp);
 
     if (!crypto) {
@@ -656,16 +700,26 @@ static int coroutine_fn GRAPH_UNLOCKED
 block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
 {
     BlockdevCreateOptionsLUKS *luks_opts;
+    BlockDriverState *hdr_bs = NULL;
     BlockDriverState *bs = NULL;
     QCryptoBlockCreateOptions create_opts;
     PreallocMode preallocation = PREALLOC_MODE_OFF;
+    unsigned int cflags = 0;
     int ret;
 
     assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
     luks_opts = &create_options->u.luks;
 
-    if (luks_opts->file == NULL) {
-        error_setg(errp, "Formatting LUKS disk requires parameter 'file'");
+    if (luks_opts->header == NULL && luks_opts->file == NULL) {
+        error_setg(errp, "Either the parameter 'header' or 'file' must "
+                   "be specified");
+        return -EINVAL;
+    }
+
+    if ((luks_opts->preallocation != PREALLOC_MODE_OFF) &&
+        (luks_opts->file == NULL)) {
+        error_setg(errp, "Parameter 'preallocation' requires 'file' to be "
+                   "specified for formatting LUKS disk");
         return -EINVAL;
     }
 
@@ -678,14 +732,38 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
         preallocation = luks_opts->preallocation;
     }
 
-    if (luks_opts->file) {
+    if (luks_opts->header) {
+        /* LUKS volume with detached header */
+        hdr_bs = bdrv_co_open_blockdev_ref(luks_opts->header, errp);
+        if (hdr_bs == NULL) {
+            return -EIO;
+        }
+
+        cflags |= QCRYPTO_BLOCK_CREATE_DETACHED;
+
+        /* Format the LUKS header node */
+        ret = block_crypto_co_create_generic(hdr_bs, 0, &create_opts,
+                                             PREALLOC_MODE_OFF, cflags, errp);
+        if (ret < 0) {
+            goto fail;
+        }
+
+        /* Format the LUKS payload node */
+        if (luks_opts->file) {
+            ret = block_crypto_co_format_luks_payload(luks_opts, errp);
+            if (ret < 0) {
+                goto fail;
+            }
+        }
+    } else if (luks_opts->file) {
+        /* LUKS volume with none-detached header */
         bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
         if (bs == NULL) {
             return -EIO;
         }
 
         ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
-                                             preallocation, errp);
+                                             preallocation, cflags, errp);
         if (ret < 0) {
             goto fail;
         }
@@ -693,7 +771,13 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
 
     ret = 0;
 fail:
-    bdrv_co_unref(bs);
+    if (hdr_bs != NULL) {
+        bdrv_co_unref(hdr_bs);
+    }
+
+    if (bs != NULL) {
+        bdrv_co_unref(bs);
+    }
     return ret;
 }
 
@@ -747,7 +831,8 @@ block_crypto_co_create_opts_luks(BlockDriver *drv, const char *filename,
     }
 
     /* Create format layer */
-    ret = block_crypto_co_create_generic(bs, size, create_opts, prealloc, errp);
+    ret = block_crypto_co_create_generic(bs, size, create_opts,
+                                         prealloc, 0, errp);
     if (ret < 0) {
         goto fail;
     }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 69a88d613d..eab15d7dd9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4960,6 +4960,8 @@
 # @file: Node to create the image format on, mandatory except when
 #        'preallocation' is not requested
 #
+# @header: Block device holding a detached LUKS header. (since 9.0)
+#
 # @size: Size of the virtual disk in bytes
 #
 # @preallocation: Preallocation mode for the new image (since: 4.2)
@@ -4970,6 +4972,7 @@
 { 'struct': 'BlockdevCreateOptionsLUKS',
   'base': 'QCryptoBlockCreateOptionsLUKS',
   'data': { '*file':            'BlockdevRef',
+            '*header':          'BlockdevRef',
             'size':             'size',
             '*preallocation':   'PreallocMode' } }
 
-- 
2.31.1
Re: [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create
Posted by Markus Armbruster 9 months, 1 week ago
yong.huang@smartx.com writes:

> From: Hyman Huang <yong.huang@smartx.com>
>
> Firstly, enable the ability to choose the block device containing
> a detachable LUKS header by adding the 'header' parameter to
> BlockdevCreateOptionsLUKS.
>
> Secondly, when formatting the LUKS volume with a detachable header,
> truncate the payload volume to length without a header size.
>
> Using the qmp blockdev command, create the LUKS volume with a
> detachable header as follows:
>
> 1. add the secret to lock/unlock the cipher stored in the
>    detached LUKS header
> $ virsh qemu-monitor-command vm '{"execute":"object-add",
>> "arguments":{"qom-type": "secret", "id": "sec0", "data": "foo"}}'
>
> 2. create a header img with 0 size
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
>> "arguments":{"job-id":"job0", "options":{"driver":"file",
>> "filename":"/path/to/detached_luks_header.img", "size":0 }}}'
>
> 3. add protocol blockdev node for header
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
>> "arguments": {"driver":"file", "filename":
>> "/path/to/detached_luks_header.img", "node-name":
>> "detached-luks-header-storage"}}'
>
> 4. create a payload img with 0 size
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
>> "arguments":{"job-id":"job1", "options":{"driver":"file",
>> "filename":"/path/to/detached_luks_payload_raw.img", "size":0}}}'
>
> 5. add protocol blockdev node for payload
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
>> "arguments": {"driver":"file", "filename":
>> "/path/to/detached_luks_payload_raw.img", "node-name":
>> "luks-payload-raw-storage"}}'
>
> 6. do the formatting with 128M size
> $ virsh qemu-monitor-command c81_node1 '{"execute":"blockdev-create",
>> "arguments":{"job-id":"job2", "options":{"driver":"luks", "header":
>> "detached-luks-header-storage", "file":"luks-payload-raw-storage",
>> "size":134217728, "preallocation":"full", "key-secret":"sec0" }}}'
>
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---

[...]

> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 69a88d613d..eab15d7dd9 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -4960,6 +4960,8 @@
>  # @file: Node to create the image format on, mandatory except when
>  #        'preallocation' is not requested
>  #
> +# @header: Block device holding a detached LUKS header. (since 9.0)
> +#

Behavior when @header is present vs. behavior when it's absent?

>  # @size: Size of the virtual disk in bytes
>  #
>  # @preallocation: Preallocation mode for the new image (since: 4.2)
> @@ -4970,6 +4972,7 @@
>  { 'struct': 'BlockdevCreateOptionsLUKS',
>    'base': 'QCryptoBlockCreateOptionsLUKS',
>    'data': { '*file':            'BlockdevRef',
> +            '*header':          'BlockdevRef',
>              'size':             'size',
>              '*preallocation':   'PreallocMode' } }
Re: [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create
Posted by Markus Armbruster 9 months, 1 week ago
One more thing...

Markus Armbruster <armbru@redhat.com> writes:

> yong.huang@smartx.com writes:
>
>> From: Hyman Huang <yong.huang@smartx.com>
>>
>> Firstly, enable the ability to choose the block device containing
>> a detachable LUKS header by adding the 'header' parameter to
>> BlockdevCreateOptionsLUKS.
>>
>> Secondly, when formatting the LUKS volume with a detachable header,
>> truncate the payload volume to length without a header size.
>>
>> Using the qmp blockdev command, create the LUKS volume with a
>> detachable header as follows:
>>
>> 1. add the secret to lock/unlock the cipher stored in the
>>    detached LUKS header
>> $ virsh qemu-monitor-command vm '{"execute":"object-add",
>>> "arguments":{"qom-type": "secret", "id": "sec0", "data": "foo"}}'
>>
>> 2. create a header img with 0 size
>> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
>>> "arguments":{"job-id":"job0", "options":{"driver":"file",
>>> "filename":"/path/to/detached_luks_header.img", "size":0 }}}'
>>
>> 3. add protocol blockdev node for header
>> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
>>> "arguments": {"driver":"file", "filename":
>>> "/path/to/detached_luks_header.img", "node-name":
>>> "detached-luks-header-storage"}}'
>>
>> 4. create a payload img with 0 size
>> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
>>> "arguments":{"job-id":"job1", "options":{"driver":"file",
>>> "filename":"/path/to/detached_luks_payload_raw.img", "size":0}}}'
>>
>> 5. add protocol blockdev node for payload
>> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
>>> "arguments": {"driver":"file", "filename":
>>> "/path/to/detached_luks_payload_raw.img", "node-name":
>>> "luks-payload-raw-storage"}}'
>>
>> 6. do the formatting with 128M size
>> $ virsh qemu-monitor-command c81_node1 '{"execute":"blockdev-create",
>>> "arguments":{"job-id":"job2", "options":{"driver":"luks", "header":
>>> "detached-luks-header-storage", "file":"luks-payload-raw-storage",
>>> "size":134217728, "preallocation":"full", "key-secret":"sec0" }}}'
>>
>> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
>> ---
>
> [...]
>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 69a88d613d..eab15d7dd9 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -4960,6 +4960,8 @@
>>  # @file: Node to create the image format on, mandatory except when
>>  #        'preallocation' is not requested
>>  #
>> +# @header: Block device holding a detached LUKS header. (since 9.0)
>> +#
>
> Behavior when @header is present vs. behavior when it's absent?

The next patch adds a member to QCryptoBlockCreateOptionsLUKS, with a
similar description, but a different name:

    # @detached-header: create a detached LUKS header. (since 9.0)

Should we name the one added here @detached-header, too?

>>  # @size: Size of the virtual disk in bytes
>>  #
>>  # @preallocation: Preallocation mode for the new image (since: 4.2)
>> @@ -4970,6 +4972,7 @@
>>  { 'struct': 'BlockdevCreateOptionsLUKS',
>>    'base': 'QCryptoBlockCreateOptionsLUKS',
>>    'data': { '*file':            'BlockdevRef',
>> +            '*header':          'BlockdevRef',
>>              'size':             'size',
>>              '*preallocation':   'PreallocMode' } }
Re: [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create
Posted by Daniel P. Berrangé 9 months, 1 week ago
On Mon, Feb 19, 2024 at 03:49:30PM +0100, Markus Armbruster wrote:
> One more thing...
> 
> Markus Armbruster <armbru@redhat.com> writes:
> 
> > yong.huang@smartx.com writes:
> >
> >> From: Hyman Huang <yong.huang@smartx.com>
> >>
> >> Firstly, enable the ability to choose the block device containing
> >> a detachable LUKS header by adding the 'header' parameter to
> >> BlockdevCreateOptionsLUKS.
> >>
> >> Secondly, when formatting the LUKS volume with a detachable header,
> >> truncate the payload volume to length without a header size.
> >>
> >> Using the qmp blockdev command, create the LUKS volume with a
> >> detachable header as follows:
> >>
> >> 1. add the secret to lock/unlock the cipher stored in the
> >>    detached LUKS header
> >> $ virsh qemu-monitor-command vm '{"execute":"object-add",
> >>> "arguments":{"qom-type": "secret", "id": "sec0", "data": "foo"}}'
> >>
> >> 2. create a header img with 0 size
> >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> >>> "arguments":{"job-id":"job0", "options":{"driver":"file",
> >>> "filename":"/path/to/detached_luks_header.img", "size":0 }}}'
> >>
> >> 3. add protocol blockdev node for header
> >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> >>> "arguments": {"driver":"file", "filename":
> >>> "/path/to/detached_luks_header.img", "node-name":
> >>> "detached-luks-header-storage"}}'
> >>
> >> 4. create a payload img with 0 size
> >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> >>> "arguments":{"job-id":"job1", "options":{"driver":"file",
> >>> "filename":"/path/to/detached_luks_payload_raw.img", "size":0}}}'
> >>
> >> 5. add protocol blockdev node for payload
> >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> >>> "arguments": {"driver":"file", "filename":
> >>> "/path/to/detached_luks_payload_raw.img", "node-name":
> >>> "luks-payload-raw-storage"}}'
> >>
> >> 6. do the formatting with 128M size
> >> $ virsh qemu-monitor-command c81_node1 '{"execute":"blockdev-create",
> >>> "arguments":{"job-id":"job2", "options":{"driver":"luks", "header":
> >>> "detached-luks-header-storage", "file":"luks-payload-raw-storage",
> >>> "size":134217728, "preallocation":"full", "key-secret":"sec0" }}}'
> >>
> >> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> >> ---
> >
> > [...]
> >
> >> diff --git a/qapi/block-core.json b/qapi/block-core.json
> >> index 69a88d613d..eab15d7dd9 100644
> >> --- a/qapi/block-core.json
> >> +++ b/qapi/block-core.json
> >> @@ -4960,6 +4960,8 @@
> >>  # @file: Node to create the image format on, mandatory except when
> >>  #        'preallocation' is not requested
> >>  #
> >> +# @header: Block device holding a detached LUKS header. (since 9.0)
> >> +#
> >
> > Behavior when @header is present vs. behavior when it's absent?
> 
> The next patch adds a member to QCryptoBlockCreateOptionsLUKS, with a
> similar description, but a different name:
> 
>     # @detached-header: create a detached LUKS header. (since 9.0)
> 
> Should we name the one added here @detached-header, too?

Yikes, that's a mistake. When I reviewed this I was somehow under the
illusion that QCryptoBlockCreateOptionsLUKS was internal use only, for
the block driver impl to interact with the crypto LUKS impl.

In fact, as the diff context below shows, QCryptoBlockCreateOptionsLUKS
is a base struct for BlockdevCreateOptionsLUKS. So in effect we have
one struct with two fields expressing similar concept.

TL;DR: the @detached-header field needs to go, as that's supposed to
be internal only. The mgmt app should only care about 'header' in the
BlockdevCreateOptionsLUKS struct.

FYI, this whole series is already merged last week. So this will need
a fixup. I'll look into it now.

> 
> >>  # @size: Size of the virtual disk in bytes
> >>  #
> >>  # @preallocation: Preallocation mode for the new image (since: 4.2)
> >> @@ -4970,6 +4972,7 @@
> >>  { 'struct': 'BlockdevCreateOptionsLUKS',
> >>    'base': 'QCryptoBlockCreateOptionsLUKS',
> >>    'data': { '*file':            'BlockdevRef',
> >> +            '*header':          'BlockdevRef',
> >>              'size':             'size',
> >>              '*preallocation':   'PreallocMode' } }

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: [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create
Posted by Markus Armbruster 9 months, 1 week ago
Daniel P. Berrangé <berrange@redhat.com> writes:

[...]

> TL;DR: the @detached-header field needs to go, as that's supposed to
> be internal only. The mgmt app should only care about 'header' in the
> BlockdevCreateOptionsLUKS struct.
>
> FYI, this whole series is already merged last week. So this will need
> a fixup. I'll look into it now.

Glad I neglected to check for a merge, because if I had, I wouldn't have
looked at the schema :)

My apologies for looking so late!

[...]
Re: [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create
Posted by Daniel P. Berrangé 9 months, 1 week ago
On Mon, Feb 19, 2024 at 02:57:13PM +0000, Daniel P. Berrangé wrote:
> On Mon, Feb 19, 2024 at 03:49:30PM +0100, Markus Armbruster wrote:
> > One more thing...
> > 
> > Markus Armbruster <armbru@redhat.com> writes:
> > 
> > > yong.huang@smartx.com writes:
> > >
> > >> From: Hyman Huang <yong.huang@smartx.com>
> > >>
> > >> Firstly, enable the ability to choose the block device containing
> > >> a detachable LUKS header by adding the 'header' parameter to
> > >> BlockdevCreateOptionsLUKS.
> > >>
> > >> Secondly, when formatting the LUKS volume with a detachable header,
> > >> truncate the payload volume to length without a header size.
> > >>
> > >> Using the qmp blockdev command, create the LUKS volume with a
> > >> detachable header as follows:
> > >>
> > >> 1. add the secret to lock/unlock the cipher stored in the
> > >>    detached LUKS header
> > >> $ virsh qemu-monitor-command vm '{"execute":"object-add",
> > >>> "arguments":{"qom-type": "secret", "id": "sec0", "data": "foo"}}'
> > >>
> > >> 2. create a header img with 0 size
> > >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> > >>> "arguments":{"job-id":"job0", "options":{"driver":"file",
> > >>> "filename":"/path/to/detached_luks_header.img", "size":0 }}}'
> > >>
> > >> 3. add protocol blockdev node for header
> > >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > >>> "arguments": {"driver":"file", "filename":
> > >>> "/path/to/detached_luks_header.img", "node-name":
> > >>> "detached-luks-header-storage"}}'
> > >>
> > >> 4. create a payload img with 0 size
> > >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> > >>> "arguments":{"job-id":"job1", "options":{"driver":"file",
> > >>> "filename":"/path/to/detached_luks_payload_raw.img", "size":0}}}'
> > >>
> > >> 5. add protocol blockdev node for payload
> > >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > >>> "arguments": {"driver":"file", "filename":
> > >>> "/path/to/detached_luks_payload_raw.img", "node-name":
> > >>> "luks-payload-raw-storage"}}'
> > >>
> > >> 6. do the formatting with 128M size
> > >> $ virsh qemu-monitor-command c81_node1 '{"execute":"blockdev-create",
> > >>> "arguments":{"job-id":"job2", "options":{"driver":"luks", "header":
> > >>> "detached-luks-header-storage", "file":"luks-payload-raw-storage",
> > >>> "size":134217728, "preallocation":"full", "key-secret":"sec0" }}}'
> > >>
> > >> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> > >> ---
> > >
> > > [...]
> > >
> > >> diff --git a/qapi/block-core.json b/qapi/block-core.json
> > >> index 69a88d613d..eab15d7dd9 100644
> > >> --- a/qapi/block-core.json
> > >> +++ b/qapi/block-core.json
> > >> @@ -4960,6 +4960,8 @@
> > >>  # @file: Node to create the image format on, mandatory except when
> > >>  #        'preallocation' is not requested
> > >>  #
> > >> +# @header: Block device holding a detached LUKS header. (since 9.0)
> > >> +#
> > >
> > > Behavior when @header is present vs. behavior when it's absent?
> > 
> > The next patch adds a member to QCryptoBlockCreateOptionsLUKS, with a
> > similar description, but a different name:
> > 
> >     # @detached-header: create a detached LUKS header. (since 9.0)
> > 
> > Should we name the one added here @detached-header, too?
> 
> Yikes, that's a mistake. When I reviewed this I was somehow under the
> illusion that QCryptoBlockCreateOptionsLUKS was internal use only, for
> the block driver impl to interact with the crypto LUKS impl.
> 
> In fact, as the diff context below shows, QCryptoBlockCreateOptionsLUKS
> is a base struct for BlockdevCreateOptionsLUKS. So in effect we have
> one struct with two fields expressing similar concept.
> 
> TL;DR: the @detached-header field needs to go, as that's supposed to
> be internal only. The mgmt app should only care about 'header' in the
> BlockdevCreateOptionsLUKS struct.
> 
> FYI, this whole series is already merged last week. So this will need
> a fixup. I'll look into it now.

In fact the '@detached-header' added in the next patch is redundant
in the QAPI, as it is never set. So this QAPI field can be deleted.

We do have a 'detached-header' QemuOpts setting which is simply a
bool flag, as opposed to a full blockdev ref, since qemu-img create
doesn't work in terms of the QAPI BlockdevCreate schema.



> 
> > 
> > >>  # @size: Size of the virtual disk in bytes
> > >>  #
> > >>  # @preallocation: Preallocation mode for the new image (since: 4.2)
> > >> @@ -4970,6 +4972,7 @@
> > >>  { 'struct': 'BlockdevCreateOptionsLUKS',
> > >>    'base': 'QCryptoBlockCreateOptionsLUKS',
> > >>    'data': { '*file':            'BlockdevRef',
> > >> +            '*header':          'BlockdevRef',
> > >>              'size':             'size',
> > >>              '*preallocation':   'PreallocMode' } }
> 
> 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 :|
> 
> 

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: [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create
Posted by Daniel P. Berrangé 10 months ago
On Tue, Jan 30, 2024 at 01:37:22PM +0800, yong.huang@smartx.com wrote:
> From: Hyman Huang <yong.huang@smartx.com>
> 
> Firstly, enable the ability to choose the block device containing
> a detachable LUKS header by adding the 'header' parameter to
> BlockdevCreateOptionsLUKS.
> 
> Secondly, when formatting the LUKS volume with a detachable header,
> truncate the payload volume to length without a header size.
> 
> Using the qmp blockdev command, create the LUKS volume with a
> detachable header as follows:
> 
> 1. add the secret to lock/unlock the cipher stored in the
>    detached LUKS header
> $ virsh qemu-monitor-command vm '{"execute":"object-add",
> > "arguments":{"qom-type": "secret", "id": "sec0", "data": "foo"}}'
> 
> 2. create a header img with 0 size
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> > "arguments":{"job-id":"job0", "options":{"driver":"file",
> > "filename":"/path/to/detached_luks_header.img", "size":0 }}}'
> 
> 3. add protocol blockdev node for header
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > "arguments": {"driver":"file", "filename":
> > "/path/to/detached_luks_header.img", "node-name":
> > "detached-luks-header-storage"}}'
> 
> 4. create a payload img with 0 size
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> > "arguments":{"job-id":"job1", "options":{"driver":"file",
> > "filename":"/path/to/detached_luks_payload_raw.img", "size":0}}}'
> 
> 5. add protocol blockdev node for payload
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > "arguments": {"driver":"file", "filename":
> > "/path/to/detached_luks_payload_raw.img", "node-name":
> > "luks-payload-raw-storage"}}'
> 
> 6. do the formatting with 128M size
> $ virsh qemu-monitor-command c81_node1 '{"execute":"blockdev-create",
> > "arguments":{"job-id":"job2", "options":{"driver":"luks", "header":
> > "detached-luks-header-storage", "file":"luks-payload-raw-storage",
> > "size":134217728, "preallocation":"full", "key-secret":"sec0" }}}'
> 
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
>  block/crypto.c       | 101 +++++++++++++++++++++++++++++++++++++++----
>  qapi/block-core.json |   3 ++
>  2 files changed, 96 insertions(+), 8 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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