Skip to content

Commit 7ad0efb

Browse files
authored
fix(pcapng): handle make(block=<bytes>) for a DSB (#2354) (#2370)
`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 that got as far as `read`; a Section Header Block with no section context failed sooner, on a missing `_byte`. `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. A Section Header Block handed raw octets with no section context -- how an SHB is normally built, since it starts its own section -- still fails on that missing `_byte` and is left for a follow-up. 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 the module lists; and the short and length-zero cases.
1 parent 8b50c1c commit 7ad0efb

2 files changed

Lines changed: 180 additions & 11 deletions

File tree

‎pcapkit/protocols/misc/pcapng.py‎

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

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

12711277
if self._ctx is not None:
@@ -1636,12 +1642,15 @@ def _check_block_floor(self, length: 'Optional[int]') -> 'None':
16361642
passes that check and is caught here instead.
16371643
16381644
Note:
1639-
This runs on the parsing path only, since :meth:`unpack` reaches it
1640-
only when no schema has been built yet and the construction path
1641-
builds one from :meth:`make` while packing its own buffer. That is
1642-
what it should do: a constructed block short enough to trip this floor
1643-
is already reported by :meth:`read`'s own Block Total Length check,
1644-
which names the length and so says more than an end-of-stream would.
1645+
This runs on both of :meth:`unpack`'s paths. On the parsing path the
1646+
tail is what the file has left to read; on the construction path it is
1647+
the octets of a block handed to :meth:`make` as raw :obj:`bytes`, which
1648+
:meth:`unpack` parses off the buffer :meth:`pack` has just filled
1649+
(:issue:`2354`) -- a caller's octets are no more trustworthy than a
1650+
file's. Octets too short for this floor are therefore refused here
1651+
rather than by :meth:`read`'s own Block Total Length check, which sees
1652+
only what gets past the floor and so names an impossible *declared*
1653+
length, never a tail too short to declare one.
16451654
16461655
Measuring an undeclared length needs the stream to seek, and nothing
16471656
guards that here because :meth:`__post_init__` has already called
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
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 that got as far as the reader.
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+
A Section Header Block handed raw octets with no section context is the one
18+
case left out, and it is not pinned here: it never reaches the reader, raising
19+
``AttributeError: 'PCAPNG' object has no attribute '_byte'`` out of the packer
20+
instead, since the byte order a section header carries is the one its own maker
21+
chose and no maker runs for raw octets. That is tracked separately.
22+
23+
Every case builds its own octets in memory and reads no capture. :mod:`pcapkit`
24+
is imported inside each test, after
25+
:func:`~tests._support.reimport_once_per_class`.
26+
27+
"""
28+
29+
import unittest
30+
import warnings
31+
from typing import TYPE_CHECKING
32+
33+
from tests._support import reimport_once_per_class
34+
35+
if TYPE_CHECKING:
36+
from typing import Any
37+
38+
#: Test tier (#1958): a hand-picked round trip, one case per block type.
39+
__tier__ = 'ALL'
40+
41+
#: A TLS key log, four-octet aligned so the Decryption Secrets Block needs no
42+
#: padding.
43+
TLS_LOG = b'CLIENT_RANDOM 00 11\n'
44+
#: ``TLSK``, the Secrets Type of a TLS key log, as the field carries it.
45+
TLS_KEY_LOG = 0x544C534B
46+
47+
#: Keywords that build one block for each of the types below, for the round trip
48+
#: that follows. A Section Header Block is not among them, for the reason the
49+
#: module docstring gives. A Packet Block and an Enhanced Packet Block both need
50+
#: a timestamp, their makers reading the parsed data model for a resolution
51+
#: without one.
52+
CASES = {
53+
'IDB': ('Interface_Description_Block', {'linktype': 1, 'snaplen': 0xFFFF}),
54+
'EPB': ('Enhanced_Packet_Block', {'packet_data': b'abcd', 'timestamp': 0}),
55+
'PB': ('Packet_Block', {'packet_data': b'abcd', 'timestamp': 0}),
56+
'SPB': ('Simple_Packet_Block', {'packet_data': b'abcde'}),
57+
'ISB': ('Interface_Statistics_Block', {'interface_id': 0, 'timestamp': 0}),
58+
'systemd': ('systemd_Journal_Export_Block', {'entries': b'MESSAGE=hi\n'}),
59+
'CB': ('Custom_Block_that_rewriters_can_copy_into_new_files',
60+
{'pen': 1, 'data': b'abcd'}),
61+
'unknown': ('Reserved_0x00000000', {'data': b'abcd'}),
62+
}
63+
64+
65+
def _dsb_body(secrets: 'bytes') -> 'bytes':
66+
"""A little-endian Decryption Secrets Block past its Block Type."""
67+
assert len(secrets) % 4 == 0, len(secrets)
68+
length = 20 + len(secrets)
69+
return b''.join((
70+
length.to_bytes(4, 'little'),
71+
TLS_KEY_LOG.to_bytes(4, 'little'),
72+
len(secrets).to_bytes(4, 'little'),
73+
secrets,
74+
length.to_bytes(4, 'little'),
75+
))
76+
77+
78+
class TestPCAPNGRawBlockBytes(unittest.TestCase):
79+
"""Pin that ``make(block=<bytes>)`` builds a block rather than raising."""
80+
81+
def setUp(self) -> None:
82+
reimport_once_per_class(self)
83+
84+
def _context(self) -> 'Any':
85+
"""A one-section context with one Ethernet interface, as the engine builds it."""
86+
from pcapkit.const.pcapng.block_type import BlockType
87+
from pcapkit.const.reg.linktype import LinkType
88+
from pcapkit.foundation.engines.pcapng import Context
89+
from pcapkit.protocols.misc.pcapng import PCAPNG
90+
91+
section = PCAPNG(num=0, sct=1, ctx=None, type=BlockType.Section_Header_Block, block={})
92+
context = Context(section.info) # type: Any
93+
interface = PCAPNG(num=1, sct=1, ctx=context, type=BlockType.Interface_Description_Block,
94+
block={'linktype': LinkType.ETHERNET, 'snaplen': 0x40000})
95+
context.interfaces.append(interface.info)
96+
return context
97+
98+
def test_a_dsb_handed_its_octets_parses_them(self) -> None:
99+
"""#2354: the issue's own case, a DSB built from raw bytes."""
100+
from pcapkit.const.pcapng.block_type import BlockType
101+
from pcapkit.const.pcapng.secrets_type import SecretsType
102+
from pcapkit.const.pcapng.tls_key_label import TLSKeyLabel
103+
from pcapkit.protocols.misc.pcapng import PCAPNG
104+
105+
body = _dsb_body(TLS_LOG)
106+
block = PCAPNG(num=2, sct=1, ctx=self._context(),
107+
type=BlockType.Decryption_Secrets_Block, block=body)
108+
109+
self.assertEqual(block.info.secrets_type, SecretsType.TLS_Key_Log)
110+
self.assertEqual(block.info.secrets_length, len(TLS_LOG))
111+
self.assertEqual(block.info.secrets_data.data, TLS_LOG.decode())
112+
self.assertIn(TLSKeyLabel.CLIENT_RANDOM, block.info.secrets_data.entries)
113+
# The octets are written back exactly as handed over, after the type.
114+
self.assertEqual(block.data,
115+
int(BlockType.Decryption_Secrets_Block).to_bytes(4, 'little') + body)
116+
117+
def test_every_block_type_handed_its_octets_round_trips(self) -> None:
118+
"""#2354: raw octets parse as the block they describe, and rebuild byte for byte."""
119+
from pcapkit.const.pcapng.block_type import BlockType
120+
from pcapkit.protocols.misc.pcapng import PCAPNG
121+
122+
for label, (name, keywords) in CASES.items():
123+
with self.subTest(block=label):
124+
type_ = getattr(BlockType, name)
125+
context = self._context()
126+
with warnings.catch_warnings():
127+
# a Packet Block warns that the block is obsolete
128+
warnings.simplefilter('ignore')
129+
built = PCAPNG(num=2, sct=1, ctx=context, type=type_, block=keywords)
130+
again = PCAPNG(num=2, sct=1, ctx=context, type=type_, block=built.data[4:])
131+
132+
self.assertEqual(again.data, built.data)
133+
self.assertEqual(str(again.info), str(built.info))
134+
135+
def test_octets_that_are_no_block_are_refused(self) -> None:
136+
"""#2354: a short or impossible block raises the library's own error."""
137+
from pcapkit.const.pcapng.block_type import BlockType
138+
from pcapkit.protocols.misc.pcapng import PCAPNG
139+
from pcapkit.utilities.exceptions import ProtocolError, StreamEOFError
140+
141+
context = self._context()
142+
# Shorter than the twelve octets a block takes, the Block Type counted.
143+
for body in (b'', b'raw', b'\x00' * 7):
144+
with self.subTest(body=body):
145+
with self.assertRaisesRegex(StreamEOFError, r'fewer than the 12 a block needs'):
146+
PCAPNG(num=2, sct=1, ctx=context,
147+
type=BlockType.Decryption_Secrets_Block, block=body)
148+
149+
# Long enough, but declaring a Block Total Length of zero.
150+
for body in (b'\x00' * 8, b'\x00' * 12):
151+
with self.subTest(body=body):
152+
with warnings.catch_warnings():
153+
warnings.simplefilter('ignore')
154+
with self.assertRaisesRegex(ProtocolError, r'invalid length: 0'):
155+
PCAPNG(num=2, sct=1, ctx=context,
156+
type=BlockType.Decryption_Secrets_Block, block=body)
157+
158+
159+
if __name__ == '__main__':
160+
unittest.main()

0 commit comments

Comments
 (0)