Hello! OpenSSL 1.1.1i changed behaviour when verifying certificates with an unknown CA [1]. Now verification continues if user callback does not consider unknown CA as an error.
The change affects test_ssl_handshake() and test_ssl_alpn_negotiate() tests that use ssl_server_cert_cb_expect_failures() callback with SERF_SSL_CERT_UNKNOWNCA. The callback will now be called one more time with failures = 0, which results in REPORT_TEST_SUITE_ERROR(). —--------------------------- There were 2 failures: 1) test_ssl_handshake: test/test_util.c:551: expected <0> but was <120199> 2) test_ssl_alpn_negotiate: test/test_util.c:551: expected <0> but was <120199> !!!FAILURES!!! Runs: 127 Passes: 125 Fails: 2 —--------------------------- I would like to suggest a fix that uses ssl_server_cert_cb_log_failures() callback for these tests and checks certificate failures on the call side. P.S. It's likely that the same problem was discussed in "Bug#978353: serf: FTBFS: test_ssl_handshake fails with OpenSSL 1.1.1i". [1] https://github.com/openssl/openssl/commit/2e06150e3928daa06d5ff70c32bffad8088ebe58 Best Regards, Denis Kovalchuk
Fix test_ssl_handshake() and test_ssl_alpn_negotiate() failures with OpenSSL 1.1.1i+. OpenSSL 1.1.1i changed behaviour when verifying certificates with an unknown CA [1]. 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 these tests, use ssl_server_cert_cb_log_failures() callback and check certificate failures on the call side. [1] https://github.com/openssl/openssl/commit/2e06150e3928daa06d5ff70c32bffad8088ebe58 * test/test_ssl.c: (ssl_server_cert_cb_log_failures): Move above to be available for test_ssl_handshake() and test_ssl_alpn_negotiate(). (test_ssl_handshake): Use ssl_server_cert_cb_log_failures. (test_ssl_alpn_negotiate): Use ssl_server_cert_cb_log_failures. Index: test/test_ssl.c =================================================================== --- test/test_ssl.c (revision 1900545) +++ test/test_ssl.c (working copy) @@ -496,6 +496,35 @@ ssl_server_cert_cb_reject(void *baton, int failure return REPORT_TEST_SUITE_ERROR(); } +/* 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 we can connect successfully to an https server. This certificate is not trusted, so a cert validation failure is expected. */ static void test_ssl_handshake(CuTest *tc) @@ -503,7 +532,6 @@ static void test_ssl_handshake(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 expected_failures; apr_status_t status; static const char *server_cert[] = { "serfservercert.pem", NULL }; @@ -514,16 +542,10 @@ static void test_ssl_handshake(CuTest *tc) server_cert, test_clientcert_none); status = setup_test_client_https_context(tb, NULL, - ssl_server_cert_cb_expect_failures, + ssl_server_cert_cb_log_failures, 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 +556,13 @@ static void test_ssl_handshake(CuTest *tc) 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); + apr_array_header_t *failure_list = tb->user_baton; + /* 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. */ + CuAssertIntEquals(tc, SERF_SSL_CERT_UNKNOWNCA, + APR_ARRAY_IDX(failure_list, 0, int)); } /* Validate that connecting to a SSLv2 only server fails. */ @@ -1197,35 +1226,6 @@ https_load_crl_conn_setup(apr_socket_t *skt, 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) { @@ -2271,7 +2271,6 @@ static void test_ssl_alpn_negotiate(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 expected_failures; apr_status_t status; static const char *server_cert[] = { "serfservercert.pem", NULL }; @@ -2295,13 +2294,10 @@ static void test_ssl_alpn_negotiate(CuTest *tc) status = setup_test_client_https_context(tb, http11_alpn_setup, - ssl_server_cert_cb_expect_failures, + ssl_server_cert_cb_log_failures, 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 +2308,13 @@ static void test_ssl_alpn_negotiate(CuTest *tc) 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); + apr_array_header_t *failure_list = tb->user_baton; + /* 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. */ + CuAssertIntEquals(tc, SERF_SSL_CERT_UNKNOWNCA, + APR_ARRAY_IDX(failure_list, 0, int)); #endif /* OPENSSL_NO_TLSEXT */ }
