Add TCPCLv4 message definitions and tcp_reassemble interface

[BrianSipos]

Description

This is a port of earlier TCPCL packet definitions from dtn-demo-agent with more modern scapy-friendly patterns of dispatchers and with a tcp_reassemble() interface.

This is an alternative to part of #4824 with some simplified structures.
Test capture files are included for demonstration.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.41%. Comparing base (56062c6) to head (0e48f49).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5076      +/-   ##
==========================================
+ Coverage   80.09%   80.41%   +0.31%     
==========================================
  Files         375      376       +1     
  Lines       97696    97844     +148     
==========================================
+ Hits        78254    78682     +428     
+ Misses      19442    19162     -280     

Files with missing lines	Coverage Δ
scapy/contrib/tcpcl.py	100.00% <100.00%> (ø)

... and 21 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[BrianSipos]

I don't know why the remaining CI job is not passing. I don't see that it relates to the changes in this PR.

[gpotter2]

Hi !

We haven't forgotten you and very sorry for the delay. We have still a few dozens of potential CVEs we need to fix from the Patch the Planet backlog, which is taking longer than expected. Once that is done this is one of our priorities.

Thanks again for the PR and time spent on this !

[polybassa]

Comments on Scapy-likeness and simplicity of the TCPCL framing, extension layout, and dispatch.

[polybassa]

tcp_reassemble() builds a packet immediately and uses the resulting class to decide what the stream contains. It never checks whether a complete message is available.

That conflicts with Scapy’s reassembly contract: return None while waiting for bytes, and return a packet once its boundary is known. Dissection is not a completeness check, particularly for StrLenField.

Reproduced:

Input	Current result	Expected
b"dtn!"	Unknown v4 message; both session versions set to 4	Wait for the rest of the contact header
b"dtn!\x04"	struct.error	Wait for the flags byte
Transfer declaring 6 data bytes, only 3 present	TCPCLXferSegment(length=6, data=b"abc")	Wait for the remaining data

The last case matters most: returning the truncated packet lets the session consume those bytes early.

A small framing routine local to this protocol is enough:

• Check the minimum header size before reading length fields.
• For fixed-size messages, check that fixed size.
• For SESS_INIT and XFER_SEGMENT, check each variable-length section in order.
• Return None until the first complete message is available.
• Leave subsequent bytes as Padding so TCPSession can handle them.

DoIP shows the contract. TCPCL needs those length checks, not a general framing framework or exception-driven parsing. Keep the existing TCPCL.extract_padding() behavior for completed messages; it already handles trailing data.

[BrianSipos]

I believe 9d059af fixes these, but some cases are not unit tested.

[polybassa]

These session keys are named initiator and responder, but they are assigned by arrival order:

if "tcpcl-version-i" not in session:
    session["tcpcl-version-i"] = int(pkt.version)
elif "tcpcl-version-r" not in session:
    session["tcpcl-version-r"] = int(pkt.version)

That does not establish either endpoint’s identity, and only the second value later controls message decoding. It adds state without reliably representing negotiation:

• A missing contact header invents both versions as v4.
• After that fallback, a later contact header is decoded as a message.
• An unknown message type still produces TCPCLBaseMsgV4, so the isinstance() check does not mean it is a recognized v4 message.
• Recording v3 as the second contact makes later calls return None, even when more bytes cannot enable supported decoding.

Reproduced v4 contact → keepalive → v4 contact: the last contact becomes TCPCLBaseMsgV4.

Recognize contact magic at message boundaries independently of whether two headers have already been seen. For this v4-focused contribution, keep only state that changes decoding. If version tracking is needed so v3 traffic is not read as v4, record actual observations and define an explicit unsupported-version behavior. A handshake state machine is not needed to preserve the two-key design.

Captures without a contact header are useful. The fabricated negotiation state is not.

[BrianSipos]

For test generation, is there an easy way to synthesize TCP/IP framing for a TCP connection (to make addresses and ports consistent) in a sequence of packets? I would like to be able to unit test these variations and be able to use the TCP packet contents (ports) to differentiate the two stream directions of the connection.

[polybassa]

One solution would be to use standard stream sockets and create a few receiver and sender threads on local host with different random ports. In this way you could send your tcpcl stream and let the kernel do the tcp stuff. Either you capture those messages live in the unit test, our you could use the pcap of such a test setup and parse the packets.

Does this help or are you looking for something different?

[BrianSipos]

That works, I didn't know if there was more specialized test helpers.

[BrianSipos]

I believe 9d059af fixes these, but uses TCP ports to distinguish stream directions to allow exactly one contact header in each direction.

[polybassa]

TCPCL describes itself as a dispatcher, but it has no dispatch_hook(). The only dispatch logic lives in tcp_reassemble().

A complete keepalive over TCP, dissected with ordinary IP(bytes(...)), currently comes out as:

IP / TCP / TCPCL / Padding

The keepalive byte stays padding. Message-level decoding only happens when the caller uses TCPSession.

Give the top-level layer normal dispatch for complete packet data, reusing the existing contact and message dispatchers. Keep the completeness checks in tcp_reassemble().

• dispatch_hook(): which packet class represents these bytes?
• tcp_reassemble(): are enough bytes available to return a complete message?

Guard an inherited top-level hook so it does not redispatch concrete subclasses recursively.

The existing variant registries are a recognizable Scapy pattern. I would keep them rather than add a generic registry mixin for two short methods.

[BrianSipos]

I believe 9d059af fixes these by adding a valid top dispatch_hook() and using it from the unit tests.

[polybassa]

Both dispatch hooks unpack a single byte through struct:

struct.unpack("!B", _pkt[4:5])[0]  # TCPCLContact
struct.unpack("!B", _pkt[:1])[0]   # TCPCLBaseMsgV4

After the existing bounds checks, _pkt[4] and _pkt[0] are enough. if _pkt and len(_pkt) >= 1: in TCPCLBaseMsgV4.dispatch_hook() reduces to if _pkt:.

A few related cleanups:

• Registry annotations should describe classes, for example Dict[int, Type["TCPCLContact"]].
• The tcp_reassemble() return annotation should allow None and follow Scapy’s classmethod annotation convention.
• TCPCLContact() and TCPCLBaseMsgV4() currently return Raw when constructed with no bytes. That is surprising for exported packet classes, even if they are mainly dispatchers. Worth distinguishing “no bytes supplied” from “invalid wire data”.
• Drop routine parsing logs where they add noise rather than something actionable.

[BrianSipos]

I believe 9d059af fixes these, and tests the no-data case.