[openssl-commits] [openssl] master update

[Matt Caswell]

The branch master has been updated
       via  91d13f1a76216f7d67f7a3068bab2287831ca615 (commit)
       via  b821df5f5b8dbb9bae109ed01076cb4b393b67e0 (commit)
      from  e43a13c807e42688c72c4f3d001112bf0a110464 (commit)


- Log -----------------------------------------------------------------
commit 91d13f1a76216f7d67f7a3068bab2287831ca615
Author: Matt Caswell <matt at openssl.org>
Date:   Mon Jun 1 17:25:29 2015 +0100

    DTLS handshake message fragments musn't span packets
    
    It should not be possible for DTLS message fragments to span multiple
    packets. However previously if the message header fitted exactly into one
    packet, and the fragment body was in the next packet then this would work.
    Obviously this would fail if packets get re-ordered mid-flight.
    
    Reviewed-by: Tim Hudson <tjh at openssl.org>

commit b821df5f5b8dbb9bae109ed01076cb4b393b67e0
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Jun 2 08:57:02 2015 +0100

    Correct type of RECORD_LAYER_get_rrec_length()
    
    The underlying field returned by RECORD_LAYER_get_rrec_length() is an
    unsigned int. The return type of the function should match that.
    
    Reviewed-by: Tim Hudson <tjh at openssl.org>

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

Summary of changes:
 ssl/d1_both.c             | 26 ++++++++++++++++++--------
 ssl/record/rec_layer_s3.c |  2 +-
 ssl/record/record.h       |  2 +-
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index 569b561..155b8bf 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -868,6 +868,20 @@ dtls1_get_message_fragment(SSL *s, int st1, int stn, long max, int *ok)
     /* parse the message fragment header */
     dtls1_get_message_header(wire, &msg_hdr);
 
+    len = msg_hdr.msg_len;
+    frag_off = msg_hdr.frag_off;
+    frag_len = msg_hdr.frag_len;
+
+    /*
+     * We must have at least frag_len bytes left in the record to be read.
+     * Fragments must not span records.
+     */
+    if (frag_len > RECORD_LAYER_get_rrec_length(&s->rlayer)) {
+        al = SSL3_AD_ILLEGAL_PARAMETER;
+        SSLerr(SSL_F_DTLS1_GET_MESSAGE_FRAGMENT, SSL_R_BAD_LENGTH);
+        goto f_err;
+    }
+
     /*
      * if this is a future (or stale) message it gets buffered
      * (or dropped)--no further processing at this time
@@ -878,10 +892,6 @@ dtls1_get_message_fragment(SSL *s, int st1, int stn, long max, int *ok)
         && !(s->d1->listen && msg_hdr.seq == 1))
         return dtls1_process_out_of_seq_message(s, &msg_hdr, ok);
 
-    len = msg_hdr.msg_len;
-    frag_off = msg_hdr.frag_off;
-    frag_len = msg_hdr.frag_len;
-
     if (frag_len && frag_len < len)
         return dtls1_reassemble_fragment(s, &msg_hdr, ok);
 
@@ -912,17 +922,16 @@ dtls1_get_message_fragment(SSL *s, int st1, int stn, long max, int *ok)
     if ((al = dtls1_preprocess_fragment(s, &msg_hdr, max)))
         goto f_err;
 
-    /* XDTLS:  ressurect this when restart is in place */
-    s->state = stn;
-
     if (frag_len > 0) {
         unsigned char *p =
             (unsigned char *)s->init_buf->data + DTLS1_HM_HEADER_LENGTH;
 
         i = s->method->ssl_read_bytes(s, SSL3_RT_HANDSHAKE,
                                       &p[frag_off], frag_len, 0);
+
         /*
-         * XDTLS: fix this--message fragments cannot span multiple packets
+         * This shouldn't ever fail due to NBIO because we already checked
+         * that we have enough data in the record
          */
         if (i <= 0) {
             s->rwstate = SSL_READING;
@@ -943,6 +952,7 @@ dtls1_get_message_fragment(SSL *s, int st1, int stn, long max, int *ok)
     }
 
     *ok = 1;
+    s->state = stn;
 
     /*
      * Note that s->init_num is *not* used as current offset in
diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 47a021d..79d3c21 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -1504,7 +1504,7 @@ int RECORD_LAYER_is_sslv2_record(RECORD_LAYER *rl)
 /*
  * Returns the length in bytes of the current rrec
  */
-int RECORD_LAYER_get_rrec_length(RECORD_LAYER *rl)
+unsigned int RECORD_LAYER_get_rrec_length(RECORD_LAYER *rl)
 {
     return SSL3_RECORD_get_length(&rl->rrec);
 }
diff --git a/ssl/record/record.h b/ssl/record/record.h
index cf1607c..6931bb4 100644
--- a/ssl/record/record.h
+++ b/ssl/record/record.h
@@ -326,7 +326,7 @@ void RECORD_LAYER_reset_read_sequence(RECORD_LAYER *rl);
 void RECORD_LAYER_reset_write_sequence(RECORD_LAYER *rl);
 int RECORD_LAYER_setup_comp_buffer(RECORD_LAYER *rl);
 int RECORD_LAYER_is_sslv2_record(RECORD_LAYER *rl);
-int RECORD_LAYER_get_rrec_length(RECORD_LAYER *rl);
+unsigned int RECORD_LAYER_get_rrec_length(RECORD_LAYER *rl);
 __owur int ssl3_pending(const SSL *s);
 __owur int ssl3_write_bytes(SSL *s, int type, const void *buf, int len);
 __owur int do_ssl3_write(SSL *s, int type, const unsigned char *buf,