TLS client: allow cert verify callback return -1 for SSL_ERROR_WANT_RETRY_VERIFY

The client-side cert verification callback function may not only return
as usual for success or 0 for failure, but also -1,
typically on failure verifying the server certificate.
This makes the handshake suspend and return control to the calling application
with SSL_ERROR_WANT_RETRY_VERIFY.
The app can for instance fetch further certificates or cert status information
needed for the verification.
Calling SSL_connect() again resumes the connection attempt
by retrying the server certificate verification step.
This process may even be repeated if need be.

The core implementation of the feature is in ssl/statem/statem_clnt.c,
splitting tls_process_server_certificate() into a preparation step
that just copies the certificates received from the server to s->session->peer_chain
(rather than having them in a local variable at first) and returns to the state machine,
and a post-processing step in tls_post_process_server_certificate() that can be repeated:
Try verifying the current contents of s->session->peer_chain basically as before,
but give the verification callback function the chance to pause connecting and
make the TLS state machine later call tls_post_process_server_certificate() again.
Otherwise processing continues as usual.

The documentation of the new feature is added to SSL_CTX_set_cert_verify_callback.pod
and SSL_want.pod.

This adds two tests:
* A generic test in test/helpers/handshake.c
  on the usability of the new server cert verification retry feature.
  It is triggered via test/ssl-tests/03-custom_verify.cnf.in (while the bulky auto-
  generated changes to test/ssl-tests/03-custom_verify.cnf can be basically ignored).
* A test in test/sslapitest.c that demonstrates the effectiveness of the approach
  for augmenting the cert chain provided by the server in between SSL_connect() calls.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13906)

7 files changed, 213 insertions, 64 deletions

diff --git a/test/helpers/handshake.c b/test/helpers/handshake.c
index e286df6cf0..03ee9a004a 100644
--- a/test/helpers/handshake.c
+++ b/test/helpers/handshake.c
@@ -314,6 +314,14 @@ static int verify_reject_cb(X509_STORE_CTX *ctx, void *arg) {
     return 0;
 }
 
+static int n_retries = 0;
+static int verify_retry_cb(X509_STORE_CTX *ctx, void *arg) {
+    if (--n_retries < 0)
+        return 1;
+    X509_STORE_CTX_set_error(ctx, X509_V_ERR_APPLICATION_VERIFICATION);
+    return -1;
+}
+
 static int verify_accept_cb(X509_STORE_CTX *ctx, void *arg) {
     return 1;
 }
@@ -527,6 +535,10 @@ static int configure_handshake_ctx(SSL_CTX *server_ctx, SSL_CTX *server2_ctx,
     case SSL_TEST_VERIFY_ACCEPT_ALL:
         SSL_CTX_set_cert_verify_callback(client_ctx, &verify_accept_cb, NULL);
         break;
+    case SSL_TEST_VERIFY_RETRY_ONCE:
+        n_retries = 1;
+        SSL_CTX_set_cert_verify_callback(client_ctx, &verify_retry_cb, NULL);
+        break;
     case SSL_TEST_VERIFY_REJECT_ALL:
         SSL_CTX_set_cert_verify_callback(client_ctx, &verify_reject_cb, NULL);
         break;
@@ -807,8 +819,10 @@ static void do_handshake_step(PEER *peer)
             peer->status = PEER_ERROR;
         } else {
             int error = SSL_get_error(peer->ssl, ret);
+
             /* Memory bios should never block with SSL_ERROR_WANT_WRITE. */
-            if (error != SSL_ERROR_WANT_READ)
+            if (error != SSL_ERROR_WANT_READ
+                    && error != SSL_ERROR_WANT_RETRY_VERIFY)
                 peer->status = PEER_ERROR;
         }
     }
@@ -1674,6 +1688,10 @@ static HANDSHAKE_RESULT *do_handshake_internal(
         ret->session_id = SSL_TEST_SESSION_ID_YES;
     ret->session_ticket_do_not_call = server_ex_data.session_ticket_do_not_call;
 
+    if (extra->client.verify_callback == SSL_TEST_VERIFY_RETRY_ONCE
+            && n_retries != -1)
+        ret->result = SSL_TEST_SERVER_FAIL;
+
 #ifndef OPENSSL_NO_NEXTPROTONEG
     SSL_get0_next_proto_negotiated(client.ssl, &proto, &proto_len);
     ret->client_npn_negotiated = dup_str(proto, proto_len);
diff --git a/test/helpers/ssl_test_ctx.c b/test/helpers/ssl_test_ctx.c
index db4752c54f..0921416be2 100644
--- a/test/helpers/ssl_test_ctx.c
+++ b/test/helpers/ssl_test_ctx.c
@@ -177,6 +177,7 @@ const char *ssl_protocol_name(int protocol)
 static const test_enum ssl_verify_callbacks[] = {
     {"None", SSL_TEST_VERIFY_NONE},
     {"AcceptAll", SSL_TEST_VERIFY_ACCEPT_ALL},
+    {"RetryOnce", SSL_TEST_VERIFY_RETRY_ONCE},
     {"RejectAll", SSL_TEST_VERIFY_REJECT_ALL},
 };
 
@@ -184,6 +185,7 @@ __owur static int parse_client_verify_callback(SSL_TEST_CLIENT_CONF *client_conf
                                                const char *value)
 {
     int ret_value;
+
     if (!parse_enum(ssl_verify_callbacks, OSSL_NELEM(ssl_verify_callbacks),
                     &ret_value, value)) {
         return 0;
diff --git a/test/helpers/ssl_test_ctx.h b/test/helpers/ssl_test_ctx.h
index 4e8d3df4ac..1c9451b751 100644
--- a/test/helpers/ssl_test_ctx.h
+++ b/test/helpers/ssl_test_ctx.h
@@ -25,6 +25,7 @@ typedef enum {
 typedef enum {
     SSL_TEST_VERIFY_NONE = 0, /* Default */
     SSL_TEST_VERIFY_ACCEPT_ALL,
+    SSL_TEST_VERIFY_RETRY_ONCE,
     SSL_TEST_VERIFY_REJECT_ALL
 } ssl_verify_callback_t;
 
diff --git a/test/recipes/80-test_ssl_new.t b/test/recipes/80-test_ssl_new.t
index e2b9349d04..6f404b5376 100644
--- a/test/recipes/80-test_ssl_new.t
+++ b/test/recipes/80-test_ssl_new.t
@@ -157,12 +157,12 @@ sub test_conf {
            "Getting output from generate_ssl_tests.pl.");
 
     SKIP: {
-        # Test 2. Compare against existing output in test/ssl_tests.cnf.
+        # Test 2. Compare against existing output in test/ssl-tests/
         skip "Skipping generated source test for $conf", 1
           if !$check_source;
 
         $run_test = is(cmp_text($output_file, $conf_file), 0,
-                       "Comparing generated sources.");
+                       "Comparing generated $output_file with $conf_file.");
       }
 
       # Test 3. Run the test.
diff --git a/test/ssl-tests/03-custom_verify.cnf b/test/ssl-tests/03-custom_verify.cnf
index 8dca715e74..e107b93b5b 100644
--- a/test/ssl-tests/03-custom_verify.cnf
+++ b/test/ssl-tests/03-custom_verify.cnf
@@ -1,16 +1,17 @@
 # Generated with generate_ssl_tests.pl
 
-num_tests = 9
+num_tests = 10
 
 test-0 = 0-verify-success
 test-1 = 1-verify-custom-reject
 test-2 = 2-verify-custom-allow
-test-3 = 3-noverify-success
-test-4 = 4-noverify-ignore-custom-reject
-test-5 = 5-noverify-accept-custom-allow
-test-6 = 6-verify-fail-no-root
-test-7 = 7-verify-custom-success-no-root
-test-8 = 8-verify-custom-fail-no-root
+test-3 = 3-verify-custom-retry
+test-4 = 4-noverify-success
+test-5 = 5-noverify-ignore-custom-reject
+test-6 = 6-noverify-accept-custom-allow
+test-7 = 7-verify-fail-no-root
+test-8 = 8-verify-custom-success-no-root
+test-9 = 9-verify-custom-fail-no-root
 # ===========================================================
 
 [0-verify-success]
@@ -91,148 +92,175 @@ VerifyCallback = AcceptAll
 
 # ===========================================================
 
-[3-noverify-success]
-ssl_conf = 3-noverify-success-ssl
+[3-verify-custom-retry]
+ssl_conf = 3-verify-custom-retry-ssl
 
-[3-noverify-success-ssl]
-server = 3-noverify-success-server
-client = 3-noverify-success-client
+[3-verify-custom-retry-ssl]
+server = 3-verify-custom-retry-server
+client = 3-verify-custom-retry-client
 
-[3-noverify-success-server]
+[3-verify-custom-retry-server]
 Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
 CipherString = DEFAULT
 PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
-[3-noverify-success-client]
+[3-verify-custom-retry-client]
 CipherString = DEFAULT
+VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
+VerifyMode = Peer
 
 [test-3]
 ExpectedResult = Success
+client = 3-verify-custom-retry-client-extra
+
+[3-verify-custom-retry-client-extra]
+VerifyCallback = RetryOnce
 
 
 # ===========================================================
 
-[4-noverify-ignore-custom-reject]
-ssl_conf = 4-noverify-ignore-custom-reject-ssl
+[4-noverify-success]
+ssl_conf = 4-noverify-success-ssl
 
-[4-noverify-ignore-custom-reject-ssl]
-server = 4-noverify-ignore-custom-reject-server
-client = 4-noverify-ignore-custom-reject-client
+[4-noverify-success-ssl]
+server = 4-noverify-success-server
+client = 4-noverify-success-client
 
-[4-noverify-ignore-custom-reject-server]
+[4-noverify-success-server]
 Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
 CipherString = DEFAULT
 PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
-[4-noverify-ignore-custom-reject-client]
+[4-noverify-success-client]
 CipherString = DEFAULT
 
 [test-4]
 ExpectedResult = Success
-client = 4-noverify-ignore-custom-reject-client-extra
 
-[4-noverify-ignore-custom-reject-client-extra]
+
+# ===========================================================
+
+[5-noverify-ignore-custom-reject]
+ssl_conf = 5-noverify-ignore-custom-reject-ssl
+
+[5-noverify-ignore-custom-reject-ssl]
+server = 5-noverify-ignore-custom-reject-server
+client = 5-noverify-ignore-custom-reject-client
+
+[5-noverify-ignore-custom-reject-server]
+Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
+CipherString = DEFAULT
+PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
+
+[5-noverify-ignore-custom-reject-client]
+CipherString = DEFAULT
+
+[test-5]
+ExpectedResult = Success
+client = 5-noverify-ignore-custom-reject-client-extra
+
+[5-noverify-ignore-custom-reject-client-extra]
 VerifyCallback = RejectAll
 
 
 # ===========================================================
 
-[5-noverify-accept-custom-allow]
-ssl_conf = 5-noverify-accept-custom-allow-ssl
+[6-noverify-accept-custom-allow]
+ssl_conf = 6-noverify-accept-custom-allow-ssl
 
-[5-noverify-accept-custom-allow-ssl]
-server = 5-noverify-accept-custom-allow-server
-client = 5-noverify-accept-custom-allow-client
+[6-noverify-accept-custom-allow-ssl]
+server = 6-noverify-accept-custom-allow-server
+client = 6-noverify-accept-custom-allow-client
 
-[5-noverify-accept-custom-allow-server]
+[6-noverify-accept-custom-allow-server]
 Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
 CipherString = DEFAULT
 PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
-[5-noverify-accept-custom-allow-client]
+[6-noverify-accept-custom-allow-client]
 CipherString = DEFAULT
 
-[test-5]
+[test-6]
 ExpectedResult = Success
-client = 5-noverify-accept-custom-allow-client-extra
+client = 6-noverify-accept-custom-allow-client-extra
 
-[5-noverify-accept-custom-allow-client-extra]
+[6-noverify-accept-custom-allow-client-extra]
 VerifyCallback = AcceptAll
 
 
 # ===========================================================
 
-[6-verify-fail-no-root]
-ssl_conf = 6-verify-fail-no-root-ssl
+[7-verify-fail-no-root]
+ssl_conf = 7-verify-fail-no-root-ssl
 
-[6-verify-fail-no-root-ssl]
-server = 6-verify-fail-no-root-server
-client = 6-verify-fail-no-root-client
+[7-verify-fail-no-root-ssl]
+server = 7-verify-fail-no-root-server
+client = 7-verify-fail-no-root-client
 
-[6-verify-fail-no-root-server]
+[7-verify-fail-no-root-server]
 Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
 CipherString = DEFAULT
 PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
-[6-verify-fail-no-root-client]
+[7-verify-fail-no-root-client]
 CipherString = DEFAULT
 VerifyMode = Peer
 
-[test-6]
+[test-7]
 ExpectedClientAlert = UnknownCA
 ExpectedResult = ClientFail
 
 
 # ===========================================================
 
-[7-verify-custom-success-no-root]
-ssl_conf = 7-verify-custom-success-no-root-ssl
+[8-verify-custom-success-no-root]
+ssl_conf = 8-verify-custom-success-no-root-ssl
 
-[7-verify-custom-success-no-root-ssl]
-server = 7-verify-custom-success-no-root-server
-client = 7-verify-custom-success-no-root-client
+[8-verify-custom-success-no-root-ssl]
+server = 8-verify-custom-success-no-root-server
+client = 8-verify-custom-success-no-root-client
 
-[7-verify-custom-success-no-root-server]
+[8-verify-custom-success-no-root-server]
 Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
 CipherString = DEFAULT
 PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
-[7-verify-custom-success-no-root-client]
+[8-verify-custom-success-no-root-client]
 CipherString = DEFAULT
 VerifyMode = Peer
 
-[test-7]
+[test-8]
 ExpectedResult = Success
-client = 7-verify-custom-success-no-root-client-extra
+client = 8-verify-custom-success-no-root-client-extra
 
-[7-verify-custom-success-no-root-client-extra]
+[8-verify-custom-success-no-root-client-extra]
 VerifyCallback = AcceptAll
 
 
 # ===========================================================
 
-[8-verify-custom-fail-no-root]
-ssl_conf = 8-verify-custom-fail-no-root-ssl
+[9-verify-custom-fail-no-root]
+ssl_conf = 9-verify-custom-fail-no-root-ssl
 
-[8-verify-custom-fail-no-root-ssl]
-server = 8-verify-custom-fail-no-root-server
-client = 8-verify-custom-fail-no-root-client
+[9-verify-custom-fail-no-root-ssl]
+server = 9-verify-custom-fail-no-root-server
+client = 9-verify-custom-fail-no-root-client
 
-[8-verify-custom-fail-no-root-server]
+[9-verify-custom-fail-no-root-server]
 Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
 CipherString = DEFAULT
 PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
-[8-verify-custom-fail-no-root-client]
+[9-verify-custom-fail-no-root-client]
 CipherString = DEFAULT
 VerifyMode = Peer
 
-[test-8]
+[test-9]
 ExpectedClientAlert = HandshakeFailure
 ExpectedResult = ClientFail
-client = 8-verify-custom-fail-no-root-client-extra
+client = 9-verify-custom-fail-no-root-client-extra
 
-[8-verify-custom-fail-no-root-client-extra]
+[9-verify-custom-fail-no-root-client-extra]
 VerifyCallback = RejectAll
 
 
diff --git a/test/ssl-tests/03-custom_verify.cnf.in b/test/ssl-tests/03-custom_verify.cnf.in
index 28b57216a1..a6b33ba4ca 100644
--- a/test/ssl-tests/03-custom_verify.cnf.in
+++ b/test/ssl-tests/03-custom_verify.cnf.in
@@ -51,6 +51,20 @@ our @tests = (
         },
     },
 
+    # Same test as above but with a custom callback that requests retry once.
+    {
+        name => "verify-custom-retry",
+        server => { },
+        client => {
+            extra => {
+                "VerifyCallback" => "RetryOnce",
+            },
+        },
+        test   => {
+            "ExpectedResult" => "Success",
+        },
+    },
+
     # Sanity-check that verification indeed succeeds if peer verification
     # is not requested.
     {
diff --git a/test/sslapitest.c b/test/sslapitest.c
index c6520482f6..4f367f118a 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -549,6 +549,91 @@ end:
 }
 #endif
 
+static int verify_retry_cb(X509_STORE_CTX *ctx, void *arg)
+{
+    int res = X509_verify_cert(ctx);
+
+    if (res == 0 && X509_STORE_CTX_get_error(ctx) ==
+        X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY)
+        return -1; /* indicate SSL_ERROR_WANT_RETRY_VERIFY */
+    return res;
+}
+
+static int test_client_cert_verify_cb(void)
+{
+    /* server key, cert, chain, and root */
+    char *skey = test_mk_file_path(certsdir, "leaf.key");
+    char *leaf = test_mk_file_path(certsdir, "leaf.pem");
+    char *int2 = test_mk_file_path(certsdir, "subinterCA.pem");
+    char *int1 = test_mk_file_path(certsdir, "interCA.pem");
+    char *root = test_mk_file_path(certsdir, "rootCA.pem");
+    X509 *crt1 = NULL, *crt2 = NULL;
+    STACK_OF(X509) *server_chain;
+    SSL_CTX *cctx = NULL, *sctx = NULL;
+    SSL *clientssl = NULL, *serverssl = NULL;
+    int testresult = 0;
+
+    if (!TEST_true(create_ssl_ctx_pair(libctx, TLS_server_method(),
+                                       TLS_client_method(), TLS1_VERSION, 0,
+                                       &sctx, &cctx, NULL, NULL)))
+        goto end;
+    if (!TEST_int_eq(SSL_CTX_use_certificate_chain_file(sctx, leaf), 1)
+            || !TEST_int_eq(SSL_CTX_use_PrivateKey_file(sctx, skey,
+                                                        SSL_FILETYPE_PEM), 1)
+            || !TEST_int_eq(SSL_CTX_check_private_key(sctx), 1))
+        goto end;
+    if (!TEST_true(SSL_CTX_load_verify_locations(cctx, root, NULL)))
+        goto end;
+    SSL_CTX_set_verify(cctx, SSL_VERIFY_PEER, NULL);
+    SSL_CTX_set_cert_verify_callback(cctx, verify_retry_cb, NULL);
+    if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl,
+                                      &clientssl, NULL, NULL)))
+        goto end;
+
+    /* attempt SSL_connect() with incomplete server chain */
+    if (!TEST_false(create_ssl_connection(serverssl, clientssl,
+                                          SSL_ERROR_WANT_RETRY_VERIFY)))
+        goto end;
+
+    /* application provides intermediate certs needed to verify server cert */
+    if (!TEST_ptr((crt1 = load_cert_pem(int1, libctx)))
+        || !TEST_ptr((crt2 = load_cert_pem(int2, libctx)))
+        || !TEST_ptr((server_chain = SSL_get_peer_cert_chain(clientssl))))
+        goto end;
+    /* add certs in reverse order to demonstrate real chain building */
+    if (!TEST_true(sk_X509_push(server_chain, crt1)))
+        goto end;
+    crt1 = NULL;
+    if (!TEST_true(sk_X509_push(server_chain, crt2)))
+        goto end;
+    crt2 = NULL;
+
+    /* continue SSL_connect(), must now succeed with completed server chain */
+    if (!TEST_true(create_ssl_connection(serverssl, clientssl,
+                                         SSL_ERROR_NONE)))
+        goto end;
+
+    testresult = 1;
+
+end:
+    X509_free(crt1);
+    X509_free(crt2);
+    SSL_shutdown(clientssl);
+    SSL_shutdown(serverssl);
+    SSL_free(serverssl);
+    SSL_free(clientssl);
+    SSL_CTX_free(sctx);
+    SSL_CTX_free(cctx);
+
+    OPENSSL_free(skey);
+    OPENSSL_free(leaf);
+    OPENSSL_free(int2);
+    OPENSSL_free(int1);
+    OPENSSL_free(root);
+
+    return testresult;
+}
+
 #ifndef OPENSSL_NO_TLS1_2
 static int full_client_hello_callback(SSL *s, int *al, void *arg)
 {
@@ -8618,6 +8703,7 @@ int setup_tests(void)
 #ifndef OPENSSL_NO_TLS1_3
     ADD_TEST(test_keylog_no_master_key);
 #endif
+    ADD_TEST(test_client_cert_verify_cb);
 #ifndef OPENSSL_NO_TLS1_2
     ADD_TEST(test_client_hello_cb);
     ADD_TEST(test_no_ems);