commit e820df93718ccb03fc3b06d6ec1c1744d57fe53d
author David Benjamin <davidben@chromium.org> Sun Feb 08 16:06:54 2015 -0500
committer Adam Langley <agl@google.com> Mon Feb 09 19:42:44 2015 +0000
tree 67115f194b91e92ddeabff71f1628886f4de9c62
parent 68070620e7858805324e58c5537a59e4caddc089

Forbid interleaving between application data and handshake protocols.

This is the source of much of renegotiation's complexity, and of OpenSSL's
implementation of it. In practice, we only care about renegotiation because of
the client auth hack. There, we can safely assume that no server will send
application data between sending the HelloRequest and completing the handshake.

BUG=429450

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

ssl/s3_lib.c

diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 9782296..1e9958d 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -1486,19 +1486,7 @@
   }
   s->s3->in_read_app_data = 1;
   ret = s->method->ssl_read_bytes(s, SSL3_RT_APPLICATION_DATA, buf, len, peek);
-  if (ret == -1 && s->s3->in_read_app_data == 2) {
-    /* ssl3_read_bytes decided to call s->handshake_func, which called
-     * ssl3_read_bytes to read handshake data. However, ssl3_read_bytes
-     * actually found application data and thinks that application data makes
-     * sense here; so disable handshake processing and try to read application
-     * data again. */
-    s->in_handshake++;
-    ret =
-        s->method->ssl_read_bytes(s, SSL3_RT_APPLICATION_DATA, buf, len, peek);
-    s->in_handshake--;
-  } else {
-    s->s3->in_read_app_data = 0;
-  }
+  s->s3->in_read_app_data = 0;
 
   return ret;
 }

ssl/s3_pkt.c

diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
index 368fb92..b22b005 100644
--- a/ssl/s3_pkt.c
+++ b/ssl/s3_pkt.c
@@ -1070,40 +1070,12 @@
     goto start;
   }
 
-  switch (rr->type) {
-    default:
-      /* We already handled all of these, with the possible exception of
-       * SSL3_RT_HANDSHAKE when s->in_handshake is set, but that should not
-       * happen when type != rr->type. */
-      assert(rr->type != SSL3_RT_CHANGE_CIPHER_SPEC &&
-             rr->type != SSL3_RT_ALERT && rr->type != SSL3_RT_HANDSHAKE);
+  /* We already handled these. */
+  assert(rr->type != SSL3_RT_CHANGE_CIPHER_SPEC && rr->type != SSL3_RT_ALERT &&
+         rr->type != SSL3_RT_HANDSHAKE);
 
-      al = SSL_AD_UNEXPECTED_MESSAGE;
-      OPENSSL_PUT_ERROR(SSL, ssl3_read_bytes, SSL_R_UNEXPECTED_RECORD);
-      goto f_err;
-
-    case SSL3_RT_APPLICATION_DATA:
-      /* At this point we were expecting handshake data but have application
-       * data. If the library was running inside ssl3_read() (i.e.
-       * |in_read_app_data| is set) and it makes sense to read application data
-       * at this point (session renegotiation not yet started), we will indulge
-       * it. */
-      if (s->s3->in_read_app_data && s->s3->total_renegotiations != 0 &&
-          (((s->state & SSL_ST_CONNECT) &&
-            s->state >= SSL3_ST_CW_CLNT_HELLO_A &&
-            s->state <= SSL3_ST_CR_SRVR_HELLO_A) ||
-           ((s->state & SSL_ST_ACCEPT) &&
-            s->state <= SSL3_ST_SW_HELLO_REQ_A &&
-            s->state >= SSL3_ST_SR_CLNT_HELLO_A))) {
-        s->s3->in_read_app_data = 2;
-        return -1;
-      } else {
-        al = SSL_AD_UNEXPECTED_MESSAGE;
-        OPENSSL_PUT_ERROR(SSL, ssl3_read_bytes, SSL_R_UNEXPECTED_RECORD);
-        goto f_err;
-      }
-  }
-  /* not reached */
+  al = SSL_AD_UNEXPECTED_MESSAGE;
+  OPENSSL_PUT_ERROR(SSL, ssl3_read_bytes, SSL_R_UNEXPECTED_RECORD);
 
 f_err:
   ssl3_send_alert(s, SSL3_AL_FATAL, al);