From c44d73587f99532f44791b637234826e2ef62193 Mon Sep 17 00:00:00 2001 From: urso Date: Wed, 22 Jun 2016 17:30:41 +0200 Subject: [PATCH] Simplify overlap handling --- packetbeat/protos/tcp/tcp.go | 69 ++++++++++++++++++++----------- packetbeat/protos/tcp/tcp_test.go | 52 +++++++++++------------ 2 files changed, 70 insertions(+), 51 deletions(-) diff --git a/packetbeat/protos/tcp/tcp.go b/packetbeat/protos/tcp/tcp.go index 033ab068377..7f912173358 100644 --- a/packetbeat/protos/tcp/tcp.go +++ b/packetbeat/protos/tcp/tcp.go @@ -31,6 +31,14 @@ type Processor interface { Process(flow *flows.FlowID, hdr *layers.TCP, pkt *protos.Packet) } +type seqCompare int + +const ( + seqLT seqCompare = -1 + seqEq seqCompare = 0 + seqGT seqCompare = 1 +) + var ( debugf = logp.MakeDebug("tcp") isDebug = false @@ -126,7 +134,7 @@ func (tcp *Tcp) Process(id *flows.FlowID, tcphdr *layers.TCP, pkt *protos.Packet conn := stream.conn if id != nil { - id.AddConnectionID(uint64(stream.conn.id)) + id.AddConnectionID(uint64(conn.id)) } if isDebug { @@ -147,7 +155,7 @@ func (tcp *Tcp) Process(id *flows.FlowID, tcphdr *layers.TCP, pkt *protos.Packet tcpStartSeq, tcpSeq, lastSeq, len(pkt.Payload)) } - if lastSeq != 0 { + if len(pkt.Payload) > 0 && lastSeq != 0 { if tcpSeqBeforeEq(tcpSeq, lastSeq) { if isDebug { debugf("Ignoring retransmitted segment. pkt.seq=%v len=%v stream.seq=%v", @@ -156,38 +164,37 @@ func (tcp *Tcp) Process(id *flows.FlowID, tcphdr *layers.TCP, pkt *protos.Packet return } - if tcpSeqBefore(lastSeq, tcpStartSeq) { - if !created { - gap := int(tcpStartSeq - lastSeq) - logp.Warn("Gap in tcp stream. last_seq: %d, seq: %d, gap: %d", lastSeq, tcpStartSeq, gap) - drop := stream.gapInStream(gap) - if drop { - if isDebug { - debugf("Dropping connection state because of gap") - } - - // drop application layer connection state and - // update stream_id for app layer analysers using stream_id for lookups - conn.id = tcp.getId() - conn.data = nil + switch tcpSeqCompare(lastSeq, tcpStartSeq) { + case seqLT: // lastSeq < tcpStartSeq => Gap in tcp stream detected + if created { + break + } + + gap := int(tcpStartSeq - lastSeq) + logp.Warn("Gap in tcp stream. last_seq: %d, seq: %d, gap: %d", lastSeq, tcpStartSeq, gap) + drop := stream.gapInStream(gap) + if drop { + if isDebug { + debugf("Dropping connection state because of gap") } + + // drop application layer connection state and + // update stream_id for app layer analysers using stream_id for lookups + conn.id = tcp.getId() + conn.data = nil } - } - // cut away packet overlap - if tcpSeqBefore(tcpStartSeq, lastSeq) { + case seqGT: + // lastSeq > tcpStartSeq => overlapping TCP segment detected. shrink packet delta := lastSeq - tcpStartSeq - // if 'overlap' already covered by previous packet -> return - if int(delta) >= len(pkt.Payload) { - return + if isDebug { + debugf("Overlapping tcp segment. last_seq %d, seq: %d, delta: %d", + lastSeq, tcpStartSeq, delta) } pkt.Payload = pkt.Payload[delta:] tcphdr.Seq += delta - if len(pkt.Payload) == 0 && !tcphdr.FIN { - return - } } } @@ -233,6 +240,18 @@ func (tcp *Tcp) getStream(pkt *protos.Packet) (stream TcpStream, created bool) { return TcpStream{conn: conn, dir: TcpDirectionOriginal}, true } +func tcpSeqCompare(seq1, seq2 uint32) seqCompare { + i := int32(seq1 - seq2) + switch { + case i == 0: + return seqEq + case i < 0: + return seqLT + default: + return seqGT + } +} + func tcpSeqBefore(seq1 uint32, seq2 uint32) bool { return int32(seq1-seq2) < 0 } diff --git a/packetbeat/protos/tcp/tcp_test.go b/packetbeat/protos/tcp/tcp_test.go index c173bdcfbcf..7bd0960dbb8 100644 --- a/packetbeat/protos/tcp/tcp_test.go +++ b/packetbeat/protos/tcp/tcp_test.go @@ -199,69 +199,69 @@ func TestTCSeqPayload(t *testing.T) { }{ {"No overlap", []segment{ - {0, []byte{1, 2, 3, 4, 5}}, - {5, []byte{6, 7, 8, 9, 10}}, + {1, []byte{1, 2, 3, 4, 5}}, + {6, []byte{6, 7, 8, 9, 10}}, }, 0, []byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, }, {"Gap drop state", []segment{ - {0, []byte{1, 2, 3, 4}}, - {14, []byte{5, 6, 7, 8}}, + {1, []byte{1, 2, 3, 4}}, + {15, []byte{5, 6, 7, 8}}, }, 10, []byte{5, 6, 7, 8}, }, {"ACK same sequence number", []segment{ - {0, []byte{1, 2}}, - {2, nil}, - {2, []byte{3, 4}}, - {4, []byte{5, 6}}, + {1, []byte{1, 2}}, + {3, nil}, + {3, []byte{3, 4}}, + {5, []byte{5, 6}}, }, 0, []byte{1, 2, 3, 4, 5, 6}, }, {"ACK same sequence number 2", []segment{ - {0, nil}, {1, nil}, - {1, []byte{1, 2}}, - {3, nil}, - {3, []byte{3, 4}}, - {5, []byte{5, 6}}, - {7, []byte{7, 8}}, - {9, nil}, + {2, nil}, + {2, []byte{1, 2}}, + {4, nil}, + {4, []byte{3, 4}}, + {6, []byte{5, 6}}, + {8, []byte{7, 8}}, + {10, nil}, }, 0, []byte{1, 2, 3, 4, 5, 6, 7, 8}, }, {"Overlap, first segment bigger", []segment{ - {0, []byte{1, 2}}, - {2, []byte{3, 4}}, - {2, []byte{3}}, - {4, []byte{5, 6}}, + {1, []byte{1, 2}}, + {3, []byte{3, 4}}, + {3, []byte{3}}, + {5, []byte{5, 6}}, }, 0, []byte{1, 2, 3, 4, 5, 6}, }, {"Overlap, second segment bigger", []segment{ - {0, []byte{1, 2}}, - {2, []byte{3}}, - {2, []byte{3, 4}}, - {4, []byte{5, 6}}, + {1, []byte{1, 2}}, + {3, []byte{3}}, + {3, []byte{3, 4}}, + {5, []byte{5, 6}}, }, 0, []byte{1, 2, 3, 4, 5, 6}, }, {"Overlap, covered", []segment{ - {0, []byte{1, 2, 3, 4}}, - {1, []byte{2, 3}}, - {4, []byte{5, 6}}, + {1, []byte{1, 2, 3, 4}}, + {2, []byte{2, 3}}, + {5, []byte{5, 6}}, }, 0, []byte{1, 2, 3, 4, 5, 6},