[openssl-commits] [openssl] master update

[Matt Caswell]

The branch master has been updated
       via  67f60be8c9ae5ff3129fcd6238baf124385a41d8 (commit)
       via  2ad226e88bee97847496e542d63c67997d5beda6 (commit)
      from  02dc0b82ab19c32bf072213feff746b5b35f8ef6 (commit)


- Log -----------------------------------------------------------------
commit 67f60be8c9ae5ff3129fcd6238baf124385a41d8
Author: Matt Caswell <matt at openssl.org>
Date:   Wed Nov 4 11:20:50 2015 +0000

    Ensure |rwstate| is set correctly on BIO_flush
    
    A BIO_flush call in the DTLS code was not correctly setting the |rwstate|
    variable to SSL_WRITING. This means that SSL_get_error() will not return
    SSL_ERROR_WANT_WRITE in the event of an IO retry.
    
    Reviewed-by: Richard Levitte <levitte at openssl.org>

commit 2ad226e88bee97847496e542d63c67997d5beda6
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Nov 3 14:45:07 2015 +0000

    Fix DTLS handshake fragment retries
    
    If using DTLS and NBIO then if a second or subsequent handshake message
    fragment hits a retry, then the retry attempt uses the wrong fragment
    offset value. This commit restores the fragment offset from the last
    attempt.
    
    Reviewed-by: Richard Levitte <levitte at openssl.org>

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

Summary of changes:
 ssl/statem/statem_dtls.c | 70 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 49 insertions(+), 21 deletions(-)

diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c
index 6d73659..5194c73 100644
--- a/ssl/statem/statem_dtls.c
+++ b/ssl/statem/statem_dtls.c
@@ -250,8 +250,44 @@ int dtls1_do_write(SSL *s, int type)
         blocksize = 0;
 
     frag_off = 0;
+    s->rwstate = SSL_NOTHING;
+
     /* s->init_num shouldn't ever be < 0...but just in case */
     while (s->init_num > 0) {
+        if (type == SSL3_RT_HANDSHAKE && s->init_off != 0) {
+            /* We must be writing a fragment other than the first one */
+
+            if (frag_off > 0) {
+                /* This is the first attempt at writing out this fragment */
+
+                if (s->init_off <= DTLS1_HM_HEADER_LENGTH) {
+                    /*
+                     * Each fragment that was already sent must at least have
+                     * contained the message header plus one other byte.
+                     * Therefore |init_off| must have progressed by at least
+                     * |DTLS1_HM_HEADER_LENGTH + 1| bytes. If not something went
+                     * wrong.
+                     */
+                    return -1;
+                }
+
+                /*
+                 * Adjust |init_off| and |init_num| to allow room for a new
+                 * message header for this fragment.
+                 */
+                s->init_off -= DTLS1_HM_HEADER_LENGTH;
+                s->init_num += DTLS1_HM_HEADER_LENGTH;
+            } else {
+                /*
+                 * We must have been called again after a retry so use the
+                 * fragment offset from our last attempt. We do not need
+                 * to adjust |init_off| and |init_num| as above, because
+                 * that should already have been done before the retry.
+                 */
+                frag_off = s->d1->w_msg_hdr.frag_off;
+            }
+        }
+
         used_len = BIO_wpending(SSL_get_wbio(s)) + DTLS1_RT_HEADER_LENGTH
             + mac_size + blocksize;
         if (s->d1->mtu > used_len)
@@ -264,8 +300,10 @@ int dtls1_do_write(SSL *s, int type)
              * grr.. we could get an error if MTU picked was wrong
              */
             ret = BIO_flush(SSL_get_wbio(s));
-            if (ret <= 0)
+            if (ret <= 0) {
+                s->rwstate = SSL_WRITING;
                 return ret;
+            }
             used_len = DTLS1_RT_HEADER_LENGTH + mac_size + blocksize;
             if (s->d1->mtu > used_len + DTLS1_HM_HEADER_LENGTH) {
                 curr_mtu = s->d1->mtu - used_len;
@@ -291,25 +329,6 @@ int dtls1_do_write(SSL *s, int type)
          * XDTLS: this function is too long.  split out the CCS part
          */
         if (type == SSL3_RT_HANDSHAKE) {
-            if (s->init_off != 0) {
-                OPENSSL_assert(s->init_off > DTLS1_HM_HEADER_LENGTH);
-                s->init_off -= DTLS1_HM_HEADER_LENGTH;
-                s->init_num += DTLS1_HM_HEADER_LENGTH;
-
-                /*
-                 * We just checked that s->init_num > 0 so this cast should
-                 * be safe
-                 */
-                if (((unsigned int)s->init_num) > curr_mtu)
-                    len = curr_mtu;
-                else
-                    len = s->init_num;
-            }
-
-            /* Shouldn't ever happen */
-            if (len > INT_MAX)
-                len = INT_MAX;
-
             if (len < DTLS1_HM_HEADER_LENGTH) {
                 /*
                  * len is so small that we really can't do anything sensible
@@ -397,7 +416,16 @@ int dtls1_do_write(SSL *s, int type)
             }
             s->init_off += ret;
             s->init_num -= ret;
-            frag_off += (ret -= DTLS1_HM_HEADER_LENGTH);
+            ret -= DTLS1_HM_HEADER_LENGTH;
+            frag_off += ret;
+
+            /*
+             * We save the fragment offset for the next fragment so we have it
+             * available in case of an IO retry. We don't know the length of the
+             * next fragment yet so just set that to 0 for now. It will be
+             * updated again later.
+             */
+            dtls1_fix_message_header(s, frag_off, 0);
         }
     }
     return (0);