Skip to content

MH: _make_opt_pad grows a PadN option by two octets per round trip #354

Closed

Description

@JarryShaw

Summary

MH._read_opt_pad reports the data model's length as the total option size (clen + 2), matching every other _read_opt_* in the module. MH._make_opt_pad then feeds that same value back as the wire Option Length. Per RFC 6275 the Option Length field excludes the Type and Length octets, so every parse → re-serialise round trip grows a PadN option by exactly two octets, and turns a 1-octet Pad1 into a 3-octet PadN.

Location

Read side — pcapkit/protocols/internet/mh.py:1085 (_read_opt_pad), lines 1128-1136:

        if code == Enum_Option.Pad1:
            size = 1
        else:
            size = clen + 2

        data = Data_PadOption(
            type=schema.type,
            length=size,
        )

Make side — pcapkit/protocols/internet/mh.py:2178 (_make_opt_pad), lines 2193-2194 and 2205-2208:

        if  option is not None:
            length = option.length
...
        return Schema_PadOption(
            type=type,
            length=length,
        )

length is passed straight through, with no - 2 and no Pad1 special case.

Observed symptom

A Mobility Header carrying one PadN (Option Length 6, i.e. 8 octets of padding) is 16 octets. Parsing and re-serialising it yields 18 octets, and the Mobility Header Header Len field silently changes from 1 to 2:

in : b';\x01\x00\x00\x00\x00\x00\x00\x01\x06\x00\x00\x00\x00\x00\x00' 16 octets
opt: PadOption(type=1, length=8)
out: b';\x02\x00\x00\x00\x00\x00\x00\x01\x08\x00\x00\x00\x00\x00\x00\x00\x00' 18 octets

Repeating read → make at the option level shows the growth compounding, two octets each time:

round 0  wire=b'\x01\x06\x00\x00\x00\x00\x00\x00'                          len=8
round 1  wire=b'\x01\x08\x00\x00\x00\x00\x00\x00\x00\x00'                  len=10   (schema.length=8,  data.length in=8)
round 2  wire=b'\x01\n\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'            len=12   (schema.length=10, data.length in=10)
round 3  wire=b'\x01\x0c\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'  len=14   (schema.length=12, data.length in=12)

The same asymmetry also changes a Pad1's option type. _read_opt_pad gives a Pad1 length=1; _make_opt_pad sees length != 0, warns, and rewrites it as a PadN:

Pad1 schema packs to b'\x00' (1 octet)
_read_opt_pad -> <PadOption type=<Option.Pad1: 0>, length=1>  (.length = 1)
_make_opt_pad -> PadOption(type=<Option.PadN: 1>, length=1), packs to b'\x01\x01\x00' (3 octets)
warning raised: ProtocolWarning: MH: [OptNo 0] invalid format

Minimal reproduction

import io

from pcapkit.const.mh.option import Option
from pcapkit.protocols.internet.mh import MH

# MH Binding Refresh Request whose 8-octet option area is one PadN,
# Option Length = 6 -> 8 octets of padding (RFC 6275 s6.2.3: N-2).
raw = bytes([59, 1, 0, 0, 0, 0, 0, 0]) + bytes([0x01, 0x06]) + bytes(6)

parsed = MH(io.BytesIO(raw), len(raw))
print('in :', raw, len(raw), 'octets')
print('opt:', parsed.info.options[Option.PadN])

again = bytes(MH(type=parsed.info.type, next=parsed.info.next, data=parsed.info))
print('out:', again, len(again), 'octets')

Correct semantics

RFC 6275, Mobility Support in IPv6:

  • §6.2.1 Format — "Option Length … 8-bit unsigned integer, representing the length in octets of the mobility option, not including the Option Type and Option Length fields."
  • §6.2.2 Pad1 — "NOTE! the format of the Pad1 option is a special case -- it has neither Option Length nor Option Data fields. The Pad1 option is used to insert one octet of padding…"
  • §6.2.3 PadN — "For N octets of padding, the Option Length field contains the value N-2, and the Option Data consists of N-2 zero-valued octets."

So for a data model length of N (total octets): PadN's wire Option Length must be N - 2, and Pad1's must be absent, i.e. 0 in the schema, never 1.

What a fix would need to touch

  • MH._make_opt_pad in pcapkit/protocols/internet/mh.py (line 2178) — convert the data model's total length into the wire Option Length in the option is not None branch, mirroring _read_opt_pad: Pad1 total 1 → wire 0, PadN total N → wire N-2. This must happen before the type-normalisation checks at lines 2196-2203, which currently misfire on a Pad1 because they see length == 1.

  • tests/protocols/internet/test_mh_unit.py:668-671 currently encodes the buggy expectation and would need updating:

                self.assertEqual(proto._make_opt_pad(
                    Option.Pad1,
                    data.PadOption(type=Option.PadN, length=4),
                ).length, 4)

    With today's code that returns 4 and passes; with the wire length computed correctly it returns 2.

  • No other _make_opt_* is affected. Every other one derives the wire length from the actual payload (length=2, length=16, length=len(data), …); _make_opt_pad is the only one that propagates the data model's total verbatim.

Related, while in this code

_read_opt_pad at pcapkit/protocols/internet/mh.py:1125-1126 rejects a PadN with Option Length 0, but RFC 6275 §6.2.3 makes N=2 (Option Length 0, no Option Data) lawful. Noted from reading the RFC alongside the code; not measured, and not part of the defect above.

Observed vs inferred

  • Observed by running code on the current tree: the 16 → 18 octet public round trip; the 8 → 10 → 12 → 14 compounding growth with the wire bytes above; the Pad1 → 3-octet PadN type change and its ProtocolWarning; that test_mh_unit.py:668-671 passes as written and the MH unit suite is green (10 passed).
  • Inferred by reading code: that a corrected _make_opt_pad returns 2 for that assertion — computed from the arithmetic, not by patching the source; the specific shape a fix should take; the RFC-lawfulness of a 2-octet PadN noted above.

Activity

  1. JarryShaw commented on Sep 15, 2026

    @JarryShaw
    OwnerAuthor

    Fixed by #398, verified on main at eedebde6d.

    The root cause was a unit mismatch between the two representations: Data_PadOption.length counts the whole option, while Schema_PadOption.length is the Option Length field — the count of padding octets after the two-octet header, and absent entirely for a Pad1. _make_opt_pad copied one into the other unconverted, so every round trip grew the option by two octets.

    _make_opt_pad now converts explicitly (option.length - 2, or 0 for Pad1), and the docstring records why. Re-making a parsed option is now length-preserving:

    Pad1   whole-option length=1 -> packs to 1 octets  OK
    PadN   whole-option length=4 -> packs to 4 octets  OK
    PadN   whole-option length=6 -> packs to 6 octets  OK
    PadN   whole-option length=8 -> packs to 8 octets  OK
    

    No warnings raised (checked with simplefilter('error')).

    Worth noting the same PR found HOPOPT and IPv6-Opts were ignoring the data model argument altogether, silently collapsing any re-made padding option to a single Pad1; both now convert correctly. And the compounding bug behind this one — _make_opt_pad being called with a PadN two octets too long while the 8-octet alignment ignored the fixed header — meant HOPOPT(next=…, options=[…]) raised ProtocolError for any option set. That is fixed too.

  2. added
    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)
    on Sep 22, 2026
  3. added this to the 1.5 milestone on Oct 6, 2026
  4. moved this to Done in PyPCAPKiton Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions