COTP Connection Confirm is off by one

[djformby]

TLDR
A recent change created an off-by-one error and broke connection setups. Claude wrote up the explanation and fix below.

Package version: python-snap7 3.1.2 (verified), and current master (verified by reading source)
Affected: snap7.server.Server (pure-Python server only — the client and libsnap7 are unaffected)
Introduced by: #806, which fixed #804

Summary

Since #806 the server's COTP Connection Confirm declares a length indicator of 9 while
emitting 10 octets. Strict clients reject the frame and the connection never completes.

#806 was addressing something real — omitting the TPDU-size parameter did make clients fall
back to the class-0 default of 128 bytes, exactly as described in #804. The parameter is now
correctly echoed. The problem is a single extra octet that came along with it.

What the spec says

RFC 905 §13.4.1 (ISO 8073 / X.224), Connection Confirm:

  1      2     3   4   5   6     7     8     p   p+1 ...end
+---+----+---+---+---+---+---+-------+--------+-------------+
|LI | CC  CDT|DST-REF|SRC-REF| CLASS |VARIABLE| USER        |
|   |1101|   |   |   |   |   | OPTION|  PART  | DATA        |
+---+----+---+---+---+---+---+-------+--------+-------------+

Note the octet numbering: CDT shares octet 2 with the TPDU code. 0xD0 is 1101 0000 —
code 1101 in the high nibble, CDT 0000 in the low nibble, and RFC 905 confirms CDT is zero
in class 0. DST-REF is octets 3–4, SRC-REF 5–6, class/option 7, and the variable part begins at
octet 8. There is no octet between the code and DST-REF.

The 0x00, # Reserved / CDT field added in #806 therefore writes a second, standalone octet for
a field that is already encoded in the byte written on the line above it.

A standalone CDT field does exist in ISO 8073 — but only in the AK and RJ TPDUs in
extended format (classes 2/3/4), where it sits after YR-TU-NR. It never applies to a CC, and
RFC 1006 / S7comm is class 0 throughout. Easy detail to cross over on; the spec lists CDT both
ways.

The bytes

current:  0300000f 09 d0 00 000f 0001 00 c00109     LI=9, 10 octets follow   <-- rejected
correct:  0300000e 09 d0    000f 0001 00 c00109     LI=9,  9 octets follow
                      ^^ extra octet

Because parsers trust the length field, the extra octet shifts every subsequent field. Wireshark
reads DST-REF as 0x0000 and SRC-REF as 0x0f00, then lands on the 0x00 as a parameter code
with length 0xC0 (192), overruns the 15-octet frame, and reports [Malformed Packet]. Apache
PLC4X fails the same way:

GeneratedDriverByteToMessageCodec - Error decoding package with content
[0300000f09d000000f000100c00109]: Unsupported case for discriminated type
parameters [parameterType=0]
...
S7ProtocolLogic - Timeout during Connection establishing, closing channel...

The connect future then never completes and the client blocks until its timeout.

Reproduction

No external client needed:

"""Minimal reproduction: the server's COTP Connection Confirm is malformed."""
import ctypes, socket, time
from snap7.server import Server
from snap7.type import SrvArea

server = Server()
db1 = (ctypes.c_ubyte * 64)()
server.register_area(SrvArea.DB, 1, db1)
server.start(tcp_port=1102)
time.sleep(1)

# A well-formed COTP Connection Request (RFC 905 13.3):
#   LI=0x11, CR+CDT=0xE0, DST-REF=0000, SRC-REF=000f, class=00,
#   params: src-tsap C1, dst-tsap C2, tpdu-size C0
cr = bytes.fromhex("0300001611e00000000f00" "c1020311" "c2020101" "c00109")

s = socket.create_connection(("127.0.0.1", 1102), timeout=5)
s.sendall(cr)
cc = s.recv(1024)
s.close()
server.stop(); server.destroy()

print("CC bytes :", cc.hex())
li = cc[4]
following = len(cc) - 5
print(f"LI       : {li}  (declares {li} octets follow)")
print(f"actual   : {following} octets follow")
print("RESULT   :", "OK" if li == following else f"MISMATCH - off by {following - li}")

On 3.1.2:

CC bytes : 0300000f09d000000f000100c00109
LI       : 9  (declares 9 octets follow)
actual   : 10 octets follow
RESULT   : MISMATCH - off by 1

With the fix below:

CC bytes : 0300000e09d0000f000100c00109
LI       : 9  (declares 9 octets follow)
actual   : 9 octets follow
RESULT   : OK

Suggested fix

snap7/server/__init__.py, in _build_cotp_cc (lines 2708 and 2711 on master):

         base_pdu = struct.pack(
-            ">BBBHHB",
+            ">BBHHB",
             pdu_length,  # PDU length
             self.COTP_CC,  # PDU type
-            0x00,  # Reserved / CDT
             self.dst_ref,  # Destination reference (client's source ref)
             self.src_ref,  # Source reference (our ref)
             0x00,  # Class/option
         )

pdu_length should not change. 6 + len(pdu_size_param) is already correct — it counts the
six fixed octets after LI that RFC 905 specifies. Only the struct format was over-counting. (The
tempting alternative, bumping pdu_length to 7, yields a self-consistent frame that is still
wrong against the spec.)

After the fix, the frame matches §13.4.1 octet for octet:

09      octet 1     LI = 9
d0      octet 2     CC (1101) + CDT (0000)
000f    octets 3-4  DST-REF
0001    octets 5-6  SRC-REF
00      octet 7     CLASS OPTION
c00109  octets 8-10 VARIABLE PART: tpdu-size = 512

Verified against Apache PLC4X 0.13.1: with the patch applied the connection completes and reads
return correct values (INT, WORD, DINT and REAL all matching seeded memory).

Possible regression test

The repro above is small enough to run as a unit test — assert cc[4] == len(cc) - 5 on the
Connection Confirm. That would have caught this, and would catch any future change to the fixed
part of the header.

[gijzelaerr]

Thanks for the detailed report and the exact wire-level analysis. The fix is up in #816.

It removes only the extra standalone CDT byte and keeps the TPDU-size negotiation introduced for #804. I also added a regression test for the exact Connection Confirm bytes and its length indicator.

Could you test the #816 branch with Apache PLC4X and confirm that connection setup and reads now complete correctly?