Repository navigation
fix(rcon): send replies, buffer fragmented packets, harden auth - #201
Merged
Merged
Conversation
- p_send/ParsePacket are now coroutines and await sock_sendall, so the auth response and command replies actually reach the client. - Each peer keeps a receive buffer and only consumes complete packets; a declared size outside 10..4096 closes the connection. struct.error can no longer escape on short reads. - Password compared with hmac.compare_digest on bytes; an empty configured password refuses all auth; startup WARNING when the password is empty or the shipped default. - The CommandHandler is resolved through TorchlightHandler at dispatch time, so sessions survive a SourceMod reconnect without pinning the old object graph. - Peer and command tasks go through Utils.FireAndForget; sockets are closed exactly once; peers capped at 16 and unauthenticated peers dropped after 30 s. - Authenticated EXECCOMMAND packets get an empty RESPONSE_VALUE with the request id so standard clients do not hang. Closes #185 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
15 of 16 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the TorchRCON server (
SourceRCONClient.py,SourceRCONServer.py): replies were never sent, fragmented packets were dropped, auth was weak, and open sessions pinned a staleCommandHandlerafter a SourceMod reconnect.1. Replies are actually sent
loop.sock_sendallis a coroutine function, andp_sendnever awaited it, so no reply reached the socket. That includedSERVERDATA_AUTH_RESPONSE, and Python warnedcoroutine ... was never awaited.send,p_sendandParsePacketare now coroutines and every send is awaited. The auth reply sequence is unchanged: an emptyRESPONSE_VALUE, thenAUTH_RESPONSEwith the request id (or-1on failure), then the welcome line on success.An authenticated
EXECCOMMANDnow also gets an emptyRESPONSE_VALUEcarrying the request id, which is what the old commented-out line intended. Standard clients that wait for a reply to their command id no longer hang. Command output is still not captured.2. Per-peer receive buffer and framing
_peer_loopused to overwritedataon everyrecv, so a packet split across TCP segments (or any command over about 1 KB) was silently lost and the stream desynced. Each peer now keeps abytearraybuffer, and only complete packets are consumed. A partial packet stays buffered until the rest arrives. A declared size outside the Source RCON bounds (< 10or> 4096) logs a warning and closes the connection. This also bounds the buffer at one maximum-size packet plus one recv chunk. Sizes are read only once 4 bytes are buffered, sostruct.errorcan no longer escape into an untracked task.3. Auth hardening
hmac.compare_digeston UTF-8 bytes. Comparing bytes also keeps non-ASCII passwords working, sincecompare_digestrejects non-ASCIIstr.SourceRCONServerlogs a startupWARNINGwhen the password is empty (RCON auth disabled) or still the shipped defaultsupersecretpassword.-1reply. Unauthenticated peers that send nothing are now dropped after 30 s, and concurrent peers are capped at 16. Before, idle sockets could growpeerswithout bound.4. No stale
CommandHandlerTorchlightHandler.Init()builds a newCommandHandleron every SourceMod reconnect, but each peer captured the one that existed at accept time. Its commands then ran against the deadAsyncClient, and the session kept the whole previous object graph alive. Peers now get aget_command_handlercallable (SourceRCONServer.GetCommandHandler), which readstorchlight_handler.command_handleron every command.5. Task and socket lifecycle
The peer handler and command dispatch use
Utils.FireAndForget(coro, self.logger)instead of bareasyncio.Task(...), so exceptions get logged instead of dropped.close()is idempotent and clears the buffer. The socket is closed exactly once, in_peer_handler'sfinally, on every path: EOF, invalid size, failed auth, auth timeout, andOSError.Remove()tolerates a peer that is already gone. The peer is now added topeersbefore its task starts.SourceRCONServer(rcon_config, torchlight_handler)and_server()are unchanged, socli.pyneeds no change.Note:
SourceRCONClient.pywas stored with CRLF line endings, unlike the rest of the repo. This change normalizes it to LF, so view the diff with "Hide whitespace".Test plan
ruff check .,ruff format --check .andmypyreport only the 3 known baseline errors on master (UP041 inFFmpegAudioPlayer.py, arg-type inFlareSolverr.py). No new errors.SourceRCONServeron 127.0.0.1 with a stubTorchlightHandler/CommandHandler, drives it with a hand-written struct-packed Source RCON client, and checks:RESPONSE_VALUE(id),AUTH_RESPONSE(id)and the welcome line are receivedRESPONSE_VALUE(id)handler.command_handler, the existing session dispatches to the new handlerstruct.errorpath)RESPONSE_VALUE(id)andAUTH_RESPONSE(-1), then the connection closesEXECCOMMANDbefore auth is ignoredpeersis empty once all clients disconnectcoroutine ... was never awaitedwarnings (run with-X dev, with RuntimeWarning raised as an error)rcon-cli)No VERSION bump (batch policy, see #177).
Closes #185
Part of #177
🤖 Generated with Claude Code