Re: ZigBee APS re-assemble with re-used sequence number

[Kenneth Soerensen <knnthsrnsn () gmail com>]

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 () gmail com>:

> 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 () xs4all nl> 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 () gmail com> 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 () wireshark org>
> > 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 () wireshark org>
> 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 () wireshark org>
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