Skip to content

Commit ea9935d

Browse files
committed
refactor(protocols)!: refuse fixed-size octets and timestamp counts the makers masked (#2676)
- _make_octets: a caller's octets for a fixed-size field raise ProtocolError at any other width instead of being padded or cut -- the IPv4, MH, HIP, ICMP, IGMP, IPX and OSPF checksums, the CALIPSO checksum, MH cookies and keygen tokens, HIP opaque and payload data, PCAP-NG ZigBee keys, IPX addresses, and the reserved, unused and zero octets of HOPOPT, IPv6-Opts, IPv6-Frag, IPv6-Route, MH, HIP, ICMP, IGMPv3, OSPF and IP_Pseudo. - _fill_short_read: a model's reserved octets are zero-filled before the check only where its short read ends at or before them, for the rebuild to cut back; any other model's are checked as a caller's are. - IPv4 TS: a slot count past nine (list) or four (dict) raises instead of being cut; the default is derived from the form. - Empty defaults of fixed-size fields become zero octets of their width. - protocol.rst documents the shared value helpers and how makers use them. - MH ipv4_dhcp.mode=True is unchanged: S is one bit, which takes a bool. - Tests: a source sweep finds every fixed-size octet field a maker writes; every refusal fails on main; cut captures rebuild as captured; the widest values build and round-trip.
1 parent 57a6c36 commit ea9935d

21 files changed

Lines changed: 1371 additions & 277 deletions

File tree

‎docs/source/pcapkit/protocols/protocol.rst‎

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,23 @@ Internal Definitions below.
8383
.. automethod:: _make_payload
8484
.. automethod:: _packing_target
8585

86+
.. automethod:: _read_unpack
87+
.. automethod:: _read_octets
88+
.. automethod:: _read_uint
89+
.. automethod:: _read_sint
90+
.. automethod:: _read_vector
91+
.. automethod:: _read_whole_vector
92+
.. automethod:: _read_uints
93+
94+
.. automethod:: _make_pack
95+
.. automethod:: _make_octets
96+
.. automethod:: _fill_short_read
97+
.. automethod:: _make_uint
98+
.. automethod:: _make_sint
99+
.. automethod:: _uint_size
100+
.. automethod:: _make_vector
101+
.. automethod:: _make_uints
102+
86103
.. automethod:: _read_mac_addr
87104
.. automethod:: _make_mac_addr
88105

@@ -138,6 +155,29 @@ The ``pyshark``, ``scapy`` and ``pypcapfile`` engines dissect with their own
138155
parsers, which do not apply the frame's layer bound (:issue:`1609`). The
139156
``dpkt`` toolkit reads what |dpkt|_ decodes, which applies neither bound.
140157

158+
Value Helpers
159+
-------------
160+
161+
A protocol reads and makes its field values through the helpers
162+
:class:`~pcapkit.protocols.protocol.Protocol` carries, rather than through
163+
copies of its own (:issue:`2614`). A maker checks a caller's values through them
164+
before packing, and raises :exc:`~pcapkit.utilities.exceptions.ProtocolError` for
165+
one its field cannot carry rather than masking it (:issue:`2617`):
166+
167+
* an unsigned integer through :meth:`~Protocol._make_uint`, which takes a
168+
:obj:`bool` for a one-bit field only;
169+
* a two's-complement integer through :meth:`~Protocol._make_sint`, the inverse of
170+
:meth:`~Protocol._read_sint`;
171+
* the octets of a fixed-size field through :meth:`~Protocol._make_octets`, which
172+
refuses any other width rather than padding or cutting it (:issue:`2676`). A
173+
value taken from a model of a cut capture goes through
174+
:meth:`~Protocol._fill_short_read` first, which zero-fills just the octets the
175+
cut left out, for the rebuild to cut back.
176+
177+
:meth:`~Protocol._uint_size` counts the octets a variable-width integer needs,
178+
and :func:`~pcapkit.corekit.fields.strings.pad_length` the padding that takes a
179+
length to an alignment boundary.
180+
141181
Data Models
142182
-----------
143183

@@ -201,6 +241,21 @@ Internal Definitions
201241
_make_index
202242
_make_payload
203243
_packing_target
244+
_read_unpack
245+
_read_octets
246+
_read_uint
247+
_read_sint
248+
_read_vector
249+
_read_whole_vector
250+
_read_uints
251+
_make_pack
252+
_make_octets
253+
_fill_short_read
254+
_make_uint
255+
_make_sint
256+
_uint_size
257+
_make_vector
258+
_make_uints
204259
_read_mac_addr
205260
_make_mac_addr
206261
_lookup_registry
@@ -228,7 +283,15 @@ Internal Definitions
228283
:meth:`~Protocol.expand_comp`, :meth:`~Protocol._read_packet`,
229284
:meth:`~Protocol._get_payload`, :meth:`~Protocol._make_data`,
230285
:meth:`~Protocol._make_index`, :meth:`~Protocol._make_payload`,
231-
:meth:`~Protocol._packing_target`, :meth:`~Protocol._read_mac_addr`,
286+
:meth:`~Protocol._packing_target`, :meth:`~Protocol._read_unpack`,
287+
:meth:`~Protocol._read_octets`, :meth:`~Protocol._read_uint`,
288+
:meth:`~Protocol._read_sint`, :meth:`~Protocol._read_vector`,
289+
:meth:`~Protocol._read_whole_vector`, :meth:`~Protocol._read_uints`,
290+
:meth:`~Protocol._make_pack`, :meth:`~Protocol._make_octets`,
291+
:meth:`~Protocol._fill_short_read`,
292+
:meth:`~Protocol._make_uint`, :meth:`~Protocol._make_sint`,
293+
:meth:`~Protocol._uint_size`, :meth:`~Protocol._make_vector`,
294+
:meth:`~Protocol._make_uints`, :meth:`~Protocol._read_mac_addr`,
232295
:meth:`~Protocol._make_mac_addr`,
233296
:meth:`~Protocol._lookup_registry`, :meth:`~Protocol._lookup_next_layer`,
234297
:meth:`~Protocol._decode_next_layer`,

‎pcapkit/protocols/application/ospf.py‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,11 @@
6565
verified on read under :data:`~pcapkit.utilities.config.CHECKSUM_VERIFY`
6666
(:issue:`2475`).
6767
68+
:meth:`~pcapkit.protocols.application.ospf.OSPF.make` raises
69+
:exc:`~pcapkit.utilities.exceptions.ProtocolError` for a checksum, or the
70+
reserved octets of cryptographic authentication data, that is not two
71+
octets, rather than padding or cutting it (:issue:`2676`).
72+
6873
"""
6974
import ipaddress
7075
from typing import TYPE_CHECKING, cast
@@ -298,9 +303,11 @@ def make(self,
298303
299304
Raises:
300305
ProtocolError: If ``auth_data`` is of an invalid type, is
301-
:obj:`bytes` that are not exactly 8 octets long, or is a
302-
:obj:`dict` missing a required key; or if ``packet_length``
303-
is not an :obj:`int` in ``0..65535``.
306+
:obj:`bytes` that are not exactly 8 octets long, is a
307+
:obj:`dict` missing a required key, or carries ``reserved``
308+
octets that are not two; or if ``packet_length`` is not an
309+
:obj:`int` in ``0..65535``, or ``checksum`` is not two
310+
octets.
304311
305312
.. versionchanged:: 1.5.0
306313
@@ -336,7 +343,8 @@ def make(self,
336343
length=packet_length,
337344
router_id=router_id,
338345
area_id=area_id,
339-
checksum=b'\x00\x00' if checksum is None else checksum,
346+
checksum=(b'\x00\x00' if checksum is None
347+
else self._make_octets(checksum, size=2, name='checksum', alias='OSPF')),
340348
auth_type=auth_type_, # type: ignore[arg-type]
341349
auth_data=data,
342350
payload=payload,
@@ -386,6 +394,13 @@ def _make_data(cls, data: 'Data_OSPF') -> 'dict[str, Any]': # type: ignore[over
386394
Key-value pairs for protocol construction.
387395
388396
"""
397+
auth = data.auth # type: Any
398+
if isinstance(auth, Data_CryptographicAuthentication):
399+
# NOTE: Reserved octets a capture cut inside are zero-filled (#1458).
400+
reserved = getattr(auth, 'reserved', b'\x00\x00')
401+
filled = cls._fill_short_read(reserved, size=2, model=data, field='auth_data')
402+
if len(filled) != len(reserved):
403+
auth = {'reserved': filled, 'key_id': auth.key_id, 'len': auth.len, 'seq': auth.seq}
389404
return {
390405
'version': data.version,
391406
'type': data.type,
@@ -394,7 +409,7 @@ def _make_data(cls, data: 'Data_OSPF') -> 'dict[str, Any]': # type: ignore[over
394409
'area_id': data.area_id,
395410
'checksum': data.chksum,
396411
'auth_type': data.autype,
397-
'auth_data': data.auth,
412+
'auth_data': auth,
398413
'payload': cls._make_payload(data),
399414
# NOTE: And the octets captured past the Packet Length (#1455).
400415
'trailer': data.get('trailer', b''),
@@ -492,8 +507,9 @@ def _make_encrypt_auth(self,
492507
493508
Raises:
494509
ProtocolError: If ``auth_data`` is of an invalid type, is
495-
:obj:`bytes` that are not exactly 8 octets long, or is a
496-
:obj:`dict` missing a required key.
510+
:obj:`bytes` that are not exactly 8 octets long, is a
511+
:obj:`dict` missing a required key, or carries ``reserved``
512+
octets that are not two.
497513
498514
"""
499515
if isinstance(auth_data, Schema_CryptographicAuthentication):
@@ -504,7 +520,8 @@ def _make_encrypt_auth(self,
504520
return Schema_CryptographicAuthentication.unpack(auth_data) # type: ignore[call-arg,misc]
505521
if isinstance(auth_data, Data_CryptographicAuthentication):
506522
return Schema_CryptographicAuthentication(
507-
reserved=getattr(auth_data, 'reserved', b'\x00\x00'),
523+
reserved=self._make_octets(getattr(auth_data, 'reserved', b'\x00\x00'), size=2,
524+
name='reserved octets', alias='OSPF'),
508525
key_id=auth_data.key_id,
509526
len=auth_data.len,
510527
seq=auth_data.seq,
@@ -514,7 +531,8 @@ def _make_encrypt_auth(self,
514531
if isinstance(auth_data, dict):
515532
try:
516533
return Schema_CryptographicAuthentication(
517-
reserved=auth_data.get('reserved', b'\x00\x00'),
534+
reserved=self._make_octets(auth_data.get('reserved', b'\x00\x00'), size=2,
535+
name='reserved octets', alias='OSPF'),
518536
key_id=auth_data['key_id'],
519537
len=auth_data['len'],
520538
seq=auth_data['seq'],

0 commit comments

Comments
 (0)