Skip to content

Commit d57c0a4

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 596dcfa commit d57c0a4

21 files changed

Lines changed: 1372 additions & 292 deletions

File tree

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

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

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

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

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

@@ -205,6 +245,21 @@ Internal Definitions
205245
_make_index
206246
_make_payload
207247
_packing_target
248+
_read_unpack
249+
_read_octets
250+
_read_uint
251+
_read_sint
252+
_read_vector
253+
_read_whole_vector
254+
_read_uints
255+
_make_pack
256+
_make_octets
257+
_fill_short_read
258+
_make_uint
259+
_make_sint
260+
_uint_size
261+
_make_vector
262+
_make_uints
208263
_read_mac_addr
209264
_make_mac_addr
210265
_lookup_registry
@@ -233,7 +288,15 @@ Internal Definitions
233288
:meth:`~Protocol._read_packet`, :meth:`~Protocol._get_payload`,
234289
:meth:`~Protocol._make_data`, :meth:`~Protocol._make_uint`,
235290
:meth:`~Protocol._make_index`, :meth:`~Protocol._make_payload`,
236-
:meth:`~Protocol._packing_target`, :meth:`~Protocol._read_mac_addr`,
291+
:meth:`~Protocol._packing_target`, :meth:`~Protocol._read_unpack`,
292+
:meth:`~Protocol._read_octets`, :meth:`~Protocol._read_uint`,
293+
:meth:`~Protocol._read_sint`, :meth:`~Protocol._read_vector`,
294+
:meth:`~Protocol._read_whole_vector`, :meth:`~Protocol._read_uints`,
295+
:meth:`~Protocol._make_pack`, :meth:`~Protocol._make_octets`,
296+
:meth:`~Protocol._fill_short_read`,
297+
:meth:`~Protocol._make_uint`, :meth:`~Protocol._make_sint`,
298+
:meth:`~Protocol._uint_size`, :meth:`~Protocol._make_vector`,
299+
:meth:`~Protocol._make_uints`, :meth:`~Protocol._read_mac_addr`,
237300
:meth:`~Protocol._make_mac_addr`,
238301
:meth:`~Protocol._lookup_registry`, :meth:`~Protocol._lookup_next_layer`,
239302
:meth:`~Protocol._decode_next_layer`,

‎pcapkit/protocols/application/ospf.py‎

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,13 @@
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
"""
74+
import collections.abc
6975
import ipaddress
7076
from typing import TYPE_CHECKING, cast
7177

@@ -301,11 +307,12 @@ def make(self,
301307
302308
Raises:
303309
ProtocolError: If ``auth_data`` is of an invalid type, is
304-
:obj:`bytes` that are not exactly 8 octets long, or is a
305-
:obj:`dict` missing a required key; if ``packet_length``
306-
is not an :obj:`int` in ``0..65535``; or if ``type`` (8 bits)
307-
or ``auth_type`` (16 bits) is a name, a :obj:`bool`, or a
308-
member or code its field cannot carry.
310+
:obj:`bytes` that are not exactly 8 octets long, is a
311+
:obj:`dict` missing a required key, or carries ``reserved``
312+
octets that are not two; if ``packet_length`` is not an
313+
:obj:`int` in ``0..65535``; if ``checksum`` is not two octets;
314+
or if ``type`` (8 bits) or ``auth_type`` (16 bits) is a name, a
315+
:obj:`bool`, or a member or code its field cannot carry.
309316
310317
.. versionchanged:: 1.5.0
311318
@@ -341,7 +348,8 @@ def make(self,
341348
length=packet_length,
342349
router_id=router_id,
343350
area_id=area_id,
344-
checksum=b'\x00\x00' if checksum is None else checksum,
351+
checksum=(b'\x00\x00' if checksum is None
352+
else self._make_octets(checksum, size=2, name='checksum', alias='OSPF')),
345353
auth_type=auth_type_,
346354
auth_data=data,
347355
payload=payload,
@@ -391,6 +399,14 @@ def _make_data(cls, data: 'Data_OSPF') -> 'dict[str, Any]': # type: ignore[over
391399
Key-value pairs for protocol construction.
392400
393401
"""
402+
auth = data.auth # type: Any
403+
# NOTE: Reserved octets a capture cut inside are zero-filled (#1458), in
404+
# the data model or the dict form ``to_dict()`` gives of it.
405+
if isinstance(auth, collections.abc.Mapping):
406+
reserved = auth.get('reserved', b'\x00\x00')
407+
filled = cls._fill_short_read(reserved, size=2, model=data, field='auth_data')
408+
if filled is not reserved:
409+
auth = {**auth, 'reserved': filled}
394410
return {
395411
'version': data.version,
396412
'type': data.type,
@@ -399,7 +415,7 @@ def _make_data(cls, data: 'Data_OSPF') -> 'dict[str, Any]': # type: ignore[over
399415
'area_id': data.area_id,
400416
'checksum': data.chksum,
401417
'auth_type': data.autype,
402-
'auth_data': data.auth,
418+
'auth_data': auth,
403419
'payload': cls._make_payload(data),
404420
# NOTE: And the octets captured past the Packet Length (#1455).
405421
'trailer': data.get('trailer', b''),
@@ -497,8 +513,9 @@ def _make_encrypt_auth(self,
497513
498514
Raises:
499515
ProtocolError: If ``auth_data`` is of an invalid type, is
500-
:obj:`bytes` that are not exactly 8 octets long, or is a
501-
:obj:`dict` missing a required key.
516+
:obj:`bytes` that are not exactly 8 octets long, is a
517+
:obj:`dict` missing a required key, or carries ``reserved``
518+
octets that are not two.
502519
503520
"""
504521
if isinstance(auth_data, Schema_CryptographicAuthentication):
@@ -509,7 +526,8 @@ def _make_encrypt_auth(self,
509526
return Schema_CryptographicAuthentication.unpack(auth_data) # type: ignore[call-arg,misc]
510527
if isinstance(auth_data, Data_CryptographicAuthentication):
511528
return Schema_CryptographicAuthentication(
512-
reserved=getattr(auth_data, 'reserved', b'\x00\x00'),
529+
reserved=self._make_octets(getattr(auth_data, 'reserved', b'\x00\x00'), size=2,
530+
name='reserved octets', alias='OSPF'),
513531
key_id=auth_data.key_id,
514532
len=auth_data.len,
515533
seq=auth_data.seq,
@@ -519,7 +537,8 @@ def _make_encrypt_auth(self,
519537
if isinstance(auth_data, dict):
520538
try:
521539
return Schema_CryptographicAuthentication(
522-
reserved=auth_data.get('reserved', b'\x00\x00'),
540+
reserved=self._make_octets(auth_data.get('reserved', b'\x00\x00'), size=2,
541+
name='reserved octets', alias='OSPF'),
523542
key_id=auth_data['key_id'],
524543
len=auth_data['len'],
525544
seq=auth_data['seq'],

0 commit comments

Comments
 (0)