From b7b68166dcbadb1c207b4b6f25b23a18a292da2d Mon Sep 17 00:00:00 2001 From: Alex Bligh Date: Tue, 5 Apr 2016 20:33:48 +0100 Subject: [PATCH 1/4] TLS: provide slightly more information when TLS certificate loading fails 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 Signed-off-by: Daniel P. Berrange --- crypto/tlscredsx509.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c index 6a0179c2e1..520d34d77e 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; } From e7ed11f083015bf34a121cfff31540cf9c2bae23 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Tue, 26 Apr 2016 10:59:09 +0100 Subject: [PATCH 2/4] crypto: remove temp files on completion of secrets test 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 --- 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 0b1fe8dd37..13fc6c4c75 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); } From b35c1f3361ebf6ec9ea5022903af4b559bff6063 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 20 May 2016 11:09:54 +0200 Subject: [PATCH 3/4] crypto: assert that qcrypto_hash_digest_len is in range 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 Signed-off-by: Paolo Bonzini Signed-off-by: Daniel P. Berrange --- crypto/hash.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crypto/hash.c b/crypto/hash.c index b90af3495a..2907bffd2e 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]; } From c8d70e59738e672021926c7747af8ef9dea15c82 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Mon, 6 Jun 2016 18:05:35 -0400 Subject: [PATCH 4/4] crypto: aes: always rename internal symbols 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 Signed-off-by: Daniel P. Berrange --- 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 a006da2224..12fb321b89 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);