diff --git a/epan/dissectors/packet-tcp.c b/epan/dissectors/packet-tcp.c index 9319f348b4..9de7bb8bd7 100644 --- a/epan/dissectors/packet-tcp.c +++ b/epan/dissectors/packet-tcp.c @@ -1733,6 +1733,7 @@ pdu_store_sequencenumber_of_next_pdu(packet_info *pinfo, guint32 seq, guint32 nx msp->nxtpdu=nxtpdu; msp->seq=seq; msp->first_frame=pinfo->num; + msp->first_frame_with_seq=pinfo->num; msp->last_frame=pinfo->num; msp->last_frame_time=pinfo->abs_ts; msp->flags=0; @@ -3065,6 +3066,7 @@ again: if ((msp = (struct tcp_multisegment_pdu *)wmem_tree_lookup32(tcpd->fwd->multisegment_pdus, seq)) && !(msp->flags & MSP_FLAGS_MISSING_FIRST_SEGMENT) && msp->last_frame != pinfo->num) { const char* str; + gboolean is_retransmission = FALSE; /* Yes. This could be because we've dissected this frame before * or because this is a retransmission of a previously-seen @@ -3074,24 +3076,35 @@ again: * find this retransmission instead of the original transmission * (breaking desegmentation if we'd already linked other segments * to the original transmission's entry). + * + * Cases to handle here: + * - In-order stream, pinfo->num matches begin of MSP. + * - In-order stream, but pinfo->num does not match the begin of the + * MSP. Must be a retransmission. + * - OoO stream where this segment fills the gap in the begin of the + * MSP. msp->first_frame is the start where the gap was detected + * (and does NOT match pinfo->num). */ - if (msp->first_frame == pinfo->num) { + if (msp->first_frame == pinfo->num || msp->first_frame_with_seq == pinfo->num) { str = ""; col_append_sep_str(pinfo->cinfo, COL_INFO, " ", "[TCP segment of a reassembled PDU]"); } else { str = "Retransmitted "; + is_retransmission = TRUE; /* TCP analysis already flags this (in COL_INFO) as a retransmission--if it's enabled */ } /* Fix for bug 3264: look up ipfd for this (first) segment, so can add tcp.reassembled_in generated field on this code path. */ - ipfd_head = fragment_get(&tcp_reassembly_table, pinfo, pinfo->num, NULL); - if (ipfd_head) { - if (ipfd_head->reassembled_in != 0) { - item = proto_tree_add_uint(tcp_tree, hf_tcp_reassembled_in, tvb, 0, - 0, ipfd_head->reassembled_in); - PROTO_ITEM_SET_GENERATED(item); + if (!is_retransmission) { + ipfd_head = fragment_get(&tcp_reassembly_table, pinfo, msp->first_frame, NULL); + if (ipfd_head) { + if (ipfd_head->reassembled_in != 0) { + item = proto_tree_add_uint(tcp_tree, hf_tcp_reassembled_in, tvb, 0, + 0, ipfd_head->reassembled_in); + PROTO_ITEM_SET_GENERATED(item); + } } } @@ -3242,7 +3255,8 @@ again: if (reassemble_ooo && !PINFO_FD_VISITED(pinfo)) { /* If the first segment of the MSP was seen, remember it. */ - if (msp->seq == seq) { + if (msp->seq == seq && msp->flags & MSP_FLAGS_MISSING_FIRST_SEGMENT) { + msp->first_frame_with_seq = pinfo->num; msp->flags &= ~MSP_FLAGS_MISSING_FIRST_SEGMENT; } /* Remember when all segments are ready to avoid subsequent diff --git a/epan/dissectors/packet-tcp.h b/epan/dissectors/packet-tcp.h index c1811fa049..a01b175cb0 100644 --- a/epan/dissectors/packet-tcp.h +++ b/epan/dissectors/packet-tcp.h @@ -170,9 +170,11 @@ struct tcp_acked { struct tcp_multisegment_pdu { guint32 seq; guint32 nxtpdu; - guint32 first_frame; + guint32 first_frame; /* The frame where this MSP was created (used as key in reassembly tables). */ guint32 last_frame; nstime_t last_frame_time; + guint32 first_frame_with_seq; /* The frame that contains the first frame that matches 'seq' + (same as 'first_frame', larger than 'first_frame' for OoO segments) */ guint32 flags; #define MSP_FLAGS_REASSEMBLE_ENTIRE_SEGMENT 0x00000001 /* Whether this MSP is finished and no more segments can be added. */ diff --git a/test/captures/http-ooo2.pcap b/test/captures/http-ooo2.pcap new file mode 100644 index 0000000000..8cab45c920 Binary files /dev/null and b/test/captures/http-ooo2.pcap differ diff --git a/test/suite_dissection.py b/test/suite_dissection.py index 0da1b17d18..9af19ec2cd 100644 --- a/test/suite_dissection.py +++ b/test/suite_dissection.py @@ -87,3 +87,37 @@ class case_dissect_tcp(subprocesstest.SubprocessTestCase): '-Y', 'dns', '-Tfields', '-edns.qry.name', )) self.assertEqual(proc.stdout_str.strip(), 'example.com') + + def test_tcp_out_of_order_first_gap(self, cmd_tshark, capture_file): + ''' + Test reporting of "reassembled_in" in the OoO frame that contains the + initial segment (Bug 15420). Additionally, test for proper reporting + when the initial segment is retransmitted. + For PDU H123 (where H is the HTTP Request header and 1, 2 and 3 are part + of the body), the order is: (SYN) 2 H H 1 3 H. + ''' + proc = self.assertRun((cmd_tshark, + '-r', capture_file('http-ooo2.pcap'), + '-otcp.reassemble_out_of_order:TRUE', + '-Tfields', + '-eframe.number', '-etcp.reassembled_in', '-e_ws.col.Info', + '-2', + )) + lines = proc.stdout_str.replace('\r', '').split('\n') + # 2 - start of OoO MSP + self.assertIn('2\t6\t[TCP Previous segment not captured]', lines[1]) + self.assertIn('[TCP segment of a reassembled PDU]', lines[1]) + # H - first time that the start of the MSP is delivered + self.assertIn('3\t6\t[TCP Out-Of-Order]', lines[2]) + self.assertIn('[TCP segment of a reassembled PDU]', lines[2]) + # H - first retransmission. + self.assertIn('4\t\t', lines[3]) + self.assertNotIn('[TCP segment of a reassembled PDU]', lines[3]) + # 1 - continue reassembly + self.assertIn('5\t6\t[TCP Out-Of-Order]', lines[4]) + self.assertIn('[TCP segment of a reassembled PDU]', lines[4]) + # 3 - finish reassembly + self.assertIn('6\t\tPUT /0 HTTP/1.1', lines[5]) + # H - second retransmission. + self.assertIn('7\t\t', lines[6]) + self.assertNotIn('[TCP segment of a reassembled PDU]', lines[6])