diff --git a/pcapkit/foundation/engines/pyshark.py b/pcapkit/foundation/engines/pyshark.py index c2093b9cb7..48dfe392bf 100644 --- a/pcapkit/foundation/engines/pyshark.py +++ b/pcapkit/foundation/engines/pyshark.py @@ -25,6 +25,7 @@ from typing import Optional from pyshark.capture.file_capture import FileCapture + from pyshark.packet.layers.base import BaseLayer from pyshark.packet.packet import Packet as PySharkPacket from pcapkit.foundation.extraction import Extractor @@ -34,6 +35,45 @@ logger = get_logger(__name__) +def _frame_layer(packet: 'PySharkPacket') -> 'Optional[BaseLayer]': + """The ``frame`` layer of a `PyShark`_ packet, or :data:`None` when the record has none. + + Looked up by name, because `PyShark`_ takes the second PDML ```` as + ``frame_info``. On a packet with a comment, that is ``pkt_comment``, which has + no ``protocols``, and the frame layer is ``layers[0]``. By its source, + :program:`tshark` 4.2.2 roots a Sysdig event at ``syscall`` and a Netflix + custom block at ``bblog``, so those records have no ``frame`` layer. + + .. _PyShark: https://kiminewt.github.io/pyshark + + """ + for layer in (packet.frame_info, *packet.layers): + if layer.layer_name == 'frame': + return layer + return None + + +def _is_packet(packet: 'PySharkPacket') -> 'bool': + """Whether a `PyShark`_ packet is a packet record, rather than another kind of tshark record. + + :program:`tshark` numbers every record it reads, not only packets: a PCAP-NG + systemd Journal Export Block, Custom Block or Sysdig Event Block is a frame + to it, measured with :program:`tshark` 4.6.9. The default engine counts + Enhanced, Simple and obsolete Packet Blocks only. + + The marker is ``frame.encap_type`` on the record's :func:`_frame_layer`. + Wireshark adds it for ``REC_TYPE_PACKET`` records only: + :file:`epan/dissectors/packet-frame.c` lines 1001-1002 in 4.2.2, and 828-890 + in 4.6.9. The summary line is not used, because it differs between those + versions. A record with no ``frame`` layer is not a packet either. + + .. _PyShark: https://kiminewt.github.io/pyshark + + """ + frame = _frame_layer(packet) + return frame is not None and 'encap_type' in frame.field_names + + class PyShark(EngineBase['PySharkPacket']): """PyShark engine support. @@ -214,10 +254,12 @@ def run(self) -> 'None': f"so 'ipv4={ext._ipv4}', 'ipv6={ext._ipv6}' and 'tcp={ext._tcp}' will be ignored", AttributeWarning, stacklevel=stacklevel()) - # setup verbose handler + # setup verbose handler; a packet always has a frame layer, since + # read_frame skips every record that does not (#1515) if ext._flag_v: ext._vfunc = lambda e, f: print( - f'Frame {e._frnum:>3d}: {f.frame_info.protocols}' # pylint: disable=protected-access + f'Frame {e._frnum:>3d}: ' # pylint: disable=protected-access + f'{cast("BaseLayer", _frame_layer(f)).protocols}' ) # extract global header @@ -230,6 +272,10 @@ def run(self) -> 'None': def read_frame(self) -> 'PySharkPacket': """Read frames with PyShark engine. + A tshark record that is not a packet, such as a PCAP-NG systemd Journal + Export Block, is skipped. Frames are numbered by count, as the default + engine numbers them, not by tshark's frame number. + Returns: Parsed frame instance. @@ -241,11 +287,15 @@ def read_frame(self) -> 'PySharkPacket': from pcapkit.toolkit.pyshark import packet2dict, tcp_traceflow ext = self._extractor - # fetch PyShark packet - packet = cast('PySharkPacket', self._extmp.next()) + # fetch the next PyShark packet that is a packet record + while True: + packet = cast('PySharkPacket', self._extmp.next()) + if _is_packet(packet): + break + logger.debug('pyshark: skipping tshark frame %s, not a packet record', packet.number) # verbose output - ext._frnum = int(packet.number) + ext._frnum += 1 ext._vfunc(ext, packet) # write plist diff --git a/tests/foundation/engines/test_engine_agreement_runtime.py b/tests/foundation/engines/test_engine_agreement_runtime.py index 874a03fc65..8cbfb67ee9 100644 --- a/tests/foundation/engines/test_engine_agreement_runtime.py +++ b/tests/foundation/engines/test_engine_agreement_runtime.py @@ -349,11 +349,6 @@ class TestEngineAgreement(harness.RoundTripBase): tuple(f'{capture}/pypcapfile/{aspect}' for capture in ('http6.cap', 'stream.pcap', 'tcp.pcap', 'test.pcap') for aspect in ('tcp', 'trace', 'datagrams', 'flows'))), - Gap(1515, 'the pyshark engine counts a Systemd Journal Export Block as a frame: tshark ' - 'numbers it as one, and read_frame takes every tshark frame ' - '(pcapkit/foundation/engines/pyshark.py:245-248)', - ('FRAMES', 'RECORDS'), ('6 frames != 5', 'frame 5: timestamp '), - ('test.pcapng/pyshark/frames', 'test.pcapng/pyshark/timestamps')), Gap(1501, 'the dpkt engine hands reassembly the Decimal timestamp dpkt reads from a ' 'nanosecond PCAP (pcapkit/foundation/engines/dpkt.py:309), where every other ' 'engine hands it a float (pcapkit/toolkit/pcap.py:73)', diff --git a/tests/foundation/engines/test_pyshark_non_packet_unit.py b/tests/foundation/engines/test_pyshark_non_packet_unit.py new file mode 100644 index 0000000000..25707b4e74 --- /dev/null +++ b/tests/foundation/engines/test_pyshark_non_packet_unit.py @@ -0,0 +1,141 @@ +# -*- coding: utf-8 -*- +"""The PyShark engine reads packet records only, as the default engine does. C.f. #1515. + +:program:`tshark` numbers every record wiretap hands it, and a PCAP-NG systemd +Journal Export Block, Custom Block or Sysdig Event Block is one. The default +engine counts Enhanced, Simple and obsolete Packet Blocks only, so a capture +holding a journal block read as one frame more through ``pyshark``, and every +later frame was off by one. + +The same lookup serves the verbose handler. ``pyshark`` takes the second PDML +```` as ``frame_info``, and on a packet that carries a comment that is +``pkt_comment``, which has no ``protocols``. So ``verbose=True`` raised +:exc:`AttributeError` on frame 1 of :file:`examples/captures/test.pcapng`. + +The packets here are stand-ins shaped like what ``pyshark`` 0.6 builds from +:program:`tshark` 4.6.9's PDML. The field names are the ones measured on +:file:`examples/captures/test.pcapng` and on a capture holding a Custom Block and +a Sysdig Event Block. ``pyshark`` itself is never imported, so this runs where it +is absent or cannot run. + +""" + +import contextlib +import importlib.util +import io +import types +import unittest + +from tests._support import reimport_once_per_class + +RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') +HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) + +#: ``frame`` layer fields of a packet record; only these carry ``encap_type``. +PACKET_FRAME = ('section_number', 'interface_id', 'interface_name', 'encap_type', 'time', + 'time_epoch', 'number', 'len', 'cap_len', 'protocols') +#: ``frame`` layer fields of a systemd Journal Export or Sysdig Event record. +RECORD_FRAME = ('section_number', 'time', 'time_utc', 'time_epoch', 'number', 'len', 'cap_len', + 'protocols') +#: ``frame.protocols`` of frame 1 of :file:`examples/captures/test.pcapng`. +PROTOCOLS = 'eth:ethertype:ip:udp:dhcp' + + +def _layer(name: str, fields: 'tuple[str, ...]' = (), **values: str) -> types.SimpleNamespace: + return types.SimpleNamespace(layer_name=name, field_names=list(fields), **values) + + +def _packet(number: int, comment: bool = False) -> types.SimpleNamespace: + """A packet record; with a comment, ``pyshark`` takes ``pkt_comment`` as ``frame_info``.""" + frame = _layer('frame', PACKET_FRAME, protocols=PROTOCOLS) + eth = _layer('eth', ('dst', 'src', 'type')) + if comment: + note = _layer('pkt_comment', ('frame_comment',)) + return types.SimpleNamespace(number=str(number), frame_info=note, layers=[frame, eth]) + return types.SimpleNamespace(number=str(number), frame_info=frame, layers=[eth]) + + +def _record(number: int, frame: 'tuple[str, ...] | None', root: str) -> types.SimpleNamespace: + """A non-packet record; ``frame=None`` for a root that is not ``frame`` at all.""" + head = _layer('frame', frame) if frame is not None else _layer(root, ('num',)) + layers = [_layer(root, ('message',))] if frame is not None else [] + return types.SimpleNamespace(number=str(number), frame_info=head, layers=layers) + + +#: Every non-packet record shape, and where it was seen. +RECORDS = { + 'systemd journal (4.6.9)': ('systemd_journal', RECORD_FRAME), + 'custom block (4.6.9)': ('DATA', ('section_number', 'interface_id', 'time', 'number', 'len')), + 'sysdig event (4.6.9)': ('sysdig', RECORD_FRAME), + # 4.2.2 roots a Sysdig event at ``syscall`` and a Netflix custom block at + # ``bblog`` instead of ``frame`` (epan/dissectors/packet-frame.c:858-869, :884-890). + 'sysdig event (4.2.2)': ('syscall', None), + 'black box log (4.2.2)': ('bblog', None), +} + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class TestPySharkNonPacketRecords(unittest.TestCase): + """:meth:`PyShark.read_frame` skips every record that is not a packet.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + @staticmethod + def _engine(records: 'list[types.SimpleNamespace]') -> 'tuple[object, types.SimpleNamespace]': + from pcapkit.foundation.engines.pyshark import PyShark + + stream = iter(records) + extractor = types.SimpleNamespace(_frnum=0, _vfunc=lambda e, f: None, _flag_q=True, + _flag_t=False, _flag_d=True, _frame=[]) + engine = PyShark.__new__(PyShark) # skip __init__, which imports pyshark + engine._extractor = extractor + engine._extmp = types.SimpleNamespace(next=lambda: next(stream)) + return engine, extractor + + def test_a_journal_record_is_not_a_frame(self) -> None: + first, last = _packet(1, comment=True), _packet(3) + engine, ext = self._engine([first, _record(2, RECORD_FRAME, 'systemd_journal'), last]) + + self.assertIs(engine.read_frame(), first) + self.assertEqual(ext._frnum, 1) + self.assertIs(engine.read_frame(), last) + self.assertEqual(ext._frnum, 2, 'frames are numbered as the default engine numbers them') + self.assertEqual(ext._frame, [first, last]) + with self.assertRaises(StopIteration): + engine.read_frame() + + def test_every_record_without_an_encapsulation_is_skipped(self) -> None: + for label, (root, frame) in RECORDS.items(): + with self.subTest(record=label): + record = _record(2, frame, root) + engine, ext = self._engine([_packet(1), record, _packet(3)]) + frames = [engine.read_frame(), engine.read_frame()] + self.assertNotIn(record, frames) + self.assertEqual([f.number for f in frames], ['1', '3']) + self.assertEqual(ext._frnum, 2) + + def test_a_trailing_record_ends_the_capture(self) -> None: + engine, ext = self._engine([_packet(1), _record(2, RECORD_FRAME, 'systemd_journal')]) + engine.read_frame() + with self.assertRaises(StopIteration): + engine.read_frame() + self.assertEqual(ext._frnum, 1) + + def test_verbose_mode_prints_a_commented_packet(self) -> None: + engine, ext = self._engine([_packet(1, comment=True), _packet(2)]) + vars(ext).update(_exlyr='none', _exptl='null', _exctx=None, _flag_r=False, _flag_v=True, + _ifnm='test.pcapng', record_header=lambda: None) + stream = engine._extmp + engine._expkg = types.SimpleNamespace(FileCapture=lambda *args, **kwargs: stream) + engine.run() # installs the verbose handler + + with contextlib.redirect_stdout(io.StringIO()) as out: + engine.read_frame() + engine.read_frame() + self.assertEqual(out.getvalue().splitlines(), + [f'Frame 1: {PROTOCOLS}', f'Frame 2: {PROTOCOLS}']) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/foundation/engines/test_runtime_engines.py b/tests/foundation/engines/test_runtime_engines.py index 1d3efedc9e..19a26d451f 100644 --- a/tests/foundation/engines/test_runtime_engines.py +++ b/tests/foundation/engines/test_runtime_engines.py @@ -432,8 +432,10 @@ def test_pyshark_run_read_frame_and_close(self) -> None: class FakeCapture: def __init__(self) -> None: self.closed = False - self.packet = types.SimpleNamespace(number='7', - frame_info=types.SimpleNamespace(protocols='eth:ip:tcp')) + # a packet record: its frame layer carries frame.encap_type (#1515) + frame = types.SimpleNamespace(layer_name='frame', field_names=['encap_type'], + protocols='eth:ip:tcp') + self.packet = types.SimpleNamespace(number='7', frame_info=frame, layers=[]) def next(self): return self.packet @@ -466,8 +468,9 @@ def close(self) -> None: with mock.patch('pcapkit.toolkit.pyshark.tcp_traceflow', return_value='trace'): packet = engine.read_frame() self.assertIs(packet, capture.packet) - self.assertEqual(extractor._frnum, 7) - self.assertEqual(sink.paths[-1], 'out/Frame 7.json') + # numbered by count, as the default engine numbers frames, not by tshark's '7' + self.assertEqual(extractor._frnum, 1) + self.assertEqual(sink.paths[-1], 'out/Frame 1.json') extractor._trace.tcp.assert_called_once_with('trace') self.assertEqual(extractor._frame, [packet]) @@ -480,7 +483,7 @@ def close(self) -> None: with mock.patch('pcapkit.toolkit.pyshark.packet2dict', return_value={'packet': True}): with mock.patch('pcapkit.toolkit.pyshark.tcp_traceflow', return_value=None): self.assertIs(engine.read_frame(), capture.packet) - self.assertEqual(stream_sink.records[-1][1], 'Frame 7') + self.assertEqual(stream_sink.records[-1][1], 'Frame 1') stream_mode._trace.tcp.assert_not_called() disabled, _ = make_extractor(_flag_q=True, _flag_t=False, _flag_d=False) diff --git a/tests/foundation/test_extraction_record_header_unit.py b/tests/foundation/test_extraction_record_header_unit.py index 6b27a6e38d..ad84155cfa 100644 --- a/tests/foundation/test_extraction_record_header_unit.py +++ b/tests/foundation/test_extraction_record_header_unit.py @@ -134,7 +134,9 @@ def close(self) -> None: return fmt, records def test_pcap_output_opens_with_the_global_header(self) -> None: - packet = types.SimpleNamespace(number='1') + # a packet record, as pyshark builds one: its frame layer carries frame.encap_type (#1515) + frame = types.SimpleNamespace(layer_name='frame', field_names=['encap_type']) + packet = types.SimpleNamespace(number='1', frame_info=frame, layers=[]) fmt, records = self._run(sample_path('in.pcap'), [packet]) self.assertEqual(fmt, 'json') self.assertEqual(list(records), ['Global Header', 'Frame 1']) diff --git a/tests/toolkit/test_pyshark_unit.py b/tests/toolkit/test_pyshark_unit.py index 8c40dd4094..21c8df081e 100644 --- a/tests/toolkit/test_pyshark_unit.py +++ b/tests/toolkit/test_pyshark_unit.py @@ -617,9 +617,8 @@ def test_journal_block_then_two_tcp_packets(self) -> None: def test_the_engine_passes_its_frame_count(self) -> None: """:meth:`PyShark.read_frame` hands :func:`tcp_traceflow` its own frame - count. Without #1531 that count is still tshark's number, so the journal - block shifts it here (2 and 3); with #1531 it is 1 and 2. Either way the - traced index is the engine's count, never ``packet.number`` read again. + count. The journal block is skipped and not counted (#1515), so the two + packets trace as 1 and 2, never as tshark's ``packet.number``. """ import types @@ -651,7 +650,7 @@ def test_the_engine_passes_its_frame_count(self) -> None: if packet is not journal: counts.append(extractor._frnum) self.assertEqual(traced, counts) - self.assertIn(traced, ([2, 3], [1, 2])) + self.assertEqual(traced, [1, 2]) @unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed')