From dbddad702630bc2d8dd20f6d0d8076ccb8831a7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Tue, 17 Jul 2018 17:54:17 +0100 Subject: tests: call qcrypto_init instead of gnutls_global_init MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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é Tested-by: Philippe Mathieu-Daudé Signed-off-by: Daniel P. Berrangé --- tests/crypto-tls-x509-helpers.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'tests') 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(); -- cgit v1.2.3 From 977a7204ab69a3591c4c6ed698abb8a6ee673679 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Wed, 18 Jul 2018 09:34:47 +0100 Subject: tests: don't silence error reporting for all tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 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 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é Tested-by: Philippe Mathieu-Daudé Signed-off-by: Daniel P. Berrangé --- tests/test-vmstate.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'tests') 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); -- cgit v1.2.3 From 68db13183f2d3193476f5d5457c47fa10ba0f9e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Wed, 18 Jul 2018 10:06:43 +0100 Subject: tests: use error_abort in places expecting errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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é Tested-by: Philippe Mathieu-Daudé Signed-off-by: Daniel P. Berrangé --- 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(-) (limited to 'tests') 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, -- cgit v1.2.3 From db0a8c70f25fe497c4b786d8edac063daa744c0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Wed, 18 Jul 2018 10:24:59 +0100 Subject: tests: fix TLS handshake failure with TLS 1.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Daniel P. Berrangé --- tests/test-crypto-tlssession.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tests') 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 -- cgit v1.2.3