diff --git a/pcapkit/dumpkit/pcap.py b/pcapkit/dumpkit/pcap.py index 73b7d8e6d2..7645079016 100644 --- a/pcapkit/dumpkit/pcap.py +++ b/pcapkit/dumpkit/pcap.py @@ -230,6 +230,16 @@ def _append_value(self, value: 'Data_Frame', file: 'IO[bytes]', name: 'str') -> 3-octet payload would raise ``SchemaWarning: packet length < 0: -3`` from a dumper that only has to copy it. + ``incl_len`` is written as ``len(value.packet)``, the octets that + follow it, not as ``value.frame_info.incl_len``. The two are equal + for every record a file holds whole. A record whose captured length + runs past the end of its file, or a PCAP-NG block whose captured + length runs past the block (:issue:`1405`), is *kept as captured* + by its reader: the declared length stays in ``frame_info``, so the + source rebuilds byte-exactly, and ``packet`` is the octets present. + Copying that declared length would claim octets the trace does not + hold, and a reader would run off the end of the file (:issue:`1541`). + """ # NOTE: The payload is read before the metadata so that a caller passing a # mapping rather than a dissected frame -- which the flow-tracing adapters @@ -242,7 +252,7 @@ def _append_value(self, value: 'Data_Frame', file: 'IO[bytes]', name: 'str') -> file.write(self._rechdr.pack(ts_sec & _UINT32_MASK, ts_frac & _UINT32_MASK, - frame_info.incl_len & _UINT32_MASK, + len(packet) & _UINT32_MASK, frame_info.orig_len & _UINT32_MASK) + packet) self._fnum += 1 diff --git a/pcapkit/protocols/misc/pcap/frame.py b/pcapkit/protocols/misc/pcap/frame.py index f92d1ef397..a35b793035 100644 --- a/pcapkit/protocols/misc/pcap/frame.py +++ b/pcapkit/protocols/misc/pcap/frame.py @@ -236,6 +236,12 @@ def read(self, length: 'Optional[int]' = None, *, _read: 'bool' = True, Returns: Data_Frame: Parsed packet data. + Warns: + pcapkit.utilities.warnings.ProtocolWarning: If the timestamp is out + of range, or if the record's ``incl_len`` runs past the end of + the data being parsed, in which case the record is kept as + captured. + """ schema = self.__header__ @@ -315,6 +321,19 @@ def read(self, length: 'Optional[int]' = None, *, _read: 'bool' = True, #: bytes: Raw frame data. self._data = self._read_fileng(self.length + _ilen) + # NOTE: A record whose ``incl_len`` runs past the end of the data is + # kept as captured, as the PCAP-NG reader keeps a captured length + # running past its block (:issue:`1405`): ``incl_len`` and ``cap_len`` + # stay as declared, so the record rebuilds byte-exactly, and the + # packet is the octets present. A writer must not copy the declared + # length, c.f. :meth:`PCAPIO._append_value + # ` (:issue:`1541`). + held = len(self._data) - self.length + if held < _ilen: + warn(f'PCAP: [Frame {self._fnum}] captured length {_ilen} runs past the data, ' + f'which holds {held} octet(s); kept as captured', + ProtocolWarning, stacklevel=stacklevel()) + # move forward to the beginning of the next frame self._file.seek(seek_cur, io.SEEK_SET) diff --git a/tests/dumpkit/test_pcap_trace_truncated_record_1541_unit.py b/tests/dumpkit/test_pcap_trace_truncated_record_1541_unit.py new file mode 100644 index 0000000000..7ee7930e38 --- /dev/null +++ b/tests/dumpkit/test_pcap_trace_truncated_record_1541_unit.py @@ -0,0 +1,150 @@ +# -*- coding: utf-8 -*- +"""A PCAP trace writes the octets a record has, not its claimed length. C.f. #1541. + +A record whose ``incl_len`` runs past the end of its file, or a PCAP-NG block +whose captured length runs past the block (#1405), is kept as captured by its +reader: the declared length stays in ``frame_info`` and the packet is the octets +present. :class:`~pcapkit.dumpkit.pcap.PCAPIO` copied the declared length into the +trace record header, so the trace claimed octets it did not hold -- measured on a +record claiming ``0x7fffffff`` over 585 octets. + +Each case traces a TCP flow whose last record is cut that way, then reads the +trace back with :mod:`struct` alone: every record must hold exactly ``incl_len`` +octets, and the last must end the file. + +Every case builds its own octets and reads no capture. Classes are imported +inside each test, after :func:`~tests._support.reimport_once_per_class`. + +""" + +import importlib.util +import io +import os +import struct +import tempfile +import unittest +import warnings + +from tests._support import close_extractor, reimport_once_per_class +from tests.foundation._roundtrip import ethernet, ipv4, pcap, pcapng, tcp + +HAS_RUNTIME = all(importlib.util.find_spec(name) is not None + for name in ('tbtrim', 'aenum', 'chardet', 'dictdumper')) + +SECOND = 1_500_000_000 +#: Captured lengths a cut record declares: a little past the data, and the +#: measured case. +CLAIMS = (1000, 0x7FFF_FFFF) + + +def _frames() -> 'list[bytes]': + """Three TCP segments of one connection, client to server.""" + return [ethernet(ipv4(tcp(b'x' * number, seq=1000 + number, syn=not number), proto=6), 0x0800) + for number in range(3)] + + +def _records(data: 'bytes') -> 'list[tuple[int, int, bytes]]': + """Every record of a little-endian PCAP file, refusing one that runs past it.""" + records, offset = [], 24 + while offset < len(data): + if offset + 16 > len(data): + raise AssertionError(f'{len(data) - offset} octet(s) after the last record') + _, _, incl, orig = struct.unpack_from(' len(data): + raise AssertionError(f'record at {offset} claims {incl} octet(s), ' + f'the file holds {len(data) - offset - 16}') + records.append((incl, orig, data[offset + 16:offset + 16 + incl])) + offset += 16 + incl + return records + + +def _cut_pcap(claim: 'int') -> 'bytes': + """A PCAP file whose last record claims ``claim`` octets and holds its frame.""" + frames = _frames() + data = pcap([(SECOND + number, 0, frame) for number, frame in enumerate(frames[:-1])]) + return data + struct.pack(' 'bytes': + """A PCAP-NG file whose last EPB declares ``claim`` octets over its frame (#1405).""" + frames = _frames() + data = pcapng([(SECOND + number, 0, frame) for number, frame in enumerate(frames[:-1])]) + ticks = (SECOND + 2) * 10 ** 6 + body = struct.pack('> 32, ticks & 0xFFFF_FFFF, claim, len(frames[-1])) + frames[-1] + body += bytes(-len(body) % 4) + return data + struct.pack(' None: + reimport_once_per_class(self) + self.tmpdir = tempfile.TemporaryDirectory() # pylint: disable=consider-using-with + self.addCleanup(self.tmpdir.cleanup) + + def trace(self, data: 'bytes') -> 'bytes': + """The PCAP file of the one TCP flow traced from ``data``.""" + from pcapkit import extract + + tracedir = tempfile.mkdtemp(dir=self.tmpdir.name) + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + extractor = extract(fin=io.BytesIO(data), nofile=True, store=False, trace=True, tcp=True, + trace_fout=tracedir, trace_format='pcap', trace_byteorder='little') + close_extractor(extractor) + flows = list(extractor.trace.tcp) + self.assertEqual([list(flow.index) for flow in flows], [[1, 2, 3]]) + with open(flows[0].fpout, 'rb') as file: + return file.read() + + def check(self, trace: 'bytes', held: 'bytes') -> None: + """The trace holds every frame whole, the cut one as the octets it has.""" + frames, records = _frames(), _records(trace) + self.assertEqual([octets for _, _, octets in records], frames[:-1] + [held]) + self.assertEqual([(incl, orig) for incl, orig, _ in records], + [(len(frame), len(frame)) for frame in frames[:-1]] + [(len(held), len(frames[-1]))]) + + def test_a_pcap_record_cut_at_eof_is_traced_as_the_octets_it_has(self) -> None: + """#1541: a record claiming more than the file holds writes what it holds.""" + for claim in CLAIMS: + with self.subTest(claim=claim): + self.check(self.trace(_cut_pcap(claim)), _frames()[-1]) + + def test_a_pcapng_captured_length_past_its_block_is_traced_as_the_octets_it_has(self) -> None: + """#1541: so does an EPB whose captured length runs past the block (#1405).""" + for claim in CLAIMS: + with self.subTest(claim=claim): + self.check(self.trace(_overrun_pcapng(claim)), _frames()[-1]) + + def test_a_pcapng_block_cut_at_eof_is_traced_as_the_octets_it_has(self) -> None: + """#1541: and an EPB the file ends inside, which holds two octets fewer than it claims.""" + data = pcapng([(SECOND + number, 0, frame) for number, frame in enumerate(_frames())]) + # the trailing Block Total Length, then the last two octets of the frame + self.check(self.trace(data[:-6]), _frames()[-1][:-2]) + + def test_the_dumper_writes_the_packet_length(self) -> None: + """#1541: ``incl_len`` is ``len(packet)``, whatever ``frame_info`` declares.""" + from pcapkit.const.reg.linktype import LinkType + from pcapkit.dumpkit.pcap import PCAPIO + from pcapkit.protocols.data.misc.pcap.frame import Frame, FrameInfo + + path = os.path.join(self.tmpdir.name, 'out.pcap') + dumper = PCAPIO(path, protocol=LinkType.ETHERNET, byteorder='little', nanosecond=False) + for incl_len, packet in ((0x7FFF_FFFF, b'abc'), (3, b'abc'), (0, b'')): + frame = Frame(frame_info=FrameInfo(ts_sec=7, ts_usec=0, incl_len=incl_len, orig_len=86), + time='time', number=1, time_epoch=7, len=86, cap_len=incl_len) + frame.__update__(packet=packet) + dumper(frame) + with open(path, 'rb') as file: + self.assertEqual(_records(file.read()), [(3, 86, b'abc'), (3, 86, b'abc'), (0, 86, b'')]) + + def test_a_whole_record_is_traced_unchanged(self) -> None: + """A record the file holds whole keeps its octets, as before #1541.""" + data = pcap([(SECOND + number, 0, frame) for number, frame in enumerate(_frames())]) + self.assertEqual(self.trace(data)[24:].hex(), data[24:].hex()) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/misc/pcap/test_frame_incl_len_overrun_1541_unit.py b/tests/protocols/misc/pcap/test_frame_incl_len_overrun_1541_unit.py new file mode 100644 index 0000000000..4248096bf0 --- /dev/null +++ b/tests/protocols/misc/pcap/test_frame_incl_len_overrun_1541_unit.py @@ -0,0 +1,93 @@ +# -*- coding: utf-8 -*- +"""A PCAP record claiming more octets than its file holds warns, and is kept as captured. + +GitHub issue #1541. A record whose ``incl_len`` runs past the end of the file +parsed silently. It is now reported the way the PCAP-NG reader reports a +captured length running past its block (#1405), and is kept as captured in the +same way: ``incl_len`` and ``cap_len`` stay as declared, the packet is the octets +present, and the frame rebuilds as exactly the octets read, from ``info`` and +from ``info.to_dict()`` alike. + +Every case builds its own octets and reads no capture. Classes are imported +inside each test, after :func:`~tests._support.reimport_once_per_class`. + +""" + +import io +import struct +import unittest +import warnings + +from tests._support import close_extractor, reimport_once_per_class + +#: Little-endian global header: v2.4, Ethernet. +GLOBAL = bytes.fromhex('d4c3b2a1' '0200' '0400' '00000000' '00000000' 'ffff0000' '01000000') +#: Sixty octets of packet data, none of them zero. +PACKET = bytes(range(1, 61)) + + +def _record(claim: 'int', packet: 'bytes' = PACKET) -> 'bytes': + """A record whose header claims ``claim`` octets, followed by ``packet``.""" + return struct.pack(' None: + reimport_once_per_class(self) + + def parse(self, octets: 'bytes') -> 'tuple[object, list[str]]': + """The frame parsed from ``octets``, and the warnings it raised.""" + from pcapkit.protocols.misc.pcap.frame import Frame + from pcapkit.protocols.misc.pcap.header import Header + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + frame = Frame(io.BytesIO(octets), num=4, header=Header(GLOBAL).info) + return frame, [str(item.message) for item in caught] + + def test_overrun_warns_and_is_kept_as_captured(self) -> None: + from pcapkit.protocols.misc.pcap.frame import Frame + from pcapkit.protocols.misc.pcap.header import Header + + header = Header(GLOBAL).info + for claim in (61, 1000, 0x7FFF_FFFF): + with self.subTest(claim=claim): + octets = _record(claim) + frame, messages = self.parse(octets) + self.assertIn(f'PCAP: [Frame 4] captured length {claim} runs past the data, ' + f'which holds {len(PACKET)} octet(s); kept as captured', messages) + + info = frame.info # type: ignore[attr-defined] + self.assertEqual((info.frame_info.incl_len, info.cap_len, info.len), (claim, claim, 86)) + self.assertEqual(bytes(info.packet), PACKET) + self.assertEqual(frame.data, octets) # type: ignore[attr-defined] + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + for data in (info, info.to_dict()): + rebuilt = Frame.from_data(data, num=4, header=header) + self.assertEqual(rebuilt.data.hex(), octets.hex()) + + def test_whole_record_does_not_warn(self) -> None: + for packet in (PACKET, PACKET + b'\x00' * 16): + with self.subTest(trailing=len(packet) - len(PACKET)): + frame, messages = self.parse(_record(len(PACKET), packet)) + self.assertFalse([text for text in messages if 'runs past' in text], messages) + self.assertEqual(bytes(frame.info.packet), PACKET) # type: ignore[attr-defined] + + def test_extraction_keeps_the_cut_frame(self) -> None: + import pcapkit + + octets = GLOBAL + _record(len(PACKET)) + _record(1000) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + extractor = pcapkit.extract(fin=io.BytesIO(octets), nofile=True, store=True, engine='default') + close_extractor(extractor) + frames = list(extractor.frame) + self.assertEqual([bytes(frame.info.packet) for frame in frames], [PACKET, PACKET]) + self.assertEqual(sum('runs past the data' in str(item.message) for item in caught), 1) + + +if __name__ == '__main__': + unittest.main()