[Qemu-devel] [PULL v1 0/4] Merge qcrypto-next 2016/06/13

[Qemu-devel] [PULL v1 0/4] Merge qcrypto-next 2016/06/13

[Daniel P. Berrange]

The following changes since commit da2fdd0bd1514a44309dd5be162ebfb6c166a716:

  Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20160611' into staging (2016-06-13 10:12:44 +0100)

are available in the git repository at:

  git://github.com/berrange/qemu tags/qcrypto-next-2016-06-13-v1

for you to fetch changes up to c8d70e59738e672021926c7747af8ef9dea15c82:

  crypto: aes: always rename internal symbols (2016-06-13 12:41:17 +0100)

----------------------------------------------------------------
Merge qcrypto-next 2016/06/13 v1

----------------------------------------------------------------

Alex Bligh (1):
  TLS: provide slightly more information when TLS certificate loading
    fails

Daniel P. Berrange (1):
  crypto: remove temp files on completion of secrets test

Mike Frysinger (1):
  crypto: aes: always rename internal symbols

Paolo Bonzini (1):
  crypto: assert that qcrypto_hash_digest_len is in range

 crypto/hash.c              |  4 +---
 crypto/tlscredsx509.c      | 17 +++++++++++------
 include/crypto/aes.h       |  5 ++---
 tests/test-crypto-secret.c |  6 ++++--
 4 files changed, 18 insertions(+), 14 deletions(-)

-- 
2.5.5

[Qemu-devel] [PULL v1 1/4] TLS: provide slightly more information when TLS certificate loading fails

[Daniel P. Berrange]

From: Alex Bligh <alex@alex.org.uk>

Give slightly more information when certification loading fails.
Rather than have no information, you now get gnutls's only slightly
less unhelpful error messages.

Signed-off-by: Alex Bligh <alex@alex.org.uk>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/tlscredsx509.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 6a0179c..520d34d 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -392,11 +392,14 @@ qcrypto_tls_creds_load_cert(QCryptoTLSCredsX509 *creds,
     gsize buflen;
     GError *gerr;
     int ret = -1;
+    int err;
 
     trace_qcrypto_tls_creds_x509_load_cert(creds, isServer, certFile);
 
-    if (gnutls_x509_crt_init(&cert) < 0) {
-        error_setg(errp, "Unable to initialize certificate");
+    err = gnutls_x509_crt_init(&cert);
+    if (err < 0) {
+        error_setg(errp, "Unable to initialize certificate: %s",
+                   gnutls_strerror(err));
         goto cleanup;
     }
 
@@ -410,11 +413,13 @@ qcrypto_tls_creds_load_cert(QCryptoTLSCredsX509 *creds,
     data.data = (unsigned char *)buf;
     data.size = strlen(buf);
 
-    if (gnutls_x509_crt_import(cert, &data, GNUTLS_X509_FMT_PEM) < 0) {
+    err = gnutls_x509_crt_import(cert, &data, GNUTLS_X509_FMT_PEM);
+    if (err < 0) {
         error_setg(errp, isServer ?
-                   "Unable to import server certificate %s" :
-                   "Unable to import client certificate %s",
-                   certFile);
+                   "Unable to import server certificate %s: %s" :
+                   "Unable to import client certificate %s: %s",
+                   certFile,
+                   gnutls_strerror(err));
         goto cleanup;
     }
 
-- 
2.5.5

[Qemu-devel] [PULL v1 2/4] crypto: remove temp files on completion of secrets test

[Daniel P. Berrange]

The secret object tests left some temporary files on disk
when completing. Ensure they are unlink, and rename them
to make it more obvious where they come from.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 tests/test-crypto-secret.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/test-crypto-secret.c b/tests/test-crypto-secret.c
index 0b1fe8d..13fc6c4 100644
--- a/tests/test-crypto-secret.c
+++ b/tests/test-crypto-secret.c
@@ -49,7 +49,7 @@ static void test_secret_indirect_good(void)
 {
     Object *sec;
     char *fname = NULL;
-    int fd = g_file_open_tmp("secretXXXXXX",
+    int fd = g_file_open_tmp("qemu-test-crypto-secret-XXXXXX",
                              &fname,
                              NULL);
 
@@ -74,6 +74,7 @@ static void test_secret_indirect_good(void)
     object_unparent(sec);
     g_free(pw);
     close(fd);
+    unlink(fname);
     g_free(fname);
 }
 
@@ -96,7 +97,7 @@ static void test_secret_indirect_emptyfile(void)
 {
     Object *sec;
     char *fname = NULL;
-    int fd = g_file_open_tmp("secretXXXXXX",
+    int fd = g_file_open_tmp("qemu-test-crypto-secretXXXXXX",
                              &fname,
                              NULL);
 
@@ -119,6 +120,7 @@ static void test_secret_indirect_emptyfile(void)
     object_unparent(sec);
     g_free(pw);
     close(fd);
+    unlink(fname);
     g_free(fname);
 }
 
-- 
2.5.5

[Qemu-devel] [PULL v1 3/4] crypto: assert that qcrypto_hash_digest_len is in range

[Daniel P. Berrange]

From: Paolo Bonzini <pbonzini@redhat.com>

Otherwise unintended results could happen.  For example,
Coverity reports a division by zero in qcrypto_afsplit_hash.
While this cannot really happen, it shows that the contract
of qcrypto_hash_digest_len can be improved.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/hash.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/crypto/hash.c b/crypto/hash.c
index b90af34..2907bff 100644
--- a/crypto/hash.c
+++ b/crypto/hash.c
@@ -36,9 +36,7 @@ static size_t qcrypto_hash_alg_size[QCRYPTO_HASH_ALG__MAX] = {
 
 size_t qcrypto_hash_digest_len(QCryptoHashAlgorithm alg)
 {
-    if (alg >= G_N_ELEMENTS(qcrypto_hash_alg_size)) {
-        return 0;
-    }
+    assert(alg < G_N_ELEMENTS(qcrypto_hash_alg_size));
     return qcrypto_hash_alg_size[alg];
 }
 
-- 
2.5.5

[Qemu-devel] [PULL v1 4/4] crypto: aes: always rename internal symbols

[Daniel P. Berrange]

From: Mike Frysinger <vapier@chromium.org>

OpenSSL's libcrypto always defines AES symbols with the same names as
qemu's local aes code.  This is problematic when enabling at least curl
as that frequently also uses libcrypto.  It might not be noticed when
running, but if you try to statically link, everything falls down.

An example snippet:
  LINK  qemu-nbd
.../libcrypto.a(aes-x86_64.o): In function 'AES_encrypt':
(.text+0x460): multiple definition of 'AES_encrypt'
crypto/aes.o:aes.c:(.text+0x670): first defined here
.../libcrypto.a(aes-x86_64.o): In function 'AES_decrypt':
(.text+0x9f0): multiple definition of 'AES_decrypt'
crypto/aes.o:aes.c:(.text+0xb30): first defined here
.../libcrypto.a(aes-x86_64.o): In function 'AES_cbc_encrypt':
(.text+0xf90): multiple definition of 'AES_cbc_encrypt'
crypto/aes.o:aes.c:(.text+0xff0): first defined here
collect2: error: ld returned 1 exit status
.../qemu-2.6.0/rules.mak:105: recipe for target 'qemu-nbd' failed
make: *** [qemu-nbd] Error 1

The aes.h header has redefines already for FreeBSD, but go ahead and
enable that for everyone since there's no real good reason to not use
a namespace all the time.

Signed-off-by: Mike Frysinger <vapier@chromium.org>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/crypto/aes.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/crypto/aes.h b/include/crypto/aes.h
index a006da2..12fb321 100644
--- a/include/crypto/aes.h
+++ b/include/crypto/aes.h
@@ -10,14 +10,13 @@ struct aes_key_st {
 };
 typedef struct aes_key_st AES_KEY;
 
-/* FreeBSD has its own AES_set_decrypt_key in -lcrypto, avoid conflicts */
-#ifdef __FreeBSD__
+/* FreeBSD/OpenSSL have their own AES functions with the same names in -lcrypto
+ * (which might be pulled in via curl), so redefine to avoid conflicts. */
 #define AES_set_encrypt_key QEMU_AES_set_encrypt_key
 #define AES_set_decrypt_key QEMU_AES_set_decrypt_key
 #define AES_encrypt QEMU_AES_encrypt
 #define AES_decrypt QEMU_AES_decrypt
 #define AES_cbc_encrypt QEMU_AES_cbc_encrypt
-#endif
 
 int AES_set_encrypt_key(const unsigned char *userKey, const int bits,
 	AES_KEY *key);
-- 
2.5.5

Re: [Qemu-devel] [PULL v1 0/4] Merge qcrypto-next 2016/06/13

[Peter Maydell]

On 13 June 2016 at 12:45, Daniel P. Berrange <berrange@redhat.com> wrote:
> The following changes since commit da2fdd0bd1514a44309dd5be162ebfb6c166a716:
>
>   Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20160611' into staging (2016-06-13 10:12:44 +0100)
>
> are available in the git repository at:
>
>   git://github.com/berrange/qemu tags/qcrypto-next-2016-06-13-v1
>
> for you to fetch changes up to c8d70e59738e672021926c7747af8ef9dea15c82:
>
>   crypto: aes: always rename internal symbols (2016-06-13 12:41:17 +0100)
>
> ----------------------------------------------------------------
> Merge qcrypto-next 2016/06/13 v1
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM