Skip to content

R1_COUNTER packs 12 octets where RFC 7401 §5.2.3 requires 16: counter is 4 octets, not the stated 8 #672

Description

@JarryShaw

R1CounterParameter.counter is a UInt32Field where RFC 7401 §5.2.3 gives the R1 generation counter 8 bytes, so the parameter packs 12 octets where the RFC total is 16 — four short, at 4 (mod 8), for both code 128 and code 129.

Found while measuring #651 (fixed in #664), which is where every number below comes from. Not fixed there: it is a field-width change with a data-model consequence and wants its own review, exactly as the SOLUTION width (#608) did in #629.

The RFC text, fetched

https://www.rfc-editor.org/rfc/rfc7401.txt, 309,319 octets, sha256 09366b9f83dc80593172304ffcf05f57afd186aacb468ac6170a8fe26944c4be. §5.2.3, verbatim:

5.2.3.  R1_COUNTER

      0                   1                   2                   3
      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |             Type              |             Length            |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |                       Reserved, 4 bytes                       |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |                R1 generation counter, 8 bytes                 |
     |                                                               |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

     Type           129
     Length         12
     R1 generation
       counter      The current generation of valid puzzles

   The R1_COUNTER parameter contains a 64-bit unsigned integer in
   network byte order, indicating the current generation of valid
   puzzles.

Reserved, 4 bytes plus R1 generation counter, 8 bytes is the Length 12 the RFC states, and "a 64-bit unsigned integer" settles the width beyond the diagram.

The code

pcapkit/protocols/schema/internet/hip.py:506:

counter: 'int' = UInt32Field()

Four octets, not eight. reserved above it is a correct PaddingField(length=4), and _make_param_r1_counter correctly writes len=12 — so the parameter declares 12 contents octets and packs 8 of them.

Both HIP parameter codes reach this one class, and both are affected. pcapkit/const/hip/parameter.py:24,27 name them R1_Counter = 128 ("v1 only, [RFC5201], Length: 12") and R1_COUNTER = 129 ("[RFC7401], Length: 12"), and RFC 5201 §5.2.3 gives the same 4 + 8 layout, so there is no version under which four octets is right.

Measured

Both codes, through the public maker, on #664's tree (pcapkit.__file__ printed and asserted under the worktree first, CPython 3.14.7):

code 128 (R1_Counter) v1: R1CounterParameter, len=12, packs 12 octets (RFC total 16), 4 mod 8
   octets: 00 80 00 0c 00 00 00 00 00 00 00 01
code 129 (R1_COUNTER) v2: R1CounterParameter, len=12, packs 12 octets (RFC total 16), 4 mod 8
   octets: 00 81 00 0c 00 00 00 00 00 00 00 01

Twelve octets: four of type and length, four of reserved, four of counter. A conformant receiver reads Length = 12 and consumes 11 + 12 - (12 + 3) % 8 = 16, so it takes four octets of whatever follows as the tail of this parameter and then reads the rest of the parameter area at a wrong offset. Writing is the same in reverse: a real peer's 16-octet R1_COUNTER has its last four octets left for the next parameter's Type.

Why it has been invisible, in three separate ways

Each of these is worth recording, because together they are why a suite that round-trips 46 of 49 HIP codes never saw it.

  1. HIP_COPIES = 2. The generator puts two copies of each parameter in a packet, and two identical four-octet shortfalls sum to eight, so the record area stays 8-aligned and the header len arithmetic comes out exact. See the note at examples/generators/options.py.
  2. The old padding rule cancelled it exactly. Before HIP parameter padding aligns the contents, not the record, so every parameter pcapkit emits is 4 (mod 8) octets #651, padding aligned the contents rather than the record, which at Length = 12 appended four surplus octets — precisely the four this parameter is missing, bringing the record to 16. That is why R1_COUNTER round-tripped at one copy on 0c7f2b7c9 and does not on fix(hip): pad HIP parameters to the record length RFC 7401 gives, not the contents (#651) #664: measured, one copy gave 4 OK of 49 codes before and 45 OK after, and R1_COUNTER is the single code that moved from OK to failing. Correcting the padding did not break it; it stopped hiding it.
  3. The round trip cannot see it at all, because pcapkit's reader consumes the same 12 octets its writer wrote. Only a comparison against the RFC's arithmetic — or a real peer — can.

It also makes the fixture walker unreliable, which is the part with the widest reach

This is the most useful thing here. #664's strongest piece of evidence is a standalone walker that parses examples/captures/options-internet.pcap using no pcapkit at all, taking its stride solely from Total Length = 11 + Length - (Length + 3) % 8. On #664's regenerated capture it reports 0 violations against 43 on the pre-fix one. That 0 is not trustworthy as a conformance statement, and this defect is why.

The walker hits the R1_COUNTER frame, reads Length = 12, advances 16, and lands four octets inside the second copy — where it reads a phantom Type = 0, Length = 0 record out of that copy's own zeroed contents, whose "padding" is four zero octets, so the zero-padding check passes and the walk ends tidily on the area boundary. It reports 92 records where 91 are real.

The reason it gets away with it is the test data: R1_COUNTER has no entry in _hip_overrides(), so the generator builds it with counter = 0, and every octet the walker mis-reads is zero. Reproduced by patching only the second copy's counter in a scratch copy of the file and re-walking:

as generated            : 92 records, 0 violations
with a non-zero counter : 92 records, 1 violations
    frame 101: type 0 padding not zeroed: aabbccdd

Two consequences for anyone measuring HIP conformance from that capture:

  • "0 violations" means "no padding violations among the records a conformant reader can find", not "the capture is conformant." Cite it that way.
  • Giving R1_COUNTER a non-zero counter in examples/generators/options.py's _hip_overrides() would stop the fixture hiding this, and is worth doing whether or not the width is fixed first — a zero-valued field cannot discriminate a width defect from a correct one, which is the same trap that made the SOLUTION width (HIP SOLUTION builder sizes with ceil(bits/4), emitting a parameter its own reader rejects #608) survive so long.

Suggested fix

Widen the field to eight octets and let the data model carry it, rather than deriving anything from the value — NumberField(length=8, signed=False) in place of UInt32Field(), matching what §5.2.3 states as a literal width for both HIP versions. Then len=12 and the packed contents agree, the record is the RFC's 16 octets, and parameter_total_len(12) == 16 needs no compensation from anywhere.

Two things to check while doing it, since both bit this parameter's neighbours:

Note that this is a wire-output and data-model change, so it wants the breaking label alongside fix on whatever PR carries it, on the same reasoning as #664.

Related

Activity

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

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions