| Message ID | 1453311539-1193-11-git-send-email-berrange@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 01/20 17:38, Daniel P. Berrange wrote: > + /* XXX Should we treat size as being total physical size > + * of the image (ie payload + encryption header), or just > + * the logical size of the image (ie payload). If the latter > + * then we need to extend 'size' to include the header > + * size */ The latter. :) > + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort); > +#define BLOCK_CRYPTO_DRIVER(name, format) \ > + static int block_crypto_probe_ ## name(const uint8_t *buf, \ > + int buf_size, \ > + const char *filename) { \ > + return block_crypto_probe_generic(format, \ > + buf, buf_size, filename); \ > + } \ > + \ > + static int block_crypto_open_ ## name(BlockDriverState *bs, \ > + QDict *options, \ > + int flags, \ > + Error **errp) \ > + { \ > + return block_crypto_open_generic(format, \ > + &block_crypto_runtime_opts_ ## name, \ > + bs, options, flags, errp); \ > + } \ > + \ > + static int block_crypto_create_ ## name(const char *filename, \ > + QemuOpts *opts, \ > + Error **errp) \ > + { \ > + return block_crypto_create_generic(format, \ > + filename, opts, errp); \ > + } \ > + \ > + BlockDriver bdrv_crypto_ ## name = { \ > + .format_name = #name, \ > + .instance_size = sizeof(BlockCrypto), \ > + .bdrv_probe = block_crypto_probe_ ## name, \ > + .bdrv_open = block_crypto_open_ ## name, \ > + .bdrv_close = block_crypto_close, \ > + .bdrv_create = block_crypto_create_ ## name, \ > + .create_opts = &block_crypto_create_opts_ ## name, \ > + \ > + .bdrv_co_readv = block_crypto_co_readv, \ > + .bdrv_co_writev = block_crypto_co_writev, \ > + .bdrv_getlength = block_crypto_getlength, \ > + } > + > +BLOCK_CRYPTO_DRIVER(luks, Q_CRYPTO_BLOCK_FORMAT_LUKS); Personally I really prefer a preprocessed version, for the ease of grep. Fam
On Thu, Jan 21, 2016 at 05:12:08PM +0800, Fam Zheng wrote: > On Wed, 01/20 17:38, Daniel P. Berrange wrote: > > + /* XXX Should we treat size as being total physical size > > + * of the image (ie payload + encryption header), or just > > + * the logical size of the image (ie payload). If the latter > > + * then we need to extend 'size' to include the header > > + * size */ > > The latter. :) Ok > > + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort); > > +#define BLOCK_CRYPTO_DRIVER(name, format) \ > > + static int block_crypto_probe_ ## name(const uint8_t *buf, \ > > + int buf_size, \ > > + const char *filename) { \ > > + return block_crypto_probe_generic(format, \ > > + buf, buf_size, filename); \ > > + } \ > > + \ > > + static int block_crypto_open_ ## name(BlockDriverState *bs, \ > > + QDict *options, \ > > + int flags, \ > > + Error **errp) \ > > + { \ > > + return block_crypto_open_generic(format, \ > > + &block_crypto_runtime_opts_ ## name, \ > > + bs, options, flags, errp); \ > > + } \ > > + \ > > + static int block_crypto_create_ ## name(const char *filename, \ > > + QemuOpts *opts, \ > > + Error **errp) \ > > + { \ > > + return block_crypto_create_generic(format, \ > > + filename, opts, errp); \ > > + } \ > > + \ > > + BlockDriver bdrv_crypto_ ## name = { \ > > + .format_name = #name, \ > > + .instance_size = sizeof(BlockCrypto), \ > > + .bdrv_probe = block_crypto_probe_ ## name, \ > > + .bdrv_open = block_crypto_open_ ## name, \ > > + .bdrv_close = block_crypto_close, \ > > + .bdrv_create = block_crypto_create_ ## name, \ > > + .create_opts = &block_crypto_create_opts_ ## name, \ > > + \ > > + .bdrv_co_readv = block_crypto_co_readv, \ > > + .bdrv_co_writev = block_crypto_co_writev, \ > > + .bdrv_getlength = block_crypto_getlength, \ > > + } > > + > > +BLOCK_CRYPTO_DRIVER(luks, Q_CRYPTO_BLOCK_FORMAT_LUKS); > > Personally I really prefer a preprocessed version, for the ease of grep. I'm not sure I understand what you mean by a preprocessed version - could you expand on that. Regards, Daniel
On Thu, 01/21 11:02, Daniel P. Berrange wrote: > On Thu, Jan 21, 2016 at 05:12:08PM +0800, Fam Zheng wrote: > > On Wed, 01/20 17:38, Daniel P. Berrange wrote: > > > + /* XXX Should we treat size as being total physical size > > > + * of the image (ie payload + encryption header), or just > > > + * the logical size of the image (ie payload). If the latter > > > + * then we need to extend 'size' to include the header > > > + * size */ > > > > The latter. :) > > Ok > > > > + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort); > > > +#define BLOCK_CRYPTO_DRIVER(name, format) \ > > > + static int block_crypto_probe_ ## name(const uint8_t *buf, \ > > > + int buf_size, \ > > > + const char *filename) { \ > > > + return block_crypto_probe_generic(format, \ > > > + buf, buf_size, filename); \ > > > + } \ > > > + \ > > > + static int block_crypto_open_ ## name(BlockDriverState *bs, \ > > > + QDict *options, \ > > > + int flags, \ > > > + Error **errp) \ > > > + { \ > > > + return block_crypto_open_generic(format, \ > > > + &block_crypto_runtime_opts_ ## name, \ > > > + bs, options, flags, errp); \ > > > + } \ > > > + \ > > > + static int block_crypto_create_ ## name(const char *filename, \ > > > + QemuOpts *opts, \ > > > + Error **errp) \ > > > + { \ > > > + return block_crypto_create_generic(format, \ > > > + filename, opts, errp); \ > > > + } \ > > > + \ > > > + BlockDriver bdrv_crypto_ ## name = { \ > > > + .format_name = #name, \ > > > + .instance_size = sizeof(BlockCrypto), \ > > > + .bdrv_probe = block_crypto_probe_ ## name, \ > > > + .bdrv_open = block_crypto_open_ ## name, \ > > > + .bdrv_close = block_crypto_close, \ > > > + .bdrv_create = block_crypto_create_ ## name, \ > > > + .create_opts = &block_crypto_create_opts_ ## name, \ > > > + \ > > > + .bdrv_co_readv = block_crypto_co_readv, \ > > > + .bdrv_co_writev = block_crypto_co_writev, \ > > > + .bdrv_getlength = block_crypto_getlength, \ > > > + } > > > + > > > +BLOCK_CRYPTO_DRIVER(luks, Q_CRYPTO_BLOCK_FORMAT_LUKS); > > > > Personally I really prefer a preprocessed version, for the ease of grep. > > I'm not sure I understand what you mean by a preprocessed version - could > you expand on that. I mean don't use macro concatenation and use plain symbols like in other block drivers. BlockDriver bdrv_crypto_luks = { .format_name = "luks", .instance_size = sizeof(BlockCrypto), .bdrv_probe = block_crypto_probe_luks, .bdrv_open = block_crypto_open_luks, ... mostly because it's easier to grep (or for refactoring with tools). But I can't how repeatitive this would be (I can see the "don't repeat yourself" with your approach). There is only one BLOCK_CRYPTO_DRIVER instance in this series. This is probably bikeshedding. Fam
On Thu, Jan 21, 2016 at 09:01:19PM +0800, Fam Zheng wrote: > On Thu, 01/21 11:02, Daniel P. Berrange wrote: > > On Thu, Jan 21, 2016 at 05:12:08PM +0800, Fam Zheng wrote: > > > On Wed, 01/20 17:38, Daniel P. Berrange wrote: > > > > + /* XXX Should we treat size as being total physical size > > > > + * of the image (ie payload + encryption header), or just > > > > + * the logical size of the image (ie payload). If the latter > > > > + * then we need to extend 'size' to include the header > > > > + * size */ > > > > > > The latter. :) > > > > Ok > > > > > > + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort); > > > > +#define BLOCK_CRYPTO_DRIVER(name, format) \ > > > > + static int block_crypto_probe_ ## name(const uint8_t *buf, \ > > > > + int buf_size, \ > > > > + const char *filename) { \ > > > > + return block_crypto_probe_generic(format, \ > > > > + buf, buf_size, filename); \ > > > > + } \ > > > > + \ > > > > + static int block_crypto_open_ ## name(BlockDriverState *bs, \ > > > > + QDict *options, \ > > > > + int flags, \ > > > > + Error **errp) \ > > > > + { \ > > > > + return block_crypto_open_generic(format, \ > > > > + &block_crypto_runtime_opts_ ## name, \ > > > > + bs, options, flags, errp); \ > > > > + } \ > > > > + \ > > > > + static int block_crypto_create_ ## name(const char *filename, \ > > > > + QemuOpts *opts, \ > > > > + Error **errp) \ > > > > + { \ > > > > + return block_crypto_create_generic(format, \ > > > > + filename, opts, errp); \ > > > > + } \ > > > > + \ > > > > + BlockDriver bdrv_crypto_ ## name = { \ > > > > + .format_name = #name, \ > > > > + .instance_size = sizeof(BlockCrypto), \ > > > > + .bdrv_probe = block_crypto_probe_ ## name, \ > > > > + .bdrv_open = block_crypto_open_ ## name, \ > > > > + .bdrv_close = block_crypto_close, \ > > > > + .bdrv_create = block_crypto_create_ ## name, \ > > > > + .create_opts = &block_crypto_create_opts_ ## name, \ > > > > + \ > > > > + .bdrv_co_readv = block_crypto_co_readv, \ > > > > + .bdrv_co_writev = block_crypto_co_writev, \ > > > > + .bdrv_getlength = block_crypto_getlength, \ > > > > + } > > > > + > > > > +BLOCK_CRYPTO_DRIVER(luks, Q_CRYPTO_BLOCK_FORMAT_LUKS); > > > > > > Personally I really prefer a preprocessed version, for the ease of grep. > > > > I'm not sure I understand what you mean by a preprocessed version - could > > you expand on that. > > I mean don't use macro concatenation and use plain symbols like in other block > drivers. > > BlockDriver bdrv_crypto_luks = { > .format_name = "luks", > .instance_size = sizeof(BlockCrypto), > .bdrv_probe = block_crypto_probe_luks, > .bdrv_open = block_crypto_open_luks, > ... > > mostly because it's easier to grep (or for refactoring with tools). > > But I can't how repeatitive this would be (I can see the "don't repeat > yourself" with your approach). There is only one BLOCK_CRYPTO_DRIVER instance > in this series. This is probably bikeshedding. Yeah, thinking about it, my macro approach is probably overkill for the forseeable future anyway. I was future proofing wrt possible addition of trucrypt support, but that's soo far off its not something i really need worry about now. Regards, Daniel
On 01/20/2016 10:38 AM, Daniel P. Berrange wrote: > Add a block driver that is capable of supporting any full disk > encryption format. This utilizes the previously added block > encryption code, and at this time supports the LUKS format. > > The driver code is capable of supporting any format supported > by the QCryptoBlock module, so it registers one block driver > for each format. > > At this time, the "luks" driver is registered. New LUKS > compatible volume can be formatted using qemu-img > > $ qemu-img create --object secret,data=123456,id=sec0 \ > -f luks -o key-secret=sec0,cipher-alg=aes-256,\ > cipher-mode=cbc,ivgen-alg=plain64,hash-alg=sha256 \ > demo.luks 10G Are you still adding -o options like this, or is it better to use the new --image-opts flag from your other series for this purpose? > > And query its size > > $ qemu-img info --object secret,data=123456,id=sec0 --source demo.luks,driver=luks,key-secret=sec0 And this definitely looks stale. > image: json:{"key-secret": "sec0", "driver": "luks", "file": {"driver": "file", "filename": "demo.luks"}} > file format: luks > virtual size: 10.0G (10737416192 bytes) That size is 2048 bytes less than 10G; what happened? With default striping, you have 4000 stripes * 32 byte aes-256 key * 8 key material + header (where I pointed out that you are using a default offset of 4096, even though 1024 would be sufficient on 512-byte sectors), or roughly 1M, occupied by the LUKS header (in fact, having the payload aligned to 1M is probably wise, even if it can pack in tighter, just because a 1M-aligned disk with a 1M header is a GOOD thing). Therefore, you should either be counting the _entire_ LUKS header as part of the image (and report a full 10G), or you should _only_ be counting the payload size (and report 10G-1M, not 10G-2k). My vote: do the same as we do for qcow2 or any other format. Make the size requested by the user as the size visible to the guest, and a fully-allocated image will take more space on the host than what the guest is using (that is, a fully-allocated 10G LUKS disk would present 10G payload to the guest but occupy 10G+1M on the host). > disk size: 132K This, however, looks reasonable - with the defaults, key material 0 will occupy just under 128k, and nothing is written in key material 1-7. > > All volumes created by this new 'luks' driver should be > capable of being opened by the kernel dm-crypt driver. > With this initial impl, not all volumes created with > dm-crypt can be opened by the QEMU 'luks' driver. This > is due to lack of support for certain algorithms, in > particular the 'xts' cipher mode. These limitations will > be addressed in a later series of patches, with the > intent that QEMU should be able to open anything that > dm-crypt LUKS supports. Cool! Would it be worth augmenting the commit message to show a sequence of commands where you loopback mount a LUKS image file created by qemu, then wire that up with cryptsetup, as a proof that the two are compatible implementations? > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > block/Makefile.objs | 2 + > block/crypto.c | 541 +++++++++++++++++++++++++++++++++++++++++++++++++++ > qapi/block-core.json | 18 +- > 3 files changed, 560 insertions(+), 1 deletion(-) > create mode 100644 block/crypto.c > > +++ b/block/crypto.c > + > +#define BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET "key-secret" > +#define BLOCK_CRYPTO_OPT_LUKS_CIPHER_ALG "cipher-alg" > +#define BLOCK_CRYPTO_OPT_LUKS_CIPHER_MODE "cipher-mode" > +#define BLOCK_CRYPTO_OPT_LUKS_IVGEN_ALG "ivgen-alg" > +#define BLOCK_CRYPTO_OPT_LUKS_IVGEN_HASH_ALG "ivgen-hash-alg" > +#define BLOCK_CRYPTO_OPT_LUKS_HASH_ALG "hash-alg" I suggested in 7/17 that user-specified striping might be a parameter worth adding; I guess this would be impacted as well. > + > +static int block_crypto_probe_generic(QCryptoBlockFormat format, > + const uint8_t *buf, > + int buf_size, > + const char *filename) > +{ > + if (qcrypto_block_has_format(format, > + buf, buf_size)) { > + return 100; I don't know why we have particular values for any of the probes, but at least this matches the arbitrary value reported by qcow2. > + } else { > + return 0; > + } > +} > + > + > +static ssize_t block_crypto_read_func(QCryptoBlock *block, > + size_t offset, > + uint8_t *buf, > + size_t buflen, > + Error **errp, > + void *opaque) > +{ > + BlockDriverState *bs = opaque; > + ssize_t ret; > + > + ret = bdrv_pread(bs->file->bs, offset, buf, buflen); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "Could not read encryption header"); Are we guaranteed that the offset being read here will always lie in the encryption header portion of the file? > + return ret; > + } > + return ret; > +} > + > + > +static ssize_t block_crypto_write_func(QCryptoBlock *block, > + size_t offset, > + const uint8_t *buf, > + size_t buflen, > + Error **errp, > + void *opaque) > +{ > + BlockDriverState *bs = opaque; > + ssize_t ret; > + > + ret = bdrv_pwrite(bs, offset, buf, buflen); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "Could not write encryption header"); Likewise. > + > +#define BLOCK_CRYPTO_MAX_SECTORS 32 16k is not enough space to represent the LUKS header and the first key material. Does this number need to be larger to allow reading up to 1M of the image to find all 8 key materials? Or is this just being used to limit the maximum amount we'll encrypt/decrypt in one pass of the loop? > + > +static coroutine_fn int > +block_crypto_co_readv(BlockDriverState *bs, int64_t sector_num, > + int remaining_sectors, QEMUIOVector *qiov) > +{ > + BlockCrypto *crypto = bs->opaque; > + int cur_nr_sectors; /* number of sectors in current iteration */ > + uint64_t bytes_done = 0; > + uint8_t *cipher_data = NULL; > + QEMUIOVector hd_qiov; > + int ret = 0; > + size_t payload_offset = qcrypto_block_get_payload_offset(crypto->block); > + > + qemu_iovec_init(&hd_qiov, qiov->niov); > + > + qemu_co_mutex_lock(&crypto->lock); > + > + while (remaining_sectors) { > + cur_nr_sectors = remaining_sectors; > + > + if (cur_nr_sectors > BLOCK_CRYPTO_MAX_SECTORS) { > + cur_nr_sectors = BLOCK_CRYPTO_MAX_SECTORS; > + } > + cipher_data = > + qemu_try_blockalign(bs->file->bs, cur_nr_sectors * 512); Why are you allocating the block in the loop? Can you hoist the allocation outside, and reuse the memory across iterations? > + > +static int64_t block_crypto_getlength(BlockDriverState *bs) > +{ > + BlockCrypto *crypto = bs->opaque; > + int64_t len = bdrv_getlength(bs->file->bs); > + > + ssize_t offset = qcrypto_block_get_payload_offset(crypto->block); > + > + len -= (offset * 512); Will need a tweak if you fix qcrypto_block_get_payload_offset to be in bytes. > +++ b/qapi/block-core.json > @@ -1546,7 +1546,7 @@ > { 'enum': 'BlockdevDriver', > 'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop', > 'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device', > - 'http', 'https', 'null-aio', 'null-co', 'parallels', > + 'http', 'https', 'luks', 'null-aio', 'null-co', 'parallels', Missing documentation; BlockDeviceInfo has been where we've documented which release introduced which drivers. > ## > +# @BlockdevOptionsLUKS > +# > +# Driver specific block device options for LUKS. > +# > +# @key-secret: #optional the ID of a QCryptoSecret object providing > +# the decryption key (since 2.6) Worth mentioning that it is mandatory except when doing a metadata-only probe.
On Fri, Feb 05, 2016 at 03:20:43PM -0700, Eric Blake wrote: > On 01/20/2016 10:38 AM, Daniel P. Berrange wrote: > > Add a block driver that is capable of supporting any full disk > > encryption format. This utilizes the previously added block > > encryption code, and at this time supports the LUKS format. > > > > The driver code is capable of supporting any format supported > > by the QCryptoBlock module, so it registers one block driver > > for each format. > > > > At this time, the "luks" driver is registered. New LUKS > > compatible volume can be formatted using qemu-img > > > > $ qemu-img create --object secret,data=123456,id=sec0 \ > > -f luks -o key-secret=sec0,cipher-alg=aes-256,\ > > cipher-mode=cbc,ivgen-alg=plain64,hash-alg=sha256 \ > > demo.luks 10G > > Are you still adding -o options like this, or is it better to use the > new --image-opts flag from your other series for this purpose? These are options for image creation, so they must still use -o. My patches only deal with options for pre-existing images. > > And query its size > > > > $ qemu-img info --object secret,data=123456,id=sec0 --source demo.luks,driver=luks,key-secret=sec0 > > And this definitely looks stale. Yes, its wrong. > > > image: json:{"key-secret": "sec0", "driver": "luks", "file": {"driver": "file", "filename": "demo.luks"}} > > file format: luks > > virtual size: 10.0G (10737416192 bytes) > > That size is 2048 bytes less than 10G; what happened? With default > striping, you have 4000 stripes * 32 byte aes-256 key * 8 key material + > header (where I pointed out that you are using a default offset of 4096, > even though 1024 would be sufficient on 512-byte sectors), or roughly > 1M, occupied by the LUKS header (in fact, having the payload aligned to > 1M is probably wise, even if it can pack in tighter, just because a > 1M-aligned disk with a 1M header is a GOOD thing). Therefore, you > should either be counting the _entire_ LUKS header as part of the image > (and report a full 10G), or you should _only_ be counting the payload > size (and report 10G-1M, not 10G-2k). It is supposed to be reporting 10G - luks header size. Not sure why this example shown 2k less - probably a bug from a much earlier version of the patchset. I think it probably dates from when I had mistakenly not multiplied payload offset by sector size or some such. > My vote: do the same as we do for qcow2 or any other format. Make the > size requested by the user as the size visible to the guest, and a > fully-allocated image will take more space on the host than what the > guest is using (that is, a fully-allocated 10G LUKS disk would present > 10G payload to the guest but occupy 10G+1M on the host). Yes, I had a todo item in the code asking which was better /* XXX Should we treat size as being total physical size * of the image (ie payload + encryption header), or just * the logical size of the image (ie payload). If the latter * then we need to extend 'size' to include the header * size */ qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort); And Fam suggested the same as you - show full 10 G to the guest. The complication is that we need to create the backing file before we format the luks header, and we don't know the size of the luks header at that point. So either I could format the luks header into a temporary in-memory buffer, or I have to create the file and then resize it afterwards, or I have to provide some way to calculate the eventual header size prior to creating it. I didn't much like any of those options :-) > > disk size: 132K > > This, however, looks reasonable - with the defaults, key material 0 will > occupy just under 128k, and nothing is written in key material 1-7. > > > > > All volumes created by this new 'luks' driver should be > > capable of being opened by the kernel dm-crypt driver. > > With this initial impl, not all volumes created with > > dm-crypt can be opened by the QEMU 'luks' driver. This > > is due to lack of support for certain algorithms, in > > particular the 'xts' cipher mode. These limitations will > > be addressed in a later series of patches, with the > > intent that QEMU should be able to open anything that > > dm-crypt LUKS supports. > > Cool! Would it be worth augmenting the commit message to show a > sequence of commands where you loopback mount a LUKS image file created > by qemu, then wire that up with cryptsetup, as a proof that the two are > compatible implementations? My intent is to add something to tests/qemu-iotests that will check interoperability between qemu + cryptsetup. Slight complication is that those io tests all expect to run unprivileged. So it'll need a manual step run privileged to create the cryptsetup disk images for testing with. > > +++ b/block/crypto.c > > > + > > +#define BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET "key-secret" > > +#define BLOCK_CRYPTO_OPT_LUKS_CIPHER_ALG "cipher-alg" > > +#define BLOCK_CRYPTO_OPT_LUKS_CIPHER_MODE "cipher-mode" > > +#define BLOCK_CRYPTO_OPT_LUKS_IVGEN_ALG "ivgen-alg" > > +#define BLOCK_CRYPTO_OPT_LUKS_IVGEN_HASH_ALG "ivgen-hash-alg" > > +#define BLOCK_CRYPTO_OPT_LUKS_HASH_ALG "hash-alg" > > I suggested in 7/17 that user-specified striping might be a parameter > worth adding; I guess this would be impacted as well. Same note as before - cryptsetup hardcodes is to 4k, so I figure it is fine todo the same right now. > > +static ssize_t block_crypto_read_func(QCryptoBlock *block, > > + size_t offset, > > + uint8_t *buf, > > + size_t buflen, > > + Error **errp, > > + void *opaque) > > +{ > > + BlockDriverState *bs = opaque; > > + ssize_t ret; > > + > > + ret = bdrv_pread(bs->file->bs, offset, buf, buflen); > > + if (ret < 0) { > > + error_setg_errno(errp, -ret, "Could not read encryption header"); > > Are we guaranteed that the offset being read here will always lie in the > encryption header portion of the file? I don't think we need to care about that here. We're just honouring reads from whereever the crypto driver wants - we don't want the block driver here to have specific knowledge of the underlying crypto format. > > > + return ret; > > + } > > + return ret; > > +} > > + > > + > > +static ssize_t block_crypto_write_func(QCryptoBlock *block, > > + size_t offset, > > + const uint8_t *buf, > > + size_t buflen, > > + Error **errp, > > + void *opaque) > > +{ > > + BlockDriverState *bs = opaque; > > + ssize_t ret; > > + > > + ret = bdrv_pwrite(bs, offset, buf, buflen); > > + if (ret < 0) { > > + error_setg_errno(errp, -ret, "Could not write encryption header"); > > Likewise. > > > + > > +#define BLOCK_CRYPTO_MAX_SECTORS 32 > > 16k is not enough space to represent the LUKS header and the first key > material. Does this number need to be larger to allow reading up to 1M > of the image to find all 8 key materials? > > Or is this just being used to limit the maximum amount we'll > encrypt/decrypt in one pass of the loop? Yeah, this is just used for batching I/O to the underling block device, so that we don't need to allocate an arbitrarily large temporary buffer for (en|de)cryption. > > +static coroutine_fn int > > +block_crypto_co_readv(BlockDriverState *bs, int64_t sector_num, > > + int remaining_sectors, QEMUIOVector *qiov) > > +{ > > + BlockCrypto *crypto = bs->opaque; > > + int cur_nr_sectors; /* number of sectors in current iteration */ > > + uint64_t bytes_done = 0; > > + uint8_t *cipher_data = NULL; > > + QEMUIOVector hd_qiov; > > + int ret = 0; > > + size_t payload_offset = qcrypto_block_get_payload_offset(crypto->block); > > + > > + qemu_iovec_init(&hd_qiov, qiov->niov); > > + > > + qemu_co_mutex_lock(&crypto->lock); > > + > > + while (remaining_sectors) { > > + cur_nr_sectors = remaining_sectors; > > + > > + if (cur_nr_sectors > BLOCK_CRYPTO_MAX_SECTORS) { > > + cur_nr_sectors = BLOCK_CRYPTO_MAX_SECTORS; > > + } > > + cipher_data = > > + qemu_try_blockalign(bs->file->bs, cur_nr_sectors * 512); > > Why are you allocating the block in the loop? Can you hoist the > allocation outside, and reuse the memory across iterations? No particular reason - this code pattern is just copied from equivalent methods in qcow2.c > > > + > > +static int64_t block_crypto_getlength(BlockDriverState *bs) > > +{ > > + BlockCrypto *crypto = bs->opaque; > > + int64_t len = bdrv_getlength(bs->file->bs); > > + > > + ssize_t offset = qcrypto_block_get_payload_offset(crypto->block); > > + > > + len -= (offset * 512); > > Will need a tweak if you fix qcrypto_block_get_payload_offset to be in > bytes. Yep > > +++ b/qapi/block-core.json > > @@ -1546,7 +1546,7 @@ > > { 'enum': 'BlockdevDriver', > > 'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop', > > 'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device', > > - 'http', 'https', 'null-aio', 'null-co', 'parallels', > > + 'http', 'https', 'luks', 'null-aio', 'null-co', 'parallels', > > Missing documentation; BlockDeviceInfo has been where we've documented > which release introduced which drivers. Ah ok > > > ## > > +# @BlockdevOptionsLUKS > > +# > > +# Driver specific block device options for LUKS. > > +# > > +# @key-secret: #optional the ID of a QCryptoSecret object providing > > +# the decryption key (since 2.6) > > Worth mentioning that it is mandatory except when doing a metadata-only > probe. Yes Regards, Daniel
On 02/08/2016 09:28 AM, Daniel P. Berrange wrote: >> My vote: do the same as we do for qcow2 or any other format. Make the >> size requested by the user as the size visible to the guest, and a >> fully-allocated image will take more space on the host than what the >> guest is using (that is, a fully-allocated 10G LUKS disk would present >> 10G payload to the guest but occupy 10G+1M on the host). > > Yes, I had a todo item in the code asking which was better > > /* XXX Should we treat size as being total physical size > * of the image (ie payload + encryption header), or just > * the logical size of the image (ie payload). If the latter > * then we need to extend 'size' to include the header > * size */ > qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort); > > And Fam suggested the same as you - show full 10 G to the guest. > > The complication is that we need to create the backing file > before we format the luks header, and we don't know the size > of the luks header at that point. So either I could format > the luks header into a temporary in-memory buffer, or I have > to create the file and then resize it afterwards, or I have > to provide some way to calculate the eventual header size > prior to creating it. I didn't much like any of those options :-) Well, if we hard-code stripes==4000, then we pretty much know that the header is just under 1M for the maximum size key (aes-256); and rounding up to 1M is nice for all cluster sizes except 2M. I suppose we could fit in 512k if we use a smaller key (aes-128), but would it hurt to just hard-code a reservation of 1M? >> Cool! Would it be worth augmenting the commit message to show a >> sequence of commands where you loopback mount a LUKS image file created >> by qemu, then wire that up with cryptsetup, as a proof that the two are >> compatible implementations? > > My intent is to add something to tests/qemu-iotests that will check > interoperability between qemu + cryptsetup. Slight complication is > that those io tests all expect to run unprivileged. So it'll need > a manual step run privileged to create the cryptsetup disk images > for testing with. We've checked in compressed binary images before; but 1M of key material won't compress well.
On Mon, Feb 08, 2016 at 01:23:28PM -0700, Eric Blake wrote: > On 02/08/2016 09:28 AM, Daniel P. Berrange wrote: > > >> My vote: do the same as we do for qcow2 or any other format. Make the > >> size requested by the user as the size visible to the guest, and a > >> fully-allocated image will take more space on the host than what the > >> guest is using (that is, a fully-allocated 10G LUKS disk would present > >> 10G payload to the guest but occupy 10G+1M on the host). > > > > Yes, I had a todo item in the code asking which was better > > > > /* XXX Should we treat size as being total physical size > > * of the image (ie payload + encryption header), or just > > * the logical size of the image (ie payload). If the latter > > * then we need to extend 'size' to include the header > > * size */ > > qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort); > > > > And Fam suggested the same as you - show full 10 G to the guest. > > > > The complication is that we need to create the backing file > > before we format the luks header, and we don't know the size > > of the luks header at that point. So either I could format > > the luks header into a temporary in-memory buffer, or I have > > to create the file and then resize it afterwards, or I have > > to provide some way to calculate the eventual header size > > prior to creating it. I didn't much like any of those options :-) > > Well, if we hard-code stripes==4000, then we pretty much know that the > header is just under 1M for the maximum size key (aes-256); and rounding > up to 1M is nice for all cluster sizes except 2M. I suppose we could > fit in 512k if we use a smaller key (aes-128), but would it hurt to just > hard-code a reservation of 1M? I actually figured out a way to deal with this - i just have to delay creation of the underling file, to be done in the callback which initilizes the luks header region. > >> Cool! Would it be worth augmenting the commit message to show a > >> sequence of commands where you loopback mount a LUKS image file created > >> by qemu, then wire that up with cryptsetup, as a proof that the two are > >> compatible implementations? > > > > My intent is to add something to tests/qemu-iotests that will check > > interoperability between qemu + cryptsetup. Slight complication is > > that those io tests all expect to run unprivileged. So it'll need > > a manual step run privileged to create the cryptsetup disk images > > for testing with. > > We've checked in compressed binary images before; but 1M of key material > won't compress well. And realistically it is going to be 10's of MB per image, since I want the test to verify more than just the header, and there's probably about 10+ different cipher combinations, so it would be many 100's of MBs to check in. So that won't fly sadly. > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > Regards, Daniel
diff --git a/block/Makefile.objs b/block/Makefile.objs index 58ef2ef..12eae77 100644 --- a/block/Makefile.objs +++ b/block/Makefile.objs @@ -23,6 +23,8 @@ block-obj-$(CONFIG_LIBSSH2) += ssh.o block-obj-y += accounting.o block-obj-y += write-threshold.o +block-obj-y += crypto.o + common-obj-y += stream.o common-obj-y += commit.o common-obj-y += backup.o diff --git a/block/crypto.c b/block/crypto.c new file mode 100644 index 0000000..2ba78bd --- /dev/null +++ b/block/crypto.c @@ -0,0 +1,541 @@ +/* + * QEMU block full disk encryption + * + * Copyright (c) 2015-2016 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see <http://www.gnu.org/licenses/>. + * + */ + +#include "config-host.h" + +#include "block/block_int.h" +#include "crypto/block.h" +#include "qapi/opts-visitor.h" +#include "qapi-visit.h" + +#define BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET "key-secret" +#define BLOCK_CRYPTO_OPT_LUKS_CIPHER_ALG "cipher-alg" +#define BLOCK_CRYPTO_OPT_LUKS_CIPHER_MODE "cipher-mode" +#define BLOCK_CRYPTO_OPT_LUKS_IVGEN_ALG "ivgen-alg" +#define BLOCK_CRYPTO_OPT_LUKS_IVGEN_HASH_ALG "ivgen-hash-alg" +#define BLOCK_CRYPTO_OPT_LUKS_HASH_ALG "hash-alg" + +typedef struct BlockCrypto BlockCrypto; + +struct BlockCrypto { + QCryptoBlock *block; + CoMutex lock; +}; + + +static int block_crypto_probe_generic(QCryptoBlockFormat format, + const uint8_t *buf, + int buf_size, + const char *filename) +{ + if (qcrypto_block_has_format(format, + buf, buf_size)) { + return 100; + } else { + return 0; + } +} + + +static ssize_t block_crypto_read_func(QCryptoBlock *block, + size_t offset, + uint8_t *buf, + size_t buflen, + Error **errp, + void *opaque) +{ + BlockDriverState *bs = opaque; + ssize_t ret; + + ret = bdrv_pread(bs->file->bs, offset, buf, buflen); + if (ret < 0) { + error_setg_errno(errp, -ret, "Could not read encryption header"); + return ret; + } + return ret; +} + + +static ssize_t block_crypto_write_func(QCryptoBlock *block, + size_t offset, + const uint8_t *buf, + size_t buflen, + Error **errp, + void *opaque) +{ + BlockDriverState *bs = opaque; + ssize_t ret; + + ret = bdrv_pwrite(bs, offset, buf, buflen); + if (ret < 0) { + error_setg_errno(errp, -ret, "Could not write encryption header"); + return ret; + } + return ret; +} + + +static ssize_t block_crypto_init_func(QCryptoBlock *block, + size_t headerlen, + Error **errp, + void *opaque) +{ + /* We don't need to do anything special to reserve space */ + return 0; +} + + +static QemuOptsList block_crypto_runtime_opts_luks = { + .name = "crypto", + .head = QTAILQ_HEAD_INITIALIZER(block_crypto_runtime_opts_luks.head), + .desc = { + { + .name = BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET, + .type = QEMU_OPT_STRING, + .help = "ID of the secret that provides the encryption key", + }, + { /* end of list */ } + }, +}; + + +static QemuOptsList block_crypto_create_opts_luks = { + .name = "crypto", + .head = QTAILQ_HEAD_INITIALIZER(block_crypto_create_opts_luks.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_SIZE, + .help = "Virtual disk size" + }, + { + .name = BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET, + .type = QEMU_OPT_STRING, + .help = "ID of the secret that provides the encryption key", + }, + { + .name = BLOCK_CRYPTO_OPT_LUKS_CIPHER_ALG, + .type = QEMU_OPT_STRING, + .help = "Name of encryption cipher algorithm", + }, + { + .name = BLOCK_CRYPTO_OPT_LUKS_CIPHER_MODE, + .type = QEMU_OPT_STRING, + .help = "Name of encryption cipher mode", + }, + { + .name = BLOCK_CRYPTO_OPT_LUKS_IVGEN_ALG, + .type = QEMU_OPT_STRING, + .help = "Name of IV generator algorithm", + }, + { + .name = BLOCK_CRYPTO_OPT_LUKS_IVGEN_HASH_ALG, + .type = QEMU_OPT_STRING, + .help = "Name of IV generator hash algorithm", + }, + { + .name = BLOCK_CRYPTO_OPT_LUKS_HASH_ALG, + .type = QEMU_OPT_STRING, + .help = "Name of encryption hash algorithm", + }, + { /* end of list */ } + }, +}; + + +static QCryptoBlockOpenOptions * +block_crypto_open_opts_init(QCryptoBlockFormat format, + QemuOpts *opts, + Error **errp) +{ + OptsVisitor *ov; + QCryptoBlockOpenOptions *ret; + Error *local_err = NULL; + + ret = g_new0(QCryptoBlockOpenOptions, 1); + ret->format = format; + + ov = opts_visitor_new(opts); + + switch (format) { + case Q_CRYPTO_BLOCK_FORMAT_LUKS: + ret->u.luks = g_new0(QCryptoBlockOptionsLUKS, 1); + visit_type_QCryptoBlockOptionsLUKS(opts_get_visitor(ov), + &ret->u.luks, "luks", &local_err); + break; + + default: + error_setg(&local_err, "Unsupported block format %d", format); + break; + } + + if (local_err) { + error_propagate(errp, local_err); + opts_visitor_cleanup(ov); + qapi_free_QCryptoBlockOpenOptions(ret); + return NULL; + } + + opts_visitor_cleanup(ov); + return ret; +} + + +static QCryptoBlockCreateOptions * +block_crypto_create_opts_init(QCryptoBlockFormat format, + QemuOpts *opts, + Error **errp) +{ + OptsVisitor *ov; + QCryptoBlockCreateOptions *ret; + Error *local_err = NULL; + + ret = g_new0(QCryptoBlockCreateOptions, 1); + ret->format = format; + + ov = opts_visitor_new(opts); + + switch (format) { + case Q_CRYPTO_BLOCK_FORMAT_LUKS: + ret->u.luks = g_new0(QCryptoBlockCreateOptionsLUKS, 1); + visit_type_QCryptoBlockCreateOptionsLUKS( + opts_get_visitor(ov), + &ret->u.luks, "luks", &local_err); + break; + + default: + error_setg(&local_err, "Unsupported block format %d", format); + break; + } + + if (local_err) { + error_propagate(errp, local_err); + opts_visitor_cleanup(ov); + qapi_free_QCryptoBlockCreateOptions(ret); + return NULL; + } + + opts_visitor_cleanup(ov); + return ret; +} + + +static int block_crypto_open_generic(QCryptoBlockFormat format, + QemuOptsList *opts_spec, + BlockDriverState *bs, + QDict *options, + int flags, + Error **errp) +{ + BlockCrypto *crypto = bs->opaque; + QemuOpts *opts = NULL; + Error *local_err = NULL; + int ret = -EINVAL; + QCryptoBlockOpenOptions *open_opts = NULL; + unsigned int cflags = 0; + + opts = qemu_opts_create(opts_spec, NULL, 0, &error_abort); + qemu_opts_absorb_qdict(opts, options, &local_err); + if (local_err) { + error_propagate(errp, local_err); + goto cleanup; + } + + open_opts = block_crypto_open_opts_init(format, opts, errp); + if (!open_opts) { + goto cleanup; + } + + if (flags & BDRV_O_NO_IO) { + cflags |= QCRYPTO_BLOCK_OPEN_NO_IO; + } + crypto->block = qcrypto_block_open(open_opts, + block_crypto_read_func, + bs, + cflags, + errp); + + if (!crypto->block) { + ret = -EIO; + goto cleanup; + } + + bs->encrypted = 1; + bs->valid_key = 1; + + qemu_co_mutex_init(&crypto->lock); + + ret = 0; + cleanup: + qapi_free_QCryptoBlockOpenOptions(open_opts); + return ret; +} + + +static int block_crypto_create_generic(QCryptoBlockFormat format, + const char *filename, + QemuOpts *opts, + Error **errp) +{ + int ret = -EINVAL; + QCryptoBlockCreateOptions *create_opts = NULL; + BlockDriverState *bs = NULL; + QCryptoBlock *crypto = NULL; + uint64_t size = 0; + + size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0), + BDRV_SECTOR_SIZE); + + create_opts = block_crypto_create_opts_init(format, opts, errp); + if (!create_opts) { + return -1; + } + + /* XXX Should we treat size as being total physical size + * of the image (ie payload + encryption header), or just + * the logical size of the image (ie payload). If the latter + * then we need to extend 'size' to include the header + * size */ + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort); + ret = bdrv_create_file(filename, opts, errp); + if (ret < 0) { + goto cleanup; + } + + ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL, + errp); + if (ret < 0) { + goto cleanup; + } + + crypto = qcrypto_block_create(create_opts, + block_crypto_init_func, + block_crypto_write_func, + bs, + errp); + + if (!crypto) { + ret = -EIO; + goto cleanup; + } + + ret = 0; + cleanup: + qcrypto_block_free(crypto); + bdrv_unref(bs); + qapi_free_QCryptoBlockCreateOptions(create_opts); + return ret; +} + +static void block_crypto_close(BlockDriverState *bs) +{ + BlockCrypto *crypto = bs->opaque; + qcrypto_block_free(crypto->block); +} + + +#define BLOCK_CRYPTO_MAX_SECTORS 32 + +static coroutine_fn int +block_crypto_co_readv(BlockDriverState *bs, int64_t sector_num, + int remaining_sectors, QEMUIOVector *qiov) +{ + BlockCrypto *crypto = bs->opaque; + int cur_nr_sectors; /* number of sectors in current iteration */ + uint64_t bytes_done = 0; + uint8_t *cipher_data = NULL; + QEMUIOVector hd_qiov; + int ret = 0; + size_t payload_offset = qcrypto_block_get_payload_offset(crypto->block); + + qemu_iovec_init(&hd_qiov, qiov->niov); + + qemu_co_mutex_lock(&crypto->lock); + + while (remaining_sectors) { + cur_nr_sectors = remaining_sectors; + + if (cur_nr_sectors > BLOCK_CRYPTO_MAX_SECTORS) { + cur_nr_sectors = BLOCK_CRYPTO_MAX_SECTORS; + } + cipher_data = + qemu_try_blockalign(bs->file->bs, cur_nr_sectors * 512); + + qemu_iovec_reset(&hd_qiov); + qemu_iovec_add(&hd_qiov, cipher_data, cur_nr_sectors * 512); + + qemu_co_mutex_unlock(&crypto->lock); + ret = bdrv_co_readv(bs->file->bs, + payload_offset + sector_num, + cur_nr_sectors, &hd_qiov); + qemu_co_mutex_lock(&crypto->lock); + if (ret < 0) { + goto cleanup; + } + + if (qcrypto_block_decrypt(crypto->block, + sector_num, + cipher_data, cur_nr_sectors * 512, + NULL) < 0) { + ret = -1; + goto cleanup; + } + + qemu_iovec_from_buf(qiov, bytes_done, + cipher_data, cur_nr_sectors * 512); + + remaining_sectors -= cur_nr_sectors; + sector_num += cur_nr_sectors; + bytes_done += cur_nr_sectors * 512; + } + + cleanup: + qemu_co_mutex_unlock(&crypto->lock); + + qemu_iovec_destroy(&hd_qiov); + qemu_vfree(cipher_data); + + return ret; +} + + +static coroutine_fn int +block_crypto_co_writev(BlockDriverState *bs, int64_t sector_num, + int remaining_sectors, QEMUIOVector *qiov) +{ + BlockCrypto *crypto = bs->opaque; + int cur_nr_sectors; /* number of sectors in current iteration */ + uint64_t bytes_done = 0; + uint8_t *cipher_data = NULL; + QEMUIOVector hd_qiov; + int ret = 0; + size_t payload_offset = qcrypto_block_get_payload_offset(crypto->block); + + qemu_iovec_init(&hd_qiov, qiov->niov); + + qemu_co_mutex_lock(&crypto->lock); + + while (remaining_sectors) { + cur_nr_sectors = remaining_sectors; + + if (cur_nr_sectors > BLOCK_CRYPTO_MAX_SECTORS) { + cur_nr_sectors = BLOCK_CRYPTO_MAX_SECTORS; + } + cipher_data = + qemu_try_blockalign(bs->file->bs, cur_nr_sectors * 512); + + qemu_iovec_to_buf(qiov, bytes_done, + cipher_data, cur_nr_sectors * 512); + + if (qcrypto_block_encrypt(crypto->block, + sector_num, + cipher_data, cur_nr_sectors * 512, + NULL) < 0) { + ret = -1; + goto cleanup; + } + + qemu_iovec_reset(&hd_qiov); + qemu_iovec_add(&hd_qiov, cipher_data, cur_nr_sectors * 512); + + qemu_co_mutex_unlock(&crypto->lock); + ret = bdrv_co_writev(bs->file->bs, + payload_offset + sector_num, + cur_nr_sectors, &hd_qiov); + qemu_co_mutex_lock(&crypto->lock); + if (ret < 0) { + goto cleanup; + } + + remaining_sectors -= cur_nr_sectors; + sector_num += cur_nr_sectors; + bytes_done += cur_nr_sectors * 512; + } + + cleanup: + qemu_co_mutex_unlock(&crypto->lock); + + qemu_iovec_destroy(&hd_qiov); + qemu_vfree(cipher_data); + + return ret; +} + + +static int64_t block_crypto_getlength(BlockDriverState *bs) +{ + BlockCrypto *crypto = bs->opaque; + int64_t len = bdrv_getlength(bs->file->bs); + + ssize_t offset = qcrypto_block_get_payload_offset(crypto->block); + + len -= (offset * 512); + + return len; +} + +#define BLOCK_CRYPTO_DRIVER(name, format) \ + static int block_crypto_probe_ ## name(const uint8_t *buf, \ + int buf_size, \ + const char *filename) { \ + return block_crypto_probe_generic(format, \ + buf, buf_size, filename); \ + } \ + \ + static int block_crypto_open_ ## name(BlockDriverState *bs, \ + QDict *options, \ + int flags, \ + Error **errp) \ + { \ + return block_crypto_open_generic(format, \ + &block_crypto_runtime_opts_ ## name, \ + bs, options, flags, errp); \ + } \ + \ + static int block_crypto_create_ ## name(const char *filename, \ + QemuOpts *opts, \ + Error **errp) \ + { \ + return block_crypto_create_generic(format, \ + filename, opts, errp); \ + } \ + \ + BlockDriver bdrv_crypto_ ## name = { \ + .format_name = #name, \ + .instance_size = sizeof(BlockCrypto), \ + .bdrv_probe = block_crypto_probe_ ## name, \ + .bdrv_open = block_crypto_open_ ## name, \ + .bdrv_close = block_crypto_close, \ + .bdrv_create = block_crypto_create_ ## name, \ + .create_opts = &block_crypto_create_opts_ ## name, \ + \ + .bdrv_co_readv = block_crypto_co_readv, \ + .bdrv_co_writev = block_crypto_co_writev, \ + .bdrv_getlength = block_crypto_getlength, \ + } + +BLOCK_CRYPTO_DRIVER(luks, Q_CRYPTO_BLOCK_FORMAT_LUKS); + +static void block_crypto_init(void) +{ + bdrv_register(&bdrv_crypto_luks); +} + +block_init(block_crypto_init); diff --git a/qapi/block-core.json b/qapi/block-core.json index 0a915ed..a194658 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1546,7 +1546,7 @@ { 'enum': 'BlockdevDriver', 'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop', 'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device', - 'http', 'https', 'null-aio', 'null-co', 'parallels', + 'http', 'https', 'luks', 'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] } @@ -1664,6 +1664,21 @@ 'data': { 'file': 'BlockdevRef' } } ## +# @BlockdevOptionsLUKS +# +# Driver specific block device options for LUKS. +# +# @key-secret: #optional the ID of a QCryptoSecret object providing +# the decryption key (since 2.6) +# +# Since: 2.6 +## +{ 'struct': 'BlockdevOptionsLUKS', + 'base': 'BlockdevOptionsGenericFormat', + 'data': { '*key-secret': 'str' } } + + +## # @BlockdevOptionsGenericCOWFormat # # Driver specific block device options for image format that have no option @@ -2000,6 +2015,7 @@ 'http': 'BlockdevOptionsFile', 'https': 'BlockdevOptionsFile', # TODO iscsi: Wait for structured options + 'luks': 'BlockdevOptionsLUKS', # TODO nbd: Should take InetSocketAddress for 'host'? # TODO nfs: Wait for structured options 'null-aio': 'BlockdevOptionsNull',
Add a block driver that is capable of supporting any full disk encryption format. This utilizes the previously added block encryption code, and at this time supports the LUKS format. The driver code is capable of supporting any format supported by the QCryptoBlock module, so it registers one block driver for each format. At this time, the "luks" driver is registered. New LUKS compatible volume can be formatted using qemu-img $ qemu-img create --object secret,data=123456,id=sec0 \ -f luks -o key-secret=sec0,cipher-alg=aes-256,\ cipher-mode=cbc,ivgen-alg=plain64,hash-alg=sha256 \ demo.luks 10G And query its size $ qemu-img info --object secret,data=123456,id=sec0 --source demo.luks,driver=luks,key-secret=sec0 image: json:{"key-secret": "sec0", "driver": "luks", "file": {"driver": "file", "filename": "demo.luks"}} file format: luks virtual size: 10.0G (10737416192 bytes) disk size: 132K All volumes created by this new 'luks' driver should be capable of being opened by the kernel dm-crypt driver. With this initial impl, not all volumes created with dm-crypt can be opened by the QEMU 'luks' driver. This is due to lack of support for certain algorithms, in particular the 'xts' cipher mode. These limitations will be addressed in a later series of patches, with the intent that QEMU should be able to open anything that dm-crypt LUKS supports. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- block/Makefile.objs | 2 + block/crypto.c | 541 +++++++++++++++++++++++++++++++++++++++++++++++++++ qapi/block-core.json | 18 +- 3 files changed, 560 insertions(+), 1 deletion(-) create mode 100644 block/crypto.c