From f6f699f20452f5e4ddb0791fa787ff38e568e962 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sun, 1 Nov 2026 05:19:29 -0500 Subject: [PATCH] fix(reassembly): abandon an IPv6 datagram on an overlapping fragment (#2547) - IP.__discard_overlap__ (False; IPv6 sets True) makes reassembly abandon a datagram once a fragment overlaps another, per RFC 8200 section 4.5; IPv4 keeps RFC 791's newest-wins overwrite. - An exact duplicate (same offset, length, M flag and data) is dropped: only its frame number is kept in index, and its header is not taken. - Completion.OVERLAP reports the abandoned datagram via IP.submit(overlap=); the overlapping fragment is not written, and the event is logged. - Buffer.extent records each merged fragment's (start, stop, mf), sorted, for IP._find_overlap; it stays empty for IPv4. - Tests: new test_ipv6_overlap_2547_unit.py; the #1566, #1585 and payload round-trip suites now expect OVERLAP for their overlapping IPv6 cases. - Docs: IPv6 datagram and buffer structures, departures list, ip.rst. --- .../foundation/reassembly/ip/index.rst | 5 + .../pcapkit/foundation/reassembly/ip/ip.rst | 2 + .../pcapkit/foundation/reassembly/ip/ipv4.rst | 1 + .../pcapkit/foundation/reassembly/ip/ipv6.rst | 30 ++- pcapkit/foundation/reassembly/data/data.py | 15 +- pcapkit/foundation/reassembly/data/ip.py | 29 ++- pcapkit/foundation/reassembly/ip.py | 99 +++++++- pcapkit/foundation/reassembly/ipv6.py | 21 ++ .../foundation/reassembly/data/test_models.py | 8 +- tests/foundation/reassembly/test_ip.py | 2 +- .../test_ip_length_limit_1585_unit.py | 45 +++- .../test_ip_oversized_datagram_1566_unit.py | 77 ++++-- .../reassembly/test_ipv6_overlap_2547_unit.py | 236 ++++++++++++++++++ .../test_reassembly_payload_roundtrip_unit.py | 23 +- 14 files changed, 521 insertions(+), 72 deletions(-) create mode 100644 tests/foundation/reassembly/test_ipv6_overlap_2547_unit.py diff --git a/docs/source/pcapkit/foundation/reassembly/ip/index.rst b/docs/source/pcapkit/foundation/reassembly/ip/index.rst index 6529458544..c4ac34bf13 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/index.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/index.rst @@ -106,6 +106,11 @@ Departures from the procedure above: * An IPv6 atomic fragment, one with fragment offset zero and **M** clear, flushes nothing: it is reassembled in isolation and the fragments buffered under its ``BUFID`` are kept, per :rfc:`6946#section-4` (:issue:`2270`). +* An IPv6 fragment that overlaps another, other than an exact duplicate, + abandons its datagram rather than overwriting the octets it overlaps, per + :rfc:`8200#section-4.5`; the datagram is reported as + :attr:`~pcapkit.foundation.reassembly.data.data.Completion.OVERLAP` + (:issue:`2547`). * ``TDL`` starts at ``-1`` rather than ``0``, so that a datagram carrying no data can complete (:issue:`1359`). * Only the final fragment's partial last ``RCVBT`` block is set. A non-final diff --git a/docs/source/pcapkit/foundation/reassembly/ip/ip.rst b/docs/source/pcapkit/foundation/reassembly/ip/ip.rst index 943fccd85c..e72ad42d49 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/ip.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/ip.rst @@ -12,9 +12,11 @@ origin, using the :rfc:`791` procedure described in :doc:`index`. :no-members: :show-inheritance: + .. autoattribute:: __discard_overlap__ .. automethod:: reassembly .. automethod:: submit .. automethod:: _detect_conflicts + .. automethod:: _find_overlap Data Models ----------- diff --git a/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst b/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst index b4f776dd26..e7e1f4a119 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst @@ -146,6 +146,7 @@ Terminology | | |--> (tuple) (first, last), absolute and | | inclusive | | |--> ... + | |--> 'extent' : (list) always empty: IPv4 keeps no extents |--> (tuple) BUFID ... .. note:: diff --git a/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst b/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst index be390d5631..21f586c21a 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst @@ -17,6 +17,7 @@ origin. Please refer to :doc:`ip` for more information. .. autoattribute:: __callback_fn__ :no-value: .. autoattribute:: __timeout__ + .. autoattribute:: __discard_overlap__ Terminology ----------- @@ -108,7 +109,7 @@ Terminology | | |--> (tuple) (first, last), absolute and inclusive | | |--> ... |--> (Info) data - | |--> 'completed' : (Completion) PARTIAL or TIMEOUT --> incomplete + | |--> 'completed' : (Completion) PARTIAL, TIMEOUT or OVERLAP --> incomplete | |--> 'id' : (Info) original packet identifier | | |--> 'src' --> (IPv6Address) ipv6.src | | |--> 'dst' --> (IPv6Address) ipv6.dst @@ -148,17 +149,19 @@ Terminology .. note:: - ``completed`` and ``conflict`` are independent signals: a datagram - can be :attr:`~pcapkit.foundation.reassembly.data.data.Completion.COMPLETE` - and still carry a non-empty ``conflict``. IPv6 reassembly shares - IPv4's resolution of an overlapping fragment's disagreement, the one - :rfc:`791` gives -- "this procedure will use the more recently - arrived copy in the data buffer" -- the opposite resolution from - TCP's first-write-wins (:rfc:`9293#section-3.10`), rather than - abandoning the datagram as :rfc:`8200#section-4.5` has a receiving - node do. So a contested range never leaves a hole on its own, and - ``conflict`` is what lets a caller tell a clean datagram from a - contested one. + A fragment that overlaps another, other than an exact duplicate, + abandons its datagram, as :rfc:`8200#section-4.5` has a receiving + node do, rather than overwriting the octets it overlaps as IPv4's + :rfc:`791` procedure does. The datagram is reported as + :attr:`Completion.OVERLAP `, + its ``payload`` the fragments merged before the overlapping one, and + ``conflict`` where that fragment disagreed with them -- empty if it + repeated their octets. So a + :attr:`~pcapkit.foundation.reassembly.data.data.Completion.COMPLETE` + IPv6 datagram never carries a conflict. An exact duplicate, with the + same offset, length, **M** flag and data, is dropped as the RFC + permits, its frame number kept in ``index``. A later fragment of an + abandoned datagram opens a new buffer (:issue:`2547`). reasm.ipv6.buffer Data structure for internal buffering when performing reassembly algorithms @@ -190,6 +193,9 @@ Terminology | | |--> (tuple) (first, last), absolute and | | inclusive | | |--> ... + | |--> 'extent' : (list) merged fragments, sorted and disjoint + | | |--> (tuple) (start, stop, mf), stop exclusive + | | |--> ... |--> (tuple) BUFID ... .. note:: diff --git a/pcapkit/foundation/reassembly/data/data.py b/pcapkit/foundation/reassembly/data/data.py index 08aa65ef7e..1d8462d87c 100644 --- a/pcapkit/foundation/reassembly/data/data.py +++ b/pcapkit/foundation/reassembly/data/data.py @@ -63,7 +63,7 @@ class Completion(EnumLookup, StrEnum): where that name is the stdlib ``enum.StrEnum``. Warning: - Being a :class:`str` whose :attr:`PARTIAL` and :attr:`TIMEOUT` members are + Being a :class:`str` whose members other than :attr:`COMPLETE` are **falsy** makes this a non-empty string that tests false, so ``bool(x)`` and ``bool(str(x))`` disagree. That is deliberate -- the truthiness above is what lets ``if datagram.completed:`` read as a boolean test -- but code @@ -91,11 +91,20 @@ class Completion(EnumLookup, StrEnum): #: discarded, so no further fragment will ever be added to this datagram. TIMEOUT = auto() + #: Reassembly was **abandoned** because a fragment overlapped another of the + #: same datagram without being an exact duplicate of it, which + #: :rfc:`8200#section-4.5` discards an IPv6 datagram for. The overlapping + #: fragment's octets are not in the payload; ``conflict`` still lists where + #: they disagreed with the octets kept. + #: + #: .. versionadded:: 1.6.0 + OVERLAP = auto() + def __bool__(self) -> 'bool': """Whether the datagram was reassembled in whole. - Only :attr:`COMPLETE` is truthy; both :attr:`PARTIAL` and - :attr:`TIMEOUT` describe an incomplete datagram. + Only :attr:`COMPLETE` is truthy; every other member describes an + incomplete datagram. Note: This override is what a :class:`str` base does *not* give -- every diff --git a/pcapkit/foundation/reassembly/data/ip.py b/pcapkit/foundation/reassembly/data/ip.py index 65c7e4ee5b..79675a92e0 100644 --- a/pcapkit/foundation/reassembly/data/ip.py +++ b/pcapkit/foundation/reassembly/data/ip.py @@ -123,15 +123,19 @@ class Datagram(DeferredPacket, Info, Generic[_AT]): #: fragment overlapped octets already buffered but did not repeat them. #: Empty when the datagram never saw a contested octet. #: - #: :rfc:`791` resolves the disagreement itself: "In the case that two or - #: more fragments contain the same data either identically or through a - #: partial overlap, this procedure will use the more recently arrived copy - #: in the data buffer and datagram delivered." So :attr:`payload` always - #: holds whichever fragment arrived *last* over a contested range -- the - #: opposite resolution from TCP's first-write-wins + #: For IPv4, :rfc:`791` resolves the disagreement itself: "In the case that + #: two or more fragments contain the same data either identically or + #: through a partial overlap, this procedure will use the more recently + #: arrived copy in the data buffer and datagram delivered." So + #: :attr:`payload` always holds whichever fragment arrived *last* over a + #: contested range -- the opposite resolution from TCP's first-write-wins #: (:rfc:`9293#section-3.10`) -- and this field is what lets a caller tell #: a clean datagram from a contested one, since a resolved conflict does - #: not, on its own, leave a hole for ``completed`` to report. + #: not, on its own, leave a hole for ``completed`` to report. IPv6 instead + #: abandons the datagram on the overlapping fragment, without writing it + #: (:rfc:`8200#section-4.5`), so there a conflict comes only with + #: :attr:`Completion.OVERLAP + #: `. #: #: :attr:`Buffer.RCVBT ` #: only records receipt in 8-octet blocks, coarser than the octet @@ -190,6 +194,15 @@ class Buffer(Info, Generic[_AT]): #: :attr:`Datagram.conflict ` #: verbatim when the buffer is submitted. conflict: 'list[tuple[int, int]]' + #: Extent of each fragment merged into this buffer, as ``(start, stop, + #: mf)``: its first octet, one past its last, and its **MF** (**M**) flag, + #: sorted by ``start``. Kept only where an overlapping fragment abandons + #: the datagram, i.e. for IPv6, whose merged fragments therefore never + #: overlap; empty for IPv4. See + #: :attr:`IP.__discard_overlap__ `. + #: + #: .. versionadded:: 1.6.0 + extent: 'list[tuple[int, int, bool]]' if TYPE_CHECKING: - def __init__(self, TDL: 'int', RCVBT: 'bytearray', index: 'list[int]', header: 'bytes', datagram: 'bytearray', timestamp: 'float', conflict: 'list[tuple[int, int]]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin + def __init__(self, TDL: 'int', RCVBT: 'bytearray', index: 'list[int]', header: 'bytes', datagram: 'bytearray', timestamp: 'float', conflict: 'list[tuple[int, int]]', extent: 'list[tuple[int, int, bool]]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin diff --git a/pcapkit/foundation/reassembly/ip.py b/pcapkit/foundation/reassembly/ip.py index 05f864974a..14f5f2b239 100644 --- a/pcapkit/foundation/reassembly/ip.py +++ b/pcapkit/foundation/reassembly/ip.py @@ -14,6 +14,8 @@ uses the elder one. """ +import bisect +import sys from typing import TYPE_CHECKING, Generic from pcapkit.foundation.reassembly.data.data import Completion @@ -21,15 +23,20 @@ Deferred, Packet) from pcapkit.foundation.reassembly.reassembly import ReassemblyBase from pcapkit.protocols.schema.internet.ipv6 import IP_MAX_LENGTH +from pcapkit.utilities.logging import get_logger if TYPE_CHECKING: - from typing import Type + from typing import Optional, Type from pcapkit.const.reg.transtype import TransType from pcapkit.protocols.internet.ip import IP as IP_Protocol __all__ = ['IP'] +#: logging.Logger: Module-level logger, a child of the package-wide +#: :data:`pcapkit.utilities.logging.logger`. +logger = get_logger(__name__) + class IP(ReassemblyBase[Packet[_AT], Datagram[_AT], BufferID, Buffer[_AT]], Generic[_AT]): # pylint: disable=abstract-method """Reassembly for IP payload. @@ -71,6 +78,12 @@ class IP(ReassemblyBase[Packet[_AT], Datagram[_AT], BufferID, Buffer[_AT]], Gene IPv6) clear is reported even when its BUFID is already buffered, rather than dropped (:issue:`2270`). + .. versionchanged:: 1.6.0 + + A subclass setting :attr:`__discard_overlap__` abandons a datagram on an + overlapping fragment instead of overwriting, and + :class:`~pcapkit.foundation.reassembly.ipv6.IPv6` does (:issue:`2547`). + """ if TYPE_CHECKING: protocol: 'Type[IP_Protocol]' @@ -82,6 +95,16 @@ class IP(ReassemblyBase[Packet[_AT], Datagram[_AT], BufferID, Buffer[_AT]], Gene #: buffer intact (:issue:`2270`). __isolate_atomic__: 'bool' = False + #: bool: Whether a fragment overlapping another of its datagram abandons + #: that datagram, reported as + #: :attr:`Completion.OVERLAP `, + #: rather than overwriting the octets it overlaps. IPv4 overwrites them + #: (:rfc:`791#section-3.2`); IPv6 overrides this, since + #: :rfc:`8200#section-4.5` discards the datagram (:issue:`2547`). + #: + #: .. versionadded:: 1.6.0 + __discard_overlap__: 'bool' = False + ########################################################################## # Methods. ########################################################################## @@ -183,7 +206,8 @@ def reassembly(self, info: 'Packet[_AT]') -> 'None': # would cover octets 0 to 65527 only, so a fragment reaching octet 65528, # e.g. at the largest Fragment Offset, would index past it (#1565) buf: 'Buffer[_AT]' - if isolated or BUFID not in self._buffer: + fresh = isolated or BUFID not in self._buffer + if fresh: buf = Buffer( TDL=-1, # Total Data Length RCVBT=bytearray(8192), # Fragment Received Bit Table @@ -192,14 +216,12 @@ def reassembly(self, info: 'Packet[_AT]') -> 'None': datagram=bytearray(IP_MAX_LENGTH), # data buffer timestamp=TS, # first-arriving fragment's clock reading conflict=[], # conflicting octet ranges + extent=[], # merged fragments' extents ) if not isolated: self._buffer[BUFID] = buf else: buf = self._buffer[BUFID] - # put header into header buffer - if not FO: - buf.__update__(header=header) # append packet index buf.index.append(info.num) @@ -254,6 +276,31 @@ def reassembly(self, info: 'Packet[_AT]') -> 'None': if conflicts: buf.conflict.extend(conflicts) + # :rfc:`8200#section-4.5` abandons a datagram, and discards every + # fragment received for it, once one fragment overlaps another; it lets + # an implementation drop an exact duplicate instead, which this does. + # The span is the length the fragment declares, not the octets it + # holds. A fresh buffer has no extent to overlap, so the ``pop`` below + # never reaches an isolated one. + if self.__discard_overlap__: + span = (FO, FO + max(length, 0), MF) + other = self._find_overlap(buf.extent, span[0], span[1]) + if other is not None: + if other == span and buf.datagram[start:stop] == payload: + return + logger.debug('%s: abandoning buffer %s on frame %d, which overlaps ' + 'octets %d-%d', self.name, BUFID, info.num, other[0], other[1] - 1) + self._dtgram.extend( + self.submit(self._buffer.pop(BUFID), bufid=BUFID, overlap=True) + ) + return + if span[1] > span[0]: + bisect.insort(buf.extent, span) + + # put header into header buffer, once the fragment is known to be kept + if not FO and not fresh: + buf.__update__(header=header) + # :rfc:`791` is explicit that an overlapping fragment's data "will use # the more recently arrived copy in the data buffer" -- the opposite of # TCP's first-write-wins (:rfc:`9293#section-3.10`) -- so the arriving @@ -410,8 +457,39 @@ def _detect_conflicts(rcvbt: 'bytearray', tdl: 'int', datagram: 'bytearray', pay index = run_stop return conflicts + @staticmethod + def _find_overlap(extent: 'list[tuple[int, int, bool]]', start: 'int', + stop: 'int') -> 'Optional[tuple[int, int, bool]]': + """Find a merged fragment that an arriving one overlaps. + + Arguments: + extent: this buffer's + :attr:`~pcapkit.foundation.reassembly.data.ip.Buffer.extent`, + sorted and pairwise disjoint. + start: the arriving fragment's first octet, i.e. its ``FO``. + stop: one past the arriving fragment's last octet. + + Returns: + The extent of a merged fragment sharing an octet with the arriving + one, or :data:`None` if none does. + + Since the extents are disjoint, only the last one starting at or before + ``start`` and the first one starting after it can reach the arriving + fragment. + + """ + if stop <= start: + return None + after = bisect.bisect_right(extent, (start, sys.maxsize, True)) + if after and extent[after - 1][1] > start: + return extent[after - 1] + if after < len(extent) and extent[after][0] < stop: + return extent[after] + return None + def submit(self, buf: 'Buffer[_AT]', *, bufid: 'tuple[_AT, _AT, int, TransType]', # type: ignore[override] # pylint: disable=arguments-differ - checked: 'bool' = False, timeout: 'bool' = False) -> 'list[Datagram[_AT]]': + checked: 'bool' = False, timeout: 'bool' = False, + overlap: 'bool' = False) -> 'list[Datagram[_AT]]': """Submit reassembled payload. Arguments: @@ -425,6 +503,11 @@ def submit(self, buf: 'Buffer[_AT]', *, bufid: 'tuple[_AT, _AT, int, TransType]' :attr:`Completion.TIMEOUT ` from :attr:`Completion.PARTIAL ` + overlap: whether this buffer is being submitted because an + overlapping fragment abandoned it (:attr:`__discard_overlap__`), + reported as :attr:`Completion.OVERLAP + ` + and never as complete Returns: Reassembled packets. @@ -437,13 +520,13 @@ def submit(self, buf: 'Buffer[_AT]', *, bufid: 'tuple[_AT, _AT, int, TransType]' datagram = buf.datagram conflict = tuple(buf.conflict) - flag = checked or self._is_complete(buf) + flag = not overlap and (checked or self._is_complete(buf)) ret = [] # type: list[Datagram[_AT]] # How completely this datagram came out, and why it stopped. Derived once, # so the two branches below cannot disagree about it. completion = Completion.COMPLETE if flag else ( - Completion.TIMEOUT if timeout else Completion.PARTIAL + Completion.OVERLAP if overlap else Completion.TIMEOUT if timeout else Completion.PARTIAL ) # if datagram is incomplete diff --git a/pcapkit/foundation/reassembly/ipv6.py b/pcapkit/foundation/reassembly/ipv6.py index 4a4f125731..fc281e2cc6 100644 --- a/pcapkit/foundation/reassembly/ipv6.py +++ b/pcapkit/foundation/reassembly/ipv6.py @@ -96,6 +96,15 @@ class IPv6(IP): isolation, without flushing a buffer that shares its BUFID (:issue:`2270`). + .. versionchanged:: 1.6.0 + + A fragment overlapping another of its datagram, other than an exact + duplicate, abandons that datagram, which is reported as + :attr:`Completion.OVERLAP + `, per + :rfc:`8200#section-4.5`; 1.5.x overwrote the octets it overlapped, as + :rfc:`791` has IPv4 do (:issue:`2547`). + """ ########################################################################## @@ -133,6 +142,18 @@ class IPv6(IP): #: discarded (:issue:`2270`). __isolate_atomic__ = True + #: bool: A fragment that overlaps another of its datagram abandons that + #: datagram: :rfc:`8200#section-4.5` says "reassembly of that packet must be + #: abandoned and all the fragments that have been received for that packet + #: must be discarded". It also lets an implementation "drop exact duplicate + #: fragments while keeping the other fragments belonging to the same + #: packet", which this one does: a fragment with the same offset, length, + #: **M** flag and data as one already merged changes nothing but + #: :attr:`Datagram.index `. + #: A later fragment of an abandoned datagram opens a new buffer, as one + #: does after a datagram completes (:issue:`2547`). + __discard_overlap__ = True + ########################################################################## # Methods. ########################################################################## diff --git a/tests/foundation/reassembly/data/test_models.py b/tests/foundation/reassembly/data/test_models.py index d7105b2cb0..b477f0f073 100644 --- a/tests/foundation/reassembly/data/test_models.py +++ b/tests/foundation/reassembly/data/test_models.py @@ -40,11 +40,13 @@ def test_ip_data_models_and_package_aliases(self) -> None: self.assertEqual(datagram.to_dict()['payload'], (b'payload',)) self.assertEqual(datagram.conflict, ()) - buffer = Buffer(-1, bytearray(b'\x01'), [7], b'ip-header', bytearray(b'payload'), 1000.0, []) + buffer = Buffer(-1, bytearray(b'\x01'), [7], b'ip-header', bytearray(b'payload'), 1000.0, [], + [(8, 15, True)]) self.assertIsInstance(buffer, IP_Buffer) self.assertEqual(buffer.index, [7]) self.assertEqual(buffer.timestamp, 1000.0) self.assertEqual(buffer.conflict, []) + self.assertEqual(buffer.extent, [(8, 15, True)]) storage = ReassemblyData((datagram,), (), ()) self.assertEqual(storage.ipv4, (datagram,)) @@ -70,6 +72,7 @@ def test_completion_is_a_string_that_reads_as_a_bool(self) -> None: self.assertTrue(Completion.COMPLETE) self.assertFalse(Completion.PARTIAL) self.assertFalse(Completion.TIMEOUT) + self.assertFalse(Completion.OVERLAP) # ... while equality against a bool stays broken, as documented self.assertNotEqual(Completion.COMPLETE, True) @@ -98,11 +101,12 @@ def test_completion_values_are_pinned(self) -> None: from pcapkit.foundation.reassembly.data.data import Completion self.assertEqual([member.name for member in Completion], - ['COMPLETE', 'PARTIAL', 'TIMEOUT']) + ['COMPLETE', 'PARTIAL', 'TIMEOUT', 'OVERLAP']) self.assertEqual(Completion.COMPLETE, 'complete') self.assertEqual(Completion.PARTIAL, 'partial') self.assertEqual(Completion.TIMEOUT, 'timeout') + self.assertEqual(Completion.OVERLAP, 'overlap') def test_tcp_data_models_and_package_aliases(self) -> None: from pcapkit.foundation.reassembly.data import (Completion, TCP_Buffer, TCP_Datagram, diff --git a/tests/foundation/reassembly/test_ip.py b/tests/foundation/reassembly/test_ip.py index 861612db1a..436ce39993 100644 --- a/tests/foundation/reassembly/test_ip.py +++ b/tests/foundation/reassembly/test_ip.py @@ -114,7 +114,7 @@ class TestIP(IP): src = ip_address('192.0.2.1') dst = ip_address('198.51.100.2') datagram, = empty.submit( - Buffer(-1, bytearray(b'\x00\x00'), [], b'', bytearray(b''), 1000.0, []), + Buffer(-1, bytearray(b'\x00\x00'), [], b'', bytearray(b''), 1000.0, [], []), bufid=(src, dst, 42, TransType.UDP), ) self.assertFalse(datagram.completed) diff --git a/tests/foundation/reassembly/test_ip_length_limit_1585_unit.py b/tests/foundation/reassembly/test_ip_length_limit_1585_unit.py index 88263b3e24..f075135eb5 100644 --- a/tests/foundation/reassembly/test_ip_length_limit_1585_unit.py +++ b/tests/foundation/reassembly/test_ip_length_limit_1585_unit.py @@ -12,7 +12,8 @@ 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. +not checked for conflicts; the last two tests pin that, the second for IPv6, +which abandons the datagram on the overlap instead (#2547). 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`. @@ -141,20 +142,40 @@ def test_overwriting_a_cut_fragments_partial_block_records_no_conflict(self) -> # RFC 791 has them do. from pcapkit.foundation.reassembly.data.data import Completion from pcapkit.foundation.reassembly.ipv4 import IPv4 + + other = bytes(b ^ 0xFF for b in DATA[56:64]) + for strict in (True, False): + with self.subTest(strict=strict): + reasm = IPv4(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, ()) + + def test_ipv6_abandons_a_datagram_overlapping_a_cut_fragment(self) -> None: + # The same fragments on IPv6: the cut one's 60 octets reach into the + # next one's, so RFC 8200 section 4.5 abandons the datagram (#2547). + # The overlapped octets sit in a hole, so no conflict is recorded. + from pcapkit.foundation.reassembly.data.data import Completion 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, ()) + for strict in (True, False): + with self.subTest(strict=strict): + reasm = IPv6(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)) + late, abandoned = reasm.datagram + self.assertIs(abandoned.completed, Completion.OVERLAP) + self.assertEqual(abandoned.index, (1, 2)) + self.assertEqual(abandoned.payload, (DATA[:56],) if strict else DATA[:56]) + self.assertEqual(abandoned.conflict, ()) + self.assertIs(late.completed, Completion.PARTIAL) + self.assertEqual(late.index, (3,)) if __name__ == '__main__': diff --git a/tests/foundation/reassembly/test_ip_oversized_datagram_1566_unit.py b/tests/foundation/reassembly/test_ip_oversized_datagram_1566_unit.py index c60d378c3c..3172732665 100644 --- a/tests/foundation/reassembly/test_ip_oversized_datagram_1566_unit.py +++ b/tests/foundation/reassembly/test_ip_oversized_datagram_1566_unit.py @@ -5,7 +5,8 @@ blocks. Guards against a completion check of ``all(RCVBT[0:(TDL + 7) // 8])``, whose slice stops silently at the table's end, so a final fragment declaring more data than fits completes its datagram short. A datagram completes only if its -total data length fits the buffer. +total data length fits the buffer. Both of those cases overlap a fragment, so on +IPv6 they are abandoned first, per RFC 8200 section 4.5 (#2547). A fragment the buffer's end clips counts as cut there: its partial last block is a hole. An 8-octet non-final fragment at the largest offset therefore receives @@ -58,38 +59,68 @@ def test_a_final_fragment_declaring_more_than_the_buffer_does_not_complete(self) # the buffer: 65579 is the issue's, and 65536 the shortest -- which a # check of ``(TDL + 7) // 8`` against ``len(RCVBT)`` lets through. from pcapkit.foundation.reassembly.data.data import Completion + from pcapkit.foundation.reassembly.ipv4 import IPv4 - for cls in self._classes(): - for tdl in (65579, 65536): - for strict in (True, False): - with self.subTest(cls=cls.__name__, tdl=tdl, strict=strict): - reasm = cls(strict=strict) - reasm(self._packet(num=1, fo=LAST, mf=False, payload=DATA[LAST:BUFFER])) - reasm(self._packet(num=2, fo=0, mf=True, payload=DATA[:64])) - reasm(self._packet(num=3, fo=64, mf=False, payload=DATA[64:tdl])) - datagram, = reasm.datagram - self.assertIs(datagram.completed, Completion.PARTIAL) - self.assertEqual(datagram.index, (1, 2, 3)) - self.assertEqual(datagram.payload, - (DATA[:BUFFER],) if strict else DATA[:BUFFER]) - self.assertEqual(datagram.conflict, ()) + for tdl in (65579, 65536): + for strict in (True, False): + with self.subTest(tdl=tdl, strict=strict): + reasm = IPv4(strict=strict) + reasm(self._packet(num=1, fo=LAST, mf=False, payload=DATA[LAST:BUFFER])) + reasm(self._packet(num=2, fo=0, mf=True, payload=DATA[:64])) + reasm(self._packet(num=3, fo=64, mf=False, payload=DATA[64:tdl])) + datagram, = reasm.datagram + self.assertIs(datagram.completed, Completion.PARTIAL) + self.assertEqual(datagram.index, (1, 2, 3)) + self.assertEqual(datagram.payload, + (DATA[:BUFFER],) if strict else DATA[:BUFFER]) + self.assertEqual(datagram.conflict, ()) def test_the_issues_repro_is_partial(self) -> None: # Octets 0-65527, a malformed 7-octet non-final fragment at the largest # offset, then a final fragment at 64 declaring Total Length 65535, # of which 8 octets were captured. from pcapkit.foundation.reassembly.data.data import Completion + from pcapkit.foundation.reassembly.ipv4 import IPv4 - for cls in self._classes(): + for strict in (True, False): + with self.subTest(strict=strict): + reasm = IPv4(strict=strict) + for num, fo in enumerate(range(0, LAST, 4096), 1): + reasm(self._packet(num=num, fo=fo, mf=True, payload=DATA[fo:min(fo + 4096, LAST)])) + reasm(self._packet(num=20, fo=LAST, mf=True, payload=b'M' * 7)) + reasm(self._packet(num=21, fo=64, mf=False, payload=DATA[64:72], tl=65535)) + datagram, = reasm.datagram + self.assertIs(datagram.completed, Completion.PARTIAL) + + def test_ipv6_abandons_both_datagrams_on_their_overlap_first(self) -> None: + # In both cases above the long final fragment reaches into the one at + # the largest offset, so RFC 8200 section 4.5 abandons the datagram on + # its arrival, before the length bound is reached (#2547). + from pcapkit.foundation.reassembly.data.data import Completion + from pcapkit.foundation.reassembly.ipv6 import IPv6 + + cases = { + 'final-past-buffer': [ + self._packet(num=1, fo=LAST, mf=False, payload=DATA[LAST:BUFFER]), + self._packet(num=2, fo=0, mf=True, payload=DATA[:64]), + self._packet(num=3, fo=64, mf=False, payload=DATA[64:65579]), + ], + 'issue-repro': [ + *(self._packet(num=num, fo=fo, mf=True, payload=DATA[fo:min(fo + 4096, LAST)]) + for num, fo in enumerate(range(0, LAST, 4096), 1)), + self._packet(num=20, fo=LAST, mf=True, payload=b'M' * 7), + self._packet(num=21, fo=64, mf=False, payload=DATA[64:72], tl=65535), + ], + } + for name, packets in cases.items(): for strict in (True, False): - with self.subTest(cls=cls.__name__, strict=strict): - reasm = cls(strict=strict) - for num, fo in enumerate(range(0, LAST, 4096), 1): - reasm(self._packet(num=num, fo=fo, mf=True, payload=DATA[fo:min(fo + 4096, LAST)])) - reasm(self._packet(num=20, fo=LAST, mf=True, payload=b'M' * 7)) - reasm(self._packet(num=21, fo=64, mf=False, payload=DATA[64:72], tl=65535)) + with self.subTest(case=name, strict=strict): + reasm = IPv6(strict=strict) + for packet in packets: + reasm(packet) datagram, = reasm.datagram - self.assertIs(datagram.completed, Completion.PARTIAL) + self.assertIs(datagram.completed, Completion.OVERLAP) + self.assertEqual(datagram.index[-1], packets[-1].num) def test_a_fragment_the_buffer_clips_counts_like_a_cut_one(self) -> None: # 8 octets at the largest offset are clipped to 7; 7 octets on a diff --git a/tests/foundation/reassembly/test_ipv6_overlap_2547_unit.py b/tests/foundation/reassembly/test_ipv6_overlap_2547_unit.py new file mode 100644 index 0000000000..bbe741956a --- /dev/null +++ b/tests/foundation/reassembly/test_ipv6_overlap_2547_unit.py @@ -0,0 +1,236 @@ +# -*- coding: utf-8 -*- +"""An overlapping IPv6 fragment abandons its datagram. + +GitHub issue #2547. IPv6 reassembly inherited IPv4's handling of an overlap, +where the arriving fragment overwrites the octets it overlaps as RFC 791 has it +do, so an overlapped datagram still came out ``COMPLETE``. RFC 8200 section 4.5 +says otherwise: "If any of the fragments being reassembled overlap with any +other fragments being reassembled for the same packet, reassembly of that packet +must be abandoned and all the fragments that have been received for that packet +must be discarded". The abandoned datagram is reported as ``OVERLAP``, the way a +timed-out one is reported as ``TIMEOUT``. + +The same section lets an implementation "drop exact duplicate fragments while +keeping the other fragments belonging to the same packet", and IPv6 reassembly +does: a fragment matching one already merged in offset, length, **M** flag and +data changes nothing but the datagram's ``index``. + +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 + +#: Datagram octets, with no zero octet in them, so a hole shows. +DATA = bytes(range(1, 33)) + + +class TestIPv6OverlapAbandonsDatagram(unittest.TestCase): + """Pin RFC 8200 section 4.5's overlap rule on IPv6, and RFC 791's on IPv4.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + def _packet(self, *, num: int, fo: int, mf: bool, payload: bytes, v6: bool = True, + header: bytes = b''): + """A fragment of the one BUFID every case shares.""" + from pcapkit.const.reg.transtype import TransType + from pcapkit.foundation.reassembly.data.ip import Packet + + if v6: + src, dst, ihl = ip_address('2001:db8::1'), ip_address('2001:db8::2'), 40 + else: + src, dst, ihl = ip_address('192.0.2.1'), ip_address('192.0.2.2'), 20 + header = header or b'H' * ihl + return Packet((src, dst, 7, TransType.UDP), num, fo, len(header), mf, + len(header) + len(payload), header, bytearray(payload), 1000.0) + + def _reassemble(self, packets, *, strict: bool = True, v6: bool = True): + from pcapkit.foundation.reassembly.ipv4 import IPv4 + from pcapkit.foundation.reassembly.ipv6 import IPv6 + + reasm = (IPv6 if v6 else IPv4)(strict=strict) + for packet in packets: + reasm(packet) + return reasm + + def test_an_overlapping_first_fragment_abandons_the_datagram(self) -> None: + # the offset-zero fragment arrives last and reaches into the one at 8 + from pcapkit.foundation.reassembly.data.data import Completion + + reasm = self._reassemble([ + self._packet(num=1, fo=8, mf=True, payload=DATA[8:16]), + self._packet(num=2, fo=16, mf=False, payload=DATA[16:20]), + self._packet(num=3, fo=0, mf=True, payload=DATA[:16]), + ]) + datagram, = reasm.datagram + self.assertIs(datagram.completed, Completion.OVERLAP) + self.assertEqual(datagram.index, (1, 2, 3)) + self.assertEqual(datagram.payload, (DATA[8:20],)) + self.assertIsNone(datagram.packet) + # the discarded fragment's header is not kept either + self.assertEqual(datagram.header, b'') + self.assertEqual(reasm._buffer, {}) + + def test_an_overlapping_middle_fragment_abandons_the_datagram(self) -> None: + # the octets it overlaps are the same, which RFC 8200 does not excuse + from pcapkit.foundation.reassembly.data.data import Completion + + reasm = self._reassemble([ + self._packet(num=1, fo=0, mf=True, payload=DATA[:16]), + self._packet(num=2, fo=24, mf=False, payload=DATA[24:28]), + self._packet(num=3, fo=8, mf=True, payload=DATA[8:24]), + ]) + datagram, = reasm.datagram + self.assertIs(datagram.completed, Completion.OVERLAP) + self.assertEqual(datagram.index, (1, 2, 3)) + self.assertEqual(datagram.payload, (DATA[:16], DATA[24:28])) + self.assertEqual(datagram.conflict, ()) + + def test_an_overlapping_last_fragment_abandons_the_datagram(self) -> None: + from pcapkit.foundation.reassembly.data.data import Completion + + other = bytes(b ^ 0xFF for b in DATA[8:20]) + for strict in (True, False): + with self.subTest(strict=strict): + reasm = self._reassemble([ + self._packet(num=1, fo=0, mf=True, payload=DATA[:16]), + self._packet(num=2, fo=8, mf=False, payload=other), + ], strict=strict) + datagram, = reasm.datagram + self.assertIs(datagram.completed, Completion.OVERLAP) + self.assertEqual(datagram.index, (1, 2)) + # the first copy stays; the overlapping one is never written + self.assertEqual(datagram.payload, (DATA[:16],) if strict else DATA[:16]) + self.assertEqual(datagram.conflict, ((8, 15),)) + + def test_the_issues_repro_is_abandoned_and_its_last_fragment_starts_anew(self) -> None: + # two different copies of the offset-zero fragment; the final fragment + # comes after the abandonment, so it opens a buffer of its own + from pcapkit.foundation.reassembly.data.data import Completion + + reasm = self._reassemble([ + self._packet(num=1, fo=0, mf=True, payload=b'A' * 8), + self._packet(num=2, fo=0, mf=True, payload=b'B' * 8), + self._packet(num=3, fo=8, mf=False, payload=b'ZZZZ'), + ]) + late, abandoned = reasm.datagram + self.assertIs(abandoned.completed, Completion.OVERLAP) + self.assertEqual(abandoned.index, (1, 2)) + self.assertEqual(abandoned.payload, (b'A' * 8,)) + self.assertEqual(abandoned.conflict, ((0, 7),)) + self.assertIs(late.completed, Completion.PARTIAL) + self.assertEqual(late.index, (3,)) + self.assertEqual(late.payload, (b'ZZZZ',)) + + def test_the_same_extent_with_another_m_flag_is_not_a_duplicate(self) -> None: + from pcapkit.foundation.reassembly.data.data import Completion + + reasm = self._reassemble([ + self._packet(num=1, fo=8, mf=True, payload=DATA[8:16]), + self._packet(num=2, fo=8, mf=False, payload=DATA[8:16]), + self._packet(num=3, fo=0, mf=True, payload=DATA[:8]), + ]) + late, abandoned = reasm.datagram + self.assertIs(abandoned.completed, Completion.OVERLAP) + self.assertEqual(abandoned.index, (1, 2)) + self.assertIs(late.completed, Completion.PARTIAL) + self.assertEqual(late.index, (3,)) + + def test_exact_duplicates_are_dropped_and_the_datagram_completes(self) -> None: + # a duplicate of the first, a middle and the last fragment, each while + # the datagram is still open + from pcapkit.foundation.reassembly.data.data import Completion + + for strict in (True, False): + with self.subTest(strict=strict): + reasm = self._reassemble([ + self._packet(num=1, fo=0, mf=True, payload=DATA[:8]), + self._packet(num=2, fo=0, mf=True, payload=DATA[:8]), + self._packet(num=3, fo=8, mf=True, payload=DATA[8:16]), + self._packet(num=4, fo=8, mf=True, payload=DATA[8:16]), + self._packet(num=5, fo=24, mf=False, payload=DATA[24:28]), + self._packet(num=6, fo=24, mf=False, payload=DATA[24:28]), + self._packet(num=7, fo=16, mf=True, payload=DATA[16:24]), + ], strict=strict) + datagram, = reasm.datagram + self.assertIs(datagram.completed, Completion.COMPLETE) + self.assertEqual(datagram.payload, DATA[:28]) + self.assertEqual(datagram.index, (1, 2, 3, 4, 5, 6, 7)) + self.assertEqual(datagram.conflict, ()) + + def test_a_dropped_duplicate_does_not_replace_the_kept_header(self) -> None: + # RFC 8200 retains only the offset-zero fragment's headers, and those + # before the Fragment header may differ between copies + from pcapkit.foundation.reassembly.data.data import Completion + + kept, other = b'K' * 40, b'D' * 40 + reasm = self._reassemble([ + self._packet(num=1, fo=0, mf=True, payload=DATA[:8], header=kept), + self._packet(num=2, fo=0, mf=True, payload=DATA[:8], header=other), + self._packet(num=3, fo=8, mf=False, payload=DATA[8:12]), + ]) + datagram, = reasm.datagram + self.assertIs(datagram.completed, Completion.COMPLETE) + self.assertEqual(datagram.header, kept) + + def test_fragments_that_only_abut_complete_the_datagram(self) -> None: + # the control: out of order, touching but never sharing an octet + from pcapkit.foundation.reassembly.data.data import Completion + + for strict in (True, False): + with self.subTest(strict=strict): + reasm = self._reassemble([ + self._packet(num=1, fo=16, mf=False, payload=DATA[16:20]), + self._packet(num=2, fo=0, mf=True, payload=DATA[:8]), + self._packet(num=3, fo=8, mf=True, payload=DATA[8:16]), + ], strict=strict) + datagram, = reasm.datagram + self.assertIs(datagram.completed, Completion.COMPLETE) + self.assertEqual(datagram.payload, DATA[:20]) + self.assertEqual(datagram.conflict, ()) + + def test_a_fragment_carrying_no_data_overlaps_nothing(self) -> None: + # an empty final fragment shares no octet, even with a duplicate of itself + from pcapkit.foundation.reassembly.data.data import Completion + + reasm = self._reassemble([ + self._packet(num=1, fo=8, mf=True, payload=DATA[8:16]), + self._packet(num=2, fo=16, mf=False, payload=b''), + self._packet(num=3, fo=16, mf=False, payload=b''), + self._packet(num=4, fo=0, mf=True, payload=DATA[:8]), + ]) + datagram, = reasm.datagram + self.assertIs(datagram.completed, Completion.COMPLETE) + self.assertEqual(datagram.payload, DATA[:16]) + + def test_the_abandonment_is_logged(self) -> None: + with self.assertLogs('pcapkit.foundation.reassembly.ip', level='DEBUG') as logs: + self._reassemble([ + self._packet(num=1, fo=0, mf=True, payload=DATA[:16]), + self._packet(num=2, fo=8, mf=True, payload=DATA[8:16]), + ]) + self.assertEqual(len(logs.records), 1) + self.assertIn('on frame 2, which overlaps octets 0-15', logs.output[0]) + + def test_ipv4_still_overwrites_as_rfc_791_asks(self) -> None: + from pcapkit.foundation.reassembly.data.data import Completion + + reasm = self._reassemble([ + self._packet(num=1, fo=0, mf=True, payload=b'A' * 8, v6=False), + self._packet(num=2, fo=0, mf=True, payload=b'B' * 8, v6=False), + self._packet(num=3, fo=8, mf=False, payload=b'ZZZZ', v6=False), + ], v6=False) + datagram, = reasm.datagram + self.assertIs(datagram.completed, Completion.COMPLETE) + self.assertEqual(datagram.index, (1, 2, 3)) + self.assertEqual(datagram.payload, b'B' * 8 + b'ZZZZ') + self.assertEqual(datagram.conflict, ((0, 7),)) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/foundation/reassembly/test_reassembly_payload_roundtrip_unit.py b/tests/foundation/reassembly/test_reassembly_payload_roundtrip_unit.py index 4435fa4dd6..689ffcfc25 100644 --- a/tests/foundation/reassembly/test_reassembly_payload_roundtrip_unit.py +++ b/tests/foundation/reassembly/test_reassembly_payload_roundtrip_unit.py @@ -21,6 +21,11 @@ 791, Linux, FreeBSD, Suricata, Zeek and Wireshark all do, so those cases expect both datagrams. +RFC 8200 section 4.5 abandons an IPv6 datagram on any overlap but an exact +duplicate (#2547), so an IPv6 overlap case whose extra piece is not one expects +the abandoned datagram, ``OVERLAP``, and a ``PARTIAL`` one from the later +fragments. + Every case builds its own octets and reads no capture. Classes are imported inside each test, after :func:`~tests._support.reimport_once_per_class`. @@ -160,11 +165,23 @@ def _ip_cases() -> 'list[Case]': # one extra piece straddling a boundary, with the same octets frames = [_ip_frame(kind, IP_PAYLOAD, *piece, 0x1234) for piece in pieces] end = next(offset + size for offset, size in pieces if offset + size >= 24) - 8 - frames.insert(1, _ip_frame(kind, IP_PAYLOAD, 8, (end - 8) - (end - 8) % 8, 0x1234)) - stream = Stream(IP_PAYLOAD, tuple(range(1, len(frames) + 1)), header) + extra = (8, (end - 8) - (end - 8) % 8) + frames.insert(1, _ip_frame(kind, IP_PAYLOAD, *extra, 0x1234)) for strict in (True, False): + if kind == 'ipv6' and extra not in pieces: + # RFC 8200 section 4.5 abandons the datagram on the extra + # piece, and the rest open a new buffer (#2547) + first = IP_PAYLOAD[:pieces[0][1]] + rest = IP_PAYLOAD[pieces[0][1]:] + streams = ( + Stream((first,) if strict else first, (1, 2), None, 'OVERLAP'), + Stream((rest,) if strict else bytes(len(first)) + rest, + tuple(range(3, len(frames) + 1)), None, 'PARTIAL'), + ) + else: + streams = (Stream(IP_PAYLOAD, tuple(range(1, len(frames) + 1)), header),) cases.append(Case(f'{kind}/{cut}/overlap/{"strict" if strict else "loose"}', - kind, tuple(frames), (stream,), strict)) + kind, tuple(frames), streams, strict)) # unfragmented: one packet, offset 0 and MF clear if kind == 'ipv4':