[Qemu-devel] [PATCH 16/17] block: remove all encryption handling APIs

["Daniel P. Berrange" <berrange@redhat.com>]

Now that all encryption keys must be provided upfront via
the QCryptoSecret API and associated block driver properties
there is no need for any explicit encryption handling APIs
in the block layer. Encryption can be handled transparently
within the block driver. We only retain an API for querying
whether an image is encrypted or not, since that is a
potentially useful piece of metadata to report to the user.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 block.c                    | 81 ++--------------------------------------------
 block/qapi.c               |  2 +-
 block/qcow.c               | 19 -----------
 block/qcow2.c              | 19 -----------
 blockdev.c                 | 69 +++++++++------------------------------
 include/block/block.h      |  4 ---
 tests/qemu-iotests/087.out |  4 +--
 7 files changed, 21 insertions(+), 177 deletions(-)

diff --git a/block.c b/block.c
index 09f2a75..bb8cb96 100644
--- a/block.c
+++ b/block.c
@@ -1542,17 +1542,8 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
         goto close_and_fail;
     }
 
-    if (!bdrv_key_required(bs)) {
-        if (bs->blk) {
-            blk_dev_change_media_cb(bs->blk, true);
-        }
-    } else if (!runstate_check(RUN_STATE_PRELAUNCH)
-               && !runstate_check(RUN_STATE_INMIGRATE)
-               && !runstate_check(RUN_STATE_PAUSED)) { /* HACK */
-        error_setg(errp,
-                   "Guest must be stopped for opening of encrypted image");
-        ret = -EBUSY;
-        goto close_and_fail;
+    if (bs->blk) {
+        blk_dev_change_media_cb(bs->blk, true);
     }
 
     QDECREF(options);
@@ -2608,74 +2599,6 @@ int bdrv_is_encrypted(BlockDriverState *bs)
     return bs->encrypted;
 }
 
-int bdrv_key_required(BlockDriverState *bs)
-{
-    BdrvChild *backing = bs->backing;
-
-    if (backing && backing->bs->encrypted && !backing->bs->valid_key) {
-        return 1;
-    }
-    return (bs->encrypted && !bs->valid_key);
-}
-
-int bdrv_set_key(BlockDriverState *bs, const char *key)
-{
-    int ret;
-    if (bs->backing && bs->backing->bs->encrypted) {
-        ret = bdrv_set_key(bs->backing->bs, key);
-        if (ret < 0)
-            return ret;
-        if (!bs->encrypted)
-            return 0;
-    }
-    if (!bs->encrypted) {
-        return -EINVAL;
-    } else if (!bs->drv || !bs->drv->bdrv_set_key) {
-        return -ENOMEDIUM;
-    }
-    ret = bs->drv->bdrv_set_key(bs, key);
-    if (ret < 0) {
-        bs->valid_key = 0;
-    } else if (!bs->valid_key) {
-        bs->valid_key = 1;
-        if (bs->blk) {
-            /* call the change callback now, we skipped it on open */
-            blk_dev_change_media_cb(bs->blk, true);
-        }
-    }
-    return ret;
-}
-
-/*
- * Provide an encryption key for @bs.
- * If @key is non-null:
- *     If @bs is not encrypted, fail.
- *     Else if the key is invalid, fail.
- *     Else set @bs's key to @key, replacing the existing key, if any.
- * If @key is null:
- *     If @bs is encrypted and still lacks a key, fail.
- *     Else do nothing.
- * On failure, store an error object through @errp if non-null.
- */
-void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp)
-{
-    if (key) {
-        if (!bdrv_is_encrypted(bs)) {
-            error_setg(errp, "Node '%s' is not encrypted",
-                      bdrv_get_device_or_node_name(bs));
-        } else if (bdrv_set_key(bs, key) < 0) {
-            error_setg(errp, QERR_INVALID_PASSWORD);
-        }
-    } else {
-        if (bdrv_key_required(bs)) {
-            error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED,
-                      "'%s' (%s) is encrypted",
-                      bdrv_get_device_or_node_name(bs),
-                      bdrv_get_encrypted_filename(bs));
-        }
-    }
-}
-
 const char *bdrv_get_format_name(BlockDriverState *bs)
 {
     return bs->drv ? bs->drv->format_name : NULL;
diff --git a/block/qapi.c b/block/qapi.c
index 355ba32..a38258f 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -42,7 +42,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs, Error **errp)
     info->ro                     = bs->read_only;
     info->drv                    = g_strdup(bs->drv->format_name);
     info->encrypted              = bs->encrypted;
-    info->encryption_key_missing = bdrv_key_required(bs);
+    info->encryption_key_missing = false;
 
     info->cache = g_new(BlockdevCacheInfo, 1);
     *info->cache = (BlockdevCacheInfo) {
diff --git a/block/qcow.c b/block/qcow.c
index ff80ef5..ccf6de1 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -222,11 +222,6 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
         ret = -EINVAL;
         goto fail;
     }
-    if (!qcrypto_cipher_supports(QCRYPTO_CIPHER_ALG_AES_128)) {
-        error_setg(errp, "AES cipher not available");
-        ret = -EINVAL;
-        goto fail;
-    }
     s->crypt_method_header = header.crypt_method;
     if (s->crypt_method_header) {
         bs->encrypted = 1;
@@ -335,19 +330,6 @@ static int qcow_reopen_prepare(BDRVReopenState *state,
     return 0;
 }
 
-static int qcow_set_key(BlockDriverState *bs, const char *key)
-{
-    BDRVQcowState *s = bs->opaque;
-
-    assert(bs->encrypted);
-    qcrypto_cipher_free(s->cipher);
-    s->cipher = qcow_get_cipher_from_key(key, NULL);
-    if (!s->cipher) {
-        return -1;
-    }
-    return 0;
-}
-
 /* The crypt function is compatible with the linux cryptoloop
    algorithm for < 4 GB images. NOTE: out_buf == in_buf is
    supported */
@@ -1078,7 +1060,6 @@ static BlockDriver bdrv_qcow = {
     .bdrv_co_writev         = qcow_co_writev,
     .bdrv_co_get_block_status   = qcow_co_get_block_status,
 
-    .bdrv_set_key           = qcow_set_key,
     .bdrv_make_empty        = qcow_make_empty,
     .bdrv_write_compressed  = qcow_write_compressed,
     .bdrv_get_info          = qcow_get_info,
diff --git a/block/qcow2.c b/block/qcow2.c
index 6ca2b25..c93df92 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1026,11 +1026,6 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         ret = -EINVAL;
         goto fail;
     }
-    if (!qcrypto_cipher_supports(QCRYPTO_CIPHER_ALG_AES_128)) {
-        error_setg(errp, "AES cipher not available");
-        ret = -EINVAL;
-        goto fail;
-    }
     s->crypt_method_header = header.crypt_method;
     if (s->crypt_method_header) {
         bs->encrypted = 1;
@@ -1262,19 +1257,6 @@ static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp)
     bs->bl.write_zeroes_alignment = s->cluster_sectors;
 }
 
-static int qcow2_set_key(BlockDriverState *bs, const char *key)
-{
-    BDRVQcow2State *s = bs->opaque;
-
-    assert(bs->encrypted);
-    qcrypto_cipher_free(s->cipher);
-    s->cipher = qcow2_get_cipher_from_key(key, NULL);
-    if (!s->cipher) {
-        return -1;
-    }
-    return 0;
-}
-
 static int qcow2_reopen_prepare(BDRVReopenState *state,
                                 BlockReopenQueue *queue, Error **errp)
 {
@@ -3193,7 +3175,6 @@ BlockDriver bdrv_qcow2 = {
     .bdrv_create        = qcow2_create,
     .bdrv_has_zero_init = bdrv_has_zero_init_1,
     .bdrv_co_get_block_status = qcow2_co_get_block_status,
-    .bdrv_set_key       = qcow2_set_key,
 
     .bdrv_co_readv          = qcow2_co_readv,
     .bdrv_co_writev         = qcow2_co_writev,
diff --git a/blockdev.c b/blockdev.c
index 8141b6b..3e8d7ca 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -564,10 +564,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
         bdrv_set_io_limits(bs, &cfg);
     }
 
-    if (bdrv_key_required(bs)) {
-        autostart = 0;
-    }
-
 err_no_bs_opts:
     qemu_opts_del(opts);
     return blk;
@@ -1865,48 +1861,10 @@ void qmp_block_passwd(bool has_device, const char *device,
                       bool has_node_name, const char *node_name,
                       const char *password, Error **errp)
 {
-    Error *local_err = NULL;
-    BlockDriverState *bs;
-    AioContext *aio_context;
-
-    bs = bdrv_lookup_bs(has_device ? device : NULL,
-                        has_node_name ? node_name : NULL,
-                        &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
-
-    aio_context = bdrv_get_aio_context(bs);
-    aio_context_acquire(aio_context);
-
-    bdrv_add_key(bs, password, errp);
-
-    aio_context_release(aio_context);
+    error_setg_errno(errp, -ENOSYS,
+                     "Setting block passwords directly is no longer supported");
 }
 
-/* Assumes AioContext is held */
-static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
-                                    int bdrv_flags, const char *format,
-                                    const char *password, Error **errp)
-{
-    Error *local_err = NULL;
-    QDict *options = NULL;
-    int ret;
-
-    if (format) {
-        options = qdict_new();
-        qdict_put(options, "driver", qstring_from_str(format));
-    }
-
-    ret = bdrv_open(&bs, filename, NULL, options, bdrv_flags, &local_err);
-    if (ret < 0) {
-        error_propagate(errp, local_err);
-        return;
-    }
-
-    bdrv_add_key(bs, password, errp);
-}
 
 void qmp_change_blockdev(const char *device, const char *filename,
                          const char *format, Error **errp)
@@ -1916,6 +1874,8 @@ void qmp_change_blockdev(const char *device, const char *filename,
     AioContext *aio_context;
     int bdrv_flags;
     Error *err = NULL;
+    QDict *options = NULL;
+    int ret;
 
     blk = blk_by_name(device);
     if (!blk) {
@@ -1937,7 +1897,17 @@ void qmp_change_blockdev(const char *device, const char *filename,
     bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
     bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
 
-    qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, format, NULL, errp);
+
+    if (format) {
+        options = qdict_new();
+        qdict_put(options, "driver", qstring_from_str(format));
+    }
+
+    ret = bdrv_open(&bs, filename, NULL, options, bdrv_flags, &err);
+    if (ret < 0) {
+        error_propagate(errp, err);
+        return;
+    }
 
 out:
     aio_context_release(aio_context);
@@ -3030,7 +3000,6 @@ out:
 void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
 {
     QmpOutputVisitor *ov = qmp_output_visitor_new();
-    BlockBackend *blk;
     QObject *obj;
     QDict *qdict;
     Error *local_err = NULL;
@@ -3068,18 +3037,12 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
 
     qdict_flatten(qdict);
 
-    blk = blockdev_init(NULL, qdict, &local_err);
+    blockdev_init(NULL, qdict, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         goto fail;
     }
 
-    if (bdrv_key_required(blk_bs(blk))) {
-        blk_unref(blk);
-        error_setg(errp, "blockdev-add doesn't support encrypted devices");
-        goto fail;
-    }
-
 fail:
     qmp_output_visitor_cleanup(ov);
 }
diff --git a/include/block/block.h b/include/block/block.h
index ac776b2..b612146 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -414,10 +414,6 @@ bool bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base);
 BlockDriverState *bdrv_next_node(BlockDriverState *bs);
 BlockDriverState *bdrv_next(BlockDriverState *bs);
 int bdrv_is_encrypted(BlockDriverState *bs);
-int bdrv_key_required(BlockDriverState *bs);
-int bdrv_set_key(BlockDriverState *bs, const char *key);
-void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp);
-int bdrv_query_missing_keys(void);
 void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
                          void *opaque);
 const char *bdrv_get_node_name(const BlockDriverState *bs);
diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
index 8299f81..50ce50d 100644
--- a/tests/qemu-iotests/087.out
+++ b/tests/qemu-iotests/087.out
@@ -52,7 +52,7 @@ QMP_VERSION
 Encrypted images are deprecated
 Support for them will be removed in a future release.
 You can use 'qemu-img convert' to convert your image to an unencrypted one.
-{"error": {"class": "GenericError", "desc": "blockdev-add doesn't support encrypted devices"}}
+{"return": {}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
 
@@ -63,7 +63,7 @@ QMP_VERSION
 Encrypted images are deprecated
 Support for them will be removed in a future release.
 You can use 'qemu-img convert' to convert your image to an unencrypted one.
-{"error": {"class": "GenericError", "desc": "Guest must be stopped for opening of encrypted image"}}
+{"return": {}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
 
-- 
2.4.3