relay/console: fix the three remaining hardening items from the review
1. UDP ENDPOINT HIJACK. `from_host` in a UDP envelope is the sender's OWN claim,
and the relay used it directly to refresh that host's downstream endpoint --
so ANY datagram claiming host N silently stole host N's traffic (a zombie pod
from a previous round, a stale NAT mapping, or anyone who guessed a host id).
The victim simply stopped receiving on a channel that still looked healthy.
The claim is now bound to the identity we actually authenticated: the IP of
that host's live TCP game connection. The PORT is deliberately not checked --
it moves on a NAT rebind, which is the whole reason the endpoint map refreshes
per datagram -- and a genuine IP change cannot happen without the TCP
connection breaking and re-registering, so a legitimate pod is never rejected.
Rejections are counted (udp_spoofed) and logged once per offending (host, IP).
Known limit: two pods on one machine share an IP, so this cannot separate
them; same-machine trust is assumed.
2. THE EGG ACK COULD BE MISSED ENTIRELY -- a silent, unrecoverable wedge. The
pod's ACK is the launch gate's ONLY signal, and it was detected by parsing a
single recv() at fixed offset 0 behind a `len(data) >= 24` test. On a real
network (loopback hid both cases) the 28-byte ACK arriving SPLIT -- 16 bytes
then 12 -- was dropped by that test and never looked at again, and two
COALESCED messages meant only the first was read. A missed ACK means that
seat never counts toward the gate, so the round can never be released.
_console_read now buffers per connection and walks every complete frame
(16 + messageLength); an implausible length drops the connection WITH A REASON
rather than mis-reading that pod all night, since a stream protocol cannot be
resynced by guessing. Bounds: CONSOLE_MSG_MAX 64K, CONSOLE_INBUF_MAX 1 MiB.
3. REMOTE-OPERATOR MODE WAS HALF A CONSOLE. LAUNCH and END MISSION could never
enable (both conditions required `console_proc is not None`, which the remote
path never sets), the pilot lights were permanently blank (SessionMonitor was
built with an empty tag list, so `seated` was always 0), and Launch-local was
refused by the same guard even though the code below it already built
BT_RELAY from the remote host. All three enable conditions now accept EITHER
channel, and the monitor ADOPTS THE ROSTER from the relay's
"roster: N pilot(s) -> hostIDs [...]: [...]" line -- which the relay replays to
every newly AUTHed operator -- so a remote operator gets real lights and a
real seat count.
VERIFIED
* scratchpad/test_relay_net.py (new, 17 checks): spoofed datagram cannot move
an endpoint and is counted; a NAT rebind on the same IP still is honoured; an
unregistered host id still dropped; the ACK is found whole, split in two,
split three ways mid-header, and when hiding behind another message;
a garbage length drops the conn; the roster line is adopted, maps a
subsequent SEATED onto a real tag, lifts `seated` off 0, and re-feeding it
preserves known state.
* Live 2-pod rig: real pods ACKed through the new reassembly path (2/2 ready,
zero desync drops), the mission launched and ran, UDP flowed throughout
(93 rx / 85 tx, udp-known [2,3]) with ZERO false spoof rejections, then a
clean StopMission + round RESET.
* scratchpad/test_relay_rearm.py still 19/19; checkctx CLEAN.
Docs updated to match (context/operator-console.md gains reassembly and
anti-hijack sections; the guide no longer claims remote mode is crippled).
Remaining open items are now recorded in the topic's frontmatter: mesh mode's
operator buttons are inert by construction (no stdin reader in that path -- left
alone, mesh self-launches), _log_launch_readiness can cry STALL on a healthy
launch-with-whoever, and a straggler's late FIN is still misread as a pod dying
mid-load.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
1cda880c6d
commit
ab5828a866
+76
-6
@@ -308,6 +308,13 @@ VALID_CAMO_COLORS = {"Black", "Brown", "Crimson", "Green", "Grey", "Tan",
|
||||
# One authenticated operator at a time -- a new AUTH displaces the old.
|
||||
# A slow/stalled operator NEVER blocks the relay: writes are buffered per
|
||||
# connection and the connection is dropped if the buffer overflows.
|
||||
# Console-protocol reassembly bounds (see _console_read). A pod message is a
|
||||
# 16-byte header + messageLength bytes; the egg chunk is the biggest thing the
|
||||
# protocol carries at 16+1024, so anything wildly past that means the stream is
|
||||
# no longer frame-aligned.
|
||||
CONSOLE_MSG_MAX = 65536
|
||||
CONSOLE_INBUF_MAX = 1 << 20
|
||||
|
||||
CONTROL_PORT_OFFSET = 7 # control port = console port + 7
|
||||
CONTROL_AUTH_TIMEOUT = 10.0 # unauthenticated sockets die after this
|
||||
CONTROL_OUTBUF_MAX = 262144 # slow-reader cutoff (bytes)
|
||||
@@ -415,6 +422,7 @@ class RelayConsoleConn:
|
||||
self.sock = sock
|
||||
self.addr = addr
|
||||
self.egg_sent = False
|
||||
self.buf = b"" # inbound reassembly (see _console_read)
|
||||
self.acked = False # pod sent AcknowledgeEggFile -- a REAL pod.
|
||||
# LIVENESS (2026-07-26): a pod whose machine sleeps, whose Wi-Fi drops,
|
||||
# or which is killed without a FIN leaves a HALF-OPEN socket. With no
|
||||
@@ -944,11 +952,46 @@ class Relay:
|
||||
flush=True)
|
||||
self._drop_console(conn)
|
||||
return
|
||||
if len(data) >= 24:
|
||||
(clid, _gid, fh, _ts) = struct.unpack_from("<iiii", data, 0)
|
||||
(mlen, mid) = struct.unpack_from("<Ii", data, 16)
|
||||
print(f"[relay] pod->console clientID={clid} fromHost={fh} msgID={mid} "
|
||||
f"len={mlen}", flush=True)
|
||||
#
|
||||
# REASSEMBLE, then walk EVERY complete message. This used to parse one
|
||||
# recv() at a fixed offset 0 and look at nothing else, which had two
|
||||
# failure modes on a real network (loopback hid both):
|
||||
# * the pod's 28-byte ACK arriving SPLIT (say 16 bytes then 12) was
|
||||
# dropped by the `len(data) >= 24` test and never seen again -- and a
|
||||
# missed ACK means that seat never counts toward the launch gate, so
|
||||
# the round can never be released. A silent, unrecoverable wedge.
|
||||
# * two messages coalescing into one recv() -- only the first was read.
|
||||
# A frame is a 16-byte header + `messageLength` bytes (so the egg's 1040
|
||||
# = 16 + 1024, and the ACK's 28 = 16 + 12).
|
||||
#
|
||||
conn.buf += data
|
||||
if len(conn.buf) > CONSOLE_INBUF_MAX:
|
||||
print(f"[relay] console conn {conn.addr[0]}:{conn.addr[1]} dropped: "
|
||||
f"inbound buffer over {CONSOLE_INBUF_MAX} bytes (desynced)",
|
||||
flush=True)
|
||||
self._drop_console(conn)
|
||||
return
|
||||
while len(conn.buf) >= 24:
|
||||
(clid, _gid, fh, _ts) = struct.unpack_from("<iiii", conn.buf, 0)
|
||||
(mlen, mid) = struct.unpack_from("<Ii", conn.buf, 16)
|
||||
#
|
||||
# A stream protocol cannot be resynced by guessing, so an impossible
|
||||
# length means we are no longer frame-aligned: say so and drop, rather
|
||||
# than silently mis-reading this pod for the rest of the night.
|
||||
#
|
||||
if mlen < 8 or mlen > CONSOLE_MSG_MAX:
|
||||
print(f"[relay] console conn {conn.addr[0]}:{conn.addr[1]} "
|
||||
f"dropped: implausible messageLength {mlen} "
|
||||
f"(clientID={clid} msgID={mid}) -- stream desynced",
|
||||
flush=True)
|
||||
self._drop_console(conn)
|
||||
return
|
||||
frame = 16 + mlen
|
||||
if len(conn.buf) < frame:
|
||||
break # partial: wait for the rest
|
||||
conn.buf = conn.buf[frame:]
|
||||
print(f"[relay] pod->console clientID={clid} fromHost={fh} "
|
||||
f"msgID={mid} len={mlen}", flush=True)
|
||||
# AcknowledgeEggFile (clientID=0 NetworkManager, msgID=4): this
|
||||
# connection is a REAL pod -- count it toward the launch gate.
|
||||
if clid == 0 and mid == 4 and not conn.acked:
|
||||
@@ -1546,12 +1589,39 @@ class Relay:
|
||||
route, from_host, _seq = struct.unpack_from(ENV_FMT_UDP, data, 0)
|
||||
if from_host not in self.by_host:
|
||||
continue # not TCP-registered: drop
|
||||
live = self.by_host.get(from_host)
|
||||
#
|
||||
# ANTI-HIJACK (2026-07-26). `from_host` is the sender's OWN claim
|
||||
# about who it is, and the next line rewrites that host's downstream
|
||||
# endpoint. Unchecked, ANY datagram claiming host N silently steals
|
||||
# host N's traffic -- a zombie pod process from a previous round, a
|
||||
# stale NAT mapping, or a third party who guessed a host id. The
|
||||
# victim then just stops receiving, on a channel that looks healthy.
|
||||
#
|
||||
# Bind the claim to the identity we actually authenticated: the IP of
|
||||
# that host's live TCP game connection. The PORT is deliberately not
|
||||
# checked -- it moves on a NAT rebind, which is the whole reason the
|
||||
# endpoint map is refreshed per datagram. A genuine IP change cannot
|
||||
# happen without the TCP connection breaking and being re-registered
|
||||
# with the new address, so this never rejects a legitimate pod.
|
||||
#
|
||||
if live is not None and endpoint[0] != live.addr[0]:
|
||||
self.stats["udp_spoofed"] = self.stats.get("udp_spoofed", 0) + 1
|
||||
if not hasattr(self, "_udp_spoof_warned"):
|
||||
self._udp_spoof_warned = set()
|
||||
key = (from_host, endpoint[0])
|
||||
if key not in self._udp_spoof_warned:
|
||||
self._udp_spoof_warned.add(key)
|
||||
print(f"[relay] UDP from {endpoint[0]}:{endpoint[1]} CLAIMS "
|
||||
f"host {from_host}, whose TCP is {live.addr[0]} -- "
|
||||
f"DROPPED (it would have stolen that pod's downstream)",
|
||||
flush=True)
|
||||
continue
|
||||
self.stats["udp_rx"] += 1
|
||||
# NAT-rebind tolerant: every datagram refreshes the endpoint map.
|
||||
self.udp_endpoint[from_host] = endpoint
|
||||
# ... and counts as LIVENESS. Without this, a pod streaming updates
|
||||
# over UDP while its TCP sits idle looks "silent" to the reaper.
|
||||
live = self.by_host.get(from_host)
|
||||
if live is not None:
|
||||
live.last_seen = time.time()
|
||||
if route == ROUTE_HELLO:
|
||||
|
||||
+29
-4
@@ -75,6 +75,12 @@ class SessionMonitor:
|
||||
RE_RELAY_HELD = re.compile(
|
||||
r"launch HELD -- still loading: (.+) \((\d+)/(\d+) ready\)")
|
||||
RE_RELAY_RESET = re.compile(r"round RESET")
|
||||
# The relay announces its roster at startup and replays that line to a
|
||||
# newly AUTHed operator. A REMOTE operator has no egg loaded, so this is
|
||||
# the only way it can learn the tags -- without them `state` stayed empty,
|
||||
# every pilot light was invisible and `seated` was always 0.
|
||||
RE_RELAY_ROSTER = re.compile(
|
||||
r"roster: (\d+) pilot\(s\) -> hostIDs \[[^\]]*\]: \[(.*)\]")
|
||||
# A ROUND HAS ENDED / THE LAUNCHER RE-ARMED. Before these, `launched`
|
||||
# could only be cleared by "WAITING FOR OPERATOR" (needs every seat of the
|
||||
# last round to re-ACK) or "round RESET" (needs every pod gone). A games
|
||||
@@ -110,6 +116,14 @@ class SessionMonitor:
|
||||
if m:
|
||||
self.eggs = int(m.group(1))
|
||||
changed = True
|
||||
m = self.RE_RELAY_ROSTER.search(line)
|
||||
if m:
|
||||
tags = re.findall(r"'([^']*)'", m.group(2))
|
||||
if tags and tags != self.tags:
|
||||
# keep any state we already know for a tag that survives
|
||||
self.tags = tags
|
||||
self.state = {t: self.state.get(t, ST_IDLE) for t in tags}
|
||||
changed = True
|
||||
if self.RE_RELAY_WAITOP.search(line):
|
||||
# WAITING FOR OPERATOR = pods are ready to launch. Reset
|
||||
# `launched` so the LAUNCH button re-enables -- this fires
|
||||
@@ -1032,8 +1046,10 @@ class Operator(QMainWindow):
|
||||
|
||||
if self.monitor.launched:
|
||||
head = "LAUNCHED — mission running"
|
||||
if (self.console_proc and not self.end_btn.isEnabled()
|
||||
and not getattr(self, "end_sent", False)):
|
||||
if (not self.end_btn.isEnabled()
|
||||
and not getattr(self, "end_sent", False)
|
||||
and (self.console_proc is not None
|
||||
or getattr(self, "remote_link", None) is not None)):
|
||||
self.end_btn.setEnabled(True)
|
||||
elif self.monitor.held_status:
|
||||
head = "STAGING — " + self.monitor.held_status
|
||||
@@ -1068,10 +1084,15 @@ class Operator(QMainWindow):
|
||||
combo = self.table.cellWidget(r, 2)
|
||||
if mech and combo is not None and combo.currentText() != mech:
|
||||
combo.setCurrentText(mech)
|
||||
# A REMOTE operator has no console_proc -- its command channel is the
|
||||
# control socket -- so requiring one made LAUNCH (and END MISSION below)
|
||||
# permanently dead in remote mode. Accept either channel.
|
||||
have_relay = (self.console_proc is not None
|
||||
or getattr(self, "remote_link", None) is not None)
|
||||
self.launch_btn.setEnabled(
|
||||
(self.monitor.ready or (self.monitor.relay_mode and seated > 0))
|
||||
and not self.monitor.launched
|
||||
and self.console_proc is not None)
|
||||
and have_relay)
|
||||
if self.monitor.stats:
|
||||
self.stats_label.setText(self.monitor.stats)
|
||||
|
||||
@@ -1090,7 +1111,11 @@ class Operator(QMainWindow):
|
||||
QMessageBox.warning(self, "Launch", "btl4.exe not found:\n" + exe)
|
||||
return
|
||||
relay = self.mode.currentIndex() == 0
|
||||
if relay and self.console_proc is None:
|
||||
# Remote mode has no console_proc but IS a live session -- and the env
|
||||
# build below already points BT_RELAY at remote_link.host, so this guard
|
||||
# was the only thing making the feature unreachable there.
|
||||
if (relay and self.console_proc is None
|
||||
and getattr(self, "remote_link", None) is None):
|
||||
QMessageBox.warning(
|
||||
self, "Launch",
|
||||
"The session is not running -- press Start Session first\n"
|
||||
|
||||
Reference in New Issue
Block a user