Skip to content

fix(toolkit): dpkt reads IPv6 in IPv4, and nothing behind MPLS or Cisco ISL (#1637, #1638, #1646) - #1655

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1637-1638-dpkt-6in4-reassembly-and-mpls-isl
Oct 13, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1637-1638-dpkt-6in4-reassembly-and-mpls-isl

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

#1637. ipv6_reassembly took _network(packet) alone. It now takes _first_ipv6 (pcapkit/toolkit/dpkt.py:935), which walks IPv4 tunnels and the headers the default engine reads past, as _tcp_carrier does, and stops at a rejected IPv4 header, a later IPv4 fragment, or any other protocol, ICMP quotes included. The octets end at each Total Length above, then at the Payload Length, or the Jumbo Payload Length when that is zero (:1157); a zero one with no Jumbo option gives no input. The Fragment header must also be among the first EXTENSION_HEADER_LIMIT (32) extension headers and within the frame's first FRAME_LAYER_LIMIT (64) layers (:1151), the two bounds IPv6._decode_next_layer walks to (pcapkit/protocols/internet/ipv6.py:555, :561). The count starts at _network_depth (:553), the link layer and each VLAN tag, and adds each IPv4 header and each header _walk_extension_headers steps over (pcapkit/toolkit/pypcapfile.py:679, split out of _past_extension_headers to count them). These rules also fix 5 IPv6-in-Ethernet Payload Length rows, and #1609's dpkt IPv6 reassembly rows; its TCP rows remain.

#1638. _network (:525) took any ip/ip6 that dpkt's Ethernet named. _ethernet_accepted (:477) now refuses a payload dpkt reached past MPLS labels, an ISL header, or a TPID other than 0x8100/0x88A8, so legacy QinQ 0x9100/0x9200 too. dpkt overwrites an ISL tag with an inner 802.1Q tag, so ISL is also read from the captured address (:518).

#1646. dpkt reads some tag stacks in two goes, e.g. 802.1Q then 802.1ad, two 802.1ad, or three 802.1Q, and keeps only the second go's tags. So _layer2bytes looked for the IP header 4 octets early per dropped tag, and ProtocolError ended the extraction: on the TCP and IPv4 paths on main, and on the 6in4 path once _first_ipv6 sliced the IPv4. _wire_hdr_len (:381) now adds the dropped tags, counted off the captured octets by _vlan_tpids (:345). _ethernet_accepted reads the TPIDs there too, so a dropped 0x9100 tag still refuses the frame.

Measured on 3.14 and 3.11 (dpkt 1.9.8): every row of both issues' repros, and every stacked-tag row of the gaps repro, matches the default engine. Of the 62 stacks of one to five 802.1Q/802.1ad tags, dpkt raised on 12 and read nothing behind 46 on main. Now the 12 agree and the 46 are unchanged (below). A nesting probe over Ethernet with 0 to 3 tags, raw IP and BSD loopback agrees on all 287 IPv6 rows, where main disagreed on 88 and raised on 123.

New tests/toolkit/test_dpkt_6in4_reassembly_and_mpls_isl_1637_1638_unit.py compares dpkt with the default engine field for field: 32 IPv6-in-IPv4 cases, 6 Payload Length cases, 20 link-layer cases, 12 chain-length cases (3 controls), 20 layer-limit cases (10 controls) and 7 tag stacks. Reverting pcapkit/ to main gives 61 failures. Reverting only the layer bound fails the 10 rows past it and both datagrams checks; reverting the chain-bound and VLAN changes as well fails 21, ProtocolError ending one read. Dropping only the later-fragment stop (or ip.offset, :986) fails the AH and Hop-by-Hop later-fragment rows.

Not fixed here, identical before and after:

Not fixed here, as on main: _tcp_carrier and _first_ipv4 do not stop at FRAME_LAYER_LIMIT (#1609), so dpkt hands TCP and IPv4 reassembly a segment or fragment past layer 64. Behind a stack dpkt reads in two goes, main raised ProtocolError on those frames instead (20 probe rows).

Closes #1637
Closes #1638
Closes #1646

…co ISL (#1637, #1638, #1646)

- #1637: ipv6_reassembly took the network layer alone, so an IPv6 fragment
  carried in IPv4 gave no input. It now takes _first_ipv6, which walks IPv4
  tunnels and the headers the default engine reads past, stops at a rejected
  IPv4 header, a later IPv4 fragment or any other protocol, and cuts the
  octets at each Total Length above. The IPv6 packet then ends at its
  Payload Length, or at the Jumbo Payload Length of a zero one; a zero one
  with no Jumbo option gives no input. The Fragment header must be among the
  first EXTENSION_HEADER_LIMIT extension headers and within FRAME_LAYER_LIMIT
  layers of the frame, as the default engine walks; _walk_extension_headers
  counts the headers in IPv4 it steps over.
- #1638: _network took any ip/ip6 that dpkt's Ethernet named, including one
  behind MPLS, Cisco ISL or a legacy QinQ TPID, which the default engine
  reads as raw octets. _ethernet_accepted now refuses those, reading ISL
  from the captured octets once an inner 802.1Q tag has replaced it.
- dpkt keeps only its second go's tags of a stack such as 802.1Q then
  802.1ad, so _layer2bytes looked for the IP header too early and raised
  ProtocolError. _wire_hdr_len now adds the dropped tags, and
  _ethernet_accepted reads the TPIDs, off the captured octets.

New tests/toolkit/test_dpkt_6in4_reassembly_and_mpls_isl_1637_1638_unit.py
compares dpkt with the default engine over 97 cases; it fails without the fix.

Closes #1637
Closes #1638
Closes #1646
@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 9a01482ec. Sonnet cross-reviewed this over five rounds. Rounds 3 and 4.1 accepted three new disagreements as disclosures; I sent all three back as code changes, because this PR must create no row where main already agrees with the default engine:

Round 4.2 checked about 5,000 of its own captures and mutants and 77,000 direct-call mutants on 3.14 and 3.11. It found no row where main agreed and this branch disagrees, and no new exception. With the fix reverted, 61 tests fail on main.

I re-ran the new test, test_dpkt_unit and the TCP-carrier test myself on both Pythons: all green. The branch merges cleanly with main, #1652, #1642 and #1653. The commit message's Closes #VLAN placeholder is now Closes #1646. Left open: #1609, #1651 and #1654.

@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 89.68% (unit tier, Python 3.14, 9a01482ec, 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 2884 145 1010 35 94.14%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17166 226 4580 191 98.06%
pcapkit/toolkit 1124 170 432 8 83.55%
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 f247dbd into main Oct 13, 2026
38 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/1637-1638-dpkt-6in4-reassembly-and-mpls-isl branch October 13, 2026 16:09
@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
…1721)

Third-party engines are best effort (#1687), so the pypcapfile code that
exists only to match the default engine comes out:

- revert #1682, #1600, #1575 and #1535, and the pypcapfile hunks of #1655,
  #1608 and #1605: reading loopback and raw-IP link types, the frame layer
  bound, the VLAN and tunnel walks, keying tunnelled TCP by the innermost
  header, stepping over AH and extension headers, and checking options
  with the default engine's parsers
- keep the helpers the dpkt and scapy adapters import, unchanged, until
  their own reverts drop them; their docstrings no longer say pypcapfile
  asks the default engine's parsers too
- re-land #1576, #1577, #1590 and #1591 as guards in _network and
  _transport, #1513's once-per-capture warning for TCP straight over
  IPv6, and #1667's packet2dict fix: an undecoded frame is reported as
  its octets, not as hex text twice as long
- keep #1569's record reader and #1690's packet2timestamp
- delete the three test modules those reverts added, the pypcapfile
  timestamp agreement class, and #1682's link-type warning test; the
  re-landed guards' tests assert constants
- drop what the reverts removed from the docs, READ_LINKTYPES included,
  and say that TCP behind a VLAN tag, in a tunnel or behind an
  extension header (an IPv6 one, or AH in IPv4) is now left out
  without a warning

The pypcapfile modules, test_new_engine_parity_runtime.py,
test_engine_runtime.py, the timestamp module and the project guards pass
on 3.11 (pypcapfile 0.12.0) and 3.14.
JarryShaw added a commit that referenced this pull request Oct 14, 2026
Third-party engines are best effort (#1687), so the dpkt code that exists
only to match the default engine comes out:

- revert #1662, #1655, #1624, #1622, #1614 and the dpkt hunks of #1605:
  the octet walk and its link-type stash, IPv6 in IPv4 and IPv4 in IPv6,
  refusing MPLS and ISL, the tunnel and extension-header walks, the IPv4
  header checks and the layer bounds
- keep #1690's timestamps, #1594's parser choice, #1621 and #1653
- re-land as guards: a layer dpkt's own lengths misplace (#1646) is
  re-serialised, not refused with ProtocolError; a Total Length below the
  IHL keeps the header whole (#1616), and a header the capture cuts is no
  input; packet2dict and packet2chain take a str payload and a raw frame
- delete the modules #1662 and #1655 added; the 1615/1616 module keeps
  only what its #1616 tests use, as no scapy test imports it now; dpkt's
  comparisons with the default engine assert constants or the capture's
  octets, or are dropped
- drop pypcapfile's VLAN_TAGS, VLAN_TAG_LEN and IPV6_EXTENSION_HEADERS,
  which only dpkt imported, and the 1668 module's _Agreement base, which
  nothing subclasses now
- the docs drop the depth claims and name what dpkt leaves out silently

The dpkt modules, the shared modules that run dpkt, tests/foundation/engines
and the project guards pass on 3.14 and 3.11 (dpkt 1.9.8, pypcapfile 0.12.0).
JarryShaw added a commit that referenced this pull request Oct 14, 2026
Third-party engines are best effort (#1687), so the dpkt code that exists
only to match the default engine comes out:

- revert #1662, #1655, #1624, #1622, #1614 and the dpkt hunks of #1605:
  the octet walk and its link-type stash, IPv6 in IPv4 and IPv4 in IPv6,
  refusing MPLS and ISL, the tunnel and extension-header walks, the IPv4
  header checks and the layer bounds
- keep #1690's timestamps, #1594's parser choice, #1621 and #1653
- re-land as guards: a layer dpkt's own lengths misplace (#1646) is
  re-serialised, not refused with ProtocolError; a Total Length below the
  IHL keeps the header whole (#1616), and a header the capture cuts is no
  input; packet2dict and packet2chain take a str payload and a raw frame
- delete the modules #1662 and #1655 added; the 1615/1616 module keeps
  only what its #1616 tests use, as no scapy test imports it now; dpkt's
  comparisons with the default engine assert constants or the capture's
  octets, or are dropped
- drop pypcapfile's VLAN_TAGS, VLAN_TAG_LEN and IPV6_EXTENSION_HEADERS,
  which only dpkt imported, and the 1668 module's _Agreement base, which
  nothing subclasses now
- the docs drop the depth claims and name what dpkt leaves out silently

The dpkt modules, the shared modules that run dpkt, tests/foundation/engines
and the project guards pass on 3.14 and 3.11 (dpkt 1.9.8, pypcapfile 0.12.0).
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

1 participant