Skip to content

fix(hip): HOST_ID packs DI-Type/DI-Length as 4 octets where RFC 7401 has one 16-bit word - #1132

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1118-hip-host-id-di
Oct 6, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1118-hip-host-id-di

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort) — ran isort/mypy; both already fail on schema/internet/hip.py at origin/main (measured), and this diff adds no imports
  • make test passes, and a test case covers the change — ran tests/protocols/internet/test_hip_host_id_di_width_unit.py, tests/protocols/test_option_roundtrip_unit.py, tests/protocols/internet/test_hip_unit.py, the two sibling HIP width modules, and tests/project
  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A, added centrally after the wave

What is the purpose of your pull request?

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • chore — anything else

Description of your pull request and other information

Closes #1118.

RFC 7401 §5.2.9 packs DI-Type (4 bits) and DI Length (12 bits) into one 16-bit word between HI Length and Algorithm; the schema's namespace said so, the BitField's length did not. length=4 → length=2.

Probe, ECDSA/NIST P-256 with a 4-octet key — before / after:

made: 02c1000c0006 00000000 0007000100000000  18 octets, len=12   # two surplus octets
made: 02c1000c0006 0000     0007000100000000  16 octets, len=12
hand-built RFC record → algorithm 1 (the curve) / algorithm 7

Rebased onto c24df4fbe after #1128 landed. With #1128's HIP_TRANSFORM fix and this one, no HIP case fails at either setting of HIP_COPIES — measured 49 OK of 49 at one copy and at two — so EXPECTED_FAILURES loses its last HIP entry and the block goes with it; the HIP_COPIES note records that instead of calling HOST_ID a gap. make_samples.py now leaves out 3 cases, none of them HIP, and options-internet.pcap carries both 0x02c1 (705) and 0x0241 (577).

Fails without the fix — reverting length=2 → length=4 turns all three new tests red: ProtocolError: HIPv2: invalid format, '02c1000e00061002000000070001aabbccdd6162…' != '02c1000e0006100200070001aabbccdd6162…', and HIAlgorithm.NULL_ENCRYPT != HIAlgorithm.ECDSA. With the fix: 3 passed; roundtrip 6 passed, 360 subtests; test_hip_unit.py 34 passed, 168 subtests; tests/project 385 passed, 1 skipped, 1289 subtests.

@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 6, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 7693f2099: GOOD TO GO (ran on Sonnet; author Opus)

  • RFC 7401 §5.2.9: DI-Type and DI Length share one 16-bit word, so length=2 is right.
    • 144 hand-built HOST_ID records (ECDSA and RSA, DI lengths 0–5) parse and re-pack correctly on this PR.
    • main misreads the algorithm in 66 cases.
  • _make_param_host_id: len = 6 + hi_len + di_len matches the RFC. Padding to the 8-octet boundary matches an independent formula across 66 combinations.
  • Regression tests: with the schema reverted, the 3 new tests fail.
  • Existing tests: test_option_roundtrip_unit (360 subtests) and test_hip_unit (34) pass.
  • BitField audit: none of the 106 schema BitFields overflows. Four reserved-bit fields checked against their RFCs are correct: MH 833, 1914, 2357 and ipv6_route 206.

Merge order with #1128: the two PRs conflict in options.py and test_option_roundtrip_unit.py, both textually and semantically. Whichever merges second has to rebase:

  • delete both HIP EXPECTED_FAILURES entries;
  • rewrite the HIP_COPIES note, which would otherwise call HOST_ID the remaining gap.

@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 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 88.75% (unit tier, Python 3.14, c25424888, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18757 1035 2342 846 90.23%
pcapkit/corekit 1873 91 578 22 94.33%
pcapkit/dumpkit 136 0 40 0 100.00%
pcapkit/foundation 2425 145 842 34 92.56%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15659 188 3944 163 98.18%
pcapkit/toolkit 487 65 144 1 85.42%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2359 1006 158 42.84%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one and removed review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 6, 2026
…has one 16-bit word

RFC 7401 section 5.2.9 packs DI-Type (4 bits) and DI Length (12 bits) into one
16-bit word between HI Length and Algorithm, and the schema's namespace says so
-- (0, 4) plus (4, 12). The BitField declared four octets for it, so a made
HOST_ID carried two surplus zero octets (18 against len=8, which HIP then
rejected as "invalid format"), and a conformant record was read two octets late,
taking the ECDSA curve as Algorithm.

The HOST_ID entry of EXPECTED_FAILURES goes with it -- the case now round-trips
-- as does the generator's note recording it as one of two gaps a second copy
could not help.
@JarryShaw
JarryShaw force-pushed the fix/1118-hip-host-id-di branch from 7693f20 to c254248 Compare October 6, 2026 20:42
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 6, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on c25424888: GOOD TO GO (ran on Sonnet; author Opus, round 2)

Delta review of the rebase after #1128.

Unverified prose detail: the note's len=8 figure for the old HOST_ID record. Round 1 measured len=12 for a 4-octet key. This does not affect the fix.

@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 6, 2026
@JarryShaw
JarryShaw merged commit a7ef878 into main Oct 6, 2026
40 checks passed
@JarryShaw
JarryShaw deleted the fix/1118-hip-host-id-di branch October 6, 2026 22:31
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(hip): HOST_ID packs DI-Type/DI-Length as 4 octets where RFC 7401 has one 16-bit word

1 participant