diff --git a/egg/with-sidecar.sh b/egg/with-sidecar.sh index 2dacc52..940294b 100755 --- a/egg/with-sidecar.sh +++ b/egg/with-sidecar.sh @@ -31,6 +31,23 @@ for v in RUSTLINK_SERVER_ID RUSTLINK_WEB_TOKEN RUSTLINK_RETAIN_DAYS RUSTLINK_WEB if [ -z "$val" ]; then unset "$v"; fi done +# The server id follows the PLUGIN's config once it exists. The plugin reads RUSTLINK_SERVER_ID only +# when it writes its first config; after that the file is canonical and the website locks it (D150). +# The sidecar refuses a plugin that names another server (D155), so a variable edited after the +# first boot would otherwise take the bridge down instead of changing nothing. Stock framework paths, +# the same ones the installer writes. `sed`, because the game image has no jq. +for f in /home/container/oxide/config/RunicGateway.json /home/container/carbon/configs/RunicGateway.json; do + [ -f "$f" ] || continue + ID="$(sed -n 's/^[[:space:]]*"ServerId":[[:space:]]*"\([^"]*\)".*$/\1/p' "$f" | head -n 1)" + if [ -n "$ID" ]; then + if [ -n "${RUSTLINK_SERVER_ID-}" ] && [ "$RUSTLINK_SERVER_ID" != "$ID" ]; then + echo "[rust-link] RUSTLINK_SERVER_ID is '$RUSTLINK_SERVER_ID' but the plugin's config names '$ID'; using '$ID', the id the website knows. The variable is read once, at the first boot." + fi + export RUSTLINK_SERVER_ID="$ID" + fi + break +done + # The website-facing bind. Pterodactyl tells a container nothing about its extra allocations, so # the port is typed into the egg, and it must be one of this server's allocations — a port that is # not fails as a bind the website never reaches, which the lines below make visible (§34.1). diff --git a/sidecar/src/app.rs b/sidecar/src/app.rs index 57763f1..10c5b85 100644 --- a/sidecar/src/app.rs +++ b/sidecar/src/app.rs @@ -87,7 +87,8 @@ where // Game link: events in, commands out. let (event_tx, mut event_rx) = mpsc::unbounded_channel::(); - let (handle, _bound) = game::serve(&cfg.game.bind, event_tx).await?; + let (handle, _bound) = + game::serve(&cfg.game.bind, cfg.game.server_id.clone(), event_tx).await?; // Live feed: every game event fans out to all connected website WebSocket clients. let (bcast_tx, _) = broadcast::channel::(1024); @@ -145,7 +146,6 @@ where // waiting caller and stop. Everything else is filed by its `type`, and by its `type` alone: // that is what keeps this process a dumb forwarder while the catalogue grows (PROTOCOL.md // §8.1). Ten new event kinds are no change here. - let configured_server_id = cfg.game.server_id.clone(); let feed_tx = bcast_tx.clone(); let route_rpc = rpc.clone(); let event_store = store.clone(); @@ -194,7 +194,6 @@ where // `type` exists to prevent, and it is invisible until somebody wonders why the // presence board has four thousand rows. if ev.kind == store::SERVER_BOARD { - check_server_id(&configured_server_id, &ev.value); info!(kind = %ev.kind, "{}", line); } else { tracing::debug!(kind = %ev.kind, "board {}", line); @@ -260,24 +259,6 @@ where Ok(()) } -/// The plugin's `serverId` is the authority; `[game].server_id` is a cross-check. A disagreement is -/// almost always two game servers pointed at one sidecar by a copied config, which is silent in -/// every other design and produces one server's history under another's name. -fn check_server_id(configured: &str, hello: &serde_json::Value) { - if configured.is_empty() { - return; - } - let announced = hello.get("serverId").and_then(|v| v.as_str()).unwrap_or(""); - if !announced.is_empty() && announced != configured { - warn!( - configured, - announced, - "the connected plugin announces a different serverId than this sidecar is configured \ - for; keeping the plugin's. Two servers sharing one sidecar is the usual cause." - ); - } -} - /// A frame's own timestamp, falling back to ours. The plugin stamps `t` at the moment the world was /// read, which is earlier than the moment we saw the line and is the one worth keeping. fn frame_t(value: &serde_json::Value) -> i64 { diff --git a/sidecar/src/config.rs b/sidecar/src/config.rs index 37cabeb..7d104ca 100644 --- a/sidecar/src/config.rs +++ b/sidecar/src/config.rs @@ -17,9 +17,11 @@ //! R8 makes the platform multi-server: one sidecar per game server, and every row the module //! stores carries the server it came from. The plugin declares its own `serverId` in `server.hello` //! and that is the authority. This setting is a **cross-check**, not a second source of truth: when -//! both are set and they disagree, the sidecar logs the disagreement loudly and keeps the plugin's. -//! Two servers pointed at one sidecar by a copy-pasted config is the mistake this catches, and it -//! is silent in every other design. +//! both are set and they disagree, the sidecar **refuses the plugin** — it closes the connection +//! before the frame is filed, and logs both ids at ERROR (D155). Two servers dialling one sidecar is +//! the mistake this catches, and it is silent in every other design. It was a warning that kept the +//! plugin's id until the phase 18 walk showed what that costs: one server's history filed under +//! another's name, visible on the website. Left blank, nothing is checked. use std::env; use std::fs; diff --git a/sidecar/src/game.rs b/sidecar/src/game.rs index 7d3faa3..8063add 100644 --- a/sidecar/src/game.rs +++ b/sidecar/src/game.rs @@ -23,7 +23,7 @@ use serde_json::Value; use tokio::io::{AsyncBufRead, AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::net::TcpListener; use tokio::sync::{mpsc, Mutex}; -use tracing::{info, warn}; +use tracing::{error, info, warn}; /// The longest line the sidecar will accept from the plugin, in bytes. /// @@ -181,8 +181,12 @@ impl GameHandle { /// The **bound** address is returned alongside the handle rather than assumed to be the one asked /// for: `127.0.0.1:0` is a legitimate thing to configure (and what the tests use), and a log line /// echoing the request rather than the result is the kind that is wrong exactly when it matters. +/// +/// `server_id` is `[game].server_id`. When it is set, a plugin that names another server is +/// refused — see [`foreign_server_id`]. pub async fn serve( addr: &str, + server_id: String, event_tx: mpsc::UnboundedSender, ) -> std::io::Result<(GameHandle, std::net::SocketAddr)> { let listener = TcpListener::bind(addr).await?; @@ -197,7 +201,9 @@ pub async fn serve( match listener.accept().await { Ok((stream, peer)) => { info!(%peer, "plugin connected"); - if let Err(e) = handle_connection(stream, &event_tx, &accept_handle).await { + if let Err(e) = + handle_connection(stream, &server_id, &event_tx, &accept_handle).await + { warn!(error = %e, "plugin connection ended"); } else { info!("plugin disconnected"); @@ -224,17 +230,44 @@ pub async fn serve( Ok((handle, bound)) } +/// The server a frame names, when it is not the one this sidecar is configured for. +/// +/// `None` means the frame may pass: no `server_id` is configured, or the frame names none, or it +/// names this one. The plugin stamps `serverId` on every frame, so the first line of a connection +/// is enough to tell — and the check runs before that line reaches the store or the feed, because +/// by the time it is filed, one server's history is already under another's name (D155). That is +/// what the phase 18 walk saw when this was a warning: a second server's plugin, parked in this +/// listener's backlog, was accepted the moment the first one reloaded, and the website showed the +/// first server with the second's hostname and wipe. +fn foreign_server_id<'a>(configured: &str, frame: &'a Value) -> Option<&'a str> { + if configured.is_empty() { + return None; + } + match frame.get("serverId").and_then(|v| v.as_str()) { + Some(announced) if !announced.is_empty() && announced != configured => Some(announced), + _ => None, + } +} + async fn handle_connection( stream: tokio::net::TcpStream, + server_id: &str, event_tx: &mpsc::UnboundedSender, handle: &GameHandle, ) -> std::io::Result<()> { stream.set_nodelay(true).ok(); let (read_half, mut write_half) = stream.into_split(); - // Install the outbound command channel for this connection. + // The outbound command channel for this connection. With a `server_id` configured it is + // installed only once a frame has named this server: until then the peer could be a plugin + // about to be refused, and a command sent in that window — a permission grant, a world write — + // would land on the wrong server. `/health` reads the same handle, so it reports the plugin + // connected only from that moment too. let (cmd_tx, mut cmd_rx) = mpsc::unbounded_channel::(); - handle.set(Some(cmd_tx)).await; + let mut pending_tx = Some(cmd_tx); + if server_id.is_empty() { + handle.set(pending_tx.take()).await; + } let mut reader = BufReader::new(read_half); let mut lines = LineReader::default(); @@ -258,6 +291,24 @@ async fn handle_connection( if !trimmed.is_empty() { match serde_json::from_str::(trimmed) { Ok(value) => { + if let Some(announced) = foreign_server_id(server_id, &value) { + error!( + configured = server_id, + announced, + "refusing a plugin that names another server: this \ + sidecar is '{server_id}', the plugin says \ + '{announced}'. Two servers are dialling one game \ + port — check each plugin config's Port against its \ + sidecar's [game].bind" + ); + return Ok(()); + } + if pending_tx.is_some() + && value.get("serverId").and_then(|v| v.as_str()) + == Some(server_id) + { + handle.set(pending_tx.take()).await; + } let kind = value .get("kind") .and_then(|k| k.as_str()) @@ -396,7 +447,7 @@ mod tests { use tokio::io::AsyncReadExt; let (tx, mut rx) = mpsc::unbounded_channel(); - let (handle, addr) = serve("127.0.0.1:0", tx).await.unwrap(); + let (handle, addr) = serve("127.0.0.1:0", String::new(), tx).await.unwrap(); let mut client = tokio::net::TcpStream::connect(addr).await.unwrap(); client @@ -415,4 +466,67 @@ mod tests { let n = client.read(&mut buf).await.unwrap(); assert_eq!(&buf[..n], b"{\"cmd\":\"ping\"}\n"); } + + #[test] + fn foreign_server_id_names_only_a_disagreement() { + let beta = serde_json::json!({ "kind": "server.hello", "serverId": "beta" }); + let alpha = serde_json::json!({ "kind": "server.hello", "serverId": "alpha" }); + let anonymous = serde_json::json!({ "kind": "pong" }); + let blank = serde_json::json!({ "kind": "pong", "serverId": "" }); + + assert_eq!(foreign_server_id("alpha", &beta), Some("beta")); + assert_eq!(foreign_server_id("alpha", &alpha), None); + assert_eq!(foreign_server_id("alpha", &anonymous), None); + assert_eq!(foreign_server_id("alpha", &blank), None); + // Nothing configured: nothing to check against. + assert_eq!(foreign_server_id("", &beta), None); + } + + /// D155, over a real socket: the plugin that names another server is closed on its first + /// frame, the frame is never forwarded, and no command could have reached it meanwhile. + #[tokio::test] + async fn a_plugin_naming_another_server_is_refused_before_anything_is_filed() { + use tokio::io::AsyncReadExt; + + let (tx, mut rx) = mpsc::unbounded_channel(); + let (handle, addr) = serve("127.0.0.1:0", "alpha".to_string(), tx).await.unwrap(); + + let mut client = tokio::net::TcpStream::connect(addr).await.unwrap(); + // Accepted, but not yet vetted: no command may go to it. + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + assert!(!handle.is_connected().await); + + client + .write_all(b"{\"kind\":\"server.hello\",\"type\":\"board\",\"serverId\":\"beta\"}\n") + .await + .unwrap(); + + let mut buf = [0u8; 16]; + let n = tokio::time::timeout(std::time::Duration::from_secs(2), client.read(&mut buf)) + .await + .expect("the sidecar closes a refused plugin") + .unwrap_or(0); + assert_eq!(n, 0); + + // The only thing forwarded is the listener's own link.down, never beta's hello. + let ev = rx.recv().await.unwrap(); + assert_eq!(ev.kind, "link.down"); + assert!(!handle.is_connected().await); + } + + #[tokio::test] + async fn the_command_channel_opens_on_the_first_frame_naming_this_server() { + let (tx, mut rx) = mpsc::unbounded_channel(); + let (handle, addr) = serve("127.0.0.1:0", "alpha".to_string(), tx).await.unwrap(); + + let mut client = tokio::net::TcpStream::connect(addr).await.unwrap(); + client + .write_all(b"{\"kind\":\"server.hello\",\"serverId\":\"alpha\"}\n") + .await + .unwrap(); + + let ev = rx.recv().await.unwrap(); + assert_eq!(ev.value["serverId"], "alpha"); + assert!(handle.is_connected().await); + } }