[openssl.org #2458] [PATCH] DTLS state changed while listening may cause infinite loop

[Robin Seggelmann via RT]

While listening for incoming connections, DTLS should answer every ClientHello 
with a HelloVerifyRequest without changing its state. However, this is not the 
case since the handshake sequence numbers are still incremented with every 
incoming message. After receiving an initial ClientHello (seq 0), seq 1 is 
expected (ClientHello with cookie) and every other initial ClientHello will be 
dropped until seq 1 has been received. This may even result in an infinite loop 
when the expected sequence number becomes > 1 due to a race condition with many 
simultaneously connecting clients, because the expected number will never 
appear while in listening state.
The changes of the attached patch prevent the increase of the sequence numbers 
while listening, so only sequence number 0 (initial ClientHello) will be 
expected and accepted. An exception exists for sequence number 1 (ClientHello 
with cookie), which has to be allowed as well. After a valid ClientHello with 
cookie has been received, the sequence numbers are set to the expected values 
to continue the handshake.
Additionally, HelloVerifyRequest messages aren't buffered and retransmitted 
anymore, since this would also change the state of the listening server 
according to RFC 4347.

Thanks to Neeraj Rajgure for providing hints!

-Robin


--- ssl/d1_both.c       3 May 2010 13:01:50 -0000       1.14.2.21
+++ ssl/d1_both.c       23 Feb 2011 13:25:54 -0000
@@ -464,7 +464,10 @@
 
        memset(msg_hdr, 0x00, sizeof(struct hm_header_st));
 
-       s->d1->handshake_read_seq++;
+       /* Don't change sequence numbers while listening */
+       if (!s->d1->listen)
+               s->d1->handshake_read_seq++;
+
        /* we just read a handshake message from the other side:
         * this means that we don't need to retransmit of the
         * buffered messages.  
@@ -813,9 +816,11 @@
 
        /* 
         * if this is a future (or stale) message it gets buffered
-        * (or dropped)--no further processing at this time 
+        * (or dropped)--no further processing at this time
+        * While listening, we accept seq 1 (ClientHello with cookie)
+        * although we're still expecting seq 0 (ClientHello)
         */
-       if ( msg_hdr.seq != s->d1->handshake_read_seq)
+       if (msg_hdr.seq != s->d1->handshake_read_seq && !(s->d1->listen && 
msg_hdr.seq == 1))
                return dtls1_process_out_of_seq_message(s, &msg_hdr, ok);
 
        len = msg_hdr.msg_len;
@@ -1322,7 +1327,8 @@
 dtls1_set_message_header(SSL *s, unsigned char *p, unsigned char mt,
                        unsigned long len, unsigned long frag_off, unsigned 
long frag_len)
        {
-       if ( frag_off == 0)
+       /* Don't change sequence numbers while listening */
+       if (frag_off == 0 && !s->d1->listen)
                {
                s->d1->handshake_write_seq = s->d1->next_handshake_write_seq;
                s->d1->next_handshake_write_seq++;

--- ssl/d1_srvr.c       27 Aug 2010 12:10:11 -0000      1.20.2.16.2.3
+++ ssl/d1_srvr.c       23 Feb 2011 13:25:54 -0000
@@ -278,6 +278,12 @@
                                {
                                ret = 2;
                                s->d1->listen = 0;
+                               /* Set expected sequence numbers
+                                * to continue the handshake.
+                                */
+                               s->d1->handshake_read_seq = 2;
+                               s->d1->handshake_write_seq = 1;
+                               s->d1->next_handshake_write_seq = 1;
                                goto end;
                                }
                        
@@ -737,9 +743,6 @@
                /* number of bytes to write */
                s->init_num=p-buf;
                s->init_off=0;
-
-               /* buffer the message to handle re-xmits */
-               dtls1_buffer_message(s, 0);
                }
 
        /* s->state = DTLS1_ST_SW_HELLO_VERIFY_REQUEST_B */

dtls-listen-state-bugs-0.9.8.patch
Description: Binary data

dtls-listen-state-bugs-1.0.0.patch
Description: Binary data

dtls-listen-state-bugs-1.0.1.patch
Description: Binary data