Switch finish_handshake to release_current_message.

With the previous DTLS change, the dispatch layer only cares about the
end of the handshake to know when to drop the current message. TLS 1.3
post-handshake messages will need a similar hook, so convert it to this
lower-level one.

BUG=83

Change-Id: I4c8c3ba55ba793afa065bf261a7bccac8816c348
Reviewed-on: https://boringssl-review.googlesource.com/8989
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

8 files changed, 55 insertions, 52 deletions

diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index 1d9cd75c..712c63b7 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -262,17 +262,6 @@ static int dtls1_is_current_message_complete(const SSL *ssl) {
   return frag != NULL && frag->reassembly == NULL;
 }
 
-/* dtls1_pop_message removes the current handshake message, which must be
- * complete, and advances to the next one. */
-static void dtls1_pop_message(SSL *ssl) {
-  assert(dtls1_is_current_message_complete(ssl));
-
-  size_t index = ssl->d1->handshake_read_seq % SSL_MAX_HANDSHAKE_FLIGHT;
-  dtls1_hm_fragment_free(ssl->d1->incoming_messages[index]);
-  ssl->d1->incoming_messages[index] = NULL;
-  ssl->d1->handshake_read_seq++;
-}
-
 /* dtls1_get_incoming_message returns the incoming message corresponding to
  * |msg_hdr|. If none exists, it creates a new one and inserts it in the
  * queue. Otherwise, it checks |msg_hdr| is consistent with the existing one. It
@@ -412,12 +401,12 @@ int dtls1_get_message(SSL *ssl, int msg_type,
      * ssl_dont_hash_message would have to have been applied to the previous
      * call. */
     assert(hash_message == ssl_hash_message);
-    assert(dtls1_is_current_message_complete(ssl));
+    assert(ssl->init_msg != NULL);
 
     ssl->s3->tmp.reuse_message = 0;
     hash_message = ssl_dont_hash_message;
-  } else if (dtls1_is_current_message_complete(ssl)) {
-    dtls1_pop_message(ssl);
+  } else {
+    dtls1_release_current_message(ssl, 0 /* don't free buffer */);
   }
 
   /* Process handshake records until the current message is ready. */
@@ -463,6 +452,21 @@ int dtls1_hash_current_message(SSL *ssl) {
                                     DTLS1_HM_HEADER_LENGTH + frag->msg_len);
 }
 
+void dtls1_release_current_message(SSL *ssl, int free_buffer) {
+  if (ssl->init_msg == NULL) {
+    return;
+  }
+
+  assert(dtls1_is_current_message_complete(ssl));
+  size_t index = ssl->d1->handshake_read_seq % SSL_MAX_HANDSHAKE_FLIGHT;
+  dtls1_hm_fragment_free(ssl->d1->incoming_messages[index]);
+  ssl->d1->incoming_messages[index] = NULL;
+  ssl->d1->handshake_read_seq++;
+
+  ssl->init_msg = NULL;
+  ssl->init_num = 0;
+}
+
 void dtls_clear_incoming_messages(SSL *ssl) {
   for (size_t i = 0; i < SSL_MAX_HANDSHAKE_FLIGHT; i++) {
     dtls1_hm_fragment_free(ssl->d1->incoming_messages[i]);
@@ -471,13 +475,14 @@ void dtls_clear_incoming_messages(SSL *ssl) {
 }
 
 int dtls_has_incoming_messages(const SSL *ssl) {
-  /* This function may not be called if there is a pending |dtls1_get_message|
-   * operation. */
-  assert(dtls1_is_current_message_complete(ssl));
-
   size_t current = ssl->d1->handshake_read_seq % SSL_MAX_HANDSHAKE_FLIGHT;
   for (size_t i = 0; i < SSL_MAX_HANDSHAKE_FLIGHT; i++) {
-    if (i != current && ssl->d1->incoming_messages[i] != NULL) {
+    /* Skip the current message. */
+    if (ssl->init_msg != NULL && i == current) {
+      assert(dtls1_is_current_message_complete(ssl));
+      continue;
+    }
+    if (ssl->d1->incoming_messages[i] != NULL) {
       return 1;
     }
   }
diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c
index ceb6e179..ab9b9119 100644
--- a/ssl/d1_pkt.c
+++ b/ssl/d1_pkt.c
@@ -224,7 +224,7 @@ again:
     }
 
     if (msg_hdr.type == SSL3_MT_FINISHED &&
-        msg_hdr.seq == ssl->d1->handshake_read_seq) {
+        msg_hdr.seq == ssl->d1->handshake_read_seq - 1) {
       if (msg_hdr.frag_off == 0) {
         /* Retransmit our last flight of messages. If the peer sends the second
          * Finished, they may not have received ours. Only do this for the
diff --git a/ssl/dtls_method.c b/ssl/dtls_method.c
index 09bd4550..e2e17265 100644
--- a/ssl/dtls_method.c
+++ b/ssl/dtls_method.c
@@ -92,12 +92,6 @@ static uint16_t dtls1_version_to_wire(uint16_t version) {
   return ~(version - 0x0201);
 }
 
-static void dtls1_finish_handshake(SSL *ssl) {
-  dtls_clear_incoming_messages(ssl);
-  ssl->init_msg = NULL;
-  ssl->init_num = 0;
-}
-
 static int dtls1_set_read_state(SSL *ssl, SSL_AEAD_CTX *aead_ctx) {
   /* Cipher changes are illegal when there are buffered incoming messages. */
   if (dtls_has_incoming_messages(ssl)) {
@@ -135,9 +129,9 @@ static const SSL_PROTOCOL_METHOD kDTLSProtocolMethod = {
     dtls1_version_to_wire,
     dtls1_new,
     dtls1_free,
-    dtls1_finish_handshake,
     dtls1_get_message,
     dtls1_hash_current_message,
+    dtls1_release_current_message,
     dtls1_read_app_data,
     dtls1_read_change_cipher_spec,
     dtls1_read_close_notify,
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index fc4318eb..5fe706ce 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -502,10 +502,9 @@ int ssl3_connect(SSL *ssl) {
         break;
 
       case SSL_ST_OK:
-        /* clean a few things up */
+        /* Clean a few things up. */
         ssl3_cleanup_key_block(ssl);
-
-        ssl->method->finish_handshake(ssl);
+        ssl->method->release_current_message(ssl, 1 /* free_buffer */);
 
         /* Remove write buffering now. */
         ssl_free_wbio_buffer(ssl);
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 4708dd0e..9f4aae3c 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -471,10 +471,9 @@ int ssl3_accept(SSL *ssl) {
         break;
 
       case SSL_ST_OK:
-        /* clean a few things up */
+        /* Clean a few things up. */
         ssl3_cleanup_key_block(ssl);
-
-        ssl->method->finish_handshake(ssl);
+        ssl->method->release_current_message(ssl, 1 /* free_buffer */);
 
         /* remove buffering on output */
         ssl_free_wbio_buffer(ssl);
diff --git a/ssl/internal.h b/ssl/internal.h
index 2fb14929..1b3f28a9 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1039,8 +1039,6 @@ struct ssl_protocol_method_st {
   uint16_t (*version_to_wire)(uint16_t version);
   int (*ssl_new)(SSL *ssl);
   void (*ssl_free)(SSL *ssl);
-  /* finish_handshake is called when a handshake completes. */
-  void (*finish_handshake)(SSL *ssl);
   /* ssl_get_message reads the next handshake message. If |msg_type| is not -1,
    * the message must have the specified type. On success, it returns one and
    * sets |ssl->s3->tmp.message_type|, |ssl->init_msg|, and |ssl->init_num|.
@@ -1051,6 +1049,9 @@ struct ssl_protocol_method_st {
    * handshake hash. It returns one on success and zero on allocation
    * failure. */
   int (*hash_current_message)(SSL *ssl);
+  /* release_current_message is called to release the current handshake message.
+   * If |free_buffer| is one, buffers will also be released. */
+  void (*release_current_message)(SSL *ssl, int free_buffer);
   int (*read_app_data)(SSL *ssl, uint8_t *buf, int len, int peek);
   int (*read_change_cipher_spec)(SSL *ssl);
   void (*read_close_notify)(SSL *ssl);
@@ -1242,6 +1243,7 @@ int ssl3_send_alert(SSL *ssl, int level, int desc);
 int ssl3_get_message(SSL *ssl, int msg_type,
                      enum ssl_hash_message_t hash_message);
 int ssl3_hash_current_message(SSL *ssl);
+void ssl3_release_current_message(SSL *ssl, int free_buffer);
 
 /* ssl3_cert_verify_hash writes the SSL 3.0 CertificateVerify hash into the
  * bytes pointed to by |out| and writes the number of bytes to |*out_len|. |out|
@@ -1321,6 +1323,7 @@ void dtls1_free(SSL *ssl);
 
 int dtls1_get_message(SSL *ssl, int mt, enum ssl_hash_message_t hash_message);
 int dtls1_hash_current_message(SSL *ssl);
+void dtls1_release_current_message(SSL *ssl, int free_buffer);
 int dtls1_dispatch_alert(SSL *ssl);
 
 /* ssl_is_wbio_buffered returns one if |ssl|'s write BIO is buffered and zero
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index 11555dd9..7608c342 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -550,14 +550,8 @@ again:
 
     ssl->s3->tmp.reuse_message = 0;
     hash_message = ssl_dont_hash_message;
-  } else if (ssl->init_msg != NULL) {
-    /* |init_buf| never contains data beyond the current message. */
-    assert(SSL3_HM_HEADER_LENGTH + ssl->init_num == ssl->init_buf->length);
-
-    /* Clear the current message. */
-    ssl->init_msg = NULL;
-    ssl->init_num = 0;
-    ssl->init_buf->length = 0;
+  } else {
+    ssl3_release_current_message(ssl, 0 /* don't free buffer */);
   }
 
   /* Read the message header, if we haven't yet. */
@@ -618,6 +612,23 @@ int ssl3_hash_current_message(SSL *ssl) {
                                     ssl->init_buf->length);
 }
 
+void ssl3_release_current_message(SSL *ssl, int free_buffer) {
+  if (ssl->init_msg != NULL) {
+    /* |init_buf| never contains data beyond the current message. */
+    assert(SSL3_HM_HEADER_LENGTH + ssl->init_num == ssl->init_buf->length);
+
+    /* Clear the current message. */
+    ssl->init_msg = NULL;
+    ssl->init_num = 0;
+    ssl->init_buf->length = 0;
+  }
+
+  if (free_buffer) {
+    BUF_MEM_free(ssl->init_buf);
+    ssl->init_buf = NULL;
+  }
+}
+
 int ssl_verify_alarm_type(long type) {
   int al;
 
diff --git a/ssl/tls_method.c b/ssl/tls_method.c
index c11c452b..923541b8 100644
--- a/ssl/tls_method.c
+++ b/ssl/tls_method.c
@@ -69,14 +69,6 @@ static uint16_t ssl3_version_from_wire(uint16_t wire_version) {
 
 static uint16_t ssl3_version_to_wire(uint16_t version) { return version; }
 
-static void ssl3_finish_handshake(SSL *ssl) {
-  BUF_MEM_free(ssl->init_buf);
-  ssl->init_buf = NULL;
-
-  ssl->init_msg = NULL;
-  ssl->init_num = 0;
-}
-
 static int ssl3_set_read_state(SSL *ssl, SSL_AEAD_CTX *aead_ctx) {
   if (ssl->s3->rrec.length != 0) {
     /* There may not be unprocessed record data at a cipher change. */
@@ -109,9 +101,9 @@ static const SSL_PROTOCOL_METHOD kTLSProtocolMethod = {
     ssl3_version_to_wire,
     ssl3_new,
     ssl3_free,
-    ssl3_finish_handshake,
     ssl3_get_message,
     ssl3_hash_current_message,
+    ssl3_release_current_message,
     ssl3_read_app_data,
     ssl3_read_change_cipher_spec,
     ssl3_read_close_notify,