diff --git a/examples/generators/options.py b/examples/generators/options.py index d11a3d69cd..48480ee408 100644 --- a/examples/generators/options.py +++ b/examples/generators/options.py @@ -976,11 +976,14 @@ def _mh_extension_build(code: 'Any', kwargs: 'dict[str, Any]') -> 'Any': #: ``Length`` had been counted in 4-octet units where the RFC counts bytes -- #: which is what moved the one-copy figure from 44 to 46, level with two copies. #: -#: Two remained, neither helped by a second copy since each fails at two copies -#: as well as at one: +#: Two remained after that, neither helped by a second copy since each failed at +#: two copies as well as at one. Both are closed now: #: -#: * ``HOST_ID`` declares ``len=8`` and packs 18. Recorded as -#: ``hip-parameter/HOST_ID``. +#: * ``HOST_ID`` declared four octets for the single 16-bit DI-Type/DI-Length +#: word of :rfc:`7401` Section 5.2.9, so it packed two surplus octets -- 18 at +#: this table's key, where ``len=8`` makes the record 16 -- and read a +#: conformant record two octets late, taking the curve as ``Algorithm``. #1118 +#: made the field two octets. #: * ``HIP_TRANSFORM`` is HIPv1-only -- ``_read_param_hip_transform`` raises for #: any other version -- and is now built at version 1 through #: :data:`HIP_VERSION`, so it round-trips and is no longer recorded. @@ -997,19 +1000,24 @@ def _mh_extension_build(code: 'Any', kwargs: 'dict[str, Any]') -> 'Any': #: So the pair was routing around nothing by the time #672, #679 and #690 had #: all landed, and dropping to one copy does not change what round-trips. #: Measured over this table's 49 HIP codes on ``5f0a1aa90`` (after #696, which -#: #689 itself waited on -- see the issue) and again after #690: +#: #689 itself waited on -- see the issue), again after #690, and again once +#: #1128 and #1118 had landed: #: #: ====================== ======== ========== #: tree one copy two copies #: ====================== ======== ========== #: ``5f0a1aa90`` 46 OK 46 OK #: after #690 47 OK 47 OK +#: after #1128 and #1118 49 OK 49 OK #: ====================== ======== ========== #: -#: ``HOST_ID`` is now the only case either setting fails, and neither its -#: ``status`` nor its ``defect`` moves between settings, so +#: ``HOST_ID`` and ``HIP_TRANSFORM`` were the only two cases either setting +#: failed at those measurements, and neither their ``status`` nor their +#: ``defect`` moved between settings, so #: :data:`tests.protocols.test_option_roundtrip_unit.EXPECTED_FAILURES` needed no -#: change to keep recording them accurately at one copy. +#: change to keep recording them accurately at one copy. #1128 and #1118 closed +#: them, which takes both columns to all 49 and leaves that table with no HIP +#: entries at all. #: #: The single-parameter case is asserted directly, and positively, by #: ``test_a_hip_packet_carrying_one_parameter_round_trips`` in diff --git a/pcapkit/protocols/schema/internet/hip.py b/pcapkit/protocols/schema/internet/hip.py index ee55d1de4f..677a9ad81a 100644 --- a/pcapkit/protocols/schema/internet/hip.py +++ b/pcapkit/protocols/schema/internet/hip.py @@ -942,7 +942,7 @@ class HostIDParameter(Parameter, code=Enum_Parameter.HOST_ID): hi_len: 'int' = UInt16Field() #: Domain ID type and length. di_data: 'DIData' = BitField( - length=4, + length=2, namespace={ 'type': (0, 4), 'len': (4, 12), diff --git a/tests/protocols/internet/test_hip_host_id_di_width_unit.py b/tests/protocols/internet/test_hip_host_id_di_width_unit.py new file mode 100644 index 0000000000..ec37c4f05e --- /dev/null +++ b/tests/protocols/internet/test_hip_host_id_di_width_unit.py @@ -0,0 +1,126 @@ +# -*- coding: utf-8 -*- +"""The HIP ``HOST_ID`` DI-Type/DI-Length word is two octets, not four. + +GitHub issue #1118. :rfc:`7401#section-5.2.9` packs ``DI-Type`` (4 bits) and +``DI Length`` (12 bits) into one 16-bit word between ``HI Length`` and +``Algorithm``. The schema declared that word as a four-octet +:class:`~pcapkit.corekit.fields.strings.BitField` while its namespace covered +only sixteen bits, so a made ``HOST_ID`` carried two surplus zero octets, and a +conformant one was read two octets late -- ``Algorithm`` came out as the ECDSA +curve that follows it. + +The DI word is non-zero in every case here (``FQDN``, two octets), because an +all-zero word reads the same at either width and cannot discriminate the defect. +""" +from __future__ import annotations + +import importlib.util +import struct +import unittest + +from tests._support import reimport_once_per_class + +RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') +HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) + +#: Header fields shared by every HIP packet built here, as in +#: :data:`examples.generators.options.HIP_BASE`. +HIP_BASE = { + 'next': 6, 'packet': 1, 'checksum': b'\x00\x00', + 'controls_anonymous': False, 'shit': 0, 'rhit': 0, 'payload': b'', +} + +PUB_KEY = b'\xaa\xbb\xcc\xdd' +DI = b'ab' + + +def rfc_host_id() -> 'bytes': + """A ``HOST_ID`` record laid out by hand from :rfc:`7401#section-5.2.9`. + + ECDSA (7) over NIST P-256 (1) with a four-octet public key, so ``HI Length`` + is six; ``DI-Type = 1`` (FQDN) and ``DI Length = 2`` share one word, + ``0x1002``. ``Length`` is ``2 + 2 + 2 + 6 + 2 = 14``, and the record is + padded to Section 5.2.1's ``11 + 14 - (14 + 3) % 8 = 24`` octets. + + Returns: + The whole record, type-and-length header and padding included. + + """ + body = struct.pack('!HHH', 6, (1 << 12) | len(DI), 7) + body += struct.pack('!H', 1) + PUB_KEY + DI + record = struct.pack('!HH', 705, len(body)) + body + return record + b'\x00' * ((-len(record)) % 8) + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class HIPHostIDDIWidthTests(unittest.TestCase): + """``HOST_ID``'s DI word width, against :rfc:`7401#section-5.2.9`.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + def test_made_host_id_packs_the_rfc_layout(self) -> None: + """A made ``HOST_ID`` is octet-for-octet the hand-built RFC record.""" + from pcapkit.const.hip.di import DITypes + from pcapkit.const.hip.ecdsa_curve import ECDSACurve + from pcapkit.const.hip.parameter import Parameter + from pcapkit.protocols.internet.hip import HIP + + schema = object.__new__(HIP)._make_param_host_id( # type: ignore[arg-type] + Parameter.HOST_ID, version=2, hi_curve=ECDSACurve.NIST_P_256, + hi_pub_key=PUB_KEY, di=DI, di_type=DITypes.FQDN) + + self.assertEqual(schema.len, 14) + # 26 octets before #1118: two zero octets after the DI word. + self.assertEqual(schema.pack().hex(), rfc_host_id().hex()) + + def test_rfc_host_id_parses_algorithm_and_di(self) -> None: + """A conformant record reads ``Algorithm = 7`` and the DI it carries.""" + from pcapkit.const.hip.di import DITypes + from pcapkit.const.hip.hi_algorithm import HIAlgorithm + from pcapkit.protocols.schema.internet.hip import HostIDParameter + + raw = rfc_host_id() + schema = HostIDParameter.unpack(raw, len(raw)) # type: ignore[arg-type] + + # Read as 1 -- the curve -- before #1118. + self.assertEqual(schema.algorithm, HIAlgorithm.ECDSA) + self.assertEqual(int(schema.algorithm), 7) + self.assertEqual(schema.di_data['type'], DITypes.FQDN) + self.assertEqual(schema.di_data['len'], len(DI)) + self.assertEqual(schema.hi.pub_key, PUB_KEY) # type: ignore[union-attr] + self.assertEqual(schema.di, DI) + self.assertEqual(schema.pack(), raw) + + def test_a_lone_host_id_survives_a_hip_packet(self) -> None: + """One ``HOST_ID`` in a real packet builds, parses and builds back. + + ``HIP.make`` derives the header's ``len`` from the parameter octets, so a + record that is not a multiple of eight raised ``HIPv2: invalid format`` + on the way back in. + """ + from pcapkit.const.hip.di import DITypes + from pcapkit.const.hip.ecdsa_curve import ECDSACurve + from pcapkit.const.hip.parameter import Parameter + from pcapkit.protocols.internet.hip import HIP + + options = {'hi_curve': ECDSACurve.NIST_P_256, 'hi_pub_key': PUB_KEY, + 'di': DI, 'di_type': DITypes.FQDN} + built = HIP(parameters=[(Parameter.HOST_ID, options)], + extension=True, version=2, **HIP_BASE).data + self.assertEqual(built[40:], rfc_host_id()) + + parsed = HIP(built, len(built), extension=True) + param = parsed.info.parameters[Parameter.HOST_ID] + self.assertEqual(int(param.algorithm), 7) + self.assertEqual(param.di_type, DITypes.FQDN) + self.assertEqual(param.di_len, len(DI)) + self.assertEqual(param.di, DI) + + again = HIP(parameters=parsed.info.parameters, extension=True, + version=2, **HIP_BASE).data + self.assertEqual(again, built) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/test_option_roundtrip_unit.py b/tests/protocols/test_option_roundtrip_unit.py index de6b2d58fb..f5c5089bca 100644 --- a/tests/protocols/test_option_roundtrip_unit.py +++ b/tests/protocols/test_option_roundtrip_unit.py @@ -375,13 +375,14 @@ class Gap(NamedTuple): # cycle closes and the entry is deleted rather than kept as documentation of # a defect that is no longer there. - # A parameter whose own packed length is not what the header arithmetic - # can represent, at one copy or two. The generator's HIP_COPIES note - # records it as the one HIP case that fails at either setting. - 'hip-parameter/HOST_ID': Gap( - 'CONSTRUCT', 'HIPv2: invalid format', - 'pcapkit/protocols/internet/hip.py:698 -- HOST_ID packs to 14 octets ' - 'with len=8, so it is not even 4-aligned'), + # ``HOST_ID`` and ``HIP_TRANSFORM`` were the last two entries here, and + # neither was helped by a second copy -- each failed at two copies as well as + # at one. #1128 built ``HIP_TRANSFORM`` at the HIPv1 its constructor demands, + # through the generator's ``HIP_VERSION``, instead of the version 2 the table + # had been using; #1118 made ``HOST_ID``'s DI-Type/DI-Length word the single + # 16-bit one of :rfc:`7401` Section 5.2.9, where the schema had declared four + # octets for it. Every HIP case round-trips now, at either setting of + # ``HIP_COPIES``, so no entries are needed here any more. # -- HTTP/2 ---------------------------------------------------------------