Fix DTLSv1_listen() sequence numbers

DTLSv1_listen() is stateless. We never increment the record read sequence
while listening, and we reflect the incoming record's sequence number in our
write sequence.

The logic for doing the write sequence reflection was *after* we had
finished processing the incoming ClientHello and before we write the
ServerHello. In the normal course of events this is fine. However if we
need to write an early alert during ClientHello processing (e.g. no shared
cipher), then we haven't done the write sequence reflection yet. This means
the alert gets written with the wrong sequence number (it will just be set
to whatever value we left it in the last time we wrote something). If the
sequence number is less than expected then the client will believe that the
incoming alert is a retransmit and will therefore drop it, causing the
client to hang waiting for a response from the server.

Fixes #2886

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2915)

diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index 9bc61536101b5ccc4e1e0639d52249090d20a396..232a6d4bf4ff7637f4ed92c2a2d13eda9f76289f 100644 (file)
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -517,6 +517,17 @@ long dtls1_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok)
         return i;
     }
 
+    /*
+     * Don't change the *message* read sequence number while listening. For
+     * the *record* write sequence we reflect the ClientHello sequence number
+     * when listening.
+     */
+    if (s->d1->listen)
+        memcpy(s->s3->write_sequence, s->s3->read_sequence,
+               sizeof(s->s3->write_sequence));
+    else
+        s->d1->handshake_read_seq++;
+
     if (mt >= 0 && s->s3->tmp.message_type != mt) {
         al = SSL_AD_UNEXPECTED_MESSAGE;
         SSLerr(SSL_F_DTLS1_GET_MESSAGE, SSL_R_UNEXPECTED_MESSAGE);
@@ -544,10 +555,6 @@ long dtls1_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok)
 
     memset(msg_hdr, 0x00, sizeof(struct hm_header_st));
 
-    /* Don't change sequence numbers while listening */
-    if (!s->d1->listen)
-        s->d1->handshake_read_seq++;
-
     s->init_msg = s->init_buf->data + DTLS1_HM_HEADER_LENGTH;
     return s->init_num;
 

diff --git a/ssl/d1_srvr.c b/ssl/d1_srvr.c
index 32eae6bab4f3e4fec6c79caeb526e8daaec79a10..8502b242e51cd32e93e7f4ff54e0109b81d6e399 100644 (file)
--- a/ssl/d1_srvr.c
+++ b/ssl/d1_srvr.c
@@ -355,15 +355,6 @@ int dtls1_accept(SSL *s)
 
             s->init_num = 0;
 
-            /*
-             * Reflect ClientHello sequence to remain stateless while
-             * listening
-             */
-            if (listen) {
-                memcpy(s->s3->write_sequence, s->s3->read_sequence,
-                       sizeof(s->s3->write_sequence));
-            }
-
             /* If we're just listening, stop here */
             if (listen && s->state == SSL3_ST_SW_SRVR_HELLO_A) {
                 ret = 2;