Wireshark-commits: [Wireshark-commits] master-2.4 b7575fc: TLS: fix broken reassembly with multiple
From: Wireshark code review <code-review-do-not-reply@xxxxxxxxxxxxx>
Date: Thu, 27 Sep 2018 09:13:01 +0000
URL: https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=b7575fc1a2a5d89c067de877f83d3ba1bb3fec46 Submitter: "Peter Wu <peter@xxxxxxxxxxxxx>" Changed: branch: master-2.4 Repository: wireshark Commits: b7575fc by Peter Wu (peter@xxxxxxxxxxxxx): TLS: fix broken reassembly with multiple PDUs in the same frame When (1) a frame has multiple TLS application data records and (2) two of them request reassembly of a new PDU, then the second fragment would be considered conflicting with the first one since the PDUs (MSPs) are identified by the frame number of the starting frame. This behavior was observed in a firefox-http2-frag.pcap (attachment 16616) which uses tcp_dissect_pdus to trigger reassembly: Frame 19: 8694 bytes on wire (69552 bits), 8694 bytes captured (69552 bits) ... Transport Layer Security (8640 bytes) TLSv1.3 Record Layer: Application Data Protocol: http2 SSL segment data (1369 bytes) <-- 7/7 last segment of previous PDU SSL segment data (1203 bytes) <-- 1/5 first segment of new PDU TLSv1.3 Record Layer: Application Data Protocol: http2 SSL segment data (1369 bytes) <-- 2/5 TLSv1.3 Record Layer: Application Data Protocol: http2 SSL segment data (1369 bytes) <-- 3/5 TLSv1.3 Record Layer: Application Data Protocol: http2 SSL segment data (1369 bytes) <-- 4/5 TLSv1.3 Record Layer: Application Data Protocol: http2 SSL segment data (976 bytes) <-- 5/5 TLSv1.3 Record Layer: Application Data Protocol: http2 SSL segment data (1369 bytes) <-- 1/? first segment of another PDU [5 Reassembled TLS segments (6286 bytes): #19(1203), #19(1369), #19(1369), #19(1369), #19(976)] [7 Reassembled TLS segments (8201 bytes): #17(1190), #17(1369), #17(1369), #18(1369), #18(1369), #18(1369), #19(166)] HyperText Transfer Protocol 2 (8201 bytes, reassembled PDU) Stream: DATA, Stream ID: 17, Length 8192 (partial entity body) ... (7/7 finishes previous reassembly, see "7 Reassembled TLS segments") HyperText Transfer Protocol 2 (1203 bytes, start of new PDU) HyperText Transfer Protocol 2 (6286 bytes, reassembled PDU) Stream: DATA, Stream ID: 17, Length 6277 (partial entity body) ... (all fragments are in this frame, see "5 Reassembled TLS segments") HyperText Transfer Protocol 2 (1369 bytes, start of another PDU) [Reassembly error, protocol SSL: Frame already added in first pass] TLS records for fragments 1/5 and 1/? both start a new PDU and would thus invoke fragment_add with the same identifier. That results in the Reassembly error which breaks further decryption. Reduce the probability of this issue by mixing in the TLS stream position of the fragment. Bug: 11173 Change-Id: I5536f3010b156555f1d7ae6dc98e08c030c8f771 Reviewed-on: https://code.wireshark.org/review/29871 Petri-Dish: Peter Wu <peter@xxxxxxxxxxxxx> Tested-by: Petri Dish Buildbot Reviewed-by: Anders Broman <a.broman58@xxxxxxxxx> (cherry picked from commit be16d87c60ae82eaef60da2bd4ce3597a05c1a30) Reviewed-on: https://code.wireshark.org/review/29886 Reviewed-by: Peter Wu <peter@xxxxxxxxxxxxx> Actions performed: from 1ce272d pcapng: Free option_content on error add b7575fc TLS: fix broken reassembly with multiple PDUs in the same frame Summary of changes: epan/dissectors/packet-ssl.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
- Prev by Date: [Wireshark-commits] master-2.6 9474283: TLS: fix broken reassembly with multiple PDUs in the same frame
- Next by Date: [Wireshark-commits] master 776b4d5: DNS: update content type for DoH draft -07 (up to draft -14)
- Previous by thread: [Wireshark-commits] master-2.6 9474283: TLS: fix broken reassembly with multiple PDUs in the same frame
- Next by thread: [Wireshark-commits] master 776b4d5: DNS: update content type for DoH draft -07 (up to draft -14)
- Index(es):