Re: TCP reassembly and Return value of a new-style dissector

[Anders Broman <anders.broman () ericsson com>]

-----Original Message-----
From: wireshark-dev-bounces () wireshark org [mailto:wireshark-dev-bounces () wireshark org] On Behalf Of Peter Wu
Sent: den 10 december 2014 17:17
To: wireshark-dev () wireshark org
Subject: Re: [Wireshark-dev] TCP reassembly and Return value of a new-style dissector

On Tuesday 09 December 2014 21:01:37 Anders Broman wrote:
> > Hi,
> > I have recently come across some problems with reassembly of SIP 
> > messages over TCP. One problem seems to be related to when a segment 
> > contains one full PDU and a segment of the next (following) PDU (in 
> > this case the first SIP line of the following PDU) is not complete.
>
> (added punctuation for easier reading)
>
> HTTP does not seem to have problems with it, probably because it tries to consume as much of a PDU as possible, saving 
> incomplete data for later desegmentation. See the attached crafted packet for example >which contains three HTTP 
> responses:
>
> 1. PDU 1 contains one full HTTP response and the begin of the next.
> 2. PDU 2 contains all the remainder of the second HTTP response, minus
>    one last character.
> 3. PDU 3 has the last character of the second request and the begin of
>    the third request .
> 4. PDU 4 contains the remainder of the third HTTP request.
>
> 'HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\nContent-Length: 1\r\n\r\n1'
> 'HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\n', 'Content-Length: 2\r\n\r\n1', '2'
> 'HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\nC', 'ontent-Length: 3\r\n\r\n123'

> > I think the ultimate solution would be for the TCP dissector to call 
> > the SIP dissector again with the next incomplete PDU after receiving 
> > the number of bytes "accepted" by the SIP dissector e.g using the 
> > "new-style dissector interface.
> > also see http://seclists.org/wireshark/2014/Jun/289

> It would indeed be nice if the dissector core could be improved to handle this situation. As I mentioned before, the 
> HTTP dissector has no problems with this because it tries to dissect all data until more data is >needed.

> This is from dissect_sip:
>
>    remaining_length = tvb_reported_length(tvb);
>    len = dissect_sip_common(tvb, 0, remaining_length, pinfo, tree, FALSE, FALSE);
>    if (len < 0)
>        return 0;   /* not SIP */
>    else
>        return len;

> dissect_http instead always returns tvb_captured_length ("all data in the PDU is part of the HTTP protocol") while 
> returning 0 means "I know for sure that this is not SIP".
>
> dissect_sip_common returns -1 meaning "need more data", but then dissect_sip return 0 meaning "no, not SIP, try a 
> different dissector!".

I'm not sure the TCP dissector cares about the return value I think it only looks at pinfo. 

I think part of my problem is that the SIP dissector mixes the heuristics and 'common' functionality. I'm actually 
looking at SIP over TCP currently, e.g dissect_sip_tcp(). I have also run into a problem where reassembly fails
When there is duplicated packets but works if the dupes are marked as ignored... ( dupes due to the way mirroring is 
set up).

> As I read the code the first step would be to have
> call_dissector()                                                [OK]
> try_conversation_dissector()
> dissector_try_heuristic()
> dissector_try_uint_new                                 [OK]
>
> Return the number of bytes consumed, 0 or -1(need more data) not sure 
> about DESEGMENT_UNTIL_FIN (-2?).
>
> If people agree the biggest change is to change
> dissector_try_heuristic() to return an int.
> What do you think?

I would not particularly mind changing the return value, but what should be the new semantics of the return value? I 
found that documentation is quite lacking here. When I tried to make the core to actual handle return values[1], many 
dissectors broke because they were not written with the documented behavior (see also the mail you linked before).

Here is a document I wrote back then to clarify it for myself 
https://git.lekensteyn.nl/peter/wireshark-notes/tree/doc/dissector.txt
--
Kind regards,
Peter
https://lekensteyn.nl

 [1]: https://git.lekensteyn.nl/peter/wireshark/commit/?h=reassembly-fixes&id=aef08cc2434b5ba5aee4422fbcf481004c62583a
___________________________________________________________________________
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