Revise SSL_cutthrough_complete and SSL_in_init.

This makes the following changes:

- SSL_cutthrough_complete no longer rederives whether cutthrough happened and
  just maintains a handshake bit.

- SSL_in_init no longer returns true if we are False Starting but haven't
  completed the handshake. That logic was awkward as it depended on querying
  in_read_app_data to force SSL_read to flush the entire handshake. Defaulting
  SSL_in_init to continue querying the full handshake and special-casing
  SSL_write is better. E.g. the check in bidirectional SSL_shutdown wants to know
  if we're in a handshake. No internal consumer of
  SSL_MODE_HANDSHAKE_CUTTHROUGH ever queries SSL_in_init directly.

- in_read_app_data is gone now that the final use is dead.

Change-Id: I05211a116d684054dfef53075cd277b1b30623b5
Reviewed-on: https://boringssl-review.googlesource.com/3336
Reviewed-by: Adam Langley <agl@google.com>

include/openssl/ssl.h

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 6989bf6..e08caee 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1398,11 +1398,15 @@
 /* Is the SSL_connection established? */
 #define SSL_get_state(a) SSL_state(a)
 #define SSL_is_init_finished(a) (SSL_state(a) == SSL_ST_OK)
-#define SSL_in_init(a) \
-  ((SSL_state(a) & SSL_ST_INIT) && !SSL_cutthrough_complete(a))
+#define SSL_in_init(a) (SSL_state(a) & SSL_ST_INIT)
 #define SSL_in_before(a) (SSL_state(a) & SSL_ST_BEFORE)
 #define SSL_in_connect_init(a) (SSL_state(a) & SSL_ST_CONNECT)
 #define SSL_in_accept_init(a) (SSL_state(a) & SSL_ST_ACCEPT)
+
+/* SSL_cutthrough_complete returns one if |s| has a pending unfinished handshake
+ * that has completed cut-through. |SSL_write| may be called at this point
+ * without waiting for the peer, but |SSL_read| will require the handshake
+ * to be completed. */
 OPENSSL_EXPORT int SSL_cutthrough_complete(const SSL *s);
 
 /* The following 2 states are kept in ssl->rstate when reads fail,

include/openssl/ssl3.h

diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h
index 8745281..8dcc2c2 100644
--- a/include/openssl/ssl3.h
+++ b/include/openssl/ssl3.h
@@ -401,8 +401,6 @@
   int total_renegotiations;
   int num_renegotiations;
 
-  int in_read_app_data;
-
   /* State pertaining to the pending handshake.
    *
    * TODO(davidben): State is current spread all over the place. Move
@@ -486,6 +484,10 @@
     /* new_mac_secret_size is unused and exists only until wpa_supplicant can
      * be updated. It is only needed for EAP-FAST, which we don't support. */
     uint8_t new_mac_secret_size;
+
+    /* Client-only: cutthrough_complete is one if there is a pending handshake,
+     * but cut-through is completed so the client may write data. */
+    char cutthrough_complete;
   } tmp;
 
   /* Connection binding to prevent renegotiation attacks */

ssl/s3_clnt.c

diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 20fb2ec..42be93a 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -529,6 +529,7 @@
         } else {
           s->state = SSL3_ST_CR_CHANGE;
         }
+        s->s3->tmp.cutthrough_complete = 1;
 
         ssl_free_wbio_buffer(s);
         ret = 1;
@@ -549,6 +550,7 @@
         s->init_num = 0;
         s->renegotiate = 0;
         s->new_session = 0;
+        s->s3->tmp.cutthrough_complete = 0;
 
         ssl_update_cache(s, SSL_SESS_CACHE_CLIENT);
         if (s->hit) {

ssl/s3_lib.c

diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 1e9958d..c230427 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -1478,17 +1478,12 @@
 }
 
 static int ssl3_read_internal(SSL *s, void *buf, int len, int peek) {
-  int ret;
-
   ERR_clear_system_error();
   if (s->s3->renegotiate) {
     ssl3_renegotiate_check(s);
   }
-  s->s3->in_read_app_data = 1;
-  ret = s->method->ssl_read_bytes(s, SSL3_RT_APPLICATION_DATA, buf, len, peek);
-  s->s3->in_read_app_data = 0;
 
-  return ret;
+  return s->method->ssl_read_bytes(s, SSL3_RT_APPLICATION_DATA, buf, len, peek);
 }
 
 int ssl3_read(SSL *s, void *buf, int len) {

ssl/s3_pkt.c

diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
index d37338f..57bb54b 100644
--- a/ssl/s3_pkt.c
+++ b/ssl/s3_pkt.c
@@ -427,7 +427,7 @@
   tot = s->s3->wnum;
   s->s3->wnum = 0;
 
-  if (SSL_in_init(s) && !s->in_handshake) {
+  if (!s->in_handshake && SSL_in_init(s) && !SSL_cutthrough_complete(s)) {
     i = s->handshake_func(s);
     if (i < 0) {
       return i;

ssl/ssl_lib.c

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index f090882..62ddaeb 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2902,18 +2902,7 @@
 }
 
 int SSL_cutthrough_complete(const SSL *s) {
-  return (
-      !s->server && /* cutthrough only applies to clients */
-      !s->hit &&    /* full-handshake */
-      s->version >= SSL3_VERSION &&
-      s->s3->in_read_app_data == 0 && /* cutthrough only applies to write() */
-      (SSL_get_mode((SSL *)s) &
-       SSL_MODE_HANDSHAKE_CUTTHROUGH) && /* cutthrough enabled */
-      ssl3_can_cutthrough(s) &&          /* cutthrough allowed */
-      s->s3->previous_server_finished_len ==
-          0 && /* not a renegotiation handshake */
-      (s->state == SSL3_ST_CR_SESSION_TICKET_A || /* ready to write app-data*/
-       s->state == SSL3_ST_CR_CHANGE || s->state == SSL3_ST_CR_FINISHED_A));
+  return s->s3->tmp.cutthrough_complete;
 }
 
 void SSL_get_structure_sizes(size_t *ssl_size, size_t *ssl_ctx_size,