diff --git a/CHANGELOG.md b/CHANGELOG.md index f58d8553c9..180b2c8df9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -350,6 +350,7 @@ This resolves [#548](https://github.com/JarryShaw/PyPCAPKit/issues/548), which r - message text: the `epb_flags` and `pack_flags` length errors say `expected 4`, the length they check, rather than `expected 8` ([#2168](https://github.com/JarryShaw/PyPCAPKit/issues/2168), [#2179](https://github.com/JarryShaw/PyPCAPKit/pull/2179)), and the IPv4 TS option's unknown-flag warning and the Mobility Header's prefix error spell `timestamp` and `mobile` ([#2171](https://github.com/JarryShaw/PyPCAPKit/issues/2171), [#2189](https://github.com/JarryShaw/PyPCAPKit/pull/2189)). - IPX dissects its payload from a registry of its own, `IPX.__proto__`, keyed by IPX Packet Type, where it looked the Packet Type up as an IP protocol number, so a type 17 (NCP) payload parsed as `IPX:UDP` and type 4 as `IPX:IPv4`. No IPX-native protocol ships, so the payload is `Raw` unless one is registered ([#2169](https://github.com/JarryShaw/PyPCAPKit/issues/2169), [#2187](https://github.com/JarryShaw/PyPCAPKit/pull/2187)). - HIP's `ECHO_RESPONSE_UNSIGNED` maker builds an `EchoResponseUnsignedParameter` schema, where it built the request's; the octets were already right ([#2170](https://github.com/JarryShaw/PyPCAPKit/issues/2170), [#2188](https://github.com/JarryShaw/PyPCAPKit/pull/2188)). +- **a breaking change to** PCAP-NG's Interface Description Block maker: `snaplen` defaults to 0, which the PCAP-NG specification reads as no limit, where its default of `2**64 - 1` was truncated to `0xFFFFFFFF` by the 32-bit SnapLen field and the made `info` disagreed with the octets. A block made without `snaplen` writes 0 there, and a `snaplen` outside `0` to `0xFFFFFFFF` raises `ProtocolError` where it was truncated silently ([#2278](https://github.com/JarryShaw/PyPCAPKit/issues/2278)). ### pcapkit.toolkit diff --git a/docs/source/changelog/1.5.0.rst b/docs/source/changelog/1.5.0.rst index f083f7c020..fe84b483bb 100644 --- a/docs/source/changelog/1.5.0.rst +++ b/docs/source/changelog/1.5.0.rst @@ -2755,6 +2755,12 @@ Fixed * HIP's ``ECHO_RESPONSE_UNSIGNED`` maker builds an ``EchoResponseUnsignedParameter`` schema, where it built the request's; the octets were already right (:issue:`2170`, :pr:`2188`). +* **a breaking change to** PCAP-NG's Interface Description Block maker: ``snaplen`` + defaults to 0, which the PCAP-NG specification reads as no limit, where its default of + ``2**64 - 1`` was truncated to ``0xFFFFFFFF`` by the 32-bit SnapLen field and the made + ``info`` disagreed with the octets. A block made without ``snaplen`` writes 0 there, + and a ``snaplen`` outside ``0`` to ``0xFFFFFFFF`` raises :exc:`.ProtocolError` where it + was truncated silently (:issue:`2278`). pcapkit.toolkit --------------- diff --git a/pcapkit/protocols/misc/pcapng.py b/pcapkit/protocols/misc/pcapng.py index 6f7e5f7b6b..e6a01b085a 100644 --- a/pcapkit/protocols/misc/pcapng.py +++ b/pcapkit/protocols/misc/pcapng.py @@ -4478,7 +4478,7 @@ def _make_block_idb(self, block: 'Optional[Data_InterfaceDescriptionBlock]' = No linktype_default: 'Optional[int]' = None, linktype_namespace: 'Optional[dict[str, int] | dict[int, str] | Type[StdlibEnum] | Type[AenumEnum]]' = None, # pylint: disable=line-too-long linktype_reversed: 'bool' = False, - snaplen: 'int' = 0xFFFF_FFFF_FFFF_FFFF, + snaplen: 'int' = 0, options: 'Optional[Option | list[Schema_Option | tuple[Enum_OptionType, dict[str, Any]] | bytes]]' = None, **kwargs: 'Any') -> 'Schema_InterfaceDescriptionBlock': """Make PCAP-NG interface description block (IDB). @@ -4489,13 +4489,25 @@ def _make_block_idb(self, block: 'Optional[Data_InterfaceDescriptionBlock]' = No linktype_default: Default value of link layer protocol type. linktype_namespace: Namespace of link layer protocol type. linktype_reversed: Reversed flag for link layer protocol type namespace. - snaplen: Snap length. + snaplen: Snap length, an unsigned 32-bit value; 0, the default, + means no limit. options: Block options. **kwargs: Arbitrary keyword arguments. Returns: Constructed block schema. + Raises: + ProtocolError: If ``snaplen`` is outside ``0`` to ``0xFFFFFFFF``, + which the 32-bit SnapLen field would truncate (:issue:`2278`). + + .. versionchanged:: 1.5.0 + + ``snaplen``'s default changed from ``0xFFFF_FFFF_FFFF_FFFF`` -- which + the 32-bit field silently truncated to ``0xFFFFFFFF`` -- to ``0``, + and a value outside the field's range now raises instead of being + truncated (:issue:`2278`). + """ if block is not None and hasattr(block, 'block_raw'): return self._make_block_short(block) # type: ignore[return-value] @@ -4506,6 +4518,8 @@ def _make_block_idb(self, block: 'Optional[Data_InterfaceDescriptionBlock]' = No else: linktype_val = self._make_index(linktype, linktype_default, namespace=linktype_namespace, # type: ignore[assignment] reversed=linktype_reversed, pack=False) + if not 0 <= snaplen <= 0xFFFF_FFFF: + raise ProtocolError(f'PCAP-NG: [IDB] snaplen must be in range 0 to 0xFFFFFFFF: {snaplen!r}') if options is not None: options_value, total_length = self._make_pcapng_options(options, namespace='if') diff --git a/tests/protocols/misc/test_pcapng_idb_snaplen_default_2278_unit.py b/tests/protocols/misc/test_pcapng_idb_snaplen_default_2278_unit.py new file mode 100644 index 0000000000..0c4df3e862 --- /dev/null +++ b/tests/protocols/misc/test_pcapng_idb_snaplen_default_2278_unit.py @@ -0,0 +1,68 @@ +# -*- coding: utf-8 -*- +"""GitHub issue #2278: an Interface Description Block's snaplen fits its 32-bit field. + +Without the fix, ``_make_block_idb`` defaults ``snaplen`` to ``2**64 - 1``, which +the 32-bit SnapLen field truncates to ``0xFFFFFFFF``, so the made info disagrees +with the octets it parses back to, and an explicit ``snaplen`` of ``2**32`` or +more, or below zero, is truncated the same way. The default is 0, which the +PCAP-NG specification reads as no limit, and a value the field cannot hold +raises :exc:`~pcapkit.utilities.exceptions.ProtocolError`. + +:mod:`pcapkit` is imported inside each test, after +:func:`~tests._support.reimport_once_per_class`. + +""" + +import unittest +from typing import TYPE_CHECKING + +from tests.protocols.misc.test_pcapng_roundtrip2_unit import PCAPNGTestCase + +if TYPE_CHECKING: + from typing import Any + +H = bytes.fromhex + + +class TestPCAPNGIDBSnaplen(PCAPNGTestCase): + """Pin the snaplen an Interface Description Block is made with.""" + + def _make(self, **block: 'Any') -> 'Any': + """Make an Ethernet Interface Description Block from ``block``.""" + from pcapkit.const.pcapng.block_type import BlockType + from pcapkit.const.reg.linktype import LinkType + from pcapkit.protocols.misc.pcapng import PCAPNG + + return PCAPNG(num=1, sct=1, ctx=self._context(), type=BlockType.Interface_Description_Block, + block={'linktype': LinkType.ETHERNET, **block}) + + def test_default_is_no_limit(self) -> None: + """The issue's case: the default writes 0, and the info says 0.""" + made = self._make() + self.assertEqual(made.data.hex(), H('01000000 14000000 0100 0000 00000000 14000000').hex()) + self.assertEqual(made.info.snaplen, 0) # type: ignore[attr-defined] + + def test_made_info_matches_the_octets(self) -> None: + from pcapkit.protocols.misc.pcapng import PCAPNG + + for snaplen in (None, 0, 1, 0x40000, 0xFFFF_FFFF): + with self.subTest(snaplen=snaplen): + made = self._make() if snaplen is None else self._make(snaplen=snaplen) + parsed = PCAPNG(made.data, len(made.data), num=1, sct=1, ctx=self._context()) + self.assertEqual(made.info.snaplen, parsed.info.snaplen) # type: ignore[attr-defined] + self.assertEqual(made.info, parsed.info) + self.assertRebuilds(made.data) + + def test_out_of_range_is_refused(self) -> None: + from pcapkit.utilities.exceptions import ProtocolError + + for snaplen in (-1, 0x1_0000_0000, 0xFFFF_FFFF_FFFF_FFFF): + with self.subTest(snaplen=snaplen): + with self.assertRaises(ProtocolError) as context: + self._make(snaplen=snaplen) + self.assertTrue(str(context.exception).endswith(f': {snaplen!r}'), + str(context.exception)) + + +if __name__ == '__main__': + unittest.main()