Skip to contentSkip to conversation

Misc enhancements to crypto APIs

PATCH In review Daniel P. Berrange 2 participants 17 messages Dec 21, 2015 – Dec 23, 2015

Subsystems (2)
Kbuild Crypto API
lore
Share & tools
lore

Daniel P. Berrange

author
#0 lore raw

cover Misc enhancements to crypto APIs

This is a small series of misc enhancements to the crypto APIs. A couple of helper methods are added for getting data sizes, some enums are converted to use QAPI schema and a couple of minor bugs are squashed

Daniel P. Berrange (6):
crypto: add ability to query the cipher key, block & IV lens
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(-)
Signature · 1 line
2.5.0
author
#1 lore raw

PATCH 1/6 crypto: add ability to query the cipher key, block & IV lens

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

Signed-off-by Daniel P. Berrange <[email protected]>
---
 crypto/cipher.c            | 48 ++++++++++++++++++++++++++++++++++++++++++++++
 include/crypto/cipher.h    | 37 +++++++++++++++++++++++++++++++++++
 tests/test-crypto-cipher.c | 10 ++++++++++
 3 files changed, 95 insertions(+)
crypto/cipher.c +48 -0
index c8bd180..d02bb32 100644
@@ -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,
include/crypto/cipher.h +37 -0
index b4d714f..aa51c89 100644
@@ -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:
tests/test-crypto-cipher.c +10 -0
index f4946a0..c687307 100644
@@ -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,
Signature · 1 line
2.5.0
Reviewed-by on v1 1/6
#2 lore raw

Re: [Qemu-devel] [PATCH 1/6] crypto: add ability to query the cipher key, block & IV lens

Quoted 2 lines · Daniel P. Berrange

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

In the subject line, I read 'lens' as a synonym for 'viewports', not 'lengths'. But I don't know if avoiding the abbreviation is worth it, because the subject line is already bordering on long. Maybe avoiding it altogether is easier?

crypto: Add additional query accessors

Quoted 8 lines
Signed-off-by Daniel P. Berrange <[email protected]>
---
 crypto/cipher.c            | 48 ++++++++++++++++++++++++++++++++++++++++++++++
 include/crypto/cipher.h    | 37 +++++++++++++++++++++++++++++++++++
 tests/test-crypto-cipher.c | 10 ++++++++++
 3 files changed, 95 insertions(+)

But no problems with the actual patch, so:

Reviewed-by Eric Blake <[email protected]>
Signature · 2 lines
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org
author
#3 lore raw

Re: [Qemu-devel] [PATCH 1/6] crypto: add ability to query the cipher key, block & IV lens

Quoted 10 lines · Eric Blake
Quoted 2 lines · Daniel P. Berrange

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

In the subject line, I read 'lens' as a synonym for 'viewports', not 'lengths'. But I don't know if avoiding the abbreviation is worth it, because the subject line is already bordering on long. Maybe avoiding it altogether is easier?

crypto: Add additional query accessors

Yes, I'll simplify this to

"crypto: add additional query accessors for cipher instances"

Quoted 10 lines
Quoted 9 lines
Signed-off-by Daniel P. Berrange <[email protected]>
---
 crypto/cipher.c            | 48 ++++++++++++++++++++++++++++++++++++++++++++++
 include/crypto/cipher.h    | 37 +++++++++++++++++++++++++++++++++++
 tests/test-crypto-cipher.c | 10 ++++++++++
 3 files changed, 95 insertions(+)
Quoted 2 lines

But no problems with the actual patch, so:

Reviewed-by Eric Blake <[email protected]>

Regards,
Daniel

Signature · 4 lines
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
author
#4 lore raw

PATCH 2/6 crypto: add ability to query hash digest len

In reply to Daniel P. Berrange

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

Signed-off-by Daniel P. Berrange <[email protected]>
---
 crypto/hash.c            | 15 +++++++++++++++
 include/crypto/hash.h    | 11 +++++++++++
 tests/test-crypto-hash.c |  5 +++++
 3 files changed, 31 insertions(+)
crypto/hash.c +15 -0
index 81e74de..5a47b90 100644
@@ -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,
include/crypto/hash.h +11 -0
index b5acbf6..3d18124 100644
@@ -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
tests/test-crypto-hash.c +5 -0
index 911437e..41721d0 100644
@@ -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((digestsize * 2) == strlen(expected_outputs[i]));
 
         ret = qcrypto_hash_digest(i,
                                   INPUT_TEXT,
Signature · 1 line
2.5.0
Reviewed-by on v1 2/6
#5 lore raw

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

Quoted 13 lines · Daniel P. Berrange

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

Signed-off-by Daniel P. Berrange <[email protected]>
---
 crypto/hash.c            | 15 +++++++++++++++
 include/crypto/hash.h    | 11 +++++++++++
 tests/test-crypto-hash.c |  5 +++++
 3 files changed, 31 insertions(+)

+++ b/tests/test-crypto-hash.c

tests/test-crypto-hash.c static void test_hash_digest(void) · line 170

Quoted 8 lines
tests/test-crypto-hash.c +5 -0
@@ -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((digestsize * 2) == strlen(expected_outputs[i]));

g_assert_cmpint() might be better here. But that's minor.

Reviewed-by Eric Blake <[email protected]>
Signature · 2 lines
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org
author
#6 lore raw

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

Quoted 12 lines · Eric Blake
Quoted 10 lines · Daniel P. Berrange

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

Signed-off-by Daniel P. Berrange <[email protected]>
---
 crypto/hash.c            | 15 +++++++++++++++
 include/crypto/hash.h    | 11 +++++++++++
 tests/test-crypto-hash.c |  5 +++++
 3 files changed, 31 insertions(+)
Quoted 12 lines

tests/test-crypto-hash.c static void test_hash_digest(void) · line 170

Quoted 10 lines

+++ b/tests/test-crypto-hash.c

tests/test-crypto-hash.c +5 -0
@@ -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((digestsize * 2) == strlen(expected_outputs[i]));

g_assert_cmpint() might be better here. But that's minor.

Ok, I've changed that.

Quoted 1 line
Reviewed-by Eric Blake <[email protected]>

Regards,
Daniel

Signature · 4 lines
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
author
#7 lore raw

PATCH 3/6 crypto: move QCryptoHashAlgorithm enum definition into QAPI

In reply to 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.

Signed-off-by Daniel P. Berrange <[email protected]>
---
 crypto/hash.c         |  4 ++--
 include/crypto/hash.h |  9 +--------
 qapi/crypto.json      | 15 +++++++++++++++
 3 files changed, 18 insertions(+), 10 deletions(-)
crypto/hash.c +2 -2
index 5a47b90..b5f81c4 100644
@@ -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,
include/crypto/hash.h +1 -8
index 3d18124..41822c0 100644
@@ -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:
qapi/crypto.json +15 -0
index 4012659..0706ded 100644
@@ -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']}
Signature · 1 line
2.5.0
Reviewed-by on v1 3/6
#8 lore raw

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

Quoted 16 lines · 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.

Signed-off-by Daniel P. Berrange <[email protected]>
---
 crypto/hash.c         |  4 ++--
 include/crypto/hash.h |  9 +--------
 qapi/crypto.json      | 15 +++++++++++++++
 3 files changed, 18 insertions(+), 10 deletions(-)
crypto/hash.c +0 -0
index 5a47b90..b5f81c4 100644

crypto/hash.c line 27

Quoted 5 lines
crypto/hash.c +1 -1
@@ -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] = {

Will need rebasing, now that commit 7fb1cf1 has landed and renamed it QCRYPTO_HASH_ALG__MAX.

Quoted 10 lines
crypto/hash.c +6 -0
+++ b/qapi/crypto.json

@@ -33,3 +33,18 @@
 { 'enum': 'QCryptoSecretFormat',
   'prefix': 'QCRYPTO_SECRET_FORMAT',
   'data': ['raw', 'base64']}
+
+
+##
+# QCryptoHashAlgorithm:
+#

crypto/hash.c line 50

Quoted 10 lines
crypto/hash.c +10 -0
+# 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']}

Otherwise looks fine. I know Markus is debating about getting rid of use of 'prefix', and this just makes that task harder, but the current munging would produce Q_CRYPTO_... even if you named the enum QCryptoHashAlg, so I'm okay with it.

Reviewed-by Eric Blake <[email protected]>
Signature · 2 lines
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org
author
#9 lore raw

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

Quoted 44 lines · Eric Blake
Quoted 16 lines · 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.

Signed-off-by Daniel P. Berrange <[email protected]>
---
 crypto/hash.c         |  4 ++--
 include/crypto/hash.h |  9 +--------
 qapi/crypto.json      | 15 +++++++++++++++
 3 files changed, 18 insertions(+), 10 deletions(-)
crypto/hash.c +0 -0
index 5a47b90..b5f81c4 100644

crypto/hash.c line 27

Quoted 5 lines
crypto/hash.c +1 -1
@@ -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] = {

Will need rebasing, now that commit 7fb1cf1 has landed and renamed it QCRYPTO_HASH_ALG__MAX.

Quoted 17 lines
crypto/hash.c +13 -0
+++ 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
+##
Quoted 8 lines

crypto/hash.c line 50

Quoted 3 lines
crypto/hash.c +3 -0
+{ 'enum': 'QCryptoHashAlgorithm',
+  'prefix': 'QCRYPTO_HASH_ALG',
+  'data': ['md5', 'sha1', 'sha256']}

Otherwise looks fine. I know Markus is debating about getting rid of use of 'prefix', and this just makes that task harder, but the current munging would produce Q_CRYPTO_... even if you named the enum QCryptoHashAlg, so I'm okay with it.

If we do decide to kill "prefix" I'm happy to help cleanup the fallout, since its all in code I've added...

Quoted 1 line
Reviewed-by Eric Blake <[email protected]>

Regards,
Daniel

Signature · 4 lines
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
author
#10 lore raw

PATCH 4/6 crypto: move QCryptoCipherAlgorithm/Mode enum definitions into QAPI

In reply to 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.

Signed-off-by Daniel P. Berrange <[email protected]>
---
 crypto/cipher.c         |  8 ++++----
 include/crypto/cipher.h | 17 ++---------------
 qapi/crypto.json        | 30 ++++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 19 deletions(-)
crypto/cipher.c +4 -4
index d02bb32..e92d49a 100644
@@ -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;
include/crypto/cipher.h +2 -15
index aa51c89..a812803 100644
@@ -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:
qapi/crypto.json +30 -0
index 0706ded..4bd690f 100644
@@ -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']}
Signature · 1 line
2.5.0
Reviewed-by on v1 4/6
#11 lore raw

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

Quoted 17 lines · 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.

Signed-off-by Daniel P. Berrange <[email protected]>
---
 crypto/cipher.c         |  8 ++++----
 include/crypto/cipher.h | 17 ++---------------
 qapi/crypto.json        | 30 ++++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 19 deletions(-)
crypto/cipher.c +0 -0
index d02bb32..e92d49a 100644

crypto/cipher.c line 24

Quoted 5 lines
crypto/cipher.c +1 -1
@@ -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] = {

Will need rebasing, now that commit 7fb1cf1 has landed and renamed it QCRYPTO_CIPHER_ALG__MAX.

Otherwise looks fine.

Reviewed-by Eric Blake <[email protected]>
Signature · 2 lines
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org
author
#12 lore raw

PATCH 5/6 crypto: ensure qapi/crypto.json is listed in qapi-modules

In reply to 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.

Signed-off-by Daniel P. Berrange <[email protected]>
---
 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Makefile +2 -1
index af3e5f1..82b2fc8 100644
@@ -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)
Signature · 1 line
2.5.0
Reviewed-by on v1 5/6
#13 lore raw

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

Quoted 8 lines · 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.

Signed-off-by Daniel P. Berrange <[email protected]>
---
 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by Eric Blake <[email protected]>

Makefile $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) · line 275

Quoted 12 lines
Makefile +2 -1
index af3e5f1..82b2fc8 100644
@@ -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

Wonder if a wildcard for qapi/*.json would be any smarter.

Signature · 2 lines
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org
author
#14 lore raw

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

Quoted 19 lines · Eric Blake
Quoted 8 lines · 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.

Signed-off-by Daniel P. Berrange <[email protected]>
---
 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by Eric Blake <[email protected]>
Quoted 7 lines
Makefile +0 -0
index af3e5f1..82b2fc8 100644
@@ -271,7 +271,8 @@ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
Quoted 7 lines

Makefile $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) · line 275

Quoted 5 lines
Makefile +2 -1
 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

Wonder if a wildcard for qapi/*.json would be any smarter.

I'm not sure if there's an official position, but it seems we mostly avoid using wildcards in the QEMU Makefiles, so I figured to stick with current practice.

Regards,
Daniel

Signature · 4 lines
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
author
#15 lore raw

PATCH 6/6 crypto: fix transposed arguments in cipher error message

In reply to Daniel P. Berrange

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

Signed-off-by Daniel P. Berrange <[email protected]>
---
 crypto/cipher.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
crypto/cipher.c +1 -1
index e92d49a..a69ff5e 100644
@@ -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;
Signature · 1 line
2.5.0
Reviewed-by on v1 6/6
#16 lore raw

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

Quoted 7 lines · Daniel P. Berrange

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

Signed-off-by Daniel P. Berrange <[email protected]>
---
 crypto/cipher.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by Eric Blake <[email protected]>
Quoted 15 lines
crypto/cipher.c +1 -1
index e92d49a..a69ff5e 100644
@@ -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;
Signature · 2 lines
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org