Author: kotkov
Date: Wed May 18 16:07:43 2022
New Revision: 1901040

URL: http://svn.apache.org/viewvc?rev=1901040&view=rev
Log:
Fix test_ssl_handshake() and test_ssl_alpn_negotiate() failures with OpenSSL
1.1.1i+.

OpenSSL 1.1.1i changed behavior when verifying certificates with an unknown CA
[1][2]. Now verification continues if user callback does not consider unknown CA
as an error. The change causes ssl_server_cert_cb_expect_failures() to be called
one more time with failures = 0, which results in REPORT_TEST_SUITE_ERROR().

To fix the problem, change the way cert failures are handled. Switch ssl tests
to a new cert callback that logs its every invocation into a string with the
following format:

  cert_cb: failures = ..., cert = (CN=..., depth=...)
  cert_cb: failures = ..., cert = (CN=..., depth=...)
  [...]

This approach has the following advantages:
1. being explicit,
2. being able to detect changes in how the callbacks are called,
3. understandable error messages, and
4. easier debugging of the related failures.

Add expectations for OpenSSL 1.1.1i+ in test_ssl_handshake() and
test_ssl_alpn_negotiate() tests.

test_ssl_renegotiate() revealed historical difference in certificate
verification behavior between OpenSSL 1.0.2 and OpenSSL 1.1.0. Unfortunately,
the true reasons for the difference are unknown.

[1] https://github.com/openssl/openssl/issues/11297
[2] 
https://github.com/openssl/openssl/commit/2e06150e3928daa06d5ff70c32bffad8088ebe58

Patch by: Denis Kovalchuk <denis.kovalchuk{_AT_}visualsvn.com>

* test/test_ssl.c
  (format_cert_failures): New.
  (ssl_server_cert_cb_log): New. Logs failures and cert info into a string log.
  (test_ssl_handshake): Use ssl_server_cert_cb_log and add expectation for
                        OpenSSL 1.1.1i+.
  (test_ssl_alpn_negotiate): Use ssl_server_cert_cb_log and add expectation for
                             OpenSSL 1.1.1i+.
  (test_ssl_renegotiate): Use ssl_server_cert_cb_log and add expectation for
                          OpenSSL 1.1.0+.
  (chain_rootca_callback_conn_setup,
   chain_callback_conn_setup,
   test_ssl_handshake_nosslv2,
   test_ssl_trust_rootca,
   test_ssl_certificate_chain_with_anchor,
   test_ssl_certificate_chain_all_from_server,
   test_ssl_expired_server_cert,
   test_ssl_future_server_cert,
   test_ssl_revoked_server_cert,
   test_setup_ssltunnel,
   test_ssl_ocsp_response_error_and_override,
   test_ssl_server_cert_with_cn_nul_byte,
   test_ssl_server_cert_with_san_nul_byte,
   test_ssl_server_cert_with_cnsan_nul_byte,
   test_ssl_server_cert_with_san_and_empty_cb): Use ssl_server_cert_cb_log.
  (ssl_server_cert_cb_expect_failures,
   ssl_server_cert_cb_expect_allok,
   ssl_server_cert_cb_log_failures,
   ocsp_response_cb_expect_failures): Removed.

* test/test_serf.h
  (TEST_RESULT_OCSP_CHECK_SUCCESSFUL): Removed.

Modified:
    serf/trunk/test/test_serf.h
    serf/trunk/test/test_ssl.c

Modified: serf/trunk/test/test_serf.h
URL: 
http://svn.apache.org/viewvc/serf/trunk/test/test_serf.h?rev=1901040&r1=1901039&r2=1901040&view=diff
==============================================================================
--- serf/trunk/test/test_serf.h (original)
+++ serf/trunk/test/test_serf.h Wed May 18 16:07:43 2022
@@ -159,7 +159,6 @@ typedef struct handler_baton_t {
 #define TEST_RESULT_CLIENT_CERTPWCB_CALLED   0x0008
 #define TEST_RESULT_AUTHNCB_CALLED           0x0010
 #define TEST_RESULT_HANDLE_RESPONSECB_CALLED 0x0020
-#define TEST_RESULT_OCSP_CHECK_SUCCESSFUL    0x0040
 
 serf_bucket_t* accept_response(serf_request_t *request,
                                serf_bucket_t *stream,

Modified: serf/trunk/test/test_ssl.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/test/test_ssl.c?rev=1901040&r1=1901039&r2=1901040&view=diff
==============================================================================
--- serf/trunk/test/test_ssl.c (original)
+++ serf/trunk/test/test_ssl.c Wed May 18 16:07:43 2022
@@ -456,34 +456,116 @@ static apr_status_t validate_rootcacert(
     return APR_SUCCESS;
 }
 
-static apr_status_t
-ssl_server_cert_cb_expect_failures(void *baton, int failures,
-                                   const serf_ssl_certificate_t *cert)
+static const char *format_cert_failures(int failures, apr_pool_t *pool)
 {
-    test_baton_t *tb = baton;
-    int expected_failures = *(int *)tb->user_baton;
+    const char *str = "";
 
-    tb->result_flags |= TEST_RESULT_SERVERCERTCB_CALLED;
+    if (failures & SERF_SSL_CERT_NOTYETVALID) {
+        str = apr_pstrcat(pool, str, *str ? "|" : "", "CERT_NOTYETVALID", 
NULL);
+        failures &= ~SERF_SSL_CERT_NOTYETVALID;
+    }
 
-    /* We expect an error from the certificate validation function. */
-    if (failures & expected_failures)
-        return APR_SUCCESS;
+    if (failures & SERF_SSL_CERT_EXPIRED) {
+        str = apr_pstrcat(pool, str, *str ? "|" : "", "CERT_EXPIRED", NULL);
+        failures &= ~SERF_SSL_CERT_EXPIRED;
+    }
+
+    if (failures & SERF_SSL_CERT_UNKNOWNCA) {
+        str = apr_pstrcat(pool, str, *str ? "|" : "", "CERT_UNKNOWNCA", NULL);
+        failures &= ~SERF_SSL_CERT_UNKNOWNCA;
+    }
+
+    if (failures & SERF_SSL_CERT_SELF_SIGNED) {
+        str = apr_pstrcat(pool, str, *str ? "|" : "", "CERT_SELF_SIGNED", 
NULL);
+        failures &= ~SERF_SSL_CERT_SELF_SIGNED;
+    }
+
+    if (failures & SERF_SSL_CERT_UNKNOWN_FAILURE) {
+        str = apr_pstrcat(pool, str, *str ? "|" : "", "CERT_UNKNOWN_FAILURE", 
NULL);
+        failures &= ~SERF_SSL_CERT_UNKNOWN_FAILURE;
+    }
+
+    if (failures & SERF_SSL_CERT_REVOKED) {
+        str = apr_pstrcat(pool, str, *str ? "|" : "", "CERT_REVOKED", NULL);
+        failures &= ~SERF_SSL_CERT_REVOKED;
+    }
+
+    if (failures & SERF_SSL_CERT_UNABLE_TO_GET_CRL) {
+        str = apr_pstrcat(pool, str, *str ? "|" : "", 
"CERT_UNABLE_TO_GET_CRL", NULL);
+        failures &= ~SERF_SSL_CERT_UNABLE_TO_GET_CRL;
+    }
+
+    if (failures & SERF_SSL_CERT_INVALID_HOST) {
+        str = apr_pstrcat(pool, str, *str ? "|" : "", "CERT_INVALID_HOST", 
NULL);
+        failures &= ~SERF_SSL_CERT_INVALID_HOST;
+    }
+
+    if (failures & SERF_SSL_OCSP_RESPONDER_TRYLATER) {
+        str = apr_pstrcat(pool, str, *str ? "|" : "", 
"OCSP_RESPONDER_TRYLATER", NULL);
+        failures &= ~SERF_SSL_OCSP_RESPONDER_TRYLATER;
+    }
+
+    if (failures & SERF_SSL_OCSP_RESPONDER_ERROR) {
+        str = apr_pstrcat(pool, str, *str ? "|" : "", "OCSP_RESPONDER_ERROR", 
NULL);
+        failures &= ~SERF_SSL_OCSP_RESPONDER_ERROR;
+    }
+
+    if (failures & SERF_SSL_OCSP_RESPONDER_UNKNOWN_FAILURE) {
+        str = apr_pstrcat(pool, str, *str ? "|" : "", 
"OCSP_RESPONDER_UNKNOWN_FAILURE", NULL);
+        failures &= ~SERF_SSL_OCSP_RESPONDER_UNKNOWN_FAILURE;
+    }
+
+    if (failures) {
+        /* Unexpected or unknown cert failure. */
+        REPORT_TEST_SUITE_ERROR();
+        abort();
+    }
+
+    if (*str)
+        return str;
     else
-        return REPORT_TEST_SUITE_ERROR();
+        return "NONE";
 }
 
-static apr_status_t
-ssl_server_cert_cb_expect_allok(void *baton, int failures,
-                                const serf_ssl_certificate_t *cert)
+/* Logs failures in tb->user_baton, for later validation. */
+static apr_status_t ssl_server_cert_cb_log(void *baton, int failures,
+                                           const serf_ssl_certificate_t *cert)
 {
     test_baton_t *tb = baton;
+    const char *cert_str;
+
     tb->result_flags |= TEST_RESULT_SERVERCERTCB_CALLED;
 
-    /* No error expected, certificate is valid. */
-    if (failures)
-        return REPORT_TEST_SUITE_ERROR();
-    else
-        return APR_SUCCESS;
+    if (cert) {
+        apr_hash_t *subject;
+        const char *common_name;
+        int depth;
+
+        subject = serf_ssl_cert_subject(cert, tb->pool);
+        if (!subject)
+            return REPORT_TEST_SUITE_ERROR();
+
+        common_name = apr_hash_get(subject, "CN", APR_HASH_KEY_STRING);
+        depth = serf_ssl_cert_depth(cert);
+
+        cert_str = apr_psprintf(tb->pool, "(CN=%s, depth=%d)", common_name, 
depth);
+    } else {
+        cert_str = "(null)";
+    }
+
+    if (!tb->user_baton)
+        tb->user_baton = "";
+
+    tb->user_baton = apr_pstrcat(
+        tb->pool,
+        tb->user_baton,
+        "cert_cb: "
+        "failures = ", format_cert_failures(failures, tb->pool),
+        ", cert = ", cert_str,
+        "\n",
+        NULL);
+
+    return APR_SUCCESS;
 }
 
 static apr_status_t
@@ -503,7 +585,6 @@ static void test_ssl_handshake(CuTest *t
     test_baton_t *tb = tc->testBaton;
     handler_baton_t handler_ctx[1];
     const int num_requests = sizeof(handler_ctx)/sizeof(handler_ctx[0]);
-    int expected_failures;
     apr_status_t status;
     static const char *server_cert[] = { "serfservercert.pem",
         NULL };
@@ -514,16 +595,10 @@ static void test_ssl_handshake(CuTest *t
                                  server_cert,
                                  test_clientcert_none);
     status = setup_test_client_https_context(tb, NULL,
-                                             
ssl_server_cert_cb_expect_failures,
+                                             ssl_server_cert_cb_log,
                                              tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
 
-    /* This unknown failures is X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE,
-       meaning the chain has only the server cert. A good candidate for its
-       own failure code. */
-    expected_failures = SERF_SSL_CERT_UNKNOWNCA;
-    tb->user_baton = &expected_failures;
-
     Given(tb->mh)
       GETRequest(URLEqualTo("/"), ChunkedBodyEqualTo("1"),
                  HeaderEqualTo("Host", tb->serv_host))
@@ -534,6 +609,26 @@ static void test_ssl_handshake(CuTest *t
 
     run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
                                                 handler_ctx, tb->pool);
+
+    /* OpenSSL 1.1.1i allows to continue verification for certificates with an
+       unknown CA. See https://github.com/openssl/openssl/issues/11297.
+
+       These unknown failures are X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY
+       and X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE. The second one means 
that
+       the chain has only the server cert. A good candidate for its own failure
+       code. */
+#if OPENSSL_VERSION_NUMBER >= 0x1010109fL /* >= 1.1.1i */
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = CERT_UNKNOWNCA, cert = (CN=localhost, depth=0)\n"
+        "cert_cb: failures = CERT_UNKNOWNCA, cert = (CN=localhost, depth=0)\n"
+        "cert_cb: failures = NONE, cert = (CN=localhost, depth=0)\n",
+        tb->user_baton);
+#else
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = CERT_UNKNOWNCA, cert = (CN=localhost, depth=0)\n"
+        "cert_cb: failures = CERT_UNKNOWNCA, cert = (CN=localhost, depth=0)\n",
+        tb->user_baton);
+#endif
 }
 
 /* Validate that connecting to a SSLv2 only server fails. */
@@ -542,7 +637,6 @@ static void test_ssl_handshake_nosslv2(C
     test_baton_t *tb = tc->testBaton;
     handler_baton_t handler_ctx[1];
     const int num_requests = sizeof(handler_ctx)/sizeof(handler_ctx[0]);
-    int expected_failures;
     apr_status_t status;
     static const char *server_cert[] = { "serfservercert.pem",
         NULL };
@@ -566,16 +660,10 @@ static void test_ssl_handshake_nosslv2(C
     tb->serv_url = apr_psprintf(tb->pool, "https://%s";, tb->serv_host);
 
     status = setup_test_client_https_context(tb, NULL,
-                                             
ssl_server_cert_cb_expect_failures,
+                                             ssl_server_cert_cb_log,
                                              tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
 
-    /* This unknown failures is X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE,
-       meaning the chain has only the server cert. A good candidate for its
-       own failure code. */
-    expected_failures = SERF_SSL_CERT_UNKNOWNCA;
-    tb->user_baton = &expected_failures;
-
     Given(tb->mh)
       GETRequest(URLEqualTo("/"), ChunkedBodyEqualTo("1"),
                  HeaderEqualTo("Host", tb->serv_host))
@@ -588,6 +676,9 @@ static void test_ssl_handshake_nosslv2(C
                                                handler_ctx, tb->pool);
     CuAssert(tc, "Serf does not disable SSLv2, but it should!",
              status != APR_SUCCESS);
+
+    /* There are no ssl_server_cert_cb_log calls. */
+    CuAssertStrEquals(tc, NULL, tb->user_baton);
 }
 
 /* Set up the ssl context with the CA and root CA certificates needed for
@@ -636,7 +727,7 @@ static void test_ssl_trust_rootca(CuTest
                                  test_clientcert_none);
     status = setup_test_client_https_context(tb,
                                              https_set_root_ca_conn_setup,
-                                             ssl_server_cert_cb_expect_allok,
+                                             ssl_server_cert_cb_log,
                                              tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
 
@@ -650,7 +741,9 @@ static void test_ssl_trust_rootca(CuTest
 
     run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
                                                 handler_ctx, tb->pool);
-    CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCB_CALLED);
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = NONE, cert = (CN=localhost, depth=0)\n",
+        tb->user_baton);
 }
 
 /* Validate that when the application rejects the cert, the context loop
@@ -740,7 +833,7 @@ chain_rootca_callback_conn_setup(apr_soc
         return status;
 
     serf_ssl_server_cert_chain_callback_set(tb->ssl_context,
-                                            ssl_server_cert_cb_expect_allok,
+                                            ssl_server_cert_cb_log,
                                             cert_chain_cb,
                                             tb);
 
@@ -763,7 +856,7 @@ static void test_ssl_certificate_chain_w
                                  test_clientcert_none);
     status = setup_test_client_https_context(tb,
                                              chain_rootca_callback_conn_setup,
-                                             ssl_server_cert_cb_expect_allok,
+                                             ssl_server_cert_cb_log,
                                              tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
 
@@ -777,7 +870,9 @@ static void test_ssl_certificate_chain_w
 
     run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
                                                 handler_ctx, tb->pool);
-    CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCB_CALLED);
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = NONE, cert = (CN=localhost, depth=0)\n",
+        tb->user_baton);
     CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCHAINCB_CALLED);
 }
 
@@ -810,7 +905,7 @@ chain_callback_conn_setup(apr_socket_t *
         return status;
 
     serf_ssl_server_cert_chain_callback_set(tb->ssl_context,
-                                            ssl_server_cert_cb_expect_allok,
+                                            ssl_server_cert_cb_log,
                                             cert_chain_all_certs_cb,
                                             tb);
 
@@ -832,7 +927,7 @@ static void test_ssl_certificate_chain_a
                                  test_clientcert_none);
     status = setup_test_client_https_context(tb,
                                              chain_callback_conn_setup,
-                                             ssl_server_cert_cb_expect_allok,
+                                             ssl_server_cert_cb_log,
                                              tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
 
@@ -847,7 +942,10 @@ static void test_ssl_certificate_chain_a
     run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
                                                 handler_ctx, tb->pool);
 
-    CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCB_CALLED);
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = CERT_SELF_SIGNED, cert = (CN=Serf Root CA, 
depth=2)\n"
+        "cert_cb: failures = NONE, cert = (CN=localhost, depth=0)\n",
+        tb->user_baton);
     CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCHAINCB_CALLED);
 }
 
@@ -1090,7 +1188,6 @@ static void test_ssl_expired_server_cert
     test_baton_t *tb = tc->testBaton;
     handler_baton_t handler_ctx[1];
     const int num_requests = sizeof(handler_ctx)/sizeof(handler_ctx[0]);
-    int expected_failures;
     apr_status_t status;
 
     static const char *expired_server_certs[] = {
@@ -1105,14 +1202,10 @@ static void test_ssl_expired_server_cert
                                  test_clientcert_none);
     status = setup_test_client_https_context(tb,
                                              NULL, /* default conn setup */
-                                             
ssl_server_cert_cb_expect_failures,
+                                             ssl_server_cert_cb_log,
                                              tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
 
-    expected_failures = SERF_SSL_CERT_SELF_SIGNED |
-                        SERF_SSL_CERT_EXPIRED;
-    tb->user_baton = &expected_failures;
-
     Given(tb->mh)
       GETRequest(URLEqualTo("/"), ChunkedBodyEqualTo("1"),
                  HeaderEqualTo("Host", tb->serv_host))
@@ -1123,8 +1216,11 @@ static void test_ssl_expired_server_cert
 
     run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
                                                 handler_ctx, tb->pool);
-    CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCB_CALLED);
-
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = CERT_SELF_SIGNED, cert = (CN=Serf Root CA, 
depth=2)\n"
+        "cert_cb: failures = CERT_EXPIRED, cert = (CN=localhost, depth=0)\n"
+        "cert_cb: failures = CERT_EXPIRED, cert = (CN=localhost, depth=0)\n",
+        tb->user_baton);
 }
 
 /* Validate that the expired certificate is reported as failure in the
@@ -1134,7 +1230,6 @@ static void test_ssl_future_server_cert(
     test_baton_t *tb = tc->testBaton;
     handler_baton_t handler_ctx[1];
     const int num_requests = sizeof(handler_ctx)/sizeof(handler_ctx[0]);
-    int expected_failures;
     apr_status_t status;
 
     static const char *future_server_certs[] = {
@@ -1149,14 +1244,10 @@ static void test_ssl_future_server_cert(
                                  test_clientcert_none);
     status = setup_test_client_https_context(tb,
                                              NULL, /* default conn setup */
-                                             
ssl_server_cert_cb_expect_failures,
+                                             ssl_server_cert_cb_log,
                                              tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
 
-    expected_failures = SERF_SSL_CERT_SELF_SIGNED |
-                        SERF_SSL_CERT_NOTYETVALID;
-    tb->user_baton = &expected_failures;
-
     Given(tb->mh)
       GETRequest(URLEqualTo("/"), ChunkedBodyEqualTo("1"),
                  HeaderEqualTo("Host", tb->serv_host))
@@ -1167,7 +1258,11 @@ static void test_ssl_future_server_cert(
 
     run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
                                                 handler_ctx, tb->pool);
-    CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCB_CALLED);
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = CERT_SELF_SIGNED, cert = (CN=Serf Root CA, 
depth=2)\n"
+        "cert_cb: failures = CERT_NOTYETVALID, cert = (CN=localhost, 
depth=0)\n"
+        "cert_cb: failures = CERT_NOTYETVALID, cert = (CN=localhost, 
depth=0)\n",
+        tb->user_baton);
 }
 
 
@@ -1197,42 +1292,12 @@ https_load_crl_conn_setup(apr_socket_t *
     return status;
 }
 
-/* Logs failures for each depth in tb->user_baton, for later validation. */
-/* TODO: use this in place of ssl_server_cert_cb_expect_failures */
-static apr_status_t
-ssl_server_cert_cb_log_failures(void *baton, int failures,
-                                const serf_ssl_certificate_t *cert)
-{
-    test_baton_t *tb = baton;
-    apr_array_header_t *failure_list = (apr_array_header_t *)tb->user_baton;
-    int depth = serf_ssl_cert_depth(cert);
-    int *failures_for_depth;
-
-    if (!tb->user_baton) {
-        /* make the array big enough to not have to worry about resizing */
-        tb->user_baton = apr_array_make(tb->pool, 10, sizeof(int));
-    }
-    failure_list = tb->user_baton;
-
-    if (depth + 1 > failure_list->nalloc) {
-        return REPORT_TEST_SUITE_ERROR();
-    }
-
-    failures_for_depth = &APR_ARRAY_IDX(failure_list, depth, int);
-    *failures_for_depth |= failures;
-
-    tb->result_flags |= TEST_RESULT_SERVERCERTCB_CALLED;
-
-    return APR_SUCCESS;
-}
-
 /* Validate that a CRL file can be loaded and revocation actually works. */
 static void test_ssl_revoked_server_cert(CuTest *tc)
 {
     test_baton_t *tb = tc->testBaton;
     handler_baton_t handler_ctx[1];
     const int num_requests = sizeof(handler_ctx)/sizeof(handler_ctx[0]);
-    int depth;
     apr_status_t status;
 
     static const char *future_server_certs[] = {
@@ -1241,28 +1306,14 @@ static void test_ssl_revoked_server_cert
         "serfrootcacert.pem",
         NULL };
 
-    static int expected_failures[] = {
-        SERF_SSL_CERT_REVOKED,
-        SERF_SSL_CERT_UNABLE_TO_GET_CRL,
-        SERF_SSL_CERT_UNABLE_TO_GET_CRL
-    };
-
     /* Set up a test context and a https server */
     setup_test_mock_https_server(tb, server_key,
                                  future_server_certs,
                                  test_clientcert_none);
 
-    /* OpenSSL first checks the revocation status before verifying the rest of
-       certificate. OpenSSL may call the application multiple times per depth,
-       e.g. once to tell that the cert is revoked, and a second time to tell
-       that the certificate itself is valid.
-       We'll have to combine the results of these multiple calls to the 
callback
-       to get a complete view of the certificate. That's why we use
-       ssl_server_cert_cb_log_failures here, and then later check expected
-       failures for each depth. */
     status = setup_test_client_https_context(tb,
                                              https_load_crl_conn_setup,
-                                             ssl_server_cert_cb_log_failures,
+                                             ssl_server_cert_cb_log,
                                              tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
 
@@ -1276,14 +1327,16 @@ static void test_ssl_revoked_server_cert
 
     run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
                                                 handler_ctx, tb->pool);
-    CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCB_CALLED);
-    for (depth = 0; depth < 3; depth++) {
-        apr_array_header_t *ary = tb->user_baton;
-
-        CuAssertIntEquals(tc, expected_failures[depth],
-                          APR_ARRAY_IDX(ary, depth, int));
-
-    }
+    /* OpenSSL first checks the revocation status before verifying the rest of
+       certificate. OpenSSL may call the application multiple times per depth,
+       e.g. once to tell that the cert is revoked, and a second time to tell
+       that the certificate itself is valid. */
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = CERT_REVOKED, cert = (CN=localhost, depth=0)\n"
+        "cert_cb: failures = CERT_UNABLE_TO_GET_CRL, cert = (CN=Serf CA, 
depth=1)\n"
+        "cert_cb: failures = CERT_UNABLE_TO_GET_CRL, cert = (CN=Serf Root CA, 
depth=2)\n"
+        "cert_cb: failures = NONE, cert = (CN=localhost, depth=0)\n",
+        tb->user_baton);
 }
 
 /* Test if serf is sets up an SSL tunnel to the proxy and doesn't contact the
@@ -1304,7 +1357,7 @@ static void test_setup_ssltunnel(CuTest
     CuAssertIntEquals(tc, APR_SUCCESS, setup_test_mock_proxy(tb));
     CuAssertIntEquals(tc, APR_SUCCESS,
             setup_serf_https_context_with_proxy(tb, chain_callback_conn_setup,
-                                                
ssl_server_cert_cb_expect_allok,
+                                                ssl_server_cert_cb_log,
                                                 tb->pool));
 
     Given(tb->mh)
@@ -1329,6 +1382,11 @@ static void test_setup_ssltunnel(CuTest
         int req_nr = APR_ARRAY_IDX(tb->handled_requests, i, int);
         CuAssertIntEquals(tc, i + 1, req_nr);
     }
+
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = CERT_SELF_SIGNED, cert = (CN=Serf Root CA, 
depth=2)\n"
+        "cert_cb: failures = NONE, cert = (CN=localhost, depth=0)\n",
+        tb->user_baton);
 }
 
 /* Test error if no creds callback */
@@ -1854,7 +1912,7 @@ static void test_ssl_renegotiate(CuTest
                                  test_clientcert_none);
     status = setup_test_client_https_context(tb,
                                              https_set_root_ca_conn_setup,
-                                             ssl_server_cert_cb_expect_allok,
+                                             ssl_server_cert_cb_log,
                                              tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
 
@@ -1880,6 +1938,20 @@ static void test_ssl_renegotiate(CuTest
                                                tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
 
+    /* There is some historical difference in certificate verification behavior
+       between OpenSSL 1.0.2 and OpenSSL 1.1.0. Unfortunately, the true reasons
+       for the difference are unknown. */
+#if OPENSSL_VERSION_NUMBER >= 0x10100000L /* >= 1.1.0 */
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = NONE, cert = (CN=localhost, depth=0)\n",
+        tb->user_baton);
+#else
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = NONE, cert = (CN=localhost, depth=0)\n"
+        "cert_cb: failures = NONE, cert = (CN=localhost, depth=0)\n",
+        tb->user_baton);
+#endif
+
     /* Check that the requests were sent and reveived by the server */
     /* Note: the test server will have received the first request twice, so
        we can't check for VerifyAllRequestsReceivedInOrder here. */
@@ -1989,31 +2061,6 @@ static void test_connect_to_non_http_ser
     CuAssertTrue(tc, tb->result_flags & TEST_RESULT_HANDLE_RESPONSECB_CALLED);
 }
 
-static apr_status_t
-ocsp_response_cb_expect_failures(void *baton, int failures,
-                                     const serf_ssl_certificate_t *cert)
-{
-    test_baton_t *tb = baton;
-    int expected_failures = tb->user_baton_l;
-
-    /* Root CA cert is selfsigned, ignore this 'failure'. */
-    failures &= ~SERF_SSL_CERT_SELF_SIGNED;
-
-    /* This callback gets called with both the server certificate and the ocsp
-       response status */
-    tb->result_flags |= TEST_RESULT_SERVERCERTCB_CALLED;
-
-    /* We expect an error from the certificate validation function. */
-    if (!failures)
-        return APR_SUCCESS;
-    if (failures & expected_failures) {
-        tb->result_flags |= TEST_RESULT_OCSP_CHECK_SUCCESSFUL;
-        return APR_SUCCESS;
-    }
-
-    return REPORT_TEST_SUITE_ERROR();
-}
-
 static void test_ssl_ocsp_response_error_and_override(CuTest *tc)
 {
     test_baton_t *tb = tc->testBaton;
@@ -2027,7 +2074,7 @@ static void test_ssl_ocsp_response_error
                                  test_clientcert_none);
     status = setup_test_client_https_context(tb,
                                              default_https_conn_setup,
-                                             ocsp_response_cb_expect_failures,
+                                             ssl_server_cert_cb_log,
                                              tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
     tb->enable_ocsp_stapling = 1;
@@ -2043,17 +2090,23 @@ static void test_ssl_ocsp_response_error
                  HeaderEqualTo("Host", tb->serv_host))
         Respond(WithCode(200), WithChunkedBody(""))
     EndGiven
-    /* Expect this error */
-    tb->user_baton_l = SERF_SSL_OCSP_RESPONDER_ERROR;
 
     create_new_request(tb, &handler_ctx[0], "GET", "/", 1);
 
     run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
                                                 handler_ctx, tb->pool);
 
-    CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCB_CALLED);
 #if !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_OCSP)
-    CuAssertTrue(tc, tb->result_flags & TEST_RESULT_OCSP_CHECK_SUCCESSFUL);
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = CERT_SELF_SIGNED, cert = (CN=Serf Root CA, 
depth=2)\n"
+        "cert_cb: failures = NONE, cert = (CN=localhost, depth=0)\n"
+        "cert_cb: failures = OCSP_RESPONDER_ERROR, cert = (null)\n",
+        tb->user_baton);
+#else
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = CERT_SELF_SIGNED, cert = (CN=Serf Root CA, 
depth=2)\n"
+        "cert_cb: failures = NONE, cert = (CN=localhost, depth=0)\n",
+        tb->user_baton);
 #endif
 }
 
@@ -2064,7 +2117,6 @@ static void test_ssl_server_cert_with_cn
     test_baton_t *tb = tc->testBaton;
     handler_baton_t handler_ctx[1];
     const int num_requests = sizeof(handler_ctx)/sizeof(handler_ctx[0]);
-    int expected_failures;
     apr_status_t status;
 
     static const char *nul_byte_server_certs[] = {
@@ -2079,14 +2131,10 @@ static void test_ssl_server_cert_with_cn
                                  test_clientcert_none);
     status = setup_test_client_https_context(tb,
                                              NULL, /* default conn setup */
-                                             
ssl_server_cert_cb_expect_failures,
+                                             ssl_server_cert_cb_log,
                                              tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
 
-    expected_failures = SERF_SSL_CERT_SELF_SIGNED |
-                        SERF_SSL_CERT_INVALID_HOST; /* NUL byte error */
-    tb->user_baton = &expected_failures;
-
     Given(tb->mh)
       GETRequest(URLEqualTo("/"), ChunkedBodyEqualTo("1"),
                  HeaderEqualTo("Host", tb->serv_host))
@@ -2097,7 +2145,10 @@ static void test_ssl_server_cert_with_cn
 
     run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
                                                 handler_ctx, tb->pool);
-    CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCB_CALLED);
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = CERT_SELF_SIGNED, cert = (CN=(null), depth=1)\n"
+        "cert_cb: failures = CERT_INVALID_HOST, cert = 
(CN=www.example.net\\00.example.com, depth=0)\n",
+        tb->user_baton);
 }
 
 /* Validate that the subject's SAN containing a '\0' byte is reported as 
failure
@@ -2107,7 +2158,6 @@ static void test_ssl_server_cert_with_sa
     test_baton_t *tb = tc->testBaton;
     handler_baton_t handler_ctx[1];
     const int num_requests = sizeof(handler_ctx)/sizeof(handler_ctx[0]);
-    int expected_failures;
     apr_status_t status;
 
     static const char *nul_byte_server_certs[] = {
@@ -2122,14 +2172,10 @@ static void test_ssl_server_cert_with_sa
                                  test_clientcert_none);
     status = setup_test_client_https_context(tb,
                                              NULL, /* default conn setup */
-                                             
ssl_server_cert_cb_expect_failures,
+                                             ssl_server_cert_cb_log,
                                              tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
 
-    expected_failures = SERF_SSL_CERT_SELF_SIGNED |
-                        SERF_SSL_CERT_INVALID_HOST; /* NUL byte error */
-    tb->user_baton = &expected_failures;
-
     Given(tb->mh)
       GETRequest(URLEqualTo("/"), ChunkedBodyEqualTo("1"),
                  HeaderEqualTo("Host", tb->serv_host))
@@ -2140,7 +2186,10 @@ static void test_ssl_server_cert_with_sa
 
     run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
                                                 handler_ctx, tb->pool);
-    CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCB_CALLED);
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = CERT_SELF_SIGNED, cert = (CN=(null), depth=1)\n"
+        "cert_cb: failures = CERT_INVALID_HOST, cert = (CN=www.example.com, 
depth=0)\n",
+        tb->user_baton);
 }
 
 /* Validate that the subject's CN and SAN containing a '\0' byte is reported
@@ -2150,7 +2199,6 @@ static void test_ssl_server_cert_with_cn
     test_baton_t *tb = tc->testBaton;
     handler_baton_t handler_ctx[1];
     const int num_requests = sizeof(handler_ctx)/sizeof(handler_ctx[0]);
-    int expected_failures;
     apr_status_t status;
 
     static const char *nul_byte_server_certs[] = {
@@ -2165,14 +2213,10 @@ static void test_ssl_server_cert_with_cn
                                  test_clientcert_none);
     status = setup_test_client_https_context(tb,
                                              NULL, /* default conn setup */
-                                             
ssl_server_cert_cb_expect_failures,
+                                             ssl_server_cert_cb_log,
                                              tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
 
-    expected_failures = SERF_SSL_CERT_SELF_SIGNED |
-                        SERF_SSL_CERT_INVALID_HOST; /* NUL byte error */
-    tb->user_baton = &expected_failures;
-
     Given(tb->mh)
       GETRequest(URLEqualTo("/"), ChunkedBodyEqualTo("1"),
                  HeaderEqualTo("Host", tb->serv_host))
@@ -2183,7 +2227,10 @@ static void test_ssl_server_cert_with_cn
 
     run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
                                                 handler_ctx, tb->pool);
-    CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCB_CALLED);
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = CERT_SELF_SIGNED, cert = (CN=(null), depth=1)\n"
+        "cert_cb: failures = CERT_INVALID_HOST, cert = 
(CN=www.example.net\\00.example.com, depth=0)\n",
+        tb->user_baton);
 }
 
 /* Validate a certificate with subjectAltName a DNS entry, but no CN. */
@@ -2205,7 +2252,7 @@ static void test_ssl_server_cert_with_sa
                                  test_clientcert_none);
     status = setup_test_client_https_context(tb,
                                              https_set_root_ca_conn_setup,
-                                             ssl_server_cert_cb_expect_allok,
+                                             ssl_server_cert_cb_log,
                                              tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
 
@@ -2219,7 +2266,9 @@ static void test_ssl_server_cert_with_sa
 
     run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
                                                 handler_ctx, tb->pool);
-    CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCB_CALLED);
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = NONE, cert = (CN=(null), depth=0)\n",
+        tb->user_baton);
 }
 
 #ifndef OPENSSL_NO_TLSEXT
@@ -2271,7 +2320,6 @@ static void test_ssl_alpn_negotiate(CuTe
     test_baton_t *tb = tc->testBaton;
     handler_baton_t handler_ctx[1];
     const int num_requests = sizeof(handler_ctx)/sizeof(handler_ctx[0]);
-    int expected_failures;
     apr_status_t status;
     static const char *server_cert[] = { "serfservercert.pem",
                                          NULL };
@@ -2295,13 +2343,10 @@ static void test_ssl_alpn_negotiate(CuTe
 
     status = setup_test_client_https_context(tb,
                                              http11_alpn_setup,
-                                             
ssl_server_cert_cb_expect_failures,
+                                             ssl_server_cert_cb_log,
                                              tb->pool);
     CuAssertIntEquals(tc, APR_SUCCESS, status);
 
-    expected_failures = SERF_SSL_CERT_UNKNOWNCA;
-    tb->user_baton = &expected_failures;
-
     Given(tb->mh)
       GETRequest(URLEqualTo("/"), ChunkedBodyEqualTo("1"),
                  HeaderEqualTo("Host", tb->serv_host))
@@ -2312,6 +2357,25 @@ static void test_ssl_alpn_negotiate(CuTe
 
     run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
                                                 handler_ctx, tb->pool);
+    /* OpenSSL 1.1.1i allows to continue verification for certificates with an
+       unknown CA. See https://github.com/openssl/openssl/issues/11297.
+
+       These unknown failures are X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY
+       and X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE. The second one means 
that
+       the chain has only the server cert. A good candidate for its own failure
+       code. */
+#if OPENSSL_VERSION_NUMBER >= 0x1010109fL /* >= 1.1.1i */
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = CERT_UNKNOWNCA, cert = (CN=localhost, depth=0)\n"
+        "cert_cb: failures = CERT_UNKNOWNCA, cert = (CN=localhost, depth=0)\n"
+        "cert_cb: failures = NONE, cert = (CN=localhost, depth=0)\n",
+        tb->user_baton);
+#else
+    CuAssertStrEquals(tc,
+        "cert_cb: failures = CERT_UNKNOWNCA, cert = (CN=localhost, depth=0)\n"
+        "cert_cb: failures = CERT_UNKNOWNCA, cert = (CN=localhost, depth=0)\n",
+        tb->user_baton);
+#endif
 #endif /* OPENSSL_NO_TLSEXT */
 }
 


Reply via email to