commit 4cbb93195f887bbc491f819907065339db70c370
author David Benjamin <davidben@google.com> Thu Aug 31 14:52:31 2017 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Fri Sep 01 15:07:12 2017 +0000
tree 3703b25648cd290af1d313dd0c27df6c6051505f
parent fd45ee7da8d897e45bf995125301f0a5a86c7792

Collapse client Finished states together.

By resolving Channel ID earlier, we can take advantage of
flight-by-flight writes.

Change-Id: I31265bda3390eb1faec976ac13d7a01ba5f6dd5f
Reviewed-on: https://boringssl-review.googlesource.com/19925
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/handshake_client.cc

diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index ee8cf7a..49d931b 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -188,8 +188,6 @@
   state_send_client_certificate,
   state_send_client_key_exchange,
   state_send_client_certificate_verify,
-  state_send_second_client_flight,
-  state_send_channel_id,
   state_send_client_finished,
   state_finish_flight,
   state_read_session_ticket,
@@ -1397,7 +1395,7 @@
   SSL *const ssl = hs->ssl;
 
   if (!hs->cert_request || !ssl_has_certificate(ssl)) {
-    hs->state = state_send_second_client_flight;
+    hs->state = state_send_client_finished;
     return ssl_hs_ok;
   }
 
@@ -1473,12 +1471,23 @@
   // The handshake buffer is no longer necessary.
   hs->transcript.FreeBuffer();
 
-  hs->state = state_send_second_client_flight;
+  hs->state = state_send_client_finished;
   return ssl_hs_ok;
 }
 
-static enum ssl_hs_wait_t do_send_second_client_flight(SSL_HANDSHAKE *hs) {
+static enum ssl_hs_wait_t do_send_client_finished(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
+  // Resolve Channel ID first, before any non-idempotent operations.
+  if (ssl->s3->tlsext_channel_id_valid) {
+    if (!ssl_do_channel_id_callback(ssl)) {
+      return ssl_hs_error;
+    }
+
+    if (ssl->tlsext_channel_id_private == NULL) {
+      hs->state = state_send_client_finished;
+      return ssl_hs_channel_id_lookup;
+    }
+  }
 
   if (!ssl->method->add_change_cipher_spec(ssl) ||
       !tls1_change_cipher_state(hs, SSL3_CHANGE_CIPHER_CLIENT_WRITE)) {
@@ -1503,41 +1512,17 @@
     }
   }
 
-  hs->state = state_send_channel_id;
-  return ssl_hs_ok;
-}
-
-static enum ssl_hs_wait_t do_send_channel_id(SSL_HANDSHAKE *hs) {
-  SSL *const ssl = hs->ssl;
-
-  if (!ssl->s3->tlsext_channel_id_valid) {
-    hs->state = state_send_client_finished;
-    return ssl_hs_ok;
+  if (ssl->s3->tlsext_channel_id_valid) {
+    ScopedCBB cbb;
+    CBB body;
+    if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_CHANNEL_ID) ||
+        !tls1_write_channel_id(hs, &body) ||
+        !ssl_add_message_cbb(ssl, cbb.get())) {
+      OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+      return ssl_hs_error;
+    }
   }
 
-  if (!ssl_do_channel_id_callback(ssl)) {
-    return ssl_hs_error;
-  }
-
-  if (ssl->tlsext_channel_id_private == NULL) {
-    hs->state = state_send_channel_id;
-    return ssl_hs_channel_id_lookup;
-  }
-
-  ScopedCBB cbb;
-  CBB body;
-  if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_CHANNEL_ID) ||
-      !tls1_write_channel_id(hs, &body) ||
-      !ssl_add_message_cbb(ssl, cbb.get())) {
-    OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
-    return ssl_hs_error;
-  }
-
-  hs->state = state_send_client_finished;
-  return ssl_hs_ok;
-}
-
-static enum ssl_hs_wait_t do_send_client_finished(SSL_HANDSHAKE *hs) {
   if (!ssl3_send_finished(hs)) {
     return ssl_hs_error;
   }
@@ -1674,7 +1659,7 @@
   }
 
   if (ssl->session != NULL) {
-    hs->state = state_send_second_client_flight;
+    hs->state = state_send_client_finished;
     return ssl_hs_ok;
   }
 
@@ -1765,12 +1750,6 @@
       case state_send_client_certificate_verify:
         ret = do_send_client_certificate_verify(hs);
         break;
-      case state_send_second_client_flight:
-        ret = do_send_second_client_flight(hs);
-        break;
-      case state_send_channel_id:
-        ret = do_send_channel_id(hs);
-        break;
       case state_send_client_finished:
         ret = do_send_client_finished(hs);
         break;
@@ -1839,10 +1818,6 @@
       return "TLS client send_client_key_exchange";
     case state_send_client_certificate_verify:
       return "TLS client send_client_certificate_verify";
-    case state_send_second_client_flight:
-      return "TLS client send_second_client_flight";
-    case state_send_channel_id:
-      return "TLS client send_channel_id";
     case state_send_client_finished:
       return "TLS client send_client_finished";
     case state_finish_flight: