commit f2401eb42b65e9f8835a8f84d266cad082516665
author David Benjamin <davidben@google.com> Mon Jul 18 22:25:05 2016 +0200
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Tue Jul 19 08:01:47 2016 +0000
tree 3a3bd0059d831f78d39887d0cb694df9ca4ba276
parent feff406782b1299a8ceda60253a0879053b39805

Add a ssl_hs_flush_and_read_message wait mode.

Every flush but the last is always immediately followed by a read. Add a
combined wait mode to make things simpler. Unfortunately, both flights
we have (the state machine doesn't write the first ClientHello) are
followed immediately by a state change, which means we still need some
state in between because we must run code after write_message but before
read_message.

(This way to fix that is to get rid of the buffer BIO, change
write_message to write_flight, and allow things like init_message /
finish_message / init_message / finish_message / set_write_state /
init_message / finish_message / write_flight.)

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

ssl/internal.h

diff --git a/ssl/internal.h b/ssl/internal.h
index a70400b..b52c974 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -844,6 +844,7 @@
   ssl_hs_read_message,
   ssl_hs_write_message,
   ssl_hs_flush,
+  ssl_hs_flush_and_read_message,
   ssl_hs_x509_lookup,
   ssl_hs_private_key_operation,
 };

ssl/tls13_both.c

diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c
index 4e9feaa..023dc0d 100644
--- a/ssl/tls13_both.c
+++ b/ssl/tls13_both.c
@@ -68,6 +68,20 @@
         OPENSSL_PUT_ERROR(SSL, SSL_R_SSL_HANDSHAKE_FAILURE);
         return -1;
 
+      case ssl_hs_flush:
+      case ssl_hs_flush_and_read_message: {
+        int ret = BIO_flush(ssl->wbio);
+        if (ret <= 0) {
+          ssl->rwstate = SSL_WRITING;
+          return ret;
+        }
+        if (hs->wait != ssl_hs_flush_and_read_message) {
+          break;
+        }
+        hs->wait = ssl_hs_read_message;
+        /* Fall-through. */
+      }
+
       case ssl_hs_read_message: {
         int ret = ssl->method->ssl_get_message(ssl, -1, ssl_dont_hash_message);
         if (ret <= 0) {
@@ -84,15 +98,6 @@
         break;
       }
 
-      case ssl_hs_flush: {
-        int ret = BIO_flush(ssl->wbio);
-        if (ret <= 0) {
-          ssl->rwstate = SSL_WRITING;
-          return ret;
-        }
-        break;
-      }
-
       case ssl_hs_x509_lookup:
         ssl->rwstate = SSL_X509_LOOKUP;
         hs->wait = ssl_hs_ok;
@@ -316,6 +321,8 @@
   if (ssl->s3->tmp.message_type != type) {
     ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNEXPECTED_MESSAGE);
     OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_MESSAGE);
+    ERR_add_error_dataf("got type %d, wanted type %d",
+                        ssl->s3->tmp.message_type, type);
     return 0;
   }
 

ssl/tls13_server.c

diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c
index ce65abf..1a1e52a 100644
--- a/ssl/tls13_server.c
+++ b/ssl/tls13_server.c
@@ -37,7 +37,6 @@
   state_complete_server_certificate_verify,
   state_send_server_finished,
   state_flush,
-  state_read_client_second_flight,
   state_process_client_certificate,
   state_process_client_certificate_verify,
   state_process_client_finished,
@@ -352,12 +351,6 @@
 }
 
 static enum ssl_hs_wait_t do_flush(SSL *ssl, SSL_HANDSHAKE *hs) {
-  hs->state = state_read_client_second_flight;
-  return ssl_hs_flush;
-}
-
-static enum ssl_hs_wait_t do_read_client_second_flight(SSL *ssl,
-                                                       SSL_HANDSHAKE *hs) {
   /* Update the secret to the master secret and derive traffic keys. */
   static const uint8_t kZeroes[EVP_MAX_MD_SIZE] = {0};
   if (!tls13_advance_key_schedule(ssl, kZeroes, hs->hash_len) ||
@@ -368,7 +361,7 @@
   }
 
   hs->state = state_process_client_certificate;
-  return ssl_hs_read_message;
+  return ssl_hs_flush_and_read_message;
 }
 
 static enum ssl_hs_wait_t do_process_client_certificate(SSL *ssl,
@@ -457,9 +450,6 @@
       case state_flush:
         ret = do_flush(ssl, hs);
         break;
-      case state_read_client_second_flight:
-        ret = do_read_client_second_flight(ssl, hs);
-        break;
       case state_process_client_certificate:
         ret = do_process_client_certificate(ssl, hs);
         break;