[Qemu-devel] [PATCH 0/6] Misc enhancements to crypto APIs

[Qemu-devel] [PATCH 0/6] Misc enhancements to crypto APIs

[Daniel P. Berrange]

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(-)

-- 
2.5.0

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

[Daniel P. Berrange]

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

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

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

On 12/21/2015 09:06 AM, Daniel P. Berrange wrote:
> 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

> 
> 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(+)
> 

But no problems with the actual patch, so:
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Daniel P. Berrange]

On Mon, Dec 21, 2015 at 09:18:42AM -0700, Eric Blake wrote:
> On 12/21/2015 09:06 AM, Daniel P. Berrange wrote:
> > 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"

> 
> > 
> > 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(+)
> > 
> 
> But no problems with the actual patch, so:
> Reviewed-by: Eric Blake <eblake@redhat.com>


Regards,
Daniel
-- 
|: 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 :|

[Qemu-devel] [PATCH 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.

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..41721d0 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((digestsize * 2) == strlen(expected_outputs[i]));
 
         ret = qcrypto_hash_digest(i,
                                   INPUT_TEXT,
-- 
2.5.0

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]

On 12/21/2015 09:06 AM, Daniel P. Berrange wrote:
> 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 <berrange@redhat.com>
> ---
>  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
> @@ -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 <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Daniel P. Berrange]

On Mon, Dec 21, 2015 at 09:22:42AM -0700, Eric Blake wrote:
> On 12/21/2015 09:06 AM, Daniel P. Berrange wrote:
> > 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 <berrange@redhat.com>
> > ---
> >  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
> > @@ -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.

> Reviewed-by: Eric Blake <eblake@redhat.com>

Regards,
Daniel
-- 
|: 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 :|

[Qemu-devel] [PATCH 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.

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..b5f81c4 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

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1959 bytes --]

On 12/21/2015 09:06 AM, Daniel P. Berrange wrote:
> 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 <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..b5f81c4 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] = {

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


> +++ 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']}

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 <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Daniel P. Berrange]

On Mon, Dec 21, 2015 at 09:27:15AM -0700, Eric Blake wrote:
> On 12/21/2015 09:06 AM, Daniel P. Berrange wrote:
> > 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 <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..b5f81c4 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] = {
> 
> Will need rebasing, now that commit 7fb1cf1 has landed and renamed it
> QCRYPTO_HASH_ALG__MAX.
> 
> 
> > +++ 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']}
> 
> 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...

> Reviewed-by: Eric Blake <eblake@redhat.com>

Regards,
Daniel
-- 
|: 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 :|

[Qemu-devel] [PATCH 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.

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..e92d49a 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

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]

On 12/21/2015 09:06 AM, Daniel P. Berrange wrote:
> 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 <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..e92d49a 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] = {

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

Otherwise looks fine.
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [PATCH 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.

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

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1179 bytes --]

On 12/21/2015 09:06 AM, Daniel P. Berrange wrote:
> 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 <berrange@redhat.com>
> ---
>  Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> 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

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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Daniel P. Berrange]

On Mon, Dec 21, 2015 at 09:32:42AM -0700, Eric Blake wrote:
> On 12/21/2015 09:06 AM, Daniel P. Berrange wrote:
> > 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 <berrange@redhat.com>
> > ---
> >  Makefile | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> > 
> > 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
> 
> 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
-- 
|: 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 :|

[Qemu-devel] [PATCH 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.

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 e92d49a..a69ff5e 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] [PATCH 6/6] crypto: fix transposed arguments in cipher error message

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 954 bytes --]

On 12/21/2015 09:06 AM, Daniel P. Berrange wrote:
> When reporting an incorrect key length for a cipher, we
> mixed up the actual vs expected arguments.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  crypto/cipher.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/crypto/cipher.c b/crypto/cipher.c
> index e92d49a..a69ff5e 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;
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]