Re: [v2 3/4] crypto: Support generic LUKS encryption
On Mon, Dec 18, 2023 at 10:15:34PM +0800, Yong Huang wrote:
> On Mon, Dec 18, 2023 at 7:16 PM Daniel P. Berrangé
> wrote:
> > > @@ -903,6 +928,17 @@ block_crypto_child_perms(BlockDriverState *bs,
> > BdrvChild *c,
> > >
> > > BlockCrypto *crypto = bs->opaque;
> > >
> > > +if (role == (role & BDRV_CHILD_METADATA)) {
> > > +/* Assign read permission only */
> > > +perm |= BLK_PERM_CONSISTENT_READ;
> > > +/* Share all permissions */
> > > +shared |= BLK_PERM_ALL;
> > > +
> > > +*nperm = perm;
> > > +*nshared = shared;
> > > +return;
> > > +}
> >
> > What is code doing ? You've set BDRV_CHILD_METADATA role on the
> > crypto->header object, but how does this block_crypto_child_perms
> > method get run against crypto->header ?
> >
> This paragraph originally aims to provide a function that multiple disks
> could share
> the same LUKS header (with read-only permission).
> But I've found that it may not work when updating the detached header after
> reviewing the patch recently :(.
>
> Should it be a separate commit, Since it kind of has nothing to do with the
> basic
> function?
Yes, that would be better as a separate commit.
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [v2 3/4] crypto: Support generic LUKS encryption
On Mon, Dec 18, 2023 at 7:16 PM Daniel P. Berrangé
wrote:
> On Thu, Dec 07, 2023 at 12:37:44AM +0800, Hyman Huang wrote:
> > By enhancing the LUKS driver, it is possible to enable
> > the detachable LUKS header and, as a result, achieve
> > general encryption for any disk format that QEMU has
> > supported.
> >
> > Take the qcow2 as an example, the usage of the generic
> > LUKS encryption as follows:
> >
> > 1. add a protocol blockdev node of data disk
> > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > > "arguments":{"node-name":"libvirt-1-storage", "driver":"file",
> > > "filename":"/path/to/test_disk.qcow2"}}'
> >
> > 2. add a protocol blockdev node of LUKS header as above.
> > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > > "arguments":{"node-name":"libvirt-2-storage", "driver":"file",
> > > "filename": "/path/to/cipher.gluks" }}'
> >
> > 3. add the secret for decrypting the cipher stored in LUKS
> >header above
> > $ virsh qemu-monitor-command vm '{"execute":"object-add",
> > > "arguments":{"qom-type":"secret", "id":
> > > "libvirt-2-storage-secret0", "data":"abc123"}}'
> >
> > 4. add the qcow2-drived blockdev format node
> > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > > "arguments":{"node-name":"libvirt-1-format", "driver":"qcow2",
> > > "file":"libvirt-1-storage"}}'
> >
> > 5. add the luks-drived blockdev to link the qcow2 disk with
> >LUKS header by specifying the field "header"
> > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > > "arguments":{"node-name":"libvirt-2-format", "driver":"luks",
> > > "file":"libvirt-1-format", "header":"libvirt-2-storage",
> > > "key-secret":"libvirt-2-format-secret0"}}'
> >
> > 6. add the virtio-blk device finally
> > $ virsh qemu-monitor-command vm '{"execute":"device_add",
> > > "arguments": {"num-queues":"1", "driver":"virtio-blk-pci",
> > > "drive": "libvirt-2-format", "id":"virtio-disk2"}}'
> >
> > The generic LUKS encryption method of starting a virtual
> > machine (VM) is somewhat similar to hot-plug in that both
> > maintaining the same json command while the starting VM
> > changes the "blockdev-add/device_add" parameters to
> > "blockdev/device".
> >
> > Signed-off-by: Hyman Huang
> > ---
> > block/crypto.c | 38 +-
> > 1 file changed, 37 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/crypto.c b/block/crypto.c
> > index f82b13d32b..7d70349463 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -40,6 +40,7 @@ struct BlockCrypto {
> > QCryptoBlock *block;
> > bool updating_keys;
> > BdrvChild *header; /* Reference to the detached LUKS header */
> > +bool detached_mode; /* If true, LUKS plays a detached header role */
> > };
> >
> >
> > @@ -64,12 +65,16 @@ 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);
> > +if (crypto->detached_mode)
> > +ret = bdrv_pread(crypto->header, offset, buflen, buf, 0);
> > +else
> > +ret = bdrv_pread(bs->file, offset, buflen, buf, 0);
>
> This can be simplified to:
>
> ret = bdrv_pread(bs->header ? bs->header : file, offset, buflen, buf,
> 0);
>
Ok.
>
> > if (ret < 0) {
> > error_setg_errno(errp, -ret, "Could not read encryption
> header");
> > return ret;
> > @@ -269,6 +274,8 @@ static int
> block_crypto_open_generic(QCryptoBlockFormat format,
> > QCryptoBlockOpenOptions *open_opts = NULL;
> > unsigned int cflags = 0;
> > QDict *cryptoopts = NULL;
> > +const char *header_bdref =
> > +qdict_get_try_str(options, "header");
> >
> > GLOBAL_STATE_CODE();
> >
> > @@ -277,6 +284,16 @@ static int
> block_crypto_open_generic(QCryptoBlockFormat format,
> > return ret;
> > }
> >
> > +if (header_bdref) {
> > +crypto->detached_mode = true;
>
> Drop this flag since it has no benefit.
>
> > +crypto->header = bdrv_open_child(NULL, options, "header", bs,
> > + _of_bds,
> BDRV_CHILD_METADATA,
> > + false, errp);
> > +if (!crypto->header) {
> > +return -EINVAL;
> > +}
> > +}
> > +
> > GRAPH_RDLOCK_GUARD_MAINLOOP();
> >
> > bs->supported_write_flags = BDRV_REQ_FUA &
> > @@ -312,6 +329,14 @@ static int
> block_crypto_open_generic(QCryptoBlockFormat format,
> > goto cleanup;
> > }
> >
> > +if (crypto->detached_mode) {
>
> if (crypto->header != NULL)
>
> > +/*
> > + * Set payload offset to zero as the file bdref has no LUKS
> > + * header under detached mode.
> > + */
> > +
Re: [v2 3/4] crypto: Support generic LUKS encryption
On Thu, Dec 07, 2023 at 12:37:44AM +0800, Hyman Huang wrote:
> By enhancing the LUKS driver, it is possible to enable
> the detachable LUKS header and, as a result, achieve
> general encryption for any disk format that QEMU has
> supported.
>
> Take the qcow2 as an example, the usage of the generic
> LUKS encryption as follows:
>
> 1. add a protocol blockdev node of data disk
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > "arguments":{"node-name":"libvirt-1-storage", "driver":"file",
> > "filename":"/path/to/test_disk.qcow2"}}'
>
> 2. add a protocol blockdev node of LUKS header as above.
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > "arguments":{"node-name":"libvirt-2-storage", "driver":"file",
> > "filename": "/path/to/cipher.gluks" }}'
>
> 3. add the secret for decrypting the cipher stored in LUKS
>header above
> $ virsh qemu-monitor-command vm '{"execute":"object-add",
> > "arguments":{"qom-type":"secret", "id":
> > "libvirt-2-storage-secret0", "data":"abc123"}}'
>
> 4. add the qcow2-drived blockdev format node
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > "arguments":{"node-name":"libvirt-1-format", "driver":"qcow2",
> > "file":"libvirt-1-storage"}}'
>
> 5. add the luks-drived blockdev to link the qcow2 disk with
>LUKS header by specifying the field "header"
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > "arguments":{"node-name":"libvirt-2-format", "driver":"luks",
> > "file":"libvirt-1-format", "header":"libvirt-2-storage",
> > "key-secret":"libvirt-2-format-secret0"}}'
>
> 6. add the virtio-blk device finally
> $ virsh qemu-monitor-command vm '{"execute":"device_add",
> > "arguments": {"num-queues":"1", "driver":"virtio-blk-pci",
> > "drive": "libvirt-2-format", "id":"virtio-disk2"}}'
>
> The generic LUKS encryption method of starting a virtual
> machine (VM) is somewhat similar to hot-plug in that both
> maintaining the same json command while the starting VM
> changes the "blockdev-add/device_add" parameters to
> "blockdev/device".
>
> Signed-off-by: Hyman Huang
> ---
> block/crypto.c | 38 +-
> 1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/block/crypto.c b/block/crypto.c
> index f82b13d32b..7d70349463 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -40,6 +40,7 @@ struct BlockCrypto {
> QCryptoBlock *block;
> bool updating_keys;
> BdrvChild *header; /* Reference to the detached LUKS header */
> +bool detached_mode; /* If true, LUKS plays a detached header role */
> };
>
>
> @@ -64,12 +65,16 @@ 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);
> +if (crypto->detached_mode)
> +ret = bdrv_pread(crypto->header, offset, buflen, buf, 0);
> +else
> +ret = bdrv_pread(bs->file, offset, buflen, buf, 0);
This can be simplified to:
ret = bdrv_pread(bs->header ? bs->header : file, offset, buflen, buf, 0);
> if (ret < 0) {
> error_setg_errno(errp, -ret, "Could not read encryption header");
> return ret;
> @@ -269,6 +274,8 @@ static int block_crypto_open_generic(QCryptoBlockFormat
> format,
> QCryptoBlockOpenOptions *open_opts = NULL;
> unsigned int cflags = 0;
> QDict *cryptoopts = NULL;
> +const char *header_bdref =
> +qdict_get_try_str(options, "header");
>
> GLOBAL_STATE_CODE();
>
> @@ -277,6 +284,16 @@ static int block_crypto_open_generic(QCryptoBlockFormat
> format,
> return ret;
> }
>
> +if (header_bdref) {
> +crypto->detached_mode = true;
Drop this flag since it has no benefit.
> +crypto->header = bdrv_open_child(NULL, options, "header", bs,
> + _of_bds, BDRV_CHILD_METADATA,
> + false, errp);
> +if (!crypto->header) {
> +return -EINVAL;
> +}
> +}
> +
> GRAPH_RDLOCK_GUARD_MAINLOOP();
>
> bs->supported_write_flags = BDRV_REQ_FUA &
> @@ -312,6 +329,14 @@ static int block_crypto_open_generic(QCryptoBlockFormat
> format,
> goto cleanup;
> }
>
> +if (crypto->detached_mode) {
if (crypto->header != NULL)
> +/*
> + * Set payload offset to zero as the file bdref has no LUKS
> + * header under detached mode.
> + */
> +qcrypto_block_set_payload_offset(crypto->block, 0);
The LUKS header stores the payload offset. If someone creates the LUKS
volume with a detached header, they may still choose to put the payload
at a non-zero offset.
So AFAICT, we should always honour the payload offset from the header,
even when detached. When
[v2 3/4] crypto: Support generic LUKS encryption
By enhancing the LUKS driver, it is possible to enable
the detachable LUKS header and, as a result, achieve
general encryption for any disk format that QEMU has
supported.
Take the qcow2 as an example, the usage of the generic
LUKS encryption as follows:
1. add a protocol blockdev node of data disk
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments":{"node-name":"libvirt-1-storage", "driver":"file",
> "filename":"/path/to/test_disk.qcow2"}}'
2. add a protocol blockdev node of LUKS header as above.
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments":{"node-name":"libvirt-2-storage", "driver":"file",
> "filename": "/path/to/cipher.gluks" }}'
3. add the secret for decrypting the cipher stored in LUKS
header above
$ virsh qemu-monitor-command vm '{"execute":"object-add",
> "arguments":{"qom-type":"secret", "id":
> "libvirt-2-storage-secret0", "data":"abc123"}}'
4. add the qcow2-drived blockdev format node
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments":{"node-name":"libvirt-1-format", "driver":"qcow2",
> "file":"libvirt-1-storage"}}'
5. add the luks-drived blockdev to link the qcow2 disk with
LUKS header by specifying the field "header"
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments":{"node-name":"libvirt-2-format", "driver":"luks",
> "file":"libvirt-1-format", "header":"libvirt-2-storage",
> "key-secret":"libvirt-2-format-secret0"}}'
6. add the virtio-blk device finally
$ virsh qemu-monitor-command vm '{"execute":"device_add",
> "arguments": {"num-queues":"1", "driver":"virtio-blk-pci",
> "drive": "libvirt-2-format", "id":"virtio-disk2"}}'
The generic LUKS encryption method of starting a virtual
machine (VM) is somewhat similar to hot-plug in that both
maintaining the same json command while the starting VM
changes the "blockdev-add/device_add" parameters to
"blockdev/device".
Signed-off-by: Hyman Huang
---
block/crypto.c | 38 +-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/block/crypto.c b/block/crypto.c
index f82b13d32b..7d70349463 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -40,6 +40,7 @@ struct BlockCrypto {
QCryptoBlock *block;
bool updating_keys;
BdrvChild *header; /* Reference to the detached LUKS header */
+bool detached_mode; /* If true, LUKS plays a detached header role */
};
@@ -64,12 +65,16 @@ 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);
+if (crypto->detached_mode)
+ret = bdrv_pread(crypto->header, offset, buflen, buf, 0);
+else
+ret = bdrv_pread(bs->file, offset, buflen, buf, 0);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not read encryption header");
return ret;
@@ -269,6 +274,8 @@ static int block_crypto_open_generic(QCryptoBlockFormat
format,
QCryptoBlockOpenOptions *open_opts = NULL;
unsigned int cflags = 0;
QDict *cryptoopts = NULL;
+const char *header_bdref =
+qdict_get_try_str(options, "header");
GLOBAL_STATE_CODE();
@@ -277,6 +284,16 @@ static int block_crypto_open_generic(QCryptoBlockFormat
format,
return ret;
}
+if (header_bdref) {
+crypto->detached_mode = true;
+crypto->header = bdrv_open_child(NULL, options, "header", bs,
+ _of_bds, BDRV_CHILD_METADATA,
+ false, errp);
+if (!crypto->header) {
+return -EINVAL;
+}
+}
+
GRAPH_RDLOCK_GUARD_MAINLOOP();
bs->supported_write_flags = BDRV_REQ_FUA &
@@ -312,6 +329,14 @@ static int block_crypto_open_generic(QCryptoBlockFormat
format,
goto cleanup;
}
+if (crypto->detached_mode) {
+/*
+ * Set payload offset to zero as the file bdref has no LUKS
+ * header under detached mode.
+ */
+qcrypto_block_set_payload_offset(crypto->block, 0);
+}
+
bs->encrypted = true;
ret = 0;
@@ -903,6 +928,17 @@ block_crypto_child_perms(BlockDriverState *bs, BdrvChild
*c,
BlockCrypto *crypto = bs->opaque;
+if (role == (role & BDRV_CHILD_METADATA)) {
+/* Assign read permission only */
+perm |= BLK_PERM_CONSISTENT_READ;
+/* Share all permissions */
+shared |= BLK_PERM_ALL;
+
+*nperm = perm;
+*nshared = shared;
+return;
+}
+
bdrv_default_perms(bs, c, role, reopen_queue, perm, shared, nperm,
nshared);
/*
--
2.39.1
