Message ID | 1432205817-16414-10-git-send-email-berrange@redhat.com |
---|---|
State | New |
Headers | show |
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 <berrange@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); >
On Fri, May 29, 2015 at 03:16:49PM +0800, Gonglei wrote: > 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 <berrange@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. Hmm, yes, the free method accepts NULL just fine. > > > + 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. We're limited by the error code abilities of this code - basically there is no where to propagate the error back up to. As & when this code is updated to properly propagate errors we can do this here. > > @@ -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 ? Yes, probably worth while as a sanity check. Regards, Daniel
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) { + 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; + } 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); + 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);
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 <berrange@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(-)