Skip to content

fix(l2tpv2,ospf)!: keep the octets past the Length field as captured (#1455) - #1460

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1455-l2tpv2-ospf-trailer
Oct 9, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1455-l2tpv2-ospf-trailer

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Closes #1455

This is the same fix as #1431/#1435. The payload now stops at the declared length. A length-less trailer field follows it and is kept as info.trailer when it is non-empty. make(trailer=) writes it outside the length. Breaking: the payload no longer includes these octets, and that covers the OSPF crypto digest (RFC 2328 D.4.3).

case before after
L2TPv2 Length 8 + pp 10 → 8 10 → 10
OSPF Packet Length 24 + zz 26 → 24 26 → 26

Checked: ARP, RARP, Ethernet, VLAN, HTTP/2, FTP and NGAP are already exact. UDP (transport, so not mine) drops octets the same way. The two other edge cases are both short reads and belong to #1458: an L2TPv2 Offset Size that points past the data, and an unpadded final SCTP chunk.

Tests: the new module passes 8 tests and 12 subtests. Without the fix, 11 fail. Also passing: the protocols --exclude protocols/internet leg (1134 tests, OK), test_capture_roundtrip_runtime.py and tests/project (419 passed, 1 skipped). Per-frame info for examples/captures/ is identical.

…1455)

L2TPv2 and OSPF read their own Length / Packet Length but no info kept the
octets after it, so from_data dropped them. Each schema now bounds its
payload by its length and gains a length-less `trailer` PayloadField after
it; read() keeps it as info.trailer when non-empty, make(trailer=) writes it
outside the declared length, and _make_data round-trips it, as IPv4/IPv6
(#1209) and IPX (#1433) do.

Breaking: the payload no longer holds those octets (the OSPF cryptographic
digest among them), and info gains a `trailer` key when they exist.
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) breaking Breaks public-facing behaviour or API (apply alongside the type label) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Coverage: 89.51% (unit tier, Python 3.14, 325c6715b, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18795 968 2356 869 90.90%
pcapkit/corekit 2043 89 636 25 94.77%
pcapkit/dumpkit 242 2 88 2 98.79%
pcapkit/foundation 2648 98 930 48 94.80%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 16887 225 4560 189 98.03%
pcapkit/toolkit 539 74 168 3 84.58%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2342 1006 157 43.25%

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

@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 325c6715b (Sonnet cross-review; the author ran on Opus).

  • L2TPv2: all 64 L/S/O × Offset Size × payload × trailer cases rebuild byte-exact, through both from_data(info) and the dict form. make()→parse→make is also exact. I re-ran the issue repro myself: 10 of 10, with trailer=b'pp'. The header size is 6 + 2·(1 + 2·S + O) + Offset Size, which matches make.
  • OSPF: 180 cases, v2/v3, types 1–5, auth 0/1/2, are exact in object form. Packet Length 0, 10, 23 and 100 against a 30-byte packet all rebuild to 30.
  • The digest really is outside Packet Length. RFC 2328 D.4.3(d) says so, and nothing decoded the digest before this change, so no field was lost.
  • Upper layers stay exact. IPv4/UDP-1701/L2TPv2/PPP rebuilds exactly in both forms.
  • Tests. The new module has 11 failures without the fix. The rest leg runs 1134 OK, tests/project passes, and the test: capture-driven parse→rebuild and dumper round trips (#1202) #1445 capture modules give 22226 subtests passed.

Two pre-existing defects, not caused by this PR:

Nit: with the L flag clear, make(trailer=…) folds the trailer into the payload when the packet is parsed again. The schema comment says so, but the make docstring does not.

Merge note: #1454's Gap(1455) goes stale when this lands. Whichever of the two merges second removes it.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 9, 2026
@JarryShaw
JarryShaw merged commit f0230af into main Oct 9, 2026
42 checks passed
@JarryShaw
JarryShaw deleted the fix/1455-l2tpv2-ospf-trailer branch October 9, 2026 03:35
JarryShaw added a commit that referenced this pull request Oct 9, 2026
…fixed

#1454 merged before the three fixes, so its stale-entry check now fails on
main: #1455 (L2TPv2/OSPF trailer), #1456/#1457 (RPL) and #1459 (HIP fixed
bits) all come back OK. Drop those entries. The HIP fixed-bit cuts and two
RPL in-IPv6 cut55 cases now parse but zero-fill, so they move under the
#1451/#1458 short-read gaps. All four edge modules pass.
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 9, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaks public-facing behaviour or API (apply alongside the type label) fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(link): L2TPv2 and OSPF drop the octets past their own Length field on rebuild

1 participant