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

URL: https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=ca423314373b0a4ce7d6bc1cf94c4995e1263ea2
Submitter: Anders Broman (a.broman58@xxxxxxxxx)
Changed: branch: master
Repository: wireshark

Commits:

ca42331 by Peter Wu (peter@xxxxxxxxxxxxx):

    tcp: add support for reassembling out-of-order segments
    
    Currently out-of-order segments will result in cutting a stream into
    two pieces while the out-of-order segment itself is ignored. For
    example, a stream of segments "ABDCE" is interpreted as "AB", "DE" with
    "C" ignored. This behavior breaks TLS decryption or prevent application
    layer PDUs (such as HTTP requests/responses) from being reconstructed.
    To fix this, buffer segments when a gap is detected.
    
    The proposed approach extends the "multi-segment PDU" (MSP) mechanism
    which is normally used for linking multiple, sequential TCP segments
    into a single PDU. When a gap is detected between segments, it is
    assumed that the segments within this gap are out-of-order and will be
    received (or retransmitted) later.
    
    The current implementation has a limitation though, if multiple gaps
    exist, then the subdissector will only be called when all gaps are
    filled (the subdissector will receive segments later than necessary).
    For example with "ACEBD", "ABC" can already be processed after "B" is
    received (with "E" still buffered), but due to how MSP are extended, it
    must receive "D" too before it reassembles "ABCDE". In practice this
    could mean that the request/response times between HTTP requests and
    responses are slightly off, but at least the stream is correct now.
    (These limitations are documented in the User's Guide.)
    
    As the feature fails at least the 802.11 decryption test where packets
    are missing (instead of OoO), hide this feature behind a preference.
    
    Tested with captures containing out-of-order TCP segments from the
    linked bug reports, comparing the effect of toggling the preference on
    the summary output of tshark, the verbose output (-V) and the two-pass
    output (-2 or -2V). Captures marked with "ok" just needed "simple"
    out-of-order handling. Captures marked with "ok2" additionally required
    the reassembly API change to set the correct reassembled length.
    
    This change does "regress" on bug 10289 though when the preference is
    enabled as retransmitted single-segment PDUs are now passed to
    subdissectors. I added a TODO comment for this unrelated cosmetic issue.
    
    Bug: 3389   # capture 2907 (HTTP) ok
    Bug: 4727   # capture 4590 (HTTP) ok
    Bug: 9461   # capture 12130 (TLS/HTTP/RPC-over-HTTP +key 12131) ok
    Bug: 12006  # capture 14236 (HTTP) ok2; capture 15261 (HTTP) ok
    Bug: 13517  # capture 15370 (HTTP) ok; capture 16059 (MQ) ok
    Bug: 13754  # capture 15593 (MySQL) ok2
    Bug: 14649  # capture 16305 (WebSocket) ok
    Change-Id: If3938c5c1c96db8f7f50e39ea779f623ce657d56
    Reviewed-on: https://code.wireshark.org/review/27943
    Petri-Dish: Peter Wu <peter@xxxxxxxxxxxxx>
    Tested-by: Petri Dish Buildbot
    Reviewed-by: Anders Broman <a.broman58@xxxxxxxxx>
    

Actions performed:

    from  e6935f9   docbook: add info about using symbolic links for git hooks.
    adds  ca42331   tcp: add support for reassembling out-of-order segments


Summary of changes:
 docbook/release-notes.asciidoc                  |   6 +-
 docbook/wsug_src/WSUG_chapter_advanced.asciidoc |  63 +++++++++
 epan/dissectors/packet-tcp.c                    | 165 +++++++++++++++++++++++-
 epan/dissectors/packet-tcp.h                    |   9 ++
 epan/reassemble.c                               |  74 +++++++++--
 epan/reassemble.h                               |  12 ++
 test/captures/http-ooo.pcap                     | Bin 0 -> 1209 bytes
 test/suite_dissection.py                        |  50 +++++++
 8 files changed, 359 insertions(+), 20 deletions(-)
 create mode 100644 test/captures/http-ooo.pcap