diff --git a/docs/source/pcapkit/protocols/internet/hip.rst b/docs/source/pcapkit/protocols/internet/hip.rst index e1eddaf60..731f851ed 100644 --- a/docs/source/pcapkit/protocols/internet/hip.rst +++ b/docs/source/pcapkit/protocols/internet/hip.rst @@ -428,6 +428,9 @@ Auxiliary Functions .. autofunction:: pcapkit.protocols.schema.internet.hip.reg_info_list_len .. autofunction:: pcapkit.protocols.schema.internet.hip.two_octet_prefix_list_len .. autofunction:: pcapkit.protocols.schema.internet.hip.transport_format_list_len +.. autofunction:: pcapkit.protocols.schema.internet.hip.encrypted_data_len +.. autofunction:: pcapkit.protocols.schema.internet.hip.parameter_total_len +.. autofunction:: pcapkit.protocols.schema.internet.hip.parameter_padding_len Data Models ----------- diff --git a/examples/generators/options.py b/examples/generators/options.py index 624326d12..82d95403b 100644 --- a/examples/generators/options.py +++ b/examples/generators/options.py @@ -948,17 +948,80 @@ def _mh_extension_build(code: 'Any', kwargs: 'dict[str, Any]') -> 'Any': #: 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. +#: Two, and **no longer for the reason it used to be**, which is the whole point +#: of this note: the same constant now routes around a different set of defects +#: from the one it was introduced for, and reading it as still being about the +#: padding rule would send the next person to a line that is already fixed. +#: +#: It was two because of the defect #651 fixed. ``HIP.make`` computes the +#: header's ``len`` field as ``total_length // 8 + 4``, which is lossless only +#: when the parameter octets are a multiple of eight; and every padding site in +#: the two HIP modules aligned the *contents* to eight, ignoring the four-octet +#: type-and-length header, so a single parameter was always ``4 (mod 8)``, the +#: floor division always dropped those four octets, and ``_read_hip_param`` +#: -- which compares the recovered length exactly -- rejected the library's own +#: single-parameter packets. Two copies summed to a multiple of eight, so the +#: arithmetic came out exact and the parameter constructors became reachable. +#: +#: #651 made the padding :rfc:`7401` Section 5.2.1's +#: ``Total Length = 11 + Length - (Length + 3) % 8``, so each parameter is a +#: multiple of eight on its own and the pair is no longer needed for that. +#: Measured over this table's 49 HIP codes, on ``0c7f2b7c9`` and on the #651 +#: tree, by running the round trip at each setting: +#: +#: ====================== ======== ========== +#: tree one copy two copies +#: ====================== ======== ========== +#: ``0c7f2b7c9`` (before) 4 OK 45 OK +#: #651 (after) 45 OK 46 OK +#: ====================== ======== ========== +#: +#: So one copy went from unusable to very nearly usable, which is the strongest +#: statement available that the padding was what made a lone parameter +#: unrepresentable -- 41 codes that could not survive alone now can. +#: +#: It is still *two*, though, because the four codes that fail at one copy fail +#: for reasons that have nothing to do with padding. Three of them pack a number +#: of contents octets that disagrees with the ``len`` they declare, so their +#: record is not 8-aligned however the padding is computed, and a pair cancels +#: that misalignment exactly as it used to cancel the padding error: +#: +#: * ``R1_COUNTER`` (129) and ``R1_Counter`` (128) both declare ``len=12``, +#: which :rfc:`7401` Section 5.2.3 agrees with, but pack 12 octets in total +#: rather than 16 -- because ``counter`` is a +#: :class:`~pcapkit.corekit.fields.numbers.UInt32Field` where the RFC +#: specifies "R1 generation counter, 8 bytes", a 64-bit unsigned integer. Four +#: octets short, not filed anywhere yet, and found while measuring #651. +#: +#: ``R1_COUNTER`` is the one code that round-tripped at one copy *before* #651 +#: and does not after, and the reason is worth keeping: at ``len=12`` the old +#: contents-aligning rule appended exactly four surplus octets, which happened +#: to fill this parameter's four-octet shortfall and bring the record to 16. +#: Two defects cancelling, again. Correcting the padding removes the +#: compensation and leaves the shortfall visible, which is the right outcome +#: and not a regression in anything but this table's tally. +#: +#: Once the width is fixed, ``R1_Counter`` will fail on its own second defect +#: instead: code 128 parses as an ``UnassignedParameter``, because the schema +#: registry is keyed on ``code=`` and ``R1CounterParameter`` declares only 129. +#: That is what ``hip-parameter/R1_Counter`` in the expected-failure table +#: records, and it is why this code fails at two copies as well as at one. +#: * ``HOST_ID`` declares ``len=8`` and packs 18. Recorded as +#: ``hip-parameter/HOST_ID`` in that table. +#: * ``HIP_TRANSFORM`` is HIPv1-only -- ``_read_param_hip_transform`` raises for +#: any other version -- while this table builds it at version 2. Nothing to do +#: with lengths at all, and it is the one whose recorded ``defect`` string in +#: that table names the header arithmetic rather than this. +#: +#: Dropping to one copy would therefore trade this module's padding workaround +#: for two freshly exposed expected-failure entries (``R1_COUNTER`` and a +#: changed status on ``R1_Counter``) and the loss of ``R1_COUNTER`` from the +#: round-tripping set -- a change about *those* defects rather than about this +#: one, and one that belongs with their fixes. The +#: single-parameter case is not lost in the meantime: it is asserted directly, +#: and now positively, by +#: ``test_a_hip_packet_carrying_one_parameter_round_trips`` in +#: :mod:`tests.protocols.test_option_roundtrip_unit`. HIP_COPIES = 2 diff --git a/pcapkit/protocols/internet/hip.py b/pcapkit/protocols/internet/hip.py index 1df6734b6..c45ef7222 100644 --- a/pcapkit/protocols/internet/hip.py +++ b/pcapkit/protocols/internet/hip.py @@ -197,6 +197,7 @@ TransportFormatListParameter as Schema_TransportFormatListParameter from pcapkit.protocols.schema.internet.hip import UnassignedParameter as Schema_UnassignedParameter from pcapkit.protocols.schema.internet.hip import ViaRVSParameter as Schema_ViaRVSParameter +from pcapkit.protocols.schema.internet.hip import parameter_total_len from pcapkit.protocols.schema.schema import Schema from pcapkit.utilities.exceptions import ProtocolError, UnsupportedCall from pcapkit.utilities.logging import SPHINX_TYPE_CHECKING @@ -793,7 +794,7 @@ def _read_param_unassigned(self, schema: 'Schema_UnassignedParameter', *, versio unassigned = Data_UnassignedParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), contents=schema.value, ) return unassigned @@ -836,7 +837,7 @@ def _read_param_esp_info(self, schema: 'Schema_ESPInfoParameter', *, version: 'i esp_info = Data_ESPInfoParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), index=schema.index, old_spi=schema.old_spi, new_spi=schema.new_spi, @@ -882,7 +883,7 @@ def _read_param_r1_counter(self, schema: 'Schema_R1CounterParameter', *, version r1_counter = Data_R1CounterParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), counter=schema.counter, ) return r1_counter @@ -988,6 +989,12 @@ def _read_locator(locator: 'Schema_Locator') -> 'Data_LocatorData | IPv6Address' locator_set = Data_LocatorSetParameter( type=schema.type, critical=bool(schema.type & 0b1), + # NOTE: The one reported record length in this module left on the + # pre-#651 expression, to match the one padding site left on it -- + # see ``LocatorSetParameter.padding`` in + # :mod:`pcapkit.protocols.schema.internet.hip` for why touching + # either alone makes a conformant parameter non-conformant. #679 + # fixes both together, and this line moves with them. length=4 + schema.len + (8 - schema.len % 8) % 8, locator_set=tuple(_locs), ) @@ -1036,7 +1043,7 @@ def _read_param_puzzle(self, schema: 'Schema_PuzzleParameter', *, version: 'int' puzzle = Data_PuzzleParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), index=_numk, lifetime=datetime.timedelta(seconds=2 ** (_time - 32)), opaque=_opak, @@ -1107,7 +1114,7 @@ def _read_param_solution(self, schema: 'Schema_SolutionParameter', *, version: ' solution = Data_SolutionParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), index=_numk, reserved=_resv, opaque=_opak, @@ -1158,7 +1165,7 @@ def _read_param_seq(self, schema: 'Schema_SEQParameter', *, version: 'int', # p seq = Data_SEQParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), id=_upid, ) return seq @@ -1199,7 +1206,7 @@ def _read_param_ack(self, schema: 'Schema_ACKParameter', *, version: 'int', # p ack = Data_ACKParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), update_id=tuple(schema.update_id), ) return ack @@ -1234,7 +1241,7 @@ def _read_param_dh_group_list(self, schema: 'Schema_DHGroupListParameter', *, ve dh_group_list = Data_DHGroupListParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), group_id=tuple(schema.groups), ) return dh_group_list @@ -1271,7 +1278,7 @@ def _read_param_diffie_hellman(self, schema: 'Schema_DiffieHellmanParameter', *, diffie_hellman = Data_DiffieHellmanParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), group_id=schema.group, pub_len=schema.pub_len, pub_val=schema.pub_val, @@ -1316,7 +1323,7 @@ def _read_param_hip_transform(self, schema: 'Schema_HIPTransformParameter', *, v hip_transform = Data_HIPTransformParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), suite_id=tuple(schema.suites), ) return hip_transform @@ -1363,7 +1370,7 @@ def _read_param_hip_cipher(self, schema: 'Schema_HIPCipherParameter', *, version hip_cipher = Data_HIPCipherParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), cipher_id=tuple(schema.ciphers), ) return hip_cipher @@ -1406,7 +1413,7 @@ def _read_param_nat_traversal_mode(self, schema: 'Schema_NATTraversalModeParamet nat_traversal_mode = Data_NATTraversalModeParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), mode_id=tuple(schema.modes), ) return nat_traversal_mode @@ -1445,7 +1452,7 @@ def _read_param_transaction_pacing(self, schema: 'Schema_TransactionPacingParame transaction_pacing = Data_TransactionPacingParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), min_ta=schema.min_ta, ) return transaction_pacing @@ -1487,7 +1494,7 @@ def _read_param_encrypted(self, schema: 'Schema_EncryptedParameter', *, version: encrypted = Data_EncryptedParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), cipher=schema.cipher, iv=getattr(schema, 'iv', None), data=schema.data, @@ -1549,7 +1556,7 @@ def _read_param_host_id(self, schema: 'Schema_HostIDParameter', *, version: 'int host_id = Data_HostIDParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), hi_len=schema.hi_len, di_type=schema.di_data['type'], di_len=schema.di_data['len'], @@ -1589,7 +1596,7 @@ def _read_param_hit_suite_list(self, schema: 'Schema_HITSuiteListParameter', *, hit_suite_list = Data_HITSuiteListParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), suite_id=tuple(schema.suites), ) return hit_suite_list @@ -1626,7 +1633,7 @@ def _read_param_cert(self, schema: 'Schema_CertParameter', *, version: 'int', # cert = Data_CertParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), cert_group=schema.cert_group, cert_count=schema.cert_count, cert_id=schema.cert_id, @@ -1668,7 +1675,7 @@ def _read_param_notification(self, schema: 'Schema_NotificationParameter', *, ve notification = Data_NotificationParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), msg_type=schema.msg_type, msg=schema.msg, ) @@ -1702,7 +1709,7 @@ def _read_param_echo_request_signed(self, schema: 'Schema_EchoRequestSignedParam echo_request_signed = Data_EchoRequestSignedParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), opaque=schema.opaque, ) return echo_request_signed @@ -1739,7 +1746,7 @@ def _read_param_reg_info(self, schema: 'Schema_RegInfoParameter', *, version: 'i reg_info = Data_RegInfoParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), lifetime=Data_Lifetime( min=datetime.timedelta(seconds=schema.min_lifetime), max=datetime.timedelta(seconds=schema.max_lifetime), @@ -1780,7 +1787,7 @@ def _read_param_reg_request(self, schema: 'Schema_RegRequestParameter', *, versi reg_request = Data_RegRequestParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), lifetime=datetime.timedelta(seconds=schema.lifetime), reg_type=tuple(schema.reg_request), ) @@ -1818,7 +1825,7 @@ def _read_param_reg_response(self, schema: 'Schema_RegResponseParameter', *, ver reg_response = Data_RegResponseParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), lifetime=datetime.timedelta(seconds=schema.lifetime), reg_type=tuple(schema.reg_response), ) @@ -1856,7 +1863,7 @@ def _read_param_reg_failed(self, schema: 'Schema_RegFailedParameter', *, version reg_failed = Data_RegFailedParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), lifetime=datetime.timedelta(seconds=schema.lifetime), reg_type=tuple(schema.reg_failed), ) @@ -1901,7 +1908,7 @@ def _read_param_reg_from(self, schema: 'Schema_RegFromParameter', *, version: 'i reg_from = Data_RegFromParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), port=schema.port, protocol=schema.protocol, address=schema.address, @@ -1936,7 +1943,7 @@ def _read_param_echo_response_signed(self, schema: 'Schema_EchoResponseSignedPar echo_response_signed = Data_EchoResponseSignedParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), opaque=schema.opaque, ) return echo_response_signed @@ -1977,7 +1984,7 @@ def _read_param_transport_format_list(self, schema: 'Schema_TransportFormatListP transport_format_list = Data_TransportFormatListParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), tf_type=tuple(schema.formats), ) return transport_format_list @@ -2020,7 +2027,7 @@ def _read_param_esp_transform(self, schema: 'Schema_ESPTransformParameter', *, v esp_transform = Data_ESPTransformParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), suite_id=tuple(schema.suites), ) return esp_transform @@ -2059,7 +2066,7 @@ def _read_param_seq_data(self, schema: 'Schema_SeqDataParameter', *, version: 'i seq_data = Data_SeqDataParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), seq=schema.seq, ) return seq_data @@ -2099,7 +2106,7 @@ def _read_param_ack_data(self, schema: 'Schema_AckDataParameter', *, version: 'i ack_data = Data_AckDataParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), ack=tuple(schema.ack), ) return ack_data @@ -2139,7 +2146,7 @@ def _read_param_payload_mic(self, schema: 'Schema_PayloadMICParameter', *, versi payload_mic = Data_PayloadMICParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), next=schema.next, payload=schema.payload, mic=schema.mic, @@ -2176,7 +2183,7 @@ def _read_param_transaction_id(self, schema: 'Schema_TransactionIDParameter', *, transaction_id = Data_TransactionIDParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), id=schema.id, ) return transaction_id @@ -2211,7 +2218,7 @@ def _read_param_overlay_id(self, schema: 'Schema_OverlayIDParameter', *, version overlay_id = Data_OverlayIDParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), id=schema.id, ) return overlay_id @@ -2263,7 +2270,7 @@ def _read_param_route_dst(self, schema: 'Schema_RouteDstParameter', *, version: route_dst = Data_RouteDstParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), flags=Data_Flags( symmetric=bool(schema.flags['symmetric']), must_follow=bool(schema.flags['must_follow']), @@ -2310,7 +2317,7 @@ def _read_param_hip_transport_mode(self, schema: 'Schema_HIPTransportModeParamet hip_transport_mode = Data_HIPTransportModeParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), port=schema.port, mode_id=tuple(schema.mode), ) @@ -2348,7 +2355,7 @@ def _read_param_hip_mac(self, schema: 'Schema_HIPMACParameter', *, version: 'int hip_mac = Data_HIPMACParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), hmac=schema.hmac, ) return hip_mac @@ -2385,7 +2392,7 @@ def _read_param_hip_mac_2(self, schema: 'Schema_HIPMAC2Parameter', *, version: ' hip_mac_2 = Data_HIPMAC2Parameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), hmac=schema.hmac, ) return hip_mac_2 @@ -2420,7 +2427,7 @@ def _read_param_hip_signature_2(self, schema: 'Schema_HIPSignature2Parameter', * hip_signature_2 = Data_HIPSignature2Parameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), algorithm=schema.algorithm, signature=schema.signature, ) @@ -2456,7 +2463,7 @@ def _read_param_hip_signature(self, schema: 'Schema_HIPSignatureParameter', *, v hip_signature = Data_HIPSignatureParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), algorithm=schema.algorithm, signature=schema.signature, ) @@ -2490,7 +2497,7 @@ def _read_param_echo_request_unsigned(self, schema: 'Schema_EchoRequestUnsignedP echo_request_unsigned = Data_EchoRequestUnsignedParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), opaque=schema.opaque, ) return echo_request_unsigned @@ -2523,7 +2530,7 @@ def _read_param_echo_response_unsigned(self, schema: 'Schema_EchoResponseUnsigne echo_response_unsigned = Data_EchoResponseUnsignedParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), opaque=schema.opaque, ) return echo_response_unsigned @@ -2570,7 +2577,7 @@ def _read_param_relay_from(self, schema: 'Schema_RelayFromParameter', *, version relay_from = Data_RelayFromParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), port=schema.port, protocol=schema.protocol, address=address, # type: ignore[arg-type] @@ -2619,7 +2626,7 @@ def _read_param_relay_to(self, schema: 'Schema_RelayToParameter', *, version: 'i relay_to = Data_RelayToParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), port=schema.port, protocol=schema.protocol, address=address, # type: ignore[arg-type] @@ -2660,7 +2667,7 @@ def _read_param_overlay_ttl(self, schema: 'Schema_OverlayTTLParameter', *, versi overlay_ttl = Data_OverlayTTLParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), ttl=datetime.timedelta(seconds=schema.ttl), ) return overlay_ttl @@ -2712,7 +2719,7 @@ def _read_param_route_via(self, schema: 'Schema_RouteViaParameter', *, version: route_via = Data_RouteViaParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), flags=Data_Flags( symmetric=bool(schema.flags['symmetric']), must_follow=bool(schema.flags['must_follow']), @@ -2758,7 +2765,7 @@ def _read_param_from(self, schema: 'Schema_FromParameter', *, version: 'int', # from_ = Data_FromParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), address=schema.address, ) return from_ @@ -2793,7 +2800,7 @@ def _read_param_rvs_hmac(self, schema: 'Schema_RVSHMACParameter', *, version: 'i rvs_hmac = Data_RVSHMACParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), hmac=schema.hmac, ) return rvs_hmac @@ -2843,7 +2850,7 @@ def _read_param_via_rvs(self, schema: 'Schema_ViaRVSParameter', *, version: 'int via_rvs = Data_ViaRVSParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), address=tuple(schema.address), ) return via_rvs @@ -2878,7 +2885,7 @@ def _read_param_relay_hmac(self, schema: 'Schema_RelayHMACParameter', version: ' relay_hmac = Data_RelayHMACParameter( type=schema.type, critical=bool(schema.type & 0b1), - length=4 + schema.len + (8 - schema.len % 8) % 8, + length=parameter_total_len(schema.len), hmac=schema.hmac, ) return relay_hmac diff --git a/pcapkit/protocols/schema/internet/hip.py b/pcapkit/protocols/schema/internet/hip.py index d76a61d20..b6ff252a4 100644 --- a/pcapkit/protocols/schema/internet/hip.py +++ b/pcapkit/protocols/schema/internet/hip.py @@ -304,6 +304,154 @@ def transport_format_list_len(pkt: 'dict[str, Any]') -> 'int': return length +def encrypted_data_len(pkt: 'dict[str, Any]') -> 'int': + """Return ``ENCRYPTED`` encrypted-data length. + + Used by the ``data`` field of :class:`EncryptedParameter`, which follows a + four-octet ``reserved`` field and a conditional sixteen-octet ``iv`` with + the ciphertext, sized by the remainder of the parameter. :rfc:`7401` + Section 5.2.18 puts all three inside ``Length`` -- + + :: + + | Type | Length | + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + | Reserved | + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + | IV | + / / + / Encrypted data / + + -- so both have to come off ``Length`` to leave the data, and + :meth:`~pcapkit.protocols.internet.hip.HIP._make_param_encrypted` writes + ``len=4 + len(iv) + len(data)`` to match. + + This subtracted the ``iv`` but not the ``reserved``, so the field claimed + four octets more than the parameter holds: on unpack it read four octets + of the *next* parameter into ``data``, and on pack it zero-extended the + ciphertext by four. That was recorded as the second half of + ``hip-parameter/ENCRYPTED`` in the round-trip suite's expected-failure + table, and left alone -- because while the padding rule was also four + octets out (#651), the two errors cancelled at some residues of ``Length`` + and not others. Measured by packing through the public maker at every + residue, the old record total agreed with :rfc:`7401` Section 5.2.1 at + ``Length % 8`` in ``{0, 5, 6, 7}`` and was eight octets over at + ``{1, 2, 3, 4}`` -- so ``Length = 8``, which the unit suite happened to + use, is one of the four where the module emitted RFC-conformant + ``ENCRYPTED`` octets while getting both halves wrong. + + Fixing the padding without fixing this would therefore have *regressed* + ``ENCRYPTED``, from right-by-accident at four of the eight residues to four + octets too long at all eight: with only the padding corrected the total + becomes ``8 + Length + pad`` against a correct ``4 + Length + pad``, which + is a uniform four-octet surplus with no residue left where it cancels. So + the two go together. + + Args: + pkt: Parameter unpacked schema. + + Returns: + Encrypted data length. + + Raises: + FieldValueError: If the parameter's ``Length`` on the wire is too short + to hold the ``reserved`` octets, and the ``iv`` octets where a + cipher that carries one was resolved -- which would otherwise + underflow the data length below zero. + + """ + length = pkt['len'] - 4 - (16 if pkt.get('iv') else 0) + if length < 0: + raise FieldValueError(f'HIP: invalid parameter length: {pkt["len"]}') + return length + + +def parameter_total_len(length: 'int') -> 'int': + """Return the total on-wire length of a HIP parameter. + + :rfc:`7401` Section 5.2.1 states the arithmetic outright, so there is + nothing here to infer from the diagram -- + + :: + + 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. + + Total Length = 11 + Length - (Length + 3) % 8; + + -- and this function is that formula, spelled the way the RFC spells it. + + The distinction that matters is *which* quantity gets aligned. ``Length`` + is "Length of the Contents, in bytes, excluding Type, Length, and + Padding", and it is the **total** -- contents plus the four octets of + ``Type`` and ``Length`` plus padding -- that must land on a multiple of + eight. Aligning the contents alone instead, as every padding site in this + module and in :mod:`pcapkit.protocols.internet.hip` did before #651, puts + every parameter at ``4 (mod 8)`` for every possible ``Length``: never a + multiple of eight and never the length the RFC gives. It is not even a + consistent offset, because the two formulas disagree in both directions -- + at ``Length = 4`` (a whole ``SEQ``) the contents are already 8-aligned + with the header, so the RFC requires no padding at all and aligning the + contents appends four octets that must not be there; at ``Length = 8`` the + contents are 8-aligned on their own, so aligning them appends nothing and + the record is left four octets short. + + Args: + length: The parameter's ``Length`` field, i.e. its contents length in + octets, excluding ``Type``, ``Length`` and ``Padding``. + + Returns: + Total length of the parameter in octets, including ``Type``, + ``Length``, ``Contents`` and ``Padding``. Always a multiple of eight. + + Raises: + FieldValueError: If ``length`` is negative. This cannot happen from + real wire bytes -- ``len`` is an unsigned 16-bit field -- but a + caller constructing a schema directly could still pass one, and + the RFC formula is meaningless there: it would answer 8 for + ``Length = -1``, a "total" smaller than the four-octet header + alone. Raising keeps this to the same floor-and-raise discipline + as :func:`two_octet_prefix_list_len`. + + """ + if length < 0: + raise FieldValueError(f'HIP: invalid parameter length: {length}') + return 11 + length - (length + 3) % 8 + + +def parameter_padding_len(pkt: 'dict[str, Any]') -> 'int': + """Return the number of padding octets a HIP parameter needs. + + Used by the ``padding`` field of every parameter schema in this module. + The count is whatever :func:`parameter_total_len` leaves over once the + four-octet ``Type`` and ``Length`` header and the ``Length`` octets of + contents are accounted for, which :rfc:`7401` Section 5.2.1 bounds at + "0-7 bytes". + + This is one function shared by every parameter rather than a lambda + repeated per class because it was previously the latter -- 46 copies of + the same wrong expression here and 49 of its counterpart in + :mod:`pcapkit.protocols.internet.hip`, which is 95 places for the + arithmetic to be wrong in and one place too few to state the RFC's reason + for it. + + Args: + pkt: Parameter unpacked schema. + + Returns: + Padding length in octets, between 0 and 7 inclusive. + + Raises: + FieldValueError: If the parameter's ``Length`` on the wire is + negative; see :func:`parameter_total_len`. + + """ + length = pkt['len'] + return parameter_total_len(length) - 4 - length + + class Parameter(EnumSchema[Enum_Parameter]): """Base schema for HIP parameters.""" @@ -322,7 +470,7 @@ class UnassignedParameter(Parameter): #: Parameter value. value: 'bytes' = BytesField(length=lambda pkt: pkt['len']) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', value: 'bytes') -> 'None': ... @@ -341,7 +489,7 @@ class ESPInfoParameter(Parameter, code=Enum_Parameter.ESP_INFO): #: New SPI. new_spi: 'int' = UInt32Field() #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', index: 'int', @@ -357,7 +505,7 @@ class R1CounterParameter(Parameter, code=Enum_Parameter.R1_COUNTER): #: R1 counter. counter: 'int' = UInt32Field() #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', counter: 'int') -> 'None': ... @@ -401,6 +549,45 @@ class LocatorSetParameter(Parameter, code=Enum_Parameter.LOCATOR_SET): item_type=SchemaField(schema=Locator), ) #: Padding. + #: + #: **This is the one parameter in this module that does not use** + #: :func:`parameter_padding_len`, **and the exclusion is deliberate. Do not + #: "finish" #651 by changing this line on its own: doing so takes a + #: conformant parameter to four octets short.** Two defects here cancel each + #: other exactly, and #679 tracks fixing them together: + #: + #: 1. This callback does not receive the parameter's ``len`` at all. ``ListField`` + #: packs each nested :class:`Locator` into the shared packet context, whose + #: own ``len`` key overwrites the parameter's, and ``padding`` is evaluated + #: after the list -- so the value seen is the last locator's ``len``, which + #: is 4 for any IPv6 locator. Measured by building this schema directly with + #: a parameter ``len`` of 9 over a single locator of ``len`` 4: the record + #: pads by the amount for 4, not the 3 that 9 would give. + #: 2. :meth:`~pcapkit.protocols.internet.hip.HIP._make_param_locator_set` sets + #: this parameter's ``len`` to ``sum(Locator.len)``, and ``Locator.len`` + #: counts 4-octet units where :rfc:`7401` Section 5.2.1's ``Length`` is a + #: byte count -- so it writes ``4n`` where the contents are ``24n`` octets. + #: + #: Because the shadowed value is always 4 for a plain IPv6 locator, the old + #: expression always appends 4, giving ``4 + 24n + 4 = 24n + 8``; and because + #: ``24n`` is a multiple of 8, the RFC total for a byte-count ``Length`` of + #: ``24n`` is ``11 + 24n - 3``, the same ``24n + 8``. Measured at n = 1, 2, 5 as + #: 32, 56 and 128 octets, on this tree and on the tree before #651 alike. So the + #: wire output is right today, by two wrongs, and correcting only the padding + #: would leave ``24n + 4``. + #: + #: That cancellation holds for a set of **plain IPv6 locators only**, and the + #: reason to say so here is that it would be easy to read the paragraph above as + #: a guarantee about this parameter in general. It is not. A locator carrying an + #: SPI is 28 octets rather than 24, and an empty set has no locator to shadow + #: ``len`` at all, so measured on both trees: the empty set packs 4 octets where + #: :rfc:`7401` Section 5.2.1 wants 8; one SPI locator packs 35; two pack 63; and + #: a mixed plain-and-SPI pair packs 59 or 60 depending on order -- none of them a + #: multiple of eight. Those shapes are non-conformant *before and after* #651, + #: byte-identically, which is exactly why leaving this line alone is the safe + #: choice rather than the correct one: it ships nothing different. #679 owns + #: making them right, and has to account for all of these shapes, not just the + #: homogeneous one. padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) if TYPE_CHECKING: @@ -433,7 +620,7 @@ class PuzzleParameter(Parameter, code=Enum_Parameter.PUZZLE): #: Random data. random: 'int' = NumberField(length=lambda pkt: pkt['len'] - 4, signed=False) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', index: 'int', lifetime: 'int', @@ -459,7 +646,7 @@ class SolutionParameter(Parameter, code=Enum_Parameter.SOLUTION): #: Solution. solution: 'int' = NumberField(length=lambda pkt: (pkt['len'] - 4) // 2, signed=False) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', index: 'int', reserved: 'int', @@ -473,7 +660,7 @@ class SEQParameter(Parameter, code=Enum_Parameter.SEQ): #: Update ID. update_id: 'int' = UInt32Field() #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', update_id: 'int') -> 'None': ... @@ -489,7 +676,7 @@ class ACKParameter(Parameter, code=Enum_Parameter.ACK): item_type=UInt32Field(), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', update_id: 'bytes | list[int]') -> 'None': ... @@ -505,7 +692,7 @@ class DHGroupListParameter(Parameter, code=Enum_Parameter.DH_GROUP_LIST): item_type=EnumField(length=1, namespace=Enum_Group), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', groups: 'list[Enum_Group]') -> 'None': ... @@ -522,7 +709,7 @@ class DiffieHellmanParameter(Parameter, code=Enum_Parameter.DIFFIE_HELLMAN): #: Diffie-Hellman value. pub_val: 'int' = NumberField(length=lambda pkt: pkt['pub_len'], signed=False) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', group: 'Enum_Group', pub_len: 'int', @@ -539,7 +726,7 @@ class HIPTransformParameter(Parameter, code=Enum_Parameter.HIP_TRANSFORM): item_type=EnumField(length=2, namespace=Enum_Suite), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', suites: 'list[Enum_Suite]') -> 'None': ... @@ -555,7 +742,7 @@ class HIPCipherParameter(Parameter, code=Enum_Parameter.HIP_CIPHER): item_type=EnumField(length=2, namespace=Enum_Cipher), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', ciphers: 'list[Enum_Cipher]') -> 'None': ... @@ -573,7 +760,7 @@ class NATTraversalModeParameter(Parameter, code=Enum_Parameter.NAT_TRAVERSAL_MOD item_type=EnumField(length=2, namespace=Enum_NATTraversal), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', modes: 'list[Enum_NATTraversal]') -> 'None': ... @@ -586,7 +773,7 @@ class TransactionPacingParameter(Parameter, code=Enum_Parameter.TRANSACTION_PACI #: Transaction pacing. min_ta: 'int' = UInt32Field() #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', min_ta: 'int') -> 'None': ... @@ -604,11 +791,9 @@ class EncryptedParameter(Parameter, code=Enum_Parameter.ENCRYPTED): lambda pkt: pkt['__cipher__'] in (Enum_Cipher.AES_128_CBC, Enum_Cipher.AES_256_CBC), ) #: Data. - data: 'bytes' = BytesField( - length=lambda pkt: pkt['len'] - (16 if pkt.get('iv') else 0), - ) + data: 'bytes' = BytesField(length=encrypted_data_len) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) @classmethod def pre_unpack(cls, packet: 'dict[str, Any]') -> 'None': @@ -713,7 +898,7 @@ class HostIDParameter(Parameter, code=Enum_Parameter.HOST_ID): #: Domain ID. di: 'bytes' = BytesField(length=lambda pkt: pkt['di_data']['len']) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', hi_len: 'int', di_data: 'DIData', @@ -774,7 +959,7 @@ class HITSuiteListParameter(Parameter, code=Enum_Parameter.HIT_SUITE_LIST): item_type=EnumField(length=1, namespace=Enum_HITSuite), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', suites: 'list[Enum_HITSuite]') -> 'None': ... @@ -795,7 +980,7 @@ class CertParameter(Parameter, code=Enum_Parameter.CERT): #: Certificate data. cert: 'bytes' = BytesField(length=lambda pkt: pkt['len'] - 4) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', cert_group: 'Enum_Group', cert_count: 'int', @@ -813,7 +998,7 @@ class NotificationParameter(Parameter, code=Enum_Parameter.NOTIFICATION): #: Notification data. msg: 'bytes' = BytesField(length=lambda pkt: pkt['len'] - 4) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', msg_type: 'Enum_NotifyMessage', msg: 'bytes') -> 'None': ... @@ -826,7 +1011,7 @@ class EchoRequestSignedParameter(Parameter, code=Enum_Parameter.ECHO_REQUEST_SIG #: Opaque data. opaque: 'bytes' = BytesField(length=lambda pkt: pkt['len']) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', opaque: 'bytes') -> 'None': ... @@ -846,7 +1031,7 @@ class RegInfoParameter(Parameter, code=Enum_Parameter.REG_INFO): item_type=EnumField(length=1, namespace=Enum_Registration), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', min_lifetime: 'int', max_lifetime: 'int', @@ -865,7 +1050,7 @@ class RegRequestParameter(Parameter, code=Enum_Parameter.REG_REQUEST): item_type=EnumField(length=1, namespace=Enum_Registration), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', lifetime: 'int', reg_request: 'list[Enum_Registration]') -> 'None': ... @@ -883,7 +1068,7 @@ class RegResponseParameter(Parameter, code=Enum_Parameter.REG_RESPONSE): item_type=EnumField(length=1, namespace=Enum_Registration), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', lifetime: 'int', reg_response: 'list[Enum_Registration]') -> 'None': ... @@ -901,7 +1086,7 @@ class RegFailedParameter(Parameter, code=Enum_Parameter.REG_FAILED): item_type=EnumField(length=1, namespace=Enum_RegistrationFailure), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', lifetime: 'int', reg_failed: 'list[Enum_RegistrationFailure]') -> 'None': ... @@ -931,7 +1116,7 @@ class EchoResponseSignedParameter(Parameter, code=Enum_Parameter.ECHO_RESPONSE_S #: Opaque data. opaque: 'bytes' = BytesField(length=lambda pkt: pkt['len']) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', opaque: 'bytes') -> 'None': ... @@ -947,7 +1132,7 @@ class TransportFormatListParameter(Parameter, code=Enum_Parameter.TRANSPORT_FORM item_type=EnumField(length=2, namespace=Enum_Parameter), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', formats: 'list[Enum_Parameter]') -> 'None': ... @@ -965,7 +1150,7 @@ class ESPTransformParameter(Parameter, code=Enum_Parameter.ESP_TRANSFORM): item_type=EnumField(length=2, namespace=Enum_ESPTransformSuite), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', suites: 'list[Enum_ESPTransformSuite]') -> 'None': ... @@ -978,7 +1163,7 @@ class SeqDataParameter(Parameter, code=Enum_Parameter.SEQ_DATA): #: Sequence number. seq: 'int' = UInt32Field() #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', seq: 'int') -> 'None': ... @@ -994,7 +1179,7 @@ class AckDataParameter(Parameter, code=Enum_Parameter.ACK_DATA): item_type=UInt32Field(), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', ack: 'list[int]') -> 'None': ... @@ -1013,7 +1198,7 @@ class PayloadMICParameter(Parameter, code=Enum_Parameter.PAYLOAD_MIC): #: MIC value. mic: 'bytes' = BytesField(length=lambda pkt: pkt['len'] - 8) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', next: 'Enum_TransType', payload: 'bytes', mic: 'bytes') -> 'None': ... @@ -1026,7 +1211,7 @@ class TransactionIDParameter(Parameter, code=Enum_Parameter.TRANSACTION_ID): #: Transaction ID. id: 'int' = NumberField(length=lambda pkt: pkt['len'], signed=False) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', id: 'int') -> 'None': ... @@ -1039,7 +1224,7 @@ class OverlayIDParameter(Parameter, code=Enum_Parameter.OVERLAY_ID): #: Overlay ID. id: 'int' = NumberField(length=lambda pkt: pkt['len'], signed=False) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', id: 'int') -> 'None': ... @@ -1062,7 +1247,7 @@ class RouteDstParameter(Parameter, code=Enum_Parameter.ROUTE_DST): item_type=IPv6AddressField(), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', flags: 'RouteFlags', hit: 'list[str | int | bytes | IPv6Address]') -> 'None': ... @@ -1080,7 +1265,7 @@ class HIPTransportModeParameter(Parameter, code=Enum_Parameter.HIP_TRANSPORT_MOD item_type=EnumField(length=2, namespace=Enum_Transport), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', port: 'int', mode: 'list[Enum_Transport]') -> 'None': ... @@ -1093,7 +1278,7 @@ class HIPMACParameter(Parameter, code=Enum_Parameter.HIP_MAC): #: HMAC value. hmac: 'bytes' = BytesField(length=lambda pkt: pkt['len']) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', hmac: 'bytes') -> 'None': ... @@ -1106,7 +1291,7 @@ class HIPMAC2Parameter(Parameter, code=Enum_Parameter.HIP_MAC_2): #: HMAC value. hmac: 'bytes' = BytesField(length=lambda pkt: pkt['len']) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', hmac: 'bytes') -> 'None': ... @@ -1121,7 +1306,7 @@ class HIPSignature2Parameter(Parameter, code=Enum_Parameter.HIP_SIGNATURE_2): #: Signature value. signature: 'bytes' = BytesField(length=lambda pkt: pkt['len'] - 2) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', algorithm: 'Enum_HIAlgorithm', signature: 'bytes') -> 'None': ... @@ -1136,7 +1321,7 @@ class HIPSignatureParameter(Parameter, code=Enum_Parameter.HIP_SIGNATURE): #: Signature value. signature: 'bytes' = BytesField(length=lambda pkt: pkt['len'] - 2) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', algorithm: 'Enum_HIAlgorithm', signature: 'bytes') -> 'None': ... @@ -1149,7 +1334,7 @@ class EchoRequestUnsignedParameter(Parameter, code=Enum_Parameter.ECHO_REQUEST_U #: Opaque data. opaque: 'bytes' = BytesField(length=lambda pkt: pkt['len']) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', opaque: 'bytes') -> 'None': ... @@ -1162,7 +1347,7 @@ class EchoResponseUnsignedParameter(Parameter, code=Enum_Parameter.ECHO_RESPONSE #: Opaque data. opaque: 'bytes' = BytesField(length=lambda pkt: pkt['len']) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', opaque: 'bytes') -> 'None': ... @@ -1211,7 +1396,7 @@ class OverlayTTLParameter(Parameter, code=Enum_Parameter.OVERLAY_TTL): #: Reserved. reserved: 'bytes' = PaddingField(length=2) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', ttl: 'int') -> 'None': ... @@ -1234,7 +1419,7 @@ class RouteViaParameter(Parameter, code=Enum_Parameter.ROUTE_VIA): item_type=IPv6AddressField(), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', flags: 'RouteFlags', hit: 'list[str | bytes | int | IPv6Address]') -> 'None': ... @@ -1247,7 +1432,7 @@ class FromParameter(Parameter, code=Enum_Parameter.FROM): #: Address. address: 'IPv6Address' = IPv6AddressField() #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', address: 'str | bytes | int | IPv6Address') -> 'None': ... @@ -1260,7 +1445,7 @@ class RVSHMACParameter(Parameter, code=Enum_Parameter.RVS_HMAC): #: HMAC value. hmac: 'bytes' = BytesField(length=lambda pkt: pkt['len']) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', hmac: 'bytes') -> 'None': ... @@ -1276,7 +1461,7 @@ class ViaRVSParameter(Parameter, code=Enum_Parameter.VIA_RVS): item_type=IPv6AddressField(), ) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', address: 'list[str | bytes | int | IPv6Address]') -> 'None': ... @@ -1289,7 +1474,7 @@ class RelayHMACParameter(Parameter, code=Enum_Parameter.RELAY_HMAC): #: HMAC value. hmac: 'bytes' = BytesField(length=lambda pkt: pkt['len']) #: Padding. - padding: 'bytes' = PaddingField(length=lambda pkt: (8 - (pkt['len'] % 8)) % 8) + padding: 'bytes' = PaddingField(length=parameter_padding_len) if TYPE_CHECKING: def __init__(self, type: 'Enum_Parameter', len: 'int', hmac: 'bytes') -> 'None': ... diff --git a/tests/corekit/test_fields_ipaddress.py b/tests/corekit/test_fields_ipaddress.py index 36d0efdfb..8d49f3224 100644 --- a/tests/corekit/test_fields_ipaddress.py +++ b/tests/corekit/test_fields_ipaddress.py @@ -548,7 +548,42 @@ def test_switch_backed_address_makers_reject_a_bool(self) -> None: self.assertIsInstance(context.exception, BaseError) self.assertIn('must not be a bool', str(context.exception)) - # the same two sites still take every legitimate value they took before + # the same two sites still take every legitimate value they took before. + # + # These two literals are unchanged by #651, and that is worth a note rather + # than being left to look like an oversight: #651 corrected HIP parameter + # padding to :rfc:`7401` Section 5.2.1's + # ``Total Length = 11 + Length - (Length + 3) % 8`` at 45 of the 46 + # parameters, and ``LOCATOR_SET`` is the one deliberately left alone. So + # these records are byte-for-byte what ``main`` emits -- verified against + # ``b34f132f6`` rather than assumed -- and they are *also* what the RFC + # asks for. + # + # The reason they are already right is that two defects in this parameter + # cancel each other exactly, which is why correcting the padding here alone + # would have broken it. ``LocatorSetParameter.padding``'s callback never + # receives the parameter's ``len``: ``ListField`` packs each nested + # ``Locator`` into the shared packet context, whose own ``len`` overwrites + # the parameter's, and ``padding`` is evaluated after the list -- so it sees + # the last locator's ``len``, which is 4 for any IPv6 locator. Meanwhile + # ``_make_param_locator_set`` writes the parameter's ``len`` as + # ``sum(Locator.len)``, in 4-octet units, where the RFC's ``Length`` is a + # byte count: ``4n`` where the contents are ``24n`` octets. + # + # Always-4 padding gives ``4 + 24n + 4 = 24n + 8``; and because ``24n`` is a + # multiple of 8, the RFC total for a byte-count ``Length`` of ``24n`` is + # ``11 + 24n - 3``, the same ``24n + 8``. Measured at n = 1, 2, 5 on both + # trees: 32, 56 and 128 octets, equal to the RFC total in every case. #679 + # tracks fixing the pair together; see ``LocatorSetParameter.padding`` for + # why neither half moves on its own. + # + # They pin exact octets rather than a length or a prefix, deliberately. + # Exact octets are the whole subject of #651, and pcapkit round-trips its + # own output whatever the padding rule says -- writer and reader shared the + # error -- so a comparison that tolerated trailing bytes would have gone on + # passing through the defect and through the fix alike. For the same reason + # these stay 32 octets: a shorter pin here would silently bless the + # four-octet shortfall that narrowing #651 exists to avoid. self.assertEqual( hip._make_param_locator_set( # type: ignore[arg-type] Parameter.LOCATOR_SET, version=2, @@ -559,7 +594,8 @@ def test_switch_backed_address_makers_reject_a_bool(self) -> None: MPTCPOption.ADD_ADDR, addr_id=1, addr='192.0.2.1').address, ipaddress.IPv4Address('192.0.2.1')) # int(True) is 1, and 1 is ::1 for an IPv6-only locator -- the escape - # hatch works and still widens to the family the wire format fixes + # hatch works and still widens to the family the wire format fixes. + # Four trailing octets shorter since #651, as above. self.assertEqual( hip._make_param_locator_set( # type: ignore[arg-type] Parameter.LOCATOR_SET, version=2, diff --git a/tests/protocols/internet/test_hip_unit.py b/tests/protocols/internet/test_hip_unit.py index 4c74e777c..4d896b5b1 100644 --- a/tests/protocols/internet/test_hip_unit.py +++ b/tests/protocols/internet/test_hip_unit.py @@ -317,9 +317,10 @@ def make_param(code, param=None, *, version, contents=b'', **kwargs): [(custom, {'contents': b'wxyz'})], version=2) self.assertEqual(seen, ['make/v2']) self.assertEqual(made_list[0].value, b'wxyz') - # 4 octets of type and length, 4 of value, 4 of padding to a multiple - # of 8 - self.assertEqual(list_len, 12) + # 4 octets of type and length plus 4 of value is already a multiple + # of 8, so RFC 7401 5.2.1 asks for no padding at all: + # 11 + 4 - (4 + 3) % 8 == 8. + self.assertEqual(list_len, 8) # ... and the OrderedMultiDict branch, which the two halves of an # issue like this are equally easy to fix one of and forget the other @@ -328,7 +329,7 @@ def make_param(code, param=None, *, version, contents=b'', **kwargs): OrderedMultiDict([(custom, parsed[custom])]), version=2) self.assertEqual(seen, ['make/v2']) self.assertEqual(made_dict[0].value, b'abcd') - self.assertEqual(dict_len, 12) + self.assertEqual(dict_len, 8) finally: registry.pop(custom, None) @@ -1946,26 +1947,29 @@ def test_hip_transport_format_list_parameter_accepts_an_empty_list_at_length_zer parses it back to confirm the corrected :func:`~pcapkit.protocols. schema.internet.hip.transport_format_list_len` accepts it again. - Two copies, not one: a single parameter's own total is always - ``4 (mod 8)`` under this module's padding rule (see - ``examples/generators/options.py``'s ``HIP_COPIES``), so one 40-octet - fixed header plus two empty ``TRANSPORT_FORMAT_LIST`` parameters (4 - octets each) is what actually lands on an 8-octet boundary. + Two copies, and since #651 not because one is unrepresentable -- one + now is -- but because two consecutive parameters are what prove the + reader's *stride*. A parser that pads by the wrong amount still reads + a lone parameter correctly and only lands in the wrong place at the + start of the next one, so a single-parameter fixture cannot tell a + correct padding rule from any other. """ from pcapkit.const.hip.parameter import Parameter from pcapkit.protocols.internet.hip import HIP - # next(1) len(1)=5 pkt(1) ver(1)=0x01 (the reserved bit that must be 1) + # next(1) len(1)=6 pkt(1) ver(1)=0x01 (the reserved bit that must be 1) # checksum(2) control(2) shit(16) rhit(16) -- the fixed 40-octet header, - # declaring one 8-octet parameter area to follow: (5 - 4) * 8 == 8. - fixed = bytes([0x3b, 0x05, 0x00, 0x01]) + bytes(2) + bytes(2) + bytes(16) + bytes(16) + # declaring one 16-octet parameter area to follow: (6 - 4) * 8 == 16. + fixed = bytes([0x3b, 0x06, 0x00, 0x01]) + bytes(2) + bytes(2) + bytes(16) + bytes(16) self.assertEqual(len(fixed), 40) # Two copies of: type(2)=2049 (TRANSPORT_FORMAT_LIST) len(2)=0, no - # formats, no padding -- 4 octets each, 8 octets together. - empty = (2049).to_bytes(2, 'big') + (0).to_bytes(2, 'big') - self.assertEqual(len(empty), 4) + # formats, then the four octets of padding RFC 7401 5.2.1 requires of a + # parameter with no contents at all -- 11 + 0 - (0 + 3) % 8 == 8, so 8 + # octets each and 16 together. + empty = (2049).to_bytes(2, 'big') + (0).to_bytes(2, 'big') + bytes(4) + self.assertEqual(len(empty), 8) raw = fixed + empty * 2 proto = HIP(raw, len(raw), extension=True) @@ -2005,20 +2009,22 @@ def test_hip_transport_format_list_parameter_parses_the_full_declared_length(sel from pcapkit.const.hip.parameter import Parameter from pcapkit.protocols.internet.hip import HIP - # next(1) len(1)=7 pkt(1) ver(1)=0x01 (the reserved bit that must be 1) + # next(1) len(1)=8 pkt(1) ver(1)=0x01 (the reserved bit that must be 1) # checksum(2) control(2) shit(16) rhit(16) -- the fixed 40-octet header, - # declaring one 24-octet parameter area to follow: (7 - 4) * 8 == 24. - fixed = bytes([0x3b, 0x07, 0x00, 0x01]) + bytes(2) + bytes(2) + bytes(16) + bytes(16) + # declaring one 32-octet parameter area to follow: (8 - 4) * 8 == 32. + fixed = bytes([0x3b, 0x08, 0x00, 0x01]) + bytes(2) + bytes(2) + bytes(16) + bytes(16) self.assertEqual(len(fixed), 40) # Two copies of: type(2)=2049 len(2)=8, four two-octet format entries - # (10, 20, 30, 40), no padding needed (8 is already a multiple of - # eight under this module's padding rule, which pads the *contents* - # to eight and ignores the four-octet type-and-length header) -- - # 12 octets each, 24 octets together. + # (10, 20, 30, 40), then four octets of padding. Eight octets of + # contents are 8-aligned on their own, which is exactly the residue at + # which the pre-#651 rule (align the contents, ignore the four-octet + # type-and-length header) appended nothing and left the record four + # octets short; RFC 7401 5.2.1 asks for 11 + 8 - (8 + 3) % 8 == 16 -- + # so 16 octets each and 32 together. one = (2049).to_bytes(2, 'big') + (8).to_bytes(2, 'big') + b''.join( - n.to_bytes(2, 'big') for n in (10, 20, 30, 40)) - self.assertEqual(len(one), 12) + n.to_bytes(2, 'big') for n in (10, 20, 30, 40)) + bytes(4) + self.assertEqual(len(one), 16) raw = fixed + one * 2 proto = HIP(raw, len(raw), extension=True) @@ -2239,21 +2245,22 @@ def test_hip_nat_traversal_mode_and_esp_transform_survive_the_full_parser(self) Before this fix, feeding a maker-built ``modes=[1]`` parameter through the full parser did not reproduce the *same* phantom-entry symptom the direct schema round trip shows -- the single 11-octet - parameter this module's ``len`` arithmetic produces is not a - multiple of eight, so :meth:`HIP.make` raises ``ProtocolError: - HIPv2: invalid format`` before a packet even exists to parse, and a - hand-built two-copy packet (mimicking the ``HIP_COPIES = 2`` trick - ``examples/generators/options.py`` uses for exactly this alignment - reason) instead corrupts the second copy and emits ``SchemaWarning: - packet length < 0``. Either way the full parser does not silently - return a wrong value; it fails outright, which is what this test - pins now that the fix makes it succeed instead. - - Two copies land on an 8-octet boundary the same way - :class:`TransportFormatListParameter`'s equivalent test does, since - a single non-empty parameter here is always ``4 (mod 8)``: the - padding rule pads the *contents* to eight and ignores the - four-octet type-and-length header. + parameter this module's ``len`` arithmetic produced was not a + multiple of eight, so :meth:`HIP.make` raised ``ProtocolError: + HIPv2: invalid format`` before a packet even existed to parse, and a + hand-built two-copy packet instead corrupted the second copy and + emitted ``SchemaWarning: packet length < 0``. Either way the full + parser did not silently return a wrong value; it failed outright, + which is what this test pins now that the fix makes it succeed + instead. + + Since #651 both parameters are 8-aligned on their own -- ``len = 4`` + (two ``reserved`` octets and one two-octet entry) is exactly the + residue at which :rfc:`7401` Section 5.2.1 wants no padding at all, + ``11 + 4 - (4 + 3) % 8 == 8``, and at which the old contents-aligning + rule appended four octets that must not have been there. The two + copies stay, because two consecutive parameters are what prove the + reader's stride rather than merely its handling of one record. """ from pcapkit.const.hip.parameter import Parameter @@ -2264,14 +2271,14 @@ def test_hip_nat_traversal_mode_and_esp_transform_survive_the_full_parser(self) nat_schema = proto._make_param_nat_traversal_mode( Parameter.NAT_TRAVERSAL_MODE, version=2, modes=[1]) nat_one = bytes(nat_schema) - self.assertEqual(nat_one, bytes.fromhex('026000040000000100000000')) - self.assertEqual(len(nat_one) % 8, 4) + self.assertEqual(nat_one, bytes.fromhex('0260000400000001')) + self.assertEqual(len(nat_one) % 8, 0) esp_schema = proto._make_param_esp_transform( Parameter.ESP_TRANSFORM, version=2, suites=[1]) esp_one = bytes(esp_schema) - self.assertEqual(esp_one, bytes.fromhex('0fff00040000000100000000')) - self.assertEqual(len(esp_one) % 8, 4) + self.assertEqual(esp_one, bytes.fromhex('0fff000400000001')) + self.assertEqual(len(esp_one) % 8, 0) for one, code, attr in ( (nat_one, Parameter.NAT_TRAVERSAL_MODE, 'mode_id'), @@ -2995,6 +3002,376 @@ def test_hip_puzzle_and_solution_accept_an_explicit_field_width(self) -> None: self.assertEqual(proto._make_param_puzzle( Parameter.PUZZLE, parsed, version=2, rhash_len=128, ).len, 20) + def test_hip_parameter_total_length_matches_the_rfc_7401_formula(self) -> None: + """#651: a HIP parameter's *total* length is what must be 8-aligned. + + :rfc:`7401` Section 5.2.1 states the arithmetic outright, so this test + compares against the RFC rather than against pcapkit -- + + :: + + All of the encoded TLV parameters have a length (that includes the + Type and Length fields), which is a multiple of 8 bytes. + + Total Length = 11 + Length - (Length + 3) % 8; + + -- and that matters more here than usual, because **pcapkit round-trips + its own output whatever this formula says**: the writer and the reader + shared one wrong expression, so their disagreement with a real peer was + invisible to every construct-parse-construct test in the suite. Only an + independent statement of the RFC's arithmetic can see it, which is what + ``rfc_total`` below is. It is written out longhand from the RFC text and + deliberately does *not* call + :func:`~pcapkit.protocols.schema.internet.hip.parameter_total_len`, + since comparing an implementation with itself asserts nothing. + + Every ``Length`` from 0 to 63 is checked, which is what makes the widths + discriminate. The defect was exactly ``4 (mod 8)``, so it is not enough + to test a handful of convenient values: + + * ``Length = 4``, a whole ``SEQ``, and ``Length = 20``, a whole + ``SOLUTION``: contents plus the four-octet header are *already* + 8-aligned, so the RFC wants **no padding at all**. The old rule + appended four octets that must not be there, and a rule that dropped + the outer ``% 8`` -- ``8 - (Length + 4) % 8`` rather than + ``(8 - (Length + 4) % 8) % 8`` -- would append eight. Only the + residue ``Length % 8 == 4`` separates the correct answer from both. + * ``Length = 0``, ``8``, ``16``: contents are 8-aligned on their own, so + the old rule appended nothing and left the record four octets short. + This is the residue at which the defect *under*-pads, and it is the + one a test of "is the result at least as long as the contents" cannot + see. + * ``Length`` not a multiple of four -- 1, 2, 3, 5, 6, 7 -- where the pad + is 3, 2, 1, 7, 6, 5. A formula that only ever moved in steps of four, + which both the old and the fixed one look like at a glance, is caught + here and nowhere else. + + The old and the correct formula never agree, at any ``Length``: there is + no residue at which this test would have passed before the fix. + + """ + from pcapkit.protocols.schema.internet import hip as hip_schema + + def rfc_total(length: int) -> int: + """RFC 7401 5.2.1, transcribed rather than imported.""" + return 11 + length - (length + 3) % 8 + + def pre_651_total(length: int) -> int: + """What every one of the 95 padding sites computed before #651.""" + return 4 + length + (8 - (length % 8)) % 8 + + for length in range(64): + with self.subTest(length=length): + total = hip_schema.parameter_total_len(length) + self.assertEqual(total, rfc_total(length)) + # the property the RFC gives the formula *for* + self.assertEqual(total % 8, 0) + # padding is "0-7 bytes, added if needed", and the record is the + # header, the contents and that padding, with nothing left over + padding = hip_schema.parameter_padding_len({'len': length}) + self.assertEqual(4 + length + padding, total) + self.assertGreaterEqual(padding, 0) + self.assertLessEqual(padding, 7) + # and the defect is gone at every single residue, not on average + self.assertNotEqual(total, pre_651_total(length)) + + # Which parameters this governs, stated rather than implied: every padding + # site in ``schema/internet/hip.py`` and every reported record length in + # ``internet/hip.py`` routes through these two helpers -- 45 of the 46 + # parameter schemas and 48 of the 49 reported lengths -- with exactly one + # exclusion, ``LOCATOR_SET``, kept on the pre-#651 expression on purpose. + # See ``LocatorSetParameter.padding`` for why, and #679 for the fix. The + # counts are asserted directly in + # :meth:`test_hip_padding_helpers_cover_every_parameter_but_locator_set`, + # so the exclusion cannot silently grow to two. + # + # The sweep above covers all eight residues, which is enough for any + # formula periodic in ``Length % 8`` -- but not for one that is not. + # A ``parameter_total_len`` that masked its argument (``length & 0xFF``, + # say) agrees on 0..63 and diverges at 256, and nothing in the generator + # builds a parameter that long, so the whole suite would pass. ``len`` is + # an unsigned 16-bit field, so check the field's entire domain; it is one + # cheap loop and it closes that gap outright. + diverged = [length for length in range(65536) + if hip_schema.parameter_total_len(length) != rfc_total(length)] + self.assertEqual( + diverged[:16], [], + f'parameter_total_len diverges from RFC 7401 5.2.1 at ' + f'{len(diverged)} of the 65536 representable Length values, first ' + f'at {diverged[:16]}' + ) + + # the three spot values worth naming, so a regression reads as a number + # rather than as a loop index + self.assertEqual(hip_schema.parameter_total_len(4), 8) # SEQ: no padding + self.assertEqual(hip_schema.parameter_total_len(8), 16) # was 12, short by 4 + self.assertEqual(hip_schema.parameter_total_len(20), 24) # SOLUTION: was 28 + self.assertEqual(hip_schema.parameter_padding_len({'len': 4}), 0) + self.assertEqual(hip_schema.parameter_padding_len({'len': 8}), 4) + + # unreachable from real wire bytes (``len`` is unsigned on the wire), but + # a direct construction call could pass a negative ``len``, for which the + # RFC formula answers 8 -- a "total" shorter than the header alone. Keep + # it to the same floor-and-raise discipline as the other length helpers. + from pcapkit.utilities.exceptions import FieldValueError + with self.assertRaisesRegex(FieldValueError, 'invalid parameter length'): + hip_schema.parameter_total_len(-1) + with self.assertRaisesRegex(FieldValueError, 'invalid parameter length'): + hip_schema.parameter_padding_len({'len': -1}) + + def test_hip_padding_helpers_cover_every_parameter_but_locator_set(self) -> None: + """#651/#679: the exclusion is exactly one parameter, and it is ``LOCATOR_SET``. + + #651 routed every HIP padding site through + :func:`~pcapkit.protocols.schema.internet.hip.parameter_padding_len`, with + one deliberate exception: ``LOCATOR_SET``, whose own two defects cancel so + exactly that correcting its padding alone would take a conformant parameter + to four octets short. #679 fixes that pair together. + + A deliberate exception needs a guard, or it grows. Two failure modes this + catches, and nothing else does: + + * **The exclusion spreading.** A later change that reverts a second + parameter to the old expression -- to make some other pinned literal + pass, say -- would be indistinguishable from this one by inspection. + * **The exclusion evaporating.** Someone "finishing" #651 by pointing + ``LocatorSetParameter.padding`` at the helper would make that parameter + emit ``24n + 4`` octets where the RFC wants ``24n + 8``, and no existing + assertion would fail: the corekit field test's literals are what + ``main`` emits, so they would go red, but from a file whose connection to + HIP padding is not obvious from its name. + + This reads the declared field objects rather than the module source, so it + is about what the schemas *do*, not about how they are written. + + """ + from pcapkit.const.hip.parameter import Parameter + from pcapkit.protocols.schema.internet import hip as hip_schema + + on_helper = [] # type: list[str] + excluded = [] # type: list[str] + unpadded = [] # type: list[str] + for name in dir(hip_schema): + obj = getattr(hip_schema, name) + if not (isinstance(obj, type) + and issubclass(obj, hip_schema.Parameter) + and obj is not hip_schema.Parameter): + continue + field = obj.__fields__.get('padding') + if field is None: + unpadded.append(name) + continue + callback = getattr(field, '_length_callback', None) + if callback is hip_schema.parameter_padding_len: + on_helper.append(name) + else: + excluded.append(name) + + self.assertEqual( + excluded, ['LocatorSetParameter'], + 'exactly one HIP parameter schema may sit outside ' + 'parameter_padding_len, and it is LocatorSetParameter (see #679). ' + 'If this list grew, the narrowing of #651 has leaked; if it emptied, ' + 'LOCATOR_SET now emits four octets too few.' + ) + + # Three schemas declare no padding field at all, which is correct rather + # than an omission and is unchanged by #651: their contents are a fixed + # 20 octets (a two-octet port, two reserved and a 16-octet address), and + # 11 + 20 - (20 + 3) % 8 == 24 == 4 + 20, so the RFC asks for no padding. + # Measured on this tree and on b34f132f6: all three pack to 24 octets. + # Listed explicitly because a *fourth* name appearing here would mean a + # parameter had quietly lost its padding field. + self.assertEqual( + sorted(unpadded), + ['RegFromParameter', 'RelayFromParameter', 'RelayToParameter']) + + self.assertEqual(len(on_helper), 45) + self.assertEqual(len(on_helper) + len(excluded) + len(unpadded), 49) + + # and the excluded one is the schema registered for LOCATOR_SET, not some + # similarly-named class that merely sorts next to it + self.assertIs(hip_schema.Parameter.registry[Parameter.LOCATOR_SET], + hip_schema.LocatorSetParameter) + + def test_hip_parameter_records_are_eight_octet_aligned_on_the_wire(self) -> None: + """#651: the octets a real parameter packs, not just the arithmetic. + + :func:`~pcapkit.protocols.schema.internet.hip.parameter_total_len` being + right is necessary but not sufficient -- all 46 padding sites in the + schema module have to *use* it. So this packs real parameter schemas, + chosen to cover the residues that discriminate, and measures the octets. + + ``SEQ`` is the one to read first. It carries a single four-octet Update + ID, so it is complete in eight octets and needs no padding whatsoever; + pre-#651 pcapkit emitted twelve, appending four octets a conformant + receiver would read as the start of the next parameter. :rfc:`7401` + Section 5.3.5 puts a ``SEQ`` or an ``ACK`` on every ``UPDATE``, so this + is not a corner of the parameter space. + + The last assertion is the stride check: two consecutive parameters + through the full parser. A wrong padding rule still reads a *lone* + parameter correctly -- it only lands in the wrong place at the start of + the next one -- so one record cannot distinguish any padding rule from + any other. + + Nothing here imports + :func:`~pcapkit.protocols.schema.internet.hip.parameter_total_len`: the + RFC's formula is written out inline, so on a pre-#651 tree this fails + with a real octet-count mismatch rather than with an + :exc:`AttributeError` for a helper that does not exist there yet. + + """ + from pcapkit.const.hip.parameter import Parameter + from pcapkit.protocols.internet.hip import HIP + + proto = object.__new__(HIP) + + # (maker, kwargs, declared Length, total octets, pre-#651 octets) + cases = [ + ('_make_param_seq', {'update_id': 0x01020304}, 4, 8, 12), + ('_make_param_esp_info', {}, 12, 16, 20), + # #608 sizes SOLUTION's ``len`` from the operands' own widths, so + # reaching the RFC's ``Length = 20`` needs eight octets of each: + # 4 + 2 * ceil(57 / 8) == 20. No ``lifetime=`` here: an earlier + # revision of this case passed one to dodge #654's ``math.log2(0)``, + # and #665 removed that keyword, after which it survived only by + # being swallowed by ``**kwargs``. Dead keywords in a table like this + # read as requirements, so it is gone. + ('_make_param_solution', {'index': 1, 'opaque': b'op', + 'random': 1 << 56, 'solution': 1 << 56}, + 20, 24, 28), + ('_make_param_unassigned', {'contents': b''}, 0, 8, 4), + ('_make_param_unassigned', {'contents': b'x'}, 1, 8, 12), + ('_make_param_unassigned', {'contents': b'x' * 5}, 5, 16, 12), + ('_make_param_unassigned', {'contents': b'x' * 8}, 8, 16, 12), + ] + code_for = { + '_make_param_seq': Parameter.SEQ, + '_make_param_esp_info': Parameter.ESP_INFO, + '_make_param_solution': Parameter.SOLUTION, + '_make_param_unassigned': Parameter.Unassigned_512, + } + + for meth_name, kwargs, declared, total, pre_651 in cases: + with self.subTest(maker=meth_name, length=declared): + schema = getattr(proto, meth_name)( + code_for[meth_name], version=2, **kwargs) + packed = bytes(schema) + self.assertEqual(schema.len, declared) + self.assertEqual(len(packed), total) + self.assertEqual(len(packed) % 8, 0) + self.assertEqual(len(packed), 11 + declared - (declared + 3) % 8) + # the octet count that would have been emitted before the fix, + # so the case is stated as a difference rather than a value + self.assertNotEqual(len(packed), pre_651) + # any padding present is zeroed, as 5.2.1 requires of the sender + self.assertEqual(packed[4 + declared:], bytes(total - 4 - declared)) + + seq = bytes(proto._make_param_seq(Parameter.SEQ, version=2, + update_id=0x01020304)) + self.assertEqual(seq, bytes.fromhex('0181000401020304')) + + # the stride: two SEQs back to back, read through the full parser. The + # 40-octet fixed header declares (len - 4) * 8 == 16 octets to follow. + fixed = bytes([0x3b, 0x06, 0x00, 0x01]) + bytes(2) + bytes(2) + bytes(16) + bytes(16) + self.assertEqual(len(fixed), 40) + parsed = HIP(fixed + seq * 2, 40 + 16, extension=True) + copies = parsed.info.parameters.getlist(Parameter.SEQ) + self.assertEqual(len(copies), 2) + for copy in copies: + self.assertEqual(copy.id, 0x01020304) + # the record length the data model reports is the RFC total, not the + # contents-aligned one -- this is the 49 sites in the protocol module + self.assertEqual(copy.length, 8) + + # and the header's own ``len`` is now exact rather than exact-in-pairs: + # 4 + 16 // 8 == 6, with nothing lost to the floor division. + rebuilt = bytes(HIP(parameters=[(Parameter.SEQ, {'update_id': 0x01020304})] * 2, + extension=True, next=6, packet=1, version=2, + checksum=b'\x00\x00', controls_anonymous=False, + shit=0, rhit=0, payload=b'')) + self.assertEqual(rebuilt[1], 6) + self.assertEqual(rebuilt[40:], seq * 2) + + def test_hip_encrypted_data_length_excludes_reserved_and_iv(self) -> None: + """#651: ``ENCRYPTED``'s ``data`` field had to be fixed with the padding. + + :rfc:`7401` Section 5.2.18 puts ``Reserved``, ``IV`` and the encrypted + data all inside ``Length``, and + ``_make_param_encrypted`` writes ``len = 4 + len(iv) + len(data)`` to + match -- but the ``data`` field's length callback subtracted only the + ``iv``, so it claimed four octets more than the parameter holds. + + The two defects cancelled at some residues of ``Length`` and not others. + Measured across all eight, the old total agreed with the RFC at + ``Length % 8`` in ``{0, 5, 6, 7}`` and was eight octets over at + ``{1, 2, 3, 4}`` -- so ``Length = 8`` is one of the four where the + module emitted RFC-conformant ``ENCRYPTED`` octets while getting both + halves wrong. Fixing the padding alone would have taken ``ENCRYPTED`` + from right at four of the eight residues to four octets too long at all + eight, so the pair is asserted here together: ``Length = 8`` (where they + used to cancel) and ``Length = 4`` (where they did not, and the record + used to be eight octets over). + + """ + from pcapkit.const.hip.cipher import Cipher + from pcapkit.const.hip.parameter import Parameter + from pcapkit.protocols.internet.hip import HIP + from pcapkit.protocols.schema.internet import hip as hip_schema + from pcapkit.utilities.exceptions import FieldValueError + + proto = object.__new__(HIP) + + # Length = 8: four ``reserved`` octets and four of data. The record is + # 16 octets, which is also what the pre-#651 tree emitted -- by the two + # errors cancelling rather than by either being right. + schema = proto._make_param_encrypted( + Parameter.ENCRYPTED, version=2, cipher=Cipher.NULL_ENCRYPT, + data=b'DATA') + self.assertEqual(schema.len, 8) + self.assertEqual(bytes(schema), + bytes.fromhex('028100080000000044415441') + bytes(4)) + self.assertEqual(len(bytes(schema)), 16) + + # Length = 4: no data at all, and the record is 8 octets. Pre-#651 this + # packed 16 -- the ``data`` field zero-extending b'' out to four octets + # and the padding rule adding four more. + empty = proto._make_param_encrypted( + Parameter.ENCRYPTED, version=2, cipher=Cipher.NULL_ENCRYPT, data=b'') + self.assertEqual(empty.len, 4) + self.assertEqual(bytes(empty), bytes.fromhex('0281000400000000')) + + # The residue claim in the docstring, asserted rather than asserted in + # prose. The old record total was ``8 + Length + (-Length % 8)``: four + # octets of ``reserved``, a ``data`` field four octets too wide, and + # contents-aligned padding. It agreed with the RFC at exactly four of + # the eight residues -- which is why a suite that only ever built + # ``Length = 8`` could not see either defect. + def old_total(length: int) -> int: + return 8 + length + (-length % 8) + + def rfc_total(length: int) -> int: + return 11 + length - (length + 3) % 8 + + self.assertEqual( + sorted({L % 8 for L in range(64) if old_total(L) == rfc_total(L)}), + [0, 5, 6, 7]) + self.assertEqual( + sorted({L % 8 for L in range(64) if old_total(L) != rfc_total(L)}), + [1, 2, 3, 4]) + + # the callback itself, at the three shapes that matter + self.assertEqual(hip_schema.encrypted_data_len({'len': 4}), 0) + self.assertEqual(hip_schema.encrypted_data_len({'len': 8}), 4) + self.assertEqual( + hip_schema.encrypted_data_len({'len': 24, 'iv': b'\x11' * 16}), 4) + # a ``Length`` too short for the fields already read must raise rather + # than drive the data length negative + with self.assertRaisesRegex(FieldValueError, 'invalid parameter length'): + hip_schema.encrypted_data_len({'len': 3}) + with self.assertRaisesRegex(FieldValueError, 'invalid parameter length'): + hip_schema.encrypted_data_len({'len': 19, 'iv': b'\x11' * 16}) if __name__ == '__main__': diff --git a/tests/protocols/test_option_roundtrip_unit.py b/tests/protocols/test_option_roundtrip_unit.py index f06f8ae47..55baedc07 100644 --- a/tests/protocols/test_option_roundtrip_unit.py +++ b/tests/protocols/test_option_roundtrip_unit.py @@ -29,9 +29,15 @@ leave the cycle closed -- the generator constructing and reconstructing the same wrong octets, which match each other and so match the assertion. Those are pinned as tests of their own rather than as entries, since an entry would have to -record ``'OK'`` as a failure: -:meth:`OptionRoundTripTests.test_a_single_hip_parameter_cannot_be_constructed` -for the HIP header arithmetic the generator's ``HIP_COPIES`` routes around. +record ``'OK'`` as a failure. HIP's parameter padding was the standing example: +it aligned the contents to eight rather than the record, so every parameter was +``4 (mod 8)``, the header ``len`` field could not represent a lone one, and the +generator's ``HIP_COPIES`` put two in a packet to make the arithmetic come out. +A round trip could never see any of it, because pcapkit's writer and reader +shared the error. #651 fixed it against :rfc:`7401` Section 5.2.1 rather than +against a round trip, and +:meth:`OptionRoundTripTests.test_a_hip_packet_carrying_one_parameter_round_trips` +is the same pin, now asserting the case closes. IPv4's ``SID`` option width was the other one, tracked as #534 and pinned here by a ``test_a_parsed_sid_option_re_emits_two_octets_too_wide`` that no longer exists: @@ -58,9 +64,10 @@ none of the assertions below has been loosened to make a failing case pass. The one place where an argument was chosen to route *around* a defect rather than into it is HIP's ``HIP_COPIES``, which puts two copies of each parameter in a -packet because one is unrepresentable; the defect that forces it is not lost, -:meth:`OptionRoundTripTests.test_a_single_hip_parameter_cannot_be_constructed` -pins it directly. +packet. One copy is representable since #651 and the constant is no longer about +padding -- see its note in the generator for the four unrelated defects that +keep it at two -- and the single-parameter case is asserted directly by +:meth:`OptionRoundTripTests.test_a_hip_packet_carrying_one_parameter_round_trips`. This module is unit tier: it constructs its own octets and reads no capture, so it runs on a fresh checkout with nothing generated. @@ -386,18 +393,19 @@ class Gap(NamedTuple): 'pcapkit/protocols/internet/hip.py:822 -- Parameter.registry[128] is ' 'UnassignedParameter, because R1CounterParameter declares code=129 only'), - # ``_make_param_encrypted`` passes ``cipher=``, which is not a field of - # ``EncryptedParameter`` -- so the cipher id is dropped with an - # ``UnknownFieldWarning`` and never reaches the wire. The mismatch itself - # comes from a second defect in the same parameter: the ``data`` length - # callback omits the four octets ``reserved`` already consumed out of - # ``len``, so ``len`` grows by four on every round trip (measured: 4 -> 8). - 'hip-parameter/ENCRYPTED': Gap( - 'MISMATCH', '', - 'pcapkit/protocols/internet/hip.py:3445 -- cipher= is not a field of ' - 'EncryptedParameter and is silently dropped; and ' - 'pcapkit/protocols/schema/internet/hip.py:463 -- the data length ' - "callback omits the 4 octets 'reserved' took out of len"), + # ``ENCRYPTED`` used to have an entry here, for two defects at once: that + # ``_make_param_encrypted`` passed ``cipher=``, a keyword + # ``EncryptedParameter`` does not accept, so the cipher id was dropped with + # an ``UnknownFieldWarning`` and never reached the wire; and that the + # ``data`` length callback omitted the four octets ``reserved`` had already + # taken out of ``len``, so ``len`` grew by four on every round trip + # (measured: 4 -> 8). The first was fixed by #556. The second was fixed + # alongside #651, because the two four-octet errors cancelled at four of the + # eight residues of ``Length`` -- measured, ``Length % 8`` in {0, 5, 6, 7} -- + # so correcting the padding on its own would have turned "right at four + # residues" into "four octets too long at all eight". With both gone the + # cycle closes and the entry is deleted rather than kept as documentation of + # a defect that is no longer there. # Two parameters whose own packed length is not what the header arithmetic # can represent even in pairs -- see HIP_COPIES in the generator for why the @@ -728,40 +736,61 @@ def test_round_trip_is_identity_or_a_recorded_gap(self) -> None: f'{outcome.detail!r}' ) - def test_a_single_hip_parameter_cannot_be_constructed(self) -> None: - """A HIP packet carrying exactly one parameter is rejected by its own reader. + def test_a_hip_packet_carrying_one_parameter_round_trips(self) -> None: + """A HIP packet carrying exactly one parameter is accepted by its own reader. - This is the defect the generator's ``HIP_COPIES = 2`` routes around, and - it is pinned here so that routing around it does not also bury it. + This assertion used to run the other way, as + ``test_a_single_hip_parameter_cannot_be_constructed``: it required the + library to *reject* its own single-parameter packets, which it did, and + pinned the defect the generator's ``HIP_COPIES = 2`` routes around so + that routing around it did not also bury it. ``HIP.make`` computes the header's ``len`` as ``total_length // 8 + 4``, which is lossless only when the parameter octets are a multiple of eight. - The parameter padding rule pads the *contents* to eight and ignores the - four-octet type-and-length header, so one parameter is always - ``4 (mod 8)``; the floor division drops those four octets, and - ``_read_hip_param`` compares the recovered length exactly and raises. - - Two copies sum to a multiple of eight, so the same parameter that fails - alone succeeds in a pair -- which is the control that makes this a - statement about the header arithmetic rather than about ``SEQ``. + Every padding site in the two HIP modules aligned the *contents* to + eight and ignored the four-octet type-and-length header, so one + parameter was always ``4 (mod 8)``; the floor division dropped those four + octets and ``_read_hip_param``, which compares the recovered length + exactly, raised. #651 made the padding :rfc:`7401` Section 5.2.1's + ``Total Length = 11 + Length - (Length + 3) % 8``, under which a lone + parameter is 8-aligned by construction, so the case now closes and this + test says so positively rather than recording the raise. + + Both halves of the old test are kept, because the pair was its control: + one copy and two copies must *both* work, and must both come back as the + octets they went out as. ``SEQ`` is the case that discriminates hardest, + at ``Length = 4``: the record needs no padding at all + (``11 + 4 - (4 + 3) % 8 == 8``), so the old rule's four appended octets + were pure surplus rather than a shortfall, and a reader that pads by any + non-zero amount lands in the wrong place at the start of the second copy. + + ``HIP_COPIES`` itself stays at two, for reasons that are no longer this + one; its own note in the generator says which. """ from pcapkit.const.hip.parameter import Parameter from pcapkit.protocols.internet.hip import HIP - from pcapkit.utilities.exceptions import ProtocolError base = dict(self.options.HIP_BASE) one = [(Parameter.SEQ, {})] # type: list[tuple[Any, dict[str, Any]]] - with self.assertRaises(ProtocolError) as caught: - HIP(parameters=one, extension=True, **base) - self.assertIn('invalid format', str(caught.exception)) + for copies in (1, 2): + with self.subTest(copies=copies): + built = bytes(HIP(parameters=one * copies, extension=True, **base)) + + # 40 octets of fixed header, then one 8-octet SEQ per copy -- + # the RFC total for Length = 4, and what the header's own + # ``len`` field can represent exactly. + self.assertEqual(len(built), 40 + 8 * copies) + self.assertEqual(built[1], 4 + copies) + + reparsed = HIP(built, len(built), extension=True) + self.assertEqual( + len(reparsed.info.parameters.getlist(Parameter.SEQ)), copies) - # The control: the identical parameter, twice, round-trips exactly. - paired = bytes(HIP(parameters=one * 2, extension=True, **base)) - reparsed = HIP(paired, len(paired), extension=True) - again = bytes(HIP(parameters=reparsed.info.parameters, extension=True, **base)) - self.assertEqual(paired, again) + again = bytes(HIP(parameters=reparsed.info.parameters, + extension=True, **base)) + self.assertEqual(built, again) def test_recorded_gaps_are_a_minority(self) -> None: """Most of the option space round-trips, and the rest is accounted for.