fix(reassembly): a datagram too long for its length field never completes (#1585)

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.

docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst

@@ -155,9 +155,13 @@ Terminology
           :meth:`IP._detect_conflicts <pcapkit.foundation.reassembly.ip.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::
 

docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst

@@ -198,6 +198,10 @@ Terminology
           :meth:`IP._detect_conflicts <pcapkit.foundation.reassembly.ip.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`).

pcapkit/foundation/reassembly/data/ip.py

@@ -128,14 +128,17 @@ class Datagram(DeferredPacket, Info, Generic[_AT]):
     #:
     #: :attr:`Buffer.RCVBT <pcapkit.foundation.reassembly.data.ip.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 <pcapkit.foundation.reassembly.data.ip.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 <pcapkit.foundation.reassembly.ip.IP._detect_conflicts>`.
     conflict: 'tuple[tuple[int, int], ...]'
 

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 <pcapkit.foundation.reassembly.reassembly.Reassembly._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]]

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)

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):