tests: use error_abort in places expecting errors

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>
This commit is contained in:
Daniel P. Berrangé 2018-07-18 10:06:43 +01:00
parent 977a7204ab
commit 68db13183f
3 changed files with 39 additions and 72 deletions

View File

@ -54,7 +54,7 @@ static QCryptoTLSCreds *test_tls_creds_create(QCryptoTLSCredsEndpoint endpoint,
"sanity-check", "yes", "sanity-check", "yes",
NULL); NULL);
if (*errp) { if (!creds) {
return NULL; return NULL;
} }
return QCRYPTO_TLS_CREDS(creds); return QCRYPTO_TLS_CREDS(creds);
@ -74,7 +74,6 @@ static void test_tls_creds(const void *opaque)
struct QCryptoTLSCredsTestData *data = struct QCryptoTLSCredsTestData *data =
(struct QCryptoTLSCredsTestData *)opaque; (struct QCryptoTLSCredsTestData *)opaque;
QCryptoTLSCreds *creds; QCryptoTLSCreds *creds;
Error *err = NULL;
#define CERT_DIR "tests/test-crypto-tlscredsx509-certs/" #define CERT_DIR "tests/test-crypto-tlscredsx509-certs/"
mkdir(CERT_DIR, 0700); 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_SERVER :
QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT), QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT),
CERT_DIR, CERT_DIR,
&err); data->expectFail ? NULL : &error_abort);
if (data->expectFail) { if (data->expectFail) {
error_free(err);
g_assert(creds == NULL); g_assert(creds == NULL);
} else { } else {
if (err) {
g_printerr("Failed to generate creds: %s\n",
error_get_pretty(err));
error_free(err);
}
g_assert(creds != NULL); g_assert(creds != NULL);
} }

View File

@ -52,28 +52,21 @@ static ssize_t testRead(char *buf, size_t len, void *opaque)
static QCryptoTLSCreds *test_tls_creds_psk_create( static QCryptoTLSCreds *test_tls_creds_psk_create(
QCryptoTLSCredsEndpoint endpoint, QCryptoTLSCredsEndpoint endpoint,
const char *dir, const char *dir)
Error **errp)
{ {
Error *err = NULL;
Object *parent = object_get_objects_root(); Object *parent = object_get_objects_root();
Object *creds = object_new_with_props( Object *creds = object_new_with_props(
TYPE_QCRYPTO_TLS_CREDS_PSK, TYPE_QCRYPTO_TLS_CREDS_PSK,
parent, parent,
(endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ? (endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ?
"testtlscredsserver" : "testtlscredsclient"), "testtlscredsserver" : "testtlscredsclient"),
&err, &error_abort,
"endpoint", (endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ? "endpoint", (endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ?
"server" : "client"), "server" : "client"),
"dir", dir, "dir", dir,
"priority", "NORMAL", "priority", "NORMAL",
NULL NULL
); );
if (err) {
error_propagate(errp, err);
return NULL;
}
return QCRYPTO_TLS_CREDS(creds); return QCRYPTO_TLS_CREDS(creds);
} }
@ -87,7 +80,6 @@ static void test_crypto_tls_session_psk(void)
int channel[2]; int channel[2];
bool clientShake = false; bool clientShake = false;
bool serverShake = false; bool serverShake = false;
Error *err = NULL;
int ret; int ret;
/* We'll use this for our fake client-server connection */ /* 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( clientCreds = test_tls_creds_psk_create(
QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT, QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT,
WORKDIR, WORKDIR);
&err);
g_assert(clientCreds != NULL); g_assert(clientCreds != NULL);
serverCreds = test_tls_creds_psk_create( serverCreds = test_tls_creds_psk_create(
QCRYPTO_TLS_CREDS_ENDPOINT_SERVER, QCRYPTO_TLS_CREDS_ENDPOINT_SERVER,
WORKDIR, WORKDIR);
&err);
g_assert(serverCreds != NULL); g_assert(serverCreds != NULL);
/* Now the real part of the test, setup the sessions */ /* Now the real part of the test, setup the sessions */
clientSess = qcrypto_tls_session_new( clientSess = qcrypto_tls_session_new(
clientCreds, NULL, NULL, 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( serverSess = qcrypto_tls_session_new(
serverCreds, NULL, NULL, serverCreds, NULL, NULL,
QCRYPTO_TLS_CREDS_ENDPOINT_SERVER, &err); QCRYPTO_TLS_CREDS_ENDPOINT_SERVER, &error_abort);
g_assert(clientSess != NULL);
g_assert(serverSess != NULL); g_assert(serverSess != NULL);
/* For handshake to work, we need to set the I/O callbacks /* 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; int rv;
if (!serverShake) { if (!serverShake) {
rv = qcrypto_tls_session_handshake(serverSess, rv = qcrypto_tls_session_handshake(serverSess,
&err); &error_abort);
g_assert(rv >= 0); g_assert(rv >= 0);
if (qcrypto_tls_session_get_handshake_status(serverSess) == if (qcrypto_tls_session_get_handshake_status(serverSess) ==
QCRYPTO_TLS_HANDSHAKE_COMPLETE) { QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
@ -154,7 +144,7 @@ static void test_crypto_tls_session_psk(void)
} }
if (!clientShake) { if (!clientShake) {
rv = qcrypto_tls_session_handshake(clientSess, rv = qcrypto_tls_session_handshake(clientSess,
&err); &error_abort);
g_assert(rv >= 0); g_assert(rv >= 0);
if (qcrypto_tls_session_get_handshake_status(clientSess) == if (qcrypto_tls_session_get_handshake_status(clientSess) ==
QCRYPTO_TLS_HANDSHAKE_COMPLETE) { 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. */ /* 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(serverSess,
g_assert(qcrypto_tls_session_check_credentials(clientSess, &err) == 0); &error_abort) == 0);
g_assert(qcrypto_tls_session_check_credentials(clientSess,
&error_abort) == 0);
object_unparent(OBJECT(serverCreds)); object_unparent(OBJECT(serverCreds));
object_unparent(OBJECT(clientCreds)); object_unparent(OBJECT(clientCreds));
@ -192,17 +184,15 @@ struct QCryptoTLSSessionTestData {
static QCryptoTLSCreds *test_tls_creds_x509_create( static QCryptoTLSCreds *test_tls_creds_x509_create(
QCryptoTLSCredsEndpoint endpoint, QCryptoTLSCredsEndpoint endpoint,
const char *certdir, const char *certdir)
Error **errp)
{ {
Error *err = NULL;
Object *parent = object_get_objects_root(); Object *parent = object_get_objects_root();
Object *creds = object_new_with_props( Object *creds = object_new_with_props(
TYPE_QCRYPTO_TLS_CREDS_X509, TYPE_QCRYPTO_TLS_CREDS_X509,
parent, parent,
(endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ? (endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ?
"testtlscredsserver" : "testtlscredsclient"), "testtlscredsserver" : "testtlscredsclient"),
&err, &error_abort,
"endpoint", (endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ? "endpoint", (endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ?
"server" : "client"), "server" : "client"),
"dir", certdir, "dir", certdir,
@ -217,11 +207,6 @@ static QCryptoTLSCreds *test_tls_creds_x509_create(
"sanity-check", "no", "sanity-check", "no",
NULL NULL
); );
if (err) {
error_propagate(errp, err);
return NULL;
}
return QCRYPTO_TLS_CREDS(creds); return QCRYPTO_TLS_CREDS(creds);
} }
@ -249,7 +234,6 @@ static void test_crypto_tls_session_x509(const void *opaque)
int channel[2]; int channel[2];
bool clientShake = false; bool clientShake = false;
bool serverShake = false; bool serverShake = false;
Error *err = NULL;
int ret; int ret;
/* We'll use this for our fake client-server connection */ /* 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( clientCreds = test_tls_creds_x509_create(
QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT, QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT,
CLIENT_CERT_DIR, CLIENT_CERT_DIR);
&err);
g_assert(clientCreds != NULL); g_assert(clientCreds != NULL);
serverCreds = test_tls_creds_x509_create( serverCreds = test_tls_creds_x509_create(
QCRYPTO_TLS_CREDS_ENDPOINT_SERVER, QCRYPTO_TLS_CREDS_ENDPOINT_SERVER,
SERVER_CERT_DIR, SERVER_CERT_DIR);
&err);
g_assert(serverCreds != NULL); g_assert(serverCreds != NULL);
acl = qemu_acl_init("tlssessionacl"); 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 */ /* Now the real part of the test, setup the sessions */
clientSess = qcrypto_tls_session_new( clientSess = qcrypto_tls_session_new(
clientCreds, data->hostname, NULL, 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( serverSess = qcrypto_tls_session_new(
serverCreds, NULL, serverCreds, NULL,
data->wildcards ? "tlssessionacl" : NULL, data->wildcards ? "tlssessionacl" : NULL,
QCRYPTO_TLS_CREDS_ENDPOINT_SERVER, &err); QCRYPTO_TLS_CREDS_ENDPOINT_SERVER, &error_abort);
g_assert(clientSess != NULL);
g_assert(serverSess != NULL); g_assert(serverSess != NULL);
/* For handshake to work, we need to set the I/O callbacks /* 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; int rv;
if (!serverShake) { if (!serverShake) {
rv = qcrypto_tls_session_handshake(serverSess, rv = qcrypto_tls_session_handshake(serverSess,
&err); &error_abort);
g_assert(rv >= 0); g_assert(rv >= 0);
if (qcrypto_tls_session_get_handshake_status(serverSess) == if (qcrypto_tls_session_get_handshake_status(serverSess) ==
QCRYPTO_TLS_HANDSHAKE_COMPLETE) { QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
@ -352,7 +334,7 @@ static void test_crypto_tls_session_x509(const void *opaque)
} }
if (!clientShake) { if (!clientShake) {
rv = qcrypto_tls_session_handshake(clientSess, rv = qcrypto_tls_session_handshake(clientSess,
&err); &error_abort);
g_assert(rv >= 0); g_assert(rv >= 0);
if (qcrypto_tls_session_get_handshake_status(clientSess) == if (qcrypto_tls_session_get_handshake_status(clientSess) ==
QCRYPTO_TLS_HANDSHAKE_COMPLETE) { 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 /* Finally make sure the server validation does what
* we were expecting * 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); g_assert(data->expectServerFail);
error_free(err);
err = NULL;
} else { } else {
g_assert(!data->expectServerFail); 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 * 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); g_assert(data->expectClientFail);
error_free(err);
err = NULL;
} else { } else {
g_assert(!data->expectClientFail); g_assert(!data->expectClientFail);
} }

View File

@ -30,6 +30,7 @@
#include "crypto/init.h" #include "crypto/init.h"
#include "crypto/tlscredsx509.h" #include "crypto/tlscredsx509.h"
#include "qemu/acl.h" #include "qemu/acl.h"
#include "qapi/error.h"
#include "qom/object_interfaces.h" #include "qom/object_interfaces.h"
#ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT #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, static QCryptoTLSCreds *test_tls_creds_create(QCryptoTLSCredsEndpoint endpoint,
const char *certdir, const char *certdir)
Error **errp)
{ {
Object *parent = object_get_objects_root(); Object *parent = object_get_objects_root();
Object *creds = object_new_with_props( Object *creds = object_new_with_props(
@ -73,7 +73,7 @@ static QCryptoTLSCreds *test_tls_creds_create(QCryptoTLSCredsEndpoint endpoint,
parent, parent,
(endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ? (endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ?
"testtlscredsserver" : "testtlscredsclient"), "testtlscredsserver" : "testtlscredsclient"),
errp, &error_abort,
"endpoint", (endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ? "endpoint", (endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER ?
"server" : "client"), "server" : "client"),
"dir", certdir, "dir", certdir,
@ -89,9 +89,6 @@ static QCryptoTLSCreds *test_tls_creds_create(QCryptoTLSCredsEndpoint endpoint,
NULL NULL
); );
if (*errp) {
return NULL;
}
return QCRYPTO_TLS_CREDS(creds); return QCRYPTO_TLS_CREDS(creds);
} }
@ -121,7 +118,6 @@ static void test_io_channel_tls(const void *opaque)
int channel[2]; int channel[2];
struct QIOChannelTLSHandshakeData clientHandshake = { false, false }; struct QIOChannelTLSHandshakeData clientHandshake = { false, false };
struct QIOChannelTLSHandshakeData serverHandshake = { false, false }; struct QIOChannelTLSHandshakeData serverHandshake = { false, false };
Error *err = NULL;
QIOChannelTest *test; QIOChannelTest *test;
GMainContext *mainloop; GMainContext *mainloop;
@ -157,14 +153,12 @@ static void test_io_channel_tls(const void *opaque)
clientCreds = test_tls_creds_create( clientCreds = test_tls_creds_create(
QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT, QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT,
CLIENT_CERT_DIR, CLIENT_CERT_DIR);
&err);
g_assert(clientCreds != NULL); g_assert(clientCreds != NULL);
serverCreds = test_tls_creds_create( serverCreds = test_tls_creds_create(
QCRYPTO_TLS_CREDS_ENDPOINT_SERVER, QCRYPTO_TLS_CREDS_ENDPOINT_SERVER,
SERVER_CERT_DIR, SERVER_CERT_DIR);
&err);
g_assert(serverCreds != NULL); g_assert(serverCreds != NULL);
acl = qemu_acl_init("channeltlsacl"); acl = qemu_acl_init("channeltlsacl");
@ -176,10 +170,10 @@ static void test_io_channel_tls(const void *opaque)
} }
clientChanSock = qio_channel_socket_new_fd( clientChanSock = qio_channel_socket_new_fd(
channel[0], &err); channel[0], &error_abort);
g_assert(clientChanSock != NULL); g_assert(clientChanSock != NULL);
serverChanSock = qio_channel_socket_new_fd( serverChanSock = qio_channel_socket_new_fd(
channel[1], &err); channel[1], &error_abort);
g_assert(serverChanSock != NULL); 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 */ /* Now the real part of the test, setup the sessions */
clientChanTLS = qio_channel_tls_new_client( clientChanTLS = qio_channel_tls_new_client(
QIO_CHANNEL(clientChanSock), clientCreds, QIO_CHANNEL(clientChanSock), clientCreds,
data->hostname, &err); data->hostname, &error_abort);
g_assert(clientChanTLS != NULL); g_assert(clientChanTLS != NULL);
serverChanTLS = qio_channel_tls_new_server( serverChanTLS = qio_channel_tls_new_server(
QIO_CHANNEL(serverChanSock), serverCreds, QIO_CHANNEL(serverChanSock), serverCreds,
"channeltlsacl", &err); "channeltlsacl", &error_abort);
g_assert(serverChanTLS != NULL); g_assert(serverChanTLS != NULL);
qio_channel_tls_handshake(clientChanTLS, qio_channel_tls_handshake(clientChanTLS,