Wireshark · Wireshark-commits: [Wireshark-commits] master-2.6 9474283: TLS: fix broken reassembly with multiple PDUs in the same fr

[Wireshark code review <code-review-do-not-reply@xxxxxxxxxxxxx>]

URL: https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=94742838186849f7bc5b954f735eea83bbbcc4d3
Submitter: "Peter Wu <peter@xxxxxxxxxxxxx>"
Changed: branch: master-2.6
Repository: wireshark

Commits:

9474283 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/29885
    Reviewed-by: Peter Wu <peter@xxxxxxxxxxxxx>
    

Actions performed:

    from  1e9bd90   pcapng: Free option_content on error
     add  9474283   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(-)