[Qemu-devel] [PATCH v3 09/10] block: convert qcow/qcow2 to use generic cipher API

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

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          | 102 +++++++++++++++++++++++++++++++++++++-------------
 block/qcow2-cluster.c |  46 ++++++++++++++++++-----
 block/qcow2.c         |  95 +++++++++++++++++++++++-----------------------
 block/qcow2.h         |  13 +++----
 4 files changed, 165 insertions(+), 91 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 5a7c6cb..1e5179a 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_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;
@@ -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);
@@ -270,38 +274,67 @@ static int qcow_set_key(BlockDriverState *bs, const char *key)
         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)
-        return -1;
-    if (AES_set_decrypt_key(keybuf, 128, &s->aes_decrypt_key) != 0)
+    qcrypto_cipher_free(s->cipher);
+    s->cipher = qcrypto_cipher_new(
+        QCRYPTO_CIPHER_ALG_AES_128,
+        QCRYPTO_CIPHER_MODE_CBC,
+        keybuf, G_N_ELEMENTS(keybuf),
+        &err);
+
+    if (!s->cipher) {
+        /* XXX would be nice if errors in this method could
+         * be properly propagate to the caller. Would need
+         * the bdrv_set_key() API signature to be fixed. */
+        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:
@@ -415,15 +448,20 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
                 if (bs->encrypted &&
                     (n_end - n_start) < s->cluster_sectors) {
                     uint64_t start_sect;
-                    assert(s->crypt_method);
+                    assert(s->cipher);
                     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;
@@ -463,7 +501,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);
@@ -530,6 +568,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);
@@ -593,10 +632,11 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
                 break;
             }
             if (bs->encrypted) {
-                assert(s->crypt_method);
-                encrypt_sectors(s, sector_num, buf, buf,
-                                n, 0,
-                                &s->aes_decrypt_key);
+                assert(s->cipher);
+                if (encrypt_sectors(s, sector_num, buf, buf,
+                                    n, false, &err) < 0) {
+                    goto fail;
+                }
             }
         }
         ret = 0;
@@ -617,6 +657,7 @@ done:
     return ret;
 
 fail:
+    error_free(err);
     ret = -EIO;
     goto done;
 }
@@ -665,12 +706,17 @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
             break;
         }
         if (bs->encrypted) {
-            assert(s->crypt_method);
+            Error *err = NULL;
+            assert(s->cipher);
             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;
@@ -707,6 +753,8 @@ static void qcow_close(BlockDriverState *bs)
 {
     BDRVQcowState *s = bs->opaque;
 
+    qcrypto_cipher_free(s->cipher);
+    s->cipher = NULL;
     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 1a5c97a..b43f186 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -339,26 +339,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,
@@ -401,10 +422,15 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs,
     }
 
     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);
+        Error *err = NULL;
+        assert(s->cipher);
+        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 e9e3fa5..7e14bcd 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -698,6 +698,11 @@ 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;
@@ -1031,6 +1036,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);
@@ -1042,30 +1048,21 @@ static int qcow2_set_key(BlockDriverState *bs, const char *key)
         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)
-        return -1;
-    if (AES_set_decrypt_key(keybuf, 128, &s->aes_decrypt_key) != 0)
+    qcrypto_cipher_free(s->cipher);
+    s->cipher = qcrypto_cipher_new(
+        QCRYPTO_CIPHER_ALG_AES_128,
+        QCRYPTO_CIPHER_MODE_CBC,
+        keybuf, G_N_ELEMENTS(keybuf),
+        &err);
+
+    if (!s->cipher) {
+        /* XXX would be nice if errors in this method could
+         * be properly propagate to the caller. Would need
+         * the bdrv_set_key() API signature to be fixed. */
+        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;
 }
 
@@ -1108,7 +1105,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;
@@ -1158,7 +1155,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);
         }
@@ -1230,7 +1227,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num,
             }
 
             if (bs->encrypted) {
-                assert(s->crypt_method);
+                assert(s->cipher);
 
                 /*
                  * For encrypted images, read everything into a temporary
@@ -1263,9 +1260,15 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num,
                 goto fail;
             }
             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);
+                assert(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);
             }
@@ -1343,7 +1346,8 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs,
             cur_nr_sectors * 512);
 
         if (bs->encrypted) {
-            assert(s->crypt_method);
+            Error *err = NULL;
+            assert(s->cipher);
             if (!cluster_data) {
                 cluster_data = qemu_try_blockalign(bs->file,
                                                    QCOW_MAX_CRYPT_CLUSTERS
@@ -1358,8 +1362,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,
@@ -1465,6 +1474,9 @@ 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);
+    s->cipher = NULL;
+
     g_free(s->unknown_header_fields);
     cleanup_unknown_header_ext(bs);
 
@@ -1481,9 +1493,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;
@@ -1493,12 +1503,8 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
      * that means we don't have to worry about reopening them here.
      */
 
-    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));
-    }
+    cipher = s->cipher;
+    s->cipher = NULL;
 
     qcow2_close(bs);
 
@@ -1523,11 +1529,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
         return;
     }
 
-    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));
-    }
+    s->cipher = cipher;
 }
 
 static size_t header_ext_add(char *buf, uint32_t magic, const void *s,
@@ -2728,8 +2730,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 462147c..72e1328 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
@@ -253,10 +253,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;
@@ -536,10 +534,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);
-- 
2.4.2