diff --git a/docs/source/pcapkit/foundation/reassembly/ip/index.rst b/docs/source/pcapkit/foundation/reassembly/ip/index.rst index 056fa9361d..02978b5406 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/index.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/index.rst @@ -94,10 +94,17 @@ Because completion frees the buffer (:rfc:`791#section-3.2`), a fragment that arrives after its datagram has completed opens a new buffer, which is later reported as a separate incomplete datagram; this is intended (:issue:`1507`). -Two departures from the procedure above: +Three departures from the procedure above: * Only the final fragment's partial last ``RCVBT`` block is set. A non-final fragment carries a multiple of 8 octets, so one ending mid-block was cut or is malformed, and setting its last block would complete the datagram with octets nobody sent (:issue:`1567`). * A datagram longer than the data buffer is never complete (:issue:`1566`). +* Nor is a datagram longer than its length field can declare: 65,535 octets + with its header, all of which IPv4's Total Length counts and all but the + fixed 40 octets of which IPv6's Payload Length counts. Linux rejects such a + datagram too (:issue:`1585`). A fragment with Total Length 0, as TCP + segmentation offload and BIG TCP leave one, declares no length + (:issue:`1555`), so a datagram it carries past that bound grows the data + buffer instead, and only the buffer bounds it. diff --git a/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst b/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst index 11be797116..9b9eb9d127 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst @@ -155,9 +155,13 @@ Terminology :meth:`IP._detect_conflicts `. ``RCVBT`` records receipt in 8-octet blocks, which is coarser than the octet a conflict needs; ``TDL`` is what recovers the exact - extent for the one block that can be partially real -- the final - fragment's own tail -- so a conflict here is never wider than the - octets that genuinely disagreed, even inside that block. + extent for the one block marked received that can be partially real + -- the final fragment's own tail -- so a conflict here is never wider + than the octets that genuinely disagreed, even inside that block. + Any other partial last block -- a non-final, truncated or clipped + fragment's -- is left unmarked (:issue:`1567`), and its octets are + not compared: a later fragment that overwrites them records no + conflict (:issue:`1585`). .. note:: diff --git a/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst b/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst index a4550b196e..c81cdfbcaa 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst @@ -198,6 +198,10 @@ Terminology :meth:`IP._detect_conflicts `. ``RCVBT`` records receipt in 8-octet blocks, which is coarser than the octet a conflict needs; ``TDL`` is what recovers the exact - extent for the one block that can be partially real -- the final - fragment's own tail -- so a conflict here is never wider than the - octets that genuinely disagreed, even inside that block. + extent for the one block marked received that can be partially real + -- the final fragment's own tail -- so a conflict here is never wider + than the octets that genuinely disagreed, even inside that block. + Any other partial last block -- a non-final, truncated or clipped + fragment's -- is left unmarked (:issue:`1567`), and its octets are + not compared: a later fragment that overwrites them records no + conflict (:issue:`1585`). diff --git a/pcapkit/foundation/reassembly/data/ip.py b/pcapkit/foundation/reassembly/data/ip.py index 9ec767db55..942f2378c8 100644 --- a/pcapkit/foundation/reassembly/data/ip.py +++ b/pcapkit/foundation/reassembly/data/ip.py @@ -128,14 +128,17 @@ class Datagram(DeferredPacket, Info, Generic[_AT]): #: #: :attr:`Buffer.RCVBT ` #: only records receipt in 8-octet blocks, coarser than the octet - #: granularity a conflict needs. Every fragment but the last is required - #: to be block-aligned (and :attr:`Packet.fo` is *always* a multiple of 8, - #: being wire-encoded in 8-octet units), so the only block that can be - #: partially real is the one holding the final fragment's own tail -- and - #: a range reported here never extends past + #: granularity a conflict needs, and an octet is compared only if its + #: block is marked. :attr:`Packet.fo` is *always* a multiple of 8, being + #: wire-encoded in 8-octet units, and only the final fragment marks a + #: partial last block, so the only marked block that can be partially real + #: is the one holding the final fragment's own tail -- and a range reported + #: here never extends past #: :attr:`Buffer.TDL ` #: for exactly that reason, even though a whole ``RCVBT`` block straddling - #: it reads as "received". See + #: it reads as "received". Any other partial last block -- a non-final, + #: truncated or clipped fragment's -- is left unmarked, so a later fragment + #: that overwrites its octets records no conflict. See #: :meth:`IP._detect_conflicts `. conflict: 'tuple[tuple[int, int], ...]' diff --git a/pcapkit/foundation/reassembly/ip.py b/pcapkit/foundation/reassembly/ip.py index 7ad4493413..675b42806f 100644 --- a/pcapkit/foundation/reassembly/ip.py +++ b/pcapkit/foundation/reassembly/ip.py @@ -29,6 +29,11 @@ __all__ = ['IP'] +#: The most octets a 16-bit length field declares -- IPv4's Total Length, the +#: header included, and IPv6's Payload Length, the fixed header excluded -- and +#: so the length the data buffer is preallocated to. +_MAX_LENGTH = 65535 + class IP(ReassemblyBase[Packet[_AT], Datagram[_AT], BufferID, Buffer[_AT]], Generic[_AT]): # pylint: disable=abstract-method """Reassembly for IP payload. @@ -44,7 +49,7 @@ class IP(ReassemblyBase[Packet[_AT], Datagram[_AT], BufferID, Buffer[_AT]], Gene does not: it is left out of the runs and ends the prefix, but once the total length is known the contiguous payload keeps its real octets, with only the rest of the hole zero-filled. A datagram - longer than the data buffer is never complete + longer than its length field can declare is never complete store: if store reassembled datagram in memory, i.e., :attr:`self._dtgram ` (if not, datagram will be discarded after callback) @@ -83,6 +88,23 @@ def _rectify_header(self, header: 'bytes', proto: 'TransType') -> 'bytes': # py """ return header + def _header_overhead(self, length: 'int') -> 'int': + """Count the header octets that a datagram's length field includes. + + The base implementation counts them all, which is what IPv4 wants: its + Total Length is the header's length plus the data's + (:rfc:`791#section-3.1`). IPv6 overrides this, because its Payload + Length leaves out the fixed header (:rfc:`8200#section-3`). + + Args: + length: Length of the datagram's header. + + Returns: + How many of those octets the length field counts. + + """ + return length + def reassembly(self, info: 'Packet[_AT]') -> 'None': """Reassembly procedure. @@ -138,7 +160,7 @@ def reassembly(self, info: 'Packet[_AT]') -> 'None': RCVBT=bytearray(8192), # Fragment Received Bit Table index=[], # index record header=header, # header buffer - datagram=bytearray(65535), # data buffer + datagram=bytearray(_MAX_LENGTH), # data buffer timestamp=TS, # first-arriving fragment's clock reading conflict=[], # conflicting octet ranges ) @@ -166,9 +188,12 @@ def reassembly(self, info: 'Packet[_AT]') -> 'None': # anything, and so does this, whatever **MF** and the offset say: an # unfragmented one is then whole rather than ``-IHL`` octets long # (:issue:`1555`). Linux's BIG TCP zeroes the field precisely because - # the datagram outgrows the 65535 octets it can declare, so the buffer - # grows to hold such a datagram rather than clipping it, ``RCVBT`` in - # step with it. + # the datagram, header included, outgrows the 65535 octets it can + # declare -- and such a datagram never completes in a buffer of that + # length (:meth:`_is_complete`). So the buffer grows past it, to hold + # the datagram rather than clip it, ``RCVBT`` in step with it; it grows + # by an octet even when the data alone would fit, since its growing is + # what lifts the bound. # # With nothing declared, a snaplen cut cannot be seen in the length. A # non-final fragment's cut still shows, as a partial last block, which @@ -179,9 +204,10 @@ def reassembly(self, info: 'Packet[_AT]') -> 'None': if TL == 0: length = len(info.payload) extent = FO + length - if extent > len(buf.datagram): - buf.datagram.extend(bytes(extent - len(buf.datagram))) - buf.RCVBT.extend(bytes((extent + 7) // 8 - len(buf.RCVBT))) + if self._header_overhead(IHL) + extent > _MAX_LENGTH: + size = max(extent, _MAX_LENGTH + 1, len(buf.datagram)) + buf.datagram.extend(bytes(size - len(buf.datagram))) + buf.RCVBT.extend(bytes((size + 7) // 8 - len(buf.RCVBT))) else: length = TL - IHL held = max(min(len(info.payload), length, len(buf.datagram) - FO), 0) @@ -243,25 +269,35 @@ def reassembly(self, info: 'Packet[_AT]') -> 'None': self.submit(self._buffer.pop(BUFID), bufid=BUFID, checked=True) ) - @staticmethod - def _is_complete(buf: 'Buffer[_AT]') -> 'bool': + def _is_complete(self, buf: 'Buffer[_AT]') -> 'bool': """Tell whether a buffer holds its whole datagram. Arguments: buf: buffer to check Returns: - Whether the total data length is known, fits the data buffer, and - every 8-octet block up to it is marked received. - - A datagram longer than the data buffer cannot be held, so it is never - complete. Checking ``RCVBT`` alone does not show that: slicing it past - its end stops silently, so the blocks past the buffer would read as - received (:issue:`1566`). + Whether the total data length is known, the datagram is no longer + than it may be, and every 8-octet block up to it is marked + received. + + A datagram is no longer than ``_MAX_LENGTH`` with its header, as + :meth:`_header_overhead` counts it: IPv4's Total Length counts the + whole header, and IPv6's Payload Length every header but the fixed one. + A longer datagram cannot be declared, so it is never complete, and + Linux's ``ip_frag_reasm`` and ``ip6_frag_reasm`` reject it too + (:issue:`1585`). A fragment with Total Length 0 declares no length, so + a buffer it has grown past that bounds the datagram by its own length + alone (:issue:`1555`). Checking ``RCVBT`` does not show either bound: + slicing it past its end stops silently, so the blocks past the buffer + would read as received (:issue:`1566`). """ TDL = buf.TDL - return 0 <= TDL <= len(buf.datagram) and all(buf.RCVBT[:(TDL + 7) // 8]) + if len(buf.datagram) > _MAX_LENGTH: + limit = len(buf.datagram) + else: + limit = _MAX_LENGTH - self._header_overhead(len(buf.header)) + return 0 <= TDL <= limit and all(buf.RCVBT[:(TDL + 7) // 8]) @staticmethod def _detect_conflicts(rcvbt: 'bytearray', tdl: 'int', datagram: 'bytearray', payload: 'bytearray', @@ -291,23 +327,30 @@ def _detect_conflicts(rcvbt: 'bytearray', tdl: 'int', datagram: 'bytearray', pay already genuinely received. :rfc:`791` marks receipt in 8-octet blocks (``RCVBT``), coarser than - the octet granularity a conflict needs: every fragment but the last is - required to be a multiple of 8 octets, and a fragment's ``FO`` is - *always* a multiple of 8 -- it is wire-encoded in 8-octet units -- so a - non-final fragment's range is always exactly block-aligned. The only - block that can be *partially* real is therefore the one holding the - final fragment's own tail: the ``RCVBT`` update in :meth:`reassembly` - sets that block's bit across its full 8 octets even though only the - octets up to ``tdl`` were ever actually written, the rest still being - ``datagram``'s zero-fill. + the octet granularity a conflict needs, so an octet is compared only if + its block is marked. A fragment's ``FO`` is *always* a multiple of 8 -- + it is wire-encoded in 8-octet units -- and the ``RCVBT`` update in + :meth:`reassembly` marks the blocks a fragment fills whole. Only the + final fragment, held in full, also marks its partial last block, across + its full 8 octets even though only the octets up to ``tdl`` were ever + actually written, the rest still being ``datagram``'s zero-fill. So once ``tdl`` is known, an octet at or past it is excluded here regardless of its block's bit -- comparing it would manufacture a conflict against a byte nothing ever really sent, over a distinction :meth:`~pcapkit.foundation.reassembly.ip.IP.submit` does not need anyway, since it never reports a payload past ``tdl``. Before ``tdl`` - is known (``tdl < 0``), every set ``rcvbt`` bit came from a non-final, - block-aligned fragment and is exact on its own, with nothing to clip. + is known (``tdl < 0``), every set ``rcvbt`` bit came from a non-final + fragment's whole block and is exact on its own, with nothing to clip. + + Any other partial last block -- a non-final, truncated or clipped + fragment's -- is written but left clear (:issue:`1567`), so its octets + are deliberately not compared: a later fragment that overwrites them + records no conflict, whether or not it agrees (:issue:`1585`). Telling + which octets of a clear block were written would take receipt state per + octet, which ``RCVBT`` does not keep. Linux never compares a non-final + fragment's partial block either: it trims an IPv4 one to a multiple of + 8 (``ip_frag_queue``) and drops an IPv6 one (``ip6_frag_queue``). """ conflicts = [] # type: list[tuple[int, int]] diff --git a/pcapkit/foundation/reassembly/ipv6.py b/pcapkit/foundation/reassembly/ipv6.py index 184ca8acbf..774a562c33 100644 --- a/pcapkit/foundation/reassembly/ipv6.py +++ b/pcapkit/foundation/reassembly/ipv6.py @@ -158,3 +158,20 @@ def _rectify_header(self, header: 'bytes', proto: 'TransType') -> 'bytes': if header[offset] != Enum_TransType.IPv6_Frag: return header return header[:offset] + bytes((int(proto),)) + header[offset + 1:] + + def _header_overhead(self, length: 'int') -> 'int': + """Count the header octets that the Payload Length includes. + + The Payload Length counts every octet after the fixed IPv6 header + (:rfc:`8200#section-3`), so a reassembled packet's is its unfragmentable + part's extension headers plus its data (:rfc:`8200#section-4.5`). The + header here is the unfragmentable part, its Fragment header excluded. + + Args: + length: Length of the datagram's header. + + Returns: + How many of those octets the Payload Length counts. + + """ + return max(length - _IPV6_HDR_LEN, 0) diff --git a/tests/foundation/reassembly/test_ip_last_fragment_offset_1565_unit.py b/tests/foundation/reassembly/test_ip_last_fragment_offset_1565_unit.py index f7b360ec9b..02737c3830 100644 --- a/tests/foundation/reassembly/test_ip_last_fragment_offset_1565_unit.py +++ b/tests/foundation/reassembly/test_ip_last_fragment_offset_1565_unit.py @@ -82,9 +82,12 @@ def test_a_lone_fragment_at_the_largest_offset_is_buffered(self) -> None: self.assertEqual(datagram.payload, bytes(LAST) + tail) def test_the_last_fragment_at_the_largest_offset_completes_its_datagram(self) -> None: + # IPv6 only: an IPv4 datagram reaching octet 65,528 is too long for its + # Total Length, header included, so it never completes (#1585). from pcapkit.foundation.reassembly.data.data import Completion + from pcapkit.foundation.reassembly.ipv6 import IPv6 - for cls in self._classes(): + for cls in (IPv6,): for size in (1, 7): for strict in (True, False): with self.subTest(cls=cls.__name__, size=size, strict=strict): diff --git a/tests/foundation/reassembly/test_ip_length_limit_1585_unit.py b/tests/foundation/reassembly/test_ip_length_limit_1585_unit.py new file mode 100644 index 0000000000..01cab4df8a --- /dev/null +++ b/tests/foundation/reassembly/test_ip_length_limit_1585_unit.py @@ -0,0 +1,161 @@ +# -*- coding: utf-8 -*- +"""An IP datagram longer than its length field can declare is never complete. + +GitHub issue #1585. The data buffer holds 65,535 octets of data, but IPv4's +Total Length counts the header as well, so fragments whose data covered octets +0-65,522 behind a 20-octet header made a 65,543-octet datagram, and it read +``COMPLETE``. Linux's ``ip_frag_reasm`` rejects it. The bound now counts the +header as the family's length field does: IPv4's Total Length counts all of it, +and IPv6's Payload Length counts the extension headers but not the fixed +40-octet header. A Total Length of 0 declares no length (#1555), so a datagram +that outgrows the bound with its header grows the buffer past it instead. + +The same issue documents, rather than changes, that the octets of a cut +non-final fragment's partial block, which is not marked received (#1567), are +not checked for conflicts; the last test pins that. + +Every case builds its fragments in memory and reads no capture. Classes are +imported inside each test, after :func:`~tests._support.reimport_once_per_class`. + +""" + +from ipaddress import ip_address +import unittest + +from tests._support import reimport_once_per_class + +#: The largest Fragment Offset, 8191 eight-octet units, in octets. +LAST = 65528 + +#: Datagram octets, with no zero octet in them, so a hole shows. +DATA = (bytes(range(1, 256)) * 258)[:65579] + + +class TestIPReassemblyLengthLimit(unittest.TestCase): + """Pin the bound on a datagram's length, header included as its family counts it.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + def _packet(self, *, num: int, fo: int, mf: bool, payload: bytes, ihl: int, tl: 'int | None' = None): + from pcapkit.const.reg.transtype import TransType + from pcapkit.foundation.reassembly.data.ip import Packet + + bufid = (ip_address('192.0.2.1'), ip_address('198.51.100.2'), 7, TransType.UDP) + return Packet(bufid, num, fo, ihl, mf, ihl + len(payload) if tl is None else tl, + b'H' * ihl, bytearray(payload), 1000.0 + num) + + def _two_fragments(self, cls, *, tdl: int, ihl: int, strict: bool, reverse: bool = False): + """Reassemble ``DATA[:tdl]`` from two fragments behind an ``ihl``-octet header.""" + packets = [self._packet(num=1, fo=0, mf=True, payload=DATA[:32768], ihl=ihl), + self._packet(num=2, fo=32768, mf=False, payload=DATA[32768:tdl], ihl=ihl)] + reasm = cls(strict=strict) + for packet in reversed(packets) if reverse else packets: + reasm(packet) + datagram, = reasm.datagram + return datagram + + def test_the_issues_repro_is_partial(self) -> None: + # case 3c: TDL 65,523 behind a 20-octet header is 65,543 octets + from pcapkit.foundation.reassembly.data.data import Completion + from pcapkit.foundation.reassembly.ipv4 import IPv4 + + for reverse in (False, True): + for strict in (True, False): + with self.subTest(reverse=reverse, strict=strict): + datagram = self._two_fragments(IPv4, tdl=65523, ihl=20, strict=strict, reverse=reverse) + self.assertIs(datagram.completed, Completion.PARTIAL) + self.assertEqual(datagram.index, (2, 1) if reverse else (1, 2)) + # every octet is still reported + self.assertEqual(datagram.payload, (DATA[:65523],) if strict else DATA[:65523]) + self.assertEqual(datagram.conflict, ()) + + def test_ipv4_counts_the_whole_header(self) -> None: + from pcapkit.foundation.reassembly.data.data import Completion + from pcapkit.foundation.reassembly.ipv4 import IPv4 + + for ihl in (20, 60): + for strict in (True, False): + with self.subTest(ihl=ihl, strict=strict): + whole = self._two_fragments(IPv4, tdl=65535 - ihl, ihl=ihl, strict=strict) + self.assertIs(whole.completed, Completion.COMPLETE) + self.assertEqual(whole.payload, DATA[:65535 - ihl]) + over = self._two_fragments(IPv4, tdl=65536 - ihl, ihl=ihl, strict=strict) + self.assertIs(over.completed, Completion.PARTIAL) + + def test_ipv6_counts_only_the_extension_headers(self) -> None: + # The fixed header is not in the Payload Length, so 65,535 octets of + # data behind it fit; an 8-octet extension header takes 8 of them. + from pcapkit.foundation.reassembly.data.data import Completion + from pcapkit.foundation.reassembly.ipv6 import IPv6 + + for ihl, tdl in ((40, 65535), (48, 65527)): + for strict in (True, False): + with self.subTest(ihl=ihl, strict=strict): + whole = self._two_fragments(IPv6, tdl=tdl, ihl=ihl, strict=strict) + self.assertIs(whole.completed, Completion.COMPLETE) + self.assertEqual(whole.payload, DATA[:tdl]) + for strict in (True, False): + with self.subTest(ihl=48, strict=strict): + over = self._two_fragments(IPv6, tdl=65528, ihl=48, strict=strict) + self.assertIs(over.completed, Completion.PARTIAL) + + def test_an_ipv4_final_fragment_at_the_largest_offset_never_completes(self) -> None: + # Its data ends past octet 65,528, so no header leaves it room. + from pcapkit.foundation.reassembly.data.data import Completion + from pcapkit.foundation.reassembly.ipv4 import IPv4 + + for size in (1, 7): + for strict in (True, False): + with self.subTest(size=size, strict=strict): + reasm = IPv4(strict=strict) + reasm(self._packet(num=1, fo=0, mf=True, payload=DATA[:32768], ihl=20)) + reasm(self._packet(num=2, fo=32768, mf=True, payload=DATA[32768:LAST], ihl=20)) + reasm(self._packet(num=3, fo=LAST, mf=False, payload=DATA[LAST:LAST + size], ihl=20)) + datagram, = reasm.datagram + self.assertIs(datagram.completed, Completion.PARTIAL) + self.assertEqual(datagram.index, (1, 2, 3)) + body = DATA[:LAST + size] + self.assertEqual(datagram.payload, (body,) if strict else body) + + def test_a_tso_datagram_is_not_bounded(self) -> None: + # Total Length 0 declares nothing, as Linux's BIG TCP leaves a datagram + # over 65,535 octets: 65,523 octets of data fit the preallocated buffer + # but not with the header, so the buffer still grows past the bound. + from pcapkit.foundation.reassembly.data.data import Completion + from pcapkit.foundation.reassembly.ipv4 import IPv4 + + for strict in (True, False): + with self.subTest(strict=strict): + reasm = IPv4(strict=strict) + reasm(self._packet(num=1, fo=0, mf=False, payload=DATA[:65523], ihl=20, tl=0)) + datagram, = reasm.datagram + self.assertIs(datagram.completed, Completion.COMPLETE) + self.assertEqual(datagram.payload, DATA[:65523]) + + def test_overwriting_a_cut_fragments_partial_block_records_no_conflict(self) -> None: + # A non-final fragment of 60 octets, as a cut IPv6 one is passed: its + # partial block, octets 56-63, is a hole (#1567). The next fragment + # overwrites 56-59 with other octets. Receipt is tracked in whole + # blocks only, so they are not compared, and the later octets win as + # RFC 791 has them do. + from pcapkit.foundation.reassembly.data.data import Completion + from pcapkit.foundation.reassembly.ipv4 import IPv4 + from pcapkit.foundation.reassembly.ipv6 import IPv6 + + other = bytes(b ^ 0xFF for b in DATA[56:64]) + for cls in (IPv4, IPv6): + for strict in (True, False): + with self.subTest(cls=cls.__name__, strict=strict): + reasm = cls(strict=strict) + reasm(self._packet(num=1, fo=0, mf=True, payload=DATA[:60], ihl=40)) + reasm(self._packet(num=2, fo=56, mf=True, payload=other, ihl=40)) + reasm(self._packet(num=3, fo=64, mf=False, payload=DATA[64:72], ihl=40)) + datagram, = reasm.datagram + self.assertIs(datagram.completed, Completion.COMPLETE) + self.assertEqual(datagram.payload, DATA[:56] + other + DATA[64:72]) + self.assertEqual(datagram.conflict, ()) + + +if __name__ == '__main__': + unittest.main()