Skip to content

fix(protocols)!: refuse an MH message or MPTCP DSS option whose length does not fit its fields (#2669) - #2683

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2669-mh-mptcp-rebuild
Nov 2, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2669-mh-mptcp-rebuild

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Nov 2, 2026 •

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Some complete but malformed MH messages and MPTCP DSS options did not rebuild as captured. A Home Test with Header Len 1 rebuilt from 24 octets to 32, and its 64-octet IPv6 packet to 72. Both now refuse, as their siblings do:

  • MH.read raises ProtocolError when a message's fixed fields run past its declared Header Len while that window was fully captured. RFC 6275 has such a message discarded, and IPv6 Routing type 2 and HIP already refuse a header length too short for their fields. Inside IPv6 the octets stay a Raw payload, so the packet rebuilds byte for byte.
  • TCP._read_mptcp_dss refuses any Length other than the one RFC 8684 gives for its A/a/M/m flags (plus 2 for the checksum), as every other MPTCP subtype and fixed-length TCP option already refuses lengths outside its RFC set.

A Header Len longer than the fields round-trips as before. A capture cut inside a message, including one whose Header Len is too short, keeps its declared Header Len through replay_short_read, so it rebuilds as captured.

Breaking: MH(raw) and TCP(raw) raise ProtocolError where they used to return a mis-parse. Mutation tests change one octet at a time in every MH message type, alone and in IPv6, and try every DSS flag combination at every length; both fail on main. Shipped captures parse identically to main.

Closes #2669

@JarryShaw JarryShaw added this to the PyPCAPKit v1.6 milestone Nov 2, 2026
@JarryShaw JarryShaw added bug Issues reporting a defect (set by the bug report template; a default, not an assessment) review: running A cross-review is in flight against the current head - no verdict yet labels Nov 2, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Nov 2, 2026
@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: running A cross-review is in flight against the current head - no verdict yet labels Nov 2, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

NEEDS CHANGES at 58d3c8861, from a Sonnet cross-review. The fix is correct and nothing regressed: RFC 6275 has a Header Len below a message's minimum discarded, RFC 8684 fixes DSS's length by its flags, and every shipped capture parses identically. What is left:

  1. Pin the capture-cut property. Dropping the __short_read__ conjunct of MH.read's guard makes every cut valid Home Test raise, and no test notices. Add a test that a valid-Header-Len message cut anywhere in its window parses and rebuilds exactly.
  2. A cut inside a malformed MH (Header Len 1, captured to 8–15 octets) still rebuilds with Header Len 2. That is the same class of defect, so fold it in, or say why it needs refactor(protocols)!: make() takes enum members or ints, dropping the X_default/X_namespace/X_reversed groups #2594.
  3. Wording: the Home Test is RFC 6275 §6.1.5, not §6.1.6. Drop the timed words from the test docstrings, and the body's IPv6 figure needs correcting.
  4. Labels: MH(raw) and TCP(raw) now raise where they used to return a mis-parse, so this gets breaking and a ! subject.

The mobility-option side findings are confirmed and filed as #2684.

@JarryShaw JarryShaw added the breaking Breaks public-facing behaviour or API (apply alongside the type label) label Nov 2, 2026
…h does not fit its fields (#2669)

MH hands its message Header Len * 8 + 2 octets, and the DSS option the
octets its Length declares. A message whose fixed fields need more, or a
DSS Length short of the fields its flags give, read the rest as zeros; a
DSS Length past them left octets no field took. MH.make and
_make_mptcp_dss wrote the length the fields give, so the rebuild differed:
a Home Test with Header Len 1 rebuilt from 24 octets to 32, and its
64-octet IPv6 packet to 72.

- MH.read refuses a whole message its Header Len cuts short, as RFC 6275
  section 9.2 has it discarded and as IPv6-Route type 2 and HIP refuse a
  short header length. Inside IPv6 the octets stay a Raw payload.
- MH.make writes the Header Len a cut capture's info declares, so its
  short read replays the header as captured, short of the message or not.
- TCP._read_mptcp_dss refuses any Length but the RFC 8684 one for its
  flags, with or without the checksum, as every other registered MPTCP
  subtype refuses a length the RFC does not give it.
- Breaking: MH(raw) and TCP(raw) raise ProtocolError on such input, where
  they returned a mis-parse.
- Tests: one-octet mutations of every registered MH message type, alone
  and in IPv6, and each cut at every offset of its message; complete DSS
  options; every DSS flag combination at every length, against RFC 8684
  figure 9. The pypcapfile trial's kept-header control cuts a valid MH
  rather than shortening its Header Len.

New modules fail with pcapkit/ reverted; protocol, toolkit, foundation,
interface and guard modules pass; captures parse identically to main.
@JarryShaw
JarryShaw force-pushed the fix/2669-mh-mptcp-rebuild branch from 58d3c88 to 1b7807d Compare November 2, 2026 21:20
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Nov 2, 2026
@JarryShaw JarryShaw changed the title fix(protocols): refuse an MH message or MPTCP DSS option whose length does not fit its fields (#2669) fix(protocols)!: refuse an MH message or MPTCP DSS option whose length does not fit its fields (#2669) Nov 2, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 1b7807da5, from a Sonnet re-review. All four earlier findings are fixed:

Shipped captures are identical to main, and the MH, IPv6, TCP, MPTCP and toolkit legs pass. The option-field cut refusals the test allows are identical on main. They are not this PR's, and are filed separately.

@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 Nov 2, 2026
@JarryShaw
JarryShaw merged commit 832f54f into main Nov 2, 2026
63 checks passed
@JarryShaw
JarryShaw deleted the fix/2669-mh-mptcp-rebuild branch November 2, 2026 21:58
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Nov 2, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Nov 2, 2026
JarryShaw added a commit that referenced this pull request Nov 2, 2026
…h refusals and the Windows leg's argument file into 1.6.0 (#2449)

- Breaking: MH(raw) and TCP(raw) (#2683) raise ProtocolError for an MH
  message whose Header Len is short of its fields, or a DSS option of a
  length its flags do not give, where 1.5.x read the gap as zeros and
  rebuilt it longer; a cut MH keeps its declared Header Len. The entry gives
  the old and new results, measured against v1.5.1.
- Folded into unreleased entries: radiotap's decoded optional fields,
  Radiotap.__field__ with register_field and register_radiotap_field, the
  RadiotapField schemas and pcapkit.const.radiotap with its crawlers (#2657),
  and cut TLV items decoding zero-padded (#2690), into the Radiotap entry,
  with the IEEE 802.11 entry's Flags clause corrected; #2657's ruling on code
  to layout tables, into the conventions entry; the Windows leg's argument
  file (#2681), into the CI legs entry, as the cycle's other CI changes have
  entries.
- No entry for the test-only 42a6133.
- CHANGELOG.md regenerated; util/changelog_md.py --check passes.
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) bug Issues reporting a defect (set by the bug report template; a default, not an assessment)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(protocols): malformed MH messages and MPTCP DSS options do not rebuild as captured

1 participant