Run select_certificate_cb multiple times

It's not completely clear to me why select_cetificate_cb behaves the way it
does, however not only is it confusing, but it makes assumptions about the
application using BoringSSL (it's not always possible to implement custom
logic outside of the callbacks provided by libssl), that make this callback
somewhat useless.

Case in point, the callback can be used for changing min/max protocol versions
based on per-site policies, and select_certificate_cb is the only place where
SSL_set_min/max_proto_version() can be used (e.g. you can't call them in
cert_cb because it's too late), but the decision on the specific versions to
use might depend on configuration that needs retrieving asynchronously from
over the network, which requires re-running the callback multiple times.

Change-Id: Ia8e151b163628545373e7fd1f327e9af207478a6
Reviewed-on: https://boringssl-review.googlesource.com/13000
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

include/openssl/ssl.h

@@ -3006,10 +3006,7 @@ OPENSSL_EXPORT int SSL_early_callback_ctx_extension_get(
  * |SSL_get_error| will return |SSL_ERROR_PENDING_CERTIFICATE|.
  *
  * Note: The |SSL_CLIENT_HELLO| is only valid for the duration of the callback
- * and is not valid while the handshake is paused. Further, unlike with most
- * callbacks, when the handshake loop is resumed, it will not call the callback
- * a second time. The caller must finish reconfiguring the connection before
- * resuming the handshake. */
+ * and is not valid while the handshake is paused. */
 OPENSSL_EXPORT void SSL_CTX_set_select_certificate_cb(
     SSL_CTX *ctx, int (*cb)(const SSL_CLIENT_HELLO *));
 

ssl/handshake_server.c

@@ -839,10 +839,6 @@ static int ssl3_get_client_hello(SSL_HANDSHAKE *hs) {
   }
 
   if (hs->state == SSL3_ST_SR_CLNT_HELLO_B) {
-    /* Unlike other callbacks, the early callback is not run a second time if
-     * paused. */
-    hs->state = SSL3_ST_SR_CLNT_HELLO_C;
-
     /* Run the early callback. */
     if (ssl->ctx->select_certificate_cb != NULL) {
       switch (ssl->ctx->select_certificate_cb(&client_hello)) {
@@ -860,6 +856,7 @@ static int ssl3_get_client_hello(SSL_HANDSHAKE *hs) {
           /* fallthrough */;
       }
     }
+    hs->state = SSL3_ST_SR_CLNT_HELLO_C;
   }
 
   /* Negotiate the protocol version if we have not done so yet. */

ssl/test/bssl_shim.cc

@@ -108,6 +108,7 @@ struct TestState {
   bssl::UniquePtr<SSL_SESSION> new_session;
   bool ticket_decrypt_done = false;
   bool alpn_select_done = false;
+  bool early_callback_ready = false;
 };
 
 static void TestStateExFree(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
@@ -479,7 +480,9 @@ static int SelectCertificateCallback(const SSL_CLIENT_HELLO *client_hello) {
 
   // Install the certificate in the early callback.
   if (config->use_early_callback) {
-    if (config->async) {
+    bool early_callback_ready =
+        GetTestState(client_hello->ssl)->early_callback_ready;
+    if (config->async && !early_callback_ready) {
       // Install the certificate asynchronously.
       return 0;
     }
@@ -1104,8 +1107,8 @@ static bool RetryAsync(SSL *ssl, int ret) {
       test_state->session = std::move(test_state->pending_session);
       return true;
     case SSL_ERROR_PENDING_CERTIFICATE:
-      // The handshake will resume without a second call to the early callback.
-      return InstallCertificate(ssl);
+      test_state->early_callback_ready = true;
+      return true;
     case SSL_ERROR_WANT_PRIVATE_KEY_OPERATION:
       test_state->private_key_retries++;
       return true;