* [Qemu-devel] [PULL 0/4] Qcrypto next patches
@ 2018-07-24 16:44 Daniel P. Berrangé
2018-07-24 16:44 ` [Qemu-devel] [PULL 1/4] tests: call qcrypto_init instead of gnutls_global_init Daniel P. Berrangé
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2018-07-24 16:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Peter Maydell
The following changes since commit 3bae150448dbd888a480f892ebbf01caec0d8329:
Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2018-07-24 15:26:01 +0100)
are available in the Git repository at:
https://github.com/berrange/qemu tags/qcrypto-next-pull-request
for you to fetch changes up to db0a8c70f25fe497c4b786d8edac063daa744c0d:
tests: fix TLS handshake failure with TLS 1.3 (2018-07-24 17:36:12 +0100)
----------------------------------------------------------------
Fix unit test compatibility with TLS 1.3
----------------------------------------------------------------
Daniel P. Berrangé (4):
tests: call qcrypto_init instead of gnutls_global_init
tests: don't silence error reporting for all tests
tests: use error_abort in places expecting errors
tests: fix TLS handshake failure with TLS 1.3
include/crypto/init.h | 2 +
stubs/error-printf.c | 3 +-
tests/crypto-tls-x509-helpers.c | 3 +-
tests/test-crypto-tlscredsx509.c | 11 +----
tests/test-crypto-tlssession.c | 80 ++++++++++++--------------------
tests/test-io-channel-tls.c | 24 ++++------
tests/test-vmstate.c | 2 +
7 files changed, 49 insertions(+), 76 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PULL 1/4] tests: call qcrypto_init instead of gnutls_global_init
2018-07-24 16:44 [Qemu-devel] [PULL 0/4] Qcrypto next patches Daniel P. Berrangé
@ 2018-07-24 16:44 ` Daniel P. Berrangé
2018-07-24 16:44 ` [Qemu-devel] [PULL 2/4] tests: don't silence error reporting for all tests Daniel P. Berrangé
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2018-07-24 16:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Peter Maydell
Calling qcrypto_init ensures that all relevant initialization is
done. In particular this honours the debugging settings and thread
settings.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
include/crypto/init.h | 2 ++
tests/crypto-tls-x509-helpers.c | 3 ++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/crypto/init.h b/include/crypto/init.h
index 04c1edf770..f79c02266b 100644
--- a/include/crypto/init.h
+++ b/include/crypto/init.h
@@ -21,6 +21,8 @@
#ifndef QCRYPTO_INIT_H
#define QCRYPTO_INIT_H
+#include "qapi/error.h"
+
int qcrypto_init(Error **errp);
#endif /* QCRYPTO_INIT_H */
diff --git a/tests/crypto-tls-x509-helpers.c b/tests/crypto-tls-x509-helpers.c
index 173d4e28fb..9b669c2a4b 100644
--- a/tests/crypto-tls-x509-helpers.c
+++ b/tests/crypto-tls-x509-helpers.c
@@ -21,6 +21,7 @@
#include "qemu/osdep.h"
#include "crypto-tls-x509-helpers.h"
+#include "crypto/init.h"
#include "qemu/sockets.h"
#ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
@@ -95,7 +96,7 @@ static gnutls_x509_privkey_t test_tls_load_key(void)
void test_tls_init(const char *keyfile)
{
- gnutls_global_init();
+ qcrypto_init(&error_abort);
if (asn1_array2tree(pkix_asn1_tab, &pkix_asn1, NULL) != ASN1_SUCCESS) {
abort();
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PULL 2/4] tests: don't silence error reporting for all tests
2018-07-24 16:44 [Qemu-devel] [PULL 0/4] Qcrypto next patches Daniel P. Berrangé
2018-07-24 16:44 ` [Qemu-devel] [PULL 1/4] tests: call qcrypto_init instead of gnutls_global_init Daniel P. Berrangé
@ 2018-07-24 16:44 ` Daniel P. Berrangé
2018-07-24 16:44 ` [Qemu-devel] [PULL 3/4] tests: use error_abort in places expecting errors Daniel P. Berrangé
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2018-07-24 16:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Peter Maydell
The test-vmstate test is a bit chatty because it triggers various
expected failure scenarios and the code in question uses error_report
instead of accepting 'Error **errp' parameters. To silence this test the
stubs for error_vprintf() were changed to send errors via
g_test_message() instead of stderr:
commit 28017e010ddf6849cfa830e898da3e44e6610952
Author: Paolo Bonzini <pbonzini@redhat.com>
Date: Mon Oct 24 18:31:03 2016 +0200
tests: send error_report to test log
Implement error_vprintf to send the output of error_report to
the test log. This silences test-vmstate.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <1477326663-67817-3-git-send-email-pbonzini@redhat.com>
Unfortunately this change has global impact across the entire test suite
and means that when tests fail for unexpected reasons, the message is
not displayed on stderr. eg when using &error_abort in a call the test
merely prints
Unexpected error in qcrypto_tls_session_check_certificate() at crypto/tlssession.c:280:
and the actual error message is hidden, making it impossible to diagnose
the failure. This is especially problematic in CI or build systems where
it isn't possible to easily pass the --debug-log flag to tests and
re-run with the test log visible.
This change makes the previous big hammer much more nuanced, providing a
flag in the stub error_vprintf() that can used on a per-test basis to
silence the errors. Only the test-vmstate silences errors initially.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
stubs/error-printf.c | 3 ++-
tests/test-vmstate.c | 2 ++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/stubs/error-printf.c b/stubs/error-printf.c
index ac6b92aa69..99c6406668 100644
--- a/stubs/error-printf.c
+++ b/stubs/error-printf.c
@@ -4,7 +4,8 @@
void error_vprintf(const char *fmt, va_list ap)
{
- if (g_test_initialized() && !g_test_subprocess()) {
+ if (g_test_initialized() && !g_test_subprocess() &&
+ getenv("QTEST_SILENT_ERRORS")) {
char *msg = g_strdup_vprintf(fmt, ap);
g_test_message("%s", msg);
g_free(msg);
diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
index 087844b6c8..37a7a93784 100644
--- a/tests/test-vmstate.c
+++ b/tests/test-vmstate.c
@@ -859,6 +859,8 @@ int main(int argc, char **argv)
module_call_init(MODULE_INIT_QOM);
+ setenv("QTEST_SILENT_ERRORS", "1", 1);
+
g_test_init(&argc, &argv, NULL);
g_test_add_func("/vmstate/simple/primitive", test_simple_primitive);
g_test_add_func("/vmstate/versioned/load/v1", test_load_v1);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PULL 3/4] tests: use error_abort in places expecting errors
2018-07-24 16:44 [Qemu-devel] [PULL 0/4] Qcrypto next patches Daniel P. Berrangé
2018-07-24 16:44 ` [Qemu-devel] [PULL 1/4] tests: call qcrypto_init instead of gnutls_global_init Daniel P. Berrangé
2018-07-24 16:44 ` [Qemu-devel] [PULL 2/4] tests: don't silence error reporting for all tests Daniel P. Berrangé
@ 2018-07-24 16:44 ` Daniel P. Berrangé
2018-07-24 16:44 ` [Qemu-devel] [PULL 4/4] tests: fix TLS handshake failure with TLS 1.3 Daniel P. Berrangé
2018-07-24 19:16 ` [Qemu-devel] [PULL 0/4] Qcrypto next patches Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2018-07-24 16:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Peter Maydell
Most of the TLS related tests are passing an in a "Error" object to
methods that are expected to fail, but then ignoring any error that is
set and instead asserting on a return value. This means that when an
error is unexpectedly raised, no information about it is printed out,
making failures hard to diagnose. Changing these tests to pass in
&error_abort will make unexpected failures print messages to stderr.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/test-crypto-tlscredsx509.c | 11 +----
tests/test-crypto-tlssession.c | 76 ++++++++++++--------------------
tests/test-io-channel-tls.c | 24 ++++------
3 files changed, 39 insertions(+), 72 deletions(-)
diff --git a/tests/test-crypto-tlscredsx509.c b/tests/test-crypto-tlscredsx509.c
index af2f80e89c..30f9ac4bbf 100644
--- a/tests/test-crypto-tlscredsx509.c
+++ b/tests/test-crypto-tlscredsx509.c
@@ -54,7 +54,7 @@ static QCryptoTLSCreds *test_tls_creds_create(QCryptoTLSCredsEndpoint endpoint,
"sanity-check", "yes",
NULL);
- if (*errp) {
+ if (!creds) {
return NULL;
}
return QCRYPTO_TLS_CREDS(creds);
@@ -74,7 +74,6 @@ static void test_tls_creds(const void *opaque)
struct QCryptoTLSCredsTestData *data =
(struct QCryptoTLSCredsTestData *)opaque;
QCryptoTLSCreds *creds;
- Error *err = NULL;
#define CERT_DIR "tests/test-crypto-tlscredsx509-certs/"
mkdir(CERT_DIR, 0700);
@@ -113,17 +112,11 @@ static void test_tls_creds(const void *opaque)
QCRYPTO_TLS_CREDS_ENDPOINT_SERVER :
QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT),
CERT_DIR,
- &err);
+ data->expectFail ? NULL : &error_abort);
if (data->expectFail) {
- error_free(err);
g_assert(creds == NULL);
} else {
- if (err) {
- g_printerr("Failed to generate creds: %s\n",
- error_get_pretty(err));
- error_free(err);
- }
g_assert(creds != NULL);
}
diff --git a/tests/test-crypto-tlssession.c b/tests/test-crypto-tlssession.c
index 7bd811796e..fd9acf9067 100644
--- a/tests/test-crypto-tlssession.c
+++ b/tests/test-crypto-tlssession.c
@@ -52,28 +52,21 @@ static ssize_t testRead(char *buf, size_t len, void *opaque)
static QCryptoTLSCreds *test_tls_creds_psk_create(
QCryptoTLSCredsEndpoint endpoint,
- const char *dir,
- Error **errp)
+ const char *dir)
{
- Error *err = NULL;
Object *parent = object_get_objects_root();
Object *creds = object_new_with_props(
TYPE_QCRYPTO_TLS_CREDS_PSK,
parent,
(endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ?
"testtlscredsserver" : "testtlscredsclient"),
- &err,
+ &error_abort,
"endpoint", (endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ?
"server" : "client"),
"dir", dir,
"priority", "NORMAL",
NULL
);
-
- if (err) {
- error_propagate(errp, err);
- return NULL;
- }
return QCRYPTO_TLS_CREDS(creds);
}
@@ -87,7 +80,6 @@ static void test_crypto_tls_session_psk(void)
int channel[2];
bool clientShake = false;
bool serverShake = false;
- Error *err = NULL;
int ret;
/* We'll use this for our fake client-server connection */
@@ -104,25 +96,23 @@ static void test_crypto_tls_session_psk(void)
clientCreds = test_tls_creds_psk_create(
QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT,
- WORKDIR,
- &err);
+ WORKDIR);
g_assert(clientCreds != NULL);
serverCreds = test_tls_creds_psk_create(
QCRYPTO_TLS_CREDS_ENDPOINT_SERVER,
- WORKDIR,
- &err);
+ WORKDIR);
g_assert(serverCreds != NULL);
/* Now the real part of the test, setup the sessions */
clientSess = qcrypto_tls_session_new(
clientCreds, NULL, NULL,
- QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT, &err);
+ QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT, &error_abort);
+ g_assert(clientSess != NULL);
+
serverSess = qcrypto_tls_session_new(
serverCreds, NULL, NULL,
- QCRYPTO_TLS_CREDS_ENDPOINT_SERVER, &err);
-
- g_assert(clientSess != NULL);
+ QCRYPTO_TLS_CREDS_ENDPOINT_SERVER, &error_abort);
g_assert(serverSess != NULL);
/* For handshake to work, we need to set the I/O callbacks
@@ -145,7 +135,7 @@ static void test_crypto_tls_session_psk(void)
int rv;
if (!serverShake) {
rv = qcrypto_tls_session_handshake(serverSess,
- &err);
+ &error_abort);
g_assert(rv >= 0);
if (qcrypto_tls_session_get_handshake_status(serverSess) ==
QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
@@ -154,7 +144,7 @@ static void test_crypto_tls_session_psk(void)
}
if (!clientShake) {
rv = qcrypto_tls_session_handshake(clientSess,
- &err);
+ &error_abort);
g_assert(rv >= 0);
if (qcrypto_tls_session_get_handshake_status(clientSess) ==
QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
@@ -165,8 +155,10 @@ static void test_crypto_tls_session_psk(void)
/* Finally make sure the server & client validation is successful. */
- g_assert(qcrypto_tls_session_check_credentials(serverSess, &err) == 0);
- g_assert(qcrypto_tls_session_check_credentials(clientSess, &err) == 0);
+ g_assert(qcrypto_tls_session_check_credentials(serverSess,
+ &error_abort) == 0);
+ g_assert(qcrypto_tls_session_check_credentials(clientSess,
+ &error_abort) == 0);
object_unparent(OBJECT(serverCreds));
object_unparent(OBJECT(clientCreds));
@@ -192,17 +184,15 @@ struct QCryptoTLSSessionTestData {
static QCryptoTLSCreds *test_tls_creds_x509_create(
QCryptoTLSCredsEndpoint endpoint,
- const char *certdir,
- Error **errp)
+ const char *certdir)
{
- Error *err = NULL;
Object *parent = object_get_objects_root();
Object *creds = object_new_with_props(
TYPE_QCRYPTO_TLS_CREDS_X509,
parent,
(endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ?
"testtlscredsserver" : "testtlscredsclient"),
- &err,
+ &error_abort,
"endpoint", (endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ?
"server" : "client"),
"dir", certdir,
@@ -217,11 +207,6 @@ static QCryptoTLSCreds *test_tls_creds_x509_create(
"sanity-check", "no",
NULL
);
-
- if (err) {
- error_propagate(errp, err);
- return NULL;
- }
return QCRYPTO_TLS_CREDS(creds);
}
@@ -249,7 +234,6 @@ static void test_crypto_tls_session_x509(const void *opaque)
int channel[2];
bool clientShake = false;
bool serverShake = false;
- Error *err = NULL;
int ret;
/* We'll use this for our fake client-server connection */
@@ -293,14 +277,12 @@ static void test_crypto_tls_session_x509(const void *opaque)
clientCreds = test_tls_creds_x509_create(
QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT,
- CLIENT_CERT_DIR,
- &err);
+ CLIENT_CERT_DIR);
g_assert(clientCreds != NULL);
serverCreds = test_tls_creds_x509_create(
QCRYPTO_TLS_CREDS_ENDPOINT_SERVER,
- SERVER_CERT_DIR,
- &err);
+ SERVER_CERT_DIR);
g_assert(serverCreds != NULL);
acl = qemu_acl_init("tlssessionacl");
@@ -314,13 +296,13 @@ static void test_crypto_tls_session_x509(const void *opaque)
/* Now the real part of the test, setup the sessions */
clientSess = qcrypto_tls_session_new(
clientCreds, data->hostname, NULL,
- QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT, &err);
+ QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT, &error_abort);
+ g_assert(clientSess != NULL);
+
serverSess = qcrypto_tls_session_new(
serverCreds, NULL,
data->wildcards ? "tlssessionacl" : NULL,
- QCRYPTO_TLS_CREDS_ENDPOINT_SERVER, &err);
-
- g_assert(clientSess != NULL);
+ QCRYPTO_TLS_CREDS_ENDPOINT_SERVER, &error_abort);
g_assert(serverSess != NULL);
/* For handshake to work, we need to set the I/O callbacks
@@ -343,7 +325,7 @@ static void test_crypto_tls_session_x509(const void *opaque)
int rv;
if (!serverShake) {
rv = qcrypto_tls_session_handshake(serverSess,
- &err);
+ &error_abort);
g_assert(rv >= 0);
if (qcrypto_tls_session_get_handshake_status(serverSess) ==
QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
@@ -352,7 +334,7 @@ static void test_crypto_tls_session_x509(const void *opaque)
}
if (!clientShake) {
rv = qcrypto_tls_session_handshake(clientSess,
- &err);
+ &error_abort);
g_assert(rv >= 0);
if (qcrypto_tls_session_get_handshake_status(clientSess) ==
QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
@@ -365,10 +347,9 @@ static void test_crypto_tls_session_x509(const void *opaque)
/* Finally make sure the server validation does what
* we were expecting
*/
- if (qcrypto_tls_session_check_credentials(serverSess, &err) < 0) {
+ if (qcrypto_tls_session_check_credentials(
+ serverSess, data->expectServerFail ? NULL : &error_abort) < 0) {
g_assert(data->expectServerFail);
- error_free(err);
- err = NULL;
} else {
g_assert(!data->expectServerFail);
}
@@ -376,10 +357,9 @@ static void test_crypto_tls_session_x509(const void *opaque)
/*
* And the same for the client validation check
*/
- if (qcrypto_tls_session_check_credentials(clientSess, &err) < 0) {
+ if (qcrypto_tls_session_check_credentials(
+ clientSess, data->expectClientFail ? NULL : &error_abort) < 0) {
g_assert(data->expectClientFail);
- error_free(err);
- err = NULL;
} else {
g_assert(!data->expectClientFail);
}
diff --git a/tests/test-io-channel-tls.c b/tests/test-io-channel-tls.c
index bb88ee870f..4900c6d433 100644
--- a/tests/test-io-channel-tls.c
+++ b/tests/test-io-channel-tls.c
@@ -30,6 +30,7 @@
#include "crypto/init.h"
#include "crypto/tlscredsx509.h"
#include "qemu/acl.h"
+#include "qapi/error.h"
#include "qom/object_interfaces.h"
#ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
@@ -64,8 +65,7 @@ static void test_tls_handshake_done(QIOTask *task,
static QCryptoTLSCreds *test_tls_creds_create(QCryptoTLSCredsEndpoint endpoint,
- const char *certdir,
- Error **errp)
+ const char *certdir)
{
Object *parent = object_get_objects_root();
Object *creds = object_new_with_props(
@@ -73,7 +73,7 @@ static QCryptoTLSCreds *test_tls_creds_create(QCryptoTLSCredsEndpoint endpoint,
parent,
(endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ?
"testtlscredsserver" : "testtlscredsclient"),
- errp,
+ &error_abort,
"endpoint", (endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ?
"server" : "client"),
"dir", certdir,
@@ -89,9 +89,6 @@ static QCryptoTLSCreds *test_tls_creds_create(QCryptoTLSCredsEndpoint endpoint,
NULL
);
- if (*errp) {
- return NULL;
- }
return QCRYPTO_TLS_CREDS(creds);
}
@@ -121,7 +118,6 @@ static void test_io_channel_tls(const void *opaque)
int channel[2];
struct QIOChannelTLSHandshakeData clientHandshake = { false, false };
struct QIOChannelTLSHandshakeData serverHandshake = { false, false };
- Error *err = NULL;
QIOChannelTest *test;
GMainContext *mainloop;
@@ -157,14 +153,12 @@ static void test_io_channel_tls(const void *opaque)
clientCreds = test_tls_creds_create(
QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT,
- CLIENT_CERT_DIR,
- &err);
+ CLIENT_CERT_DIR);
g_assert(clientCreds != NULL);
serverCreds = test_tls_creds_create(
QCRYPTO_TLS_CREDS_ENDPOINT_SERVER,
- SERVER_CERT_DIR,
- &err);
+ SERVER_CERT_DIR);
g_assert(serverCreds != NULL);
acl = qemu_acl_init("channeltlsacl");
@@ -176,10 +170,10 @@ static void test_io_channel_tls(const void *opaque)
}
clientChanSock = qio_channel_socket_new_fd(
- channel[0], &err);
+ channel[0], &error_abort);
g_assert(clientChanSock != NULL);
serverChanSock = qio_channel_socket_new_fd(
- channel[1], &err);
+ channel[1], &error_abort);
g_assert(serverChanSock != NULL);
/*
@@ -193,12 +187,12 @@ static void test_io_channel_tls(const void *opaque)
/* Now the real part of the test, setup the sessions */
clientChanTLS = qio_channel_tls_new_client(
QIO_CHANNEL(clientChanSock), clientCreds,
- data->hostname, &err);
+ data->hostname, &error_abort);
g_assert(clientChanTLS != NULL);
serverChanTLS = qio_channel_tls_new_server(
QIO_CHANNEL(serverChanSock), serverCreds,
- "channeltlsacl", &err);
+ "channeltlsacl", &error_abort);
g_assert(serverChanTLS != NULL);
qio_channel_tls_handshake(clientChanTLS,
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PULL 4/4] tests: fix TLS handshake failure with TLS 1.3
2018-07-24 16:44 [Qemu-devel] [PULL 0/4] Qcrypto next patches Daniel P. Berrangé
` (2 preceding siblings ...)
2018-07-24 16:44 ` [Qemu-devel] [PULL 3/4] tests: use error_abort in places expecting errors Daniel P. Berrangé
@ 2018-07-24 16:44 ` Daniel P. Berrangé
2018-07-24 19:16 ` [Qemu-devel] [PULL 0/4] Qcrypto next patches Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2018-07-24 16:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Peter Maydell
When gnutls negotiates TLS 1.3 instead of 1.2, the order of messages
sent by the handshake changes. This exposed a logic bug in the test
suite which caused us to wait for the server to see handshake
completion, but not wait for the client to see completion. The result
was the client didn't receive the certificate for verification and the
test failed.
This is exposed in Fedora 29 rawhide which has just enabled TLS 1.3 in
its GNUTLS builds.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/test-crypto-tlssession.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/test-crypto-tlssession.c b/tests/test-crypto-tlssession.c
index fd9acf9067..6fa9950afb 100644
--- a/tests/test-crypto-tlssession.c
+++ b/tests/test-crypto-tlssession.c
@@ -151,7 +151,7 @@ static void test_crypto_tls_session_psk(void)
clientShake = true;
}
}
- } while (!clientShake && !serverShake);
+ } while (!clientShake || !serverShake);
/* Finally make sure the server & client validation is successful. */
@@ -341,7 +341,7 @@ static void test_crypto_tls_session_x509(const void *opaque)
clientShake = true;
}
}
- } while (!clientShake && !serverShake);
+ } while (!clientShake || !serverShake);
/* Finally make sure the server validation does what
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PULL 0/4] Qcrypto next patches
2018-07-24 16:44 [Qemu-devel] [PULL 0/4] Qcrypto next patches Daniel P. Berrangé
` (3 preceding siblings ...)
2018-07-24 16:44 ` [Qemu-devel] [PULL 4/4] tests: fix TLS handshake failure with TLS 1.3 Daniel P. Berrangé
@ 2018-07-24 19:16 ` Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2018-07-24 19:16 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: QEMU Developers
On 24 July 2018 at 17:44, Daniel P. Berrangé <berrange@redhat.com> wrote:
> The following changes since commit 3bae150448dbd888a480f892ebbf01caec0d8329:
>
> Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2018-07-24 15:26:01 +0100)
>
> are available in the Git repository at:
>
> https://github.com/berrange/qemu tags/qcrypto-next-pull-request
>
> for you to fetch changes up to db0a8c70f25fe497c4b786d8edac063daa744c0d:
>
> tests: fix TLS handshake failure with TLS 1.3 (2018-07-24 17:36:12 +0100)
>
> ----------------------------------------------------------------
> Fix unit test compatibility with TLS 1.3
>
> ----------------------------------------------------------------
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-07-24 19:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-24 16:44 [Qemu-devel] [PULL 0/4] Qcrypto next patches Daniel P. Berrangé
2018-07-24 16:44 ` [Qemu-devel] [PULL 1/4] tests: call qcrypto_init instead of gnutls_global_init Daniel P. Berrangé
2018-07-24 16:44 ` [Qemu-devel] [PULL 2/4] tests: don't silence error reporting for all tests Daniel P. Berrangé
2018-07-24 16:44 ` [Qemu-devel] [PULL 3/4] tests: use error_abort in places expecting errors Daniel P. Berrangé
2018-07-24 16:44 ` [Qemu-devel] [PULL 4/4] tests: fix TLS handshake failure with TLS 1.3 Daniel P. Berrangé
2018-07-24 19:16 ` [Qemu-devel] [PULL 0/4] Qcrypto next patches Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).