osmith
/
wireshark
forked from osmocom/wireshark
1
0
Fork 0
Code Pull Requests Releases Wiki Activity

TCP: pass data after a ZeroWindowProbe to subdissectors 

If the single byte within a ZeroWindowProbe triggers reassembly within a
subdissector, a new MSP will be created with just a single byte. Be sure
not to mark subsequent segments that contain the full segment data as
retransmission as this prevents the subdissector from seeing the data.

Bug: 15427
Change-Id: I36ae2622689c6606c99cdff70b6beba4b9d25ca7
Reviewed-on: https://code.wireshark.org/review/31732
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Jasper Bongertz <jasper@packet-foo.com>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
This commit is contained in:
Peter Wu 2019-01-25 14:05:24 +01:00
parent 596f538b5b
commit 1527177cb9
4 changed files with 50 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
docbook/release-notes.asciidoc
View File

@ -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.

23
epan/dissectors/packet-tcp.c
View File

@ -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;

BIN
test/captures/retrans-tls.pcap Normal file
View File

Binary file not shown.

25
test/suite_dissection.py
View File

@ -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')