Repository navigation
fix(toolkit): scapy walks link types, layer bounds and IPv6 extension headers as the default engine does (#1609, #1626, #1650) - #1663
Conversation
JarryShaw
added
fix
|
Verdict: GOOD TO GO at The reviewer ran its own 394-case probe, covering link types, NULL and LOOP families in both byte orders, and depth and extension-chain sweeps:
NULL and LOOP no longer depend on the host's This conflicts with the dpkt PR only in Left open on #1609: the pypcapfile and pyshark parts, which neither PR touches. |
|
Coverage: 89.46% (unit tier, Python 3.14,
Per-file detail: the |
|
resolve conflicts |
… headers as the default engine does (#1609, #1626, #1650) - #1650: the first layer was whatever Scapy dissected for the link type, so TCP and IP fragments at LINUX_SLL, LINUX_SLL2, 12 and PPP were handed on. _network now starts the walk only at NULL, ETHERNET, LOOP, RAW, IPV4 and IPV6, and reads a loopback family by the default engine's rules (2 is IPv4; 24, 28, 30 are IPv6) in its byte order, re-dissecting the packet where Scapy's host-dependent bindings differ. A packet built by hand takes its link type from its first layer. - #1609: _dissected now counts each layer, and stops at FRAME_LAYER_LIMIT, so nothing is read past layer 64, behind VLAN tags included. - #1626: _ipv6_chain walks an IPv6 extension header chain over the octets, up to EXTENSION_HEADER_LIMIT and the frame bound, keeping Scapy's layers while they match. TCP, IPv4 and IPv6 behind AH, Mobility, HIP, Shim6 and AH with Next Header 0 are now read, and so is the Fragment header behind them; IPv6 reassembly takes it from the octets. New tests/toolkit/test_scapy_linktype_and_layer_bounds_1609_1626_1650_unit.py compares scapy with the default engine over 90 cases; all 6 tests fail against main and pass with the fix, on 3.14 and 3.11. Closes #1650 Part of #1609, #1626
|
Recorded: resolving the conflicts. With #1662 merged, the only conflict is in |
) The FRAME_LAYER_LIMIT section listed scapy among the engines that do not apply the bound; it now counts the layers it walks and stops at it. Part of #1609
|
Verdict: GOOD TO GO at
One new commit, |
8fc314f to
e42881a
Compare
JarryShaw
removed
the
review: good-to-go
Third-party engines are best effort (#1687), so the scapy code that exists only to match the default engine comes out: - revert #1683, #1663, #1652 and #1631, and the scapy hunks of #1614: the link-type and loopback-family walk, the frame and extension-header bounds, the IPv6 chain walk over the octets, the stops at GRE, VXLAN, PPPoE and the other layers the default engine leaves raw and at ICMP quotes, and the IPv4 header checks - keep #1624's walk (TCP behind AH or an IPv6 extension header in IPv4, and the later-fragment stop), #1605's keying by the carrying IP header and #1589's trailer cut; the walk calls _walk_extension_headers, which the pypcapfile revert keeps - re-land as guards: a zero-length frame is no IPv4 input and has no packet2dict octets (#1676); IPv4 in IPv6 ends at the Payload Length (#1623); a packet built by hand is read off its octets (#1676); ipv6_reassembly takes only its own IPv6 layer's Fragment header, and none in a later IPv4 fragment - delete the four scapy agreement modules and test_scapy_unit's default-engine checks; the guards' tests assert constants - docs: scapy applies the extension-header bound only behind IPv4 and the frame-layer bound nowhere, and its silent gaps are listed where gaps are said to be announced The scapy toolkit and engine modules, the shared modules that run scapy, and the project and tier guards pass on 3.14 (scapy 2.7.0) and 3.11 (scapy 2.8.0).
Third-party engines are best effort (#1687), so the scapy code that exists only to match the default engine comes out: - revert #1683, #1663, #1652 and #1631, and the scapy hunks of #1614: the link-type and loopback-family walk, the frame and extension-header bounds, the IPv6 chain walk over the octets, the stops at GRE, VXLAN, PPPoE and the other layers the default engine leaves raw and at ICMP quotes, and the IPv4 header checks - keep #1624's walk (TCP behind AH or an IPv6 extension header in IPv4, and the later-fragment stop), #1605's keying by the carrying IP header and #1589's trailer cut; the walk calls _walk_extension_headers, which the pypcapfile revert keeps - re-land as guards: a zero-length frame is no IPv4 input and has no packet2dict octets (#1676); IPv4 in IPv6 ends at the Payload Length (#1623); a packet built by hand is read off its octets (#1676); ipv6_reassembly takes only its own IPv6 layer's Fragment header, and none in a later IPv4 fragment - delete the four scapy agreement modules and test_scapy_unit's default-engine checks; the guards' tests assert constants - docs: scapy applies the extension-header bound only behind IPv4 and the frame-layer bound nowhere, and its silent gaps are listed where gaps are said to be announced The scapy toolkit and engine modules, the shared modules that run scapy, and the project and tier guards pass on 3.14 (scapy 2.7.0) and 3.11 (scapy 2.8.0).
1 participant
Please follow the guide below
You will be asked some questions, please read them carefully and answer honestly
Put an
xinto all the boxes [ ] relevant to your pull request (like that [x])Use Preview tab to see how your pull request will actually look like
Searched for similar pull requests
Followed the coding style (
make pylint,make mypy,make isort)make testpasses, and a test case covers the change — tests cover it; only the scapy, engine and toolkit selections were run, not the whole suiteAdded a changelog entry under
docs/source/changelog/and regeneratedCHANGELOG.md, if the change is user-visible — N/A — centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657What is the purpose of your pull request?
Tick the commit type your subject line carries.
fix— corrects a defectfeat— adds a featureperf— changes performance, not behaviourrefactor— changes neither behaviour nor performancetest— tests onlydocs— documentation onlyci— workflows or build toolingrelease— bumps the version or rolls up a distributionchore— anything elseDescription of your pull request and other information
The scapy adapters now walk a frame as the default engine dissects it:
FRAME_LAYER_LIMIT(64) orEXTENSION_HEADER_LIMIT(32) is handed over, behind VLAN tags included.Measured over 141 cases (default / dpkt / scapy, plus pypcapfile on 3.11), on 3.14.8 (scapy 2.7.0) and 3.11.15 (scapy 2.8.0), with identical cells on both: 66 scapy cells move to the default engine's inputs, none of the other 75 changes, and no other engine's cell changes. Three of the 66 are outside the issues' rows: 6in4 fragments behind Hop-by-Hop or Mobility in IPv4, and a Fragment header behind an 8-octet AH, now give IPv6 reassembly input.
New
tests/toolkit/test_scapy_linktype_and_layer_bounds_1609_1626_1650_unit.py: 6 tests, all failing onmain(20 failed, 1 passed) and passing with the fix (6 passed, 15 subtests), on both interpreters. Coverage ofpcapkit/toolkit/scapy.pygoes from 99.37% to 99.58%. Separate commits drop the scapyGapfrom the #1604 chain test and update two docs pages that listed scapy as unbounded.Closes #1650
Part of #1609, #1626