[openssl-commits] [openssl] OpenSSL_1_0_2-stable update

[Matt Caswell]

The branch OpenSSL_1_0_2-stable has been updated
       via  7321d7944e56e3cf7f5cf80679e6c88a130167f2 (commit)
      from  c4bfccfff85cea4dd16d701868e0efa9d80384cc (commit)


- Log -----------------------------------------------------------------
commit 7321d7944e56e3cf7f5cf80679e6c88a130167f2
Author: Matt Caswell <matt at openssl.org>
Date:   Mon Mar 13 10:30:49 2017 +0000

    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 at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/2915)

-----------------------------------------------------------------------

Summary of changes:
 ssl/d1_both.c | 15 +++++++++++----
 ssl/d1_srvr.c |  9 ---------
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index 9bc6153..232a6d4 100644
--- 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 32eae6b..8502b24 100644
--- 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;