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 <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3750)

6 files changed, 114 insertions, 95 deletions

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 <stdio.h>
+#include <string.h>
 
 #include <openssl/opensslconf.h>
 #include <openssl/err.h>
@@ -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 {                                        \