[Kenneth Soerensen <knnthsrnsn@xxxxxxxxx>]

Hi Jaap and John

I don't think ZigBee knowledge is required here and I guess this is a general issue for protocols with short sequence numbers.

My concern with your suggestion is that we will need to maintain a sliding window and perform rollover detection for each node pair and direction (A ZigBee network has multiple nodes communicating in both directions).

Currently, the APS dissector is using the generic fragment_add_seq_check().

I have looked at fragment_add_seq_single_aging(), which seems to handle something like this. However, this is for protocols with a single sequence number (as John mentioned). Furthermore, the aging is based on frame numbers, which will not work for ZigBee because the number of frames between APS packets with the same sequence number will depend heavily on the amount of other traffic in the network.

I have considered to create a function which uses timestamps for aging instead. A fragmented APS packet is considered to be completely transferred within a reasonable time compared to the time between sequence number rollover. But then again the _fragment_item struct used by the re-assembler does not store packet timestamps.

The first fragment in an APS packet has a special attribute so I have also considered to simply re-start the reassembly for that sequence number when a first fragment is dissected.

/Kenneth

2018-08-02 21:13 GMT+02:00 John Thacker <johnthacker@xxxxxxxxx>:

I had to deal with this problem when I added support for protocols like PPP MP with a single sequence number that increments each fragment instead of separate sequence numbers and fragment numbers. (See Appendix A of RFC 4623 (PWE3 Fragmentation and Reassembly) for a list of protocols that use this style, including PPP MP (RFC 1990), PWE3 MPLS (RFC 4385), L2TPv2 (RFC 2661), L2TPv3 (RFC 3931), ATM, and Frame Relay.)

The problem happened in some long captures I had with some dropped packets and where the "short sequence number" variant was used for MLPPP, which is 12 bits (so a little bit bigger than the 8 bits with ZigBee, but not having separate fragment sequence numbers effectively costs a couple bits.) See in particular fragment_add_seq_single_aging(), and all the fragment_add_seq_single_* functions in general. My approach, which may not have been optimal, used a REASSEMBLE_FLAGS_AGING flag and a max_age parameter to discard partially reassembled fragments. When wireshark gets a new fragment and looks in the table of partially reassembled fragments to add it to, if the partially reassembled fragment comes from a frame number that is very old, that partially reassembled fragment is discarded. I ended up using the previously unused frame field of the partially reassembled fragment head for this (fh->frame), storing the frame number of most recently added fragment in this as well.

Take a look at those functions, as it may provide some help for your approach. It worked for me on those files that I had.

John Thacker

On Thu, Aug 2, 2018 at 2:04 PM Jaap Keuter <jaap.keuter@xxxxxxxxx> wrote:

Hi,

Not burdened by any ZigBee domain knowledge I would say that a seq# rollover would require a clearing of the non-reassembled fragments. But not all of them because we could still be in the process of reassembling the part of the stream with the not-yet rolled over seq#. A sliding window of non-reassembled fragments, of about half the seq# range, moved forward by the next received seq#, could be sufficient. All in all this would be an extension of the generic reassembly routenes, assuming they are used...

Thanks,

Jaap

On 2 Aug 2018, at 12:17, Kenneth Soerensen <knnthsrnsn@xxxxxxxxx> wrote:

Hi

I have filled bug https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=15021

Any idea how we can fix this?

The packet re-assembler is confused by ZigBee APS re-using sequence numbers, which makes it hard to distinguish what fragments belong to specific re-assembled packets.

/Kenneth

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request@wireshark.org?subject=unsubscribe

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request@wireshark.org?subject=unsubscribe