diff mbox

[07/17] qcow2: add a 'keyid' parameter to qcow2 options

Message ID 1445267389-21846-8-git-send-email-berrange@redhat.com
State New
Headers show

Commit Message

Daniel P. Berrangé Oct. 19, 2015, 3:09 p.m. UTC
Add a 'keyid' parameter that refers to the ID of a
QCryptoSecret instance that provides the encryption key.

$QEMU \
    -object secret,id=sec0,filename=/home/berrange/encrypted.pw \
    -drive file=/home/berrange/encrypted.qcow2,keyid=sec0

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 block/qcow2.c        | 80 +++++++++++++++++++++++++++++++++++++---------------
 block/qcow2.h        |  1 +
 qapi/block-core.json |  8 ++++--
 3 files changed, 64 insertions(+), 25 deletions(-)

Comments

Eric Blake Oct. 19, 2015, 11:29 p.m. UTC | #1
On 10/19/2015 09:09 AM, Daniel P. Berrange wrote:
> Add a 'keyid' parameter that refers to the ID of a
> QCryptoSecret instance that provides the encryption key.
> 
> $QEMU \
>     -object secret,id=sec0,filename=/home/berrange/encrypted.pw \
>     -drive file=/home/berrange/encrypted.qcow2,keyid=sec0
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  block/qcow2.c        | 80 +++++++++++++++++++++++++++++++++++++---------------
>  block/qcow2.h        |  1 +
>  qapi/block-core.json |  8 ++++--
>  3 files changed, 64 insertions(+), 25 deletions(-)
> 

> +++ b/qapi/block-core.json
> @@ -1567,7 +1567,7 @@
>  # Driver specific block device options for qcow.
>  #
>  # @keyid:                 #optional ID of the "secret" object providing the
> -#                         AES decryption key.
> +#                         AES decryption key (since 2.5)

Looks like this line...

>  #
>  # Since: 2.5
>  ##
> @@ -1611,6 +1611,9 @@
>  #                         caches. The interval is in seconds. The default value
>  #                         is 0 and it disables this feature (since 2.5)
>  #
> +# @keyid:                 #optional ID of the "secret" object providing the
> +#                         AES decryption key.

...and this line should be swapped.

> +#
>  # Since: 1.7

(For qcow, the entire struct is new so @keyid doesn't need versioning;
for qcow2, the struct existed since 1.7 and we are extending it in 2.5)
Eric Blake Oct. 28, 2015, 1:58 p.m. UTC | #2
On 10/19/2015 05:29 PM, Eric Blake wrote:
> On 10/19/2015 09:09 AM, Daniel P. Berrange wrote:
>> Add a 'keyid' parameter that refers to the ID of a
>> QCryptoSecret instance that provides the encryption key.
>>
>> $QEMU \
>>     -object secret,id=sec0,filename=/home/berrange/encrypted.pw \
>>     -drive file=/home/berrange/encrypted.qcow2,keyid=sec0
>>
>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>> ---
>>  block/qcow2.c        | 80 +++++++++++++++++++++++++++++++++++++---------------
>>  block/qcow2.h        |  1 +
>>  qapi/block-core.json |  8 ++++--
>>  3 files changed, 64 insertions(+), 25 deletions(-)
>>
> 
>> +++ b/qapi/block-core.json
>> @@ -1567,7 +1567,7 @@
>>  # Driver specific block device options for qcow.
>>  #
>>  # @keyid:                 #optional ID of the "secret" object providing the
>> -#                         AES decryption key.
>> +#                         AES decryption key (since 2.5)
> 
> Looks like this line...
> 
>>  #
>>  # Since: 2.5
>>  ##
>> @@ -1611,6 +1611,9 @@
>>  #                         caches. The interval is in seconds. The default value
>>  #                         is 0 and it disables this feature (since 2.5)
>>  #
>> +# @keyid:                 #optional ID of the "secret" object providing the
>> +#                         AES decryption key.
> 
> ...and this line should be swapped.
> 

Also, do you want to change BlockdevOptionsQcow2 to have a base class of
BlockdevOptionsQcow, and get keyid by inheritance rather than by direct
declaration?  Doesn't matter in the long run (once my qapi patches land
that provide the information without going through an extra 'base->' layer).
diff mbox

Patch

diff --git a/block/qcow2.c b/block/qcow2.c
index bacc4f2..3b108b0 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -34,6 +34,7 @@ 
 #include "qapi-event.h"
 #include "trace.h"
 #include "qemu/option_int.h"
+#include "crypto/secret.h"
 
 /*
   Differences with QCOW:
@@ -472,6 +473,11 @@  static QemuOptsList qcow2_runtime_opts = {
             .type = QEMU_OPT_NUMBER,
             .help = "Clean unused cache entries after this time (in seconds)",
         },
+        {
+            .name = QCOW2_OPT_KEY_ID,
+            .type = QEMU_OPT_STRING,
+            .help = "ID of the secret that provides the encryption key",
+        },
         { /* end of list */ }
     },
 };
@@ -589,6 +595,31 @@  static void read_cache_sizes(BlockDriverState *bs, QemuOpts *opts,
     }
 }
 
+static QCryptoCipher *qcow2_get_cipher_from_key(const char *key,
+                                                Error **errp)
+{
+    uint8_t keybuf[16];
+    int len, i;
+
+    memset(keybuf, 0, 16);
+    len = strlen(key);
+    if (len > 16) {
+        len = 16;
+    }
+    /* XXX: we could compress the chars to 7 bits to increase
+       entropy */
+    for (i = 0; i < len; i++) {
+        keybuf[i] = key[i];
+    }
+
+    return qcrypto_cipher_new(
+        QCRYPTO_CIPHER_ALG_AES_128,
+        QCRYPTO_CIPHER_MODE_CBC,
+        keybuf, G_N_ELEMENTS(keybuf),
+        errp);
+}
+
+
 typedef struct Qcow2ReopenState {
     Qcow2Cache *l2_table_cache;
     Qcow2Cache *refcount_block_cache;
@@ -596,6 +627,7 @@  typedef struct Qcow2ReopenState {
     int overlap_check;
     bool discard_passthrough[QCOW2_DISCARD_MAX];
     uint64_t cache_clean_interval;
+    QCryptoCipher *cipher;
 } Qcow2ReopenState;
 
 static int qcow2_update_options_prepare(BlockDriverState *bs,
@@ -611,6 +643,8 @@  static int qcow2_update_options_prepare(BlockDriverState *bs,
     int i;
     Error *local_err = NULL;
     int ret;
+    const char *keyid;
+    char *key;
 
     opts = qemu_opts_create(&qcow2_runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
@@ -754,6 +788,24 @@  static int qcow2_update_options_prepare(BlockDriverState *bs,
     r->discard_passthrough[QCOW2_DISCARD_OTHER] =
         qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false);
 
+    keyid = qemu_opt_get(opts, QCOW2_OPT_KEY_ID);
+    if (keyid) {
+        key = qcrypto_secret_lookup_as_utf8(keyid,
+                                            errp);
+        if (!key) {
+            ret = -ENOENT;
+            goto fail;
+        }
+
+        r->cipher = qcow2_get_cipher_from_key(key,
+                                              errp);
+        g_free(key);
+        if (!r->cipher) {
+            ret = -ENOSYS;
+            goto fail;
+        }
+    }
+
     ret = 0;
 fail:
     qemu_opts_del(opts);
@@ -788,6 +840,9 @@  static void qcow2_update_options_commit(BlockDriverState *bs,
         s->cache_clean_interval = r->cache_clean_interval;
         cache_clean_timer_init(bs, bdrv_get_aio_context(bs));
     }
+
+    qcrypto_cipher_free(s->cipher);
+    s->cipher = r->cipher;
 }
 
 static void qcow2_update_options_abort(BlockDriverState *bs,
@@ -799,6 +854,7 @@  static void qcow2_update_options_abort(BlockDriverState *bs,
     if (r->refcount_block_cache) {
         qcow2_cache_destroy(bs, r->refcount_block_cache);
     }
+    qcrypto_cipher_free(r->cipher);
 }
 
 static int qcow2_update_options(BlockDriverState *bs, QDict *options,
@@ -1202,33 +1258,11 @@  static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp)
 static int qcow2_set_key(BlockDriverState *bs, const char *key)
 {
     BDRVQcow2State *s = bs->opaque;
-    uint8_t keybuf[16];
-    int len, i;
-    Error *err = NULL;
 
-    memset(keybuf, 0, 16);
-    len = strlen(key);
-    if (len > 16)
-        len = 16;
-    /* XXX: we could compress the chars to 7 bits to increase
-       entropy */
-    for(i = 0;i < len;i++) {
-        keybuf[i] = key[i];
-    }
     assert(bs->encrypted);
-
     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);
-
+    s->cipher = qcow2_get_cipher_from_key(key, NULL);
     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;
diff --git a/block/qcow2.h b/block/qcow2.h
index 3512263..7e2f046 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -97,6 +97,7 @@ 
 #define QCOW2_OPT_L2_CACHE_SIZE "l2-cache-size"
 #define QCOW2_OPT_REFCOUNT_CACHE_SIZE "refcount-cache-size"
 #define QCOW2_OPT_CACHE_CLEAN_INTERVAL "cache-clean-interval"
+#define QCOW2_OPT_KEY_ID "keyid"
 
 typedef struct QCowHeader {
     uint32_t magic;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 513fe93..9f0ec68 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1567,7 +1567,7 @@ 
 # Driver specific block device options for qcow.
 #
 # @keyid:                 #optional ID of the "secret" object providing the
-#                         AES decryption key.
+#                         AES decryption key (since 2.5)
 #
 # Since: 2.5
 ##
@@ -1611,6 +1611,9 @@ 
 #                         caches. The interval is in seconds. The default value
 #                         is 0 and it disables this feature (since 2.5)
 #
+# @keyid:                 #optional ID of the "secret" object providing the
+#                         AES decryption key.
+#
 # Since: 1.7
 ##
 { 'struct': 'BlockdevOptionsQcow2',
@@ -1623,7 +1626,8 @@ 
             '*cache-size': 'int',
             '*l2-cache-size': 'int',
             '*refcount-cache-size': 'int',
-            '*cache-clean-interval': 'int' } }
+            '*cache-clean-interval': 'int',
+            '*keyid': 'str' } }
 
 
 ##