Wireshark mailing list archives
Re: Use of tcp_dissect_pdus() with a protocol which has a variable length "PDU length" field
From: Peter Wu <peter () lekensteyn nl>
Date: Fri, 20 Feb 2015 19:55:33 +0100
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-request () wireshark org?subject=unsubscribe
Current thread:
- Re: Use of tcp_dissect_pdus() with a protocol which has a variable length "PDU length" field Peter Wu (Feb 20)