From 13d5721049fe01a780b9c2f3c21de4bafeabc0fc Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 11 Aug 2026 19:29:50 +0200 Subject: [PATCH] feat(gamestream): the ENet control port exists only while a pairing does (WP0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rusty_enet — a c2rust-style transpile of C ENet, 158 unsafe sites — parsed unauthenticated UDP on 47999 from GameStream startup, before any client had ever paired: the host's entire pre-auth-reachable unsafe surface. Pairing itself is HTTPS on nvhttp and never touches the port, so it now binds only while the paired-client list is non-empty: a Gate in control.rs reconciles the port to the list (armed only under --gamestream), pairing phase 4 brings it up before the new client can /launch, and removing the last pairing tears it down — a live client gets the same termination+disconnect farewell as a host-side session end. A never-paired host on a hostile LAN exposes no ENet. En route: the management API's unpair never called save_paired, so a restart resurrected the client — and would now have silently re-opened the port; it persists (the test now runs against a throwaway PUNKTFUNK_CONFIG_DIR so it can't clobber a real paired.json). rusty_enet is pinned =0.4.0 per the WP, left to the cargo-audit job to flag advisories against it. Gate (amd64 container): clippy --all-targets -D warnings clean; gamestream::control 6/6; mgmt::tests 37/37 incl. the regenerated api/openapi.json. On-box .133 verification (ports/pair/stream) still owed. --- api/openapi.json | 2 +- crates/punktfunk-host/Cargo.toml | 7 +- .../punktfunk-host/src/gamestream/control.rs | 127 +++++++++++++++++- crates/punktfunk-host/src/gamestream/mod.rs | 20 ++- .../punktfunk-host/src/gamestream/nvhttp.rs | 10 +- crates/punktfunk-host/src/mgmt/clients.rs | 17 ++- crates/punktfunk-host/src/mgmt/tests.rs | 25 ++++ 7 files changed, 196 insertions(+), 12 deletions(-) diff --git a/api/openapi.json b/api/openapi.json index df25b2cf..e9d1efb7 100644 --- a/api/openapi.json +++ b/api/openapi.json @@ -53,7 +53,7 @@ "clients" ], "summary": "Unpair a client", - "description": "Removes the client's certificate from the pairing store. Caveat: the nvhttp TLS layer\ndoes not yet reject unlisted certificates (`gamestream/tls.rs` accepts any well-formed\nclient cert — a planned hardening step), so until that lands this removes the client\nfrom the listing without severing its ability to reconnect.", + "description": "Removes the client's certificate from the pairing store (persisted — the removal survives a\nhost restart). Removing the last pairing also closes the GameStream ENet control port\n(UDP 47999), which is only bound while at least one pairing exists. Caveat: the nvhttp TLS\nlayer does not yet reject unlisted certificates (`gamestream/tls.rs` accepts any well-formed\nclient cert — a planned hardening step), so until that lands this removes the client\nfrom the listing without severing its ability to reconnect.", "operationId": "unpairClient", "parameters": [ { diff --git a/crates/punktfunk-host/Cargo.toml b/crates/punktfunk-host/Cargo.toml index 5984dd9e..10cb5ddb 100644 --- a/crates/punktfunk-host/Cargo.toml +++ b/crates/punktfunk-host/Cargo.toml @@ -105,7 +105,12 @@ futures-util = "0.3" # Webhook signing (X-Punktfunk-Signature: sha256=) for operator hooks; pairs with # the existing sha2. Already in the lockfile transitively. hmac = "0.12" -rusty_enet = "0.4" +# GameStream control-stream ENet — a c2rust-style transpile of C ENet (158 unsafe sites; raw +# pointer arithmetic, manual allocation). Its port binds only while a pairing exists (rust-safety +# WP0, gamestream/control.rs), but the crate stays a transpiled C stack on the paired-client path: +# pinned EXACTLY so a bump is a deliberate, reviewed act rather than a lockfile refresh, and left +# to the cargo-audit job (audit.yml) to flag advisories against it. +rusty_enet = "=0.4.0" serde = { version = "1", features = ["derive"] } serde_json = "1" # Management/control-plane REST API + OpenAPI (control pane, M2). `axum_extras` wires diff --git a/crates/punktfunk-host/src/gamestream/control.rs b/crates/punktfunk-host/src/gamestream/control.rs index bd23f550..001b6366 100644 --- a/crates/punktfunk-host/src/gamestream/control.rs +++ b/crates/punktfunk-host/src/gamestream/control.rs @@ -20,7 +20,12 @@ //! `hex::decode(rikey)`. We auto-detect the exact scheme via [`decrypt_control`] on the first //! packet that authenticates, since GCM gives no partial credit. //! -//! Runs on its own native thread for the host's lifetime. +//! Runs on its own native thread — but only while at least one client is paired. `rusty_enet` +//! is a c2rust-style transpile of C ENet (raw-pointer arithmetic, manual allocation), and its +//! fragment reassembly / peer state machine run BEFORE the AES-GCM decrypt below — the host's +//! only pre-authentication unsafe surface (rust-safety WP0). Pairing itself never touches +//! 47999 (the PIN ceremony is HTTPS on nvhttp), so [`sync`] keeps the port closed until the +//! first pairing lands and tears it down when the last one is removed. use super::{AppState, CONTROL_PORT}; use crate::inject::gamepad::GamepadManager; @@ -29,12 +34,93 @@ use punktfunk_core::input::InputEvent; use punktfunk_core::quic::HdrMeta; use rusty_enet::{Event, Host, HostSettings, Packet, PeerID}; use std::net::UdpSocket; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::Sender; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use std::time::Duration; -/// Bind the ENet control host on 47999 and service it forever on a dedicated thread. -pub fn spawn(state: Arc) -> Result<()> { +/// Lifecycle gate for the control port (rust-safety WP0): binds 47999 only while the +/// paired-client list is non-empty, so a never-paired host exposes no ENet at all. +pub(crate) struct Gate { + /// Set once by `serve` when the GameStream planes are enabled (`--gamestream`). Without it + /// [`sync`] is a no-op — the management API's unpair endpoint also runs on native-only + /// hosts, and those must never bind a GameStream port. + enabled: AtomicBool, + /// The live listener; `None` = port closed. The mutex serializes concurrent reconciles + /// (two pairings in quick succession must not double-bind), and the bind/teardown + /// decision re-reads the paired list inside it so a pair racing an unpair cannot leave + /// the port in the wrong state. + running: Mutex>, +} + +impl Gate { + pub(crate) fn new() -> Gate { + Gate { + enabled: AtomicBool::new(false), + running: Mutex::new(None), + } + } + + /// Arm the gate — [`sync`] stays a no-op until this is called (from `serve`'s + /// GameStream branch, the single existing source of truth for "compat planes on"). + pub(crate) fn enable(&self) { + self.enabled.store(true, Ordering::SeqCst); + } +} + +/// A bound control port being serviced: the stop signal plus the thread observing it. +struct Running { + /// Tells the service thread to say goodbye to a connected peer and exit (closing the + /// socket with it). + stop: Arc, + thread: std::thread::JoinHandle<()>, +} + +/// Reconcile the control port to the paired-client list: bound while at least one pairing +/// exists, closed when none remain. Idempotent and race-free (see [`Gate::running`]); call it +/// wherever the paired list changes — startup, pairing phase 4, unpair. +pub(crate) fn sync(state: &Arc) -> Result<()> { + let gate = &state.control_gate; + if !gate.enabled.load(Ordering::SeqCst) { + return Ok(()); + } + let mut slot = gate.running.lock().unwrap_or_else(|e| e.into_inner()); + // Reap a listener whose thread died (a panic would otherwise leave a Running that + // serves nobody and blocks every future rebind). + if slot.as_ref().is_some_and(|r| r.thread.is_finished()) { + if let Some(r) = slot.take() { + let _ = r.thread.join(); + } + } + let want = !state + .paired + .lock() + .unwrap_or_else(|e| e.into_inner()) + .is_empty(); + match (slot.is_some(), want) { + (false, true) => { + *slot = Some(spawn(state.clone())?); + Ok(()) + } + (true, false) => { + let r = slot.take().expect("slot checked non-empty"); + r.stop.store(true, Ordering::SeqCst); + // Join before returning: it guarantees the socket is closed before a re-pair can + // ask for a rebind. Bounded — the loop ticks every 2 ms, plus a ~100 ms farewell + // flush when a client was connected — and unpair-all is a rare operator action. + let _ = r.thread.join(); + tracing::info!( + port = CONTROL_PORT, + "ENet control torn down — no paired clients remain" + ); + Ok(()) + } + _ => Ok(()), + } +} + +/// Bind the ENet control host on 47999 and service it on a dedicated thread until `stop`. +fn spawn(state: Arc) -> Result { let socket = UdpSocket::bind(("0.0.0.0", CONTROL_PORT)).context("bind control UDP")?; socket .set_nonblocking(true) @@ -52,7 +138,9 @@ pub fn spawn(state: Arc) -> Result<()> { .map_err(|e| anyhow!("ENet host init: {e:?}"))?; tracing::info!(port = CONTROL_PORT, "ENet control listening"); - std::thread::Builder::new() + let stop = Arc::new(AtomicBool::new(false)); + let stop_seen = stop.clone(); + let thread = std::thread::Builder::new() .name("punktfunk-control".into()) .spawn(move || { // GCM scheme detected from the first authenticating packet; reused thereafter. @@ -83,6 +171,33 @@ pub fn spawn(state: Arc) -> Result<()> { // has to go out *because* the session ended could no longer be sealed. let mut last_key: Option<[u8; 16]> = None; loop { + // WP0 teardown: the last pairing was removed while we were live. Tell a + // connected client the session is over — termination + disconnect, the same + // farewell the host-side session end below uses — rather than vanish on it, + // flush briefly so the disconnect actually reaches the wire, then end the + // session and exit. Dropping `host` closes the socket. + if stop_seen.load(Ordering::SeqCst) { + if let Some(pid) = peer { + if let (Some(scheme), Some(key)) = (detected, last_key) { + let pt = termination_plaintext(); + let wire = encrypt_control(&key, &scheme, host_seq, &pt); + if let Err(e) = host.peer_mut(pid).send(0, &Packet::reliable(&wire[..])) + { + tracing::warn!(error = ?e, "control: termination send failed"); + } + } + host.peer_mut(pid).disconnect_later(0); + // Bounded flush: enough ticks for ENet to emit the termination and + // the disconnect handshake; we are exiting either way. + for _ in 0..50 { + while matches!(host.service(), Ok(Some(_))) {} + std::thread::sleep(Duration::from_millis(2)); + } + } + state.end_session("control stream stopped — last pairing removed"); + tracing::info!(port = CONTROL_PORT, "control: stopped (no paired clients)"); + return; + } loop { match host.service() { Ok(Some(event)) => match event { @@ -282,7 +397,7 @@ pub fn spawn(state: Arc) -> Result<()> { } }) .context("spawn control thread")?; - Ok(()) + Ok(Running { stop, thread }) } /// Decode the lost-frame range from an invalidate-reference-frames (0x0301) control message: two diff --git a/crates/punktfunk-host/src/gamestream/mod.rs b/crates/punktfunk-host/src/gamestream/mod.rs index a4f06e67..8d0be637 100644 --- a/crates/punktfunk-host/src/gamestream/mod.rs +++ b/crates/punktfunk-host/src/gamestream/mod.rs @@ -174,6 +174,9 @@ pub struct AppState { pub pairing: pairing::Pairing, /// Pinned (paired) client certificate DERs — the post-pair allow-list. pub paired: std::sync::Mutex>>, + /// The ENet control port's lifecycle gate (rust-safety WP0): 47999 is bound only while + /// `paired` is non-empty — see [`control::sync`] / [`sync_control`]. + pub(crate) control_gate: control::Gate, /// The active launch session (set by `/launch`, consumed by RTSP/media). pub launch: std::sync::Mutex>, /// Negotiated video config from RTSP ANNOUNCE (consumed by the stream on PLAY). @@ -282,6 +285,7 @@ impl AppState { identity, pairing: pairing::Pairing::new(), paired: std::sync::Mutex::new(load_paired()), + control_gate: control::Gate::new(), launch: std::sync::Mutex::new(None), stream: std::sync::Mutex::new(None), audio_params: std::sync::Mutex::new(audio::AudioParams::default()), @@ -297,6 +301,14 @@ impl AppState { } } +/// Reconcile the ENet control port to the paired-client list — bound iff at least one pairing +/// exists (rust-safety WP0; see [`control::sync`]). A crate-visible wrapper so callers outside +/// `gamestream` (the management API's unpair) can reach it past the private `control` module. +/// A no-op unless `serve` armed the gate (`--gamestream`). +pub(crate) fn sync_control(state: &Arc) -> Result<()> { + control::sync(state) +} + /// Run the host (blocks): mDNS, the nvhttp servers, and the management REST API. /// `native = Some(cfg)` makes this the **unified** host — it also runs the native punktfunk/1 /// QUIC server on `cfg.port` in the same process, sharing one [`crate::native_pairing`] handle with @@ -392,7 +404,13 @@ pub fn serve( None }; rtsp::spawn(state.clone()).context("start RTSP server")?; - control::spawn(state.clone()).context("start ENet control server")?; + // WP0 (rust-safety): the ENet control port is the host's one pre-auth-reachable + // unsafe surface (`rusty_enet` is a transpiled C stack), so it binds only while a + // pairing exists — a never-paired host on a hostile LAN exposes no ENet at all. + // Pairing is HTTPS on nvhttp and never touches 47999; phase 4 re-syncs the port the + // moment the first client pins, so it is up before that client can `/launch`. + state.control_gate.enable(); + sync_control(&state).context("start ENet control server")?; tracing::info!( port = native.port, "unified host: GameStream/Moonlight compat + native punktfunk/1 (QUIC)" diff --git a/crates/punktfunk-host/src/gamestream/nvhttp.rs b/crates/punktfunk-host/src/gamestream/nvhttp.rs index 4c0e4af2..b0a61789 100644 --- a/crates/punktfunk-host/src/gamestream/nvhttp.rs +++ b/crates/punktfunk-host/src/gamestream/nvhttp.rs @@ -389,7 +389,15 @@ async fn h_pair( } else if let Some(v) = q.get("serverchallengeresp") { st.pairing.serverchallengeresp(&st.identity, &uniqueid, v) } else if let Some(v) = q.get("clientpairingsecret") { - st.pairing.clientpairingsecret(&uniqueid, v, &st.paired) + let r = st.pairing.clientpairingsecret(&uniqueid, v, &st.paired); + // Phase 4 may just have pinned the FIRST pairing — bring the ENet control port up now + // (idempotent; rust-safety WP0) so this client's imminent /launch finds the control + // stream listening. Moonlight connects control before video, so "eventually up" would + // be an aborted session. + if let Err(e) = super::sync_control(&st) { + tracing::warn!(error = %format!("{e:#}"), "control port sync after pairing failed"); + } + r } else { Ok(pair_error_xml()) }; diff --git a/crates/punktfunk-host/src/mgmt/clients.rs b/crates/punktfunk-host/src/mgmt/clients.rs index e84dfb01..b6f21ccb 100644 --- a/crates/punktfunk-host/src/mgmt/clients.rs +++ b/crates/punktfunk-host/src/mgmt/clients.rs @@ -76,8 +76,10 @@ pub(crate) fn client_info(der: &[u8]) -> PairedClient { /// Unpair a client /// -/// Removes the client's certificate from the pairing store. Caveat: the nvhttp TLS layer -/// does not yet reject unlisted certificates (`gamestream/tls.rs` accepts any well-formed +/// Removes the client's certificate from the pairing store (persisted — the removal survives a +/// host restart). Removing the last pairing also closes the GameStream ENet control port +/// (UDP 47999), which is only bound while at least one pairing exists. Caveat: the nvhttp TLS +/// layer does not yet reject unlisted certificates (`gamestream/tls.rs` accepts any well-formed /// client cert — a planned hardening step), so until that lands this removes the client /// from the listing without severing its ability to reconnect. #[utoipa::path( @@ -110,6 +112,17 @@ pub(crate) async fn unpair_client( let before = paired.len(); paired.retain(|der| !hex::encode(Sha256::digest(der)).eq_ignore_ascii_case(&fingerprint)); if paired.len() < before { + // Persist the removal — without this the unpair lasted only until the next host + // restart, which now also matters below: a resurrected pairing would silently + // re-open the control port. + crate::gamestream::save_paired(&paired); + drop(paired); + // The last pairing going away closes the ENet control port (rust-safety WP0). A + // no-op while other pairings remain — or on a native-only host, where the gate is + // never armed. + if let Err(e) = crate::gamestream::sync_control(&st.app) { + tracing::warn!(error = %format!("{e:#}"), "control port sync after unpair failed"); + } tracing::info!(fingerprint, "management API: client unpaired"); StatusCode::NO_CONTENT.into_response() } else { diff --git a/crates/punktfunk-host/src/mgmt/tests.rs b/crates/punktfunk-host/src/mgmt/tests.rs index cc2ff8cb..855f5e48 100644 --- a/crates/punktfunk-host/src/mgmt/tests.rs +++ b/crates/punktfunk-host/src/mgmt/tests.rs @@ -763,6 +763,22 @@ async fn status_reflects_runtime_state() { #[tokio::test] async fn paired_clients_list_and_unpair() { + // Unpair PERSISTS (save_paired → paired.json in the config dir), so point the config dir + // at a throwaway tempdir — this test must never rewrite the dev box's real pairing store. + // The guard restores the previous value even if an assertion below panics. + struct EnvGuard(Option); + impl Drop for EnvGuard { + fn drop(&mut self) { + match self.0.take() { + Some(v) => std::env::set_var("PUNKTFUNK_CONFIG_DIR", v), + None => std::env::remove_var("PUNKTFUNK_CONFIG_DIR"), + } + } + } + let tmp = tempfile::tempdir().unwrap(); + let _env = EnvGuard(std::env::var_os("PUNKTFUNK_CONFIG_DIR")); + std::env::set_var("PUNKTFUNK_CONFIG_DIR", tmp.path()); + let state = test_state(); let app = test_app(state.clone(), None); @@ -803,6 +819,15 @@ async fn paired_clients_list_and_unpair() { let (_, body) = send(&app, get_req("/api/v1/clients")).await; assert_eq!(body, serde_json::json!([])); assert_eq!(send(&app, del(fingerprint)).await.0, StatusCode::NOT_FOUND); + + // The unpair persisted: paired.json in the (test-scoped) config dir holds the emptied + // list — a restart must not resurrect the pairing (it would re-open the control port). + // (`PUNKTFUNK_CONFIG_DIR` is used verbatim — no `punktfunk` subdirectory appended.) + let disk = std::fs::read(tmp.path().join("paired.json")).expect("unpair persisted paired.json"); + assert_eq!( + serde_json::from_slice::>>(&disk).unwrap(), + Vec::>::new() + ); } #[tokio::test]