Repository navigation
fix(toolkit): scapy reads IP and TCP only in layers the default engine dissects (#1635, #1636) - #1652
Conversation
…e dissects (#1635, #1636) - ipv6_reassembly took packet['IPv6'], which can lie in an ICMP quote, a later IPv4 fragment, GRE or VXLAN, and a Fragment header from anywhere below it, e.g. the inner packet of 6in6. It now takes the first IPv6 layer the default engine dissects (_first_ipv6) and a Fragment header only from that layer's own extension header chain (_fragment_header). - _tcp_carrier and _first_ipv4 stepped over every layer that is not IP, so TCP and IP fragments inside GRE, VXLAN, PPPoE, L2TP and LLC/SNAP were handed on. _carries now lets a walk past the first layer, a VLAN tag, IP or an extension header only, and _dissected walks the layers the default engine dissects for _first_ipv4 and _first_ipv6. So scapy also reads no IP at link types 9 and 50 (with ff 03), 51, 105, 127 and 192, which the default engine leaves raw. New tests/toolkit/test_scapy_ipv6_fragment_and_encap_walk_1635_1636_unit.py fails 5 of 6 with pcapkit/ reverted, passes on 3.14 and 3.11. Closes #1635 Closes #1636
|
Verdict: GOOD TO GO at Round 3's checks: 10 tests fail with |
|
Coverage: 89.61% (unit tier, Python 3.14,
Per-file detail: the |
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).
) (#1764) Prose only: the AST of every changed module, string expression statements blanked, is identical to origin/main's. - Timed context: "this PR" names its PR (#755, #802, #836, #854, #869, #874, #922, #926); "today", "as of writing", "right now", "until now" and a "now installs" are anchored or dropped. - Stale state: closed issues no longer read as open or in flight (#576, #651/#664, #674, #527, #531), and the minting, KNOWN_DEFECTS, vendor-depth and MPTCP-parse narratives are put in the past tense where later changes retired them. - Moved or renamed names: beholder's home, HTTP._read_http_header, PCAPNGSectionRuleTests, test_new_engine_parity_runtime, and the scapy cast that #1652 removed. - Line references: about 50 file:line pointers into pcapkit/, pyproject.toml, Pipfile and sibling tests re-measured. - Counts re-derived: raise targets and nested defs in test_docstring_contract, and the four returned nested defs it names. - A pytest-subtests claim corrected to pytest's own reporting. Refs #1103
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 changeAdded 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
#1635.
ipv6_reassemblytookpacket['IPv6'], which can sit in an ICMP quote, a later IPv4 fragment, GRE or VXLAN, thenIPv6ExtHdrFragment in ipv6searched past the chain into the packet inside. It now takes the first IPv6 layer the default engine dissects (_first_ipv6,pcapkit/toolkit/scapy.py:492) and a Fragment header only from that layer's own chain (_fragment_header,:520; used at:762). The issue's "within #1609's bound" clause is not met here and stays with #1609: neither function counts headers against the 32 bound. With n Hop-by-Hop headers then a Fragment header, n = 32 or 33, the default engine gives 0 IPv6 inputs and scapy and dpkt give 2, before and after.#1636.
_tcp_carrierand_first_ipv4stepped over every layer that is not IP._carries(:389) now lets a walk past the first layer, a VLAN tag, IP or an extension header only, and drops a rejected IPv4 header or a later IPv4 fragment as before._dissected(:436) feeds_first_ipv4and_first_ipv6, and_tcp_carrierchecks each layer (:634).Measured on 3.14 (scapy 2.7.0) and 3.11 (scapy 2.8.0): scapy was wrong in 7 of #1635's 9 rows and 9 of #1636's 10; all now match the default engine. A probe of 13 #1609/#1626 rows gives identical scapy cells before and after.
New
tests/toolkit/test_scapy_ipv6_fragment_and_encap_walk_1635_1636_unit.py: 21 TCP and 25 IP-fragment cases against the default engine, six link types, and packets built by hand. Revertingpcapkit/fails 5 of its 6 tests on both Pythons (pytest10 failed, 2 passed, one subtest per link type). Before the fix scapy disagreed on 14 TCP, 12 IPv6 and 8 IPv4 cases and all six link types.Also changed, each now matching the default engine:
ff 03, and PPP_ETHER (51), scapy handed TCP over IPv4 and IPv6 to reassembly and tracing, and both fragments of an IPv4 and of an IPv6 datagram to IP reassembly, and now hands over none, as the default engine leaves those link types raw. This is user-visible for Wi-Fi captures. It is a side effect of_carries(root=True)letting exactly one layer through: there scapy dissects a second layer before IP (802.11, LLC or PPP), which is no VLAN tag, IP or extension header. The new test pins all six.Not fixed here, identical before and after:
ff 03, link type 12, and NULL (0) or LOOP (108) frames carrying IPv6 at the LinuxAF_INET6value 10. The default engine reads none of them. Scapy also misses IPv6 the default engine reads, at the BSD values 24, 28 and 30 in both NULL and LOOP frames;Closes #1635
Closes #1636