From 019e47ce564e9d57ed85c4ebe0672518b6a349f5 Mon Sep 17 00:00:00 2001 From: Pauli Date: Thu, 22 Jun 2017 14:00:55 +1000 Subject: Remove uses of the TEST_check macro. This macro aborts the test which prevents later tests from executing. It also bypasses the test framework output functionality. Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/3750) --- test/cipherlist_test.c | 64 +++++++++++++++++++++++++++++------------------- test/ssl_test.c | 46 +++++++++++++++++----------------- test/ssl_test_ctx.c | 38 +++++++++++++--------------- test/ssl_test_ctx_test.c | 14 ++++++++--- test/sslcorrupttest.c | 45 ++++++++++++++++++---------------- test/testutil.h | 2 +- 6 files changed, 114 insertions(+), 95 deletions(-) (limited to 'test') diff --git a/test/cipherlist_test.c b/test/cipherlist_test.c index 61551d9223..f4d1b353e2 100644 --- a/test/cipherlist_test.c +++ b/test/cipherlist_test.c @@ -1,5 +1,5 @@ /* - * Copyright 2016 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 2016-2017 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the OpenSSL licenses, (the "License"); * you may not use this file except in compliance with the License. @@ -9,6 +9,7 @@ */ #include +#include #include #include @@ -27,14 +28,27 @@ typedef struct cipherlist_test_fixture { } CIPHERLIST_TEST_FIXTURE; -static CIPHERLIST_TEST_FIXTURE set_up(const char *const test_case_name) +static void tear_down(CIPHERLIST_TEST_FIXTURE *fixture) { - CIPHERLIST_TEST_FIXTURE fixture; + if (fixture != NULL) { + SSL_CTX_free(fixture->server); + SSL_CTX_free(fixture->client); + fixture->server = fixture->client = NULL; + } +} + +static CIPHERLIST_TEST_FIXTURE *set_up(const char *const test_case_name) +{ + static CIPHERLIST_TEST_FIXTURE fixture; + + memset(&fixture, 0, sizeof(fixture)); fixture.test_case_name = test_case_name; - fixture.server = SSL_CTX_new(TLS_server_method()); - fixture.client = SSL_CTX_new(TLS_client_method()); - TEST_check(fixture.client != NULL && fixture.server != NULL); - return fixture; + if (!TEST_ptr(fixture.server = SSL_CTX_new(TLS_server_method())) + || !TEST_ptr(fixture.client = SSL_CTX_new(TLS_client_method()))) { + tear_down(&fixture); + return NULL; + } + return &fixture; } /* @@ -123,16 +137,18 @@ static const uint32_t default_ciphers_in_order[] = { static int test_default_cipherlist(SSL_CTX *ctx) { - STACK_OF(SSL_CIPHER) *ciphers; - SSL *ssl; + STACK_OF(SSL_CIPHER) *ciphers = NULL; + SSL *ssl = NULL; int i, ret = 0, num_expected_ciphers, num_ciphers; uint32_t expected_cipher_id, cipher_id; - ssl = SSL_new(ctx); - TEST_check(ssl != NULL); + if (ctx == NULL) + return 0; + + if (!TEST_ptr(ssl = SSL_new(ctx)) + || !TEST_ptr(ciphers = SSL_get1_supported_ciphers(ssl))) + goto err; - ciphers = SSL_get1_supported_ciphers(ssl); - TEST_check(ciphers != NULL); num_expected_ciphers = OSSL_NELEM(default_ciphers_in_order); num_ciphers = sk_SSL_CIPHER_num(ciphers); if (!TEST_int_eq(num_ciphers, num_expected_ciphers)) @@ -155,20 +171,15 @@ static int test_default_cipherlist(SSL_CTX *ctx) return ret; } -static int execute_test(CIPHERLIST_TEST_FIXTURE fixture) -{ - return test_default_cipherlist(fixture.server) - && test_default_cipherlist(fixture.client); -} - -static void tear_down(CIPHERLIST_TEST_FIXTURE fixture) +static int execute_test(CIPHERLIST_TEST_FIXTURE *fixture) { - SSL_CTX_free(fixture.server); - SSL_CTX_free(fixture.client); + return fixture != NULL + && test_default_cipherlist(fixture->server) + && test_default_cipherlist(fixture->client); } #define SETUP_CIPHERLIST_TEST_FIXTURE() \ - SETUP_TEST_FIXTURE(CIPHERLIST_TEST_FIXTURE, set_up) + SETUP_TEST_FIXTURE(CIPHERLIST_TEST_FIXTURE *, set_up) #define EXECUTE_CIPHERLIST_TEST() \ EXECUTE_TEST(execute_test, tear_down) @@ -182,8 +193,11 @@ static int test_default_cipherlist_implicit() static int test_default_cipherlist_explicit() { SETUP_CIPHERLIST_TEST_FIXTURE(); - TEST_check(SSL_CTX_set_cipher_list(fixture.server, "DEFAULT")); - TEST_check(SSL_CTX_set_cipher_list(fixture.client, "DEFAULT")); + if (fixture == NULL) + return 0; + if (!TEST_true(SSL_CTX_set_cipher_list(fixture->server, "DEFAULT")) + || !TEST_true(SSL_CTX_set_cipher_list(fixture->client, "DEFAULT"))) + tear_down(fixture); EXECUTE_CIPHERLIST_TEST(); } diff --git a/test/ssl_test.c b/test/ssl_test.c index 06dd18915d..c8e71bb8b1 100644 --- a/test/ssl_test.c +++ b/test/ssl_test.c @@ -359,15 +359,16 @@ static int test_handshake(int idx) server_ctx = SSL_CTX_new(DTLS_server_method()); if (test_ctx->extra.server.servername_callback != SSL_TEST_SERVERNAME_CB_NONE) { - server2_ctx = SSL_CTX_new(DTLS_server_method()); - TEST_check(server2_ctx != NULL); + if (!TEST_ptr(server2_ctx = SSL_CTX_new(DTLS_server_method()))) + goto err; } client_ctx = SSL_CTX_new(DTLS_client_method()); if (test_ctx->handshake_mode == SSL_TEST_HANDSHAKE_RESUME) { resume_server_ctx = SSL_CTX_new(DTLS_server_method()); resume_client_ctx = SSL_CTX_new(DTLS_client_method()); - TEST_check(resume_server_ctx != NULL); - TEST_check(resume_client_ctx != NULL); + if (!TEST_ptr(resume_server_ctx) + || !TEST_ptr(resume_client_ctx)) + goto err; } } #endif @@ -376,23 +377,24 @@ static int test_handshake(int idx) /* SNI on resumption isn't supported/tested yet. */ if (test_ctx->extra.server.servername_callback != SSL_TEST_SERVERNAME_CB_NONE) { - server2_ctx = SSL_CTX_new(TLS_server_method()); - TEST_check(server2_ctx != NULL); + if (!TEST_ptr(server2_ctx = SSL_CTX_new(TLS_server_method()))) + goto err; } client_ctx = SSL_CTX_new(TLS_client_method()); if (test_ctx->handshake_mode == SSL_TEST_HANDSHAKE_RESUME) { resume_server_ctx = SSL_CTX_new(TLS_server_method()); resume_client_ctx = SSL_CTX_new(TLS_client_method()); - TEST_check(resume_server_ctx != NULL); - TEST_check(resume_client_ctx != NULL); + if (!TEST_ptr(resume_server_ctx) + || !TEST_ptr(resume_client_ctx)) + goto err; } } - TEST_check(server_ctx != NULL); - TEST_check(client_ctx != NULL); - - TEST_check(CONF_modules_load(conf, test_app, 0) > 0); + if (!TEST_ptr(server_ctx) + || !TEST_ptr(client_ctx) + || !TEST_int_gt(CONF_modules_load(conf, test_app, 0), 0)) + goto err; if (!SSL_CTX_config(server_ctx, "server") || !SSL_CTX_config(client_ctx, "client")) { @@ -427,23 +429,21 @@ err: int test_main(int argc, char **argv) { - int result = 0; + int result = EXIT_FAILURE; long num_tests; - if (argc != 2) - return 1; - - conf = NCONF_new(NULL); - TEST_check(conf != NULL); - - /* argv[1] should point to the test conf file */ - TEST_check(NCONF_load(conf, argv[1], NULL) > 0); - - TEST_check(NCONF_get_number_e(conf, NULL, "num_tests", &num_tests)); + if (!TEST_int_eq(argc, 2) + || !TEST_ptr(conf = NCONF_new(NULL)) + /* argv[1] should point to the test conf file */ + || !TEST_int_gt(NCONF_load(conf, argv[1], NULL), 0) + || !TEST_int_ne(NCONF_get_number_e(conf, NULL, "num_tests", + &num_tests), 0)) + goto err; ADD_ALL_TESTS(test_handshake, (int)(num_tests)); result = run_tests(argv[0]); +err: NCONF_free(conf); return result; } diff --git a/test/ssl_test_ctx.c b/test/ssl_test_ctx.c index 78f15ca1aa..b581cdfb94 100644 --- a/test/ssl_test_ctx.c +++ b/test/ssl_test_ctx.c @@ -30,6 +30,7 @@ static int parse_boolean(const char *value, int *result) *result = 0; return 1; } + TEST_error("parse_boolean given: '%s'", value); return 0; } @@ -44,8 +45,7 @@ static int parse_boolean(const char *value, int *result) { \ OPENSSL_free(ctx->field); \ ctx->field = OPENSSL_strdup(value); \ - TEST_check(ctx->field != NULL); \ - return 1; \ + return TEST_ptr(ctx->field); \ } #define IMPLEMENT_SSL_TEST_INT_OPTION(struct_type, name, field) \ @@ -627,17 +627,15 @@ static const ssl_test_server_option ssl_test_server_options[] = { { "SRPPassword", &parse_server_srp_password }, }; -/* - * Since these methods are used to create tests, we use TEST_check liberally - * for malloc failures and other internal errors. - */ SSL_TEST_CTX *SSL_TEST_CTX_new() { SSL_TEST_CTX *ret; - ret = OPENSSL_zalloc(sizeof(*ret)); - TEST_check(ret != NULL); - ret->app_data_size = default_app_data_size; - ret->max_fragment_size = default_max_fragment_size; + + /* The return code is checked by caller */ + if ((ret = OPENSSL_zalloc(sizeof(*ret))) != NULL) { + ret->app_data_size = default_app_data_size; + ret->max_fragment_size = default_max_fragment_size; + } return ret; } @@ -681,8 +679,8 @@ static int parse_client_options(SSL_TEST_CLIENT_CONF *client, const CONF *conf, int i; size_t j; - sk_conf = NCONF_get_section(conf, client_section); - TEST_check(sk_conf != NULL); + if (!TEST_ptr(sk_conf = NCONF_get_section(conf, client_section))) + return 0; for (i = 0; i < sk_CONF_VALUE_num(sk_conf); i++) { int found = 0; @@ -714,8 +712,8 @@ static int parse_server_options(SSL_TEST_SERVER_CONF *server, const CONF *conf, int i; size_t j; - sk_conf = NCONF_get_section(conf, server_section); - TEST_check(sk_conf != NULL); + if (!TEST_ptr(sk_conf = NCONF_get_section(conf, server_section))) + return 0; for (i = 0; i < sk_CONF_VALUE_num(sk_conf); i++) { int found = 0; @@ -742,16 +740,14 @@ static int parse_server_options(SSL_TEST_SERVER_CONF *server, const CONF *conf, SSL_TEST_CTX *SSL_TEST_CTX_create(const CONF *conf, const char *test_section) { - STACK_OF(CONF_VALUE) *sk_conf; - SSL_TEST_CTX *ctx; + STACK_OF(CONF_VALUE) *sk_conf = NULL; + SSL_TEST_CTX *ctx = NULL; int i; size_t j; - sk_conf = NCONF_get_section(conf, test_section); - TEST_check(sk_conf != NULL); - - ctx = SSL_TEST_CTX_new(); - TEST_check(ctx != NULL); + if (!TEST_ptr(sk_conf = NCONF_get_section(conf, test_section)) + || !TEST_ptr(ctx = SSL_TEST_CTX_new())) + goto err; for (i = 0; i < sk_CONF_VALUE_num(sk_conf); i++) { int found = 0; diff --git a/test/ssl_test_ctx_test.c b/test/ssl_test_ctx_test.c index e24f0fb2cf..08775354cc 100644 --- a/test/ssl_test_ctx_test.c +++ b/test/ssl_test_ctx_test.c @@ -1,5 +1,5 @@ /* - * Copyright 2016 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 2016-2017 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the OpenSSL license (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -101,6 +101,7 @@ static SSL_TEST_CTX_TEST_FIXTURE set_up(const char *const test_case_name) { SSL_TEST_CTX_TEST_FIXTURE fixture; + memset(&fixture, 0, sizeof(fixture)); fixture.test_case_name = test_case_name; TEST_ptr(fixture.expected_ctx = SSL_TEST_CTX_new()); return fixture; @@ -162,7 +163,8 @@ static int test_good_configuration() fixture.expected_ctx->extra.client.servername = SSL_TEST_SERVERNAME_SERVER2; fixture.expected_ctx->extra.client.npn_protocols = OPENSSL_strdup("foo,bar"); - TEST_check(fixture.expected_ctx->extra.client.npn_protocols != NULL); + if (!TEST_ptr(fixture.expected_ctx->extra.client.npn_protocols)) + goto err; fixture.expected_ctx->extra.server.servername_callback = SSL_TEST_SERVERNAME_IGNORE_MISMATCH; @@ -170,13 +172,17 @@ static int test_good_configuration() fixture.expected_ctx->resume_extra.server2.alpn_protocols = OPENSSL_strdup("baz"); - TEST_check( - fixture.expected_ctx->resume_extra.server2.alpn_protocols != NULL); + if (!TEST_ptr(fixture.expected_ctx->resume_extra.server2.alpn_protocols)) + goto err; fixture.expected_ctx->resume_extra.client.ct_validation = SSL_TEST_CT_VALIDATION_STRICT; EXECUTE_SSL_TEST_CTX_TEST(); + +err: + tear_down(fixture); + return 0; } static const char *bad_configurations[] = { diff --git a/test/sslcorrupttest.c b/test/sslcorrupttest.c index c480f2348b..a92e0d79d3 100644 --- a/test/sslcorrupttest.c +++ b/test/sslcorrupttest.c @@ -1,5 +1,5 @@ /* - * Copyright 2016 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 2016-2017 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the OpenSSL license (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -41,8 +41,8 @@ static int tls_corrupt_write(BIO *bio, const char *in, int inl) char *copy; if (docorrupt) { - copy = BUF_memdup(in, inl); - TEST_check(copy != NULL); + if (!TEST_ptr(copy = BUF_memdup(in, inl))) + return 0; /* corrupt last bit of application data */ copy[inl-1] ^= 1; ret = BIO_write(next, copy, inl); @@ -141,15 +141,13 @@ static int setup_cipher_list() { SSL_CTX *ctx = NULL; SSL *ssl = NULL; - static STACK_OF(SSL_CIPHER) *sk_ciphers = NULL; - int i, numciphers; + STACK_OF(SSL_CIPHER) *sk_ciphers = NULL; + int i, j, numciphers = 0; - ctx = SSL_CTX_new(TLS_server_method()); - TEST_check(ctx != NULL); - ssl = SSL_new(ctx); - TEST_check(ssl != NULL); - sk_ciphers = SSL_get1_supported_ciphers(ssl); - TEST_check(sk_ciphers != NULL); + if (!TEST_ptr(ctx = SSL_CTX_new(TLS_server_method())) + || !TEST_ptr(ssl = SSL_new(ctx)) + || !TEST_ptr(sk_ciphers = SSL_get1_supported_ciphers(ssl))) + goto err; /* * The |cipher_list| will be filled only with names of RSA ciphers, @@ -158,16 +156,19 @@ static int setup_cipher_list() */ cipher_list = OPENSSL_malloc(sk_SSL_CIPHER_num(sk_ciphers) * sizeof(cipher_list[0])); - TEST_check(cipher_list != NULL); + if (!TEST_ptr(cipher_list)) + goto err; - for (numciphers = 0, i = 0; i < sk_SSL_CIPHER_num(sk_ciphers); i++) { + for (j = 0, i = 0; i < sk_SSL_CIPHER_num(sk_ciphers); i++) { const SSL_CIPHER *cipher = sk_SSL_CIPHER_value(sk_ciphers, i); if (SSL_CIPHER_get_auth_nid(cipher) == NID_auth_rsa) - cipher_list[numciphers++] = SSL_CIPHER_get_name(cipher); + cipher_list[j++] = SSL_CIPHER_get_name(cipher); } - TEST_check(numciphers != 0); + if (TEST_int_ne(j, 0)) + numciphers = j; +err: sk_SSL_CIPHER_free(sk_ciphers); SSL_free(ssl); SSL_CTX_free(ctx); @@ -249,18 +250,20 @@ static int test_ssl_corrupt(int testidx) int test_main(int argc, char *argv[]) { - int ret; + int ret = EXIT_FAILURE, n; if (argc != 3) { - TEST_error("Usage error"); - return 0; + TEST_error("Usage error: require cert and private key files"); + return ret; } cert = argv[1]; privkey = argv[2]; - ADD_ALL_TESTS(test_ssl_corrupt, setup_cipher_list()); - - ret = run_tests(argv[0]); + n = setup_cipher_list(); + if (n > 0) { + ADD_ALL_TESTS(test_ssl_corrupt, n); + ret = run_tests(argv[0]); + } bio_f_tls_corrupt_filter_free(); OPENSSL_free(cipher_list); diff --git a/test/testutil.h b/test/testutil.h index 1f3c19ff68..8bc561b104 100644 --- a/test/testutil.h +++ b/test/testutil.h @@ -393,7 +393,7 @@ void test_perror(const char *s); /* * For "impossible" conditions such as malloc failures or bugs in test code, * where continuing the test would be meaningless. Note that OPENSSL_assert - * is fatal, and is never compiled out. + * is fatal, and is never compiled out. This macro should be avoided. */ # define TEST_check(condition) \ do { \ -- cgit v1.2.3