* [PATCH v4 0/7] Support generic Luks encryption
@ 2024-01-30 5:37 yong.huang
2024-01-30 5:37 ` [PATCH v4 1/7] crypto: Support LUKS volume with detached header yong.huang
` (6 more replies)
0 siblings, 7 replies; 28+ messages in thread
From: yong.huang @ 2024-01-30 5:37 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé, Eric Blake, Markus Armbruster,
Hanna Reitz, Kevin Wolf, yong.huang
From: Hyman Huang <yong.huang@smartx.com>
Sorry for the late post of version 4. The modifications are as follows:
v4:
- Rebase on master
- squash [PATCH v3 02/10] to [PATCH v3 01/10]
- refactor the logic of block_crypto_open_generic in [PATCH v3 02/10]
as Daniel suggestted:
a. drop the invalid parameter check for "header" option and use the
ERRP_GUARD to probe the error of bdrv_open_child instead.
b. add a new emum entry to QCryptoBlockOpenFlags to instruct the
process of LUKS volume open skip the payload overlap check instead
of using the macro INVALID_SECTOR_OFFSET.
- drop the detached_header_size field and use local variable instead in
[PATCH v3 04/10]
- drop the detached-header option in [PATCH v3 04/10]
- drop the commit [PATCH v3 05/10]:
crypto: Mark the payload_offset_sector invalid for detached LUKS header
- introduce QCryptoBlockCreateFlags to instruct the creation process to
set the payload_offset_sector to 0 in [PATCH v4 03/7]
- refactor the logic of block_crypto_co_create_luks in [PATCH v3 06/10]:
a. fix the compile failure
b. use the existing 'fail:' label to handle the error logic
- refine the comment in qapi suggested by Markus.
- modify the test case to accommodate the new implementation.
Thanks for commenting on this series, please review.
Best regared,
Yong
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
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 implement
the LUKS volume with a detached header.
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 benefits to doing 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
Take the raw-format image as an example to introduce the usage
of the LUKS volume with a detached header:
1. prepare detached LUKS header images
$ 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
2. block-add a protocol blockdev node of payload image
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments":{"node-name":"libvirt-1-storage", "driver":"file",
> "filename":"test-payload.img"}}'
3. block-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": "test-header.img" }}'
4. object-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"}}'
5. block-add the raw-drived blockdev format node
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments":{"node-name":"libvirt-1-format", "driver":"raw",
> "file":"libvirt-1-storage"}}'
6. block-add the luks-drived blockdev to link the raw disk
with the 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"}}'
7. hot-plug 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"}}'
Starting a VM with a LUKS volume with detached header 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 (7):
crypto: Support LUKS volume with detached header
qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS
crypto: Modify the qcrypto_block_create to support creation flags
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 case for LUKS volume with detached header
MAINTAINERS | 5 +
block.c | 5 +-
block/crypto.c | 144 ++++++++++--
block/crypto.h | 8 +
block/qcow.c | 2 +-
block/qcow2.c | 2 +-
crypto/block-luks.c | 41 +++-
crypto/block.c | 4 +-
crypto/blockpriv.h | 2 +
include/crypto/block.h | 16 ++
qapi/block-core.json | 13 +-
qapi/crypto.json | 8 +-
tests/qemu-iotests/210.out | 4 +
tests/qemu-iotests/tests/luks-detached-header | 218 ++++++++++++++++++
.../tests/luks-detached-header.out | 5 +
tests/unit/test-crypto-block.c | 2 +
16 files changed, 447 insertions(+), 32 deletions(-)
create mode 100755 tests/qemu-iotests/tests/luks-detached-header
create mode 100644 tests/qemu-iotests/tests/luks-detached-header.out
--
2.31.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v4 1/7] crypto: Support LUKS volume with detached header
2024-01-30 5:37 [PATCH v4 0/7] Support generic Luks encryption yong.huang
@ 2024-01-30 5:37 ` yong.huang
2024-01-31 10:55 ` Daniel P. Berrangé
2024-01-30 5:37 ` [PATCH v4 2/7] qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS yong.huang
` (5 subsequent siblings)
6 siblings, 1 reply; 28+ messages in thread
From: yong.huang @ 2024-01-30 5:37 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé, Eric Blake, Markus Armbruster,
Hanna Reitz, Kevin Wolf, yong.huang
From: Hyman Huang <yong.huang@smartx.com>
By enhancing the LUKS driver, it is possible to implement
the LUKS volume with a detached header.
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 benefits to doing 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
Take the raw-format image as an example to introduce the usage
of the LUKS volume with a detached header:
1. prepare detached LUKS header images
$ 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
2. block-add a protocol blockdev node of payload image
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments":{"node-name":"libvirt-1-storage", "driver":"file",
> "filename":"test-payload.img"}}'
3. block-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": "test-header.img" }}'
4. object-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"}}'
5. block-add the raw-drived blockdev format node
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments":{"node-name":"libvirt-1-format", "driver":"raw",
> "file":"libvirt-1-storage"}}'
6. block-add the luks-drived blockdev to link the raw disk
with the 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"}}'
7. hot-plug 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"}}'
Starting a VM with a LUKS volume with detached header 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>
---
block/crypto.c | 21 +++++++++++++++++++--
crypto/block-luks.c | 11 +++++++----
include/crypto/block.h | 5 +++++
qapi/block-core.json | 5 ++++-
4 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/block/crypto.c b/block/crypto.c
index 921933a5e5..68656158e9 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 */
};
@@ -63,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;
@@ -84,12 +87,14 @@ static int block_crypto_write_func(QCryptoBlock *block,
Error **errp)
{
BlockDriverState *bs = opaque;
+ BlockCrypto *crypto = bs->opaque;
ssize_t ret;
GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
- ret = bdrv_pwrite(bs->file, offset, buflen, buf, 0);
+ ret = bdrv_pwrite(crypto->header ? crypto->header : bs->file,
+ offset, buflen, buf, 0);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not write encryption header");
return ret;
@@ -262,6 +267,8 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
int flags,
Error **errp)
{
+ ERRP_GUARD();
+
BlockCrypto *crypto = bs->opaque;
QemuOpts *opts = NULL;
int ret;
@@ -276,6 +283,13 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
return ret;
}
+ crypto->header = bdrv_open_child(NULL, options, "header", bs,
+ &child_of_bds, BDRV_CHILD_METADATA,
+ true, errp);
+ if (*errp != NULL) {
+ return -EINVAL;
+ }
+
GRAPH_RDLOCK_GUARD_MAINLOOP();
bs->supported_write_flags = BDRV_REQ_FUA &
@@ -299,6 +313,9 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
if (flags & BDRV_O_NO_IO) {
cflags |= QCRYPTO_BLOCK_OPEN_NO_IO;
}
+ if (crypto->header != NULL) {
+ cflags |= QCRYPTO_BLOCK_OPEN_DETACHED;
+ }
crypto->block = qcrypto_block_open(open_opts, NULL,
block_crypto_read_func,
bs,
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index fb01ec38bb..10373aaba4 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -457,12 +457,15 @@ qcrypto_block_luks_load_header(QCryptoBlock *block,
* Does basic sanity checks on the LUKS header
*/
static int
-qcrypto_block_luks_check_header(const QCryptoBlockLUKS *luks, Error **errp)
+qcrypto_block_luks_check_header(const QCryptoBlockLUKS *luks,
+ unsigned int flags,
+ Error **errp)
{
size_t i, j;
unsigned int header_sectors = QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
+ bool detached = flags & QCRYPTO_BLOCK_OPEN_DETACHED;
if (memcmp(luks->header.magic, qcrypto_block_luks_magic,
QCRYPTO_BLOCK_LUKS_MAGIC_LEN) != 0) {
@@ -494,7 +497,7 @@ qcrypto_block_luks_check_header(const QCryptoBlockLUKS *luks, Error **errp)
return -1;
}
- if (luks->header.payload_offset_sector <
+ if (!detached && luks->header.payload_offset_sector <
DIV_ROUND_UP(QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET,
QCRYPTO_BLOCK_LUKS_SECTOR_SIZE)) {
error_setg(errp, "LUKS payload is overlapping with the header");
@@ -543,7 +546,7 @@ qcrypto_block_luks_check_header(const QCryptoBlockLUKS *luks, Error **errp)
return -1;
}
- if (start1 + len1 > luks->header.payload_offset_sector) {
+ if (!detached && start1 + len1 > luks->header.payload_offset_sector) {
error_setg(errp,
"Keyslot %zu is overlapping with the encrypted payload",
i);
@@ -1203,7 +1206,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
goto fail;
}
- if (qcrypto_block_luks_check_header(luks, errp) < 0) {
+ if (qcrypto_block_luks_check_header(luks, flags, errp) < 0) {
goto fail;
}
diff --git a/include/crypto/block.h b/include/crypto/block.h
index 4f63a37872..d0d97f5d12 100644
--- a/include/crypto/block.h
+++ b/include/crypto/block.h
@@ -66,6 +66,7 @@ bool qcrypto_block_has_format(QCryptoBlockFormat format,
typedef enum {
QCRYPTO_BLOCK_OPEN_NO_IO = (1 << 0),
+ QCRYPTO_BLOCK_OPEN_DETACHED = (1 << 1),
} QCryptoBlockOpenFlags;
/**
@@ -95,6 +96,10 @@ typedef enum {
* metadata such as the payload offset. There will be
* no cipher or ivgen objects available.
*
+ * If @flags contains QCRYPTO_BLOCK_OPEN_DETACHED then
+ * the open process will be optimized to skip the LUKS
+ * payload overlap check.
+ *
* If any part of initializing the encryption context
* fails an error will be returned. This could be due
* to the volume being in the wrong format, a cipher
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 48c181e55d..ae604c6019 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3365,11 +3365,14 @@
# decryption key (since 2.6). Mandatory except when doing a
# metadata-only probe of the image.
#
+# @header: block device holding 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.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 2/7] qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS
2024-01-30 5:37 [PATCH v4 0/7] Support generic Luks encryption yong.huang
2024-01-30 5:37 ` [PATCH v4 1/7] crypto: Support LUKS volume with detached header yong.huang
@ 2024-01-30 5:37 ` yong.huang
2024-02-19 14:22 ` Markus Armbruster
2024-01-30 5:37 ` [PATCH v4 3/7] crypto: Modify the qcrypto_block_create to support creation flags yong.huang
` (4 subsequent siblings)
6 siblings, 1 reply; 28+ messages in thread
From: yong.huang @ 2024-01-30 5:37 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé, Eric Blake, Markus Armbruster,
Hanna Reitz, Kevin Wolf, yong.huang
From: Hyman Huang <yong.huang@smartx.com>
To support detached LUKS header creation, make the existing 'file'
field in BlockdevCreateOptionsLUKS optional.
Signed-off-by: Hyman Huang <yong.huang@smartx.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.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 68656158e9..e87dc84111 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -663,9 +663,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) {
@@ -677,10 +677,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 ae604c6019..69a88d613d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4957,7 +4957,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
#
@@ -4968,7 +4969,7 @@
##
{ 'struct': 'BlockdevCreateOptionsLUKS',
'base': 'QCryptoBlockCreateOptionsLUKS',
- 'data': { 'file': 'BlockdevRef',
+ 'data': { '*file': 'BlockdevRef',
'size': 'size',
'*preallocation': 'PreallocMode' } }
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 3/7] crypto: Modify the qcrypto_block_create to support creation flags
2024-01-30 5:37 [PATCH v4 0/7] Support generic Luks encryption yong.huang
2024-01-30 5:37 ` [PATCH v4 1/7] crypto: Support LUKS volume with detached header yong.huang
2024-01-30 5:37 ` [PATCH v4 2/7] qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS yong.huang
@ 2024-01-30 5:37 ` yong.huang
2024-01-31 10:59 ` Daniel P. Berrangé
2024-01-30 5:37 ` [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create yong.huang
` (3 subsequent siblings)
6 siblings, 1 reply; 28+ messages in thread
From: yong.huang @ 2024-01-30 5:37 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé, Eric Blake, Markus Armbruster,
Hanna Reitz, Kevin Wolf, yong.huang
From: Hyman Huang <yong.huang@smartx.com>
Expand the signature of qcrypto_block_create to enable the
formation of LUKS volumes with detachable headers. To accomplish
that, introduce QCryptoBlockCreateFlags to instruct the creation
process to set the payload_offset_sector to 0.
Signed-off-by: Hyman Huang <yong.huang@smartx.com>
---
block/crypto.c | 1 +
block/qcow.c | 2 +-
block/qcow2.c | 2 +-
crypto/block-luks.c | 28 +++++++++++++++++++++-------
crypto/block.c | 4 +++-
crypto/blockpriv.h | 2 ++
include/crypto/block.h | 11 +++++++++++
tests/unit/test-crypto-block.c | 2 ++
8 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/block/crypto.c b/block/crypto.c
index e87dc84111..1b3f87922a 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -369,6 +369,7 @@ block_crypto_co_create_generic(BlockDriverState *bs, int64_t size,
block_crypto_create_init_func,
block_crypto_create_write_func,
&data,
+ 0,
errp);
if (!crypto) {
diff --git a/block/qcow.c b/block/qcow.c
index c6d0e15f1e..ca8e1d5ec8 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -885,7 +885,7 @@ qcow_co_create(BlockdevCreateOptions *opts, Error **errp)
header.crypt_method = cpu_to_be32(QCOW_CRYPT_AES);
crypto = qcrypto_block_create(qcow_opts->encrypt, "encrypt.",
- NULL, NULL, NULL, errp);
+ NULL, NULL, NULL, 0, errp);
if (!crypto) {
ret = -EINVAL;
goto exit;
diff --git a/block/qcow2.c b/block/qcow2.c
index 9bee66fff5..204f5854cf 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3216,7 +3216,7 @@ qcow2_set_up_encryption(BlockDriverState *bs,
crypto = qcrypto_block_create(cryptoopts, "encrypt.",
qcow2_crypto_hdr_init_func,
qcow2_crypto_hdr_write_func,
- bs, errp);
+ bs, 0, errp);
if (!crypto) {
return -EINVAL;
}
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 10373aaba4..8ad7cc44a5 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -1304,6 +1304,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
const char *hash_alg;
g_autofree char *cipher_mode_spec = NULL;
uint64_t iters;
+ uint64_t detached_header_size;
memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts));
if (!luks_opts.has_iter_time) {
@@ -1532,19 +1533,32 @@ 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) {
+ /*
+ * For a detached LUKS header image, set the payload_offset_sector
+ * to 0 to specify the starting point for read/write
+ */
+ luks->header.payload_offset_sector = 0;
+ } 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;
+ 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, 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..506ea1d1a3 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -87,6 +87,7 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
QCryptoBlockInitFunc initfunc,
QCryptoBlockWriteFunc writefunc,
void *opaque,
+ unsigned int flags,
Error **errp)
{
QCryptoBlock *block = g_new0(QCryptoBlock, 1);
@@ -102,6 +103,7 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
}
block->driver = qcrypto_block_drivers[options->format];
+ block->detached_header = flags & QCRYPTO_BLOCK_CREATE_DETACHED;
if (block->driver->create(block, options, optprefix, initfunc,
writefunc, opaque, errp) < 0) {
@@ -146,7 +148,7 @@ qcrypto_block_calculate_payload_offset(QCryptoBlockCreateOptions *create_opts,
qcrypto_block_create(create_opts, optprefix,
qcrypto_block_headerlen_hdr_init_func,
qcrypto_block_headerlen_hdr_write_func,
- len, errp);
+ len, 0, errp);
return crypto != NULL;
}
diff --git a/crypto/blockpriv.h b/crypto/blockpriv.h
index 3c7ccea504..836f3b4726 100644
--- a/crypto/blockpriv.h
+++ b/crypto/blockpriv.h
@@ -42,6 +42,8 @@ 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 */
};
struct QCryptoBlockDriver {
diff --git a/include/crypto/block.h b/include/crypto/block.h
index d0d97f5d12..92e823c9f2 100644
--- a/include/crypto/block.h
+++ b/include/crypto/block.h
@@ -116,6 +116,10 @@ QCryptoBlock *qcrypto_block_open(QCryptoBlockOpenOptions *options,
size_t n_threads,
Error **errp);
+typedef enum {
+ QCRYPTO_BLOCK_CREATE_DETACHED = (1 << 0),
+} QCryptoBlockCreateFlags;
+
/**
* qcrypto_block_create:
* @options: the encryption options
@@ -123,6 +127,7 @@ QCryptoBlock *qcrypto_block_open(QCryptoBlockOpenOptions *options,
* @initfunc: callback for initializing volume header
* @writefunc: callback for writing data to the volume header
* @opaque: data to pass to @initfunc and @writefunc
+ * @flags: bitmask of QCryptoBlockCreateFlags values
* @errp: pointer to a NULL-initialized error object
*
* Create a new block encryption object for initializing
@@ -134,6 +139,11 @@ QCryptoBlock *qcrypto_block_open(QCryptoBlockOpenOptions *options,
* generating new master keys, etc as required. Any existing
* data present on the volume will be irrevocably destroyed.
*
+ * If @flags contains QCRYPTO_BLOCK_CREATE_DETACHED then
+ * the open process will set the payload_offset_sector to 0
+ * to specify the starting point for the read/write of a
+ * detached LUKS header image.
+ *
* If any part of initializing the encryption context
* fails an error will be returned. This could be due
* to the volume being in the wrong format, a cipher
@@ -147,6 +157,7 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
QCryptoBlockInitFunc initfunc,
QCryptoBlockWriteFunc writefunc,
void *opaque,
+ unsigned int flags,
Error **errp);
/**
diff --git a/tests/unit/test-crypto-block.c b/tests/unit/test-crypto-block.c
index 347cd5f3d7..6cfc817a92 100644
--- a/tests/unit/test-crypto-block.c
+++ b/tests/unit/test-crypto-block.c
@@ -283,6 +283,7 @@ static void test_block(gconstpointer opaque)
test_block_init_func,
test_block_write_func,
&header,
+ 0,
&error_abort);
g_assert(blk);
@@ -362,6 +363,7 @@ test_luks_bad_header(gconstpointer data)
test_block_init_func,
test_block_write_func,
&buf,
+ 0,
&error_abort);
g_assert(blk);
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create
2024-01-30 5:37 [PATCH v4 0/7] Support generic Luks encryption yong.huang
` (2 preceding siblings ...)
2024-01-30 5:37 ` [PATCH v4 3/7] crypto: Modify the qcrypto_block_create to support creation flags yong.huang
@ 2024-01-30 5:37 ` yong.huang
2024-01-31 11:49 ` Daniel P. Berrangé
2024-02-19 14:24 ` Markus Armbruster
2024-01-30 5:37 ` [PATCH v4 5/7] block: Support detached LUKS header creation using qemu-img yong.huang
` (2 subsequent siblings)
6 siblings, 2 replies; 28+ messages in thread
From: yong.huang @ 2024-01-30 5:37 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé, Eric Blake, Markus Armbruster,
Hanna Reitz, Kevin Wolf, yong.huang
From: Hyman Huang <yong.huang@smartx.com>
Firstly, enable the ability to choose the block device containing
a detachable LUKS header by adding the 'header' parameter to
BlockdevCreateOptionsLUKS.
Secondly, when formatting the LUKS volume with a detachable header,
truncate the payload volume to length without a header size.
Using the qmp blockdev command, create the LUKS volume with a
detachable header as follows:
1. add the secret to lock/unlock the cipher stored in the
detached LUKS header
$ virsh qemu-monitor-command vm '{"execute":"object-add",
> "arguments":{"qom-type": "secret", "id": "sec0", "data": "foo"}}'
2. create a header img with 0 size
$ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> "arguments":{"job-id":"job0", "options":{"driver":"file",
> "filename":"/path/to/detached_luks_header.img", "size":0 }}}'
3. add protocol blockdev node for header
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments": {"driver":"file", "filename":
> "/path/to/detached_luks_header.img", "node-name":
> "detached-luks-header-storage"}}'
4. create a payload img with 0 size
$ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> "arguments":{"job-id":"job1", "options":{"driver":"file",
> "filename":"/path/to/detached_luks_payload_raw.img", "size":0}}}'
5. add protocol blockdev node for payload
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments": {"driver":"file", "filename":
> "/path/to/detached_luks_payload_raw.img", "node-name":
> "luks-payload-raw-storage"}}'
6. do the formatting with 128M size
$ virsh qemu-monitor-command c81_node1 '{"execute":"blockdev-create",
> "arguments":{"job-id":"job2", "options":{"driver":"luks", "header":
> "detached-luks-header-storage", "file":"luks-payload-raw-storage",
> "size":134217728, "preallocation":"full", "key-secret":"sec0" }}}'
Signed-off-by: Hyman Huang <yong.huang@smartx.com>
---
block/crypto.c | 101 +++++++++++++++++++++++++++++++++++++++----
qapi/block-core.json | 3 ++
2 files changed, 96 insertions(+), 8 deletions(-)
diff --git a/block/crypto.c b/block/crypto.c
index 1b3f87922a..8e7ee5e9ac 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -162,6 +162,48 @@ error:
return ret;
}
+static int coroutine_fn GRAPH_UNLOCKED
+block_crypto_co_format_luks_payload(BlockdevCreateOptionsLUKS *luks_opts,
+ Error **errp)
+{
+ BlockDriverState *bs = NULL;
+ BlockBackend *blk = NULL;
+ Error *local_error = NULL;
+ int ret;
+
+ if (luks_opts->size > INT64_MAX) {
+ return -EFBIG;
+ }
+
+ bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
+ if (bs == NULL) {
+ return -EIO;
+ }
+
+ blk = blk_co_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE,
+ BLK_PERM_ALL, errp);
+ if (!blk) {
+ ret = -EPERM;
+ goto fail;
+ }
+
+ ret = blk_truncate(blk, luks_opts->size, true,
+ luks_opts->preallocation, 0, &local_error);
+ if (ret < 0) {
+ if (ret == -EFBIG) {
+ /* Replace the error message with a better one */
+ error_free(local_error);
+ error_setg(errp, "The requested file size is too large");
+ }
+ goto fail;
+ }
+
+ ret = 0;
+
+fail:
+ bdrv_co_unref(bs);
+ return ret;
+}
static QemuOptsList block_crypto_runtime_opts_luks = {
.name = "crypto",
@@ -341,7 +383,9 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
static int coroutine_fn GRAPH_UNLOCKED
block_crypto_co_create_generic(BlockDriverState *bs, int64_t size,
QCryptoBlockCreateOptions *opts,
- PreallocMode prealloc, Error **errp)
+ PreallocMode prealloc,
+ unsigned int flags,
+ Error **errp)
{
int ret;
BlockBackend *blk;
@@ -369,7 +413,7 @@ block_crypto_co_create_generic(BlockDriverState *bs, int64_t size,
block_crypto_create_init_func,
block_crypto_create_write_func,
&data,
- 0,
+ flags,
errp);
if (!crypto) {
@@ -656,16 +700,26 @@ static int coroutine_fn GRAPH_UNLOCKED
block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
{
BlockdevCreateOptionsLUKS *luks_opts;
+ BlockDriverState *hdr_bs = NULL;
BlockDriverState *bs = NULL;
QCryptoBlockCreateOptions create_opts;
PreallocMode preallocation = PREALLOC_MODE_OFF;
+ unsigned int cflags = 0;
int ret;
assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
luks_opts = &create_options->u.luks;
- if (luks_opts->file == NULL) {
- error_setg(errp, "Formatting LUKS disk requires parameter 'file'");
+ if (luks_opts->header == NULL && luks_opts->file == NULL) {
+ error_setg(errp, "Either the parameter 'header' or 'file' must "
+ "be specified");
+ return -EINVAL;
+ }
+
+ if ((luks_opts->preallocation != PREALLOC_MODE_OFF) &&
+ (luks_opts->file == NULL)) {
+ error_setg(errp, "Parameter 'preallocation' requires 'file' to be "
+ "specified for formatting LUKS disk");
return -EINVAL;
}
@@ -678,14 +732,38 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
preallocation = luks_opts->preallocation;
}
- if (luks_opts->file) {
+ if (luks_opts->header) {
+ /* LUKS volume with detached header */
+ hdr_bs = bdrv_co_open_blockdev_ref(luks_opts->header, errp);
+ if (hdr_bs == NULL) {
+ return -EIO;
+ }
+
+ cflags |= QCRYPTO_BLOCK_CREATE_DETACHED;
+
+ /* Format the LUKS header node */
+ ret = block_crypto_co_create_generic(hdr_bs, 0, &create_opts,
+ PREALLOC_MODE_OFF, cflags, errp);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ /* Format the LUKS payload node */
+ if (luks_opts->file) {
+ ret = block_crypto_co_format_luks_payload(luks_opts, errp);
+ if (ret < 0) {
+ goto fail;
+ }
+ }
+ } else if (luks_opts->file) {
+ /* LUKS volume with none-detached header */
bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
if (bs == NULL) {
return -EIO;
}
ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
- preallocation, errp);
+ preallocation, cflags, errp);
if (ret < 0) {
goto fail;
}
@@ -693,7 +771,13 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
ret = 0;
fail:
- bdrv_co_unref(bs);
+ if (hdr_bs != NULL) {
+ bdrv_co_unref(hdr_bs);
+ }
+
+ if (bs != NULL) {
+ bdrv_co_unref(bs);
+ }
return ret;
}
@@ -747,7 +831,8 @@ block_crypto_co_create_opts_luks(BlockDriver *drv, const char *filename,
}
/* Create format layer */
- ret = block_crypto_co_create_generic(bs, size, create_opts, prealloc, errp);
+ ret = block_crypto_co_create_generic(bs, size, create_opts,
+ prealloc, 0, errp);
if (ret < 0) {
goto fail;
}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 69a88d613d..eab15d7dd9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4960,6 +4960,8 @@
# @file: Node to create the image format on, mandatory except when
# 'preallocation' is not requested
#
+# @header: Block device holding a detached LUKS header. (since 9.0)
+#
# @size: Size of the virtual disk in bytes
#
# @preallocation: Preallocation mode for the new image (since: 4.2)
@@ -4970,6 +4972,7 @@
{ 'struct': 'BlockdevCreateOptionsLUKS',
'base': 'QCryptoBlockCreateOptionsLUKS',
'data': { '*file': 'BlockdevRef',
+ '*header': 'BlockdevRef',
'size': 'size',
'*preallocation': 'PreallocMode' } }
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 5/7] block: Support detached LUKS header creation using qemu-img
2024-01-30 5:37 [PATCH v4 0/7] Support generic Luks encryption yong.huang
` (3 preceding siblings ...)
2024-01-30 5:37 ` [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create yong.huang
@ 2024-01-30 5:37 ` yong.huang
2024-01-31 11:50 ` Daniel P. Berrangé
` (2 more replies)
2024-01-30 5:37 ` [PATCH v4 6/7] crypto: Introduce 'detached-header' field in QCryptoBlockInfoLUKS yong.huang
2024-01-30 5:37 ` [PATCH v4 7/7] tests: Add case for LUKS volume with detached header yong.huang
6 siblings, 3 replies; 28+ messages in thread
From: yong.huang @ 2024-01-30 5:37 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé, Eric Blake, Markus Armbruster,
Hanna Reitz, Kevin Wolf, yong.huang
From: Hyman Huang <yong.huang@smartx.com>
Even though a LUKS header might be created with cryptsetup,
qemu-img should be enhanced to accommodate it as well.
Add the 'detached-header' option to specify the creation of
a detached LUKS header. This is how it is used:
$ qemu-img create --object secret,id=sec0,data=abc123 -f luks
> -o cipher-alg=aes-256,cipher-mode=xts -o key-secret=sec0
> -o detached-header=true header.luks
Using qemu-img or cryptsetup tools to query information of
an LUKS header image as follows:
Assume a detached LUKS header image has been created by:
$ dd if=/dev/zero of=test-header.img bs=1M count=32
$ dd if=/dev/zero of=test-payload.img bs=1M count=1000
$ cryptsetup luksFormat --header test-header.img test-payload.img
> --force-password --type luks1
Header image information could be queried using cryptsetup:
$ cryptsetup luksDump test-header.img
or qemu-img:
$ qemu-img info 'json:{"driver":"luks","file":{"filename":
> "test-payload.img"},"header":{"filename":"test-header.img"}}'
When using qemu-img, keep in mind that the entire disk
information specified by the JSON-format string above must be
supplied on the commandline; if not, an overlay check will reveal
a problem with the LUKS volume check logic.
Signed-off-by: Hyman Huang <yong.huang@smartx.com>
---
block.c | 5 ++++-
block/crypto.c | 10 +++++++++-
block/crypto.h | 8 ++++++++
qapi/crypto.json | 5 ++++-
4 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 30afdcbba6..1ed9214f66 100644
--- a/block.c
+++ b/block.c
@@ -7357,7 +7357,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-header", false))) {
error_setg(errp, "Image creation needs a size parameter");
goto out;
}
diff --git a/block/crypto.c b/block/crypto.c
index 8e7ee5e9ac..65426d3a16 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -231,6 +231,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_HEADER(""),
{ /* end of list */ }
},
};
@@ -405,7 +406,7 @@ block_crypto_co_create_generic(BlockDriverState *bs, int64_t size,
data = (struct BlockCryptoCreateData) {
.blk = blk,
- .size = size,
+ .size = flags & QCRYPTO_BLOCK_CREATE_DETACHED ? 0 : size,
.prealloc = prealloc,
};
@@ -791,6 +792,9 @@ block_crypto_co_create_opts_luks(BlockDriver *drv, const char *filename,
PreallocMode prealloc;
char *buf = NULL;
int64_t size;
+ bool detached_hdr =
+ qemu_opt_get_bool(opts, "detached-header", false);
+ unsigned int cflags = 0;
int ret;
Error *local_err = NULL;
@@ -830,6 +834,10 @@ block_crypto_co_create_opts_luks(BlockDriver *drv, const char *filename,
goto fail;
}
+ if (detached_hdr) {
+ cflags |= QCRYPTO_BLOCK_CREATE_DETACHED;
+ }
+
/* Create format layer */
ret = block_crypto_co_create_generic(bs, size, create_opts,
prealloc, 0, errp);
diff --git a/block/crypto.h b/block/crypto.h
index 72e792c9af..dc3d2d5ed9 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_HEADER "detached-header"
#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_HEADER(prefix) \
+ { \
+ .name = prefix BLOCK_CRYPTO_OPT_LUKS_DETACHED_HEADER, \
+ .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 fd3d46ebd1..62fd145223 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -223,6 +223,8 @@
# @iter-time: number of milliseconds to spend in PBKDF passphrase
# processing. Currently defaults to 2000. (since 2.8)
#
+# @detached-header: create a detached LUKS header. (since 9.0)
+#
# Since: 2.6
##
{ 'struct': 'QCryptoBlockCreateOptionsLUKS',
@@ -232,7 +234,8 @@
'*ivgen-alg': 'QCryptoIVGenAlgorithm',
'*ivgen-hash-alg': 'QCryptoHashAlgorithm',
'*hash-alg': 'QCryptoHashAlgorithm',
- '*iter-time': 'int'}}
+ '*iter-time': 'int',
+ '*detached-header': 'bool'}}
##
# @QCryptoBlockOpenOptions:
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 6/7] crypto: Introduce 'detached-header' field in QCryptoBlockInfoLUKS
2024-01-30 5:37 [PATCH v4 0/7] Support generic Luks encryption yong.huang
` (4 preceding siblings ...)
2024-01-30 5:37 ` [PATCH v4 5/7] block: Support detached LUKS header creation using qemu-img yong.huang
@ 2024-01-30 5:37 ` yong.huang
2024-01-31 11:50 ` Daniel P. Berrangé
2024-01-30 5:37 ` [PATCH v4 7/7] tests: Add case for LUKS volume with detached header yong.huang
6 siblings, 1 reply; 28+ messages in thread
From: yong.huang @ 2024-01-30 5:37 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé, Eric Blake, Markus Armbruster,
Hanna Reitz, Kevin Wolf, yong.huang
From: Hyman Huang <yong.huang@smartx.com>
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 8ad7cc44a5..3c168aa86f 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -1260,6 +1260,7 @@ 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->detached_header = (block->payload_offset == 0) ? true : false;
return 0;
@@ -1884,6 +1885,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 62fd145223..f8b00cdc4d 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -314,6 +314,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
@@ -330,6 +332,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.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 7/7] tests: Add case for LUKS volume with detached header
2024-01-30 5:37 [PATCH v4 0/7] Support generic Luks encryption yong.huang
` (5 preceding siblings ...)
2024-01-30 5:37 ` [PATCH v4 6/7] crypto: Introduce 'detached-header' field in QCryptoBlockInfoLUKS yong.huang
@ 2024-01-30 5:37 ` yong.huang
2024-01-31 11:53 ` Daniel P. Berrangé
2024-02-09 12:43 ` Daniel P. Berrangé
6 siblings, 2 replies; 28+ messages in thread
From: yong.huang @ 2024-01-30 5:37 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé, Eric Blake, Markus Armbruster,
Hanna Reitz, Kevin Wolf, yong.huang
From: Hyman Huang <yong.huang@smartx.com>
Also, 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 +
| 218 ++++++++++++++++++
| 5 +
3 files changed, 228 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/MAINTAINERS b/MAINTAINERS
index dfaca8323e..fddd3348c2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3402,6 +3402,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
--git a/tests/qemu-iotests/tests/luks-detached-header b/tests/qemu-iotests/tests/luks-detached-header
new file mode 100755
index 0000000000..f0b5f3921c
--- /dev/null
+++ b/tests/qemu-iotests/tests/luks-detached-header
@@ -0,0 +1,218 @@
+#!/usr/bin/env python3
+# group: rw auto
+#
+# Test LUKS volume with detached 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')
+detached_header_raw_img = \
+ "json:{\"driver\":\"luks\",\"file\":{\"filename\":\"%s\"},\"header\":{\"filename\":\"%s\"}}" % (detached_payload_raw_img, detached_header_img1)
+detached_header_qcow2_img = \
+ "json:{\"driver\":\"luks\",\"file\":{\"filename\":\"%s\"},\"header\":{\"filename\":\"%s\"}}" % (detached_payload_qcow2_img, detached_header_img2)
+
+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-header=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_raw_img)['format-specific']
+ self.assertEqual(data['type'], imgfmt)
+ self.assertEqual(data['data']['detached-header'], True)
+
+ data = qemu_img_info(detached_header_qcow2_img)['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'])
--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.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v4 1/7] crypto: Support LUKS volume with detached header
2024-01-30 5:37 ` [PATCH v4 1/7] crypto: Support LUKS volume with detached header yong.huang
@ 2024-01-31 10:55 ` Daniel P. Berrangé
0 siblings, 0 replies; 28+ messages in thread
From: Daniel P. Berrangé @ 2024-01-31 10:55 UTC (permalink / raw)
To: yong.huang
Cc: qemu-devel, Eric Blake, Markus Armbruster, Hanna Reitz,
Kevin Wolf
On Tue, Jan 30, 2024 at 01:37:19PM +0800, yong.huang@smartx.com wrote:
> From: Hyman Huang <yong.huang@smartx.com>
>
> By enhancing the LUKS driver, it is possible to implement
> the LUKS volume with a detached header.
>
> 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 benefits to doing 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
>
> Take the raw-format image as an example to introduce the usage
> of the LUKS volume with a detached header:
>
> 1. prepare detached LUKS header images
> $ 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
>
> 2. block-add a protocol blockdev node of payload image
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > "arguments":{"node-name":"libvirt-1-storage", "driver":"file",
> > "filename":"test-payload.img"}}'
>
> 3. block-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": "test-header.img" }}'
>
> 4. object-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"}}'
>
> 5. block-add the raw-drived blockdev format node
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > "arguments":{"node-name":"libvirt-1-format", "driver":"raw",
> > "file":"libvirt-1-storage"}}'
>
> 6. block-add the luks-drived blockdev to link the raw disk
> with the 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"}}'
>
> 7. hot-plug 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"}}'
>
> Starting a VM with a LUKS volume with detached header 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>
> ---
> block/crypto.c | 21 +++++++++++++++++++--
> crypto/block-luks.c | 11 +++++++----
> include/crypto/block.h | 5 +++++
> qapi/block-core.json | 5 ++++-
> 4 files changed, 35 insertions(+), 7 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 :|
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 3/7] crypto: Modify the qcrypto_block_create to support creation flags
2024-01-30 5:37 ` [PATCH v4 3/7] crypto: Modify the qcrypto_block_create to support creation flags yong.huang
@ 2024-01-31 10:59 ` Daniel P. Berrangé
0 siblings, 0 replies; 28+ messages in thread
From: Daniel P. Berrangé @ 2024-01-31 10:59 UTC (permalink / raw)
To: yong.huang
Cc: qemu-devel, Eric Blake, Markus Armbruster, Hanna Reitz,
Kevin Wolf
On Tue, Jan 30, 2024 at 01:37:21PM +0800, yong.huang@smartx.com wrote:
> From: Hyman Huang <yong.huang@smartx.com>
>
> Expand the signature of qcrypto_block_create to enable the
> formation of LUKS volumes with detachable headers. To accomplish
> that, introduce QCryptoBlockCreateFlags to instruct the creation
> process to set the payload_offset_sector to 0.
>
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
> block/crypto.c | 1 +
> block/qcow.c | 2 +-
> block/qcow2.c | 2 +-
> crypto/block-luks.c | 28 +++++++++++++++++++++-------
> crypto/block.c | 4 +++-
> crypto/blockpriv.h | 2 ++
> include/crypto/block.h | 11 +++++++++++
> tests/unit/test-crypto-block.c | 2 ++
> 8 files changed, 42 insertions(+), 10 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 :|
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create
2024-01-30 5:37 ` [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create yong.huang
@ 2024-01-31 11:49 ` Daniel P. Berrangé
2024-02-19 14:24 ` Markus Armbruster
1 sibling, 0 replies; 28+ messages in thread
From: Daniel P. Berrangé @ 2024-01-31 11:49 UTC (permalink / raw)
To: yong.huang
Cc: qemu-devel, Eric Blake, Markus Armbruster, Hanna Reitz,
Kevin Wolf
On Tue, Jan 30, 2024 at 01:37:22PM +0800, yong.huang@smartx.com wrote:
> From: Hyman Huang <yong.huang@smartx.com>
>
> Firstly, enable the ability to choose the block device containing
> a detachable LUKS header by adding the 'header' parameter to
> BlockdevCreateOptionsLUKS.
>
> Secondly, when formatting the LUKS volume with a detachable header,
> truncate the payload volume to length without a header size.
>
> Using the qmp blockdev command, create the LUKS volume with a
> detachable header as follows:
>
> 1. add the secret to lock/unlock the cipher stored in the
> detached LUKS header
> $ virsh qemu-monitor-command vm '{"execute":"object-add",
> > "arguments":{"qom-type": "secret", "id": "sec0", "data": "foo"}}'
>
> 2. create a header img with 0 size
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> > "arguments":{"job-id":"job0", "options":{"driver":"file",
> > "filename":"/path/to/detached_luks_header.img", "size":0 }}}'
>
> 3. add protocol blockdev node for header
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > "arguments": {"driver":"file", "filename":
> > "/path/to/detached_luks_header.img", "node-name":
> > "detached-luks-header-storage"}}'
>
> 4. create a payload img with 0 size
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> > "arguments":{"job-id":"job1", "options":{"driver":"file",
> > "filename":"/path/to/detached_luks_payload_raw.img", "size":0}}}'
>
> 5. add protocol blockdev node for payload
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > "arguments": {"driver":"file", "filename":
> > "/path/to/detached_luks_payload_raw.img", "node-name":
> > "luks-payload-raw-storage"}}'
>
> 6. do the formatting with 128M size
> $ virsh qemu-monitor-command c81_node1 '{"execute":"blockdev-create",
> > "arguments":{"job-id":"job2", "options":{"driver":"luks", "header":
> > "detached-luks-header-storage", "file":"luks-payload-raw-storage",
> > "size":134217728, "preallocation":"full", "key-secret":"sec0" }}}'
>
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
> block/crypto.c | 101 +++++++++++++++++++++++++++++++++++++++----
> qapi/block-core.json | 3 ++
> 2 files changed, 96 insertions(+), 8 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 5/7] block: Support detached LUKS header creation using qemu-img
2024-01-30 5:37 ` [PATCH v4 5/7] block: Support detached LUKS header creation using qemu-img yong.huang
@ 2024-01-31 11:50 ` Daniel P. Berrangé
2024-02-09 12:27 ` Daniel P. Berrangé
2024-02-19 14:24 ` Markus Armbruster
2 siblings, 0 replies; 28+ messages in thread
From: Daniel P. Berrangé @ 2024-01-31 11:50 UTC (permalink / raw)
To: yong.huang
Cc: qemu-devel, Eric Blake, Markus Armbruster, Hanna Reitz,
Kevin Wolf
On Tue, Jan 30, 2024 at 01:37:23PM +0800, yong.huang@smartx.com wrote:
> From: Hyman Huang <yong.huang@smartx.com>
>
> Even though a LUKS header might be created with cryptsetup,
> qemu-img should be enhanced to accommodate it as well.
>
> Add the 'detached-header' option to specify the creation of
> a detached LUKS header. This is how it is used:
> $ qemu-img create --object secret,id=sec0,data=abc123 -f luks
> > -o cipher-alg=aes-256,cipher-mode=xts -o key-secret=sec0
> > -o detached-header=true header.luks
>
> Using qemu-img or cryptsetup tools to query information of
> an LUKS header image as follows:
>
> Assume a detached LUKS header image has been created by:
> $ dd if=/dev/zero of=test-header.img bs=1M count=32
> $ dd if=/dev/zero of=test-payload.img bs=1M count=1000
> $ cryptsetup luksFormat --header test-header.img test-payload.img
> > --force-password --type luks1
>
> Header image information could be queried using cryptsetup:
> $ cryptsetup luksDump test-header.img
>
> or qemu-img:
> $ qemu-img info 'json:{"driver":"luks","file":{"filename":
> > "test-payload.img"},"header":{"filename":"test-header.img"}}'
>
> When using qemu-img, keep in mind that the entire disk
> information specified by the JSON-format string above must be
> supplied on the commandline; if not, an overlay check will reveal
> a problem with the LUKS volume check logic.
>
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
> block.c | 5 ++++-
> block/crypto.c | 10 +++++++++-
> block/crypto.h | 8 ++++++++
> qapi/crypto.json | 5 ++++-
> 4 files changed, 25 insertions(+), 3 deletions(-)
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 :|
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 6/7] crypto: Introduce 'detached-header' field in QCryptoBlockInfoLUKS
2024-01-30 5:37 ` [PATCH v4 6/7] crypto: Introduce 'detached-header' field in QCryptoBlockInfoLUKS yong.huang
@ 2024-01-31 11:50 ` Daniel P. Berrangé
0 siblings, 0 replies; 28+ messages in thread
From: Daniel P. Berrangé @ 2024-01-31 11:50 UTC (permalink / raw)
To: yong.huang
Cc: qemu-devel, Eric Blake, Markus Armbruster, Hanna Reitz,
Kevin Wolf
On Tue, Jan 30, 2024 at 01:37:24PM +0800, yong.huang@smartx.com wrote:
> From: Hyman Huang <yong.huang@smartx.com>
>
> 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(+)
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 :|
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 7/7] tests: Add case for LUKS volume with detached header
2024-01-30 5:37 ` [PATCH v4 7/7] tests: Add case for LUKS volume with detached header yong.huang
@ 2024-01-31 11:53 ` Daniel P. Berrangé
2024-02-09 12:43 ` Daniel P. Berrangé
1 sibling, 0 replies; 28+ messages in thread
From: Daniel P. Berrangé @ 2024-01-31 11:53 UTC (permalink / raw)
To: yong.huang
Cc: qemu-devel, Eric Blake, Markus Armbruster, Hanna Reitz,
Kevin Wolf
On Tue, Jan 30, 2024 at 01:37:25PM +0800, yong.huang@smartx.com wrote:
> From: Hyman Huang <yong.huang@smartx.com>
>
> Also, 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 +
> tests/qemu-iotests/tests/luks-detached-header | 218 ++++++++++++++++++
> .../tests/luks-detached-header.out | 5 +
> 3 files changed, 228 insertions(+)
> create mode 100755 tests/qemu-iotests/tests/luks-detached-header
> create mode 100644 tests/qemu-iotests/tests/luks-detached-header.out
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 :|
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 5/7] block: Support detached LUKS header creation using qemu-img
2024-01-30 5:37 ` [PATCH v4 5/7] block: Support detached LUKS header creation using qemu-img yong.huang
2024-01-31 11:50 ` Daniel P. Berrangé
@ 2024-02-09 12:27 ` Daniel P. Berrangé
2024-02-19 14:24 ` Markus Armbruster
2 siblings, 0 replies; 28+ messages in thread
From: Daniel P. Berrangé @ 2024-02-09 12:27 UTC (permalink / raw)
To: yong.huang
Cc: qemu-devel, Eric Blake, Markus Armbruster, Hanna Reitz,
Kevin Wolf
On Tue, Jan 30, 2024 at 01:37:23PM +0800, yong.huang@smartx.com wrote:
> From: Hyman Huang <yong.huang@smartx.com>
>
> Even though a LUKS header might be created with cryptsetup,
> qemu-img should be enhanced to accommodate it as well.
>
> Add the 'detached-header' option to specify the creation of
> a detached LUKS header. This is how it is used:
> $ qemu-img create --object secret,id=sec0,data=abc123 -f luks
> > -o cipher-alg=aes-256,cipher-mode=xts -o key-secret=sec0
> > -o detached-header=true header.luks
>
> Using qemu-img or cryptsetup tools to query information of
> an LUKS header image as follows:
>
> Assume a detached LUKS header image has been created by:
> $ dd if=/dev/zero of=test-header.img bs=1M count=32
> $ dd if=/dev/zero of=test-payload.img bs=1M count=1000
> $ cryptsetup luksFormat --header test-header.img test-payload.img
> > --force-password --type luks1
>
> Header image information could be queried using cryptsetup:
> $ cryptsetup luksDump test-header.img
>
> or qemu-img:
> $ qemu-img info 'json:{"driver":"luks","file":{"filename":
> > "test-payload.img"},"header":{"filename":"test-header.img"}}'
>
> When using qemu-img, keep in mind that the entire disk
> information specified by the JSON-format string above must be
> supplied on the commandline; if not, an overlay check will reveal
> a problem with the LUKS volume check logic.
>
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
> block.c | 5 ++++-
> block/crypto.c | 10 +++++++++-
> block/crypto.h | 8 ++++++++
> qapi/crypto.json | 5 ++++-
> 4 files changed, 25 insertions(+), 3 deletions(-)
> diff --git a/block/crypto.c b/block/crypto.c
> index 8e7ee5e9ac..65426d3a16 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -791,6 +792,9 @@ block_crypto_co_create_opts_luks(BlockDriver *drv, const char *filename,
> PreallocMode prealloc;
> char *buf = NULL;
> int64_t size;
> + bool detached_hdr =
> + qemu_opt_get_bool(opts, "detached-header", false);
> + unsigned int cflags = 0;
> int ret;
> Error *local_err = NULL;
>
> @@ -830,6 +834,10 @@ block_crypto_co_create_opts_luks(BlockDriver *drv, const char *filename,
> goto fail;
> }
>
> + if (detached_hdr) {
> + cflags |= QCRYPTO_BLOCK_CREATE_DETACHED;
> + }
> +
We're setting cflags but not using it ever.
> /* Create format layer */
> ret = block_crypto_co_create_generic(bs, size, create_opts,
> prealloc, 0, errp);
This '0' here should be replaced by 'cflags', since you're
checking for QCRYPTO_BLOCK_CREATE_DETACHED inside the
block_crypto_co_create_generic method.
I'll make this change when I merge this, so no need to resend.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 7/7] tests: Add case for LUKS volume with detached header
2024-01-30 5:37 ` [PATCH v4 7/7] tests: Add case for LUKS volume with detached header yong.huang
2024-01-31 11:53 ` Daniel P. Berrangé
@ 2024-02-09 12:43 ` Daniel P. Berrangé
1 sibling, 0 replies; 28+ messages in thread
From: Daniel P. Berrangé @ 2024-02-09 12:43 UTC (permalink / raw)
To: yong.huang
Cc: qemu-devel, Eric Blake, Markus Armbruster, Hanna Reitz,
Kevin Wolf
On Tue, Jan 30, 2024 at 01:37:25PM +0800, yong.huang@smartx.com wrote:
> From: Hyman Huang <yong.huang@smartx.com>
>
> Also, 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 +
> tests/qemu-iotests/tests/luks-detached-header | 218 ++++++++++++++++++
> .../tests/luks-detached-header.out | 5 +
> 3 files changed, 228 insertions(+)
> create mode 100755 tests/qemu-iotests/tests/luks-detached-header
> create mode 100644 tests/qemu-iotests/tests/luks-detached-header.out
Pylint has some complaints on this which I needed to fix. Most of the
issues were long line length related.
Rather than manmually fix it, I decided to run it through the
'black -l 80' to bulk reformat in a standard style. There were
a few bits black didn't want to fix, so I refactored a couple.
> diff --git a/tests/qemu-iotests/tests/luks-detached-header b/tests/qemu-iotests/tests/luks-detached-header
> new file mode 100755
> index 0000000000..f0b5f3921c
> --- /dev/null
> +++ b/tests/qemu-iotests/tests/luks-detached-header
> @@ -0,0 +1,218 @@
> +#!/usr/bin/env python3
> +# group: rw auto
> +#
> +# Test LUKS volume with detached 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
img_info_log si unused, so I dropped that.
> +
> +
> +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')
> +detached_header_raw_img = \
> + "json:{\"driver\":\"luks\",\"file\":{\"filename\":\"%s\"},\"header\":{\"filename\":\"%s\"}}" % (detached_payload_raw_img, detached_header_img1)
> +detached_header_qcow2_img = \
> + "json:{\"driver\":\"luks\",\"file\":{\"filename\":\"%s\"},\"header\":{\"filename\":\"%s\"}}" % (detached_payload_qcow2_img, detached_header_img2)
Black doesn't like to break strings, so I decided to turn
this into python objects and get rid of the formatting:
detached_header_raw_img = "json:" + json.dumps(
{
"driver": "luks",
"file": {"filename": detached_payload_raw_img},
"header": {
"filename": detached_header_img1,
},
}
)
> + # 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', '')
Here, I just declared the qemu-io command ahead of time
cmd = 'qemu-io luks-2-payload-raw-format "write -P 41 0 64k"'
result = self.vm.qmp(
"human-monitor-command",
command_line=cmd
)
> +
> + 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()
> +
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 :|
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 2/7] qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS
2024-01-30 5:37 ` [PATCH v4 2/7] qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS yong.huang
@ 2024-02-19 14:22 ` Markus Armbruster
2024-02-20 7:31 ` Yong Huang
0 siblings, 1 reply; 28+ messages in thread
From: Markus Armbruster @ 2024-02-19 14:22 UTC (permalink / raw)
To: yong.huang
Cc: qemu-devel, Daniel P . Berrangé, Eric Blake, Hanna Reitz,
Kevin Wolf
yong.huang@smartx.com writes:
> From: Hyman Huang <yong.huang@smartx.com>
>
> To support detached LUKS header creation, make the existing 'file'
> field in BlockdevCreateOptionsLUKS optional.
>
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
[...]
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index ae604c6019..69a88d613d 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -4957,7 +4957,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
You mean when @preallocation is "off"?
Cases:
1. @file is mandatory
2. @file is optional and present
3. @file is optional and absent
Ignorant question: behavior in each case?
> #
> # @size: Size of the virtual disk in bytes
> #
> @@ -4968,7 +4969,7 @@
> ##
> { 'struct': 'BlockdevCreateOptionsLUKS',
> 'base': 'QCryptoBlockCreateOptionsLUKS',
> - 'data': { 'file': 'BlockdevRef',
> + 'data': { '*file': 'BlockdevRef',
> 'size': 'size',
> '*preallocation': 'PreallocMode' } }
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create
2024-01-30 5:37 ` [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create yong.huang
2024-01-31 11:49 ` Daniel P. Berrangé
@ 2024-02-19 14:24 ` Markus Armbruster
2024-02-19 14:49 ` Markus Armbruster
1 sibling, 1 reply; 28+ messages in thread
From: Markus Armbruster @ 2024-02-19 14:24 UTC (permalink / raw)
To: yong.huang
Cc: qemu-devel, Daniel P . Berrangé, Eric Blake,
Markus Armbruster, Hanna Reitz, Kevin Wolf
yong.huang@smartx.com writes:
> From: Hyman Huang <yong.huang@smartx.com>
>
> Firstly, enable the ability to choose the block device containing
> a detachable LUKS header by adding the 'header' parameter to
> BlockdevCreateOptionsLUKS.
>
> Secondly, when formatting the LUKS volume with a detachable header,
> truncate the payload volume to length without a header size.
>
> Using the qmp blockdev command, create the LUKS volume with a
> detachable header as follows:
>
> 1. add the secret to lock/unlock the cipher stored in the
> detached LUKS header
> $ virsh qemu-monitor-command vm '{"execute":"object-add",
>> "arguments":{"qom-type": "secret", "id": "sec0", "data": "foo"}}'
>
> 2. create a header img with 0 size
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
>> "arguments":{"job-id":"job0", "options":{"driver":"file",
>> "filename":"/path/to/detached_luks_header.img", "size":0 }}}'
>
> 3. add protocol blockdev node for header
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
>> "arguments": {"driver":"file", "filename":
>> "/path/to/detached_luks_header.img", "node-name":
>> "detached-luks-header-storage"}}'
>
> 4. create a payload img with 0 size
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
>> "arguments":{"job-id":"job1", "options":{"driver":"file",
>> "filename":"/path/to/detached_luks_payload_raw.img", "size":0}}}'
>
> 5. add protocol blockdev node for payload
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
>> "arguments": {"driver":"file", "filename":
>> "/path/to/detached_luks_payload_raw.img", "node-name":
>> "luks-payload-raw-storage"}}'
>
> 6. do the formatting with 128M size
> $ virsh qemu-monitor-command c81_node1 '{"execute":"blockdev-create",
>> "arguments":{"job-id":"job2", "options":{"driver":"luks", "header":
>> "detached-luks-header-storage", "file":"luks-payload-raw-storage",
>> "size":134217728, "preallocation":"full", "key-secret":"sec0" }}}'
>
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
[...]
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 69a88d613d..eab15d7dd9 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -4960,6 +4960,8 @@
> # @file: Node to create the image format on, mandatory except when
> # 'preallocation' is not requested
> #
> +# @header: Block device holding a detached LUKS header. (since 9.0)
> +#
Behavior when @header is present vs. behavior when it's absent?
> # @size: Size of the virtual disk in bytes
> #
> # @preallocation: Preallocation mode for the new image (since: 4.2)
> @@ -4970,6 +4972,7 @@
> { 'struct': 'BlockdevCreateOptionsLUKS',
> 'base': 'QCryptoBlockCreateOptionsLUKS',
> 'data': { '*file': 'BlockdevRef',
> + '*header': 'BlockdevRef',
> 'size': 'size',
> '*preallocation': 'PreallocMode' } }
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 5/7] block: Support detached LUKS header creation using qemu-img
2024-01-30 5:37 ` [PATCH v4 5/7] block: Support detached LUKS header creation using qemu-img yong.huang
2024-01-31 11:50 ` Daniel P. Berrangé
2024-02-09 12:27 ` Daniel P. Berrangé
@ 2024-02-19 14:24 ` Markus Armbruster
2 siblings, 0 replies; 28+ messages in thread
From: Markus Armbruster @ 2024-02-19 14:24 UTC (permalink / raw)
To: yong.huang
Cc: qemu-devel, Daniel P . Berrangé, Eric Blake,
Markus Armbruster, Hanna Reitz, Kevin Wolf
yong.huang@smartx.com writes:
> From: Hyman Huang <yong.huang@smartx.com>
>
> Even though a LUKS header might be created with cryptsetup,
> qemu-img should be enhanced to accommodate it as well.
>
> Add the 'detached-header' option to specify the creation of
> a detached LUKS header. This is how it is used:
> $ qemu-img create --object secret,id=sec0,data=abc123 -f luks
>> -o cipher-alg=aes-256,cipher-mode=xts -o key-secret=sec0
>> -o detached-header=true header.luks
>
> Using qemu-img or cryptsetup tools to query information of
> an LUKS header image as follows:
>
> Assume a detached LUKS header image has been created by:
> $ dd if=/dev/zero of=test-header.img bs=1M count=32
> $ dd if=/dev/zero of=test-payload.img bs=1M count=1000
> $ cryptsetup luksFormat --header test-header.img test-payload.img
>> --force-password --type luks1
>
> Header image information could be queried using cryptsetup:
> $ cryptsetup luksDump test-header.img
>
> or qemu-img:
> $ qemu-img info 'json:{"driver":"luks","file":{"filename":
>> "test-payload.img"},"header":{"filename":"test-header.img"}}'
>
> When using qemu-img, keep in mind that the entire disk
> information specified by the JSON-format string above must be
> supplied on the commandline; if not, an overlay check will reveal
> a problem with the LUKS volume check logic.
>
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
[...]
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index fd3d46ebd1..62fd145223 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -223,6 +223,8 @@
> # @iter-time: number of milliseconds to spend in PBKDF passphrase
> # processing. Currently defaults to 2000. (since 2.8)
> #
> +# @detached-header: create a detached LUKS header. (since 9.0)
> +#
Behavior when @detached-header is present vs. behavior when it's absent?
> # Since: 2.6
> ##
> { 'struct': 'QCryptoBlockCreateOptionsLUKS',
> @@ -232,7 +234,8 @@
> '*ivgen-alg': 'QCryptoIVGenAlgorithm',
> '*ivgen-hash-alg': 'QCryptoHashAlgorithm',
> '*hash-alg': 'QCryptoHashAlgorithm',
> - '*iter-time': 'int'}}
> + '*iter-time': 'int',
> + '*detached-header': 'bool'}}
>
> ##
> # @QCryptoBlockOpenOptions:
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create
2024-02-19 14:24 ` Markus Armbruster
@ 2024-02-19 14:49 ` Markus Armbruster
2024-02-19 14:57 ` Daniel P. Berrangé
0 siblings, 1 reply; 28+ messages in thread
From: Markus Armbruster @ 2024-02-19 14:49 UTC (permalink / raw)
To: yong.huang
Cc: qemu-devel, Daniel P . Berrangé, Eric Blake, Hanna Reitz,
Kevin Wolf
One more thing...
Markus Armbruster <armbru@redhat.com> writes:
> yong.huang@smartx.com writes:
>
>> From: Hyman Huang <yong.huang@smartx.com>
>>
>> Firstly, enable the ability to choose the block device containing
>> a detachable LUKS header by adding the 'header' parameter to
>> BlockdevCreateOptionsLUKS.
>>
>> Secondly, when formatting the LUKS volume with a detachable header,
>> truncate the payload volume to length without a header size.
>>
>> Using the qmp blockdev command, create the LUKS volume with a
>> detachable header as follows:
>>
>> 1. add the secret to lock/unlock the cipher stored in the
>> detached LUKS header
>> $ virsh qemu-monitor-command vm '{"execute":"object-add",
>>> "arguments":{"qom-type": "secret", "id": "sec0", "data": "foo"}}'
>>
>> 2. create a header img with 0 size
>> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
>>> "arguments":{"job-id":"job0", "options":{"driver":"file",
>>> "filename":"/path/to/detached_luks_header.img", "size":0 }}}'
>>
>> 3. add protocol blockdev node for header
>> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
>>> "arguments": {"driver":"file", "filename":
>>> "/path/to/detached_luks_header.img", "node-name":
>>> "detached-luks-header-storage"}}'
>>
>> 4. create a payload img with 0 size
>> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
>>> "arguments":{"job-id":"job1", "options":{"driver":"file",
>>> "filename":"/path/to/detached_luks_payload_raw.img", "size":0}}}'
>>
>> 5. add protocol blockdev node for payload
>> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
>>> "arguments": {"driver":"file", "filename":
>>> "/path/to/detached_luks_payload_raw.img", "node-name":
>>> "luks-payload-raw-storage"}}'
>>
>> 6. do the formatting with 128M size
>> $ virsh qemu-monitor-command c81_node1 '{"execute":"blockdev-create",
>>> "arguments":{"job-id":"job2", "options":{"driver":"luks", "header":
>>> "detached-luks-header-storage", "file":"luks-payload-raw-storage",
>>> "size":134217728, "preallocation":"full", "key-secret":"sec0" }}}'
>>
>> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
>> ---
>
> [...]
>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 69a88d613d..eab15d7dd9 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -4960,6 +4960,8 @@
>> # @file: Node to create the image format on, mandatory except when
>> # 'preallocation' is not requested
>> #
>> +# @header: Block device holding a detached LUKS header. (since 9.0)
>> +#
>
> Behavior when @header is present vs. behavior when it's absent?
The next patch adds a member to QCryptoBlockCreateOptionsLUKS, with a
similar description, but a different name:
# @detached-header: create a detached LUKS header. (since 9.0)
Should we name the one added here @detached-header, too?
>> # @size: Size of the virtual disk in bytes
>> #
>> # @preallocation: Preallocation mode for the new image (since: 4.2)
>> @@ -4970,6 +4972,7 @@
>> { 'struct': 'BlockdevCreateOptionsLUKS',
>> 'base': 'QCryptoBlockCreateOptionsLUKS',
>> 'data': { '*file': 'BlockdevRef',
>> + '*header': 'BlockdevRef',
>> 'size': 'size',
>> '*preallocation': 'PreallocMode' } }
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create
2024-02-19 14:49 ` Markus Armbruster
@ 2024-02-19 14:57 ` Daniel P. Berrangé
2024-02-19 15:02 ` Daniel P. Berrangé
2024-02-19 15:43 ` Markus Armbruster
0 siblings, 2 replies; 28+ messages in thread
From: Daniel P. Berrangé @ 2024-02-19 14:57 UTC (permalink / raw)
To: Markus Armbruster
Cc: yong.huang, qemu-devel, Eric Blake, Hanna Reitz, Kevin Wolf
On Mon, Feb 19, 2024 at 03:49:30PM +0100, Markus Armbruster wrote:
> One more thing...
>
> Markus Armbruster <armbru@redhat.com> writes:
>
> > yong.huang@smartx.com writes:
> >
> >> From: Hyman Huang <yong.huang@smartx.com>
> >>
> >> Firstly, enable the ability to choose the block device containing
> >> a detachable LUKS header by adding the 'header' parameter to
> >> BlockdevCreateOptionsLUKS.
> >>
> >> Secondly, when formatting the LUKS volume with a detachable header,
> >> truncate the payload volume to length without a header size.
> >>
> >> Using the qmp blockdev command, create the LUKS volume with a
> >> detachable header as follows:
> >>
> >> 1. add the secret to lock/unlock the cipher stored in the
> >> detached LUKS header
> >> $ virsh qemu-monitor-command vm '{"execute":"object-add",
> >>> "arguments":{"qom-type": "secret", "id": "sec0", "data": "foo"}}'
> >>
> >> 2. create a header img with 0 size
> >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> >>> "arguments":{"job-id":"job0", "options":{"driver":"file",
> >>> "filename":"/path/to/detached_luks_header.img", "size":0 }}}'
> >>
> >> 3. add protocol blockdev node for header
> >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> >>> "arguments": {"driver":"file", "filename":
> >>> "/path/to/detached_luks_header.img", "node-name":
> >>> "detached-luks-header-storage"}}'
> >>
> >> 4. create a payload img with 0 size
> >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> >>> "arguments":{"job-id":"job1", "options":{"driver":"file",
> >>> "filename":"/path/to/detached_luks_payload_raw.img", "size":0}}}'
> >>
> >> 5. add protocol blockdev node for payload
> >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> >>> "arguments": {"driver":"file", "filename":
> >>> "/path/to/detached_luks_payload_raw.img", "node-name":
> >>> "luks-payload-raw-storage"}}'
> >>
> >> 6. do the formatting with 128M size
> >> $ virsh qemu-monitor-command c81_node1 '{"execute":"blockdev-create",
> >>> "arguments":{"job-id":"job2", "options":{"driver":"luks", "header":
> >>> "detached-luks-header-storage", "file":"luks-payload-raw-storage",
> >>> "size":134217728, "preallocation":"full", "key-secret":"sec0" }}}'
> >>
> >> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> >> ---
> >
> > [...]
> >
> >> diff --git a/qapi/block-core.json b/qapi/block-core.json
> >> index 69a88d613d..eab15d7dd9 100644
> >> --- a/qapi/block-core.json
> >> +++ b/qapi/block-core.json
> >> @@ -4960,6 +4960,8 @@
> >> # @file: Node to create the image format on, mandatory except when
> >> # 'preallocation' is not requested
> >> #
> >> +# @header: Block device holding a detached LUKS header. (since 9.0)
> >> +#
> >
> > Behavior when @header is present vs. behavior when it's absent?
>
> The next patch adds a member to QCryptoBlockCreateOptionsLUKS, with a
> similar description, but a different name:
>
> # @detached-header: create a detached LUKS header. (since 9.0)
>
> Should we name the one added here @detached-header, too?
Yikes, that's a mistake. When I reviewed this I was somehow under the
illusion that QCryptoBlockCreateOptionsLUKS was internal use only, for
the block driver impl to interact with the crypto LUKS impl.
In fact, as the diff context below shows, QCryptoBlockCreateOptionsLUKS
is a base struct for BlockdevCreateOptionsLUKS. So in effect we have
one struct with two fields expressing similar concept.
TL;DR: the @detached-header field needs to go, as that's supposed to
be internal only. The mgmt app should only care about 'header' in the
BlockdevCreateOptionsLUKS struct.
FYI, this whole series is already merged last week. So this will need
a fixup. I'll look into it now.
>
> >> # @size: Size of the virtual disk in bytes
> >> #
> >> # @preallocation: Preallocation mode for the new image (since: 4.2)
> >> @@ -4970,6 +4972,7 @@
> >> { 'struct': 'BlockdevCreateOptionsLUKS',
> >> 'base': 'QCryptoBlockCreateOptionsLUKS',
> >> 'data': { '*file': 'BlockdevRef',
> >> + '*header': 'BlockdevRef',
> >> 'size': 'size',
> >> '*preallocation': 'PreallocMode' } }
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create
2024-02-19 14:57 ` Daniel P. Berrangé
@ 2024-02-19 15:02 ` Daniel P. Berrangé
2024-02-19 15:43 ` Markus Armbruster
1 sibling, 0 replies; 28+ messages in thread
From: Daniel P. Berrangé @ 2024-02-19 15:02 UTC (permalink / raw)
To: Markus Armbruster, yong.huang, qemu-devel, Eric Blake,
Hanna Reitz, Kevin Wolf
On Mon, Feb 19, 2024 at 02:57:13PM +0000, Daniel P. Berrangé wrote:
> On Mon, Feb 19, 2024 at 03:49:30PM +0100, Markus Armbruster wrote:
> > One more thing...
> >
> > Markus Armbruster <armbru@redhat.com> writes:
> >
> > > yong.huang@smartx.com writes:
> > >
> > >> From: Hyman Huang <yong.huang@smartx.com>
> > >>
> > >> Firstly, enable the ability to choose the block device containing
> > >> a detachable LUKS header by adding the 'header' parameter to
> > >> BlockdevCreateOptionsLUKS.
> > >>
> > >> Secondly, when formatting the LUKS volume with a detachable header,
> > >> truncate the payload volume to length without a header size.
> > >>
> > >> Using the qmp blockdev command, create the LUKS volume with a
> > >> detachable header as follows:
> > >>
> > >> 1. add the secret to lock/unlock the cipher stored in the
> > >> detached LUKS header
> > >> $ virsh qemu-monitor-command vm '{"execute":"object-add",
> > >>> "arguments":{"qom-type": "secret", "id": "sec0", "data": "foo"}}'
> > >>
> > >> 2. create a header img with 0 size
> > >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> > >>> "arguments":{"job-id":"job0", "options":{"driver":"file",
> > >>> "filename":"/path/to/detached_luks_header.img", "size":0 }}}'
> > >>
> > >> 3. add protocol blockdev node for header
> > >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > >>> "arguments": {"driver":"file", "filename":
> > >>> "/path/to/detached_luks_header.img", "node-name":
> > >>> "detached-luks-header-storage"}}'
> > >>
> > >> 4. create a payload img with 0 size
> > >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> > >>> "arguments":{"job-id":"job1", "options":{"driver":"file",
> > >>> "filename":"/path/to/detached_luks_payload_raw.img", "size":0}}}'
> > >>
> > >> 5. add protocol blockdev node for payload
> > >> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > >>> "arguments": {"driver":"file", "filename":
> > >>> "/path/to/detached_luks_payload_raw.img", "node-name":
> > >>> "luks-payload-raw-storage"}}'
> > >>
> > >> 6. do the formatting with 128M size
> > >> $ virsh qemu-monitor-command c81_node1 '{"execute":"blockdev-create",
> > >>> "arguments":{"job-id":"job2", "options":{"driver":"luks", "header":
> > >>> "detached-luks-header-storage", "file":"luks-payload-raw-storage",
> > >>> "size":134217728, "preallocation":"full", "key-secret":"sec0" }}}'
> > >>
> > >> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> > >> ---
> > >
> > > [...]
> > >
> > >> diff --git a/qapi/block-core.json b/qapi/block-core.json
> > >> index 69a88d613d..eab15d7dd9 100644
> > >> --- a/qapi/block-core.json
> > >> +++ b/qapi/block-core.json
> > >> @@ -4960,6 +4960,8 @@
> > >> # @file: Node to create the image format on, mandatory except when
> > >> # 'preallocation' is not requested
> > >> #
> > >> +# @header: Block device holding a detached LUKS header. (since 9.0)
> > >> +#
> > >
> > > Behavior when @header is present vs. behavior when it's absent?
> >
> > The next patch adds a member to QCryptoBlockCreateOptionsLUKS, with a
> > similar description, but a different name:
> >
> > # @detached-header: create a detached LUKS header. (since 9.0)
> >
> > Should we name the one added here @detached-header, too?
>
> Yikes, that's a mistake. When I reviewed this I was somehow under the
> illusion that QCryptoBlockCreateOptionsLUKS was internal use only, for
> the block driver impl to interact with the crypto LUKS impl.
>
> In fact, as the diff context below shows, QCryptoBlockCreateOptionsLUKS
> is a base struct for BlockdevCreateOptionsLUKS. So in effect we have
> one struct with two fields expressing similar concept.
>
> TL;DR: the @detached-header field needs to go, as that's supposed to
> be internal only. The mgmt app should only care about 'header' in the
> BlockdevCreateOptionsLUKS struct.
>
> FYI, this whole series is already merged last week. So this will need
> a fixup. I'll look into it now.
In fact the '@detached-header' added in the next patch is redundant
in the QAPI, as it is never set. So this QAPI field can be deleted.
We do have a 'detached-header' QemuOpts setting which is simply a
bool flag, as opposed to a full blockdev ref, since qemu-img create
doesn't work in terms of the QAPI BlockdevCreate schema.
>
> >
> > >> # @size: Size of the virtual disk in bytes
> > >> #
> > >> # @preallocation: Preallocation mode for the new image (since: 4.2)
> > >> @@ -4970,6 +4972,7 @@
> > >> { 'struct': 'BlockdevCreateOptionsLUKS',
> > >> 'base': 'QCryptoBlockCreateOptionsLUKS',
> > >> 'data': { '*file': 'BlockdevRef',
> > >> + '*header': 'BlockdevRef',
> > >> 'size': 'size',
> > >> '*preallocation': 'PreallocMode' } }
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create
2024-02-19 14:57 ` Daniel P. Berrangé
2024-02-19 15:02 ` Daniel P. Berrangé
@ 2024-02-19 15:43 ` Markus Armbruster
1 sibling, 0 replies; 28+ messages in thread
From: Markus Armbruster @ 2024-02-19 15:43 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: yong.huang, qemu-devel, Eric Blake, Hanna Reitz, Kevin Wolf
Daniel P. Berrangé <berrange@redhat.com> writes:
[...]
> TL;DR: the @detached-header field needs to go, as that's supposed to
> be internal only. The mgmt app should only care about 'header' in the
> BlockdevCreateOptionsLUKS struct.
>
> FYI, this whole series is already merged last week. So this will need
> a fixup. I'll look into it now.
Glad I neglected to check for a merge, because if I had, I wouldn't have
looked at the schema :)
My apologies for looking so late!
[...]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 2/7] qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS
2024-02-19 14:22 ` Markus Armbruster
@ 2024-02-20 7:31 ` Yong Huang
2024-02-20 8:55 ` Markus Armbruster
0 siblings, 1 reply; 28+ messages in thread
From: Yong Huang @ 2024-02-20 7:31 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Daniel P . Berrangé, Eric Blake, Hanna Reitz,
Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 2377 bytes --]
On Tue, Feb 20, 2024 at 2:31 PM Markus Armbruster <armbru@redhat.com> wrote:
> yong.huang@smartx.com writes:
>
> > From: Hyman Huang <yong.huang@smartx.com>
> >
> > To support detached LUKS header creation, make the existing 'file'
> > field in BlockdevCreateOptionsLUKS optional.
> >
> > Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
> [...]
>
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index ae604c6019..69a88d613d 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -4957,7 +4957,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
>
> You mean when @preallocation is "off"?
>
> Cases:
>
> 1. @file is mandatory
>
When @preallocation is specified to PREALLOC_MODE_ON, file
is mandatory because preallocation aims to act on payload data that
@file holds.
> 2. @file is optional and present
>
When @preallocation is not specified or equals to PREALLOC_MODE_OFF,
@file if optional.
If @file present，there are two cases:
1. @header is absent, the creation process degenerate to the origin action.
2. @header is present, the creation process would trunk the payload data
image that @file holds and do the LUKS formatting on the image that
@header refers;
>
> 3. @file is optional and absent
>
When @preallocation is not specified or equals to PREALLOC_MODE_OFF,
@file if optional.
If @file is absent, do the LUKS formatting only.
Note that Either the parameter 'header' or 'file' must be specified.
Here's my interpretation; do let me know if any of the points are off or
need to be refactored.
>
> Ignorant question: behavior in each case?
>
> > #
> > # @size: Size of the virtual disk in bytes
> > #
> > @@ -4968,7 +4969,7 @@
> > ##
> > { 'struct': 'BlockdevCreateOptionsLUKS',
> > 'base': 'QCryptoBlockCreateOptionsLUKS',
> > - 'data': { 'file': 'BlockdevRef',
> > + 'data': { '*file': 'BlockdevRef',
> > 'size': 'size',
> > '*preallocation': 'PreallocMode' } }
>
>
Thanks,
Yong
--
Best regards
[-- Attachment #2: Type: text/html, Size: 6337 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 2/7] qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS
2024-02-20 7:31 ` Yong Huang
@ 2024-02-20 8:55 ` Markus Armbruster
2024-02-20 9:13 ` Yong Huang
0 siblings, 1 reply; 28+ messages in thread
From: Markus Armbruster @ 2024-02-20 8:55 UTC (permalink / raw)
To: Yong Huang
Cc: qemu-devel, Daniel P . Berrangé, Eric Blake, Hanna Reitz,
Kevin Wolf
Yong Huang <yong.huang@smartx.com> writes:
> On Tue, Feb 20, 2024 at 2:31 PM Markus Armbruster <armbru@redhat.com> wrote:
>
>> yong.huang@smartx.com writes:
>>
>> > From: Hyman Huang <yong.huang@smartx.com>
>> >
>> > To support detached LUKS header creation, make the existing 'file'
>> > field in BlockdevCreateOptionsLUKS optional.
>> >
>> > Signed-off-by: Hyman Huang <yong.huang@smartx.com>
>> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>
>> [...]
>>
>> > diff --git a/qapi/block-core.json b/qapi/block-core.json
>> > index ae604c6019..69a88d613d 100644
>> > --- a/qapi/block-core.json
>> > +++ b/qapi/block-core.json
>> > @@ -4957,7 +4957,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
>>
>> You mean when @preallocation is "off"?
>>
>> Cases:
>>
>> 1. @file is mandatory
>>
>
> When @preallocation is specified to PREALLOC_MODE_ON, file
> is mandatory because preallocation aims to act on payload data that
> @file holds.
>
>
>> 2. @file is optional and present
>>
>
> When @preallocation is not specified or equals to PREALLOC_MODE_OFF,
> @file if optional.
> If @file present，there are two cases:
> 1. @header is absent, the creation process degenerate to the origin action.
> 2. @header is present, the creation process would trunk the payload data
> image that @file holds and do the LUKS formatting on the image that
> @header refers;
>
>
>>
>> 3. @file is optional and absent
>>
>
> When @preallocation is not specified or equals to PREALLOC_MODE_OFF,
> @file if optional.
> If @file is absent, do the LUKS formatting only.
> Note that Either the parameter 'header' or 'file' must be specified.
>
> Here's my interpretation; do let me know if any of the points are off or
> need to be refactored.
>
>
>>
>> Ignorant question: behavior in each case?
Thanks! Would it make sense to work the above into the documentation?
[...]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 2/7] qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS
2024-02-20 8:55 ` Markus Armbruster
@ 2024-02-20 9:13 ` Yong Huang
2024-02-20 9:41 ` Markus Armbruster
0 siblings, 1 reply; 28+ messages in thread
From: Yong Huang @ 2024-02-20 9:13 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Daniel P . Berrangé, Eric Blake, Hanna Reitz,
Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 2574 bytes --]
On Tue, Feb 20, 2024 at 4:56 PM Markus Armbruster <armbru@redhat.com> wrote:
> Yong Huang <yong.huang@smartx.com> writes:
>
> > On Tue, Feb 20, 2024 at 2:31 PM Markus Armbruster <armbru@redhat.com>
> wrote:
> >
> >> yong.huang@smartx.com writes:
> >>
> >> > From: Hyman Huang <yong.huang@smartx.com>
> >> >
> >> > To support detached LUKS header creation, make the existing 'file'
> >> > field in BlockdevCreateOptionsLUKS optional.
> >> >
> >> > Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> >> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> >>
> >> [...]
> >>
> >> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> >> > index ae604c6019..69a88d613d 100644
> >> > --- a/qapi/block-core.json
> >> > +++ b/qapi/block-core.json
> >> > @@ -4957,7 +4957,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
> >>
> >> You mean when @preallocation is "off"?
> >>
> >> Cases:
> >>
> >> 1. @file is mandatory
> >>
> >
> > When @preallocation is specified to PREALLOC_MODE_ON, file
> > is mandatory because preallocation aims to act on payload data that
> > @file holds.
> >
> >
> >> 2. @file is optional and present
> >>
> >
> > When @preallocation is not specified or equals to PREALLOC_MODE_OFF,
> > @file if optional.
> > If @file present，there are two cases:
> > 1. @header is absent, the creation process degenerate to the origin
> action.
> > 2. @header is present, the creation process would trunk the payload data
> > image that @file holds and do the LUKS formatting on the image that
> > @header refers;
> >
> >
> >>
> >> 3. @file is optional and absent
> >>
> >
> > When @preallocation is not specified or equals to PREALLOC_MODE_OFF,
> > @file if optional.
> > If @file is absent, do the LUKS formatting only.
> > Note that Either the parameter 'header' or 'file' must be specified.
> >
> > Here's my interpretation; do let me know if any of the points are off or
> > need to be refactored.
> >
> >
> >>
> >> Ignorant question: behavior in each case?
>
> Thanks! Would it make sense to work the above into the documentation?
>
You mean adding the above interpretation to the following patch?
https://patchew.org/QEMU/c2049499aa05758b4cf18dcec942694ed454a980.1708358310.git.yong.huang@smartx.com/
> [...]
>
>
Yong
--
Best regards
[-- Attachment #2: Type: text/html, Size: 4884 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 2/7] qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS
2024-02-20 9:13 ` Yong Huang
@ 2024-02-20 9:41 ` Markus Armbruster
2024-02-20 10:09 ` Yong Huang
0 siblings, 1 reply; 28+ messages in thread
From: Markus Armbruster @ 2024-02-20 9:41 UTC (permalink / raw)
To: Yong Huang
Cc: Markus Armbruster, qemu-devel, Daniel P . Berrangé,
Eric Blake, Hanna Reitz, Kevin Wolf
Yong Huang <yong.huang@smartx.com> writes:
> On Tue, Feb 20, 2024 at 4:56 PM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Yong Huang <yong.huang@smartx.com> writes:
>>
>> > On Tue, Feb 20, 2024 at 2:31 PM Markus Armbruster <armbru@redhat.com>
>> wrote:
>> >
>> >> yong.huang@smartx.com writes:
>> >>
>> >> > From: Hyman Huang <yong.huang@smartx.com>
>> >> >
>> >> > To support detached LUKS header creation, make the existing 'file'
>> >> > field in BlockdevCreateOptionsLUKS optional.
>> >> >
>> >> > Signed-off-by: Hyman Huang <yong.huang@smartx.com>
>> >> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> >>
>> >> [...]
>> >>
>> >> > diff --git a/qapi/block-core.json b/qapi/block-core.json
>> >> > index ae604c6019..69a88d613d 100644
>> >> > --- a/qapi/block-core.json
>> >> > +++ b/qapi/block-core.json
>> >> > @@ -4957,7 +4957,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
>> >>
>> >> You mean when @preallocation is "off"?
>> >>
>> >> Cases:
>> >>
>> >> 1. @file is mandatory
>> >>
>> >
>> > When @preallocation is specified to PREALLOC_MODE_ON, file
>> > is mandatory because preallocation aims to act on payload data that
>> > @file holds.
>> >
>> >
>> >> 2. @file is optional and present
>> >>
>> >
>> > When @preallocation is not specified or equals to PREALLOC_MODE_OFF,
>> > @file if optional.
>> > If @file present，there are two cases:
>> > 1. @header is absent, the creation process degenerate to the origin action.
>> > 2. @header is present, the creation process would trunk the payload data
>> > image that @file holds and do the LUKS formatting on the image that
>> > @header refers;
>> >
>> >
>> >>
>> >> 3. @file is optional and absent
>> >>
>> >
>> > When @preallocation is not specified or equals to PREALLOC_MODE_OFF,
>> > @file if optional.
>> > If @file is absent, do the LUKS formatting only.
>> > Note that Either the parameter 'header' or 'file' must be specified.
>> >
>> > Here's my interpretation; do let me know if any of the points are off or
>> > need to be refactored.
>> >
>> >
>> >>
>> >> Ignorant question: behavior in each case?
>>
>> Thanks! Would it make sense to work the above into the documentation?
>>
>
> You mean adding the above interpretation to the following patch?
>
> https://patchew.org/QEMU/c2049499aa05758b4cf18dcec942694ed454a980.1708358310.git.yong.huang@smartx.com/
To the doc comments.
The doc comments are the source code for the "QEMU QMP Reference
Manual". That manual should fully explain what the QMP commands do.
Information on how to best use the commands, or an introduction to
concepts behind the commands can also be useful, but is often a bad fit
for a *reference* manual. We can put it elsewhere then.
Makes sense?
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 2/7] qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS
2024-02-20 9:41 ` Markus Armbruster
@ 2024-02-20 10:09 ` Yong Huang
0 siblings, 0 replies; 28+ messages in thread
From: Yong Huang @ 2024-02-20 10:09 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Daniel P . Berrangé, Eric Blake, Hanna Reitz,
Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 3496 bytes --]
On Tue, Feb 20, 2024 at 5:47 PM Markus Armbruster <armbru@redhat.com> wrote:
> Yong Huang <yong.huang@smartx.com> writes:
>
> > On Tue, Feb 20, 2024 at 4:56 PM Markus Armbruster <armbru@redhat.com>
> wrote:
> >
> >> Yong Huang <yong.huang@smartx.com> writes:
> >>
> >> > On Tue, Feb 20, 2024 at 2:31 PM Markus Armbruster <armbru@redhat.com>
> >> wrote:
> >> >
> >> >> yong.huang@smartx.com writes:
> >> >>
> >> >> > From: Hyman Huang <yong.huang@smartx.com>
> >> >> >
> >> >> > To support detached LUKS header creation, make the existing 'file'
> >> >> > field in BlockdevCreateOptionsLUKS optional.
> >> >> >
> >> >> > Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> >> >> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> >> >>
> >> >> [...]
> >> >>
> >> >> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> >> >> > index ae604c6019..69a88d613d 100644
> >> >> > --- a/qapi/block-core.json
> >> >> > +++ b/qapi/block-core.json
> >> >> > @@ -4957,7 +4957,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
> >> >>
> >> >> You mean when @preallocation is "off"?
> >> >>
> >> >> Cases:
> >> >>
> >> >> 1. @file is mandatory
> >> >>
> >> >
> >> > When @preallocation is specified to PREALLOC_MODE_ON, file
> >> > is mandatory because preallocation aims to act on payload data that
> >> > @file holds.
> >> >
> >> >
> >> >> 2. @file is optional and present
> >> >>
> >> >
> >> > When @preallocation is not specified or equals to PREALLOC_MODE_OFF,
> >> > @file if optional.
> >> > If @file present，there are two cases:
> >> > 1. @header is absent, the creation process degenerate to the origin
> action.
> >> > 2. @header is present, the creation process would trunk the payload
> data
> >> > image that @file holds and do the LUKS formatting on the image that
> >> > @header refers;
> >> >
> >> >
> >> >>
> >> >> 3. @file is optional and absent
> >> >>
> >> >
> >> > When @preallocation is not specified or equals to PREALLOC_MODE_OFF,
> >> > @file if optional.
> >> > If @file is absent, do the LUKS formatting only.
> >> > Note that Either the parameter 'header' or 'file' must be specified.
> >> >
> >> > Here's my interpretation; do let me know if any of the points are off
> or
> >> > need to be refactored.
> >> >
> >> >
> >> >>
> >> >> Ignorant question: behavior in each case?
> >>
> >> Thanks! Would it make sense to work the above into the documentation?
> >>
> >
> > You mean adding the above interpretation to the following patch?
> >
> >
> https://patchew.org/QEMU/c2049499aa05758b4cf18dcec942694ed454a980.1708358310.git.yong.huang@smartx.com/
>
> To the doc comments.
>
> The doc comments are the source code for the "QEMU QMP Reference
> Manual". That manual should fully explain what the QMP commands do.
> Information on how to best use the commands, or an introduction to
> concepts behind the commands can also be useful, but is often a bad fit
> for a *reference* manual. We can put it elsewhere then.
>
> Makes sense?
>
>
Of course yes, it can be somewhat complex to use the LUKS volume with
a detachable header, but users may find some relief from the
interpretation.
I'll try it.
--
Best regards
[-- Attachment #2: Type: text/html, Size: 6345 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2024-02-20 10:12 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-30 5:37 [PATCH v4 0/7] Support generic Luks encryption yong.huang
2024-01-30 5:37 ` [PATCH v4 1/7] crypto: Support LUKS volume with detached header yong.huang
2024-01-31 10:55 ` Daniel P. Berrangé
2024-01-30 5:37 ` [PATCH v4 2/7] qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS yong.huang
2024-02-19 14:22 ` Markus Armbruster
2024-02-20 7:31 ` Yong Huang
2024-02-20 8:55 ` Markus Armbruster
2024-02-20 9:13 ` Yong Huang
2024-02-20 9:41 ` Markus Armbruster
2024-02-20 10:09 ` Yong Huang
2024-01-30 5:37 ` [PATCH v4 3/7] crypto: Modify the qcrypto_block_create to support creation flags yong.huang
2024-01-31 10:59 ` Daniel P. Berrangé
2024-01-30 5:37 ` [PATCH v4 4/7] block: Support detached LUKS header creation using blockdev-create yong.huang
2024-01-31 11:49 ` Daniel P. Berrangé
2024-02-19 14:24 ` Markus Armbruster
2024-02-19 14:49 ` Markus Armbruster
2024-02-19 14:57 ` Daniel P. Berrangé
2024-02-19 15:02 ` Daniel P. Berrangé
2024-02-19 15:43 ` Markus Armbruster
2024-01-30 5:37 ` [PATCH v4 5/7] block: Support detached LUKS header creation using qemu-img yong.huang
2024-01-31 11:50 ` Daniel P. Berrangé
2024-02-09 12:27 ` Daniel P. Berrangé
2024-02-19 14:24 ` Markus Armbruster
2024-01-30 5:37 ` [PATCH v4 6/7] crypto: Introduce 'detached-header' field in QCryptoBlockInfoLUKS yong.huang
2024-01-31 11:50 ` Daniel P. Berrangé
2024-01-30 5:37 ` [PATCH v4 7/7] tests: Add case for LUKS volume with detached header yong.huang
2024-01-31 11:53 ` Daniel P. Berrangé
2024-02-09 12:43 ` Daniel P. Berrangé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).