You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Every padding site in the HIP schemas computes the pad from the contents length alone — (8 - (pkt['len'] % 8)) % 8 — which aligns len rather than len + 4. RFC 7401 §5.2.1 requires the total length of a TLV parameter, type and length fields included, to be a multiple of 8 octets. Aligning the contents instead puts every parameter pcapkit writes at 4 (mod 8), never 0 (mod 8), for every possible Length value.
The sharp part: pcapkit round-trips these records perfectly, because its writer and its reader share the same error. A real peer does not share it. So no round-trip test can detect this, however many parameters it covers — the comparison has to be against the RFC, not against pcapkit. That is why this survived a suite that already round-trips 29 of the 49 HIP parameter codes.
All of the encoded TLV parameters have a length (that includes the
Type and Length fields), which is a multiple of 8 bytes. When
needed, padding MUST be added to the end of the parameter so that the
total length is a multiple of 8 bytes. This rule ensures proper
alignment of data. Any added padding bytes MUST be zeroed by the
sender, and their values SHOULD NOT be checked by the receiver.
The Length field indicates the length of the Contents field (in
bytes). Consequently, the total length of the TLV parameter
(including Type, Length, Contents, and Padding) is related to the
Length field according to the following formula:
Total Length = 11 + Length - (Length + 3) % 8;
where % is the modulo operator.
The RFC states the arithmetic explicitly, so there is nothing here to infer from a diagram. Length is the contents length — "Length of the Contents, in bytes, excluding Type, Length, and Padding" — and the total is what must be a multiple of eight.
The sites
git grep -c '% 8' over the two HIP modules — the trailing number is a match count, not a line number — and every one of those 95 matches is a padding computation:
All 49 in the protocol module are the reported record length in the data model, e.g. pcapkit/protocols/internet/hip.py:796:
length=4+schema.len+ (8-schema.len%8) %8,
Neither expression has a + 4 inside the modulus. Grepping for anything else: grep -n '% 8' … | grep -v <that expression> returns nothing in either file, so there are no other % 8 uses and no site that already gets it right.
Measured
On origin/main (a62aed134), CPython 3.14.7, importing from an immutable git archive snapshot whose pcapkit.__file__ is printed and asserted rather than trusting the editable install:
A len=8 SOLUTION emits twelve octets where sixteen are required, exactly as reported. The defect is not confined to that case, and it is not a consistent offset in one direction — pcapkit both under-pads and over-pads depending on the residue:
L | L%8 | pcapkit pad | pcapkit total | RFC pad | RFC total | agree?
0 | 0 | 0 | 4 | 4 | 8 | no
1 | 1 | 7 | 12 | 3 | 8 | no
4 | 4 | 4 | 12 | 0 | 8 | no
8 | 0 | 0 | 12 | 4 | 16 | no
12 | 4 | 4 | 20 | 0 | 16 | no
16 | 0 | 0 | 20 | 4 | 24 | no
20 | 4 | 4 | 28 | 0 | 24 | no
pcapkit total is a multiple of 8 for any L? False
pcapkit agrees with RFC for any L in 0..63? False
Those two False results are the whole issue in one line. Across every contents length from 0 to 63 there is not one for which pcapkit's record length is a multiple of eight, and not one for which it matches the RFC.
L = 4 is the case that shows it is not merely a shortfall. SEQ carries a single 4-octet Update ID, so it is complete in eight octets and needs no padding at all — and pcapkit appends four:
SEQ is not a corner of the parameter space. RFC 7401 §5.3.5 states that "An UPDATE packet without either a SEQ or an ACK parameter is invalid", so one of the two is on every UPDATE, and both carry contents that are a multiple of four octets — precisely the residue at which pcapkit adds four octets that must not be there.
The consequence on pcapkit's own reader, same run:
One SOLUTION per packet, built then re-parsed by pcapkit:
REJECTED: ProtocolError: HIPv2: invalid format
Two SOLUTIONs per packet:
built 96 octets
parsed OK <- writer and reader share the same padding error
One parameter is 4 (mod 8) and the header length field cannot represent it; two are 0 (mod 8) and it can. The error cancels in pairs, which is why the symptom looks like an odd restriction on packet shape rather than like a padding bug.
The codebase already documents this
examples/generators/options.py:949-962 describes the defect in terms and works around it, which is why the round-trip suite is green:
#: How many copies of the parameter under test go in one packet.
#:
#: Two, not one, and this is not padding for its own sake. ``HIP.make``
#: computes the header's ``len`` field as ``total_length // 8 + 4``, which is
#: only lossless when the parameter octets are a multiple of eight -- and the
#: parameter padding rule pads the *contents* to eight, ignoring the four-octet
#: type-and-length header, so a single parameter is always ``4 (mod 8)`` and
#: always loses those four octets. ``_read_hip_param`` then checks the value
#: exactly and rejects the packet. Two copies sum to a multiple of eight, so
#: the arithmetic is exact and the parameter constructors become reachable:
#: measured, this is the difference between 4 and 29 of the 49 codes
#: round-tripping. The single-parameter case is not lost -- it is what
#: ``hip-parameter-single`` in the test's expected-failure table records.
HIP_COPIES = 2
"the parameter padding rule pads the contents to eight, ignoring the four-octet type-and-length header" is this defect, already written down. It was read as a constraint to be accommodated rather than as a bug, and HIP_COPIES = 2 is the accommodation — it is the difference between 4 and 29 codes round-tripping. The hip-parameter-single expected-failure entry is the same defect recorded as a known limitation.
Consequence
Two directions, both interoperability failures rather than crashes.
Writing: every HIP packet pcapkit produces carries parameters whose total length violates §5.2.1. A conformant receiver reads Length, computes 11 + Length - (Length + 3) % 8, and consumes that many octets — for a len=8 SOLUTION it consumes sixteen where pcapkit wrote twelve, taking four octets of the following parameter as padding and then reading the rest of the parameter area at a wrong offset.
Reading: given a conformant packet, pcapkit consumes 4 + Length + (8 - Length % 8) % 8 and lands in the wrong place by the same amount. At Length = 20 it consumes 28 octets of a 24-octet record, so the next parameter's type is read four octets into the wrong field.
Neither shows up in this repository's fixtures, because every HIP fixture was produced by this library and is self-consistent.
Fixing it rewrites every HIP fixture
The padding is on the wire, so correcting the formula changes the octets of every HIP parameter in every synthesised capture, and the header len field along with them. examples/generators/make_samples.py regenerates them wholesale, but the diff is the entire HIP fixture set, and HIP_COPIES = 2 and the hip-parameter-single expected-failure entry both become removable at the same time. That is a wide change and wants to be its own, which is why #629 measured it and deliberately left it alone.
Re-verified here from scratch against a62aed134 before filing: the RFC fetched rather than paraphrased, the grep counts re-run, and the arithmetic executed rather than reasoned about.
Every padding site in the HIP schemas computes the pad from the contents length alone —
(8 - (pkt['len'] % 8)) % 8— which alignslenrather thanlen + 4. RFC 7401 §5.2.1 requires the total length of a TLV parameter, type and length fields included, to be a multiple of 8 octets. Aligning the contents instead puts every parameter pcapkit writes at 4 (mod 8), never 0 (mod 8), for every possibleLengthvalue.The sharp part: pcapkit round-trips these records perfectly, because its writer and its reader share the same error. A real peer does not share it. So no round-trip test can detect this, however many parameters it covers — the comparison has to be against the RFC, not against pcapkit. That is why this survived a suite that already round-trips 29 of the 49 HIP parameter codes.
The RFC text, fetched
Fetched from
https://www.rfc-editor.org/rfc/rfc7401.txt(309,319 octets, sha25609366b9f83dc80593172304ffcf05f57afd186aacb468ac6170a8fe26944c4be). RFC 7401 §5.2.1, "TLV Format", verbatim:The RFC states the arithmetic explicitly, so there is nothing here to infer from a diagram.
Lengthis the contents length — "Length of the Contents, in bytes, excluding Type, Length, and Padding" — and the total is what must be a multiple of eight.The sites
git grep -c '% 8'over the two HIP modules — the trailing number is a match count, not a line number — and every one of those 95 matches is a padding computation:All 46 in the schema module are the same expression, once per parameter class, e.g.
pcapkit/protocols/schema/internet/hip.py:325:All 49 in the protocol module are the reported record length in the data model, e.g.
pcapkit/protocols/internet/hip.py:796:Neither expression has a
+ 4inside the modulus. Grepping for anything else:grep -n '% 8' … | grep -v <that expression>returns nothing in either file, so there are no other% 8uses and no site that already gets it right.Measured
On
origin/main(a62aed134), CPython 3.14.7, importing from an immutablegit archivesnapshot whosepcapkit.__file__is printed and asserted rather than trusting the editable install:A
len=8SOLUTION emits twelve octets where sixteen are required, exactly as reported. The defect is not confined to that case, and it is not a consistent offset in one direction — pcapkit both under-pads and over-pads depending on the residue:Those two
Falseresults are the whole issue in one line. Across every contents length from 0 to 63 there is not one for which pcapkit's record length is a multiple of eight, and not one for which it matches the RFC.L = 4is the case that shows it is not merely a shortfall. SEQ carries a single 4-octet Update ID, so it is complete in eight octets and needs no padding at all — and pcapkit appends four:SEQ is not a corner of the parameter space. RFC 7401 §5.3.5 states that "An UPDATE packet without either a SEQ or an ACK parameter is invalid", so one of the two is on every UPDATE, and both carry contents that are a multiple of four octets — precisely the residue at which pcapkit adds four octets that must not be there.
The consequence on pcapkit's own reader, same run:
One parameter is 4 (mod 8) and the header length field cannot represent it; two are 0 (mod 8) and it can. The error cancels in pairs, which is why the symptom looks like an odd restriction on packet shape rather than like a padding bug.
The codebase already documents this
examples/generators/options.py:949-962describes the defect in terms and works around it, which is why the round-trip suite is green:"the parameter padding rule pads the contents to eight, ignoring the four-octet type-and-length header" is this defect, already written down. It was read as a constraint to be accommodated rather than as a bug, and
HIP_COPIES = 2is the accommodation — it is the difference between 4 and 29 codes round-tripping. Thehip-parameter-singleexpected-failure entry is the same defect recorded as a known limitation.Consequence
Two directions, both interoperability failures rather than crashes.
Writing: every HIP packet pcapkit produces carries parameters whose total length violates §5.2.1. A conformant receiver reads
Length, computes11 + Length - (Length + 3) % 8, and consumes that many octets — for alen=8SOLUTION it consumes sixteen where pcapkit wrote twelve, taking four octets of the following parameter as padding and then reading the rest of the parameter area at a wrong offset.Reading: given a conformant packet, pcapkit consumes
4 + Length + (8 - Length % 8) % 8and lands in the wrong place by the same amount. AtLength = 20it consumes 28 octets of a 24-octet record, so the next parameter's type is read four octets into the wrong field.Neither shows up in this repository's fixtures, because every HIP fixture was produced by this library and is self-consistent.
Fixing it rewrites every HIP fixture
The padding is on the wire, so correcting the formula changes the octets of every HIP parameter in every synthesised capture, and the header
lenfield along with them.examples/generators/make_samples.pyregenerates them wholesale, but the diff is the entire HIP fixture set, andHIP_COPIES = 2and thehip-parameter-singleexpected-failure entry both become removable at the same time. That is a wide change and wants to be its own, which is why #629 measured it and deliberately left it alone.Notes
lenformula; this is the padding, in all 95 sites.a62aed134before filing: the RFC fetched rather than paraphrased, the grep counts re-run, and the arithmetic executed rather than reasoned about.375e9d411,a62aed134's parent, where HIP SOLUTION builder sizes with ceil(bits/4), emitting a parameter its own reader rejects #608 is still unfixed — the schema module is byte-identical between the two commits (sha256b56203d2a044e69a91743c812bf801af619494b7323e89f34b358135ec74814d), so fix(hip): size the SOLUTION parameter as two whole-octet fields (#608) #629 did not touch any padding site.lenarithmetic, where this was found). The other three side-findings from that work are Re-serialising a parsed HIP PUZZLE or SOLUTION loses leading zero octets: a len=20 parameter rebuilds as len=6 #653 (re-serialising loses leading zero octets), SOLUTION's Reserved octet is written as a PUZZLE Lifetime, and lifetime=0 escapes a bare ValueError from math.log2 #654 (SOLUTION'sReservedoctet written as aLifetime) and Neither the HIP PUZZLE nor SOLUTION builder consults version when sizing, so HIPv1 rejects what it builds #655 (neither builder consultsversion). This one is the only one of the four that affects every HIP parameter rather than PUZZLE and SOLUTION alone, and the only one whose fix rewrites the fixtures.