Simplify overlap handling

elastic · Jun 27, 2016 · c44d735 · c44d735
1 parent f42aa72
commit c44d735
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 51 deletions.
diff --git 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
@@ -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},