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
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