On 2015/5/21 18:56, Daniel P. Berrange wrote:
> Switch the qcow/qcow2 block driver over to use the generic cipher
> API, this allows it to use the pluggable AES implementations,
> instead of being hardcoded to use QEMU's built-in impl.
>
> Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> ---
> block/qcow.c | 100
> ++++++++++++++++++++++++++++++++++++--------------
> block/qcow2-cluster.c | 46 ++++++++++++++++++-----
> block/qcow2.c | 94 ++++++++++++++++++++++++-----------------------
> block/qcow2.h | 13 +++----
> 4 files changed, 162 insertions(+), 91 deletions(-)
>
> diff --git a/block/qcow.c b/block/qcow.c
> index 50dbcee..7338d1d 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -25,7 +25,7 @@
> #include "block/block_int.h"
> #include "qemu/module.h"
> #include <zlib.h>
> -#include "crypto/aes.h"
> +#include "crypto/cipher.h"
> #include "migration/migration.h"
>
> /**************************************************************/
> @@ -71,10 +71,8 @@ typedef struct BDRVQcowState {
> uint8_t *cluster_cache;
> uint8_t *cluster_data;
> uint64_t cluster_cache_offset;
> - uint32_t crypt_method; /* current crypt method, 0 if no key yet */
> + QCryptoCipher *cipher; /* NULL if no key yet */
> uint32_t crypt_method_header;
> - AES_KEY aes_encrypt_key;
> - AES_KEY aes_decrypt_key;
> CoMutex lock;
> Error *migration_blocker;
> } BDRVQcowState;
> @@ -153,6 +151,11 @@ static int qcow_open(BlockDriverState *bs, QDict
> *options, int flags,
> ret = -EINVAL;
> goto fail;
> }
> + if (!qcrypto_cipher_supports(QCRYPTO_CIPHER_ALG_AES)) {
> + 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;
> @@ -259,6 +262,7 @@ static int qcow_set_key(BlockDriverState *bs, const char
> *key)
> BDRVQcowState *s = bs->opaque;
> uint8_t keybuf[16];
> int len, i;
> + Error *err;
>
> memset(keybuf, 0, 16);
> len = strlen(key);
> @@ -269,38 +273,66 @@ static int qcow_set_key(BlockDriverState *bs, const
> char *key)
> for(i = 0;i < len;i++) {
> keybuf[i] = key[i];
> }
> - s->crypt_method = s->crypt_method_header;
>
> - if (AES_set_encrypt_key(keybuf, 128, &s->aes_encrypt_key) != 0)
> - return -1;
> - if (AES_set_decrypt_key(keybuf, 128, &s->aes_decrypt_key) != 0)
> + if (s->cipher) {
This above check is superfluous.
> + qcrypto_cipher_free(s->cipher);
> + }
> + s->cipher = qcrypto_cipher_new(
> + QCRYPTO_CIPHER_ALG_AES,
> + QCRYPTO_CIPHER_MODE_CBC,
> + keybuf, G_N_ELEMENTS(keybuf),
> + &err);
> +
> + if (!s->cipher) {
> + error_free(err);
Maybe we should report the error message before free it.
It's the same for below error handling.
> 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 */
> -static void encrypt_sectors(BDRVQcowState *s, int64_t sector_num,
> - uint8_t *out_buf, const uint8_t *in_buf,
> - int nb_sectors, int enc,
> - const AES_KEY *key)
> +static int encrypt_sectors(BDRVQcowState *s, int64_t sector_num,
> + uint8_t *out_buf, const uint8_t *in_buf,
> + int nb_sectors, bool enc, Error **errp)
> {
> union {
> uint64_t ll[2];
> uint8_t b[16];
> } ivec;
> int i;
> + int ret;
>
> for(i = 0; i < nb_sectors; i++) {
> ivec.ll[0] = cpu_to_le64(sector_num);
> ivec.ll[1] = 0;
> - AES_cbc_encrypt(in_buf, out_buf, 512, key,
> - ivec.b, enc);
> + if (qcrypto_cipher_setiv(s->cipher,
> + ivec.b, G_N_ELEMENTS(ivec.b),
> + errp) < 0) {
> + return -1;
> + }
> + if (enc) {
> + ret = qcrypto_cipher_encrypt(s->cipher,
> + in_buf,
> + out_buf,
> + 512,
> + errp);
> + } else {
> + ret = qcrypto_cipher_decrypt(s->cipher,
> + in_buf,
> + out_buf,
> + 512,
> + errp);
> + }
> + if (ret < 0) {
> + return -1;
> + }
> sector_num++;
> in_buf += 512;
> out_buf += 512;
> }
> + return 0;
> }
>
> /* 'allocate' is:
> @@ -411,17 +443,22 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
> bdrv_truncate(bs->file, cluster_offset + s->cluster_size);
> /* if encrypted, we must initialize the cluster
> content which won't be written */
> - if (s->crypt_method &&
> + if (s->cipher &&
> (n_end - n_start) < s->cluster_sectors) {
> uint64_t start_sect;
> start_sect = (offset & ~(s->cluster_size - 1)) >> 9;
> memset(s->cluster_data + 512, 0x00, 512);
> for(i = 0; i < s->cluster_sectors; i++) {
> if (i < n_start || i >= n_end) {
> - encrypt_sectors(s, start_sect + i,
> - s->cluster_data,
> - s->cluster_data + 512, 1, 1,
> - &s->aes_encrypt_key);
> + Error *err = NULL;
> + if (encrypt_sectors(s, start_sect + i,
> + s->cluster_data,
> + s->cluster_data + 512, 1,
> + true, &err) < 0) {
> + error_free(err);
> + errno = EIO;
> + return -1;
> + }
> if (bdrv_pwrite(bs->file, cluster_offset + i *
> 512,
> s->cluster_data, 512) != 512)
> return -1;
> @@ -461,7 +498,7 @@ static int64_t coroutine_fn
> qcow_co_get_block_status(BlockDriverState *bs,
> if (!cluster_offset) {
> return 0;
> }
> - if ((cluster_offset & QCOW_OFLAG_COMPRESSED) || s->crypt_method) {
> + if ((cluster_offset & QCOW_OFLAG_COMPRESSED) || s->cipher) {
> return BDRV_BLOCK_DATA;
> }
> cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS);
> @@ -528,6 +565,7 @@ static coroutine_fn int qcow_co_readv(BlockDriverState
> *bs, int64_t sector_num,
> QEMUIOVector hd_qiov;
> uint8_t *buf;
> void *orig_buf;
> + Error *err = NULL;
>
> if (qiov->niov > 1) {
> buf = orig_buf = qemu_try_blockalign(bs, qiov->size);
> @@ -590,10 +628,11 @@ static coroutine_fn int qcow_co_readv(BlockDriverState
> *bs, int64_t sector_num,
> if (ret < 0) {
> break;
> }
> - if (s->crypt_method) {
> - encrypt_sectors(s, sector_num, buf, buf,
> - n, 0,
> - &s->aes_decrypt_key);
> + if (s->cipher) {
> + if (encrypt_sectors(s, sector_num, buf, buf,
> + n, false, &err) < 0) {
> + goto fail;
> + }
> }
> }
> ret = 0;
> @@ -614,6 +653,7 @@ done:
> return ret;
>
> fail:
> + error_free(err);
> ret = -EIO;
> goto done;
> }
> @@ -661,12 +701,17 @@ static coroutine_fn int qcow_co_writev(BlockDriverState
> *bs, int64_t sector_num,
> ret = -EIO;
> break;
> }
> - if (s->crypt_method) {
> + if (s->cipher) {
> + Error *err = NULL;
> if (!cluster_data) {
> cluster_data = g_malloc0(s->cluster_size);
> }
> - encrypt_sectors(s, sector_num, cluster_data, buf,
> - n, 1, &s->aes_encrypt_key);
> + if (encrypt_sectors(s, sector_num, cluster_data, buf,
> + n, true, &err) < 0) {
> + error_free(err);
> + ret = -EIO;
> + break;
> + }
> src_buf = cluster_data;
> } else {
> src_buf = buf;
> @@ -703,6 +748,7 @@ static void qcow_close(BlockDriverState *bs)
> {
> BDRVQcowState *s = bs->opaque;
>
> + qcrypto_cipher_free(s->cipher);
> g_free(s->l1_table);
> qemu_vfree(s->l2_cache);
> g_free(s->cluster_cache);
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index ed2b44d..afc2d90 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -342,26 +342,47 @@ static int count_contiguous_free_clusters(uint64_t
> nb_clusters, uint64_t *l2_tab
> /* The crypt function is compatible with the linux cryptoloop
> algorithm for < 4 GB images. NOTE: out_buf == in_buf is
> supported */
> -void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num,
> - uint8_t *out_buf, const uint8_t *in_buf,
> - int nb_sectors, int enc,
> - const AES_KEY *key)
> +int qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num,
> + uint8_t *out_buf, const uint8_t *in_buf,
> + int nb_sectors, bool enc,
> + Error **errp)
> {
> union {
> uint64_t ll[2];
> uint8_t b[16];
> } ivec;
> int i;
> + int ret;
>
> for(i = 0; i < nb_sectors; i++) {
> ivec.ll[0] = cpu_to_le64(sector_num);
> ivec.ll[1] = 0;
> - AES_cbc_encrypt(in_buf, out_buf, 512, key,
> - ivec.b, enc);
> + if (qcrypto_cipher_setiv(s->cipher,
> + ivec.b, G_N_ELEMENTS(ivec.b),
> + errp) < 0) {
> + return -1;
> + }
> + if (enc) {
> + ret = qcrypto_cipher_encrypt(s->cipher,
> + in_buf,
> + out_buf,
> + 512,
> + errp);
> + } else {
> + ret = qcrypto_cipher_decrypt(s->cipher,
> + in_buf,
> + out_buf,
> + 512,
> + errp);
> + }
> + if (ret < 0) {
> + return -1;
> + }
> sector_num++;
> in_buf += 512;
> out_buf += 512;
> }
> + return 0;
> }
>
> static int coroutine_fn copy_sectors(BlockDriverState *bs,
> @@ -403,10 +424,15 @@ static int coroutine_fn copy_sectors(BlockDriverState
> *bs,
> goto out;
> }
>
> - if (s->crypt_method) {
> - qcow2_encrypt_sectors(s, start_sect + n_start,
> - iov.iov_base, iov.iov_base, n, 1,
> - &s->aes_encrypt_key);
> + if (s->cipher) {
> + Error *err = NULL;
> + if (qcow2_encrypt_sectors(s, start_sect + n_start,
> + iov.iov_base, iov.iov_base, n,
> + true, &err) < 0) {
> + ret = -EIO;
> + error_free(err);
> + goto out;
> + }
> }
>
> ret = qcow2_pre_write_overlap_check(bs, 0,
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 83e5ec9..2d1100d 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -694,6 +694,11 @@ static int qcow2_open(BlockDriverState *bs, QDict
> *options, int flags,
> ret = -EINVAL;
> goto fail;
> }
> + if (!qcrypto_cipher_supports(QCRYPTO_CIPHER_ALG_AES)) {
> + 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;
> @@ -1026,6 +1031,7 @@ static int qcow2_set_key(BlockDriverState *bs, const
> char *key)
> BDRVQcowState *s = bs->opaque;
> uint8_t keybuf[16];
> int len, i;
> + Error *err = NULL;
>
> memset(keybuf, 0, 16);
> len = strlen(key);
> @@ -1036,30 +1042,20 @@ static int qcow2_set_key(BlockDriverState *bs, const
> char *key)
> for(i = 0;i < len;i++) {
> keybuf[i] = key[i];
> }
> - s->crypt_method = s->crypt_method_header;
>
> - if (AES_set_encrypt_key(keybuf, 128, &s->aes_encrypt_key) != 0)
> - return -1;
> - if (AES_set_decrypt_key(keybuf, 128, &s->aes_decrypt_key) != 0)
> + if (s->cipher) {
> + qcrypto_cipher_free(s->cipher);
> + }
> + s->cipher = qcrypto_cipher_new(
> + QCRYPTO_CIPHER_ALG_AES,
> + QCRYPTO_CIPHER_MODE_CBC,
> + keybuf, G_N_ELEMENTS(keybuf),
> + &err);
> +
> + if (!s->cipher) {
> + error_free(err);
> return -1;
> -#if 0
> - /* test */
> - {
> - uint8_t in[16];
> - uint8_t out[16];
> - uint8_t tmp[16];
> - for(i=0;i<16;i++)
> - in[i] = i;
> - AES_encrypt(in, tmp, &s->aes_encrypt_key);
> - AES_decrypt(tmp, out, &s->aes_decrypt_key);
> - for(i = 0; i < 16; i++)
> - printf(" %02x", tmp[i]);
> - printf("\n");
> - for(i = 0; i < 16; i++)
> - printf(" %02x", out[i]);
> - printf("\n");
> }
> -#endif
> return 0;
> }
>
> @@ -1102,7 +1098,7 @@ static int64_t coroutine_fn
> qcow2_co_get_block_status(BlockDriverState *bs,
> }
>
> if (cluster_offset != 0 && ret != QCOW2_CLUSTER_COMPRESSED &&
> - !s->crypt_method) {
> + !s->cipher) {
> index_in_cluster = sector_num & (s->cluster_sectors - 1);
> cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS);
> status |= BDRV_BLOCK_OFFSET_VALID | cluster_offset;
> @@ -1152,7 +1148,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState
> *bs, int64_t sector_num,
>
> /* prepare next request */
> cur_nr_sectors = remaining_sectors;
> - if (s->crypt_method) {
> + if (s->cipher) {
> cur_nr_sectors = MIN(cur_nr_sectors,
> QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors);
> }
> @@ -1223,7 +1219,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState
> *bs, int64_t sector_num,
> goto fail;
> }
>
> - if (s->crypt_method) {
> + if (s->cipher) {
> /*
> * For encrypted images, read everything into a temporary
> * contiguous buffer on which the AES functions can work.
> @@ -1254,9 +1250,15 @@ static coroutine_fn int
> qcow2_co_readv(BlockDriverState *bs, int64_t sector_num,
> if (ret < 0) {
> goto fail;
> }
> - if (s->crypt_method) {
> - qcow2_encrypt_sectors(s, sector_num, cluster_data,
> - cluster_data, cur_nr_sectors, 0, &s->aes_decrypt_key);
> + if (s->cipher) {
> + Error *err = NULL;
> + if (qcow2_encrypt_sectors(s, sector_num, cluster_data,
> + cluster_data, cur_nr_sectors,
> false,
> + &err) < 0) {
> + error_free(err);
> + ret = -EIO;
> + goto fail;
> + }
> qemu_iovec_from_buf(qiov, bytes_done,
> cluster_data, 512 * cur_nr_sectors);
> }
> @@ -1314,7 +1316,7 @@ static coroutine_fn int
> qcow2_co_writev(BlockDriverState *bs,
> trace_qcow2_writev_start_part(qemu_coroutine_self());
> index_in_cluster = sector_num & (s->cluster_sectors - 1);
> cur_nr_sectors = remaining_sectors;
> - if (s->crypt_method &&
> + if (s->cipher &&
> cur_nr_sectors >
> QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors - index_in_cluster)
> {
> cur_nr_sectors =
> @@ -1333,7 +1335,8 @@ static coroutine_fn int
> qcow2_co_writev(BlockDriverState *bs,
> qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
> cur_nr_sectors * 512);
>
> - if (s->crypt_method) {
> + if (s->cipher) {
> + Error *err = NULL;
> if (!cluster_data) {
> cluster_data = qemu_try_blockalign(bs->file,
> QCOW_MAX_CRYPT_CLUSTERS
> @@ -1348,8 +1351,13 @@ static coroutine_fn int
> qcow2_co_writev(BlockDriverState *bs,
> QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
> qemu_iovec_to_buf(&hd_qiov, 0, cluster_data, hd_qiov.size);
>
> - qcow2_encrypt_sectors(s, sector_num, cluster_data,
> - cluster_data, cur_nr_sectors, 1, &s->aes_encrypt_key);
> + if (qcow2_encrypt_sectors(s, sector_num, cluster_data,
> + cluster_data, cur_nr_sectors,
> + true, &err) < 0) {
> + error_free(err);
> + ret = -EIO;
> + goto fail;
> + }
>
> qemu_iovec_reset(&hd_qiov);
> qemu_iovec_add(&hd_qiov, cluster_data,
> @@ -1455,6 +1463,8 @@ static void qcow2_close(BlockDriverState *bs)
> qcow2_cache_destroy(bs, s->l2_table_cache);
> qcow2_cache_destroy(bs, s->refcount_block_cache);
>
> + qcrypto_cipher_free(s->cipher);
> +
Do we need to set s->cipher = NULL ?
Regards,
-Gonglei
> g_free(s->unknown_header_fields);
> cleanup_unknown_header_ext(bs);
>
> @@ -1471,9 +1481,7 @@ static void qcow2_invalidate_cache(BlockDriverState
> *bs, Error **errp)
> {
> BDRVQcowState *s = bs->opaque;
> int flags = s->flags;
> - AES_KEY aes_encrypt_key;
> - AES_KEY aes_decrypt_key;
> - uint32_t crypt_method = 0;
> + QCryptoCipher *cipher = NULL;
> QDict *options;
> Error *local_err = NULL;
> int ret;
> @@ -1483,11 +1491,8 @@ static void qcow2_invalidate_cache(BlockDriverState
> *bs, Error **errp)
> * that means we don't have to worry about reopening them here.
> */
>
> - if (s->crypt_method) {
> - crypt_method = s->crypt_method;
> - memcpy(&aes_encrypt_key, &s->aes_encrypt_key,
> sizeof(aes_encrypt_key));
> - memcpy(&aes_decrypt_key, &s->aes_decrypt_key,
> sizeof(aes_decrypt_key));
> - }
> + cipher = s->cipher;
> + s->cipher = NULL;
>
> qcow2_close(bs);
>
> @@ -1512,11 +1517,7 @@ static void qcow2_invalidate_cache(BlockDriverState
> *bs, Error **errp)
> return;
> }
>
> - if (crypt_method) {
> - s->crypt_method = crypt_method;
> - memcpy(&s->aes_encrypt_key, &aes_encrypt_key,
> sizeof(aes_encrypt_key));
> - memcpy(&s->aes_decrypt_key, &aes_decrypt_key,
> sizeof(aes_decrypt_key));
> - }
> + s->cipher = cipher;
> }
>
> static size_t header_ext_add(char *buf, uint32_t magic, const void *s,
> @@ -2717,8 +2718,9 @@ static int qcow2_amend_options(BlockDriverState *bs,
> QemuOpts *opts,
> backing_format = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT);
> } else if (!strcmp(desc->name, BLOCK_OPT_ENCRYPT)) {
> encrypt = qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT,
> - s->crypt_method);
> - if (encrypt != !!s->crypt_method) {
> + !!s->cipher);
> +
> + if (encrypt != !!s->cipher) {
> fprintf(stderr, "Changing the encryption flag is not "
> "supported.\n");
> return -ENOTSUP;
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 3cf5318..4af1471 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -25,7 +25,7 @@
> #ifndef BLOCK_QCOW2_H
> #define BLOCK_QCOW2_H
>
> -#include "crypto/aes.h"
> +#include "crypto/cipher.h"
> #include "block/coroutine.h"
>
> //#define DEBUG_ALLOC
> @@ -250,10 +250,8 @@ typedef struct BDRVQcowState {
>
> CoMutex lock;
>
> - uint32_t crypt_method; /* current crypt method, 0 if no key yet */
> + QCryptoCipher *cipher; /* current cipher, NULL if no key yet */
> uint32_t crypt_method_header;
> - AES_KEY aes_encrypt_key;
> - AES_KEY aes_decrypt_key;
> uint64_t snapshots_offset;
> int snapshots_size;
> unsigned int nb_snapshots;
> @@ -533,10 +531,9 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t
> min_size,
> int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index);
> void qcow2_l2_cache_reset(BlockDriverState *bs);
> int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset);
> -void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num,
> - uint8_t *out_buf, const uint8_t *in_buf,
> - int nb_sectors, int enc,
> - const AES_KEY *key);
> +int qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num,
> + uint8_t *out_buf, const uint8_t *in_buf,
> + int nb_sectors, bool enc, Error **errp);
>
> int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
> int *num, uint64_t *cluster_offset);
>
