reassembly: when ReassemblyComplete returns true, final ACK re-creates connection again
- Dominant language
- Go
- Stars
- 6.8k
- Forks
- 1.2k
- PR merge metrics
- No merged PRs in 30d
Description
The documentation states:
```
// It should return true if the connection should be removed from the pool
// It can return false if it want to see subsequent packets with Accept(), e.g. to
// see FIN-ACK, for deeper state-machine analysis.
```
Somewhat ambiguous comment in the reassemblydump tells the following:
https://github.com/google/gopacket/blob/b40365fca6ccb41d3892c4caa0f6e7545ee2fdcf/examples/reassemblydump/main.go#L451-L458
What it actually means in practice, is that when connection is closed with e.g. `[FIN,ACK]`, `[ACK]`, `[FIN,ACK]`, `[ACK]`, `ReassemblyComplete` is called on the second `[FIN,ACK]` packet. If `ReassemblyComplete` returns true, connection will be removed from the pool. The last `[ACK]` will actually recreate connection again, which will become stuck in the pool forever (or until it will be flushed with `FlushWithOptions`).
In other words, it's kinda lose-lose scenario. No matter what you'll return from `ReassemblyComplete`, you'll either have original connection stuck in the pool forever, or final ACK recreating connection which will be stuck forever anyway.
I noticed it when analysing some dumps. Running `StreamPool.Dump` at some point gave me some connections with noticeably different activity times:
```
[10.66.85.2->10.65.85.1 4242->52538] c2s: created:2015-12-18 19:36:34.654147 +0000 UTC, last:2015-12-18 19:36:34.654147 +0000 UTC, s2c: created:2015-12-18 19:36:34.654147 +0000 UTC, last:2015-12-18 19:36:34.654147 +0000 UTC
[10.65.85.1->10.66.85.2 40927->4242] c2s: created:2015-12-18 19:50:22.275428 +0000 UTC, last:2015-12-18 19:50:22.275428 +0000 UTC, s2c: created:2015-12-18 19:50:22.275428 +0000 UTC, last:2015-12-18 19:50:22.275428 +0000 UTC
```
Notice the first connection has the same activity time in all fields. It's actually the timestamp of that last ACK packet.
Contributor guide
Assessment
This issue has not been assessed yet.