[Qemu-devel] [PATCH v2 0/3] crypto: nettle fixes

[Qemu-devel] [PATCH v2 0/3] crypto: nettle fixes

[Radim Krčmář]

[1/3] fixes a build problem with new nettle libraries.
Peter Maydell found a potential undefined behavior with some architectures that
is fixed in [2/3].  (I haven't checked if QEMU runs on one.)
[3/3] refactors [2/3];  feel free to squish or drop it.

Radim Krčmář (3):
  crypto: fix build with nettle >= 3.0.0
  crypto: avoid undefined behavior in nettle calls
  crypto: use CPP for wrapper definitions in nettle

 configure              |  4 +++-
 crypto/cipher-nettle.c | 38 ++++++++++++++++++++++++++++++++------
 2 files changed, 35 insertions(+), 7 deletions(-)

-- 
2.4.5

[Qemu-devel] [PATCH v2 1/3] crypto: fix build with nettle >= 3.0.0

[Radim Krčmář]

In nettle 3, cbc_encrypt() accepts 'nettle_cipher_func' instead of
'nettle_crypt_func' and these two differ in 'const' qualifier of the
first argument.  The build fails with:

  In file included from crypto/cipher.c:71:0:
  ./crypto/cipher-nettle.c: In function ‘qcrypto_cipher_encrypt’:
  ./crypto/cipher-nettle.c:154:38: error: passing argument 2 of
  ‘nettle_cbc_encrypt’ from incompatible pointer type
           cbc_encrypt(ctx->ctx_encrypt, ctx->alg_encrypt,
                                               ^
  In file included from ./crypto/cipher-nettle.c:24:0,
                   from crypto/cipher.c:71:
  /usr/include/nettle/cbc.h:48:1: note: expected
  ‘void (*)(const void *, size_t, uint8_t *, const uint8_t *)
  but argument is of type
  ‘void (*)(      void *, size_t, uint8_t *, const uint8_t *)

To allow both versions, we switch to the new definition and #if typedef
it for old versions.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 configure              |  4 +++-
 crypto/cipher-nettle.c | 16 ++++++++++------
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/configure b/configure
index 33b945530e64..cc0338ddbd14 100755
--- a/configure
+++ b/configure
@@ -2183,6 +2183,7 @@ if test "$gnutls_nettle" != "no"; then
     if $pkg_config --exists "nettle"; then
         nettle_cflags=`$pkg_config --cflags nettle`
         nettle_libs=`$pkg_config --libs nettle`
+        nettle_version=`$pkg_config --modversion nettle`
         libs_softmmu="$nettle_libs $libs_softmmu"
         libs_tools="$nettle_libs $libs_tools"
         QEMU_CFLAGS="$QEMU_CFLAGS $nettle_cflags"
@@ -4490,7 +4491,7 @@ echo "GTK support       $gtk"
 echo "GNUTLS support    $gnutls"
 echo "GNUTLS hash       $gnutls_hash"
 echo "GNUTLS gcrypt     $gnutls_gcrypt"
-echo "GNUTLS nettle     $gnutls_nettle"
+echo "GNUTLS nettle     $gnutls_nettle ${gnutls_nettle+($nettle_version)}"
 echo "VTE support       $vte"
 echo "curses support    $curses"
 echo "curl support      $curl"
@@ -4858,6 +4859,7 @@ if test "$gnutls_gcrypt" = "yes" ; then
 fi
 if test "$gnutls_nettle" = "yes" ; then
   echo "CONFIG_GNUTLS_NETTLE=y" >> $config_host_mak
+  echo "CONFIG_NETTLE_VERSION_MAJOR=${nettle_version%%.*}" >> $config_host_mak
 fi
 if test "$vte" = "yes" ; then
   echo "CONFIG_VTE=y" >> $config_host_mak
diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index e5a14bc1393f..e61aaa29f049 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -23,12 +23,16 @@
 #include <nettle/des.h>
 #include <nettle/cbc.h>
 
+#if CONFIG_NETTLE_VERSION_MAJOR < 3
+typedef nettle_crypt_func nettle_cipher_func;
+#endif
+
 typedef struct QCryptoCipherNettle QCryptoCipherNettle;
 struct QCryptoCipherNettle {
     void *ctx_encrypt;
     void *ctx_decrypt;
-    nettle_crypt_func *alg_encrypt;
-    nettle_crypt_func *alg_decrypt;
+    nettle_cipher_func *alg_encrypt;
+    nettle_cipher_func *alg_decrypt;
     uint8_t *iv;
     size_t niv;
 };
@@ -83,8 +87,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         des_set_key(ctx->ctx_encrypt, rfbkey);
         g_free(rfbkey);
 
-        ctx->alg_encrypt = (nettle_crypt_func *)des_encrypt;
-        ctx->alg_decrypt = (nettle_crypt_func *)des_decrypt;
+        ctx->alg_encrypt = (nettle_cipher_func *)des_encrypt;
+        ctx->alg_decrypt = (nettle_cipher_func *)des_decrypt;
 
         ctx->niv = DES_BLOCK_SIZE;
         break;
@@ -98,8 +102,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         aes_set_encrypt_key(ctx->ctx_encrypt, nkey, key);
         aes_set_decrypt_key(ctx->ctx_decrypt, nkey, key);
 
-        ctx->alg_encrypt = (nettle_crypt_func *)aes_encrypt;
-        ctx->alg_decrypt = (nettle_crypt_func *)aes_decrypt;
+        ctx->alg_encrypt = (nettle_cipher_func *)aes_encrypt;
+        ctx->alg_decrypt = (nettle_cipher_func *)aes_decrypt;
 
         ctx->niv = AES_BLOCK_SIZE;
         break;
-- 
2.4.5

[Qemu-devel] [PATCH v2 2/3] crypto: avoid undefined behavior in nettle calls

[Radim Krčmář]

Calling a function pointer that was cast from an incompatible function
results in undefined behavior.  'void *' isn't compatible with 'struct
XXX *', so we can't cast to nettle_cipher_func, but have to provide a
wrapper.  (Conversion from 'void *' to 'struct XXX *' might require
computation, which won't be done if we drop argument's true type, and
pointers can have different sizes so passing arguments on stack would
bug.)

Having two different prototypes based on nettle version doesn't make
this solution any nicer.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 crypto/cipher-nettle.c | 43 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index e61aaa29f049..61f1cd3417d3 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -25,8 +25,43 @@
 
 #if CONFIG_NETTLE_VERSION_MAJOR < 3
 typedef nettle_crypt_func nettle_cipher_func;
+
+typedef void *       cipher_ctx_t;
+typedef unsigned     cipher_length_t;
+#else
+typedef const void * cipher_ctx_t;
+typedef size_t       cipher_length_t;
 #endif
 
+nettle_cipher_func aes_encrypt_wrapper;
+nettle_cipher_func aes_decrypt_wrapper;
+nettle_cipher_func des_encrypt_wrapper;
+nettle_cipher_func des_decrypt_wrapper;
+
+void aes_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+                         uint8_t *dst, const uint8_t *src)
+{
+    aes_encrypt(ctx, length, dst, src);
+}
+
+void aes_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+                         uint8_t *dst, const uint8_t *src)
+{
+    aes_encrypt(ctx, length, dst, src);
+}
+
+void des_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+                         uint8_t *dst, const uint8_t *src)
+{
+    des_encrypt(ctx, length, dst, src);
+}
+
+void des_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+                         uint8_t *dst, const uint8_t *src)
+{
+    des_decrypt(ctx, length, dst, src);
+}
+
 typedef struct QCryptoCipherNettle QCryptoCipherNettle;
 struct QCryptoCipherNettle {
     void *ctx_encrypt;
@@ -87,8 +122,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         des_set_key(ctx->ctx_encrypt, rfbkey);
         g_free(rfbkey);
 
-        ctx->alg_encrypt = (nettle_cipher_func *)des_encrypt;
-        ctx->alg_decrypt = (nettle_cipher_func *)des_decrypt;
+        ctx->alg_encrypt = des_encrypt_wrapper;
+        ctx->alg_decrypt = des_decrypt_wrapper;
 
         ctx->niv = DES_BLOCK_SIZE;
         break;
@@ -102,8 +137,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         aes_set_encrypt_key(ctx->ctx_encrypt, nkey, key);
         aes_set_decrypt_key(ctx->ctx_decrypt, nkey, key);
 
-        ctx->alg_encrypt = (nettle_cipher_func *)aes_encrypt;
-        ctx->alg_decrypt = (nettle_cipher_func *)aes_decrypt;
+        ctx->alg_encrypt = aes_encrypt_wrapper;
+        ctx->alg_decrypt = aes_decrypt_wrapper;
 
         ctx->niv = AES_BLOCK_SIZE;
         break;
-- 
2.4.5

Re: [Qemu-devel] [PATCH v2 2/3] crypto: avoid undefined behavior in nettle calls

[Radim Krčmář]

2015-07-10 19:18+0200, Radim Krčmář:
> diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
> +void aes_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
> +                         uint8_t *dst, const uint8_t *src)

Wrappers should be static (rushed posting).  I'll send v3 on Wed/Thu.

> +{
> +    aes_encrypt(ctx, length, dst, src);
> +}

[Qemu-devel] [PATCH v2 3/3] crypto: use CPP for wrapper definitions in nettle

[Radim Krčmář]

It's horrible both ways and I prefer this one.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 crypto/cipher-nettle.c | 47 +++++++++++++++++------------------------------
 1 file changed, 17 insertions(+), 30 deletions(-)

diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index 61f1cd3417d3..78b6b05a27d2 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -33,34 +33,21 @@ typedef const void * cipher_ctx_t;
 typedef size_t       cipher_length_t;
 #endif
 
-nettle_cipher_func aes_encrypt_wrapper;
-nettle_cipher_func aes_decrypt_wrapper;
-nettle_cipher_func des_encrypt_wrapper;
-nettle_cipher_func des_decrypt_wrapper;
+#define WRAP(cipher) \
+    nettle_cipher_func cipher##_wrapper; \
+    void cipher##_wrapper(cipher_ctx_t ctx, cipher_length_t length, \
+                          uint8_t *dst, const uint8_t *src) \
+    { \
+        cipher(ctx, length, dst, src); \
+    }
 
-void aes_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
-                         uint8_t *dst, const uint8_t *src)
-{
-    aes_encrypt(ctx, length, dst, src);
-}
+#define WRAPPED(cipher) \
+    cipher##_wrapper
 
-void aes_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
-                         uint8_t *dst, const uint8_t *src)
-{
-    aes_encrypt(ctx, length, dst, src);
-}
-
-void des_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
-                         uint8_t *dst, const uint8_t *src)
-{
-    des_encrypt(ctx, length, dst, src);
-}
-
-void des_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
-                         uint8_t *dst, const uint8_t *src)
-{
-    des_decrypt(ctx, length, dst, src);
-}
+WRAP(aes_encrypt)
+WRAP(aes_decrypt)
+WRAP(des_encrypt)
+WRAP(des_decrypt)
 
 typedef struct QCryptoCipherNettle QCryptoCipherNettle;
 struct QCryptoCipherNettle {
@@ -122,8 +109,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         des_set_key(ctx->ctx_encrypt, rfbkey);
         g_free(rfbkey);
 
-        ctx->alg_encrypt = des_encrypt_wrapper;
-        ctx->alg_decrypt = des_decrypt_wrapper;
+        ctx->alg_encrypt = WRAPPED(des_encrypt);
+        ctx->alg_decrypt = WRAPPED(des_decrypt);
 
         ctx->niv = DES_BLOCK_SIZE;
         break;
@@ -137,8 +124,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         aes_set_encrypt_key(ctx->ctx_encrypt, nkey, key);
         aes_set_decrypt_key(ctx->ctx_decrypt, nkey, key);
 
-        ctx->alg_encrypt = aes_encrypt_wrapper;
-        ctx->alg_decrypt = aes_decrypt_wrapper;
+        ctx->alg_encrypt = WRAPPED(aes_encrypt);
+        ctx->alg_decrypt = WRAPPED(aes_decrypt);
 
         ctx->niv = AES_BLOCK_SIZE;
         break;
-- 
2.4.5