fix(tcp): parse all five MP_CAPABLE lengths of RFC 8684 (#1042)

CHANGELOG.md

@@ -153,6 +153,7 @@ Layer-limited extraction: `layer='link'` no longer sets the termination flag on
 - `TCP._make_mptcp_addaddr` could not build an `ADD_ADDR` option end to end: its `kind=`/`length=` arguments were rejected with `UnknownFieldWarning` and dropped, and `.pack()` then raised `KeyError: 'length'` from `port`'s own condition, `pkt['length'] in (10, 22)`. The cause was one layer up: `MPTCP`, the base of every Multipath TCP subtype schema, declared `kind` and `length` only under `typing.TYPE_CHECKING` rather than as real fields, unlike `Option`. That dropped `kind=`/`length=` for every `_make_mptcp_*` constructor, so `MPTCP` now declares both for real ([#541](https://github.com/JarryShaw/PyPCAPKit/issues/541)). The missing fields broke parsing too: a subtype schema's leading field read the `kind` octet itself, an off-by-two in field alignment rather than a wire-format change (a correct sender's octets were always right). Spec-correct `ADD_ADDR` and `MP_PRIO` options failed with `FieldError: TCP: [OptNo 30] 3 invalid IP version` and `KeyError: 'length'`; both parse now.
 - two dropped-keyword/wrong-cast defects flagged in review during this release and never filed until now: HIP's `_make_param_encrypted` passed `cipher=` to a schema with no such field, so an AES-cipher `ENCRYPTED` parameter built through `make` packed without its IV; and IPv6-Route's `RPL.post_process`, which runs on every `Schema.pack`, assumed `self.addresses` was still the concatenated `bytes` a parse leaves, and raised slicing the `list[bytes]` a `make`-built multi-address header holds ([#556](https://github.com/JarryShaw/PyPCAPKit/issues/556)).
 - two more defects [#541](https://github.com/JarryShaw/PyPCAPKit/issues/541) exposed by letting construction reach code that had never run. `MPTCP.subtype` was still `typing.TYPE_CHECKING`-only, so `TCP(options=[(Enum_Option.Multipath_TCP, ...)])` raised `AttributeError: ... has no attribute 'subtype'` for every subtype but `MP_JOIN`: the convenience constructor reads the schema straight back through `_read_mptcp_*` with no byte round trip, so `_MPTCP.post_process`, the only code that set `subtype`, never ran. Fixed on the construction path (`TCP._make_mode_mp`) rather than by adding a third real field the way `kind`/`length` got in [#541](https://github.com/JarryShaw/PyPCAPKit/issues/541): `subtype` is already packed as 4 bits of each subtype's own `test` bitfield, and a second field for the same bits would double-encode them or need a "derive, don't pack" field kind this library lacks ([#566](https://github.com/JarryShaw/PyPCAPKit/issues/566)). Separately, `_make_mptcp_capable` wrote `length=20 if rkey is None else 32` where [RFC 8684](https://datatracker.ietf.org/doc/html/rfc8684) section 3.1 gives 12 and 20, and `MPTCPCapable.rkey`'s condition (`pkt['length'] != 32`) dropped the receiver's key for exactly the length the maker used for "key present": a key-absent MP_CAPABLE could not be built, and a key-present one lost its key on the wire. Both, and the guard in `_read_mptcp_capable`, now agree on 12/20. **This changes MP_CAPABLE's packed output**: a 20-octet key-present option built or parsed under the old code becomes 12 octets with no key, or 20 octets with the key present, depending on which the caller meant ([#567](https://github.com/JarryShaw/PyPCAPKit/issues/567)).
+- TCP's MP_CAPABLE now parses and builds all five lengths of [RFC 8684](https://datatracker.ietf.org/doc/html/rfc8684) section 3.1 -- 4 (SYN, no key), 12, 20, 22 (adds the Data-Level Length) and 24 (adds the Checksum) -- where `_read_mptcp_capable` raised `ProtocolError: TCP: [OptNo 30] invalid format` for anything but 12 and 20. In the schema `skey` is now present only for `length > 4` and `rkey` for `length > 12`, and two fields are new: `dl_len` (16 bits, `length > 20`) and `checksum` (2 octets, `length > 22`). `_make_mptcp_capable` takes `dl_len` and `checksum` too, where it used to drop them silently through `**kwargs`, and picks the length from how many of `skey`, `rkey`, `dl_len` and `checksum` are not `None`. A field given without the one before it raises `ProtocolError`. The data model `MPTCPCapable` gains `dl_len` and `checksum`, and `skey` becomes `Optional[int]`: code that constructs it directly must now pass both new fields, and `to_dict()` carries two more keys ([#1042](https://github.com/JarryShaw/PyPCAPKit/issues/1042)).
 - IPv6-Route's RPL routing data was five octets wide at the front where [RFC 6554](https://datatracker.ietf.org/doc/html/rfc6554) section 3 gives four, with three further defects behind it. `CmprI`, `CmprE` and `Pad` are each 4 bits sharing a 32-bit word with a 20-bit `Reserved`, but the schema declared `cmpr_i` and `cmpr_e` as whole octets, so a constructed two-address header packed to 41 octets while its `Hdr Ext Len` of 5 declared 48. Correcting the width is a **wire-format change** and reshapes the schema: `RPL(cmpr_i=..., cmpr_e=...)` is now `RPL(cmpr={'cmpr_i': ..., 'cmpr_e': ...})`, beside the existing `pad={'pad_len': ...}`. Behind it, the reader's `header.length % 16` guard read `Hdr Ext Len` as an octet count and assumed 16-octet addresses, which an SRH carries only when `CmprI` and `CmprE` are both 0 -- the unit confusion [#487](https://github.com/JarryShaw/PyPCAPKit/issues/487) fixed for Source Route and Type 2, left by [#489](https://github.com/JarryShaw/PyPCAPKit/pull/489) for want of a working RPL round trip to validate against. It is replaced by section 4.2's address-count arithmetic, `n = (((Hdr Ext Len * 8) - Pad - (16 - CmprE)) / (16 - CmprI)) + 1`, which the reader now requires to be non-negative and whole. `RPL.post_process` subtracted `pad_len` a second time from a buffer whose length callback had already taken it off, losing one address per `16 - CmprI` octets of padding, and set `ip` only when it had parsed octets, so `IPv6_Route(type=..., data={'ip': [...]})` raised a bare `AttributeError: 'RPL' object has no attribute 'ip'`. And `_make_data_type_rpl` computed `Pad` as `8 - length % 8` without the outer `% 8`, so an aligned address vector got a full 8 octets of padding where section 3 requires `Pad` to be 0 when `CmprI` and `CmprE` are both 0. The narrower `cmpr_i` also removes a latent divide-by-zero (a whole octet could hold 16, making `16 - CmprI` zero). `ipv6-route-type/RPL_Source_Route_Header` round-trips and its `EXPECTED_FAILURES` entry is deleted; none of this has been checked against a real RPL capture ([#564](https://github.com/JarryShaw/PyPCAPKit/issues/564)).
 - `L2TPv2` parsed any version nibble, so an L2TPv3 datagram was reported as v2 with a tunnel and session ID read out of v3's Control Connection ID. [RFC 2661](https://datatracker.ietf.org/doc/html/rfc2661) §3.1 fixes `Ver` at 2 and reserves 1 for L2F, and `L2TPv2.version` documented that any other value "is a different protocol reached through a different class", but nothing enforced it, so the hard-coded `Literal[2]` property and `info.version` answered 2 and 3 on the same octets. `read` now raises `ProtocolError`, degrading the payload to `Raw` through the existing `beholder` path with the reason recorded. **This affects real captures**: [RFC 3931](https://datatracker.ietf.org/doc/html/rfc3931) §4.1.2 puts L2TPv3 on port 1701 too, the port `UDP.__proto__` binds, so `Ethernet:IPv4:UDP:L2TPv2:Raw` with invented field values becomes `Ethernet:IPv4:UDP:Raw` with the octets preserved.
 

docs/source/changelog/1.5.0.rst

@@ -1383,6 +1383,19 @@ Fixed
   MP_CAPABLE's packed output**: a 20-octet key-present option built or parsed under
   the old code becomes 12 octets with no key, or 20 octets with the key present,
   depending on which the caller meant (:issue:`567`).
+* TCP's MP_CAPABLE now parses and builds all five lengths of :rfc:`8684` section 3.1
+  -- 4 (SYN, no key), 12, 20, 22 (adds the Data-Level Length) and 24 (adds the
+  Checksum) -- where ``_read_mptcp_capable`` raised
+  ``ProtocolError: TCP: [OptNo 30] invalid format`` for anything but 12 and 20. In the schema ``skey`` is now present
+  only for ``length > 4`` and ``rkey`` for ``length > 12``, and two fields are new:
+  ``dl_len`` (16 bits, ``length > 20``) and ``checksum`` (2 octets, ``length > 22``).
+  ``_make_mptcp_capable`` takes ``dl_len`` and ``checksum`` too, where it used to
+  drop them silently through ``**kwargs``, and picks the length from how many of
+  ``skey``, ``rkey``, ``dl_len`` and ``checksum`` are not ``None``. A field given
+  without the one before it raises ``ProtocolError``. The data model
+  ``MPTCPCapable`` gains ``dl_len`` and ``checksum``, and ``skey`` becomes
+  ``Optional[int]``: code that constructs it directly must now pass both new fields,
+  and ``to_dict()`` carries two more keys (:issue:`1042`).
 * IPv6-Route's RPL routing data was five octets wide at the front where :rfc:`6554`
   section 3 gives four, with three further defects behind it. ``CmprI``, ``CmprE`` and
   ``Pad`` are each 4 bits sharing a 32-bit word with a 20-bit ``Reserved``, but the

docs/source/contributing/conventions/process.rst

@@ -101,7 +101,7 @@ commands down instead of a figure that will be stale by the next merge:
 The grouping scheme was settled on :issue:`918`: **a section per top-level module, with**
 ``Added``/``Changed``/``Fixed`` **nested inside each** -- module granularity, not
 per-file and not per-subpackage. The file carries **9** module-level sections holding
-164 entries, and no entry carries an inline kind label::
+165 entries, and no entry carries an inline kind label::
 
    $ grep -cE '^\* \*\*(Added|Changed|Fixed)\*\*' docs/source/changelog/1.5.0.rst
    0

pcapkit/protocols/data/transport/tcp.py

@@ -411,12 +411,16 @@ class MPTCPCapable(MPTCP):
     #: Flags.
     flags: 'MPTCPCapableFlag'
     #: Option sender's key.
-    skey: 'int'
+    skey: 'Optional[int]'
     #: Option receiver's key.
     rkey: 'Optional[int]'
+    #: Data-level length.
+    dl_len: 'Optional[int]'
+    #: Checksum.
+    checksum: 'Optional[bytes]'
 
     if TYPE_CHECKING:
-        def __init__(self, kind: 'OptionNumber', length: 'int', subtype: 'MPTCPOption', version: 'int', flags: 'MPTCPCapableFlag', skey: 'int', rkey: 'Optional[int]') -> 'None': ...  # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin
+        def __init__(self, kind: 'OptionNumber', length: 'int', subtype: 'MPTCPOption', version: 'int', flags: 'MPTCPCapableFlag', skey: 'Optional[int]', rkey: 'Optional[int]', dl_len: 'Optional[int]', checksum: 'Optional[bytes]') -> 'None': ...  # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin
 
 
 class MPTCPJoin(MPTCP):

pcapkit/protocols/schema/transport/tcp.py

@@ -844,19 +844,33 @@ class MPTCPCapable(MPTCP, code=Enum_MPTCPOption.MP_CAPABLE):
         'ext': (1, 1),
         'hsa': (7, 1),
     })
-    #: Option sender's key.
-    skey: 'int' = UInt64Field()
-    #: Option receiver's key.
-    #:
-    #: :rfc:`8684` section 3.1 gives MP_CAPABLE as 12 octets without this key
-    #: and 20 octets with it, so the field is present only for the latter.
+    # NOTE: :rfc:`8684` section 3.1 figure 4 gives MP_CAPABLE five lengths --
+    # 4 (SYN, header only), 12 (sender's key), 20 (both keys), 22 (both keys
+    # and Data-Level Length) and 24 (that plus the Checksum) -- so each field
+    # below is present from the length that first carries it.
+    #: Option sender's key (if option length > 4).
+    skey: 'int' = ConditionalField(
+        UInt64Field(),
+        lambda pkt: pkt['length'] > 4,
+    )
+    #: Option receiver's key (if option length > 12).
     rkey: 'int' = ConditionalField(
         UInt64Field(),
-        lambda pkt: pkt['length'] == 20,
+        lambda pkt: pkt['length'] > 12,
+    )
+    #: Data-level length (if option length > 20).
+    dl_len: 'int' = ConditionalField(
+        UInt16Field(),
+        lambda pkt: pkt['length'] > 20,
+    )
+    #: Checksum (if option length > 22).
+    checksum: 'bytes' = ConditionalField(
+        BytesField(length=2),
+        lambda pkt: pkt['length'] > 22,
     )
 
     if TYPE_CHECKING:
-        def __init__(self, kind: 'Enum_Option', length: 'int', test: 'MPTCPSubtypeCapable', flags: 'MPTCPCapableFlags', skey: 'int', rkey: 'Optional[int]') -> 'None': ...
+        def __init__(self, kind: 'Enum_Option', length: 'int', test: 'MPTCPSubtypeCapable', flags: 'MPTCPCapableFlags', skey: 'Optional[int]', rkey: 'Optional[int]', dl_len: 'Optional[int]', checksum: 'Optional[bytes]') -> 'None': ...
 
 
 class MPTCPJoin(MPTCP, code=Enum_MPTCPOption.MP_JOIN):  # register as a placeholder

pcapkit/protocols/transport/tcp.py

@@ -1500,7 +1500,7 @@ def _read_mptcp_unknown(self, schema: 'Schema_MPTCPUnknown', *, options: 'Option
     def _read_mptcp_capable(self, schema: 'Schema_MPTCPCapable', *, options: 'Option') -> 'Data_MPTCPCapable':  # pylint: disable=unused-argument
         """Read Multipath Capable option.
 
-        Structure of ``MP_CAPABLE`` [:rfc:`6824`]:
+        Structure of ``MP_CAPABLE`` [:rfc:`8684`]:
 
         .. code-block:: text
 
@@ -1510,13 +1510,15 @@ def _read_mptcp_capable(self, schema: 'Schema_MPTCPCapable', *, options: 'Option
            |     Kind      |    Length     |Subtype|Version|A|B|C|D|E|F|G|H|
            +---------------+---------------+-------+-------+---------------+
            |                   Option Sender's Key (64 bits)               |
-           |                                                               |
+           |                      (if option Length > 4)                   |
            |                                                               |
            +---------------------------------------------------------------+
            |                  Option Receiver's Key (64 bits)              |
-           |                     (if option Length == 20)                  |
+           |                      (if option Length > 12)                  |
            |                                                               |
-           +---------------------------------------------------------------+
+           +-------------------------------+-------------------------------+
+           |  Data-Level Length (16 bits)  |  Checksum (16 bits, optional) |
+           +-------------------------------+-------------------------------+
 
         Arguments:
             schema: parsed option schema
@@ -1526,13 +1528,13 @@ def _read_mptcp_capable(self, schema: 'Schema_MPTCPCapable', *, options: 'Option
             Parsed option data.
 
         Raises:
-            ProtocolError: If length is **NOT** ``12`` or ``20``.
+            ProtocolError: If length is **NOT** ``4``, ``12``, ``20``, ``22``
+                or ``24``.
 
         """
-        # NOTE: :rfc:`8684` section 3.1 gives MP_CAPABLE as 12 octets without
-        # the receiver's key and 20 octets with it, hence this guard and the
-        # ``rkey=`` test below.
-        if schema.length not in (12, 20):
+        # NOTE: :rfc:`8684` section 3.1 defines exactly these five lengths;
+        # the schema's field conditions follow from them.
+        if schema.length not in (4, 12, 20, 22, 24):
             raise ProtocolError(f'{self.alias}: [OptNo {schema.kind}] invalid format')
 
         data = Data_MPTCPCapable(
@@ -1545,8 +1547,10 @@ def _read_mptcp_capable(self, schema: 'Schema_MPTCPCapable', *, options: 'Option
                 ext=bool(schema.flags['ext']),
                 hsa=bool(schema.flags['hsa']),
             ),
-            skey=schema.skey,
-            rkey=schema.rkey if schema.length == 20 else None,
+            skey=schema.skey if schema.length > 4 else None,
+            rkey=schema.rkey if schema.length > 12 else None,
+            dl_len=schema.dl_len if schema.length > 20 else None,
+            checksum=schema.checksum if schema.length > 22 else None,
         )
         return data
 
@@ -2735,11 +2739,17 @@ def _make_mptcp_capable(self, subtype: 'Enum_MPTCPOption', opt: 'Optional[Data_M
                             flag_req: 'bool' = False,
                             flag_ext: 'bool' = False,
                             flag_hsa: 'bool' = False,
-                            skey: 'int' = 0,
+                            skey: 'Optional[int]' = 0,
                             rkey: 'Optional[int]' = 0,
+                            dl_len: 'Optional[int]' = None,
+                            checksum: 'Optional[bytes]' = None,
                             **kwargs: 'Any') -> 'Schema_MPTCPCapable':
         """Make multipath TCP capable option.
 
+        Each field is present only if every field before it is, which selects
+        one of the five :rfc:`8684` section 3.1 lengths: 4 (``skey=None``), 12
+        (``rkey=None``), 20 (``dl_len=None``), 22 (``checksum=None``) or 24.
+
         Args:
             subtype: MPTCP subtype
             opt: option data
@@ -2749,11 +2759,16 @@ def _make_mptcp_capable(self, subtype: 'Enum_MPTCPOption', opt: 'Optional[Data_M
             flag_hsa: use of HMAC-SHA1 flag
             skey: option sender's key
             rkey: option receiver's key
+            dl_len: data-level length
+            checksum: checksum
             **kwargs: arbitrary keyword arguments
 
         Returns:
             Constructed option schema.
 
+        Raises:
+            ProtocolError: If a field is given without the field before it.
+
         """
         if opt is not None:
             version = opt.version
@@ -2762,12 +2777,19 @@ def _make_mptcp_capable(self, subtype: 'Enum_MPTCPOption', opt: 'Optional[Data_M
             flag_hsa = opt.flags.hsa
             skey = opt.skey
             rkey = opt.rkey
+            dl_len = opt.dl_len
+            checksum = opt.checksum
+
+        fields = (skey, rkey, dl_len, checksum)
+        present = fields.index(None) if None in fields else len(fields)
+        if any(field is not None for field in fields[present:]):
+            raise ProtocolError(f'{self.alias}: [OptNo {Enum_Option.Multipath_TCP}] invalid format')
 
         return Schema_MPTCPCapable(
             kind=cast('Enum_Option', Enum_Option.Multipath_TCP),
-            # NOTE: :rfc:`8684` section 3.1 gives MP_CAPABLE as 12 octets
-            # without the receiver's key and 20 octets with it.
-            length=12 if rkey is None else 20,
+            # NOTE: :rfc:`8684` section 3.1 gives MP_CAPABLE as 4, 12, 20, 22
+            # or 24 octets, by how many of the fields above are present.
+            length=(4, 12, 20, 22, 24)[present],
             test={
                 'subtype': subtype.value,
                 'version': version,
@@ -2779,6 +2801,8 @@ def _make_mptcp_capable(self, subtype: 'Enum_MPTCPOption', opt: 'Optional[Data_M
             },
             skey=skey,
             rkey=rkey,
+            dl_len=dl_len,
+            checksum=checksum,
         )
 
     def _make_mptcp_join(self, subtype: 'Enum_MPTCPOption', opt: 'Optional[Data_MPTCPJoin]' = None, **kwargs: 'Any') -> 'Schema_MPTCPJoin':