google / google/gopacket

tcpassembly reorder bug

Open
#81 8 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
6.8k
Forks
1.2k
PR merge metrics
No merged PRs in 30d

Description

Dear Graeme Connell,

There are a few problems with the TCP "reordering" of tcpassembly:
- If the Assembler receives duplicate out of order packets then it will emit an extra Reassembly struct into the stream. The extra Reassembly struct will contain an empty Bytes payload. This is not desired behavior I think.
- The Assembler does not ignore coalesce injection packets; they corrupt the reassembly stream.
- The Assembler does not emulate the reordering properties of Selective Acknowledgement. That is, if two out of order packet (with sequence numbers that come after the TCP FSM's "next Sequence") with the same sequence number are received... the second packet's payload is coalesced into the stream whereas in implementations of Selective Acknowledgement the first packet wins. (if my understanding of Selective Ackowledgement is correct)

https://github.com/david415/gopacket/tree/test-reorder
https://github.com/david415/gopacket/commit/89677d3b4ea713a753d343e06b060a24a4c01a89
AND
https://github.com/david415/gopacket/commit/ca61db8ba85cb6bbb1c1d742fb08f9932df48ff6

I've already fixed these problems in Honeybadger... here:
https://github.com/david415/HoneyBadger/blob/51-fix-retrospective-1/ordered_coalesce.go#L315-L325

...that is, I fixed that code snippet and it was inspired by your `addNextFromConn`

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.