Reject stray post-Finished messages in DTLS.

This is in preparation for switching finish_handshake to a
release_current_message hook. finish_handshake in DTLS is also
responsible for releasing any memory associated with extra messages in
the handshake.

Except that's not right and we need to make it an error anyway. Given
that the rest of the DTLS dispatch layer already strongly assumes there
is only one message in epoch one, putting the check in the fragment
processing works fine enough. Add tests for this.

This will certainly need revising when DTLS 1.3 happens (perhaps just a
version check, perhaps bringing finish_handshake back as a function that
can fail... which means we need a state just before SSL_ST_OK), but DTLS
1.3 post-handshake messages haven't really been written down, so let's
do the easy thing for now and add a test for when it gets more
interesting.

This removes the sequence number reset in the DTLS code. That reset
never did anything becase we don't and never will renego. We should make
sure DTLS 1.3 does not bring the reset back for post-handshake stuff.
(It was wrong in 1.2 too. Penultimate-flight retransmits and renego
requests are ambiguous in DTLS.)

BUG=83

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

1 files changed, 7 insertions, 0 deletions

diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index 331703ad..1d9cd75c 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -368,6 +368,13 @@ start:
       return -1;
     }
 
+    /* The encrypted epoch in DTLS has only one handshake message. */
+    if (ssl->d1->r_epoch == 1 && msg_hdr.seq != ssl->d1->handshake_read_seq) {
+      OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_RECORD);
+      ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNEXPECTED_MESSAGE);
+      return -1;
+    }
+
     if (msg_hdr.seq < ssl->d1->handshake_read_seq ||
         msg_hdr.seq >
             (unsigned)ssl->d1->handshake_read_seq + SSL_MAX_HANDSHAKE_FLIGHT) {