google / google/gopacket

reassembly: when ReassemblyComplete returns true, final ACK re-creates connection again

Open
#427 2 comments 1 reaction 0 assignees View on GitHub
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

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.