fsp-server: reply checksums must use fsplib's VERIFIER formula, not its writer
Swiss hung on "setting up device" while the server logged perfectly reasonable
GET_PRO replies. fsplib was dropping every one and retrying -- visible in the
log as the same request repeating with the low 3 sequence bits counting up,
since fsp_transaction encodes the retry number there.
FSP has TWO checksum formulas and they differ by the packet length:
fsp_pkt_write (what a client SENDS):
checksum = sum(bytes, sum byte zeroed) + used
sum_byte = checksum + (checksum >> 8)
fsp_pkt_read (what a client ACCEPTS):
mysum = sum(bytes) - sum_byte <- no length term at all
ok if ((mysum + (mysum >> 8)) & 0xff) == sum_byte
cube/patches/bba/tcpip.c matches the WRITE form (`sum = size` to start) and
never verifies replies, so the in-game path tolerated our packets; the menu's
fsplib did not. Replies are now built the way fsp_pkt_read verifies.
Incoming requests are accepted under either formula, since a tolerated request
is far easier to diagnose than a rejected one.
⚠️ The self-test passed throughout the bug, and that is the part worth
remembering: it verified replies with the SERVER'S OWN checksum function, so
the two agreed with each other and disagreed with Swiss. Identical in shape to
the RX descriptor format, where our writer, our readers and our tests were all
self-consistent and all wrong. The test now transcribes fsp_pkt_read literally
as fsplib_verify() and checks every reply against that, and mutating the server
back to the with-length formula fails it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -10,11 +10,30 @@ fails = []
|
||||
|
||||
|
||||
def csum(pkt):
|
||||
"""fsp_pkt_write / tcpip.c: INCLUDES the length. For requests."""
|
||||
t = len(pkt) + sum(pkt)
|
||||
t += t >> 8
|
||||
return t & 0xFF
|
||||
|
||||
|
||||
def fsplib_verify(pkt):
|
||||
"""fsp_pkt_read, transcribed literally -- this is what Swiss's menu client
|
||||
accepts, and it does NOT add the length:
|
||||
|
||||
mysum = -ptr[SUM]; for(i..recv_len) mysum += ptr[i];
|
||||
mysum = (mysum + (mysum >> 8)) & 0xff;
|
||||
ok if mysum == ptr[SUM]
|
||||
|
||||
⚠️ Verify replies with THIS, never with the server's own function: an
|
||||
earlier version of this test used the server's symmetric checksum, passed
|
||||
every assertion, and the real Swiss client rejected every reply."""
|
||||
mysum = -pkt[1]
|
||||
for b in pkt:
|
||||
mysum += b
|
||||
mysum = (mysum + (mysum >> 8)) & 0xFF
|
||||
return mysum == pkt[1]
|
||||
|
||||
|
||||
def req(cmd, seq, pos, path, pw=PW, extra=b""):
|
||||
data = path.encode() + (b"\n" + pw.encode() if pw else b"") + b"\0"
|
||||
body = struct.pack("!BBHHHI", cmd, 0, 0x1234, seq, len(data), pos) + data + extra
|
||||
@@ -23,9 +42,10 @@ def req(cmd, seq, pos, path, pw=PW, extra=b""):
|
||||
|
||||
def parse(pkt):
|
||||
cmd, cs, key, seq, ln, pos = struct.unpack("!BBHHHI", pkt[:HSIZE])
|
||||
zeroed = pkt[:1] + b"\0" + pkt[2:]
|
||||
# Replies are checked with FSPLIB's verifier, which is the only opinion
|
||||
# that matters -- not with csum() above, which is the request formula.
|
||||
return dict(cmd=cmd, seq=seq, pos=pos, data=pkt[HSIZE:HSIZE + ln],
|
||||
csum_ok=csum(zeroed) == cs)
|
||||
csum_ok=fsplib_verify(pkt))
|
||||
|
||||
|
||||
tmp = tempfile.mkdtemp()
|
||||
@@ -45,7 +65,11 @@ A = ("127.0.0.1", 2121)
|
||||
|
||||
def rt(pkt):
|
||||
s.sendto(pkt, A)
|
||||
return parse(s.recv(2048))
|
||||
raw = s.recv(2048)
|
||||
if not fsplib_verify(raw):
|
||||
fails.append(f"reply to cmd {pkt[0]:#04x} FAILS fsplib's verifier "
|
||||
f"-- Swiss would drop it and retry")
|
||||
return parse(raw)
|
||||
|
||||
try:
|
||||
# T1 checksum both ways
|
||||
|
||||
Reference in New Issue
Block a user