| Message ID | c55f6e98595c654d17c16a569422bec5c03ddaa0.1703482349.git.yong.huang@smartx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support generic Luks encryption | expand |
On Mon, Dec 25, 2023 at 01:45:04PM +0800, Hyman Huang wrote: > By enhancing the LUKS driver, it is possible to enable > the detachable LUKS header and, as a result, achieve > general encryption for any disk format that QEMU has > supported. > > Take the qcow2 as an example, the usage of the generic > LUKS encryption as follows: > > 1. add a protocol blockdev node of data disk > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", > > "arguments":{"node-name":"libvirt-1-storage", "driver":"file", > > "filename":"/path/to/test_disk.qcow2"}}' > > 2. add a protocol blockdev node of LUKS header as above. > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", > > "arguments":{"node-name":"libvirt-2-storage", "driver":"file", > > "filename": "/path/to/cipher.gluks" }}' > > 3. add the secret for decrypting the cipher stored in LUKS > header above > $ virsh qemu-monitor-command vm '{"execute":"object-add", > > "arguments":{"qom-type":"secret", "id": > > "libvirt-2-storage-secret0", "data":"abc123"}}' > > 4. add the qcow2-drived blockdev format node > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", > > "arguments":{"node-name":"libvirt-1-format", "driver":"qcow2", > > "file":"libvirt-1-storage"}}' > > 5. add the luks-drived blockdev to link the qcow2 disk with > LUKS header by specifying the field "header" > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", > > "arguments":{"node-name":"libvirt-2-format", "driver":"luks", > > "file":"libvirt-1-format", "header":"libvirt-2-storage", > > "key-secret":"libvirt-2-format-secret0"}}' > > 6. add the virtio-blk device finally > $ virsh qemu-monitor-command vm '{"execute":"device_add", > > "arguments": {"num-queues":"1", "driver":"virtio-blk-pci", > > "drive": "libvirt-2-format", "id":"virtio-disk2"}}' > > The generic LUKS encryption method of starting a virtual > machine (VM) is somewhat similar to hot-plug in that both > maintaining the same json command while the starting VM > changes the "blockdev-add/device_add" parameters to > "blockdev/device". > > Signed-off-by: Hyman Huang <yong.huang@smartx.com> > Message-Id: <910801f303da1601051479d3b7e5c2c6b4e01eb7.1701879996.git.yong.huang@smartx.com> > --- > block/crypto.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/block/crypto.c b/block/crypto.c > index f82b13d32b..6063879bac 100644 > --- a/block/crypto.c > +++ b/block/crypto.c > @@ -64,12 +64,14 @@ static int block_crypto_read_func(QCryptoBlock *block, > Error **errp) > { > BlockDriverState *bs = opaque; > + BlockCrypto *crypto = bs->opaque; > ssize_t ret; > > GLOBAL_STATE_CODE(); > GRAPH_RDLOCK_GUARD_MAINLOOP(); > > - ret = bdrv_pread(bs->file, offset, buflen, buf, 0); > + ret = bdrv_pread(crypto->header ? crypto->header : bs->file, > + offset, buflen, buf, 0); > if (ret < 0) { > error_setg_errno(errp, -ret, "Could not read encryption header"); > return ret; > @@ -269,6 +271,7 @@ static int block_crypto_open_generic(QCryptoBlockFormat format, > QCryptoBlockOpenOptions *open_opts = NULL; > unsigned int cflags = 0; > QDict *cryptoopts = NULL; > + const char *hdr_bdref = qdict_get_try_str(options, "header"); This is an invalid check to make, because it is assuming the user is referencing a separate blockdev node name and doesn't work for an inline definition. eg qemu-img info 'json:{"driver":"luks","file":{"filename":"test-payload.img"},"header":{"filename":"test-header.img"}}' > > GLOBAL_STATE_CODE(); > > @@ -277,6 +280,15 @@ static int block_crypto_open_generic(QCryptoBlockFormat format, > return ret; > } > > + if (hdr_bdref) { Get rid of this 'if' clause and unconditionally call the next line: > + crypto->header = bdrv_open_child(NULL, options, "header", bs, > + &child_of_bds, BDRV_CHILD_METADATA, > + false, errp); but pass 'true' instead of 'false' here to allow the child to be absent, and thus let it return NULL. > + if (!crypto->header) { You'll need to then check '*errp != NULL' instead You'll also need "ERRP_GUARD" at the start of the method > + return -EINVAL; > + } > + } > + > GRAPH_RDLOCK_GUARD_MAINLOOP(); > > bs->supported_write_flags = BDRV_REQ_FUA & This patch should be combined with the previous patch that adds the new QAPI schema element as splitting them doesn't add value. Testing this patch with the changes I suggest above, however, still does not work: $ dd if=/dev/zero of=test-header.img bs=1M count=32 $ dd if=/dev/zero of=test-payload.img bs=1M count=1000 $ cryptsetup luksFormat --header test-header.img test-payload.img --force-password --type luks1 $ qemu-img info 'json:{"driver":"luks","file":{"filename":"test-payload.img"},"header":{"filename":"test-header.img"}}' qemu-img: Could not open 'json:{"driver":"luks","file":{"filename":"test-payload.img"},"header":{"filename":"test-header.img"}}': LUKS payload is overlapping with the header You need to pass some info into qcrypto_block_open to tell it that the header is detached. Add a new enum entry to QCryptoBlockOpenFlags perhaps. Then skip the LUKS payload overlap check. With regards, Daniel
On Thu, Jan 4, 2024 at 10:40 PM Daniel P. Berrangé <berrange@redhat.com> wrote: > On Mon, Dec 25, 2023 at 01:45:04PM +0800, Hyman Huang wrote: > > By enhancing the LUKS driver, it is possible to enable > > the detachable LUKS header and, as a result, achieve > > general encryption for any disk format that QEMU has > > supported. > > > > Take the qcow2 as an example, the usage of the generic > > LUKS encryption as follows: > > > > 1. add a protocol blockdev node of data disk > > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", > > > "arguments":{"node-name":"libvirt-1-storage", "driver":"file", > > > "filename":"/path/to/test_disk.qcow2"}}' > > > > 2. add a protocol blockdev node of LUKS header as above. > > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", > > > "arguments":{"node-name":"libvirt-2-storage", "driver":"file", > > > "filename": "/path/to/cipher.gluks" }}' > > > > 3. add the secret for decrypting the cipher stored in LUKS > > header above > > $ virsh qemu-monitor-command vm '{"execute":"object-add", > > > "arguments":{"qom-type":"secret", "id": > > > "libvirt-2-storage-secret0", "data":"abc123"}}' > > > > 4. add the qcow2-drived blockdev format node > > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", > > > "arguments":{"node-name":"libvirt-1-format", "driver":"qcow2", > > > "file":"libvirt-1-storage"}}' > > > > 5. add the luks-drived blockdev to link the qcow2 disk with > > LUKS header by specifying the field "header" > > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", > > > "arguments":{"node-name":"libvirt-2-format", "driver":"luks", > > > "file":"libvirt-1-format", "header":"libvirt-2-storage", > > > "key-secret":"libvirt-2-format-secret0"}}' > > > > 6. add the virtio-blk device finally > > $ virsh qemu-monitor-command vm '{"execute":"device_add", > > > "arguments": {"num-queues":"1", "driver":"virtio-blk-pci", > > > "drive": "libvirt-2-format", "id":"virtio-disk2"}}' > > > > The generic LUKS encryption method of starting a virtual > > machine (VM) is somewhat similar to hot-plug in that both > > maintaining the same json command while the starting VM > > changes the "blockdev-add/device_add" parameters to > > "blockdev/device". > > > > Signed-off-by: Hyman Huang <yong.huang@smartx.com> > > Message-Id: < > 910801f303da1601051479d3b7e5c2c6b4e01eb7.1701879996.git.yong.huang@smartx.com > > > > --- > > block/crypto.c | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/block/crypto.c b/block/crypto.c > > index f82b13d32b..6063879bac 100644 > > --- a/block/crypto.c > > +++ b/block/crypto.c > > @@ -64,12 +64,14 @@ static int block_crypto_read_func(QCryptoBlock > *block, > > Error **errp) > > { > > BlockDriverState *bs = opaque; > > + BlockCrypto *crypto = bs->opaque; > > ssize_t ret; > > > > GLOBAL_STATE_CODE(); > > GRAPH_RDLOCK_GUARD_MAINLOOP(); > > > > - ret = bdrv_pread(bs->file, offset, buflen, buf, 0); > > + ret = bdrv_pread(crypto->header ? crypto->header : bs->file, > > + offset, buflen, buf, 0); > > if (ret < 0) { > > error_setg_errno(errp, -ret, "Could not read encryption > header"); > > return ret; > > @@ -269,6 +271,7 @@ static int > block_crypto_open_generic(QCryptoBlockFormat format, > > QCryptoBlockOpenOptions *open_opts = NULL; > > unsigned int cflags = 0; > > QDict *cryptoopts = NULL; > > + const char *hdr_bdref = qdict_get_try_str(options, "header"); > > This is an invalid check to make, because it is assuming the user is > referencing a separate blockdev node name and doesn't work for an > inline definition. eg > > qemu-img info > 'json:{"driver":"luks","file":{"filename":"test-payload.img"},"header":{"filename":"test-header.img"}}' > > > > > > GLOBAL_STATE_CODE(); > > > > @@ -277,6 +280,15 @@ static int > block_crypto_open_generic(QCryptoBlockFormat format, > > return ret; > > } > > > > + if (hdr_bdref) { > > Get rid of this 'if' clause and unconditionally call the next line: > > > + crypto->header = bdrv_open_child(NULL, options, "header", bs, > > + &child_of_bds, > BDRV_CHILD_METADATA, > > + false, errp); > > but pass 'true' instead of 'false' here to allow the child to be absent, > and thus let it return NULL. > > > + if (!crypto->header) { > > You'll need to then check '*errp != NULL' instead > > You'll also need "ERRP_GUARD" at the start of the method > > > + return -EINVAL; > > + } > > + } > > + > > GRAPH_RDLOCK_GUARD_MAINLOOP(); > > > > bs->supported_write_flags = BDRV_REQ_FUA & > > This patch should be combined with the previous patch that adds the new > QAPI schema element as splitting them doesn't add value. > > > Testing this patch with the changes I suggest above, however, still does > not work: > > $ dd if=/dev/zero of=test-header.img bs=1M count=32 > $ dd if=/dev/zero of=test-payload.img bs=1M count=1000 > $ cryptsetup luksFormat --header test-header.img test-payload.img > --force-password --type luks1 > $ qemu-img info > 'json:{"driver":"luks","file":{"filename":"test-payload.img"},"header":{"filename":"test-header.img"}}' > qemu-img: Could not open > 'json:{"driver":"luks","file":{"filename":"test-payload.img"},"header":{"filename":"test-header.img"}}': > LUKS payload is overlapping with the header > > You need to pass some info into qcrypto_block_open to tell it that the > header is detached. Add a new enum entry to QCryptoBlockOpenFlags > perhaps. Then skip the LUKS payload overlap check. > > Ok, I'll try this way and do a test using cryptsetup tools in the next version. Thanks for testing this series. > With regards, > Daniel > -- > |: https://berrange.com -o- > https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- > https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- > https://www.instagram.com/dberrange :| > >
diff --git a/block/crypto.c b/block/crypto.c index f82b13d32b..6063879bac 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -64,12 +64,14 @@ static int block_crypto_read_func(QCryptoBlock *block, Error **errp) { BlockDriverState *bs = opaque; + BlockCrypto *crypto = bs->opaque; ssize_t ret; GLOBAL_STATE_CODE(); GRAPH_RDLOCK_GUARD_MAINLOOP(); - ret = bdrv_pread(bs->file, offset, buflen, buf, 0); + ret = bdrv_pread(crypto->header ? crypto->header : bs->file, + offset, buflen, buf, 0); if (ret < 0) { error_setg_errno(errp, -ret, "Could not read encryption header"); return ret; @@ -269,6 +271,7 @@ static int block_crypto_open_generic(QCryptoBlockFormat format, QCryptoBlockOpenOptions *open_opts = NULL; unsigned int cflags = 0; QDict *cryptoopts = NULL; + const char *hdr_bdref = qdict_get_try_str(options, "header"); GLOBAL_STATE_CODE(); @@ -277,6 +280,15 @@ static int block_crypto_open_generic(QCryptoBlockFormat format, return ret; } + if (hdr_bdref) { + crypto->header = bdrv_open_child(NULL, options, "header", bs, + &child_of_bds, BDRV_CHILD_METADATA, + false, errp); + if (!crypto->header) { + return -EINVAL; + } + } + GRAPH_RDLOCK_GUARD_MAINLOOP(); bs->supported_write_flags = BDRV_REQ_FUA &
By enhancing the LUKS driver, it is possible to enable the detachable LUKS header and, as a result, achieve general encryption for any disk format that QEMU has supported. Take the qcow2 as an example, the usage of the generic LUKS encryption as follows: 1. add a protocol blockdev node of data disk $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", > "arguments":{"node-name":"libvirt-1-storage", "driver":"file", > "filename":"/path/to/test_disk.qcow2"}}' 2. add a protocol blockdev node of LUKS header as above. $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", > "arguments":{"node-name":"libvirt-2-storage", "driver":"file", > "filename": "/path/to/cipher.gluks" }}' 3. add the secret for decrypting the cipher stored in LUKS header above $ virsh qemu-monitor-command vm '{"execute":"object-add", > "arguments":{"qom-type":"secret", "id": > "libvirt-2-storage-secret0", "data":"abc123"}}' 4. add the qcow2-drived blockdev format node $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", > "arguments":{"node-name":"libvirt-1-format", "driver":"qcow2", > "file":"libvirt-1-storage"}}' 5. add the luks-drived blockdev to link the qcow2 disk with LUKS header by specifying the field "header" $ virsh qemu-monitor-command vm '{"execute":"blockdev-add", > "arguments":{"node-name":"libvirt-2-format", "driver":"luks", > "file":"libvirt-1-format", "header":"libvirt-2-storage", > "key-secret":"libvirt-2-format-secret0"}}' 6. add the virtio-blk device finally $ virsh qemu-monitor-command vm '{"execute":"device_add", > "arguments": {"num-queues":"1", "driver":"virtio-blk-pci", > "drive": "libvirt-2-format", "id":"virtio-disk2"}}' The generic LUKS encryption method of starting a virtual machine (VM) is somewhat similar to hot-plug in that both maintaining the same json command while the starting VM changes the "blockdev-add/device_add" parameters to "blockdev/device". Signed-off-by: Hyman Huang <yong.huang@smartx.com> Message-Id: <910801f303da1601051479d3b7e5c2c6b4e01eb7.1701879996.git.yong.huang@smartx.com> --- block/crypto.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)