[Backport 3.6] Defragment incoming TLS handshake messages (context extension pointer)

ChangeLog.d/tls-hs-defrag-in.txt

@@ -0,0 +1,2 @@
+Changes
+   * Defragment incoming TLS handshake messages.

include/mbedtls/ssl.h

@@ -1684,6 +1684,16 @@ struct mbedtls_ssl_config {
 #endif
 };
 
+/* We need to add more fields to ssl_context, but can't in this branch (3.6)
+ * since we're not allowed to change the ABI. So, put a pointer to this struct
+ * in place of on of the existing pointers in ssl_context - picked in_iv
+ * because it's one of the least used, so that minimizes the disruption. */
+typedef struct {
+    unsigned char *MBEDTLS_PRIVATE(in_iv);       /*!< ivlen-byte IV                    */
+    unsigned char *MBEDTLS_PRIVATE(in_hshdr);    /*!< original handshake header start  */
+    size_t MBEDTLS_PRIVATE(in_hsfraglen);        /*!< accumulated hs fragments length  */
+} mbedtls_ssl_context_in_ext;
+
 struct mbedtls_ssl_context {
     const mbedtls_ssl_config *MBEDTLS_PRIVATE(conf); /*!< configuration information          */
 
@@ -1795,7 +1805,7 @@ struct mbedtls_ssl_context {
                                                   *   (the end is marked by in_len).   */
 #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */
     unsigned char *MBEDTLS_PRIVATE(in_len);      /*!< two-bytes message length field   */
-    unsigned char *MBEDTLS_PRIVATE(in_iv);       /*!< ivlen-byte IV                    */
+    unsigned char *MBEDTLS_PRIVATE(in_ext);      /*!< extension structure (ABI compat) */
     unsigned char *MBEDTLS_PRIVATE(in_msg);      /*!< message contents (in_iv+ivlen)   */
     unsigned char *MBEDTLS_PRIVATE(in_offt);     /*!< read offset in application data  */
 

library/ssl_misc.h

@@ -409,6 +409,11 @@ uint32_t mbedtls_ssl_get_extension_mask(unsigned int extension_type);
 #define MBEDTLS_CLIENT_HELLO_RANDOM_LEN 32
 #define MBEDTLS_SERVER_HELLO_RANDOM_LEN 32
 
+static inline mbedtls_ssl_context_in_ext *mbedtls_ssl_get_in_ext(const mbedtls_ssl_context *ssl)
+{
+    return (mbedtls_ssl_context_in_ext *) ssl->in_ext;
+}
+
 #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH)
 /**
  * \brief          Return the maximum fragment length (payload, in bytes) for
@@ -1830,10 +1835,11 @@ void mbedtls_ssl_set_timer(mbedtls_ssl_context *ssl, uint32_t millisecs);
 MBEDTLS_CHECK_RETURN_CRITICAL
 int mbedtls_ssl_check_timer(mbedtls_ssl_context *ssl);
 
-void mbedtls_ssl_reset_in_out_pointers(mbedtls_ssl_context *ssl);
+void mbedtls_ssl_reset_in_pointers(mbedtls_ssl_context *ssl);
+void mbedtls_ssl_update_in_pointers(mbedtls_ssl_context *ssl);
+void mbedtls_ssl_reset_out_pointers(mbedtls_ssl_context *ssl);
 void mbedtls_ssl_update_out_pointers(mbedtls_ssl_context *ssl,
                                      mbedtls_ssl_transform *transform);
-void mbedtls_ssl_update_in_pointers(mbedtls_ssl_context *ssl);
 
 MBEDTLS_CHECK_RETURN_CRITICAL
 int mbedtls_ssl_session_reset_int(mbedtls_ssl_context *ssl, int partial);

library/ssl_msg.c

@@ -3220,13 +3220,18 @@ static uint32_t ssl_get_hs_total_len(mbedtls_ssl_context const *ssl)
 
 int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl)
 {
-    if (ssl->in_msglen < mbedtls_ssl_hs_hdr_len(ssl)) {
+    /* First handshake fragment must at least include the header. */
+    if (ssl->in_msglen < mbedtls_ssl_hs_hdr_len(ssl) && ssl->in_hslen == 0) {
         MBEDTLS_SSL_DEBUG_MSG(1, ("handshake message too short: %" MBEDTLS_PRINTF_SIZET,
                                   ssl->in_msglen));
         return MBEDTLS_ERR_SSL_INVALID_RECORD;
     }
 
-    ssl->in_hslen = mbedtls_ssl_hs_hdr_len(ssl) + ssl_get_hs_total_len(ssl);
+    if (ssl->in_hslen == 0) {
+        ssl->in_hslen = mbedtls_ssl_hs_hdr_len(ssl) + ssl_get_hs_total_len(ssl);
+        mbedtls_ssl_get_in_ext(ssl)->in_hsfraglen = 0;
+        mbedtls_ssl_get_in_ext(ssl)->in_hshdr = ssl->in_hdr;
+    }
 
     MBEDTLS_SSL_DEBUG_MSG(3, ("handshake message: msglen ="
                               " %" MBEDTLS_PRINTF_SIZET ", type = %u, hslen = %"
@@ -3292,10 +3297,61 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl)
         }
     } else
 #endif /* MBEDTLS_SSL_PROTO_DTLS */
-    /* With TLS we don't handle fragmentation (for now) */
-    if (ssl->in_msglen < ssl->in_hslen) {
-        MBEDTLS_SSL_DEBUG_MSG(1, ("TLS handshake fragmentation not supported"));
-        return MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE;
+    {
+        if (mbedtls_ssl_get_in_ext(ssl)->in_hsfraglen > ssl->in_hslen) {
+            return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
+        }
+        int ret;
+        const size_t hs_remain = ssl->in_hslen - mbedtls_ssl_get_in_ext(ssl)->in_hsfraglen;
+        MBEDTLS_SSL_DEBUG_MSG(3,
+                              ("handshake fragment: %" MBEDTLS_PRINTF_SIZET " .. %"
+                               MBEDTLS_PRINTF_SIZET " of %"
+                               MBEDTLS_PRINTF_SIZET " msglen %" MBEDTLS_PRINTF_SIZET,
+                               mbedtls_ssl_get_in_ext(ssl)->in_hsfraglen,
+                               mbedtls_ssl_get_in_ext(ssl)->in_hsfraglen +
+                               (hs_remain <= ssl->in_msglen ? hs_remain : ssl->in_msglen),
+                               ssl->in_hslen, ssl->in_msglen));
+        if (ssl->in_msglen < hs_remain) {
+            mbedtls_ssl_get_in_ext(ssl)->in_hsfraglen += ssl->in_msglen;
+            ssl->in_hdr = ssl->in_msg + ssl->in_msglen;
+            ssl->in_msglen = 0;
+            mbedtls_ssl_update_in_pointers(ssl);
+            return MBEDTLS_ERR_SSL_CONTINUE_PROCESSING;
+        }
+        if (mbedtls_ssl_get_in_ext(ssl)->in_hshdr != ssl->in_hdr) {
+            /*
+             * At mbedtls_ssl_get_in_ext(ssl)->in_hshdr we have a sequence of records that cover the next handshake
+             * record, each with its own record header that we need to remove.
+             * Note that the reassembled record size may not equal the size of the message,
+             * there maybe bytes from the next message following it.
+             */
+            size_t merged_rec_len = 0;
+            unsigned char *p = mbedtls_ssl_get_in_ext(ssl)->in_hshdr, *q = NULL;
+            do {
+                mbedtls_record rec;
+                ret = ssl_parse_record_header(ssl, p, mbedtls_ssl_in_hdr_len(ssl), &rec);
+                if (ret != 0) {
+                    return ret;
+                }
+                merged_rec_len += rec.data_len;
+                p = rec.buf + rec.buf_len;
+                if (q != NULL) {
+                    memmove(q, rec.buf + rec.data_offset, rec.data_len);
+                    q += rec.data_len;
+                } else {
+                    q = p;
+                }
+            } while (merged_rec_len < ssl->in_hslen);
+            ssl->in_hdr = mbedtls_ssl_get_in_ext(ssl)->in_hshdr;
+            mbedtls_ssl_update_in_pointers(ssl);
+            ssl->in_msglen = merged_rec_len;
+            /* Adjust message length. */
+            MBEDTLS_PUT_UINT16_BE(merged_rec_len, ssl->in_len, 0);
+            mbedtls_ssl_get_in_ext(ssl)->in_hsfraglen = 0;
+            mbedtls_ssl_get_in_ext(ssl)->in_hshdr = NULL;
+            MBEDTLS_SSL_DEBUG_BUF(4, "reassembled record",
+                                  ssl->in_hdr, mbedtls_ssl_in_hdr_len(ssl) + merged_rec_len);
+        }
     }
 
     return 0;
@@ -4640,6 +4696,16 @@ static int ssl_consume_current_message(mbedtls_ssl_context *ssl)
             return MBEDTLS_ERR_SSL_INTERNAL_ERROR;
         }
 
+        if (mbedtls_ssl_get_in_ext(ssl)->in_hsfraglen != 0) {
+            /* Not all handshake fragments have arrived, do not consume. */
+            MBEDTLS_SSL_DEBUG_MSG(3,
+                                  ("waiting for more fragments (%" MBEDTLS_PRINTF_SIZET " of %"
+                                   MBEDTLS_PRINTF_SIZET ", %" MBEDTLS_PRINTF_SIZET " left)",
+                                   mbedtls_ssl_get_in_ext(ssl)->in_hsfraglen, ssl->in_hslen,
+                                   ssl->in_hslen - mbedtls_ssl_get_in_ext(ssl)->in_hsfraglen));
+            return 0;
+        }
+
         /*
          * Get next Handshake message in the current record
          */
@@ -4665,6 +4731,7 @@ static int ssl_consume_current_message(mbedtls_ssl_context *ssl)
             ssl->in_msglen -= ssl->in_hslen;
             memmove(ssl->in_msg, ssl->in_msg + ssl->in_hslen,
                     ssl->in_msglen);
+            MBEDTLS_PUT_UINT16_BE(ssl->in_msglen, ssl->in_len, 0);
 
             MBEDTLS_SSL_DEBUG_BUF(4, "remaining content in record",
                                   ssl->in_msg, ssl->in_msglen);
@@ -4888,7 +4955,7 @@ static int ssl_get_next_record(mbedtls_ssl_context *ssl)
 #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID)
                 ssl->in_len = ssl->in_cid + rec.cid_len;
 #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */
-                ssl->in_iv  = ssl->in_msg = ssl->in_len + 2;
+                mbedtls_ssl_get_in_ext(ssl)->in_iv  = ssl->in_msg = ssl->in_len + 2;
                 ssl->in_msglen = rec.data_len;
 
                 ret = ssl_check_client_reconnect(ssl);
@@ -5007,7 +5074,7 @@ static int ssl_get_next_record(mbedtls_ssl_context *ssl)
 #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID)
     ssl->in_len = ssl->in_cid + rec.cid_len;
 #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */
-    ssl->in_iv  = ssl->in_len + 2;
+    mbedtls_ssl_get_in_ext(ssl)->in_iv  = ssl->in_len + 2;
 
     /* The record content type may change during decryption,
      * so re-read it. */
@@ -5313,7 +5380,7 @@ void mbedtls_ssl_update_out_pointers(mbedtls_ssl_context *ssl,
 void mbedtls_ssl_update_in_pointers(mbedtls_ssl_context *ssl)
 {
     /* This function sets the pointers to match the case
-     * of unprotected TLS/DTLS records, with both  ssl->in_iv
+     * of unprotected TLS/DTLS records, with both  mbedtls_ssl_get_in_ext(ssl)->in_iv
      * and ssl->in_msg pointing to the beginning of the record
      * content.
      *
@@ -5335,44 +5402,55 @@ void mbedtls_ssl_update_in_pointers(mbedtls_ssl_context *ssl)
 #else /* MBEDTLS_SSL_DTLS_CONNECTION_ID */
         ssl->in_len = ssl->in_ctr + MBEDTLS_SSL_SEQUENCE_NUMBER_LEN;
 #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */
-        ssl->in_iv  = ssl->in_len + 2;
+        mbedtls_ssl_get_in_ext(ssl)->in_iv  = ssl->in_len + 2;
     } else
 #endif
     {
-        ssl->in_ctr = ssl->in_hdr - MBEDTLS_SSL_SEQUENCE_NUMBER_LEN;
+        ssl->in_ctr = ssl->in_buf;
         ssl->in_len = ssl->in_hdr + 3;
 #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID)
         ssl->in_cid = ssl->in_len;
 #endif
-        ssl->in_iv  = ssl->in_hdr + 5;
+        mbedtls_ssl_get_in_ext(ssl)->in_iv  = ssl->in_hdr + 5;
     }
 
     /* This will be adjusted at record decryption time. */
-    ssl->in_msg = ssl->in_iv;
+    ssl->in_msg = mbedtls_ssl_get_in_ext(ssl)->in_iv;
 }
 
 /*
  * Setup an SSL context
  */
 
-void mbedtls_ssl_reset_in_out_pointers(mbedtls_ssl_context *ssl)
+void mbedtls_ssl_reset_in_pointers(mbedtls_ssl_context *ssl)
+{
+#if defined(MBEDTLS_SSL_PROTO_DTLS)
+    if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) {
+        ssl->in_hdr = ssl->in_buf;
+    } else
+#endif  /* MBEDTLS_SSL_PROTO_DTLS */
+    {
+        ssl->in_hdr = ssl->in_buf + MBEDTLS_SSL_SEQUENCE_NUMBER_LEN;
+    }
+
+    /* Derive other internal pointers. */
+    mbedtls_ssl_update_in_pointers(ssl);
+}
+
+void mbedtls_ssl_reset_out_pointers(mbedtls_ssl_context *ssl)
 {
     /* Set the incoming and outgoing record pointers. */
 #if defined(MBEDTLS_SSL_PROTO_DTLS)
     if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) {
         ssl->out_hdr = ssl->out_buf;
-        ssl->in_hdr  = ssl->in_buf;
     } else
 #endif /* MBEDTLS_SSL_PROTO_DTLS */
     {
         ssl->out_ctr = ssl->out_buf;
-        ssl->out_hdr = ssl->out_buf + 8;
-        ssl->in_hdr  = ssl->in_buf  + 8;
+        ssl->out_hdr = ssl->out_buf + MBEDTLS_SSL_SEQUENCE_NUMBER_LEN;
     }
-
     /* Derive other internal pointers. */
     mbedtls_ssl_update_out_pointers(ssl, NULL /* no transform enabled */);
-    mbedtls_ssl_update_in_pointers(ssl);
 }
 
 /*

library/ssl_tls.c

@@ -344,12 +344,17 @@ static void handle_buffer_resizing(mbedtls_ssl_context *ssl, int downsizing,
                                    size_t out_buf_new_len)
 {
     int modified = 0;
-    size_t written_in = 0, iv_offset_in = 0, len_offset_in = 0;
+    size_t written_in = 0, iv_offset_in = 0, len_offset_in = 0, hdr_in = 0;
     size_t written_out = 0, iv_offset_out = 0, len_offset_out = 0;
+    size_t hshdr_in = 0;
     if (ssl->in_buf != NULL) {
         written_in = ssl->in_msg - ssl->in_buf;
-        iv_offset_in = ssl->in_iv - ssl->in_buf;
+        iv_offset_in = mbedtls_ssl_get_in_ext(ssl)->in_iv - ssl->in_buf;
         len_offset_in = ssl->in_len - ssl->in_buf;
+        hdr_in = ssl->in_hdr - ssl->in_buf;
+        if (mbedtls_ssl_get_in_ext(ssl)->in_hshdr != NULL) {
+            hshdr_in = mbedtls_ssl_get_in_ext(ssl)->in_hshdr - ssl->in_buf;
+        }
         if (downsizing ?
             ssl->in_buf_len > in_buf_new_len && ssl->in_left < in_buf_new_len :
             ssl->in_buf_len < in_buf_new_len) {
@@ -381,7 +386,10 @@ static void handle_buffer_resizing(mbedtls_ssl_context *ssl, int downsizing,
     }
     if (modified) {
         /* Update pointers here to avoid doing it twice. */
-        mbedtls_ssl_reset_in_out_pointers(ssl);
+        ssl->in_hdr = ssl->in_buf + hdr_in;
+        mbedtls_ssl_update_in_pointers(ssl);
+        mbedtls_ssl_reset_out_pointers(ssl);
+
         /* Fields below might not be properly updated with record
          * splitting or with CID, so they are manually updated here. */
         ssl->out_msg = ssl->out_buf + written_out;
@@ -390,7 +398,10 @@ static void handle_buffer_resizing(mbedtls_ssl_context *ssl, int downsizing,
 
         ssl->in_msg = ssl->in_buf + written_in;
         ssl->in_len = ssl->in_buf + len_offset_in;
-        ssl->in_iv = ssl->in_buf + iv_offset_in;
+        mbedtls_ssl_get_in_ext(ssl)->in_iv = ssl->in_buf + iv_offset_in;
+        if (mbedtls_ssl_get_in_ext(ssl)->in_hshdr != NULL) {
+            mbedtls_ssl_get_in_ext(ssl)->in_hshdr = ssl->in_buf + hshdr_in;
+        }
     }
 }
 #endif /* MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH */
@@ -1398,6 +1409,14 @@ int mbedtls_ssl_setup(mbedtls_ssl_context *ssl,
         ret = MBEDTLS_ERR_SSL_ALLOC_FAILED;
         goto error;
     }
+    ssl->in_ext = mbedtls_calloc(1, sizeof(mbedtls_ssl_context_in_ext));
+    if (ssl->in_ext == NULL) {
+        MBEDTLS_SSL_DEBUG_MSG(1,
+                              ("alloc(%" MBEDTLS_PRINTF_SIZET " bytes) failed",
+                               sizeof(mbedtls_ssl_context_in_ext)));
+        ret = MBEDTLS_ERR_SSL_ALLOC_FAILED;
+        goto error;
+    }
 
 #if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH)
     ssl->out_buf_len = out_buf_len;
@@ -1409,7 +1428,8 @@ int mbedtls_ssl_setup(mbedtls_ssl_context *ssl,
         goto error;
     }
 
-    mbedtls_ssl_reset_in_out_pointers(ssl);
+    mbedtls_ssl_reset_in_pointers(ssl);
+    mbedtls_ssl_reset_out_pointers(ssl);
 
 #if defined(MBEDTLS_SSL_DTLS_SRTP)
     memset(&ssl->dtls_srtp_info, 0, sizeof(ssl->dtls_srtp_info));
@@ -1423,6 +1443,7 @@ int mbedtls_ssl_setup(mbedtls_ssl_context *ssl,
 
 error:
     mbedtls_free(ssl->in_buf);
+    mbedtls_free(ssl->in_ext);
     mbedtls_free(ssl->out_buf);
 
     ssl->conf = NULL;
@@ -1432,12 +1453,13 @@ int mbedtls_ssl_setup(mbedtls_ssl_context *ssl,
     ssl->out_buf_len = 0;
 #endif
     ssl->in_buf = NULL;
+    ssl->in_ext = NULL;
     ssl->out_buf = NULL;
 
     ssl->in_hdr = NULL;
     ssl->in_ctr = NULL;
     ssl->in_len = NULL;
-    ssl->in_iv = NULL;
+    mbedtls_ssl_get_in_ext(ssl)->in_iv = NULL;
     ssl->in_msg = NULL;
 
     ssl->out_hdr = NULL;
@@ -1474,7 +1496,8 @@ void mbedtls_ssl_session_reset_msg_layer(mbedtls_ssl_context *ssl,
     /* Cancel any possibly running timer */
     mbedtls_ssl_set_timer(ssl, 0);
 
-    mbedtls_ssl_reset_in_out_pointers(ssl);
+    mbedtls_ssl_reset_in_pointers(ssl);
+    mbedtls_ssl_reset_out_pointers(ssl);
 
     /* Reset incoming message parsing */
     ssl->in_offt    = NULL;
@@ -1484,6 +1507,8 @@ void mbedtls_ssl_session_reset_msg_layer(mbedtls_ssl_context *ssl,
     ssl->in_hslen   = 0;
     ssl->keep_current_message = 0;
     ssl->transform_in  = NULL;
+    mbedtls_ssl_get_in_ext(ssl)->in_hshdr = NULL;
+    mbedtls_ssl_get_in_ext(ssl)->in_hsfraglen = 0;
 
 #if defined(MBEDTLS_SSL_PROTO_DTLS)
     ssl->next_record_offset = 0;
@@ -5542,6 +5567,11 @@ void mbedtls_ssl_free(mbedtls_ssl_context *ssl)
         ssl->in_buf = NULL;
     }
 
+    if (ssl->in_ext != NULL) {
+        mbedtls_free(ssl->in_ext);
+        ssl->in_ext = NULL;
+    }
+
     if (ssl->transform) {
         mbedtls_ssl_transform_free(ssl->transform);
         mbedtls_free(ssl->transform);