SSL: refactor (split up) handshake dissection

Split up parsing of data from the record layer and actual dissection of
the handshake messages. This prepares for handshake reassembly feature.

No functional change in dissection result is intended.

While at it, remove useless "content_type" parameter (it is always
Handshake) and add some comments. Also fix one edge case that is
normally not triggered (the "offset + length <= record_length" sanity
check did not include the four bytes for the type and length fields).

Change-Id: I0ca08c1e044c43b92bca690c7596f609b3bb1673

1 files changed, 103 insertions, 73 deletions

diff --git a/epan/dissectors/packet-ssl.c b/epan/dissectors/packet-ssl.c
index 869355b076..8b1bad7765 100644
--- a/epan/dissectors/packet-ssl.c
+++ b/epan/dissectors/packet-ssl.c
@@ -556,12 +556,12 @@ static void dissect_ssl3_alert(tvbuff_t *tvb, packet_info *pinfo,
                                const SslSession *session);
 
 /* handshake protocol dissector */
-static void dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
+static void process_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
                                    proto_tree *tree, guint32 offset,
                                    guint32 record_length, gboolean maybe_encrypted,
                                    SslSession *session, gint is_from_server,
                                    SslDecryptSession *conv_data,
-                                   const guint8 content_type, const guint16 version);
+                                   const guint16 version);
 
 /* heartbeat message dissector */
 static void dissect_ssl3_heartbeat(tvbuff_t *tvb, packet_info *pinfo,
@@ -1845,13 +1845,13 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
         break;
     case SSL_ID_HANDSHAKE:
         if (decrypted) {
-            dissect_ssl3_handshake(decrypted, pinfo, ssl_record_tree, 0,
+            process_ssl3_handshake(decrypted, pinfo, ssl_record_tree, 0,
                                    tvb_reported_length(decrypted), FALSE, session,
-                                   is_from_server, ssl, content_type, version);
+                                   is_from_server, ssl, version);
         } else {
-            dissect_ssl3_handshake(tvb, pinfo, ssl_record_tree, offset,
+            process_ssl3_handshake(tvb, pinfo, ssl_record_tree, offset,
                                    record_length, TRUE, session,
-                                   is_from_server, ssl, content_type, version);
+                                   is_from_server, ssl, version);
         }
         break;
     case SSL_ID_APP_DATA:
@@ -2001,10 +2001,9 @@ dissect_ssl3_alert(tvbuff_t *tvb, packet_info *pinfo,
 static void
 dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
                        proto_tree *tree, guint32 offset,
-                       guint32 record_length, gboolean maybe_encrypted,
                        SslSession *session, gint is_from_server,
-                       SslDecryptSession *ssl, const guint8 content_type,
-                       const guint16 version)
+                       SslDecryptSession *ssl, const guint16 version,
+                       gboolean is_first_message)
 {
     /*     struct {
      *         HandshakeType msg_type;
@@ -2030,79 +2029,35 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
     const gchar *msg_type_str;
     guint8       msg_type;
     guint32      length;
-    gboolean     first_iteration = TRUE;
     proto_item  *ti;
 
-    /*
-     * If this is not a decrypted buffer, then perhaps it is still in plaintext.
-     * Heuristics: if the buffer is too small, it is likely not encrypted.
-     * Otherwise assume that the Handshake does not contain two successive
-     * HelloRequest messages (type=0x00 length=0x000000, type=0x00). If this
-     * occurs, then we have possibly found the explicit nonce preceding the
-     * encrypted contents for GCM/CCM cipher suites as used in TLS 1.2.
-     */
-    if (maybe_encrypted) {
-        maybe_encrypted = tvb_bytes_exist(tvb, offset, 5) && tvb_get_ntoh40(tvb, offset) == 0;
-    }
-
-    /* just as there can be multiple records per packet, there
-     * can be multiple messages per record as long as they have
-     * the same content type
-     *
-     * we really only care about this for handshake messages
-     */
-
-    /* set record_length to the max offset */
-    record_length += offset;
-    while (offset < record_length)
     {
         guint32 hs_offset = offset;
 
         msg_type = tvb_get_guint8(tvb, offset);
         length   = tvb_get_ntoh24(tvb, offset + 1);
 
-        /* Check the length in the handshake message. Assume it's an
-         * encrypted handshake message if the message would pass
-         * the record_length boundary. This is a workaround for the
-         * situation where the first octet of the encrypted handshake
-         * message is actually a known handshake message type.
-         */
-        if (!maybe_encrypted && offset + length <= record_length)
-            msg_type_str = try_val_to_str(msg_type, ssl_31_handshake_type);
-        else
-            msg_type_str = NULL;
+        msg_type_str = try_val_to_str(msg_type, ssl_31_handshake_type);
+        /* Caller should have checked that the msg type is valid! */
+        DISSECTOR_ASSERT(msg_type_str != NULL);
 
         ssl_debug_printf("dissect_ssl3_handshake iteration %d type %d offset %d length %d "
-            "bytes, remaining %d \n", first_iteration, msg_type, offset, length, record_length);
-        if (!msg_type_str && !first_iteration)
-        {
-            /* only dissect / report messages if they're
-             * either the first message in this record
-             * or they're a valid message type
-             */
-            return;
-        }
+            "bytes\n", is_first_message, msg_type, offset, length);
 
         /*
          * Update our info string
          */
-        col_append_sep_str(pinfo->cinfo, COL_INFO, NULL,
-                            (msg_type_str != NULL) ? msg_type_str : "Encrypted Handshake Message");
+        col_append_sep_str(pinfo->cinfo, COL_INFO, NULL, msg_type_str);
 
         /* set the label text on the record layer expanding node */
-        if (first_iteration)
-        {
-            proto_item_set_text(tree, "%s Record Layer: %s Protocol: %s",
+        if (is_first_message) {
+            proto_item_set_text(tree, "%s Record Layer: Handshake Protocol: %s",
                     val_to_str_const(version, ssl_version_short_names, "SSL"),
-                    val_to_str_const(content_type, ssl_31_content_type, "unknown"),
-                    (msg_type_str!=NULL) ? msg_type_str :
-                    "Encrypted Handshake Message");
-        }
-        else
-        {
-            proto_item_set_text(tree, "%s Record Layer: %s Protocol: %s",
+                    msg_type_str);
+        } else {
+            /* TODO just append sth like ", Certificate" instead? */
+            proto_item_set_text(tree, "%s Record Layer: Handshake Protocol: %s",
                     val_to_str_const(version, ssl_version_short_names, "SSL"),
-                    val_to_str_const(content_type, ssl_31_content_type, "unknown"),
                     "Multiple Handshake Messages");
         }
 
@@ -2112,13 +2067,7 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
         ssl_hand_tree = proto_item_add_subtree(ti, ett_ssl_handshake);
 
         /* set the text label on the subtree node */
-        proto_item_set_text(ssl_hand_tree, "Handshake Protocol: %s",
-                (msg_type_str != NULL) ? msg_type_str :
-                "Encrypted Handshake Message");
-
-        /* if we don't have a valid handshake type, just quit dissecting */
-        if (!msg_type_str)
-            return;
+        proto_item_set_text(ssl_hand_tree, "Handshake Protocol: %s", msg_type_str);
 
         /* add nodes for the message type and message length */
         proto_tree_add_uint(ssl_hand_tree, hf_ssl_handshake_type,
@@ -2278,9 +2227,90 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
                 dissect_ssl3_hnd_encrypted_exts(tvb, ssl_hand_tree, offset);
                 break;
         }
+    }
+}
+
+/* process Handshake protocol messages, performing reassembly if needed. */
+static void
+process_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
+                       proto_tree *tree, guint32 offset,
+                       guint32 record_length, gboolean maybe_encrypted,
+                       SslSession *session, gint is_from_server,
+                       SslDecryptSession *ssl, const guint16 version)
+{
+    /*
+     * A single record can have multiple handshake messages and a single
+     * handshake message can span multiple records. This function uses a loop to
+     * handle the former and reassembly to handle the latter.
+     */
+    guint32      offset_end = offset + record_length;
+    const gchar *msg_type_str;
+    guint8       msg_type;
+    guint32      length;
+    gboolean     is_first_message = TRUE;
+    gboolean     is_continuation = FALSE;
+
+    if (!is_continuation) {
+        /*
+         * If this is not a decrypted buffer, then perhaps it is still in plaintext.
+         * Heuristics: if the buffer is too small, it is likely not encrypted.
+         * Otherwise assume that the Handshake does not contain two successive
+         * HelloRequest messages (type=0x00 length=0x000000, type=0x00). If this
+         * occurs, then we have possibly found the explicit nonce preceding the
+         * encrypted contents for GCM/CCM cipher suites as used in TLS 1.2.
+         */
+        if (maybe_encrypted) {
+            maybe_encrypted = tvb_bytes_exist(tvb, offset, 5) && tvb_get_ntoh40(tvb, offset) == 0;
+        }
+        if (maybe_encrypted) {
+            msg_type = tvb_get_guint8(tvb, offset);
+            msg_type_str = try_val_to_str(msg_type, ssl_31_handshake_type);
+            /*
+             * TODO reduce false detection of plaintext from 7.8% (20/256) to
+             * .61% by checking the next valid content type.
+             */
+            if (msg_type_str == NULL) {
+                col_append_sep_str(pinfo->cinfo, COL_INFO, NULL,
+                                   "Encrypted Handshake Message");
+
+                /* set the label text on the record layer expanding node */
+                proto_item_set_text(tree, "%s Record Layer: Handshake Protocol: %s",
+                        val_to_str_const(version, ssl_version_short_names, "SSL"),
+                        "Encrypted Handshake Message");
+                return;
+            }
+        }
+    }
+
+    while (offset < offset_end) {
+        msg_type = tvb_get_guint8(tvb, offset);
+        length   = tvb_get_ntoh24(tvb, offset + 1);
+
+        /* Check the length in the handshake message. Assume it's an
+         * encrypted handshake message if the message would pass
+         * the record_length boundary. This is a workaround for the
+         * situation where the first octet of the encrypted handshake
+         * message is actually a known handshake message type.
+         */
+        if (!maybe_encrypted && offset + 4 + length <= offset_end)
+            msg_type_str = try_val_to_str(msg_type, ssl_31_handshake_type);
+        else
+            msg_type_str = NULL;
+
+        if (!msg_type_str) {
+            /* only dissect further messages if the message type is valid. */
+            return;
+        }
+
+        /*
+         * We have a valid message type and received all data, start the
+         * actual handshake dissection.
+         */
+        dissect_ssl3_handshake(tvb, pinfo, tree, offset, session,
+                               is_from_server, ssl, version, is_first_message);
 
-        offset += length;
-        first_iteration = FALSE; /* set up for next pass, if any */
+        offset += 4 + length;
+        is_first_message = FALSE; /* set up for next pass, if any */
     }
 }