diff options
author | Daniel P. Berrange <berrange@redhat.com> | 2015-05-12 17:09:18 +0100 |
---|---|---|
committer | Kevin Wolf <kwolf@redhat.com> | 2015-05-22 17:08:01 +0200 |
commit | 8336aafae1451d54c81dd2b187b45f7c45d2428e (patch) | |
tree | 9522e81e80c456021576ad40e1e96b49d9284303 /block | |
parent | aa4f592a1dd9aea5f5c36f6ff4b22b5bd208162a (diff) | |
download | qemu-8336aafae1451d54c81dd2b187b45f7c45d2428e.zip qemu-8336aafae1451d54c81dd2b187b45f7c45d2428e.tar.gz qemu-8336aafae1451d54c81dd2b187b45f7c45d2428e.tar.bz2 |
qcow2/qcow: protect against uninitialized encryption key
When a qcow[2] file is opened, if the header reports an
encryption method, this is used to set the 'crypt_method_header'
field on the BDRVQcow[2]State struct, and the 'encrypted' flag
in the BDRVState struct.
When doing I/O operations, the 'crypt_method' field on the
BDRVQcow[2]State struct is checked to determine if encryption
needs to be applied.
The crypt_method_header value is copied into crypt_method when
the bdrv_set_key() method is called.
The QEMU code which opens a block device is expected to always
do a check
if (bdrv_is_encrypted(bs)) {
bdrv_set_key(bs, ....key...);
}
If code forgets to do this, then 'crypt_method' is never set
and so when I/O is performed, QEMU writes plain text data
into a sector which is expected to contain cipher text, or
when reading, will return cipher text instead of plain
text.
Change the qcow[2] code to consult bs->encrypted when deciding
whether encryption is required, and assert(s->crypt_method)
to protect against cases where the caller forgets to set the
encryption key.
Also put an assert in the set_key methods to protect against
the case where the caller sets an encryption key on a block
device that does not have encryption
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Diffstat (limited to 'block')
-rw-r--r-- | block/qcow.c | 10 | ||||
-rw-r--r-- | block/qcow2-cluster.c | 3 | ||||
-rw-r--r-- | block/qcow2.c | 18 |
3 files changed, 21 insertions, 10 deletions
diff --git a/block/qcow.c b/block/qcow.c index ab89328..911e59f 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -269,6 +269,7 @@ static int qcow_set_key(BlockDriverState *bs, const char *key) for(i = 0;i < len;i++) { keybuf[i] = key[i]; } + assert(bs->encrypted); s->crypt_method = s->crypt_method_header; if (AES_set_encrypt_key(keybuf, 128, &s->aes_encrypt_key) != 0) @@ -411,9 +412,10 @@ 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 (bs->encrypted && (n_end - n_start) < s->cluster_sectors) { uint64_t start_sect; + assert(s->crypt_method); start_sect = (offset & ~(s->cluster_size - 1)) >> 9; memset(s->cluster_data + 512, 0x00, 512); for(i = 0; i < s->cluster_sectors; i++) { @@ -590,7 +592,8 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, if (ret < 0) { break; } - if (s->crypt_method) { + if (bs->encrypted) { + assert(s->crypt_method); encrypt_sectors(s, sector_num, buf, buf, n, 0, &s->aes_decrypt_key); @@ -661,7 +664,8 @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num, ret = -EIO; break; } - if (s->crypt_method) { + if (bs->encrypted) { + assert(s->crypt_method); if (!cluster_data) { cluster_data = g_malloc0(s->cluster_size); } diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index d74426c..1a5c97a 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -400,7 +400,8 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs, goto out; } - if (s->crypt_method) { + if (bs->encrypted) { + assert(s->crypt_method); qcow2_encrypt_sectors(s, start_sect + n_start, iov.iov_base, iov.iov_base, n, 1, &s->aes_encrypt_key); diff --git a/block/qcow2.c b/block/qcow2.c index b9a72e3..f7b4cc6 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1037,6 +1037,7 @@ static int qcow2_set_key(BlockDriverState *bs, const char *key) for(i = 0;i < len;i++) { keybuf[i] = key[i]; } + assert(bs->encrypted); s->crypt_method = s->crypt_method_header; if (AES_set_encrypt_key(keybuf, 128, &s->aes_encrypt_key) != 0) @@ -1224,7 +1225,9 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, goto fail; } - if (s->crypt_method) { + if (bs->encrypted) { + assert(s->crypt_method); + /* * For encrypted images, read everything into a temporary * contiguous buffer on which the AES functions can work. @@ -1255,7 +1258,8 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, if (ret < 0) { goto fail; } - if (s->crypt_method) { + if (bs->encrypted) { + assert(s->crypt_method); qcow2_encrypt_sectors(s, sector_num, cluster_data, cluster_data, cur_nr_sectors, 0, &s->aes_decrypt_key); qemu_iovec_from_buf(qiov, bytes_done, @@ -1315,7 +1319,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 (bs->encrypted && cur_nr_sectors > QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors - index_in_cluster) { cur_nr_sectors = @@ -1334,7 +1338,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 (bs->encrypted) { + assert(s->crypt_method); if (!cluster_data) { cluster_data = qemu_try_blockalign(bs->file, QCOW_MAX_CRYPT_CLUSTERS @@ -1484,7 +1489,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) { + if (bs->encrypted) { + assert(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)); @@ -1513,7 +1519,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) return; } - if (crypt_method) { + if (bs->encrypted) { 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)); |