commit 47749a6a2991689d377900b7ecee66f6700bce5a
author David Benjamin <davidben@google.com> Mon Jun 13 13:56:33 2016 -0400
committer Adam Langley <agl@google.com> Wed Jun 22 20:14:10 2016 +0000
tree b09757ce5c841daf93a8e81088274424f92b45f2
parent b111f7a0e45eac2e11b54446ca91d60225100c37

Make the handshake state machines more linear.

TLS 1.2 has a long series of optional messages within a flight. We really
should send and process these synchronously. In the meantime, the 'skip'
pattern is probably the best we can get away with. Otherwise we have too many
state transitions to think about. (The business with CCS, NPN, and ChannelID is
particularly a headache. Session tickets aren't great either.)

Change-Id: I84e391a6410046372cf9c6989be056a27606ad19
Reviewed-on: https://boringssl-review.googlesource.com/8246
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>

ssl/handshake_server.c

diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 93cb632..6e0121b 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -273,11 +273,7 @@
           goto end;
         }
         if (ssl->hit) {
-          if (ssl->tlsext_ticket_expected) {
-            ssl->state = SSL3_ST_SW_SESSION_TICKET_A;
-          } else {
-            ssl->state = SSL3_ST_SW_CHANGE_A;
-          }
+          ssl->state = SSL3_ST_SW_SESSION_TICKET_A;
         } else {
           ssl->state = SSL3_ST_SW_CERT_A;
         }
@@ -290,22 +286,21 @@
           if (ret <= 0) {
             goto end;
           }
-          if (ssl->s3->tmp.certificate_status_expected) {
-            ssl->state = SSL3_ST_SW_CERT_STATUS_A;
-          } else {
-            ssl->state = SSL3_ST_SW_KEY_EXCH_A;
-          }
         } else {
           skip = 1;
-          ssl->state = SSL3_ST_SW_KEY_EXCH_A;
         }
+        ssl->state = SSL3_ST_SW_CERT_STATUS_A;
         break;
 
       case SSL3_ST_SW_CERT_STATUS_A:
       case SSL3_ST_SW_CERT_STATUS_B:
-        ret = ssl3_send_certificate_status(ssl);
-        if (ret <= 0) {
-          goto end;
+        if (ssl->s3->tmp.certificate_status_expected) {
+          ret = ssl3_send_certificate_status(ssl);
+          if (ret <= 0) {
+            goto end;
+          }
+        } else {
+          skip = 1;
         }
         ssl->state = SSL3_ST_SW_KEY_EXCH_A;
         break;
@@ -391,31 +386,29 @@
           goto end;
         }
 
-        if (ssl->s3->next_proto_neg_seen) {
-          ssl->state = SSL3_ST_SR_NEXT_PROTO_A;
-        } else if (ssl->s3->tlsext_channel_id_valid) {
-          ssl->state = SSL3_ST_SR_CHANNEL_ID_A;
-        } else {
-          ssl->state = SSL3_ST_SR_FINISHED_A;
-        }
+        ssl->state = SSL3_ST_SR_NEXT_PROTO_A;
         break;
 
       case SSL3_ST_SR_NEXT_PROTO_A:
-        ret = ssl3_get_next_proto(ssl);
-        if (ret <= 0) {
-          goto end;
-        }
-        if (ssl->s3->tlsext_channel_id_valid) {
-          ssl->state = SSL3_ST_SR_CHANNEL_ID_A;
+        if (ssl->s3->next_proto_neg_seen) {
+          ret = ssl3_get_next_proto(ssl);
+          if (ret <= 0) {
+            goto end;
+          }
         } else {
-          ssl->state = SSL3_ST_SR_FINISHED_A;
+          skip = 1;
         }
+        ssl->state = SSL3_ST_SR_CHANNEL_ID_A;
         break;
 
       case SSL3_ST_SR_CHANNEL_ID_A:
-        ret = ssl3_get_channel_id(ssl);
-        if (ret <= 0) {
-          goto end;
+        if (ssl->s3->tlsext_channel_id_valid) {
+          ret = ssl3_get_channel_id(ssl);
+          if (ret <= 0) {
+            goto end;
+          }
+        } else {
+          skip = 1;
         }
         ssl->state = SSL3_ST_SR_FINISHED_A;
         break;
@@ -429,10 +422,8 @@
         ssl->method->received_flight(ssl);
         if (ssl->hit) {
           ssl->state = SSL_ST_OK;
-        } else if (ssl->tlsext_ticket_expected) {
-          ssl->state = SSL3_ST_SW_SESSION_TICKET_A;
         } else {
-          ssl->state = SSL3_ST_SW_CHANGE_A;
+          ssl->state = SSL3_ST_SW_SESSION_TICKET_A;
         }
 
         /* If this is a full handshake with ChannelID then record the hashshake
@@ -448,9 +439,13 @@
 
       case SSL3_ST_SW_SESSION_TICKET_A:
       case SSL3_ST_SW_SESSION_TICKET_B:
-        ret = ssl3_send_new_session_ticket(ssl);
-        if (ret <= 0) {
-          goto end;
+        if (ssl->tlsext_ticket_expected) {
+          ret = ssl3_send_new_session_ticket(ssl);
+          if (ret <= 0) {
+            goto end;
+          }
+        } else {
+          skip = 1;
         }
         ssl->state = SSL3_ST_SW_CHANGE_A;
         break;