Wireshark · Wireshark-dev: [Wireshark-dev] Use of tcp_dissect_pdus() with a protocol which has a variable length "PDU length" f

[Bill Meier <wmeier@xxxxxxxxxxx>]

To: TCP re-assembly experts:


The MQTT protocol dissected by packet-mqtt.c runs over TCP. The field 
which specifies the MQTT PDU length can be 1 to 4 bytes; the length of a 
complete MQTT PDU can be less than 4 bytes.

So: trying use tcp_dissect_pdus() won't work since the "fixed length"
needed to handle the maximum size of the "PDU length" field is larger 
than the possible minimum PDU size.

One approach is to assume that the complete length field will, with high 
probability, always be in 1 TCP segment and thus available even if 
specifying a 'fixed length' which includes only a 1 byte PDU length 
field.  (This is certainly not guaranteed).

Or, obviously, the dissector could do reassembly as described in 
README.dissector section 2.7.2 "Modifying the pinfo struct".

However, digging a little deeper, I note that tcp_dissect_pdus() is 
doing something related to "want_pdu_tracking" (which I've never delved 
into and which is not mentioned in README.dissector).

So it occurred to me that using a modified tcp_dissect_pdus() which 
allows the get_pdu_len function to return DESEGMENT_ONE_MORE_SEGMENT 
might be a workable approach.

So: I added the following to tcp_dissect_pdus() and modified
the packet-mqtt.c get_pdu_len() function appropriately.

(added code in tcp_dissect_pdus):

         plen = (*get_pdu_len)(pinfo, tvb, offset);
+
+        /* Is "more data" being requested before the PDU length can be
            determined ?
+         *  If so, request same.
+         */
+        if (plen == DESEGMENT_ONE_MORE_SEGMENT) {
+            if (!proto_desegment || !pinfo->can_desegment) {
+                REPORT_DISSECTOR_BUG("DESEGMENT_ONE_MORE_SEGMENT not 
allowed");
+            }
+            pinfo->desegment_offset = offset;
+            pinfo->desegment_len = DESEGMENT_ONE_MORE_SEGMENT;
+            return;
+        }
+
         if (plen < fixed_len) {


My questions:

1. Is this a reasonable approach (it works AOK in my tests) ?

2. If not, and packet-mqtt should do reassembly itself, does the 
reassembly code also need to do the "want_pdu_tracking" stuff ?

Bill