_make_mptcp_*/_read_mptcp_* length arithmetic is wrong at six more sites beyond MP_CAPABLE

[JarryShaw]

_make_mptcp_*/_read_mptcp_* length arithmetic is wrong at six more sites beyond MP_CAPABLE

Found while fixing #566 and #567, which #567 explicitly invited ("Worth checking the sibling
_make_mptcp_* helpers' length arithmetic in the same pass... this may not be the only one").
Filed separately rather than folded into that PR, matching how #566/#567 themselves were split out
from #541.

All measurements below are against fix/566-567-mptcp-subtype-and-capable-length (which already
carries the #566/#567 fixes), using TCP.__new__(TCP)._make_mptcp_*(...).pack() directly so the
maker's own arithmetic is isolated from the round-trip harness in
tests/protocols/test_option_roundtrip_unit.py.

1. MP_FASTCLOSE: the maker, the schema, and the parser all disagree

• Maker: pcapkit/protocols/transport/tcp.py:3054 (_make_mptcp_fastclose) writes length=12,
  which matches RFC 8684 section 3.5 ("Fast Close").
• Schema: pcapkit/protocols/schema/transport/tcp.py:907 (MPTCPFastclose.test) declares only 1
  octet (BitField(length=1, namespace={'subtype': (0, 4)})), where the RFC figure (quoted in this
  method's own docstring at pcapkit/protocols/transport/tcp.py:1875) shows Subtype sharing a
  32-bit row with 12 reserved bits -- i.e. 2 octets, not 1. Measured:
  _make_mptcp_fastclose(MPTCPOption.MP_FASTCLOSE, key=9).pack() produces 1e0c700000000000000009,
  11 octets, against a declared length of 12.
• Parser: pcapkit/protocols/transport/tcp.py:1893 (_read_mptcp_fastclose) rejects anything but
  schema.length == 16, a third, independent number that agrees with neither the maker nor the
  RFC figure.

Net effect: TCP(options=[(Enum_Option.Multipath_TCP, {'subtype': Enum_MPTCPOption.MP_FASTCLOSE, 'key': 9})], ...) raises ProtocolError: TCP: [OptNo 30] invalid format from the parser guard,
because the maker's correct length (12) fails the parser's wrong check (!= 16). This is why
tcp-mptcp/MP_FASTCLOSE is still in EXPECTED_FAILURES in
tests/protocols/test_option_roundtrip_unit.py after #566/#567 land, now for this reason rather
than the subtype AttributeError it used to fail with.

2. MP_JOIN SYN/ACK: wrong length, and hmac silently dropped on reconstruction

pcapkit/protocols/transport/tcp.py:2790 (_make_join_synack) writes length=12. The schema
(MPTCPJoinSYNACK in pcapkit/protocols/schema/transport/tcp.py: test(1) + addr_id(1) +
hmac(8, BytesField(length=8)) + nonce(4, UInt32Field), plus the 2 octets of kind/length)
totals 16 octets, matching RFC 8684 section 3.2's MP_JOIN-SYN/ACK. 12 is _make_join_syn's own
(correct) length for the SYN form, copy-pasted into the SYN/ACK maker without recomputing for the
wider payload.

Separately, pcapkit/protocols/transport/tcp.py:2782-2786's if opt is not None: branch sets
backup, addr_id, and nonce (twice -- nonce = opt.nonce appears on both line 2785 and 2786)
but never sets hmac = opt.hmac. Reconstructing from a parsed option therefore always uses the
hmac: bytes = bytes(8) default instead of the value that was actually parsed.

3. MP_JOIN ACK: length off by a factor of three

pcapkit/protocols/transport/tcp.py:2820 (_make_join_ack) writes length=8. The schema
(MPTCPJoinACK: test(1) + reserved(1, PaddingField(length=1)) + hmac(20,
BytesField(length=20)), plus kind/length) totals 24 octets, matching RFC 8684 section 3.2's
MP_JOIN-ACK (the 160-bit HMAC alone is 20 octets).

4. REMOVE_ADDR: length hardcoded to 4 regardless of how many address IDs are given

pcapkit/protocols/transport/tcp.py:2970 (_make_mptcp_remove) writes length=4 unconditionally,
but addr_id is a list[int] of arbitrary size (MPTCPRemoveAddress.addr_id is a ListField
sized pkt['length'] - 3). Measured:

• addr_id=[1, 2] packs 1e04400102, 5 octets, declared length 4.
• addr_id=[] packs 1e0440, 3 octets, declared length 4.

The generator's own fixture (examples/generators/options.py's _mptcp_overrides) happens to use
addr_id=[1] -- exactly one ID -- which is the one list length the hardcoded 4 is correct for, so
tcp-mptcp/REMOVE_ADDR reads 'OK' in the round-trip suite without exercising this.

5. MP_PRIO: length hardcoded to 4 even when addr_id is omitted

pcapkit/protocols/transport/tcp.py:2999 (_make_mptcp_prio) writes length=4 unconditionally,
but MPTCPPriority.addr_id is itself ConditionalField(UInt8Field(), lambda pkt: pkt['length'] == 4) -- i.e. present only when length is 4. With addr_id=None (the default), the maker still
declares length 4, so the predicate is satisfied and a phantom all-zero address-ID octet gets
packed anyway. Measured: _make_mptcp_prio(MPTCPOption.MP_PRIO) (no addr_id) packs 1e045000,
4 octets including the phantom 00, where RFC 8684 section 3.3.8 gives 3 octets for the
address-ID-less form.

6. DSS: declared length does not match what the schema actually packs

pcapkit/protocols/transport/tcp.py:2873 (_make_mptcp_dss) computes
length=4 + (4 if flag_A else 0) + (4 if flag_a else 0) + (12 if flag_M else 0) + (4 if flag_m else 0) -- i.e. it assumes the Data ACK field is 4 octets when present-and-not-extended. But the schema
field it is describing, MPTCPDSS.ack in pcapkit/protocols/schema/transport/tcp.py:796-797, is
ConditionalField(NumberField(length=lambda pkt: 8 if pkt['flags']['a'] else 0), ...) -- 8 octets
when extended, 0 octets otherwise, not 4. MPTCPDSS.dsn at lines 801-802 has the identical
shape and the identical bug.

Measured with the generator's own override args (ack=1, dsn=2, ssn=3, dl_len=4, checksum=b'\x00\x00', none of which set the 64-bit extension flags): _make_mptcp_dss(...).pack()
produces 1e1420050000000300040000, 12 octets, against a declared length of 20 -- and the
ack/dsn values are not actually on the wire at all, since their fields packed zero octets.
tcp-mptcp/DSS reads 'OK' in the round-trip suite despite this, because construct -> parse ->
reconstruct produces the same wrong 12 octets each time and the suite only checks that the cycle is
self-consistent, not that it matches RFC 8684 -- exactly the blind spot
tests/protocols/test_option_roundtrip_unit.py's own module docstring names ("a defect can leave
the cycle closed").

There is also a harmless duplicate key in the same call:
pcapkit/protocols/transport/tcp.py:2879 and :2883 both write 'A': flag_A into the same
flags={...} dict literal.

Suggested split

Each of the six looks independent enough to fix on its own PR, the way #566 and #567 were split
from #541 and from each other:

• MP_FASTCLOSE needs its schema widened (a real reserved field, not just a wider test) and its
  parser's 16 corrected to 12, together -- fixing only one would leave the other broken, the
  same shape as TCP._make_mptcp_capable writes length 20/32 where RFC 8684 gives 12/20, and now packs wrong bytes rather than crashing #567.
• MP_JOIN SYN/ACK needs its length corrected to 16 and its dropped hmac reconstruction fixed.
• MP_JOIN ACK needs its length corrected to 24.
• REMOVE_ADDR needs its length computed from len(addr_id_list) rather than hardcoded.
• MP_PRIO needs its length to depend on whether addr_id is actually given.
• DSS needs its schema's ack/dsn field-length lambdas corrected to 4/8 (not 0/8), which will
  also change what its maker needs to compute.

None of these are fixed by #566/#567: those two are scoped to MPTCP.subtype and MP_CAPABLE's own
length/rkey arithmetic specifically, and this issue is the record that the sibling helpers were
checked, as suggested, rather than left unexamined.

[JarryShaw]

Correction to one of the six sites, from #579's cross-review, which re-derived all of them against both the code and RFC 8684.

MP_JOIN SYN is not defective. This issue lists it among the wrong lengths; its length=12 is correct — RFC 8684 §3.2 gives Length = 12 for the initial SYN form. The two MP_JOIN defects are the other two forms:

• SYN/ACK writes 12 where the RFC gives 16.
• ACK writes 8 where the RFC gives 24.

So the count of six stands, but one entry was mis-attributed within the MP_JOIN family.

Two of the other five also gain a precise RFC figure rather than an inferred one:

• REMOVE_ADDR hardcodes length=4 where the RFC gives Length = 3 + n — so the defect is not merely a wrong constant but that a variable-length option has a constant at all, which is why addr_id=[1, 2] packs 5 and addr_id=[] packs 3.
• DSS's over-read is confirmed by its own diagnostic, packet length < 0: -8, rather than only by reading the field declarations.

MP_FASTCLOSE, MP_PRIO and the DSS arithmetic are confirmed exactly as filed.

One related note while this is being read: the fix for #566/#567 in #579 removes a dependency on the #431 short-read accommodation for MP_CAPABLE. Measured with only the predicate varied on a 12-octet option, the old != 32 produced packet length < 0: -8 — an 8-octet over-read that survived only because rjust() zero-padded it — while the corrected == 20 is clean. Several of the lengths in this issue may be masking similar over-reads, so whoever takes this should watch for packet length < 0 warnings disappearing as each is fixed; their disappearance is a signal the length was previously wrong rather than merely inelegant.

[JarryShaw]

Correction to this issue's body, and one defect split out of it.

The RFC citation was wrong. The body cited RFC 8684 §3.7 for MP_FASTCLOSE; §3.7 is "Fallback" (MP_FAIL). Fast Close is §3.5. Verified against the published RFC, and the body is now amended. I have not confirmed the figure number for §3.5 — the copy I read was truncated before it — so I have deliberately left a figure reference out rather than guess one. Caught by the cross-derivation on #585, not by me.

tcp-mptcp/MP_JOIN is a separate defect and is now #587. Its EXPECTED_FAILURES entry must stay until #587 is fixed; it fails for an ordering reason unrelated to length arithmetic — TCP._make builds the options at tcp.py:547 and only assigns self._flags at tcp.py:567, while _make_mptcp_join branches on that attribute at tcp.py:2695-2699. Measured: constructing an MP_JOIN option on a fresh instance raises AttributeError: 'TCP' object has no attribute '_flags'.

tcp-mptcp/MP_FASTCLOSE's entry, by contrast, becomes stale the moment this issue is fixed and is being deleted inside #585, since deleting it separately would turn main red until that PR merged.

Two further notes on that entry's recorded text for whoever reads it next: it carries the same §3.7 mis-citation, and its line references (tcp.py:1893, tcp.py:3054, schema/transport/tcp.py:907) predate #579 and have since moved.