[PATCH RESEND v3 00/10] Support generic Luks encryption

[PATCH RESEND v3 00/10] Support generic Luks encryption

[Hyman Huang]

v3:
- Rebase on master
- Add a test case for detached LUKS header
- Adjust the design to honour preallocation of the payload device
- Adjust the design to honour the payload offset from the header,
  even when detached
- Support detached LUKS header creation using qemu-img
- Support detached LUKS header querying
- Do some code clean

Thanks for commenting on this series, please review.

Best regared,

Yong

v2:
- Simplify the design by reusing the LUKS driver to implement
  the generic Luks encryption, thank Daniel for the insightful 
  advice.
- rebase on master. 

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.

By enhancing the LUKS driver, it is possible to enable
the detachable LUKS header and, as a result, achieve
general encryption for any disk format that QEMU has
supported.

Take the qcow2 as an example, the usage of the generic
LUKS encryption as follows:

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 LUKS
   header 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 link the qcow2 disk with
   LUKS header by specifying the field "header"
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments":{"node-name":"libvirt-2-format", "driver":"luks",
> "file":"libvirt-1-format", "header":"libvirt-2-storage",
> "key-secret":"libvirt-2-format-secret0"}}'

6. add the virtio-blk device finally
$ virsh qemu-monitor-command vm '{"execute":"device_add",
> "arguments": {"num-queues":"1", "driver":"virtio-blk-pci",
> "drive": "libvirt-2-format", "id":"virtio-disk2"}}'

The generic LUKS encryption method of starting a virtual
machine (VM) is somewhat similar to hot-plug in that both
maintaining the same json command while the starting VM
changes the "blockdev-add/device_add" parameters to
"blockdev/device".

Hyman Huang (10):
  crypto: Introduce option and structure for detached LUKS header
  crypto: Support generic LUKS encryption
  qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS
  crypto: Introduce creation option and structure for detached LUKS
    header
  crypto: Mark the payload_offset_sector invalid for detached LUKS
    header
  block: Support detached LUKS header creation using blockdev-create
  block: Support detached LUKS header creation using qemu-img
  crypto: Introduce 'detached-header' field in QCryptoBlockInfoLUKS
  tests: Add detached LUKS header case
  MAINTAINERS: Add section "Detached LUKS header"

 MAINTAINERS                                   |   5 +
 block.c                                       |   5 +-
 block/crypto.c                                | 146 ++++++++++--
 block/crypto.h                                |   8 +
 crypto/block-luks.c                           |  49 +++-
 crypto/block.c                                |   1 +
 crypto/blockpriv.h                            |   3 +
 qapi/block-core.json                          |  14 +-
 qapi/crypto.json                              |  13 +-
 tests/qemu-iotests/210.out                    |   4 +
 tests/qemu-iotests/tests/luks-detached-header | 214 ++++++++++++++++++
 .../tests/luks-detached-header.out            |   5 +
 12 files changed, 436 insertions(+), 31 deletions(-)
 create mode 100755 tests/qemu-iotests/tests/luks-detached-header
 create mode 100644 tests/qemu-iotests/tests/luks-detached-header.out

-- 
2.39.1

[PATCH RESEND v3 01/10] crypto: Introduce option and structure for detached LUKS header

[Hyman Huang]

Add the "header" option for the LUKS format. This field would be
used to identify the blockdev's position where a detachable LUKS
header is stored.

In addition, introduce header field in struct BlockCrypto

Signed-off-by: Hyman Huang <yong.huang@smartx.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <5b99f60c7317092a563d7ca3fb4b414197015eb2.1701879996.git.yong.huang@smartx.com>
---
 block/crypto.c       | 1 +
 qapi/block-core.json | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/block/crypto.c b/block/crypto.c
index 921933a5e5..f82b13d32b 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -39,6 +39,7 @@ typedef struct BlockCrypto BlockCrypto;
 struct BlockCrypto {
     QCryptoBlock *block;
     bool updating_keys;
+    BdrvChild *header;  /* Reference to the detached LUKS header */
 };
 
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index ca390c5700..10be08d08f 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 header. (since 9.0)
+#
 # Since: 2.9
 ##
 { 'struct': 'BlockdevOptionsLUKS',
   'base': 'BlockdevOptionsGenericFormat',
-  'data': { '*key-secret': 'str' } }
+  'data': { '*key-secret': 'str',
+            '*header': 'BlockdevRef'} }
 
 ##
 # @BlockdevOptionsGenericCOWFormat:
-- 
2.39.1

[PATCH RESEND v3 02/10] crypto: Support generic LUKS encryption

[Hyman Huang]

By enhancing the LUKS driver, it is possible to enable
the detachable LUKS header and, as a result, achieve
general encryption for any disk format that QEMU has
supported.

Take the qcow2 as an example, the usage of the generic
LUKS encryption as follows:

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 LUKS
   header 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 link the qcow2 disk with
   LUKS header by specifying the field "header"
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments":{"node-name":"libvirt-2-format", "driver":"luks",
> "file":"libvirt-1-format", "header":"libvirt-2-storage",
> "key-secret":"libvirt-2-format-secret0"}}'

6. add the virtio-blk device finally
$ virsh qemu-monitor-command vm '{"execute":"device_add",
> "arguments": {"num-queues":"1", "driver":"virtio-blk-pci",
> "drive": "libvirt-2-format", "id":"virtio-disk2"}}'

The generic LUKS encryption method of starting a virtual
machine (VM) is somewhat similar to hot-plug in that both
maintaining the same json command while the starting VM
changes the "blockdev-add/device_add" parameters to
"blockdev/device".

Signed-off-by: Hyman Huang <yong.huang@smartx.com>
Message-Id: <910801f303da1601051479d3b7e5c2c6b4e01eb7.1701879996.git.yong.huang@smartx.com>
---
 block/crypto.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/block/crypto.c b/block/crypto.c
index f82b13d32b..6063879bac 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -64,12 +64,14 @@ static int block_crypto_read_func(QCryptoBlock *block,
                                   Error **errp)
 {
     BlockDriverState *bs = opaque;
+    BlockCrypto *crypto = bs->opaque;
     ssize_t ret;
 
     GLOBAL_STATE_CODE();
     GRAPH_RDLOCK_GUARD_MAINLOOP();
 
-    ret = bdrv_pread(bs->file, offset, buflen, buf, 0);
+    ret = bdrv_pread(crypto->header ? crypto->header : bs->file,
+                     offset, buflen, buf, 0);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not read encryption header");
         return ret;
@@ -269,6 +271,7 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
     QCryptoBlockOpenOptions *open_opts = NULL;
     unsigned int cflags = 0;
     QDict *cryptoopts = NULL;
+    const char *hdr_bdref = qdict_get_try_str(options, "header");
 
     GLOBAL_STATE_CODE();
 
@@ -277,6 +280,15 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
         return ret;
     }
 
+    if (hdr_bdref) {
+        crypto->header = bdrv_open_child(NULL, options, "header", bs,
+                                         &child_of_bds, BDRV_CHILD_METADATA,
+                                         false, errp);
+        if (!crypto->header) {
+            return -EINVAL;
+        }
+    }
+
     GRAPH_RDLOCK_GUARD_MAINLOOP();
 
     bs->supported_write_flags = BDRV_REQ_FUA &
-- 
2.39.1

[PATCH RESEND v3 03/10] qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS

[Hyman Huang]

To support detached LUKS header creation, make the existing 'file'
filed in BlockdevCreateOptionsLUKS optional, while also adding an
extra optional 'header' field in the next commit.

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

diff --git a/block/crypto.c b/block/crypto.c
index 6063879bac..78fbe79c95 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -659,9 +659,9 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
     assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
     luks_opts = &create_options->u.luks;
 
-    bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
-    if (bs == NULL) {
-        return -EIO;
+    if (luks_opts->file == NULL) {
+        error_setg(errp, "Formatting LUKS disk requires parameter 'file'");
+        return -EINVAL;
     }
 
     create_opts = (QCryptoBlockCreateOptions) {
@@ -673,10 +673,17 @@ 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,
-                                         preallocation, errp);
-    if (ret < 0) {
-        goto fail;
+    if (luks_opts->file) {
+        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);
+        if (ret < 0) {
+            goto fail;
+        }
     }
 
     ret = 0;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 10be08d08f..9ac256c489 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4945,7 +4945,8 @@
 #
 # Driver specific image creation options for LUKS.
 #
-# @file: Node to create the image format on
+# @file: Node to create the image format on, mandatory except when
+#        'preallocation' is not requested
 #
 # @size: Size of the virtual disk in bytes
 #
@@ -4956,7 +4957,7 @@
 ##
 { 'struct': 'BlockdevCreateOptionsLUKS',
   'base': 'QCryptoBlockCreateOptionsLUKS',
-  'data': { 'file':             'BlockdevRef',
+  'data': { '*file':            'BlockdevRef',
             'size':             'size',
             '*preallocation':   'PreallocMode' } }
 
-- 
2.39.1

[PATCH RESEND v3 04/10] crypto: Introduce creation option and structure for detached LUKS header

[Hyman Huang]

Introduce 'header' field in BlockdevCreateOptionsLUKS to support
detached LUKS header creation. Meanwhile, introduce header-related
field in QCryptoBlock.

Signed-off-by: Hyman Huang <yong.huang@smartx.com>
---
 crypto/blockpriv.h   | 3 +++
 qapi/block-core.json | 3 +++
 qapi/crypto.json     | 5 ++++-
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/crypto/blockpriv.h b/crypto/blockpriv.h
index 3c7ccea504..6289aea961 100644
--- a/crypto/blockpriv.h
+++ b/crypto/blockpriv.h
@@ -42,6 +42,9 @@ struct QCryptoBlock {
     size_t niv;
     uint64_t payload_offset; /* In bytes */
     uint64_t sector_size; /* In bytes */
+
+    bool detached_header; /* True if disk has a detached LUKS header */
+    uint64_t detached_header_size; /* LUKS header size plus key slot size */
 };
 
 struct QCryptoBlockDriver {
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9ac256c489..8aec179926 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4948,6 +4948,8 @@
 # @file: Node to create the image format on, mandatory except when
 #        'preallocation' is not requested
 #
+# @header: Detached LUKS header node to format. (since 9.0)
+#
 # @size: Size of the virtual disk in bytes
 #
 # @preallocation: Preallocation mode for the new image (since: 4.2)
@@ -4958,6 +4960,7 @@
 { 'struct': 'BlockdevCreateOptionsLUKS',
   'base': 'QCryptoBlockCreateOptionsLUKS',
   'data': { '*file':            'BlockdevRef',
+            '*header':          'BlockdevRef',
             'size':             'size',
             '*preallocation':   'PreallocMode' } }
 
diff --git a/qapi/crypto.json b/qapi/crypto.json
index fd3d46ebd1..6b4e86cb81 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -195,10 +195,13 @@
 #     decryption key.  Mandatory except when probing image for
 #     metadata only.
 #
+# @detached-header: if true, disk has detached LUKS header.
+#
 # Since: 2.6
 ##
 { 'struct': 'QCryptoBlockOptionsLUKS',
-  'data': { '*key-secret': 'str' }}
+  'data': { '*key-secret': 'str',
+            '*detached-header': 'bool' }}
 
 ##
 # @QCryptoBlockCreateOptionsLUKS:
-- 
2.39.1

[PATCH RESEND v3 05/10] crypto: Mark the payload_offset_sector invalid for detached LUKS header

[Hyman Huang]

Set the payload_offset_sector to a value that is nearly never reached
in order to mark it as invalid and indicate that 0 should be the offset
of the read/write operation on the 'file' protocol blockdev node.

Signed-off-by: Hyman Huang <yong.huang@smartx.com>
---
 crypto/block-luks.c | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index fb01ec38bb..48443ffcae 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -34,6 +34,8 @@
 
 #include "qemu/bitmap.h"
 
+#define INVALID_SECTOR_OFFSET UINT32_MAX
+
 /*
  * Reference for the LUKS format implemented here is
  *
@@ -136,6 +138,13 @@ struct QCryptoBlockLUKS {
 };
 
 
+static inline uint32_t
+qcrypto_block_luks_payload_offset(uint32_t sector)
+{
+    return sector == INVALID_SECTOR_OFFSET ? 0 :
+        sector * QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
+}
+
 static int qcrypto_block_luks_cipher_name_lookup(const char *name,
                                                  QCryptoCipherMode mode,
                                                  uint32_t key_bytes,
@@ -1255,8 +1264,8 @@ qcrypto_block_luks_open(QCryptoBlock *block,
     }
 
     block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
-    block->payload_offset = luks->header.payload_offset_sector *
-        block->sector_size;
+    block->payload_offset =
+        qcrypto_block_luks_payload_offset(luks->header.payload_offset_sector);
 
     return 0;
 
@@ -1529,16 +1538,28 @@ qcrypto_block_luks_create(QCryptoBlock *block,
         slot->stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
     }
 
-    /* The total size of the LUKS headers is the partition header + key
-     * slot headers, rounded up to the nearest sector, combined with
-     * the size of each master key material region, also rounded up
-     * to the nearest sector */
-    luks->header.payload_offset_sector = header_sectors +
-            QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS * split_key_sectors;
+    if (block->detached_header) {
+        /*
+         * Set the payload_offset_sector to a value that is nearly never
+         * reached in order to mark it as invalid and indicate that 0 should
+         * be the offset of the read/write operation on the 'file' protocol
+         * blockdev node. Here the UINT32_MAX is choosed
+         */
+        luks->header.payload_offset_sector = INVALID_SECTOR_OFFSET;
+    } else {
+        /*
+         * The total size of the LUKS headers is the partition header + key
+         * slot headers, rounded up to the nearest sector, combined with
+         * the size of each master key material region, also rounded up
+         * to the nearest sector
+         */
+        luks->header.payload_offset_sector = header_sectors +
+                QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS * split_key_sectors;
+    }
 
     block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
-    block->payload_offset = luks->header.payload_offset_sector *
-        block->sector_size;
+    block->payload_offset =
+        qcrypto_block_luks_payload_offset(luks->header.payload_offset_sector);
 
     /* Reserve header space to match payload offset */
     initfunc(block, block->payload_offset, opaque, &local_err);
-- 
2.39.1

[PATCH RESEND v3 06/10] block: Support detached LUKS header creation using blockdev-create

[Hyman Huang]

The LUKS disk with detached header consists of a separate LUKS
header and payload. This LUKS disk type should be formatted
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      | 109 ++++++++++++++++++++++++++++++++++++++++----
 crypto/block-luks.c |   6 ++-
 crypto/block.c      |   1 +
 3 files changed, 106 insertions(+), 10 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index 78fbe79c95..76cc8bda49 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -160,6 +160,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",
@@ -651,6 +693,7 @@ 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;
@@ -659,8 +702,22 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
     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' should "
+                   "be specified");
+        return -EINVAL;
+    }
+
+    if (luks_opts->detached_header && luks_opts->header == NULL) {
+        error_setg(errp, "Formatting a detached LUKS disk requries "
+                   "'header' to be specified");
+        return -EINVAL;
+    }
+
+    if ((luks_opts->preallocation != PREALLOC_MODE_OFF) &&
+        (luks_opts->file == NULL)) {
+        error_setg(errp, "Parameter 'preallocation' requries 'file' to be "
+                   "specified for formatting LUKS disk");
         return -EINVAL;
     }
 
@@ -673,7 +730,40 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
         preallocation = luks_opts->preallocation;
     }
 
-    if (luks_opts->file) {
+    if (luks_opts->header) {
+        hdr_bs = bdrv_co_open_blockdev_ref(luks_opts->header, errp);
+        if (hdr_bs == NULL) {
+            return -EIO;
+        }
+
+        /*
+         * If blockdev reference of header is specified,
+         * detached_header default to true
+         */
+        create_opts.u.luks.detached_header = true;
+
+        /* Format the LUKS header node */
+        ret = block_crypto_co_create_generic(hdr_bs, 0, &create_opts,
+                                             PREALLOC_MODE_OFF, errp);
+        if (ret < 0) {
+            goto hdr_bs_failed;
+        }
+
+        /* Format the LUKS payload node */
+        if (luks_opts->file) {
+            ret = block_crypto_co_format_luks_payload(luks_opts, errp);
+            if (ret < 0) {
+                goto hdr_bs_failed;
+            }
+        }
+
+        ret = 0;
+
+hdr_bs_failed:
+        bdrv_co_unref(hdr_bs);
+        return ret;
+    } else if (luks_opts->file) {
+        /* None detached LUKS header path */
         bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
         if (bs == NULL) {
             return -EIO;
@@ -682,14 +772,15 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
         ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
                                              preallocation, errp);
         if (ret < 0) {
-            goto fail;
+            goto bs_failed;
         }
-    }
 
-    ret = 0;
-fail:
-    bdrv_co_unref(bs);
-    return ret;
+        ret = 0;
+
+bs_failed:
+        bdrv_co_unref(bs);
+        return ret;
+    }
 }
 
 static int coroutine_fn GRAPH_UNLOCKED
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 48443ffcae..474c7aee2e 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -1561,8 +1561,12 @@ qcrypto_block_luks_create(QCryptoBlock *block,
     block->payload_offset =
         qcrypto_block_luks_payload_offset(luks->header.payload_offset_sector);
 
+    block->detached_header_size =
+        (header_sectors + QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS *
+         split_key_sectors) * block->sector_size;
+
     /* Reserve header space to match payload offset */
-    initfunc(block, block->payload_offset, opaque, &local_err);
+    initfunc(block, block->detached_header_size, opaque, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         goto error;
diff --git a/crypto/block.c b/crypto/block.c
index 7bb4b74a37..ea493f056e 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -102,6 +102,7 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
     }
 
     block->driver = qcrypto_block_drivers[options->format];
+    block->detached_header = options->u.luks.detached_header;
 
     if (block->driver->create(block, options, optprefix, initfunc,
                               writefunc, opaque, errp) < 0) {
-- 
2.39.1

[PATCH RESEND v3 07/10] block: Support detached LUKS header creation using qemu-img

[Hyman Huang]

Add the 'detached-mode' 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-mode=true header.luks

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

diff --git a/block.c b/block.c
index bfb0861ec6..fa9ce36928 100644
--- a/block.c
+++ b/block.c
@@ -7517,7 +7517,10 @@ void bdrv_img_create(const char *filename, const char *fmt,
         goto out;
     }
 
-    if (size == -1) {
+    /* Parameter 'size' is not needed for detached LUKS header */
+    if (size == -1 &&
+        !(!strcmp(fmt, "luks") &&
+          qemu_opt_get_bool(opts, "detached-mode", false))) {
         error_setg(errp, "Image creation needs a size parameter");
         goto out;
     }
diff --git a/block/crypto.c b/block/crypto.c
index 76cc8bda49..812c3c28f5 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -229,6 +229,7 @@ static QemuOptsList block_crypto_create_opts_luks = {
         BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG(""),
         BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG(""),
         BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME(""),
+        BLOCK_CRYPTO_OPT_DEF_LUKS_DETACHED_MODE(""),
         { /* end of list */ }
     },
 };
@@ -793,6 +794,8 @@ block_crypto_co_create_opts_luks(BlockDriver *drv, const char *filename,
     PreallocMode prealloc;
     char *buf = NULL;
     int64_t size;
+    bool detached_mode =
+        qemu_opt_get_bool(opts, "detached-mode", false);
     int ret;
     Error *local_err = NULL;
 
@@ -832,8 +835,12 @@ block_crypto_co_create_opts_luks(BlockDriver *drv, const char *filename,
         goto fail;
     }
 
+   /* The detached_header default to true if detached-mode is specified */
+    create_opts->u.luks.detached_header = detached_mode ? true : false;
+
     /* Create format layer */
-    ret = block_crypto_co_create_generic(bs, size, create_opts, prealloc, errp);
+    ret = block_crypto_co_create_generic(bs, detached_mode ? 0 : size,
+                                         create_opts, prealloc, errp);
     if (ret < 0) {
         goto fail;
     }
diff --git a/block/crypto.h b/block/crypto.h
index 72e792c9af..bceefd45bd 100644
--- a/block/crypto.h
+++ b/block/crypto.h
@@ -41,6 +41,7 @@
 #define BLOCK_CRYPTO_OPT_LUKS_IVGEN_HASH_ALG "ivgen-hash-alg"
 #define BLOCK_CRYPTO_OPT_LUKS_HASH_ALG "hash-alg"
 #define BLOCK_CRYPTO_OPT_LUKS_ITER_TIME "iter-time"
+#define BLOCK_CRYPTO_OPT_LUKS_DETACHED_MODE "detached-mode"
 #define BLOCK_CRYPTO_OPT_LUKS_KEYSLOT "keyslot"
 #define BLOCK_CRYPTO_OPT_LUKS_STATE "state"
 #define BLOCK_CRYPTO_OPT_LUKS_OLD_SECRET "old-secret"
@@ -100,6 +101,13 @@
         .help = "Select new state of affected keyslots (active/inactive)",\
     }
 
+#define BLOCK_CRYPTO_OPT_DEF_LUKS_DETACHED_MODE(prefix)     \
+    {                                                         \
+        .name = prefix BLOCK_CRYPTO_OPT_LUKS_DETACHED_MODE, \
+        .type = QEMU_OPT_BOOL,                                \
+        .help = "Create a detached LUKS header",              \
+    }
+
 #define BLOCK_CRYPTO_OPT_DEF_LUKS_KEYSLOT(prefix)              \
     {                                                          \
         .name = prefix BLOCK_CRYPTO_OPT_LUKS_KEYSLOT,          \
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 6b4e86cb81..8e81aa8454 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -226,6 +226,8 @@
 # @iter-time: number of milliseconds to spend in PBKDF passphrase
 #     processing.  Currently defaults to 2000. (since 2.8)
 #
+# @detached-mode: create a detached LUKS header. (since 9.0)
+#
 # Since: 2.6
 ##
 { 'struct': 'QCryptoBlockCreateOptionsLUKS',
@@ -235,7 +237,8 @@
             '*ivgen-alg': 'QCryptoIVGenAlgorithm',
             '*ivgen-hash-alg': 'QCryptoHashAlgorithm',
             '*hash-alg': 'QCryptoHashAlgorithm',
-            '*iter-time': 'int'}}
+            '*iter-time': 'int',
+            '*detached-mode': 'bool'}}
 
 ##
 # @QCryptoBlockOpenOptions:
-- 
2.39.1

[PATCH RESEND v3 08/10] crypto: Introduce 'detached-header' field in QCryptoBlockInfoLUKS

[Hyman Huang]

When querying the LUKS disk with the qemu-img tool or other APIs,
add information about whether the LUKS header is detached.

Additionally, update the test case with the appropriate
modification.

Signed-off-by: Hyman Huang <yong.huang@smartx.com>
---
 crypto/block-luks.c        | 2 ++
 qapi/crypto.json           | 3 +++
 tests/qemu-iotests/210.out | 4 ++++
 3 files changed, 9 insertions(+)

diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 474c7aee2e..c5e53b4ee4 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -1266,6 +1266,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
     block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
     block->payload_offset =
         qcrypto_block_luks_payload_offset(luks->header.payload_offset_sector);
+    block->detached_header = (block->payload_offset == 0) ? true : false;
 
     return 0;
 
@@ -1892,6 +1893,7 @@ static int qcrypto_block_luks_get_info(QCryptoBlock *block,
     info->u.luks.master_key_iters = luks->header.master_key_iterations;
     info->u.luks.uuid = g_strndup((const char *)luks->header.uuid,
                                   sizeof(luks->header.uuid));
+    info->u.luks.detached_header = block->detached_header;
 
     for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
         slot = g_new0(QCryptoBlockInfoLUKSSlot, 1);
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 8e81aa8454..336c880b5d 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -317,6 +317,8 @@
 #
 # @hash-alg: the master key hash algorithm
 #
+# @detached-header: whether the LUKS header is detached (Since 9.0)
+#
 # @payload-offset: offset to the payload data in bytes
 #
 # @master-key-iters: number of PBKDF2 iterations for key material
@@ -333,6 +335,7 @@
            'ivgen-alg': 'QCryptoIVGenAlgorithm',
            '*ivgen-hash-alg': 'QCryptoHashAlgorithm',
            'hash-alg': 'QCryptoHashAlgorithm',
+           'detached-header': 'bool',
            'payload-offset': 'int',
            'master-key-iters': 'int',
            'uuid': 'str',
diff --git a/tests/qemu-iotests/210.out b/tests/qemu-iotests/210.out
index 96d9f749dd..94b29b2120 100644
--- a/tests/qemu-iotests/210.out
+++ b/tests/qemu-iotests/210.out
@@ -18,6 +18,7 @@ virtual size: 128 MiB (134217728 bytes)
 encrypted: yes
 Format specific information:
     ivgen alg: plain64
+    detached header: false
     hash alg: sha256
     cipher alg: aes-256
     uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
@@ -70,6 +71,7 @@ virtual size: 64 MiB (67108864 bytes)
 encrypted: yes
 Format specific information:
     ivgen alg: plain64
+    detached header: false
     hash alg: sha1
     cipher alg: aes-128
     uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
@@ -125,6 +127,7 @@ virtual size: 0 B (0 bytes)
 encrypted: yes
 Format specific information:
     ivgen alg: plain64
+    detached header: false
     hash alg: sha256
     cipher alg: aes-256
     uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
@@ -195,6 +198,7 @@ virtual size: 0 B (0 bytes)
 encrypted: yes
 Format specific information:
     ivgen alg: plain64
+    detached header: false
     hash alg: sha256
     cipher alg: aes-256
     uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
-- 
2.39.1

[PATCH RESEND v3 09/10] tests: Add detached LUKS header case

[Hyman Huang]

Signed-off-by: Hyman Huang <yong.huang@smartx.com>
---
 tests/qemu-iotests/tests/luks-detached-header | 214 ++++++++++++++++++
 .../tests/luks-detached-header.out            |   5 +
 2 files changed, 219 insertions(+)
 create mode 100755 tests/qemu-iotests/tests/luks-detached-header
 create mode 100644 tests/qemu-iotests/tests/luks-detached-header.out

diff --git a/tests/qemu-iotests/tests/luks-detached-header b/tests/qemu-iotests/tests/luks-detached-header
new file mode 100755
index 0000000000..cf305bfa47
--- /dev/null
+++ b/tests/qemu-iotests/tests/luks-detached-header
@@ -0,0 +1,214 @@
+#!/usr/bin/env python3
+# group: rw auto
+#
+# Test detached LUKS header
+#
+# Copyright (C) 2024 SmartX Inc.
+#
+# Authors:
+#     Hyman Huang <yong.huang@smartx.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import os
+import iotests
+from iotests import imgfmt, qemu_img_create, img_info_log, qemu_img_info, QMPTestCase
+
+
+image_size = 128 * 1024 * 1024
+
+luks_img = os.path.join(iotests.test_dir, 'luks.img')
+detached_header_img1 = os.path.join(iotests.test_dir, 'detached_header.img1')
+detached_header_img2 = os.path.join(iotests.test_dir, 'detached_header.img2')
+detached_payload_raw_img = os.path.join(iotests.test_dir, 'detached_payload_raw.img')
+detached_payload_qcow2_img = os.path.join(iotests.test_dir, 'detached_payload_qcow2.img')
+
+secret_obj = 'secret,id=sec0,data=foo'
+luks_opts = 'key-secret=sec0'
+
+
+class TestDetachedLUKSHeader(QMPTestCase):
+    def setUp(self) -> None:
+        self.vm = iotests.VM()
+        self.vm.add_object(secret_obj)
+        self.vm.launch()
+
+        # 1. Create the normal LUKS disk with 128M size
+        self.vm.blockdev_create({ 'driver': 'file',
+                                  'filename': luks_img,
+                                  'size': 0 })
+        self.vm.qmp_log('blockdev-add', driver='file', filename=luks_img,
+                         node_name='luks-1-storage')
+        result = self.vm.blockdev_create({ 'driver': imgfmt,
+                                           'file': 'luks-1-storage',
+                                           'key-secret': 'sec0',
+                                           'size': image_size,
+                                           'iter-time': 10 })
+        # None is expected
+        self.assertEqual(result, None)
+
+        # 2. Create the LUKS disk with detached header (raw)
+
+        # Create detached LUKS header
+        self.vm.blockdev_create({ 'driver': 'file',
+                                  'filename': detached_header_img1,
+                                  'size': 0 })
+        self.vm.qmp_log('blockdev-add', driver='file', filename=detached_header_img1,
+                         node_name='luks-2-header-storage')
+
+        # Create detached LUKS raw payload
+        self.vm.blockdev_create({ 'driver': 'file',
+                                  'filename': detached_payload_raw_img,
+                                  'size': 0 })
+        self.vm.qmp_log('blockdev-add', driver='file',
+                         filename=detached_payload_raw_img,
+                         node_name='luks-2-payload-storage')
+
+        # Format LUKS disk with detached header
+        result = self.vm.blockdev_create({ 'driver': imgfmt,
+                                           'header': 'luks-2-header-storage',
+                                           'file': 'luks-2-payload-storage',
+                                           'key-secret': 'sec0',
+                                           'preallocation': 'full',
+                                           'size': image_size,
+                                           'iter-time': 10 })
+        self.assertEqual(result, None)
+
+        self.vm.shutdown()
+
+        # 3. Create the LUKS disk with detached header (qcow2)
+
+        # Create detached LUKS header using qemu-img
+        res = qemu_img_create('-f', 'luks', '--object', secret_obj, '-o', luks_opts,
+                              '-o', "detached-mode=true", detached_header_img2)
+        assert res.returncode == 0
+
+        # Create detached LUKS qcow2 payload
+        res = qemu_img_create('-f', 'qcow2', detached_payload_qcow2_img, str(image_size))
+        assert res.returncode == 0
+
+    def tearDown(self) -> None:
+        os.remove(luks_img)
+        os.remove(detached_header_img1)
+        os.remove(detached_header_img2)
+        os.remove(detached_payload_raw_img)
+        os.remove(detached_payload_qcow2_img)
+
+        # Check if there was any qemu-io run that failed
+        if 'Pattern verification failed' in self.vm.get_log():
+            print('ERROR: Pattern verification failed:')
+            print(self.vm.get_log())
+            self.fail('qemu-io pattern verification failed')
+
+    def test_img_creation(self) -> None:
+        # Check if the images created above are expected
+
+        data = qemu_img_info(luks_img)['format-specific']
+        self.assertEqual(data['type'], imgfmt)
+        self.assertEqual(data['data']['detached-header'], False)
+
+        data = qemu_img_info(detached_header_img1)['format-specific']
+        self.assertEqual(data['type'], imgfmt)
+        self.assertEqual(data['data']['detached-header'], True)
+
+        data = qemu_img_info(detached_header_img2)['format-specific']
+        self.assertEqual(data['type'], imgfmt)
+        self.assertEqual(data['data']['detached-header'], True)
+
+        # Check if preallocation works
+        size = qemu_img_info(detached_payload_raw_img)['actual-size']
+        self.assertGreaterEqual(size, image_size)
+
+    def test_detached_luks_header(self) -> None:
+        self.vm.launch()
+
+        # 1. Add the disk created above
+
+        # Add normal LUKS disk
+        self.vm.qmp_log('blockdev-add', driver='file', filename=luks_img,
+                         node_name='luks-1-storage')
+        result = self.vm.qmp_log('blockdev-add', driver='luks', file='luks-1-storage',
+                                  key_secret='sec0', node_name='luks-1-format')
+
+        # Expected result{ "return": {} }
+        self.assert_qmp(result, 'return', {})
+
+        # Add detached LUKS header with raw payload
+        self.vm.qmp_log('blockdev-add', driver='file', filename=detached_header_img1,
+                         node_name='luks-header1-storage')
+
+        self.vm.qmp_log('blockdev-add', driver='file', filename=detached_payload_raw_img,
+                         node_name='luks-2-payload-raw-storage')
+
+        result = self.vm.qmp_log('blockdev-add', driver=imgfmt,
+                                  header='luks-header1-storage',
+                                  file='luks-2-payload-raw-storage',
+                                  key_secret='sec0',
+                                  node_name='luks-2-payload-raw-format')
+        self.assert_qmp(result, 'return', {})
+
+        # Add detached LUKS header with qcow2 payload
+        self.vm.qmp_log('blockdev-add', driver='file', filename=detached_header_img2,
+                         node_name='luks-header2-storage')
+
+        self.vm.qmp_log('blockdev-add', driver='file', filename=detached_payload_qcow2_img,
+                         node_name='luks-3-payload-qcow2-storage')
+
+        result = self.vm.qmp_log('blockdev-add', driver=imgfmt,
+                                  header='luks-header2-storage',
+                                  file='luks-3-payload-qcow2-storage',
+                                  key_secret='sec0',
+                                  node_name='luks-3-payload-qcow2-format')
+        self.assert_qmp(result, 'return', {})
+
+        # 2. Do I/O test
+
+        # Do some I/O to the image to see whether it still works
+        # (Pattern verification will be checked by tearDown())
+
+        # Normal LUKS disk
+        result = self.vm.qmp_log('human-monitor-command',
+                                  command_line='qemu-io luks-1-format "write -P 40 0 64k"')
+        self.assert_qmp(result, 'return', '')
+
+        result = self.vm.qmp_log('human-monitor-command',
+                                 command_line='qemu-io luks-1-format "read -P 40 0 64k"')
+        self.assert_qmp(result, 'return', '')
+
+        # Detached LUKS header with raw payload
+        result = self.vm.qmp('human-monitor-command',
+                             command_line='qemu-io luks-2-payload-raw-format "write -P 41 0 64k"')
+        self.assert_qmp(result, 'return', '')
+
+        result = self.vm.qmp('human-monitor-command',
+                             command_line='qemu-io luks-2-payload-raw-format "read -P 41 0 64k"')
+        self.assert_qmp(result, 'return', '')
+
+        # Detached LUKS header with qcow2 payload
+        result = self.vm.qmp('human-monitor-command',
+                             command_line='qemu-io luks-3-payload-qcow2-format "write -P 42 0 64k"')
+        self.assert_qmp(result, 'return', '')
+
+        result = self.vm.qmp('human-monitor-command',
+                             command_line='qemu-io luks-3-payload-qcow2-format "read -P 42 0 64k"')
+        self.assert_qmp(result, 'return', '')
+
+        self.vm.shutdown()
+
+
+if __name__ == '__main__':
+    # Test image creation and I/O
+    iotests.main(supported_fmts=['luks'],
+                 supported_protocols=['file'])
diff --git a/tests/qemu-iotests/tests/luks-detached-header.out b/tests/qemu-iotests/tests/luks-detached-header.out
new file mode 100644
index 0000000000..fbc63e62f8
--- /dev/null
+++ b/tests/qemu-iotests/tests/luks-detached-header.out
@@ -0,0 +1,5 @@
+..
+----------------------------------------------------------------------
+Ran 2 tests
+
+OK
-- 
2.39.1

[PATCH RESEND v3 10/10] MAINTAINERS: Add section "Detached LUKS header"

[Hyman Huang]

I've built interests in block cryptography and also
have been working on projects related to this
subsystem.

Add a section to the MAINTAINERS file for detached
LUKS header, it only has a test case in it currently.

Signed-off-by: Hyman Huang <yong.huang@smartx.com>
---
 MAINTAINERS | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 395f26ba86..f0f7b889a3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3391,6 +3391,11 @@ F: migration/dirtyrate.c
 F: migration/dirtyrate.h
 F: include/sysemu/dirtyrate.h
 
+Detached LUKS header
+M: Hyman Huang <yong.huang@smartx.com>
+S: Maintained
+F: tests/qemu-iotests/tests/luks-detached-header
+
 D-Bus
 M: Marc-André Lureau <marcandre.lureau@redhat.com>
 S: Maintained
-- 
2.39.1

Re: [PATCH RESEND v3 02/10] crypto: Support generic LUKS encryption

[Daniel P. Berrangé]

On Mon, Dec 25, 2023 at 01:45:04PM +0800, Hyman Huang wrote:
> By enhancing the LUKS driver, it is possible to enable
> the detachable LUKS header and, as a result, achieve
> general encryption for any disk format that QEMU has
> supported.
> 
> Take the qcow2 as an example, the usage of the generic
> LUKS encryption as follows:
> 
> 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 LUKS
>    header 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 link the qcow2 disk with
>    LUKS header by specifying the field "header"
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > "arguments":{"node-name":"libvirt-2-format", "driver":"luks",
> > "file":"libvirt-1-format", "header":"libvirt-2-storage",
> > "key-secret":"libvirt-2-format-secret0"}}'
> 
> 6. add the virtio-blk device finally
> $ virsh qemu-monitor-command vm '{"execute":"device_add",
> > "arguments": {"num-queues":"1", "driver":"virtio-blk-pci",
> > "drive": "libvirt-2-format", "id":"virtio-disk2"}}'
> 
> The generic LUKS encryption method of starting a virtual
> machine (VM) is somewhat similar to hot-plug in that both
> maintaining the same json command while the starting VM
> changes the "blockdev-add/device_add" parameters to
> "blockdev/device".
> 
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> Message-Id: <910801f303da1601051479d3b7e5c2c6b4e01eb7.1701879996.git.yong.huang@smartx.com>
> ---
>  block/crypto.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/block/crypto.c b/block/crypto.c
> index f82b13d32b..6063879bac 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -64,12 +64,14 @@ static int block_crypto_read_func(QCryptoBlock *block,
>                                    Error **errp)
>  {
>      BlockDriverState *bs = opaque;
> +    BlockCrypto *crypto = bs->opaque;
>      ssize_t ret;
>  
>      GLOBAL_STATE_CODE();
>      GRAPH_RDLOCK_GUARD_MAINLOOP();
>  
> -    ret = bdrv_pread(bs->file, offset, buflen, buf, 0);
> +    ret = bdrv_pread(crypto->header ? crypto->header : bs->file,
> +                     offset, buflen, buf, 0);
>      if (ret < 0) {
>          error_setg_errno(errp, -ret, "Could not read encryption header");
>          return ret;
> @@ -269,6 +271,7 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
>      QCryptoBlockOpenOptions *open_opts = NULL;
>      unsigned int cflags = 0;
>      QDict *cryptoopts = NULL;
> +    const char *hdr_bdref = qdict_get_try_str(options, "header");

This is an invalid check to make, because it is assuming the user is
referencing a separate blockdev node name and doesn't work for an
inline definition. eg

  qemu-img info  'json:{"driver":"luks","file":{"filename":"test-payload.img"},"header":{"filename":"test-header.img"}}'


>  
>      GLOBAL_STATE_CODE();
>  
> @@ -277,6 +280,15 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
>          return ret;
>      }
>  
> +    if (hdr_bdref) {

Get rid of this 'if' clause and unconditionally call the next line:

> +        crypto->header = bdrv_open_child(NULL, options, "header", bs,
> +                                         &child_of_bds, BDRV_CHILD_METADATA,
> +                                         false, errp);

but pass 'true' instead of 'false' here to allow the child to be absent,
and thus let it return NULL.

> +        if (!crypto->header) {

You'll need to then check  '*errp != NULL' instead

You'll also need "ERRP_GUARD" at the start of the method

> +            return -EINVAL;
> +        }
> +    }
> +
>      GRAPH_RDLOCK_GUARD_MAINLOOP();
>  
>      bs->supported_write_flags = BDRV_REQ_FUA &

This patch should be combined with the previous patch that adds the new
QAPI schema element as splitting them doesn't add value.


Testing this patch with the changes I suggest above, however, still does
not work:

  $ 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
  $ qemu-img info  'json:{"driver":"luks","file":{"filename":"test-payload.img"},"header":{"filename":"test-header.img"}}'
  qemu-img: Could not open 'json:{"driver":"luks","file":{"filename":"test-payload.img"},"header":{"filename":"test-header.img"}}': LUKS payload is overlapping with the header


You need to pass some info into qcrypto_block_open to tell it that the
header is detached. Add a new enum entry to QCryptoBlockOpenFlags
perhaps.  Then skip the LUKS payload overlap check.

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 RESEND v3 03/10] qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS

[Daniel P. Berrangé]

On Mon, Dec 25, 2023 at 01:45:05PM +0800, Hyman Huang wrote:
> To support detached LUKS header creation, make the existing 'file'
> filed in BlockdevCreateOptionsLUKS optional, while also adding an
> extra optional 'header' field in the next commit.
> 
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
>  block/crypto.c       | 21 ++++++++++++++-------
>  qapi/block-core.json |  5 +++--
>  2 files changed, 17 insertions(+), 9 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 :|

Re: [PATCH RESEND v3 06/10] block: Support detached LUKS header creation using blockdev-create

[Daniel P. Berrangé]

On Mon, Dec 25, 2023 at 01:45:08PM +0800, Hyman Huang wrote:
> The LUKS disk with detached header consists of a separate LUKS
> header and payload. This LUKS disk type should be formatted
> 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      | 109 ++++++++++++++++++++++++++++++++++++++++----
>  crypto/block-luks.c |   6 ++-
>  crypto/block.c      |   1 +
>  3 files changed, 106 insertions(+), 10 deletions(-)
> 
> diff --git a/block/crypto.c b/block/crypto.c
> index 78fbe79c95..76cc8bda49 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -160,6 +160,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",
> @@ -651,6 +693,7 @@ 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;
> @@ -659,8 +702,22 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
>      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' should "

s/should/must/

> +                   "be specified");
> +        return -EINVAL;
> +    }
> +
> +    if (luks_opts->detached_header && luks_opts->header == NULL) {
> +        error_setg(errp, "Formatting a detached LUKS disk requries "

typo   s/requries/requires/

> +                   "'header' to be specified");
> +        return -EINVAL;
> +    }
> +
> +    if ((luks_opts->preallocation != PREALLOC_MODE_OFF) &&
> +        (luks_opts->file == NULL)) {
> +        error_setg(errp, "Parameter 'preallocation' requries 'file' to be "

typo   s/requries/requires/

> +                   "specified for formatting LUKS disk");
>          return -EINVAL;
>      }
>  
> @@ -673,7 +730,40 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
>          preallocation = luks_opts->preallocation;
>      }
>  
> -    if (luks_opts->file) {
> +    if (luks_opts->header) {
> +        hdr_bs = bdrv_co_open_blockdev_ref(luks_opts->header, errp);
> +        if (hdr_bs == NULL) {
> +            return -EIO;
> +        }
> +
> +        /*
> +         * If blockdev reference of header is specified,
> +         * detached_header default to true
> +         */
> +        create_opts.u.luks.detached_header = true;
> +
> +        /* Format the LUKS header node */
> +        ret = block_crypto_co_create_generic(hdr_bs, 0, &create_opts,
> +                                             PREALLOC_MODE_OFF, errp);
> +        if (ret < 0) {
> +            goto hdr_bs_failed;
> +        }
> +
> +        /* Format the LUKS payload node */
> +        if (luks_opts->file) {
> +            ret = block_crypto_co_format_luks_payload(luks_opts, errp);
> +            if (ret < 0) {
> +                goto hdr_bs_failed;
> +            }
> +        }
> +
> +        ret = 0;
> +
> +hdr_bs_failed:

I'd suggest we just make the existing 'fail:' label
cope with unref'ing  hdr_bs if it is non-NULL,
rather than having multiple goto label choices.

> +        bdrv_co_unref(hdr_bs);
> +        return ret;
> +    } else if (luks_opts->file) {
> +        /* None detached LUKS header path */
>          bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
>          if (bs == NULL) {
>              return -EIO;
> @@ -682,14 +772,15 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
>          ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
>                                               preallocation, errp);
>          if (ret < 0) {
> -            goto fail;
> +            goto bs_failed;
>          }

> -    }
>  
> -    ret = 0;
> -fail:
> -    bdrv_co_unref(bs);
> -    return ret;
> +        ret = 0;
> +
> +bs_failed:
> +        bdrv_co_unref(bs);
> +        return ret;
> +    }
>  }
>  
>  static int coroutine_fn GRAPH_UNLOCKED
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index 48443ffcae..474c7aee2e 100644
> --- a/crypto/block-luks.c
> +++ b/crypto/block-luks.c
> @@ -1561,8 +1561,12 @@ qcrypto_block_luks_create(QCryptoBlock *block,
>      block->payload_offset =
>          qcrypto_block_luks_payload_offset(luks->header.payload_offset_sector);
>  
> +    block->detached_header_size =
> +        (header_sectors + QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS *
> +         split_key_sectors) * block->sector_size;

Storing this in 'detached_header_size' struct field looks redundant,
as the onlY place that reads it is the next line. Remove the struct
field and use a local variable.

> +
>      /* Reserve header space to match payload offset */
> -    initfunc(block, block->payload_offset, opaque, &local_err);
> +    initfunc(block, block->detached_header_size, opaque, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          goto error;
> diff --git a/crypto/block.c b/crypto/block.c
> index 7bb4b74a37..ea493f056e 100644
> --- a/crypto/block.c
> +++ b/crypto/block.c
> @@ -102,6 +102,7 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
>      }
>  
>      block->driver = qcrypto_block_drivers[options->format];
> +    block->detached_header = options->u.luks.detached_header;

You cannot access a 'luks' field here, as this can be used for
non-LUKS formats. In an earlier patch I suggested using a enum
flag to indicate detached header when opening a device. that
will work for creating it too.

>  
>      if (block->driver->create(block, options, optprefix, initfunc,
>                                writefunc, opaque, errp) < 0) {
> -- 
> 2.39.1
> 

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 RESEND v3 05/10] crypto: Mark the payload_offset_sector invalid for detached LUKS header

[Daniel P. Berrangé]

On Mon, Dec 25, 2023 at 01:45:07PM +0800, Hyman Huang wrote:
> Set the payload_offset_sector to a value that is nearly never reached
> in order to mark it as invalid and indicate that 0 should be the offset
> of the read/write operation on the 'file' protocol blockdev node.
> 
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
>  crypto/block-luks.c | 41 +++++++++++++++++++++++++++++++----------
>  1 file changed, 31 insertions(+), 10 deletions(-)
> 
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index fb01ec38bb..48443ffcae 100644
> --- a/crypto/block-luks.c
> +++ b/crypto/block-luks.c
> @@ -34,6 +34,8 @@
>  
>  #include "qemu/bitmap.h"
>  
> +#define INVALID_SECTOR_OFFSET UINT32_MAX
> +
>  /*
>   * Reference for the LUKS format implemented here is
>   *
> @@ -136,6 +138,13 @@ struct QCryptoBlockLUKS {
>  };
>  
>  
> +static inline uint32_t
> +qcrypto_block_luks_payload_offset(uint32_t sector)
> +{
> +    return sector == INVALID_SECTOR_OFFSET ? 0 :
> +        sector * QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
> +}
> +
>  static int qcrypto_block_luks_cipher_name_lookup(const char *name,
>                                                   QCryptoCipherMode mode,
>                                                   uint32_t key_bytes,
> @@ -1255,8 +1264,8 @@ qcrypto_block_luks_open(QCryptoBlock *block,
>      }
>  
>      block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
> -    block->payload_offset = luks->header.payload_offset_sector *
> -        block->sector_size;
> +    block->payload_offset =
> +        qcrypto_block_luks_payload_offset(luks->header.payload_offset_sector);
>  
>      return 0;
>  
> @@ -1529,16 +1538,28 @@ qcrypto_block_luks_create(QCryptoBlock *block,
>          slot->stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
>      }
>  
> -    /* The total size of the LUKS headers is the partition header + key
> -     * slot headers, rounded up to the nearest sector, combined with
> -     * the size of each master key material region, also rounded up
> -     * to the nearest sector */
> -    luks->header.payload_offset_sector = header_sectors +
> -            QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS * split_key_sectors;
> +    if (block->detached_header) {
> +        /*
> +         * Set the payload_offset_sector to a value that is nearly never
> +         * reached in order to mark it as invalid and indicate that 0 should
> +         * be the offset of the read/write operation on the 'file' protocol
> +         * blockdev node. Here the UINT32_MAX is choosed
> +         */
> +        luks->header.payload_offset_sector = INVALID_SECTOR_OFFSET;

We should be setting payload offset to 0 when using a detached header
to match behaviour of 'cryptsetup' tools.

> +    } else {
> +        /*
> +         * The total size of the LUKS headers is the partition header + key
> +         * slot headers, rounded up to the nearest sector, combined with
> +         * the size of each master key material region, also rounded up
> +         * to the nearest sector
> +         */
> +        luks->header.payload_offset_sector = header_sectors +
> +                QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS * split_key_sectors;
> +    }
>  
>      block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
> -    block->payload_offset = luks->header.payload_offset_sector *
> -        block->sector_size;
> +    block->payload_offset =
> +        qcrypto_block_luks_payload_offset(luks->header.payload_offset_sector);
>  
>      /* Reserve header space to match payload offset */
>      initfunc(block, block->payload_offset, opaque, &local_err);
> -- 
> 2.39.1
> 

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 RESEND v3 04/10] crypto: Introduce creation option and structure for detached LUKS header

[Daniel P. Berrangé]

On Mon, Dec 25, 2023 at 01:45:06PM +0800, Hyman Huang wrote:
> Introduce 'header' field in BlockdevCreateOptionsLUKS to support
> detached LUKS header creation. Meanwhile, introduce header-related
> field in QCryptoBlock.
> 
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
>  crypto/blockpriv.h   | 3 +++
>  qapi/block-core.json | 3 +++
>  qapi/crypto.json     | 5 ++++-
>  3 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/crypto/blockpriv.h b/crypto/blockpriv.h
> index 3c7ccea504..6289aea961 100644
> --- a/crypto/blockpriv.h
> +++ b/crypto/blockpriv.h
> @@ -42,6 +42,9 @@ struct QCryptoBlock {
>      size_t niv;
>      uint64_t payload_offset; /* In bytes */
>      uint64_t sector_size; /* In bytes */
> +
> +    bool detached_header; /* True if disk has a detached LUKS header */
> +    uint64_t detached_header_size; /* LUKS header size plus key slot size */

This field can be replaced by a local variable I believe.

>  };
>  
>  struct QCryptoBlockDriver {
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 9ac256c489..8aec179926 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -4948,6 +4948,8 @@
>  # @file: Node to create the image format on, mandatory except when
>  #        'preallocation' is not requested
>  #
> +# @header: Detached LUKS header node to format. (since 9.0)
> +#
>  # @size: Size of the virtual disk in bytes
>  #
>  # @preallocation: Preallocation mode for the new image (since: 4.2)
> @@ -4958,6 +4960,7 @@
>  { 'struct': 'BlockdevCreateOptionsLUKS',
>    'base': 'QCryptoBlockCreateOptionsLUKS',
>    'data': { '*file':            'BlockdevRef',
> +            '*header':          'BlockdevRef',
>              'size':             'size',
>              '*preallocation':   'PreallocMode' } }
>  
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index fd3d46ebd1..6b4e86cb81 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -195,10 +195,13 @@
>  #     decryption key.  Mandatory except when probing image for
>  #     metadata only.
>  #
> +# @detached-header: if true, disk has detached LUKS header.
> +#
>  # Since: 2.6
>  ##
>  { 'struct': 'QCryptoBlockOptionsLUKS',
> -  'data': { '*key-secret': 'str' }}
> +  'data': { '*key-secret': 'str',
> +            '*detached-header': 'bool' }}

I don't think we need this change if we pass this info as an enum flag

>  
>  ##
>  # @QCryptoBlockCreateOptionsLUKS:
> -- 
> 2.39.1
> 

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 RESEND v3 08/10] crypto: Introduce 'detached-header' field in QCryptoBlockInfoLUKS

[Daniel P. Berrangé]

On Mon, Dec 25, 2023 at 01:45:10PM +0800, Hyman Huang wrote:
> When querying the LUKS disk with the qemu-img tool or other APIs,
> add information about whether the LUKS header is detached.
> 
> Additionally, update the test case with the appropriate
> modification.
> 
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
>  crypto/block-luks.c        | 2 ++
>  qapi/crypto.json           | 3 +++
>  tests/qemu-iotests/210.out | 4 ++++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index 474c7aee2e..c5e53b4ee4 100644
> --- a/crypto/block-luks.c
> +++ b/crypto/block-luks.c
> @@ -1266,6 +1266,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
>      block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
>      block->payload_offset =
>          qcrypto_block_luks_payload_offset(luks->header.payload_offset_sector);
> +    block->detached_header = (block->payload_offset == 0) ? true : false;

We shouldn't be looking at payload_offset for this - when we pass
in an enum flag to indicate detached header, we'll know directly.

>  
>      return 0;
>  
> @@ -1892,6 +1893,7 @@ static int qcrypto_block_luks_get_info(QCryptoBlock *block,
>      info->u.luks.master_key_iters = luks->header.master_key_iterations;
>      info->u.luks.uuid = g_strndup((const char *)luks->header.uuid,
>                                    sizeof(luks->header.uuid));
> +    info->u.luks.detached_header = block->detached_header;
>  
>      for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
>          slot = g_new0(QCryptoBlockInfoLUKSSlot, 1);
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index 8e81aa8454..336c880b5d 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -317,6 +317,8 @@
>  #
>  # @hash-alg: the master key hash algorithm
>  #
> +# @detached-header: whether the LUKS header is detached (Since 9.0)
> +#
>  # @payload-offset: offset to the payload data in bytes
>  #
>  # @master-key-iters: number of PBKDF2 iterations for key material
x> @@ -333,6 +335,7 @@
>             'ivgen-alg': 'QCryptoIVGenAlgorithm',
>             '*ivgen-hash-alg': 'QCryptoHashAlgorithm',
>             'hash-alg': 'QCryptoHashAlgorithm',
> +           'detached-header': 'bool',
>             'payload-offset': 'int',
>             'master-key-iters': 'int',
>             'uuid': 'str',
> diff --git a/tests/qemu-iotests/210.out b/tests/qemu-iotests/210.out
> index 96d9f749dd..94b29b2120 100644
> --- a/tests/qemu-iotests/210.out
> +++ b/tests/qemu-iotests/210.out
> @@ -18,6 +18,7 @@ virtual size: 128 MiB (134217728 bytes)
>  encrypted: yes
>  Format specific information:
>      ivgen alg: plain64
> +    detached header: false
>      hash alg: sha256
>      cipher alg: aes-256
>      uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
> @@ -70,6 +71,7 @@ virtual size: 64 MiB (67108864 bytes)
>  encrypted: yes
>  Format specific information:
>      ivgen alg: plain64
> +    detached header: false
>      hash alg: sha1
>      cipher alg: aes-128
>      uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
> @@ -125,6 +127,7 @@ virtual size: 0 B (0 bytes)
>  encrypted: yes
>  Format specific information:
>      ivgen alg: plain64
> +    detached header: false
>      hash alg: sha256
>      cipher alg: aes-256
>      uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
> @@ -195,6 +198,7 @@ virtual size: 0 B (0 bytes)
>  encrypted: yes
>  Format specific information:
>      ivgen alg: plain64
> +    detached header: false
>      hash alg: sha256
>      cipher alg: aes-256
>      uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
> -- 
> 2.39.1
> 

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 RESEND v3 02/10] crypto: Support generic LUKS encryption

[Yong Huang]

[-- Attachment #1: Type: text/plain, Size: 6296 bytes --]

On Thu, Jan 4, 2024 at 10:40 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Mon, Dec 25, 2023 at 01:45:04PM +0800, Hyman Huang wrote:
> > By enhancing the LUKS driver, it is possible to enable
> > the detachable LUKS header and, as a result, achieve
> > general encryption for any disk format that QEMU has
> > supported.
> >
> > Take the qcow2 as an example, the usage of the generic
> > LUKS encryption as follows:
> >
> > 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 LUKS
> >    header 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 link the qcow2 disk with
> >    LUKS header by specifying the field "header"
> > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > > "arguments":{"node-name":"libvirt-2-format", "driver":"luks",
> > > "file":"libvirt-1-format", "header":"libvirt-2-storage",
> > > "key-secret":"libvirt-2-format-secret0"}}'
> >
> > 6. add the virtio-blk device finally
> > $ virsh qemu-monitor-command vm '{"execute":"device_add",
> > > "arguments": {"num-queues":"1", "driver":"virtio-blk-pci",
> > > "drive": "libvirt-2-format", "id":"virtio-disk2"}}'
> >
> > The generic LUKS encryption method of starting a virtual
> > machine (VM) is somewhat similar to hot-plug in that both
> > maintaining the same json command while the starting VM
> > changes the "blockdev-add/device_add" parameters to
> > "blockdev/device".
> >
> > Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> > Message-Id: <
> 910801f303da1601051479d3b7e5c2c6b4e01eb7.1701879996.git.yong.huang@smartx.com
> >
> > ---
> >  block/crypto.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/crypto.c b/block/crypto.c
> > index f82b13d32b..6063879bac 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -64,12 +64,14 @@ static int block_crypto_read_func(QCryptoBlock
> *block,
> >                                    Error **errp)
> >  {
> >      BlockDriverState *bs = opaque;
> > +    BlockCrypto *crypto = bs->opaque;
> >      ssize_t ret;
> >
> >      GLOBAL_STATE_CODE();
> >      GRAPH_RDLOCK_GUARD_MAINLOOP();
> >
> > -    ret = bdrv_pread(bs->file, offset, buflen, buf, 0);
> > +    ret = bdrv_pread(crypto->header ? crypto->header : bs->file,
> > +                     offset, buflen, buf, 0);
> >      if (ret < 0) {
> >          error_setg_errno(errp, -ret, "Could not read encryption
> header");
> >          return ret;
> > @@ -269,6 +271,7 @@ static int
> block_crypto_open_generic(QCryptoBlockFormat format,
> >      QCryptoBlockOpenOptions *open_opts = NULL;
> >      unsigned int cflags = 0;
> >      QDict *cryptoopts = NULL;
> > +    const char *hdr_bdref = qdict_get_try_str(options, "header");
>
> This is an invalid check to make, because it is assuming the user is
> referencing a separate blockdev node name and doesn't work for an
> inline definition. eg
>
>   qemu-img info
> 'json:{"driver":"luks","file":{"filename":"test-payload.img"},"header":{"filename":"test-header.img"}}'
>
>
> >
> >      GLOBAL_STATE_CODE();
> >
> > @@ -277,6 +280,15 @@ static int
> block_crypto_open_generic(QCryptoBlockFormat format,
> >          return ret;
> >      }
> >
> > +    if (hdr_bdref) {
>
> Get rid of this 'if' clause and unconditionally call the next line:
>
> > +        crypto->header = bdrv_open_child(NULL, options, "header", bs,
> > +                                         &child_of_bds,
> BDRV_CHILD_METADATA,
> > +                                         false, errp);
>
> but pass 'true' instead of 'false' here to allow the child to be absent,
> and thus let it return NULL.
>
> > +        if (!crypto->header) {
>
> You'll need to then check  '*errp != NULL' instead
>
> You'll also need "ERRP_GUARD" at the start of the method
>
> > +            return -EINVAL;
> > +        }
> > +    }
> > +
> >      GRAPH_RDLOCK_GUARD_MAINLOOP();
> >
> >      bs->supported_write_flags = BDRV_REQ_FUA &
>
> This patch should be combined with the previous patch that adds the new
> QAPI schema element as splitting them doesn't add value.
>
>
> Testing this patch with the changes I suggest above, however, still does
> not work:
>
>   $ 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
>   $ qemu-img info
> 'json:{"driver":"luks","file":{"filename":"test-payload.img"},"header":{"filename":"test-header.img"}}'
>   qemu-img: Could not open
> 'json:{"driver":"luks","file":{"filename":"test-payload.img"},"header":{"filename":"test-header.img"}}':
> LUKS payload is overlapping with the header


>
> You need to pass some info into qcrypto_block_open to tell it that the
> header is detached. Add a new enum entry to QCryptoBlockOpenFlags
> perhaps.  Then skip the LUKS payload overlap check.
>
> Ok, I'll try this way and do a test using cryptsetup tools in the next
version.
Thanks for testing this series.


> 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

[-- Attachment #2: Type: text/html, Size: 10299 bytes --]

Re: [PATCH RESEND v3 04/10] crypto: Introduce creation option and structure for detached LUKS header

[Yong Huang]

[-- Attachment #1: Type: text/plain, Size: 3065 bytes --]

On Thu, Jan 4, 2024 at 10:51 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Mon, Dec 25, 2023 at 01:45:06PM +0800, Hyman Huang wrote:
> > Introduce 'header' field in BlockdevCreateOptionsLUKS to support
> > detached LUKS header creation. Meanwhile, introduce header-related
> > field in QCryptoBlock.
> >
> > Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> > ---
> >  crypto/blockpriv.h   | 3 +++
> >  qapi/block-core.json | 3 +++
> >  qapi/crypto.json     | 5 ++++-
> >  3 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/crypto/blockpriv.h b/crypto/blockpriv.h
> > index 3c7ccea504..6289aea961 100644
> > --- a/crypto/blockpriv.h
> > +++ b/crypto/blockpriv.h
> > @@ -42,6 +42,9 @@ struct QCryptoBlock {
> >      size_t niv;
> >      uint64_t payload_offset; /* In bytes */
> >      uint64_t sector_size; /* In bytes */
> > +
> > +    bool detached_header; /* True if disk has a detached LUKS header */
> > +    uint64_t detached_header_size; /* LUKS header size plus key slot
> size */
>
> This field can be replaced by a local variable I believe.
>
> >  };
> >
> >  struct QCryptoBlockDriver {
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 9ac256c489..8aec179926 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -4948,6 +4948,8 @@
> >  # @file: Node to create the image format on, mandatory except when
> >  #        'preallocation' is not requested
> >  #
> > +# @header: Detached LUKS header node to format. (since 9.0)
> > +#
> >  # @size: Size of the virtual disk in bytes
> >  #
> >  # @preallocation: Preallocation mode for the new image (since: 4.2)
> > @@ -4958,6 +4960,7 @@
> >  { 'struct': 'BlockdevCreateOptionsLUKS',
> >    'base': 'QCryptoBlockCreateOptionsLUKS',
> >    'data': { '*file':            'BlockdevRef',
> > +            '*header':          'BlockdevRef',
> >              'size':             'size',
> >              '*preallocation':   'PreallocMode' } }
> >
> > diff --git a/qapi/crypto.json b/qapi/crypto.json
> > index fd3d46ebd1..6b4e86cb81 100644
> > --- a/qapi/crypto.json
> > +++ b/qapi/crypto.json
> > @@ -195,10 +195,13 @@
> >  #     decryption key.  Mandatory except when probing image for
> >  #     metadata only.
> >  #
> > +# @detached-header: if true, disk has detached LUKS header.
> > +#
> >  # Since: 2.6
> >  ##
> >  { 'struct': 'QCryptoBlockOptionsLUKS',
> > -  'data': { '*key-secret': 'str' }}
> > +  'data': { '*key-secret': 'str',
> > +            '*detached-header': 'bool' }}
>
> I don't think we need this change if we pass this info as an enum flag
>

Agree.


> >
> >  ##
> >  # @QCryptoBlockCreateOptionsLUKS:
> > --
> > 2.39.1
> >
>
> 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

[-- Attachment #2: Type: text/html, Size: 5291 bytes --]

Re: [PATCH RESEND v3 06/10] block: Support detached LUKS header creation using blockdev-create

[Markus Armbruster]

Fails to compile for me:

../block/crypto.c: In function ‘block_crypto_co_create_luks’:
../block/crypto.c:784:1: error: control reaches end of non-void function [-Werror=return-type]
  784 | }
      | ^

Re: [PATCH RESEND v3 01/10] crypto: Introduce option and structure for detached LUKS header

[Markus Armbruster]

Hyman Huang <yong.huang@smartx.com> writes:

> Add the "header" option for the LUKS format. This field would be
> used to identify the blockdev's position where a detachable LUKS
> header is stored.
>
> In addition, introduce header field in struct BlockCrypto
>
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Message-Id: <5b99f60c7317092a563d7ca3fb4b414197015eb2.1701879996.git.yong.huang@smartx.com>
> ---
>  block/crypto.c       | 1 +
>  qapi/block-core.json | 6 +++++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/block/crypto.c b/block/crypto.c
> index 921933a5e5..f82b13d32b 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -39,6 +39,7 @@ typedef struct BlockCrypto BlockCrypto;
>  struct BlockCrypto {
>      QCryptoBlock *block;
>      bool updating_keys;
> +    BdrvChild *header;  /* Reference to the detached LUKS header */
>  };
>  
>  
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index ca390c5700..10be08d08f 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 header. (since 9.0)

This will come out like

    "header": "BlockdevRef" (optional)
       optional reference to the location of a blockdev storing a detached
       LUKS header. (since 9.0)

in the manual.  Scratch "optional".

Moreover, a BlockdevRef is a "Reference to a block device" (quote from
its doc comment), not a "reference to the location of a blockdev".
Better simplify to something like "block device holding a detached LUKS
header".

But that's just phrasing.  The contents could perhaps use improvement,
too.  Let's start with this question: what's a detachable LUKS header,
and why would anybody want to use it?

> +#
>  # Since: 2.9
>  ##
>  { 'struct': 'BlockdevOptionsLUKS',
>    'base': 'BlockdevOptionsGenericFormat',
> -  'data': { '*key-secret': 'str' } }
> +  'data': { '*key-secret': 'str',
> +            '*header': 'BlockdevRef'} }
>  
>  ##
>  # @BlockdevOptionsGenericCOWFormat:

Re: [PATCH RESEND v3 01/10] crypto: Introduce option and structure for detached LUKS header

[Daniel P. Berrangé]

On Thu, Jan 11, 2024 at 03:35:10PM +0100, Markus Armbruster wrote:
> Hyman Huang <yong.huang@smartx.com> writes:
> 
> > Add the "header" option for the LUKS format. This field would be
> > used to identify the blockdev's position where a detachable LUKS
> > header is stored.
> >
> > In addition, introduce header field in struct BlockCrypto
> >
> > Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > Message-Id: <5b99f60c7317092a563d7ca3fb4b414197015eb2.1701879996.git.yong.huang@smartx.com>
> > ---
> >  block/crypto.c       | 1 +
> >  qapi/block-core.json | 6 +++++-
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/crypto.c b/block/crypto.c
> > index 921933a5e5..f82b13d32b 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -39,6 +39,7 @@ typedef struct BlockCrypto BlockCrypto;
> >  struct BlockCrypto {
> >      QCryptoBlock *block;
> >      bool updating_keys;
> > +    BdrvChild *header;  /* Reference to the detached LUKS header */
> >  };
> >  
> >  
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index ca390c5700..10be08d08f 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 header. (since 9.0)
> 
> This will come out like
> 
>     "header": "BlockdevRef" (optional)
>        optional reference to the location of a blockdev storing a detached
>        LUKS header. (since 9.0)
> 
> in the manual.  Scratch "optional".
> 
> Moreover, a BlockdevRef is a "Reference to a block device" (quote from
> its doc comment), not a "reference to the location of a blockdev".
> Better simplify to something like "block device holding a detached LUKS
> header".
> 
> But that's just phrasing.  The contents could perhaps use improvement,
> too.  Let's start with this question: what's a detachable LUKS header,
> and why would anybody want to use it?

Normally a LUKS volume has a layout:

  disk:  | header | key material | disk payload data |

With a detached LUKS header, you need 2 disks so getting

  disk1:  | header | key material |
  disk2:  | disk payload data |

There are a variety of reasons to do this

 * Secrecy - the disk2 cannot be identified as containing LUKS volume
             since there's no header

 * Control - if access to the disk1 is restricted, then even if someone
             has access to disk2 they can't unlock it. Might be useful
	     if you have disks on NFS but want to restrict which host
	     can launch a VM instance from it, by dynamically providing
	     access to the header to a designated host

 * Flexibility - your application data volume may be a given size and
                 it is inconvenient to resize it to add encryption.
		 You can store the LUKS header separately and use
		 the existing storage volume for payload

 * Recovery - corruption of a bit in the header may make the entire
              payload inaccessible. It might be convenient to take
	      backups of the header. If your primary disk header
	      becomes corrupt, you can unlock the data still by
	      pointing to the backup detached header.

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 RESEND v3 06/10] block: Support detached LUKS header creation using blockdev-create

[Yong Huang]

[-- Attachment #1: Type: text/plain, Size: 548 bytes --]

On Thu, Jan 11, 2024 at 10:05 PM Markus Armbruster <armbru@redhat.com>
wrote:

> Fails to compile for me:
>
> ../block/crypto.c: In function ‘block_crypto_co_create_luks’:
> ../block/crypto.c:784:1: error: control reaches end of non-void function
> [-Werror=return-type]
>   784 | }
>       | ^
>
>
Ok, I'll check it out and fix it in the next version.

By the way, I'm stuck with some other work, so version 4 might not be
finished for
a few weeks, but I'll get to it as soon as I can.

Thanks,
Yong

-- 
Best regards

[-- Attachment #2: Type: text/html, Size: 1744 bytes --]

Re: [PATCH RESEND v3 01/10] crypto: Introduce option and structure for detached LUKS header

[Yong Huang]

[-- Attachment #1: Type: text/plain, Size: 4208 bytes --]

On Thu, Jan 11, 2024 at 10:58 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Thu, Jan 11, 2024 at 03:35:10PM +0100, Markus Armbruster wrote:
> > Hyman Huang <yong.huang@smartx.com> writes:
> >
> > > Add the "header" option for the LUKS format. This field would be
> > > used to identify the blockdev's position where a detachable LUKS
> > > header is stored.
> > >
> > > In addition, introduce header field in struct BlockCrypto
> > >
> > > Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > > Message-Id: <
> 5b99f60c7317092a563d7ca3fb4b414197015eb2.1701879996.git.yong.huang@smartx.com
> >
> > > ---
> > >  block/crypto.c       | 1 +
> > >  qapi/block-core.json | 6 +++++-
> > >  2 files changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/block/crypto.c b/block/crypto.c
> > > index 921933a5e5..f82b13d32b 100644
> > > --- a/block/crypto.c
> > > +++ b/block/crypto.c
> > > @@ -39,6 +39,7 @@ typedef struct BlockCrypto BlockCrypto;
> > >  struct BlockCrypto {
> > >      QCryptoBlock *block;
> > >      bool updating_keys;
> > > +    BdrvChild *header;  /* Reference to the detached LUKS header */
> > >  };
> > >
> > >
> > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > index ca390c5700..10be08d08f 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 header. (since 9.0)
> >
> > This will come out like
> >
> >     "header": "BlockdevRef" (optional)
> >        optional reference to the location of a blockdev storing a
> detached
> >        LUKS header. (since 9.0)
> >
> > in the manual.  Scratch "optional".
> >
> > Moreover, a BlockdevRef is a "Reference to a block device" (quote from
> > its doc comment), not a "reference to the location of a blockdev".
> > Better simplify to something like "block device holding a detached LUKS
> > header".
> >
> > But that's just phrasing.  The contents could perhaps use improvement,
> > too.  Let's start with this question: what's a detachable LUKS header,
> > and why would anybody want to use it?
>
> Normally a LUKS volume has a layout:
>
>   disk:  | header | key material | disk payload data |
>
> With a detached LUKS header, you need 2 disks so getting
>
>   disk1:  | header | key material |
>   disk2:  | disk payload data |
>
> There are a variety of reasons to do this
>
>  * Secrecy - the disk2 cannot be identified as containing LUKS volume
>              since there's no header
>
>  * Control - if access to the disk1 is restricted, then even if someone
>              has access to disk2 they can't unlock it. Might be useful
>              if you have disks on NFS but want to restrict which host
>              can launch a VM instance from it, by dynamically providing
>              access to the header to a designated host
>
>  * Flexibility - your application data volume may be a given size and
>                  it is inconvenient to resize it to add encryption.
>                  You can store the LUKS header separately and use
>                  the existing storage volume for payload
>
>  * Recovery - corruption of a bit in the header may make the entire
>               payload inaccessible. It might be convenient to take
>               backups of the header. If your primary disk header
>               becomes corrupt, you can unlock the data still by
>               pointing to the backup detached header.
>
Thank you, Daniel, for the incisive summary. IMHO, the reason listed
above could be added to the document directly :) .

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

[-- Attachment #2: Type: text/html, Size: 6894 bytes --]