[TDS] Convert to using tcp_dissect_pdus.

Bring some modernity to this dissector and use tcp_dissect_pdus.  Also an excuse to
remove the conversation_set_dissector in the heuristic dissector which was generating
some false positives because the heuristic isn't that strong.

Bug: 12882
Change-Id: Ibb04fd4fbc819acd1dc96d6259b047c897ec2de6
Reviewed-on: https://code.wireshark.org/review/19125
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>

1 files changed, 72 insertions, 214 deletions

diff --git a/epan/dissectors/packet-tds.c b/epan/dissectors/packet-tds.c
index b51e51849d..3056184b70 100644
--- a/epan/dissectors/packet-tds.c
+++ b/epan/dissectors/packet-tds.c
@@ -98,8 +98,6 @@
  *
  * Here are some of the (hefty) limitations of the current code
  *
- * . We currently do not handle netlib headers that cross packet boundaries.
- *   This should be an easy fix.
  * . I probably could have used the packet reassembly stuff, but I started
  *   this at version 0.8.20, so c'est la vie. It wouldn't have covered the
  *   netlib stuff anyway, so no big loss.
@@ -3796,9 +3794,8 @@ static void
 dissect_netlib_buffer(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
     int offset = 0;
-    proto_item *tds_item = NULL;
-    proto_tree *tds_tree = NULL;
-    proto_tree *tds_status_tree = NULL;
+    proto_item *tds_item;
+    proto_tree *tds_tree;
     guint8 type;
     guint8 status;
     guint16 channel;
@@ -3810,6 +3807,15 @@ dissect_netlib_buffer(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
     conversation_t *conv;
     tds_conv_info_t *tds_info;
 
+    static const int *status_flags[] = {
+        &hf_tds_status_eom,
+        &hf_tds_status_ignore,
+        &hf_tds_status_event_notif,
+        &hf_tds_status_reset_conn,
+        &hf_tds_status_reset_conn_skip_tran,
+        NULL
+    };
+
     if(detect_tls(tvb))
     {
         tds_item = proto_tree_add_item(tree, hf_tds_prelogin, tvb, 0, -1, ENC_NA);
@@ -3825,26 +3831,22 @@ dissect_netlib_buffer(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
         conversation_add_proto_data(conv, proto_tds, tds_info);
     }
 
-    type = tvb_get_guint8(tvb, offset);
-    status = tvb_get_guint8(tvb, offset + 1);
-    channel = tvb_get_ntohs(tvb, offset + 4);
-    packet_number = tvb_get_guint8(tvb, offset + 6);
-
     /* create display subtree for the protocol */
     tds_item = proto_tree_add_item(tree, proto_tds, tvb, offset, -1, ENC_NA);
     tds_tree = proto_item_add_subtree(tds_item, ett_tds);
+
+    type = tvb_get_guint8(tvb, offset);
     proto_tree_add_item(tds_tree, hf_tds_type, tvb, offset, 1, ENC_LITTLE_ENDIAN);
-    tds_item = proto_tree_add_item(tds_tree, hf_tds_status, tvb, offset + 1, 1, ENC_LITTLE_ENDIAN);
-    tds_status_tree = proto_item_add_subtree(tds_item, ett_tds_status);
-    proto_tree_add_item(tds_status_tree, hf_tds_status_eom, tvb, offset + 1, 1, ENC_BIG_ENDIAN);
-    proto_tree_add_item(tds_status_tree, hf_tds_status_ignore, tvb, offset + 1, 1, ENC_BIG_ENDIAN);
-    proto_tree_add_item(tds_status_tree, hf_tds_status_event_notif, tvb, offset + 1, 1, ENC_BIG_ENDIAN);
-    proto_tree_add_item(tds_status_tree, hf_tds_status_reset_conn, tvb, offset + 1, 1, ENC_BIG_ENDIAN);
-    proto_tree_add_item(tds_status_tree, hf_tds_status_reset_conn_skip_tran,tvb, offset + 1, 1, ENC_BIG_ENDIAN);
+
+    status = tvb_get_guint8(tvb, offset + 1);
+    proto_tree_add_bitmask(tds_tree, tvb, offset+1, hf_tds_status, ett_tds_status, status_flags, ENC_LITTLE_ENDIAN);
     proto_tree_add_item(tds_tree, hf_tds_length, tvb, offset + 2, 2, ENC_BIG_ENDIAN);
+    channel = tvb_get_ntohs(tvb, offset + 4);
     proto_tree_add_item(tds_tree, hf_tds_channel, tvb, offset + 4, 2, ENC_BIG_ENDIAN);
+    packet_number = tvb_get_guint8(tvb, offset + 6);
     proto_tree_add_item(tds_tree, hf_tds_packet_number, tvb, offset + 6, 1, ENC_LITTLE_ENDIAN);
     proto_tree_add_item(tds_tree, hf_tds_window, tvb, offset + 7, 1, ENC_LITTLE_ENDIAN);
+
     offset += 8;        /* skip Netlib header */
 
     /*
@@ -3945,165 +3947,45 @@ dissect_netlib_buffer(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 }
 
 static int
-dissect_tds_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void * data _U_)
+dissect_tds_pdu(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_)
 {
-    volatile gboolean first_time = TRUE;
-    volatile int offset = 0;
-    guint length_remaining;
-    guint8 type;
-    volatile guint16 plen;
-    guint length;
-    tvbuff_t *volatile next_tvb;
-    proto_item *tds_item = NULL;
-    proto_tree *tds_tree = NULL;
+    guint32 type;
 
-    while ((length_remaining = tvb_reported_length_remaining(tvb, offset)) > 0) {
+    col_set_str(pinfo->cinfo, COL_PROTOCOL, "TDS");
+    col_clear(pinfo->cinfo, COL_INFO);
 
-        /*
-         * Can we do reassembly?
-         */
-        if (tds_desegment && pinfo->can_desegment) {
-            /*
-             * Yes - is the fixed-length part of the PDU
-             * split across segment boundaries?
-             */
-            if (length_remaining < 8) {
-                /*
-                 * Yes.  Tell the TCP dissector where the data for this message
-                 * starts in the data it handed us and that we need "some more
-                 * data."  Don't tell it exactly how many bytes we need because
-                 * if/when we ask for even more (after the header) that will
-                 * break reassembly.
-                 */
-                pinfo->desegment_offset = offset;
-                pinfo->desegment_len = DESEGMENT_ONE_MORE_SEGMENT;
-                return tvb_captured_length(tvb);
-            }
-        }
-
-        type = tvb_get_guint8(tvb, offset);
-
-        /* Special test for TLS to that we don't have lots of incorrect reports of malformed packets */
-        if(type == TDS_TLS_PKT)
-        {
-            plen = tvb_get_ntohs(tvb, offset + 3) + 5;
-        } else
-            plen = tvb_get_ntohs(tvb, offset + 2);
-
-        if (plen < 8) {
-            /*
-             * The length is less than the header length.
-             * Put in the type, status, and length, and
-             * report the length as bogus.
-             */
-            if (tree) {
-                /* create display subtree for the protocol */
-                tds_item = proto_tree_add_item(tree, proto_tds,
-                                               tvb, offset, -1, ENC_NA);
-
-                tds_tree = proto_item_add_subtree(tds_item,
-                                                  ett_tds);
-                proto_tree_add_uint(tds_tree, hf_tds_type, tvb,
-                                    offset, 1, type);
-
-                if(type != TDS_TLS_PKT)
-                {
-                    proto_tree_add_item(tds_tree, hf_tds_status,
-                                    tvb, offset + 1, 1, ENC_BIG_ENDIAN);
-                    tds_item = proto_tree_add_uint(tds_tree, hf_tds_length, tvb, offset + 2, 2, plen);
-                    expert_add_info_format(pinfo, tds_item, &ei_tds_invalid_length, "bogus, should be >= 8");
-                }
-            }
+    type = tvb_get_guint8(tvb, 0);
+    col_append_sep_fstr(pinfo->cinfo, COL_INFO, ",", "%s", val_to_str(type, packet_type_names, "Unknown Packet Type: %u"));
 
-            /*
-             * Give up - we can't dissect any more of this
-             * data.
-             */
-            break;
-        }
+    dissect_netlib_buffer(tvb, pinfo, tree);
 
-        /*
-         * Can we do reassembly?
-         */
-        if (tds_desegment && pinfo->can_desegment) {
-            /*
-             * Yes - is the PDU split across segment boundaries?
-             */
-            if (length_remaining < plen) {
-                /*
-                 * Yes.  Tell the TCP dissector where the
-                 * data for this message starts in the data
-                 * it handed us, and how many more bytes we
-                 * need, and return.
-                 */
-                pinfo->desegment_offset = offset;
-                pinfo->desegment_len = plen - length_remaining;
-                return tvb_captured_length(tvb);
-            }
-        }
+    col_set_fence(pinfo->cinfo, COL_INFO);
 
-        if (first_time) {
-            col_set_str(pinfo->cinfo, COL_PROTOCOL, "TDS");
-
-            /*
-             * Set the packet description based on its TDS packet
-             * type.
-             */
-            col_add_str(pinfo->cinfo, COL_INFO,
-                        val_to_str(type, packet_type_names,
-                                   "Unknown Packet Type: %u"));
-            first_time = FALSE;
-        }
+    return tvb_captured_length(tvb);
+}
 
-        /*
-         * Construct a tvbuff containing the amount of the payload
-         * we have available.  Make its reported length the amount
-         * of data in the PDU.
-         *
-         * XXX - if reassembly isn't enabled. the subdissector will
-         * throw a BoundsError exception, rather than a
-         * ReportedBoundsError exception.  We really want a tvbuff
-         * where the length is "length", the reported length is
-         * "plen", and the "if the snapshot length were infinite"
-         * length is the minimum of the reported length of the tvbuff
-         * handed to us and "plen", with a new type of exception
-         * thrown if the offset is within the reported length but
-         * beyond that third length, with that exception getting the
-         * "Unreassembled Packet" error.
-         */
-        length = length_remaining;
-        if (length > plen)
-            length = plen;
-        next_tvb = tvb_new_subset_length_caplen(tvb, offset, length, plen);
+static guint
+get_tds_pdu_len(packet_info *pinfo _U_, tvbuff_t *tvb, int offset, void *data _U_)
+{
+    guint16 plen;
+    guint8 type;
 
-        /*
-         * Dissect the Netlib buffer.
-         *
-         * If it gets an error that means there's no point in
-         * dissecting any more Netlib buffers, rethrow the
-         * exception in question.
-         *
-         * If it gets any other error, report it and continue, as that
-         * means that Netlib buffer got an error, but that doesn't mean
-         * we should stop dissecting Netlib buffers within this frame
-         * or chunk of reassembled data.
-         */
-        TRY {
-            dissect_netlib_buffer(next_tvb, pinfo, tree);
-        }
-        CATCH_NONFATAL_ERRORS {
+    type = tvb_get_guint8(tvb, offset);
 
-            show_exception(tvb, pinfo, tree, EXCEPT_CODE, GET_MESSAGE);
-        }
-        ENDTRY;
+    /* Special test for TLS to that we don't have lots of incorrect reports of malformed packets */
+    if(type == TDS_TLS_PKT)
+        plen = tvb_get_ntohs(tvb, offset + 3) + 5;
+    else
+        plen = tvb_get_ntohs(tvb, offset + 2);
 
-        /*
-         * Step to the next Netlib buffer.
-         */
-        offset += plen;
-    }
+   return plen;
+}
 
-    return tvb_captured_length(tvb);
+static int
+dissect_tds(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void * data _U_)
+{
+   tcp_dissect_pdus(tvb, pinfo, tree, tds_desegment, 8, get_tds_pdu_len, dissect_tds_pdu, data);
+   return tvb_captured_length(tvb);
 }
 
 static gboolean
@@ -4113,76 +3995,52 @@ dissect_tds_tcp_heur(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *
     guint8 type;
     guint8 status;
     guint16 plen;
-    conversation_t *conv;
 
     /*
      * If we don't have even enough data for a Netlib header,
      * just say it's not TDS.
      */
-    if (tvb_captured_length(tvb) < 8)
+    if (tvb_reported_length(tvb) < 8)
         return FALSE;
 
     /*
      * Quickly scan all the data we have in order to see if
      * everything in it looks like Netlib traffic.
      */
-    while (tvb_bytes_exist(tvb, offset, 1)) {
-        /*
-         * Check the type field.
-         */
-        type = tvb_get_guint8(tvb, offset);
-        if (!is_valid_tds_type(type))
-            return FALSE;
 
-        /*
-         * Check the status field, if it's present.
-         */
-        if (!tvb_bytes_exist(tvb, offset + 1, 1))
-            break;
-        status = tvb_get_guint8(tvb, offset + 1);
-        if (!is_valid_tds_status(status))
-            return FALSE;
-
-        /*
-         * Get the length of the PDU.
-         */
-        if (!tvb_bytes_exist(tvb, offset + 2, 2))
-            break;
-        plen = tvb_get_ntohs(tvb, offset + 2);
-        if (plen < 8) {
-            /*
-             * The length is less than the header length.
-             * That's bogus.
-             */
-            return FALSE;
-        }
+    /*
+     * Check the type field.
+     */
+    type = tvb_get_guint8(tvb, offset);
+    if (!is_valid_tds_type(type))
+        return FALSE;
 
-        /*
-         * If we're at the beginning of the segment, check the
-         * payload if it's a login packet.
-         */
-        if (offset == 0) {
-            if (!netlib_check_login_pkt(tvb, offset, pinfo, type))
-                return FALSE;
-        }
+    /*
+     * Check the status field
+     */
+    status = tvb_get_guint8(tvb, offset + 1);
+    if (!is_valid_tds_status(status))
+        return FALSE;
 
+    /*
+     * Get the length of the PDU.
+     */
+    plen = tvb_get_ntohs(tvb, offset + 2);
+    if (plen < 8) {
         /*
-         * Step to the next Netlib buffer.
+         * The length is less than the header length.
+         * That's bogus.
          */
-        offset += plen;
+        return FALSE;
     }
 
-    /*
-     * OK, it passes the test; assume the rest of this conversation
-     * is TDS.
-     */
-    conv = find_or_create_conversation(pinfo);
-    conversation_set_dissector(conv, tds_tcp_handle);
+    if (!netlib_check_login_pkt(tvb, offset, pinfo, type))
+        return FALSE;
 
     /*
      * Now dissect it as TDS.
      */
-    dissect_tds_message(tvb, pinfo, tree, data);
+    dissect_tds(tvb, pinfo, tree, data);
 
     return TRUE;
 }
@@ -5579,7 +5437,7 @@ proto_register_tds(void)
     expert_register_field_array(expert_tds, ei, array_length(ei));
 
 /* Allow dissector to be found by name. */
-    tds_tcp_handle = register_dissector("tds", dissect_tds_message, proto_tds);
+    tds_tcp_handle = register_dissector("tds", dissect_tds, proto_tds);
 
     tds_module = prefs_register_protocol(proto_tds, apply_tds_prefs);
     prefs_register_bool_preference(tds_module, "desegment_buffers",