commit 25fe85b38cf4b6f5acd8aa6469c03a88126a1d25
author David Benjamin <davidben@google.com> Tue Aug 09 20:00:32 2016 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Thu Aug 11 15:45:00 2016 +0000
tree 936445b117aae274a5729fd0e85419231fce4bcd
parent fddbadcba9dd220d965fa7a9ff32d8c9e4103349

Insert a state before cert_cb.

If cert_cb runs asynchronously, we end up repeating a large part of very
stateful ClientHello processing. This seems to be mostly fine and there
are few users of server-side cert_cb (it's a new API in 1.0.2), but it's
a little scary.

This is also visible to external consumers because some callbacks get
called multiple times. We especially should try to avoid that as there
is no guarantee that these callbacks are idempotent and give the same
answer each time.

Change-Id: I212b2325eae2cfca0fb423dace101e466c5e5d4e
Reviewed-on: https://boringssl-review.googlesource.com/10224
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/tls13_server.c

diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c
index ec44436..a1aeeea 100644
--- a/ssl/tls13_server.c
+++ b/ssl/tls13_server.c
@@ -29,6 +29,7 @@
 
 enum server_hs_state_t {
   state_process_client_hello = 0,
+  state_select_parameters,
   state_send_hello_retry_request,
   state_flush_hello_retry_request,
   state_process_second_client_hello,
@@ -150,9 +151,12 @@
     return ssl_hs_error;
   }
 
-  /* Let cert callback update server certificates if required.
-   *
-   * TODO(davidben): Can this get run earlier? */
+  hs->state = state_select_parameters;
+  return ssl_hs_ok;
+}
+
+static enum ssl_hs_wait_t do_select_parameters(SSL *ssl, SSL_HANDSHAKE *hs) {
+  /* Call |cert_cb| to update server certificates if required. */
   if (ssl->cert->cert_cb != NULL) {
     int rv = ssl->cert->cert_cb(ssl, ssl->cert->cert_cb_arg);
     if (rv == 0) {
@@ -161,11 +165,19 @@
       return ssl_hs_error;
     }
     if (rv < 0) {
-      hs->state = state_process_client_hello;
+      hs->state = state_select_parameters;
       return ssl_hs_x509_lookup;
     }
   }
 
+  struct ssl_early_callback_ctx client_hello;
+  if (!ssl_early_callback_init(ssl, &client_hello, ssl->init_msg,
+                               ssl->init_num)) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_CLIENTHELLO_PARSE_FAILED);
+    ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+    return ssl_hs_error;
+  }
+
   const SSL_CIPHER *cipher =
       ssl3_choose_cipher(ssl, &client_hello, ssl_get_cipher_preferences(ssl));
   if (cipher == NULL) {
@@ -529,6 +541,9 @@
       case state_process_client_hello:
         ret = do_process_client_hello(ssl, hs);
         break;
+      case state_select_parameters:
+        ret = do_select_parameters(ssl, hs);
+        break;
       case state_send_hello_retry_request:
         ret = do_send_hello_retry_request(ssl, hs);
         break;