Re: [Wireshark-commits] rev 37826: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-rpcap.c

[Guy Harris <guy () alum mit edu>]

On Jun 29, 2011, at 1:20 AM, Stig Bjørlykke wrote:

> On Wed, Jun 29, 2011 at 9:47 AM, Guy Harris <guy () alum mit edu> 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.
___________________________________________________________________________
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