[Qemu-devel] [PULL v1 0/6] Misc crypto changes & fixes

[Qemu-devel] [PULL v1 0/6] Misc crypto changes & fixes

[Daniel P. Berrange]

The following changes since commit 5dc42c186d63b7b338594fc071cf290805dcc5a5:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2015-12-22 14:21:42 +0000)

are available in the git repository at:

  git://github.com/berrange/qemu tags/pull-crypto-fixes-2015-12-23-1

for you to fetch changes up to 50de6261510301d586f0411b3fdb11e4cd4fdb52:

  crypto: fix transposed arguments in cipher error message (2015-12-23 11:02:20 +0000)

----------------------------------------------------------------
Merge misc crypto changes & fixes

----------------------------------------------------------------
Daniel P. Berrange (6):
      crypto: add additional query accessors for cipher instances
      crypto: add ability to query hash digest len
      crypto: move QCryptoHashAlgorithm enum definition into QAPI
      crypto: move QCryptoCipherAlgorithm/Mode enum definitions into QAPI
      crypto: ensure qapi/crypto.json is listed in qapi-modules
      crypto: fix transposed arguments in cipher error message

 Makefile                   |  3 ++-
 crypto/cipher.c            | 54 +++++++++++++++++++++++++++++++++++++++++++---
 crypto/hash.c              | 17 ++++++++++++++-
 include/crypto/cipher.h    | 54 +++++++++++++++++++++++++++++++++-------------
 include/crypto/hash.h      | 20 ++++++++++-------
 qapi/crypto.json           | 45 ++++++++++++++++++++++++++++++++++++++
 tests/test-crypto-cipher.c | 10 +++++++++
 tests/test-crypto-hash.c   |  5 +++++
 8 files changed, 180 insertions(+), 28 deletions(-)
-- 
2.5.0

[Qemu-devel] [PULL v1 1/6] crypto: add additional query accessors for cipher instances

[Daniel P. Berrange]

Adds new methods to allow querying the length of the cipher
key, block size and initialization vectors.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/cipher.c            | 48 ++++++++++++++++++++++++++++++++++++++++++++++
 include/crypto/cipher.h    | 37 +++++++++++++++++++++++++++++++++++
 tests/test-crypto-cipher.c | 10 ++++++++++
 3 files changed, 95 insertions(+)

diff --git a/crypto/cipher.c b/crypto/cipher.c
index c8bd180..d02bb32 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -28,6 +28,54 @@ static size_t alg_key_len[QCRYPTO_CIPHER_ALG_LAST] = {
     [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
 };
 
+static size_t alg_block_len[QCRYPTO_CIPHER_ALG_LAST] = {
+    [QCRYPTO_CIPHER_ALG_AES_128] = 16,
+    [QCRYPTO_CIPHER_ALG_AES_192] = 16,
+    [QCRYPTO_CIPHER_ALG_AES_256] = 16,
+    [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
+};
+
+static bool mode_need_iv[QCRYPTO_CIPHER_MODE_LAST] = {
+    [QCRYPTO_CIPHER_MODE_ECB] = false,
+    [QCRYPTO_CIPHER_MODE_CBC] = true,
+};
+
+
+size_t qcrypto_cipher_get_block_len(QCryptoCipherAlgorithm alg)
+{
+    if (alg >= G_N_ELEMENTS(alg_key_len)) {
+        return 0;
+    }
+    return alg_block_len[alg];
+}
+
+
+size_t qcrypto_cipher_get_key_len(QCryptoCipherAlgorithm alg)
+{
+    if (alg >= G_N_ELEMENTS(alg_key_len)) {
+        return 0;
+    }
+    return alg_key_len[alg];
+}
+
+
+size_t qcrypto_cipher_get_iv_len(QCryptoCipherAlgorithm alg,
+                                 QCryptoCipherMode mode)
+{
+    if (alg >= G_N_ELEMENTS(alg_block_len)) {
+        return 0;
+    }
+    if (mode >= G_N_ELEMENTS(mode_need_iv)) {
+        return 0;
+    }
+
+    if (mode_need_iv[mode]) {
+        return alg_block_len[alg];
+    }
+    return 0;
+}
+
+
 static bool
 qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
                                    size_t nkey,
diff --git a/include/crypto/cipher.h b/include/crypto/cipher.h
index b4d714f..aa51c89 100644
--- a/include/crypto/cipher.h
+++ b/include/crypto/cipher.h
@@ -107,6 +107,43 @@ struct QCryptoCipher {
  */
 bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg);
 
+/**
+ * qcrypto_cipher_get_block_len:
+ * @alg: the cipher algorithm
+ *
+ * Get the required data block size in bytes. When
+ * encrypting data, it must be a multiple of the
+ * block size.
+ *
+ * Returns: the block size in bytes
+ */
+size_t qcrypto_cipher_get_block_len(QCryptoCipherAlgorithm alg);
+
+
+/**
+ * qcrypto_cipher_get_key_len:
+ * @alg: the cipher algorithm
+ *
+ * Get the required key size in bytes.
+ *
+ * Returns: the key size in bytes
+ */
+size_t qcrypto_cipher_get_key_len(QCryptoCipherAlgorithm alg);
+
+
+/**
+ * qcrypto_cipher_get_iv_len:
+ * @alg: the cipher algorithm
+ * @mode: the cipher mode
+ *
+ * Get the required initialization vector size
+ * in bytes, if one is required.
+ *
+ * Returns: the IV size in bytes, or 0 if no IV is permitted
+ */
+size_t qcrypto_cipher_get_iv_len(QCryptoCipherAlgorithm alg,
+                                 QCryptoCipherMode mode);
+
 
 /**
  * qcrypto_cipher_new:
diff --git a/tests/test-crypto-cipher.c b/tests/test-crypto-cipher.c
index f4946a0..c687307 100644
--- a/tests/test-crypto-cipher.c
+++ b/tests/test-crypto-cipher.c
@@ -229,6 +229,7 @@ static void test_cipher(const void *opaque)
     uint8_t *key, *iv, *ciphertext, *plaintext, *outtext;
     size_t nkey, niv, nciphertext, nplaintext;
     char *outtexthex;
+    size_t ivsize, keysize, blocksize;
 
     nkey = unhex_string(data->key, &key);
     niv = unhex_string(data->iv, &iv);
@@ -245,6 +246,15 @@ static void test_cipher(const void *opaque)
         &error_abort);
     g_assert(cipher != NULL);
 
+    keysize = qcrypto_cipher_get_key_len(data->alg);
+    blocksize = qcrypto_cipher_get_block_len(data->alg);
+    ivsize = qcrypto_cipher_get_iv_len(data->alg, data->mode);
+
+    g_assert_cmpint(keysize, ==, nkey);
+    g_assert_cmpint(ivsize, ==, niv);
+    if (niv) {
+        g_assert_cmpint(blocksize, ==, niv);
+    }
 
     if (iv) {
         g_assert(qcrypto_cipher_setiv(cipher,
-- 
2.5.0

[Qemu-devel] [PULL v1 2/6] crypto: add ability to query hash digest len

[Daniel P. Berrange]

Add a qcrypto_hash_digest_len() method which allows querying of
the raw digest size for a given hash algorithm.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/hash.c            | 15 +++++++++++++++
 include/crypto/hash.h    | 11 +++++++++++
 tests/test-crypto-hash.c |  5 +++++
 3 files changed, 31 insertions(+)

diff --git a/crypto/hash.c b/crypto/hash.c
index 81e74de..5a47b90 100644
--- a/crypto/hash.c
+++ b/crypto/hash.c
@@ -30,6 +30,12 @@ static int qcrypto_hash_alg_map[QCRYPTO_HASH_ALG_LAST] = {
     [QCRYPTO_HASH_ALG_SHA256] = GNUTLS_DIG_SHA256,
 };
 
+static size_t qcrypto_hash_alg_size[QCRYPTO_HASH_ALG_LAST] = {
+    [QCRYPTO_HASH_ALG_MD5] = 16,
+    [QCRYPTO_HASH_ALG_SHA1] = 20,
+    [QCRYPTO_HASH_ALG_SHA256] = 32,
+};
+
 gboolean qcrypto_hash_supports(QCryptoHashAlgorithm alg)
 {
     if (alg < G_N_ELEMENTS(qcrypto_hash_alg_map)) {
@@ -38,6 +44,15 @@ gboolean qcrypto_hash_supports(QCryptoHashAlgorithm alg)
     return false;
 }
 
+size_t qcrypto_hash_digest_len(QCryptoHashAlgorithm alg)
+{
+    if (alg >= G_N_ELEMENTS(qcrypto_hash_alg_size)) {
+        return 0;
+    }
+    return qcrypto_hash_alg_size[alg];
+}
+
+
 int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
                         const struct iovec *iov,
                         size_t niov,
diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index b5acbf6..3d18124 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -44,6 +44,17 @@ typedef enum {
  */
 gboolean qcrypto_hash_supports(QCryptoHashAlgorithm alg);
 
+
+/**
+ * qcrypto_hash_digest_len:
+ * @alg: the hash algorithm
+ *
+ * Determine the size of the hash digest in bytes
+ *
+ * Returns: the digest length in bytes
+ */
+size_t qcrypto_hash_digest_len(QCryptoHashAlgorithm alg);
+
 /**
  * qcrypto_hash_bytesv:
  * @alg: the hash algorithm
diff --git a/tests/test-crypto-hash.c b/tests/test-crypto-hash.c
index 911437e..3ec31dd 100644
--- a/tests/test-crypto-hash.c
+++ b/tests/test-crypto-hash.c
@@ -163,6 +163,11 @@ static void test_hash_digest(void)
     for (i = 0; i < G_N_ELEMENTS(expected_outputs) ; i++) {
         int ret;
         char *digest;
+        size_t digestsize;
+
+        digestsize = qcrypto_hash_digest_len(i);
+
+        g_assert_cmpint(digestsize * 2, ==, strlen(expected_outputs[i]));
 
         ret = qcrypto_hash_digest(i,
                                   INPUT_TEXT,
-- 
2.5.0

[Qemu-devel] [PULL v1 3/6] crypto: move QCryptoHashAlgorithm enum definition into QAPI

[Daniel P. Berrange]

The QCryptoHashAlgorithm enum is defined in the crypto/hash.h
header. In the future some QAPI types will want to reference
the hash enums, so move the enum definition into QAPI too.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/hash.c         |  4 ++--
 include/crypto/hash.h |  9 +--------
 qapi/crypto.json      | 15 +++++++++++++++
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/crypto/hash.c b/crypto/hash.c
index 5a47b90..6e83f43 100644
--- a/crypto/hash.c
+++ b/crypto/hash.c
@@ -24,13 +24,13 @@
 #include <gnutls/gnutls.h>
 #include <gnutls/crypto.h>
 
-static int qcrypto_hash_alg_map[QCRYPTO_HASH_ALG_LAST] = {
+static int qcrypto_hash_alg_map[QCRYPTO_HASH_ALG__MAX] = {
     [QCRYPTO_HASH_ALG_MD5] = GNUTLS_DIG_MD5,
     [QCRYPTO_HASH_ALG_SHA1] = GNUTLS_DIG_SHA1,
     [QCRYPTO_HASH_ALG_SHA256] = GNUTLS_DIG_SHA256,
 };
 
-static size_t qcrypto_hash_alg_size[QCRYPTO_HASH_ALG_LAST] = {
+static size_t qcrypto_hash_alg_size[QCRYPTO_HASH_ALG__MAX] = {
     [QCRYPTO_HASH_ALG_MD5] = 16,
     [QCRYPTO_HASH_ALG_SHA1] = 20,
     [QCRYPTO_HASH_ALG_SHA256] = 32,
diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index 3d18124..41822c0 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -24,14 +24,7 @@
 #include "qemu-common.h"
 #include "qapi/error.h"
 
-typedef enum {
-    QCRYPTO_HASH_ALG_MD5,
-    QCRYPTO_HASH_ALG_SHA1,
-    QCRYPTO_HASH_ALG_SHA256,
-
-    QCRYPTO_HASH_ALG_LAST
-} QCryptoHashAlgorithm;
-
+/* See also "QCryptoHashAlgorithm" defined in qapi/crypto.json */
 
 /**
  * qcrypto_hash_supports:
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 4012659..0706ded 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -33,3 +33,18 @@
 { 'enum': 'QCryptoSecretFormat',
   'prefix': 'QCRYPTO_SECRET_FORMAT',
   'data': ['raw', 'base64']}
+
+
+##
+# QCryptoHashAlgorithm:
+#
+# The supported algorithms for computing content digests
+#
+# @md5: MD5. Should not be used in any new code, legacy compat only
+# @sha1: SHA-1. Should not be used in any new code, legacy compat only
+# @sha256: SHA-256. Current recommended strong hash.
+# Since: 2.6
+##
+{ 'enum': 'QCryptoHashAlgorithm',
+  'prefix': 'QCRYPTO_HASH_ALG',
+  'data': ['md5', 'sha1', 'sha256']}
-- 
2.5.0

[Qemu-devel] [PULL v1 4/6] crypto: move QCryptoCipherAlgorithm/Mode enum definitions into QAPI

[Daniel P. Berrange]

The QCryptoCipherAlgorithm and QCryptoCipherMode enums are
defined in the crypto/cipher.h header. In the future some
QAPI types will want to reference the hash enums, so move
the enum definition into QAPI too.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/cipher.c         |  8 ++++----
 include/crypto/cipher.h | 17 ++---------------
 qapi/crypto.json        | 30 ++++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/crypto/cipher.c b/crypto/cipher.c
index d02bb32..a24677c 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -21,21 +21,21 @@
 #include "crypto/cipher.h"
 
 
-static size_t alg_key_len[QCRYPTO_CIPHER_ALG_LAST] = {
+static size_t alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
     [QCRYPTO_CIPHER_ALG_AES_128] = 16,
     [QCRYPTO_CIPHER_ALG_AES_192] = 24,
     [QCRYPTO_CIPHER_ALG_AES_256] = 32,
     [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
 };
 
-static size_t alg_block_len[QCRYPTO_CIPHER_ALG_LAST] = {
+static size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
     [QCRYPTO_CIPHER_ALG_AES_128] = 16,
     [QCRYPTO_CIPHER_ALG_AES_192] = 16,
     [QCRYPTO_CIPHER_ALG_AES_256] = 16,
     [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
 };
 
-static bool mode_need_iv[QCRYPTO_CIPHER_MODE_LAST] = {
+static bool mode_need_iv[QCRYPTO_CIPHER_MODE__MAX] = {
     [QCRYPTO_CIPHER_MODE_ECB] = false,
     [QCRYPTO_CIPHER_MODE_CBC] = true,
 };
@@ -81,7 +81,7 @@ qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
                                    size_t nkey,
                                    Error **errp)
 {
-    if ((unsigned)alg >= QCRYPTO_CIPHER_ALG_LAST) {
+    if ((unsigned)alg >= QCRYPTO_CIPHER_ALG__MAX) {
         error_setg(errp, "Cipher algorithm %d out of range",
                    alg);
         return false;
diff --git a/include/crypto/cipher.h b/include/crypto/cipher.h
index aa51c89..a812803 100644
--- a/include/crypto/cipher.h
+++ b/include/crypto/cipher.h
@@ -26,21 +26,8 @@
 
 typedef struct QCryptoCipher QCryptoCipher;
 
-typedef enum {
-    QCRYPTO_CIPHER_ALG_AES_128,
-    QCRYPTO_CIPHER_ALG_AES_192,
-    QCRYPTO_CIPHER_ALG_AES_256,
-    QCRYPTO_CIPHER_ALG_DES_RFB, /* A stupid variant on DES for VNC */
-
-    QCRYPTO_CIPHER_ALG_LAST
-} QCryptoCipherAlgorithm;
-
-typedef enum {
-    QCRYPTO_CIPHER_MODE_ECB,
-    QCRYPTO_CIPHER_MODE_CBC,
-
-    QCRYPTO_CIPHER_MODE_LAST
-} QCryptoCipherMode;
+/* See also "QCryptoCipherAlgorithm" and "QCryptoCipherMode"
+ * enums defined in qapi/crypto.json */
 
 /**
  * QCryptoCipher:
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 0706ded..4bd690f 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -48,3 +48,33 @@
 { 'enum': 'QCryptoHashAlgorithm',
   'prefix': 'QCRYPTO_HASH_ALG',
   'data': ['md5', 'sha1', 'sha256']}
+
+
+##
+# QCryptoCipherAlgorithm:
+#
+# The supported algorithms for content encryption ciphers
+#
+# @aes-128: AES with 128 bit / 16 byte keys
+# @aes-192: AES with 192 bit / 24 byte keys
+# @aes-256: AES with 256 bit / 32 byte keys
+# @des-rfb: RFB specific variant of single DES. Do not use except in VNC.
+# Since: 2.6
+##
+{ 'enum': 'QCryptoCipherAlgorithm',
+  'prefix': 'QCRYPTO_CIPHER_ALG',
+  'data': ['aes-128', 'aes-192', 'aes-256', 'des-rfb']}
+
+
+##
+# QCryptoCipherMode:
+#
+# The supported modes for content encryption ciphers
+#
+# @ecb: Electronic Code Book
+# @cbc: Cipher Block Chaining
+# Since: 2.6
+##
+{ 'enum': 'QCryptoCipherMode',
+  'prefix': 'QCRYPTO_CIPHER_MODE',
+  'data': ['ecb', 'cbc']}
-- 
2.5.0

[Qemu-devel] [PULL v1 5/6] crypto: ensure qapi/crypto.json is listed in qapi-modules

[Daniel P. Berrange]

The rebuild of qapi-types.c/h is not correctly triggered
when qapi/crypto.json is changed because it was missing
from the list of files in the qapi-modules variable.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index af3e5f1..82b2fc8 100644
--- a/Makefile
+++ b/Makefile
@@ -271,7 +271,8 @@ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
 
 qapi-modules = $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.json \
                $(SRC_PATH)/qapi/block.json $(SRC_PATH)/qapi/block-core.json \
-               $(SRC_PATH)/qapi/event.json $(SRC_PATH)/qapi/introspect.json
+               $(SRC_PATH)/qapi/event.json $(SRC_PATH)/qapi/introspect.json \
+               $(SRC_PATH)/qapi/crypto.json
 
 qapi-types.c qapi-types.h :\
 $(qapi-modules) $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
-- 
2.5.0

[Qemu-devel] [PULL v1 6/6] crypto: fix transposed arguments in cipher error message

[Daniel P. Berrange]

When reporting an incorrect key length for a cipher, we
mixed up the actual vs expected arguments.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/cipher.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/cipher.c b/crypto/cipher.c
index a24677c..7c33348 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -89,7 +89,7 @@ qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
 
     if (alg_key_len[alg] != nkey) {
         error_setg(errp, "Cipher key length %zu should be %zu",
-                   alg_key_len[alg], nkey);
+                   nkey, alg_key_len[alg]);
         return false;
     }
     return true;
-- 
2.5.0

Re: [Qemu-devel] [PULL v1 0/6] Misc crypto changes & fixes

[Peter Maydell]

On 23 December 2015 at 11:13, Daniel P. Berrange <berrange@redhat.com> wrote:
> The following changes since commit 5dc42c186d63b7b338594fc071cf290805dcc5a5:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2015-12-22 14:21:42 +0000)
>
> are available in the git repository at:
>
>   git://github.com/berrange/qemu tags/pull-crypto-fixes-2015-12-23-1
>
> for you to fetch changes up to 50de6261510301d586f0411b3fdb11e4cd4fdb52:
>
>   crypto: fix transposed arguments in cipher error message (2015-12-23 11:02:20 +0000)
>
> ----------------------------------------------------------------
> Merge misc crypto changes & fixes
>
> ----------------------------------------------------------------
> Daniel P. Berrange (6):
>       crypto: add additional query accessors for cipher instances
>       crypto: add ability to query hash digest len
>       crypto: move QCryptoHashAlgorithm enum definition into QAPI
>       crypto: move QCryptoCipherAlgorithm/Mode enum definitions into QAPI
>       crypto: ensure qapi/crypto.json is listed in qapi-modules
>       crypto: fix transposed arguments in cipher error message

Applied, thanks.

-- PMM