diff options
| -rw-r--r-- | docbook/release-notes.asciidoc | 2 | ||||
| -rw-r--r-- | epan/dissectors/packet-tcp.c | 23 | ||||
| -rw-r--r-- | test/captures/retrans-tls.pcap | bin | 0 -> 158 bytes | |||
| -rw-r--r-- | test/suite_dissection.py | 25 |
4 files changed, 50 insertions, 0 deletions
diff --git a/docbook/release-notes.asciidoc b/docbook/release-notes.asciidoc index 8b7451ad2e..1dfe7e37ed 100644 --- a/docbook/release-notes.asciidoc +++ b/docbook/release-notes.asciidoc @@ -29,6 +29,8 @@ The following bugs have been fixed: //* cveidlink:2014-2486[] //* Wireshark slowly leaked water under the kitchen sink over the course of several months, causing a big mess. +* Data following a TCP ZeroWindowProbe is marked as retransmission and not passed to subdissectors (wsbuglink:15427[]) + //_Non-empty section placeholder._ Dumpcap might not quit if Wireshark or TShark crashes. diff --git a/epan/dissectors/packet-tcp.c b/epan/dissectors/packet-tcp.c index ba1ebff815..44cf550364 100644 --- a/epan/dissectors/packet-tcp.c +++ b/epan/dissectors/packet-tcp.c @@ -3071,10 +3071,33 @@ again: if (tcpd) { /* Have we seen this PDU before (and is it the start of a multi- * segment PDU)? + * + * If the sequence number was seen before, it is part of a + * retransmission if the whole segment fits within the MSP. + * (But if this is this frame was already visited and the first frame of + * the MSP matches the current frame, then it is not a retransmission, + * but the start of a new MSP.) + * + * If only part of the segment fits in the MSP, then either: + * - The previous segment included with the MSP was a Zero Window Probe + * with one byte of data and the subdissector just asked for one more + * byte. Do not mark it as retransmission (Bug 15427). + * - Data was actually being retransmitted, but with additional data + * (Bug 13523). Do not mark it as retransmission to handle the extra + * bytes. (NOTE Due to the TCP_A_RETRANSMISSION check below, such + * extra data will still be ignored.) + * - The MSP contains multiple segments, but the subdissector finished + * reassembly using a subset of the final segment (thus "msp->nxtpdu" + * is smaller than the nxtseq of the previous segment). If that final + * segment was retransmitted, then "nxtseq > msp->nxtpdu". + * Unfortunately that will *not* be marked as retransmission here. + * The next TCP_A_RETRANSMISSION hopefully takes care of it though. + * * Only shortcircuit here when the first segment of the MSP is known, * and when this this first segment is not one to complete the MSP. */ if ((msp = (struct tcp_multisegment_pdu *)wmem_tree_lookup32(tcpd->fwd->multisegment_pdus, seq)) && + nxtseq <= msp->nxtpdu && !(msp->flags & MSP_FLAGS_MISSING_FIRST_SEGMENT) && msp->last_frame != pinfo->num) { const char* str; gboolean is_retransmission = FALSE; diff --git a/test/captures/retrans-tls.pcap b/test/captures/retrans-tls.pcap Binary files differnew file mode 100644 index 0000000000..90be0de74b --- /dev/null +++ b/test/captures/retrans-tls.pcap diff --git a/test/suite_dissection.py b/test/suite_dissection.py index 9af19ec2cd..9a62e933cc 100644 --- a/test/suite_dissection.py +++ b/test/suite_dissection.py @@ -121,3 +121,28 @@ class case_dissect_tcp(subprocesstest.SubprocessTestCase): # H - second retransmission. self.assertIn('7\t\t', lines[6]) self.assertNotIn('[TCP segment of a reassembled PDU]', lines[6]) + + def test_tcp_reassembly_more_data_1(self, cmd_tshark, capture_file): + ''' + Tests that reassembly also works when a new packet begins at the same + sequence number as the initial segment. This models behavior with the + ZeroWindowProbe: the initial segment contains a single byte. The second + segment contains that byte, plus the remainder. + ''' + proc = self.assertRun((cmd_tshark, + '-r', capture_file('retrans-tls.pcap'), + '-Ytls', '-Tfields', '-eframe.number', '-etls.record.length',)) + output = proc.stdout_str.replace('\r', '') + # First pass dissection actually accepted the first frame as TLS, but + # subsequently requested reassembly. + self.assertEqual(output, '1\t\n2\t16\n') + + def test_tcp_reassembly_more_data_2(self, cmd_tshark, capture_file): + ''' + Like test_tcp_reassembly_more_data_1, but checks the second pass (-2). + ''' + proc = self.assertRun((cmd_tshark, + '-r', capture_file('retrans-tls.pcap'), + '-Ytls', '-Tfields', '-eframe.number', '-etls.record.length', '-2')) + output = proc.stdout_str.replace('\r', '') + self.assertEqual(output, '2\t16\n') |