Skip to content

Commit b816978

Browse files
committed
fix(pcapng): handle make(block=<bytes>) for a DSB (#2354)
`block` is typed to accept raw bytes, and the header schema writes such a block back verbatim past the Block Type -- but `read` then looked for the block's fields on the schema `make` returned, which held the octets, so a Decryption Secrets Block built that way raised `AttributeError: 'bytes' object has no attribute 'length'`. So did every other block type. `unpack` now parses those octets off the buffer `pack` has just filled, as a reader of the file would, instead of reusing the schema that carries them. The block that comes back is the one the octets describe, and it rebuilds byte for byte; octets that are no block raise `StreamEOFError` or `ProtocolError` instead. Tested by a new unit module: the issue's DSB, with its TLS key log decoded and its octets written back unchanged; one round trip per block type; and the short and length-zero cases.
1 parent 7b57bb0 commit b816978

2 files changed

Lines changed: 163 additions & 5 deletions

File tree

‎pcapkit/protocols/misc/pcapng.py‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1260,14 +1260,20 @@ def unpack(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_PCAPN
12601260
global packet data to underlying methods. This is useful when
12611261
the packet data is not available in the current instance.
12621262
1263+
A block handed to :meth:`make` as raw :obj:`bytes` is written back
1264+
verbatim past the Block Type, so the schema :meth:`make` returns
1265+
holds the octets rather than the block's fields. Those octets are
1266+
parsed here, off the buffer :meth:`pack` has just filled, which is
1267+
what a reader of the file would do with them (:issue:`2354`).
1268+
12631269
"""
1264-
if cast('Optional[Schema_PCAPNG]', self.__header__) is None:
1270+
header = cast('Optional[Schema_PCAPNG]', self.__header__)
1271+
if header is None or isinstance(header.block, bytes):
12651272
packet = kwargs.get('__packet__', {}) # packet data
12661273

1267-
# NOTE: Reached on the parsing path only, with no flag needed: the
1268-
# construction path sets ``__header__`` from ``make`` while packing
1269-
# its own buffer, so the branch above is already false there. A
1270-
# ``_read`` guard would be a branch that can never be taken.
1274+
# NOTE: A tail too short for any block is refused here, on the
1275+
# construction path as on the parsing one: a caller's raw octets
1276+
# are no more trustworthy than a file's.
12711277
self._check_block_floor(length)
12721278

12731279
if self._ctx is not None:
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
# -*- coding: utf-8 -*-
2+
"""A PCAP-NG block handed over as raw bytes parses and rebuilds.
3+
4+
This guards against GitHub issue #2354. ``block`` is typed to accept
5+
:obj:`bytes`, and the header schema writes such a block back verbatim past the
6+
Block Type -- but the schema the maker returned then held octets where the
7+
reader expected fields, so a Decryption Secrets Block built that way raised
8+
``AttributeError: 'bytes' object has no attribute 'length'``, and so did every
9+
other block type.
10+
11+
The octets are parsed off the buffer the construction path has just packed, the
12+
way the short-block path parses the body it kept: the block that comes back is
13+
the one the octets describe, and it rebuilds byte for byte. Octets that are no
14+
block at all are refused with the library's own errors rather than an
15+
:exc:`AttributeError`.
16+
17+
Every case builds its own octets in memory and reads no capture. :mod:`pcapkit`
18+
is imported inside each test, after
19+
:func:`~tests._support.reimport_once_per_class`.
20+
21+
"""
22+
23+
import unittest
24+
import warnings
25+
from typing import TYPE_CHECKING
26+
27+
from tests._support import reimport_once_per_class
28+
29+
if TYPE_CHECKING:
30+
from typing import Any
31+
32+
#: Test tier (#1958): a hand-picked round trip, one case per block type.
33+
__tier__ = 'ALL'
34+
35+
#: A TLS key log, four-octet aligned so the Decryption Secrets Block needs no
36+
#: padding.
37+
TLS_LOG = b'CLIENT_RANDOM 00 11\n'
38+
#: ``TLSK``, the Secrets Type of a TLS key log, as the field carries it.
39+
TLS_KEY_LOG = 0x544C534B
40+
41+
#: Keywords that build one block of each type, for the round trip below. A
42+
#: Packet Block and an Enhanced Packet Block both need a timestamp, their makers
43+
#: reading the parsed data model for a resolution without one.
44+
CASES = {
45+
'IDB': ('Interface_Description_Block', {'linktype': 1, 'snaplen': 0xFFFF}),
46+
'EPB': ('Enhanced_Packet_Block', {'packet_data': b'abcd', 'timestamp': 0}),
47+
'PB': ('Packet_Block', {'packet_data': b'abcd', 'timestamp': 0}),
48+
'SPB': ('Simple_Packet_Block', {'packet_data': b'abcde'}),
49+
'ISB': ('Interface_Statistics_Block', {'interface_id': 0, 'timestamp': 0}),
50+
'systemd': ('systemd_Journal_Export_Block', {'entries': b'MESSAGE=hi\n'}),
51+
'CB': ('Custom_Block_that_rewriters_can_copy_into_new_files',
52+
{'pen': 1, 'data': b'abcd'}),
53+
'unknown': ('Reserved_0x00000000', {'data': b'abcd'}),
54+
}
55+
56+
57+
def _dsb_body(secrets: 'bytes') -> 'bytes':
58+
"""A little-endian Decryption Secrets Block past its Block Type."""
59+
assert len(secrets) % 4 == 0, len(secrets)
60+
length = 20 + len(secrets)
61+
return b''.join((
62+
length.to_bytes(4, 'little'),
63+
TLS_KEY_LOG.to_bytes(4, 'little'),
64+
len(secrets).to_bytes(4, 'little'),
65+
secrets,
66+
length.to_bytes(4, 'little'),
67+
))
68+
69+
70+
class TestPCAPNGRawBlockBytes(unittest.TestCase):
71+
"""Pin that ``make(block=<bytes>)`` builds a block rather than raising."""
72+
73+
def setUp(self) -> None:
74+
reimport_once_per_class(self)
75+
76+
def _context(self) -> 'Any':
77+
"""A one-section context with one Ethernet interface, as the engine builds it."""
78+
from pcapkit.const.pcapng.block_type import BlockType
79+
from pcapkit.const.reg.linktype import LinkType
80+
from pcapkit.foundation.engines.pcapng import Context
81+
from pcapkit.protocols.misc.pcapng import PCAPNG
82+
83+
section = PCAPNG(num=0, sct=1, ctx=None, type=BlockType.Section_Header_Block, block={})
84+
context = Context(section.info) # type: Any
85+
interface = PCAPNG(num=1, sct=1, ctx=context, type=BlockType.Interface_Description_Block,
86+
block={'linktype': LinkType.ETHERNET, 'snaplen': 0x40000})
87+
context.interfaces.append(interface.info)
88+
return context
89+
90+
def test_a_dsb_handed_its_octets_parses_them(self) -> None:
91+
"""#2354: the issue's own case, a DSB built from raw bytes."""
92+
from pcapkit.const.pcapng.block_type import BlockType
93+
from pcapkit.const.pcapng.secrets_type import SecretsType
94+
from pcapkit.const.pcapng.tls_key_label import TLSKeyLabel
95+
from pcapkit.protocols.misc.pcapng import PCAPNG
96+
97+
body = _dsb_body(TLS_LOG)
98+
block = PCAPNG(num=2, sct=1, ctx=self._context(),
99+
type=BlockType.Decryption_Secrets_Block, block=body)
100+
101+
self.assertEqual(block.info.secrets_type, SecretsType.TLS_Key_Log)
102+
self.assertEqual(block.info.secrets_length, len(TLS_LOG))
103+
self.assertEqual(block.info.secrets_data.data, TLS_LOG.decode())
104+
self.assertIn(TLSKeyLabel.CLIENT_RANDOM, block.info.secrets_data.entries)
105+
# The octets are written back exactly as handed over, after the type.
106+
self.assertEqual(block.data,
107+
int(BlockType.Decryption_Secrets_Block).to_bytes(4, 'little') + body)
108+
109+
def test_every_block_type_handed_its_octets_round_trips(self) -> None:
110+
"""#2354: raw octets parse as the block they describe, and rebuild byte for byte."""
111+
from pcapkit.const.pcapng.block_type import BlockType
112+
from pcapkit.protocols.misc.pcapng import PCAPNG
113+
114+
for label, (name, keywords) in CASES.items():
115+
with self.subTest(block=label):
116+
type_ = getattr(BlockType, name)
117+
context = self._context()
118+
with warnings.catch_warnings():
119+
# a Packet Block warns that the block is obsolete
120+
warnings.simplefilter('ignore')
121+
built = PCAPNG(num=2, sct=1, ctx=context, type=type_, block=keywords)
122+
again = PCAPNG(num=2, sct=1, ctx=context, type=type_, block=built.data[4:])
123+
124+
self.assertEqual(again.data, built.data)
125+
self.assertEqual(str(again.info), str(built.info))
126+
127+
def test_octets_that_are_no_block_are_refused(self) -> None:
128+
"""#2354: a short or impossible block raises the library's own error."""
129+
from pcapkit.const.pcapng.block_type import BlockType
130+
from pcapkit.protocols.misc.pcapng import PCAPNG
131+
from pcapkit.utilities.exceptions import ProtocolError, StreamEOFError
132+
133+
context = self._context()
134+
# Shorter than the twelve octets a block takes, the Block Type counted.
135+
for body in (b'', b'raw', b'\x00' * 7):
136+
with self.subTest(body=body):
137+
with self.assertRaisesRegex(StreamEOFError, r'fewer than the 12 a block needs'):
138+
PCAPNG(num=2, sct=1, ctx=context,
139+
type=BlockType.Decryption_Secrets_Block, block=body)
140+
141+
# Long enough, but declaring a Block Total Length of zero.
142+
for body in (b'\x00' * 8, b'\x00' * 12):
143+
with self.subTest(body=body):
144+
with warnings.catch_warnings():
145+
warnings.simplefilter('ignore')
146+
with self.assertRaisesRegex(ProtocolError, r'invalid length: 0'):
147+
PCAPNG(num=2, sct=1, ctx=context,
148+
type=BlockType.Decryption_Secrets_Block, block=body)
149+
150+
151+
if __name__ == '__main__':
152+
unittest.main()

0 commit comments

Comments
 (0)