Re: [Qemu-block] [PATCH v5 6/7] block: add generic full disk encryption driver

[Kevin Wolf]

Am 17.03.2016 um 18:51 hat Daniel P. Berrange geschrieben:
> 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. This patch only registers the "luks" driver
> since the "qcow" driver is there only for back-compatibility
> with existing qcow built-in encryption.
> 
> New LUKS compatible volumes can be formatted using qemu-img
> with defaults for all settings.
> 
> $ qemu-img create --object secret,data=123456,id=sec0 \
>       -f luks -o key-secret=sec0 demo.luks 10G
> 
> Alternatively the cryptographic settings can be explicitly
> set
> 
> $ 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 demo.img
> image: demo.img
> file format: luks
> virtual size: 10G (10737418240 bytes)
> disk size: 132K
> encrypted: yes
> 
> Note that it was not necessary to provide the password
> when querying info for the volume. The password is only
> required when performing I/O on the volume
> 
> All volumes created by this new 'luks' driver should be
> capable of being opened by the kernel dm-crypt driver.
> 
> The only algorithms listed in the LUKS spec that are
> not currently supported by this impl are sha512 and
> ripemd160 hashes and cast6 cipher.
> 
> A new I/O test is added which is used to validate the
> interoperability of the QEMU implementation of LUKS,
> with the dm-crypt/cryptsetup implementation. Due to
> the need to run cryptsetup as root, this test requires
> that the user have password-less sudo configured.
> 
> The test is set to only run against the 'luks' format
> so needs to be manually invoked with
> 
>   cd tests/qemu-iotests
>   ./check -luks 149
> 
> Reviewed-by: Eric Blake <address@hidden>
> Signed-off-by: Daniel P. Berrange <address@hidden>

This is a huge patch. I'm not sure if it wouldn't be better to split off
the test. I also think that having some test cases that don't require
root privileges would be helpful, so maybe the test can be split in two
halves as well, one requiring passwordless sudo and the other without
special requirements.

>  block/Makefile.objs        |    2 +
>  block/crypto.c             |  587 ++++++++++++
>  qapi/block-core.json       |   22 +-
>  tests/qemu-iotests/149     |  521 ++++++++++
>  tests/qemu-iotests/149.out | 2252 
> ++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/common  |    7 +
>  tests/qemu-iotests/group   |    1 +
>  7 files changed, 3390 insertions(+), 2 deletions(-)
>  create mode 100644 block/crypto.c
>  create mode 100755 tests/qemu-iotests/149
>  create mode 100644 tests/qemu-iotests/149.out
> 
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index cdd8655..3426a15 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -23,6 +23,8 @@ block-obj-$(CONFIG_LIBSSH2) += ssh.o
>  block-obj-y += accounting.o dirty-bitmap.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..5efca6c
> --- /dev/null
> +++ b/block/crypto.c
> @@ -0,0 +1,587 @@
> +/*
> + * 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 "qemu/osdep.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)) {

This is short enough for a single line.

> +        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;
> +}
> +
> +
> +struct BlockCryptoCreateData {
> +    const char *filename;
> +    QemuOpts *opts;
> +    BlockDriverState *bs;
> +    uint64_t size;
> +};
> +
> +
> +static ssize_t block_crypto_write_func(QCryptoBlock *block,
> +                                       size_t offset,
> +                                       const uint8_t *buf,
> +                                       size_t buflen,
> +                                       Error **errp,
> +                                       void *opaque)
> +{
> +    struct BlockCryptoCreateData *data = opaque;
> +    ssize_t ret;
> +
> +    ret = bdrv_pwrite(data->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)
> +{
> +    struct BlockCryptoCreateData *data = opaque;
> +    int ret;
> +
> +    /* User provided size should reflect amount of space made
> +     * available to the guest, so we must take account of that
> +     * which will be used by the crypto header
> +     */
> +    data->size += headerlen;
> +
> +    qemu_opt_set_number(data->opts, BLOCK_OPT_SIZE, data->size, 
> &error_abort);
> +    ret = bdrv_create_file(data->filename, data->opts, errp);
> +    if (ret < 0) {
> +        return -1;
> +    }
> +
> +    ret = bdrv_open(&data->bs, data->filename, NULL, NULL,
> +                    BDRV_O_RDWR | BDRV_O_PROTOCOL, errp);

We should probably use blk_open() here like the other image format
drivers do now, and preferably use blk_*() functions to access the
image.

You will also want to specify BDRV_O_CACHE_WB (while it exists, I'm
going to remove it soon).

> +    if (ret < 0) {
> +        return -1;
> +    }
> +
> +    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 = NULL;
> +    Error *local_err = NULL;
> +
> +    ret = g_new0(QCryptoBlockOpenOptions, 1);
> +    ret->format = format;
> +
> +    ov = opts_visitor_new(opts);
> +
> +    visit_start_struct(opts_get_visitor(ov),
> +                       "luks", NULL, 0, &local_err);

As this refers to "luks" specifically, shouldn't it be inside the switch
below?

> +    if (local_err) {
> +        goto out;
> +    }
> +
> +    switch (format) {
> +    case Q_CRYPTO_BLOCK_FORMAT_LUKS:
> +        visit_type_QCryptoBlockOptionsLUKS_members(
> +            opts_get_visitor(ov), &ret->u.luks, &local_err);
> +        break;
> +
> +    default:
> +        error_setg(&local_err, "Unsupported block format %d", format);
> +        break;
> +    }
> +    error_propagate(errp, local_err);
> +    local_err = NULL;
> +
> +    visit_end_struct(opts_get_visitor(ov), &local_err);
> +
> + out:
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        qapi_free_QCryptoBlockOpenOptions(ret);
> +        ret = NULL;
> +    }
> +    opts_visitor_cleanup(ov);
> +    return ret;
> +}
> +
> +
> +static QCryptoBlockCreateOptions *
> +block_crypto_create_opts_init(QCryptoBlockFormat format,
> +                              QemuOpts *opts,
> +                              Error **errp)
> +{
> +    OptsVisitor *ov;
> +    QCryptoBlockCreateOptions *ret = NULL;
> +    Error *local_err = NULL;
> +
> +    ret = g_new0(QCryptoBlockCreateOptions, 1);
> +    ret->format = format;
> +
> +    ov = opts_visitor_new(opts);
> +
> +    visit_start_struct(opts_get_visitor(ov),
> +                       "luks", NULL, 0, &local_err);

Same here.

> +    if (local_err) {
> +        goto out;
> +    }
> +
> +    switch (format) {
> +    case Q_CRYPTO_BLOCK_FORMAT_LUKS:
> +        visit_type_QCryptoBlockCreateOptionsLUKS_members(
> +            opts_get_visitor(ov), &ret->u.luks, &local_err);
> +        break;
> +
> +    default:
> +        error_setg(&local_err, "Unsupported block format %d", format);
> +        break;
> +    }
> +    error_propagate(errp, local_err);
> +    local_err = NULL;
> +
> +    visit_end_struct(opts_get_visitor(ov), &local_err);
> +
> + out:
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        qapi_free_QCryptoBlockCreateOptions(ret);
> +        ret = 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;
> +    QCryptoBlock *crypto = NULL;
> +    struct BlockCryptoCreateData data;
> +
> +    data.size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                         BDRV_SECTOR_SIZE);
> +    data.opts = opts;
> +    data.filename = filename;
> +    data.bs = NULL;
> +
> +    create_opts = block_crypto_create_opts_init(format, opts, errp);
> +    if (!create_opts) {
> +        return -1;
> +    }
> +
> +    crypto = qcrypto_block_create(create_opts,
> +                                  block_crypto_init_func,
> +                                  block_crypto_write_func,
> +                                  &data,
> +                                  errp);
> +
> +    if (!crypto) {
> +        ret = -EIO;
> +        goto cleanup;
> +    }
> +
> +    ret = 0;
> + cleanup:
> +    qcrypto_block_free(crypto);
> +    bdrv_unref(data.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) / 512;
> +
> +    qemu_iovec_init(&hd_qiov, qiov->niov);
> +
> +    qemu_co_mutex_lock(&crypto->lock);
> +
> +    /* Bounce buffer so we have a linear mem region for
> +     * entire sector. XXX optimize so we avoid bounce
> +     * buffer in case that qiov->niov == 1
> +     */
> +    cipher_data =
> +        qemu_try_blockalign(bs->file->bs, BLOCK_CRYPTO_MAX_SECTORS * 512);

As long as we create an individual buffer for each request, shouldn't
MIN(BLOCK_CRYPTO_MAX_SECTORS * 512, qiov->size) be enough?

> +    if (cipher_data == NULL) {
> +        ret = -ENOMEM;
> +        goto cleanup;
> +    }
> +
> +    while (remaining_sectors) {
> +        cur_nr_sectors = remaining_sectors;
> +
> +        if (cur_nr_sectors > BLOCK_CRYPTO_MAX_SECTORS) {
> +            cur_nr_sectors = BLOCK_CRYPTO_MAX_SECTORS;
> +        }
> +
> +        qemu_iovec_reset(&hd_qiov);
> +        qemu_iovec_add(&hd_qiov, cipher_data, cur_nr_sectors * 512);
> +
> +        qemu_co_mutex_unlock(&crypto->lock);

Between qemu_co_mutex_lock() and here, there is no yield point...

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

Need a real -errno code here.

> +            goto cleanup;
> +        }

...nor is there one between here and the end of the function.

So what does this CoMutex protect? If qcrypto_block_decrypt() needs this
for some reason (it doesn't seem to be touching anything that isn't per
request, but maybe I'm missing something), would it be clearer to put
the locking only around that call?

> +
> +        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) / 512;
> +
> +    qemu_iovec_init(&hd_qiov, qiov->niov);
> +
> +    qemu_co_mutex_lock(&crypto->lock);

Same locking question as above.

> +    /* Bounce buffer so we have a linear mem region for
> +     * entire sector. XXX optimize so we avoid bounce
> +     * buffer in case that qiov->niov == 1
> +     */
> +    cipher_data =
> +        qemu_try_blockalign(bs->file->bs, BLOCK_CRYPTO_MAX_SECTORS * 512);

Same as above as well.

> +    if (cipher_data == NULL) {
> +        ret = -ENOMEM;
> +        goto cleanup;
> +    }
> +
> +    while (remaining_sectors) {
> +        cur_nr_sectors = remaining_sectors;
> +
> +        if (cur_nr_sectors > BLOCK_CRYPTO_MAX_SECTORS) {
> +            cur_nr_sectors = BLOCK_CRYPTO_MAX_SECTORS;
> +        }
> +
> +        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;
> +
> +    return len;
> +}
> +
> +
> +static int block_crypto_probe_luks(const uint8_t *buf,
> +                                   int buf_size,
> +                                   const char *filename) {
> +    return block_crypto_probe_generic(Q_CRYPTO_BLOCK_FORMAT_LUKS,
> +                                      buf, buf_size, filename);
> +}
> +
> +static int block_crypto_open_luks(BlockDriverState *bs,
> +                                  QDict *options,
> +                                  int flags,
> +                                  Error **errp)
> +{
> +    return block_crypto_open_generic(Q_CRYPTO_BLOCK_FORMAT_LUKS,
> +                                     &block_crypto_runtime_opts_luks,
> +                                     bs, options, flags, errp);
> +}
> +
> +static int block_crypto_create_luks(const char *filename,
> +                                    QemuOpts *opts,
> +                                    Error **errp)
> +{
> +    return block_crypto_create_generic(Q_CRYPTO_BLOCK_FORMAT_LUKS,
> +                                       filename, opts, errp);
> +}
> +
> +BlockDriver bdrv_crypto_luks = {
> +    .format_name        = "luks",
> +    .instance_size      = sizeof(BlockCrypto),
> +    .bdrv_probe         = block_crypto_probe_luks,
> +    .bdrv_open          = block_crypto_open_luks,
> +    .bdrv_close         = block_crypto_close,
> +    .bdrv_create        = block_crypto_create_luks,
> +    .create_opts        = &block_crypto_create_opts_luks,
> +
> +    .bdrv_co_readv      = block_crypto_co_readv,
> +    .bdrv_co_writev     = block_crypto_co_writev,
> +    .bdrv_getlength     = block_crypto_getlength,
> +};

Rather minimalistic, but we can always add the missing functions later.

> +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 9bf1b22..f2103b6 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -242,11 +242,12 @@
>  # @drv: the name of the block format used to open the backing device. As of
>  #       0.14.0 this can be: 'blkdebug', 'bochs', 'cloop', 'cow', 'dmg',
>  #       'file', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
> -#       'http', 'https', 'nbd', 'parallels', 'qcow',
> +#       'http', 'https', 'luks', 'nbd', 'parallels', 'qcow',
>  #       'qcow2', 'raw', 'tftp', 'vdi', 'vmdk', 'vpc', 'vvfat'
>  #       2.2: 'archipelago' added, 'cow' dropped
>  #       2.3: 'host_floppy' deprecated
>  #       2.5: 'host_floppy' dropped
> +#       2.6: 'luks' added
>  #
>  # @backing_file: #optional the name of the backing file (for copy-on-write)
>  #
> @@ -1639,7 +1640,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' ] }
>  
> @@ -1757,6 +1758,22 @@
>    '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). Mandatory except when
> +#              doing a metadata-only probe of the image.
> +#
> +# Since: 2.6
> +##
> +{ 'struct': 'BlockdevOptionsLUKS',
> +  'base': 'BlockdevOptionsGenericFormat',
> +  'data': { '*key-secret': 'str' } }
> +
> +
> +##
>  # @BlockdevOptionsGenericCOWFormat
>  #
>  # Driver specific block device options for image format that have no option
> @@ -2093,6 +2110,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',

Kevin