Hi,
While looking at improving the Websocket dissector, I found the need to
support variable length fields in tcp_dissect_pdus. Here is Bills
original mail (which had no replies).
On Fri, May 09, 2014 at 11:02:45AM -0400, Bill Meier wrote:
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) ?
This approach looks fine to me. I have taken the same approach (but
replacing REPORT_DISSECTOR_BUG by DISSECTOR_ASSERT in
https://code.wireshark.org/review/7279
2. If not, and packet-mqtt should do reassembly itself, does the reassembly
code also need to do the want_pdu_tracking stuff ?
Bill
Looking at the current implementation of tcp_dissect_pdus, it is not
necessary to change want_pdu_tracking as the size of the new PDU is not
yet known.
Since this is an interesting API update, I thought that informing the
list would be a good idea.
Kind regards,
Peter
___
Sent via:Wireshark-dev mailing list wireshark-dev@wireshark.org
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
