Skip to content

fix(toolkit): pyshark traces TCP behind an IPv4 extension header as the default engine does (#1625) - #1642

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1625-pyshark-ipv4-extension-headers-rebased
Oct 13, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1625-pyshark-ipv4-extension-headers-rebased

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Description of your pull request and other information

Replaces #1628, which was closed for conflicts.

  • Cause. For TCP behind an AH or IPv6 extension header in IPv4 (protocols 0, 43, 44, 51, 60, 135, 139), tshark applies its own dissection rules. Its PDML carries nothing past a Mobility or HIP header, and it reads TCP behind Shim6 (140), which the default engine leaves raw (ipv6_ext.py:586). So pyshark traced TCP the default engine rejects and missed TCP it reads.
  • Fix. For such a frame, tcp_traceflow reads the TCP from the frame's octets by pypcapfile's rules, the same as fix(toolkit): dpkt and scapy find TCP in IPv4 where the default engine does (#1606, #1617) #1624 does for dpkt and scapy (pcapkit/toolkit/pyshark.py:631-1016). The walk starts at the outermost IP layer the default engine dissects (_network_start), so GRE, VXLAN and the 32-header chain bound stop it in the same place. The link type comes from the capture file, not tshark's frame.encap_type, which reads DLT 12 and 14 as raw IP. That covers both the PCAP header and each PCAP-NG interface (_linktypes, pcapkit/foundation/engines/pyshark.py:75-148).
  • Octets. -T pdml refuses -x, so the engine starts a second tshark -T ek -x -J frame, filtered by OCTETS_FILTER (pyshark.py:354). It reaps that process on close. If the pass or the capture file fails, the engine warns once with PySharkWarning and falls back to tshark's dissection, as main does.
  • Measured on Python 3.11 with tshark 4.6.9: 16,800 random frames over link types 0, 1, 12, 14, 101, 108, 113, 228, 229 and 276, plus 3,000 PCAP-NG frames over 14 interfaces in two sections. Against main, 0 new disagreements and 1,717 fixed.
  • Tests. New tests/toolkit/test_pyshark_ipv4_extension_headers_1625_unit.py, 28 tests. With pcapkit/ reverted all 28 fail, 144 failures counting subtests. The fix(protocols): a chain of ~80+ IPv6 extension headers exhausts recursion and leaves the payload undissected #1604 table drops its ipv4-33/pyshark/trace row. Its ipv6-33 row stays, so fix(toolkit): the dpkt, scapy and pyshark engines do not stop an extension-header chain at 32 #1609 stays open. Also green with this branch merged into main at b375656e1, on both 3.11 and 3.14.

Not fixed here; each is unchanged from main:

Closes #1625

…he default engine does (#1625)

- tshark dissects TCP behind an AH or IPv6 extension header in IPv4 by its
  own rules, leaves a Mobility or HIP header's payload out of the PDML, and
  reads past Shim6, which the default engine leaves raw in IPv4.
- For such a frame, tcp_traceflow now reads the TCP from its octets by the
  rules the other engines share (pypcapfile's _upper_layer and
  _default_accepts), from the outermost IP layer the default engine
  dissects: through IP-in-IP tunnels and no other, up to
  EXTENSION_HEADER_LIMIT headers. Over a link layer the default engine
  reads no IP over, the frame is read as tshark dissects it.
- That layer is found by the frame's link type in the capture file, which
  the engine reads itself, not by frame.encap_type: tshark reads link
  types 12 and 14 as raw IP (101), which the default engine leaves raw.
- The octets come from a second tshark process (-T ek -x -J frame), started
  on first use, read forward, and reaped on close. If it or the capture
  file cannot be read, the engine warns with PySharkWarning and traces such
  frames as tshark dissects them.
- Drop ipv4-33/pyshark/trace from the #1609 Gap row, which this fixes.

Tests: new tests/toolkit/test_pyshark_ipv4_extension_headers_1625_unit.py.
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 13, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 24814ab68. Sonnet cross-review over three rounds:

  • Round 1 regressed GRE, VXLAN and outer IPv6.
  • Round 2 walked DLT 12 and 14 as raw IP.
  • Round 3 reads the link type from the capture file.

In the reviewer's own 12,244-frame sweep, which includes multi-section PCAP-NG, round 3 adds 0 new disagreements and fixes 580. I merged it into main at b375656e1 and ran eight neighbouring test files on 3.11 and 3.14: all green. Notes, not blocking: DLT 12 and 14 match main but do not agree with the default engine (#1639), and the outer-IPv6 Mobility/HIP gap belongs to #1626.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 89.71% (unit tier, Python 3.14, 24814ab68, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 3 20 3 95.16%
pcapkit/const 18858 954 2366 868 91.06%
pcapkit/corekit 2366 67 760 35 96.48%
pcapkit/dumpkit 275 3 100 3 98.40%
pcapkit/foundation 3030 152 1074 35 94.23%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17166 226 4580 191 98.06%
pcapkit/toolkit 1142 168 440 5 84.13%
pcapkit/utilities 494 4 146 4 98.75%
pcapkit/vendor 4409 2342 1006 157 43.25%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw merged commit 86ff271 into main Oct 13, 2026
40 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 13, 2026
@JarryShaw
JarryShaw deleted the fix/1625-pyshark-ipv4-extension-headers-rebased branch October 13, 2026 16:05
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 13, 2026
JarryShaw added a commit that referenced this pull request Oct 14, 2026
Third-party engines are best effort (#1687), so the pyshark code that
exists only to match the default engine comes out:

- revert #1686, #1670 and #1642: the second tshark pass that read a
  frame's octets, the link type read from the capture file, the 64-layer
  bound, reading past tshark's 802.1Q limit, and keying a source-routed
  segment by ip.cur_rt
- revert #1613's tshark preferences that turned IP reassembly off, with
  its FileCapture assertion in test_runtime_engines.py and its sentence
  in the engines index
- keep #1613's partial-TCP-header guard and #1605's keying by the IP
  header that carries the segment
- re-land #1671's crash fix: a TCP layer whose IP layer lacks ip.src or
  ip.dst is not traced, rather than raising AttributeError
- delete the three test modules those reverts added and the agreement
  checks in test_pyshark_unit.py; the #1611 and #1671 checks assert
  constants

The pyshark test modules, test_runtime_engines.py, test_engine_runtime.py
and the project guards pass on 3.11 (tshark 4.6.9) and 3.14.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(toolkit): pyshark traces TCP behind an extension header in IPv4 that the default engine rejects, and misses it behind Mobility and HIP

1 participant