fix(hip,ipv6-route): stop dropping the ENCRYPTED IV and RPL's pack-time cast (#556)

[JarryShaw]

Closes #556. Two small defects flagged in review earlier this release and never filed.

1. HIP._make_param_encrypted dropped the IV. hip.py:3533 passed cipher= to Schema_EncryptedParameter, a keyword the schema does not declare. Neither spelling lands: __cipher__ is a pre_unpack packet-context key, not a field, so cipher= and __cipher__ both draw UnknownFieldWarning and are dropped. Every AES-cipher ENCRYPTED parameter built through make therefore packed without its IV. Flagged in #430's review and re-flagged in #434 as "carried across unchanged"; it survived because the schema warns rather than raising.

The fix sets the resolved cipher as a plain attribute after construction, so pack()'s packet.update(self.__dict__) carries it into pre_unpack, which now trusts it ahead of its own HIP_CIPHER sibling lookup — a standalone make call has no options list for that lookup to search anyway.

2. RPL.post_process cast a list as bytes. schema/internet/ipv6_route.py:207 cast self.addresses to bytes unconditionally, but it runs on every Schema.pack, not only after a parse, and a schema built via make (IPv6_Route._make_data_type_rpl) still holds addresses as list[bytes]. Slicing and re-joining that list as the concatenated buffer a parse produces raised at pack time. Reproduced during #489's review and left with only a code comment. The fix skips SRH decompression when addresses is not bytes.

Both defects share the shape that made them invisible: the schema layer warns where it could raise. Whether that should be fatal during make is deliberately not decided here — it has an eleven-site precondition in _make_mptcp_* and belongs in its own issue.

Fixing defect 2 unmasks two pre-existing defects rather than introducing one. With RPL.post_process no longer raising, the RPL round-trip case now reaches IPv6_Route._read_data_type_rpl's own length guard, pcapkit/protocols/internet/ipv6_route.py:612 (if header.length % 16 != 0:), and fails there instead: ProtocolError: IPv6-Route: [TypeNo 3] invalid format. That guard treats header.length (Hdr Ext Len) as octets when it is actually in 8-octet units — the same unit confusion #487 fixed for Source Route and Type 2. But the header the guard is judging is not well-formed either way: RPL's fixed area (cmpr_i + cmpr_e + pad) packs to 5 octets against the 4 RFC 6554 specifies (echoed in the method's own docstring diagram) — measured, it constructs to 41 octets against the 48 its own Hdr Ext Len of 5 declares. Filed as #564. Neither is fixed here: RPL addresses are also variable-length under cmpr_i/cmpr_e, so no fixed bound is obviously right even once the units and the field widths are both correct, and nothing has been checked against a real RPL capture. tests/protocols/test_option_roundtrip_unit.py's EXPECTED_FAILURES entry for ipv6-route-type/RPL_Source_Route_Header is re-pointed at these, not deleted, since the case still does not construct — only the reasons changed, and the entry now names both.

9 files, +190/-18, one commit, on top of 8cfd6ab01.

Provenance, verified rather than asserted. Earlier drafts of this PR body said the test evidence was missing because the authoring session was killed by a host OOM (from this branch's own coverage run -m pytest tests, which reached 36.9 GB RSS). That is no longer true. I reproduced both halves of the "unmasked, not introduced" claim directly, using plain pytest (never coverage run -m pytest tests) and reading exit codes from files rather than summary lines. A cross-review on a separate model reviewed this PR independently and reports having reproduced the same unmasking and the #564 field-width measurement on its own, with exit codes in its own comment on this PR:

• On origin/main at 8cfd6ab01 (before this PR's fix), pytest tests/protocols/test_option_roundtrip_unit.py -k round_trip_is_identity exits 0 — the RPL case fails there exactly as the old EXPECTED_FAILURES entry recorded (does not appear to be an IPv4 or IPv6 address).
• With this PR's fix applied but the old, unedited EXPECTED_FAILURES entry still in place, the same run exits 1 with exactly one SUBFAILED (ipv6-route-type/RPL_Source_Route_Header, 321 of 322 subtests passing), reproducing the CI failure verbatim: AssertionError: 'does not appear to be an IPv4 or IPv6 address' not found in 'ProtocolError: IPv6-Route: [TypeNo 3] invalid format'.
• With the EXPECTED_FAILURES entry re-pointed as described above, the same run exits 0 again, with all 322 subtests passing.
• The full CI "unit tier" command (pytest -q --ignore=tests/integration --ignore-glob='*_runtime.py' --ignore-glob='*_regression.py') exits 0: 1121 passed, 5 skipped, 256 warnings, 2704 subtests passed.
• Building the RPL schema directly via IPv6_Route.make() and packing it (bypassing the read-back guard) confirms the RPL schema's fixed area is 5 octets where RFC 6554 gives 4, so a built header is wider than its own Hdr Ext Len #564 numbers above: Hdr Ext Len field = 5, actual packed total = 41 octets, (Hdr Ext Len + 1) * 8 = 48 octets.

EXPECTED_FAILURES had 54 entries before this PR and has 54 after — this re-points one entry, it does not add or delete any.

[JarryShaw]

❌ NEEDS CHANGES

Cross-model review (Opus 5; the PR was authored on Sonnet), head 9025729f2.

Both code fixes are correct, and I independently supplied the fails-without-the-fix proof the description says is missing. The EXPECTED_FAILURES re-point is right to be a re-point rather than a deletion — I proved the ProtocolError is unmasked, not introduced — and its status, fragment and cited line are all correct.

Two changes needed, one substantive:

1. The entry's defect prose asserts something measurably false. It reads "the reader rejects a well-formed header". The header this case actually builds is not well-formed: it packs to 41 octets while its own Hdr Ext Len of 5 declares 48, because the RPL schema's fixed area is 5 octets (cmpr_i 1 + cmpr_e 1 + pad 3) where RFC 6554 — per the ASCII diagram in _read_data_type_rpl's own docstring — specifies 4 (CmprI 4b | CmprE 4b | Pad 4b | Reserved 20b = 32 bits). So there is a third pre-existing defect sitting behind the % 16 guard, and as written this entry tells the next person the header is fine and only the guard is wrong. Someone who fixes % 16 on the strength of it will find the case still red and no note explaining why. Please reword, and file the octet-width defect as its own issue so the entry can point at it.

2. The description's stated shape is stale. It says "8 files, +166/-11, one commit, on top of 4c6b12189". Actual: 9 files, +180/-18, one commit, on top of 8cfd6ab01. The commit shape itself is now correct — the earlier merge commit is gone and this is properly rebased to one commit.

Also worth relaxing: the provenance caveat says "the fails-without proof this repository normally requires is therefore missing". It is no longer missing — I obtained it for both defects and both exit codes are in the detailed comment below.

Nothing else I checked disputes the change. Detail follows.

[JarryShaw]

Detailed cross-review — #561 @ 9025729f2

Reviewer: Opus 5, per the standing rule that an agent-raised PR is cross-reviewed on a different model than the one that wrote it (this PR: Sonnet). Every measurement below was run on Python 3.14.7 with PYTHONSAFEPATH=1 and PYTHONPATH pinned to my worktree, asserting pcapkit.__file__ resolves inside it before importing anything else. Exit codes are read out of files, never off a summary line — pytest 9.1.1 prints a failing subtest's parent as PASSED, so a run with subtest failures can show zero FAILED lines.

The force-push from c0705bc84 to 9025729f2 changed only tests/protocols/test_option_roundtrip_unit.py, so the source-level measurements I took against the earlier head carry over verbatim; I re-ran both probes against 9025729f2 to confirm.

---

1. Defect 1 was fixed on the correct side — and the alternative would have moved the bug, not fixed it

This was the claim I most wanted to falsify, because picking the wrong side here relocates the defect. I was briefed that the schema field is __cipher__ and that the maker's keyword was simply misspelled. That framing is wrong, and the PR's is right.

EncryptedParameter's declared fields are exactly:

['type', 'len', 'reserved', 'iv', 'data', 'padding']

Neither cipher nor __cipher__ is among them. __cipher__ is only ever read, by the iv field's ConditionalField predicate. Constructing with each spelling in turn:

constructor keyword	warning drawn	IV in packed bytes
cipher=Cipher.AES_128_CBC	UnknownFieldWarning: 'cipher' is not a valid field name	absent
__cipher__=Cipher.AES_128_CBC	UnknownFieldWarning: '__cipher__' is not a valid field name	absent

Identical. Re-spelling the maker's keyword to __cipher__ would have changed nothing, and declaring it as a field would put a cipher ID on the wire that ENCRYPTED does not carry — the ID lives in the preceding HIP_CIPHER parameter. Setting it as a plain attribute so pack()'s packet.update(self.__dict__) carries it into pre_unpack is the only one of the three that works. It is also symmetric with the parse path, where post_process already does self.cipher = packet['__cipher__'].

The defect was worse than the description states. The dropped keyword did not merely omit the IV; it produced an internally inconsistent parameter:

02 81 00 18 | 00 00 00 00 | 'DATA' | 00 00 00 00
^^^^^ ^^^^^
type  len = 0x18 = 24

len declares 24 octets of value while only 8 are present. That is not a parameter missing a field, it is an unparseable one.

One thing I checked and found correct rather than suspicious: pre_unpack's new early return does packet.pop('cipher'), and Schema.pack calls it on a dict the caller may own. The pop is load-bearing, not sloppy — with two ENCRYPTED parameters in one packet where only the first carries a resolved cipher, leaving cipher in the shared context would silently hijack the second one's sibling lookup. Popping it is what keeps them independent.

2. Defect 2's test does exercise pack()

I was asked to confirm this specifically, since constructing an RPL schema does not reproduce the fault. It does: the test calls bytes(rpl_schema) and bytes(header), and Schema.__bytes__ calls self.pack() when __updated__ is set, which __update__ sets at construction. Both the direct schema level and the public make entry point are covered.

3. Fails-without-the-fix proof — supplied, since the description says it is missing

Reverted each fix in isolation, with the PR's tests in place. Restored from byte-identical copies afterwards and confirmed git status clean and md5sums matching in both cases.

tree	test	exit code (from file)	failure
PR head, both fixes	both affected test files	0 — 75 passed, 146 subtests	—
HIP fix reverted to main	-k encrypted_preserves_iv	1	UnknownFieldWarning("'cipher' is not a valid field name") + ProtocolWarning('HIP: [ParamNo 641] missing HIP_CIPHER parameter')
RPL fix reverted to main	-k rpl_packs_a_multi_address	1	ValueError: [b' \x01\r\xb8…\x01', b' \x01\r\xb8…\x02'] does not appear to be an IPv4 or IPv6 address

Both defects are real and both tests genuinely pin them.

Minor: the HIP test's first assertion is assertEqual(caught, []), so in the reverted state it fails on the warning and never reaches assertIn(iv, packed) — the assertion that names the actual harm. Asserting the IV first would make the failure message say "the IV is gone" rather than "a warning fired". Not blocking; I confirmed the IV loss independently above.

4. The ProtocolError was unmasked, not introduced — proven

This is the part of the PR I was most sceptical of, and it holds up.

tree	exit code (from file)	result
main @ 8cfd6ab01	0	322 subtests passed
main + only the RPL schema fix (HIP untouched)	1	exactly one SUBFAILED(case='ipv6-route-type/RPL_Source_Route_Header'), detail ProtocolError: IPv6-Route: [TypeNo 3] invalid format
PR head 9025729f2 (full, with the re-point)	0	6 passed, 358 subtests

The middle row is the decisive one: 18 lines in one file, no HIP involvement, and the new failure appears. And the guard itself is untouched by this PR — if header.length % 16 != 0: is at line 612 and its raise at 613 on main and on the PR head alike, same line numbers. So the entry must be re-pointed rather than deleted, exactly as claimed.

The guard is genuinely broken, independently confirmed. Instrumenting it for the recorded case gives header.length (Hdr Ext Len) = 5, so 5 % 16 = 5 and it raises. Two lines below the guard, ipv6_route_header_length(header.length) converts from 8-octet units — which settles the unit question from the code itself. A % 16 != 0 test on a value in 8-octet units accepts only Hdr Ext Len ∈ {0, 16, 32, …}, i.e. it rejects very nearly every real RPL header. Same class of confusion as #487.

5. The re-pointed entry is correct in every field I could check

• status='CONSTRUCT' — correct, and not obviously so. The raise comes from a _read_* method, but STATUSES' own docstring records that ProtocolBase.__post_init__ packs and then unpacks, "so a _read_* fault on a perfectly good pack also lands here". The harness did report CONSTRUCT.
• fragment='IPv6-Route: [TypeNo 3] invalid format' — correct and specific, satisfying Gap.fragment's requirement of the alias plus the bracketed code, and not the bare 'invalid format' form the docstring warns occurs 205 times. I verified it pins one site: Routing.RPL_Source_Route_Header == 3, and the three sites in this file that print [TypeNo …] invalid format sit in _read_data_type_src (511), _read_data_type_2 (559) and _read_data_type_rpl (612/613) — each reachable only for its own routing type, so [TypeNo 3] can only come from 613's site.
• defect line :612 — correct. 612 is the condition (the cause), 613 the raise, matching the convention in neighbouring entries, which name the cause (e.g. the TCP entry cites tcp.py:2506 -- _make_mode_timeout sets length=3).
• It also fixes a pre-existing error. The old entry cited schema/internet/ipv6_route.py:156, which is SourceRoute.ip — a different class in a different file. The real defective cast was at line 208 (post_process begins at 198). That entry had been wrong by ~52 lines and the wrong file; the re-point corrects both.

6. The one substantive problem: "well-formed" is false

The new defect string reads "the reader rejects a well-formed header". Measured, on the exact case the entry records — the harness builds it from {'ip': ['2001:db8::1', '2001:db8::2']} with next=TransType.UDP, which is precisely what I reproduced (the leading 11 octet matches):

packed:  11 05 03 00 | 00 00 | 00 00 00 | <32 octets of addresses>
         next=UDP       cmpr_i cmpr_e  pad(3)
         Hdr Ext Len = 5
         actual packed octets = 41
         octets declared by Hdr Ext Len = 8 + 8*5 = 48   -> inconsistent by 7

Per-field widths from the schema's own buffer: cmpr_i 1, cmpr_e 1, pad 3, addresses 32, padding 0. The fixed (non-address) area is therefore 5 octets. RFC 6554 — as drawn in _read_data_type_rpl's own docstring — lays that area out as a single 32-bit row:

| CmprI | CmprE |  Pad  |               Reserved                |

CmprI and CmprE are 4 bits each and share one octet; the area is 4 octets, not 5. A spec-correct two-address RPL header is 4 + 4 + 32 = 40 octets, i.e. Hdr Ext Len = (40-8)/8 = 4. The schema emits one octet too many, which is also why the declared length cannot be made consistent.

So there is a third pre-existing defect behind the % 16 guard, and the header being rejected is not well-formed. This matters because the defect field is, in the table's own words, "the field that makes the entry worth keeping rather than just silencing": as written it directs the next person to fix % 16 and implies that will close the cycle. It will not. Please reword to say what is actually true — the guard is wrong and the header it is handed is malformed independently — and file the octet-width defect so the entry can cite it.

I am flagging this as a prose/accuracy problem in the entry, not as a demand that this PR fix the RPL round trip. Leaving that unfixed here is the right call and the entry says why convincingly.

7. Other checks

• Changelog is generated, not hand-written. python util/changelog_md.py --check → exit 0, "CHANGELOG.md is in step with docs/source/changelog/1.5.0.rst". This is the check CI runs as Changelog drift.
• Commit shape — one commit, 9025729f2, on top of current main 8cfd6ab01, no merge commit. (The earlier head c0705bc84 was a merge commit despite the description claiming one rebased commit; the force-push fixed that.)
• Scope — pcapkit/protocols/internet/ipv6_route.py is in the diff as a comment rewrite only; it does not touch the guard. Consistent with the description.

Could not verify

• CI. Reached deliberately on local evidence only, per my brief; I did not wait on GitHub Actions and make no claim about its tally.
• Behaviour on any interpreter other than CPython 3.14.7. Everything above is single-version.
• Real-capture RPL behaviour. Like the PR, I have no real RPL capture. My octet-width finding is derived from the module's own RFC 6554 diagram and the schema's packed field widths, not from wire data.
• Whether the HIP fix is correct for a multi-ENCRYPTED-parameter packet parsed from a real HIP capture. I reasoned about the pop making siblings independent and believe it is right, but I exercised it only through make, not against a capture containing two ENCRYPTED parameters with different ciphers.

[JarryShaw]

✅ GOOD TO MERGE

Re-review after the force-push, head d42940414 (previously reviewed at 9025729f2). Cross-model: Opus 5; the PR was authored on Sonnet.

Both items from my earlier ❌ NEEDS CHANGES are resolved, and resolved properly rather than papered over:

1. The false "well-formed" claim is gone, and the entry now records all three defects in the order they sit behind each other, with the measured numbers (5-octet fixed area against RFC 6554's 4; 41 octets packed against the 48 its own Hdr Ext Len declares). Better than what I asked for — I suggested rewording and filing the octet-width defect; you did both and filed it as RPL schema's fixed area is 5 octets where RFC 6554 gives 4, so a built header is wider than its own Hdr Ext Len #564 with the diagram citation, so the entry now points somewhere. That is the thing that makes the entry useful to whoever fixes % 16: they will now know the case still will not close afterwards, and why.
2. The description's stated shape is now exact. I measured 9 files changed, 190 insertions(+), 18 deletions(-), one commit, on top of 8cfd6ab01 — matching the body verbatim. The provenance paragraph is also corrected rather than left overstating a gap that no longer exists.

Re-verified on this head:

check	exit code (from file)	result
tests/protocols/test_option_roundtrip_unit.py	0	6 passed, 358 subtests
test_hip_unit.py + test_ipv6_extension_unit.py	0	75 passed, 146 subtests
util/changelog_md.py --check	0	in step with 1.5.0.rst

And importantly: git diff c0705bc84 d42940414 -- pcapkit/ is empty, so the two force-pushes changed only the test table. Every source-level measurement in my earlier detailed comment — the __cipher__-is-not-a-field probe, the fails-without-the-fix exit codes for both defects, and the main vs main+RPL-fix-only unmasking proof — applies to this head unchanged. I did not have to take any of it on trust a second time, and I did not.

The one remaining nit from my earlier comment is unchanged and still not blocking: the HIP test asserts assertEqual(caught, []) before assertIn(iv, packed), so in a reverted tree it reports "a warning fired" rather than "the IV is gone". Worth swapping the order the next time that file is touched; not worth a revision now.

Nothing else to raise. Good work on #564 — that was the right call over folding a speculative RPL fix into this PR.