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>
			
			
This commit is contained in:
		
							parent
							
								
									aa4f592a1d
								
							
						
					
					
						commit
						8336aafae1
					
				
							
								
								
									
										10
									
								
								block/qcow.c
								
								
								
								
							
							
						
						
									
										10
									
								
								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);
 | 
			
		||||
            }
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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);
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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));
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in New Issue