Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] rev 37826: /trunk/epan/dissectors/ /trun
From: Guy Harris <guy@xxxxxxxxxxxx>
Date: Wed, 29 Jun 2011 10:31:37 -0700
On Jun 29, 2011, at 1:20 AM, Stig Bjørlykke wrote:

> On Wed, Jun 29, 2011 at 9:47 AM, Guy Harris <guy@xxxxxxxxxxxx> wrote:
>> 
> 
>> Well, it fixes the problem with that *particular* capture.
> 
> Sure, but it does the exact check as in dissect_rpcap_packet() and
> discards the message before it's recognized as RPCAP.

...even if the packet *is* valid but cut short by the snaplen.

>> The underlying problem is that proto_tree_add_item() might not end up doing anything that checks the validity of the offset, so if nitems in dissect_rpcap_filter() has an absurdly large value, the loop in dissect_rpcap_filter() can go well past the end of the packet and only stop when it's put "too many" items into the protocol tree - unfortunately, "too many" is large enough that this could take a while.
> 
> Then we also want to add a sanity check for nitems, to avoid a
> absurdly large loop.

That's one way of handling it, but if the dissection in the loop always checks whether the stuff to be dissected is available, even if TRY_TO_FAKE_THIS_ITEM isn't putting anything into the protocol tree, the loop will throw an exception as soon as it goes past the end of the packet.

>> OK, so what should dissect_rpcap_packet() return in the case where the caplen value is bogus?
> 
> 0, as in nothing decoded when registered as a "new" dissector, as the
> very first version (from revision 26633) of the dissector.

dissect_rpcap_packet() isn't the dissector, dissect_rpcap() is, and, by the time it gets to dissect_rpcap_packet(), it should already have concluded that it's an rpcap packet.

> I wanted to keep the offsets so we can use the dissector as "new" again.

The "old" vs. "new" vs. heuristic thing needs work.

The problem is that the "new" dissectors mix up two things that shouldn't be mixed up:

	1) "is this a packet for this protocol or not";

	2) "how much data in the tvbuff I was handed belongs to this packet".

The problem is that there are some cases where a packet for the protocol could have zero bytes of data and still have stuff to put into the protocol tree.  Theose cases are, as I remember, request/response protocols where:

	1) the request type is in the request but not the response;

	2) the field used to match requests and responses are in the protocol atop which the request/response protocol runs;

so a reply to a request might be "empty" but still need to be dissected by, if nothing else, looking up the field in question and showing what type of request this is a reply to, and perhaps also showing the frame number of the matching request, service response time, etc..

A dissector for that protocol couldn't be a "new"-style protocol, as it would have to return 0 for packets that it dissected.

Perhaps if we had a type of dissector that used a ptvcursor, the ptvcursor could be used to keep track of how much the dissector actually dissects, and the dissector could return TRUE or FALSE based on whether it accepts the packet or not.  We might be able to convert all "old"-style dissectors and all "new"-style dissectors to the "new new" style; heuristic dissectors could also be "new new" style.

> I have a long term project, which depends on whether libpcap will
> support rpcap :)

Hopefully the project doesn't depend on whether rpcap is the *only* scheme for remote packet capture that libpcap supports.