From 0b9b531726d5b18ab1ef9d071a312a3c578743e3 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Thu, 24 Jan 2019 16:32:28 +0100 Subject: [PATCH] tcp: fix reporting of "Reassembled in" for OoO initial segment When the initial segment is OoO, it was recognized as retransmitted. Fix this by remembering which frame actually contains the initial segment. Bug: 15420 Change-Id: If63e2ff581775ff9d396a612839f1bfab30f111f Reviewed-on: https://code.wireshark.org/review/31720 Petri-Dish: Peter Wu Tested-by: Petri Dish Buildbot Reviewed-by: Peter Wu --- epan/dissectors/packet-tcp.c | 30 ++++++++++++++++++++++-------- epan/dissectors/packet-tcp.h | 4 +++- test/captures/http-ooo2.pcap | Bin 0 -> 536 bytes test/suite_dissection.py | 34 ++++++++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 9 deletions(-) create mode 100644 test/captures/http-ooo2.pcap 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 0000000000000000000000000000000000000000..8cab45c92064710b31e7364007a24e0148b987d3 GIT binary patch literal 536 zcmca|c+)~A1{MYw`2U}Q;R%cbq&1+}6~ts@U~phdJIlquzzD=lbEt*Gq(eg#^bHg|LP7%c4fPCpxt#O!N>cMmbbV6u(n~U|6wG+Jc}>J dU~WIXf`P#h>@HT~boRk?o?ODfU`%5l0RZe{OZxx- literal 0 HcmV?d00001 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])