From 6b3e793b394ef3e1c53a4eaaa78e4732d95da4b7 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 15 Aug 2026 10:47:55 +0200 Subject: [PATCH] fix(gamestream): bind the UDP/ENet media plane to the launch owner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit security-review 2026-08-15 findings 1, 2, 13. The Moonlight-compat plane bound its UDP video/audio endpoints to the first datagram from anyone and let any ENet peer keep a connection (pinning per-peer reassembly memory) — the peer_ip the RTSP/launch planes already enforce was never threaded to the media/control sockets. - stream.rs/audio.rs: the video/audio endpoint learn now discards datagrams whose source IP is not the launch owner's until the 10s budget is spent, so an off-path LAN peer can no longer win the endpoint race and be handed the (plaintext) video stream. - control.rs: an OwnerFilteredSocket drops non-owner datagrams before ENet allocates any per-peer state (closes the ~32 MiB x peer_limit pin and the source-spoof injection variant), and the Event::Receive arm now honors only the tracked session peer's input as defense-in-depth. GameStream is runtime opt-in and off in the shipped unit, so this is deferrable but the code's own comments claimed a peer bind already protected these paths. --- crates/punktfunk-host/src/gamestream/audio.rs | 31 +++++++-- .../punktfunk-host/src/gamestream/control.rs | 67 ++++++++++++++++++- crates/punktfunk-host/src/gamestream/rtsp.rs | 4 ++ .../punktfunk-host/src/gamestream/stream.rs | 29 +++++++- 4 files changed, 122 insertions(+), 9 deletions(-) diff --git a/crates/punktfunk-host/src/gamestream/audio.rs b/crates/punktfunk-host/src/gamestream/audio.rs index 58069c25..0ca6bb83 100644 --- a/crates/punktfunk-host/src/gamestream/audio.rs +++ b/crates/punktfunk-host/src/gamestream/audio.rs @@ -218,12 +218,15 @@ pub fn start( params: AudioParams, audio_cap: AudioCapSlot, on_lost: super::OnSessionLost, + owner_ip: Option, ) { let _ = std::thread::Builder::new() .name("punktfunk-audio".into()) .spawn(move || { tracing::info!(?params, "audio stream starting"); - if let Err(e) = run(&running, &gcm_key, rikeyid, params, &audio_cap, &on_lost) { + if let Err(e) = run( + &running, &gcm_key, rikeyid, params, &audio_cap, &on_lost, owner_ip, + ) { tracing::error!(error = %format!("{e:#}"), "audio stream failed"); } running.store(false, Ordering::SeqCst); @@ -243,6 +246,7 @@ pub fn start( _params: AudioParams, _audio_cap: AudioCapSlot, _on_lost: super::OnSessionLost, + _owner_ip: Option, ) { tracing::error!("GameStream audio requires Linux (PipeWire) or Windows (WASAPI) + libopus"); running.store(false, std::sync::atomic::Ordering::SeqCst); @@ -256,6 +260,7 @@ fn run( params: AudioParams, audio_cap: &std::sync::Mutex>>, on_lost: &super::OnSessionLost, + owner_ip: Option, ) -> Result<()> { let sock = UdpSocket::bind(("0.0.0.0", AUDIO_PORT)).context("bind audio UDP")?; // Grow SO_SNDBUF/RCVBUF; the opt-in DSCP/QoS tag happens after connect below (Windows @@ -265,9 +270,27 @@ fn run( sock.set_read_timeout(Some(Duration::from_secs(10)))?; tracing::debug!(port = AUDIO_PORT, "audio: awaiting client ping"); let mut probe = [0u8; 256]; - let (_, client) = sock - .recv_from(&mut probe) - .context("audio: no client ping within 10s")?; + // Same owner-IP bind as the video plane (LaunchSession::peer_ip): only the launching peer's + // pings are honored, so an off-path LAN peer cannot capture the audio endpoint (a DoS here, as + // audio payload is AES-CBC under `rikey`). `None` keeps the pre-owner behavior. + // security-review 2026-08-15 finding 1. + let client = { + let deadline = std::time::Instant::now() + Duration::from_secs(10); + loop { + let remaining = deadline.saturating_duration_since(std::time::Instant::now()); + if remaining.is_zero() { + anyhow::bail!("audio: no client ping from the launch owner within 10s"); + } + sock.set_read_timeout(Some(remaining))?; + let (_, src) = sock + .recv_from(&mut probe) + .context("audio: no client ping within 10s")?; + if owner_ip.is_some_and(|ip| ip != src.ip()) { + continue; + } + break src; + } + }; sock.connect(client) .context("connect client audio endpoint")?; // Opt-in DSCP/QoS-tag this as the audio class (PUNKTFUNK_DSCP=1); the guard keeps the diff --git a/crates/punktfunk-host/src/gamestream/control.rs b/crates/punktfunk-host/src/gamestream/control.rs index d3217dc3..eead98d0 100644 --- a/crates/punktfunk-host/src/gamestream/control.rs +++ b/crates/punktfunk-host/src/gamestream/control.rs @@ -119,6 +119,54 @@ pub(crate) fn sync(state: &Arc) -> Result<()> { } } +/// A [`rusty_enet::Socket`] that drops datagrams whose source IP is not the launch owner's. +/// +/// `rusty_enet` 0.4.0 exposes no setter for `maximum_waiting_data` (the C default of 32 MiB of +/// per-peer reassembly), so an off-path LAN peer that connects on 47999 can pin ~32 MiB × the +/// `peer_limit` and occupy peer slots without ever authenticating — and the same unfiltered path +/// lets an on-path attacker spoof the owner's source to feed the tracked peer. Filtering at the +/// socket drops those datagrams BEFORE ENet allocates any per-peer state. The owner is read live +/// from `launch` on each receive: before `/launch` (owner `None`) the filter passes everything, +/// matching the plane's existing "trust the connect when no owner is captured" fallback used by +/// the `Event::Connect` arm below. security-review 2026-08-15 findings 2 and 13. +struct OwnerFilteredSocket { + inner: UdpSocket, + state: Arc, +} + +impl rusty_enet::Socket for OwnerFilteredSocket { + type Address = std::net::SocketAddr; + type Error = std::io::Error; + + fn init(&mut self, opts: rusty_enet::SocketOptions) -> Result<(), std::io::Error> { + rusty_enet::Socket::init(&mut self.inner, opts) + } + + fn send(&mut self, address: Self::Address, buffer: &[u8]) -> Result { + rusty_enet::Socket::send(&mut self.inner, address, buffer) + } + + fn receive( + &mut self, + buffer: &mut [u8; rusty_enet::MTU_MAX], + ) -> Result, std::io::Error> { + // Loop so a dropped non-owner datagram doesn't starve a following owner datagram in the + // same drain; the inner socket is non-blocking, so this returns `Ok(None)` on WouldBlock. + loop { + match rusty_enet::Socket::receive(&mut self.inner, buffer)? { + Some((addr, received)) => { + let owner = self.state.launch.lock().unwrap().and_then(|s| s.peer_ip); + if owner.is_some_and(|ip| ip != addr.ip()) { + continue; + } + return Ok(Some((addr, received))); + } + None => return Ok(None), + } + } + } +} + /// 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")?; @@ -126,7 +174,10 @@ fn spawn(state: Arc) -> Result { .set_nonblocking(true) .context("control socket nonblocking")?; let mut host = Host::new( - socket, + OwnerFilteredSocket { + inner: socket, + state: state.clone(), + }, HostSettings { peer_limit: 4, // Moonlight connects with CTRL_CHANNEL_COUNT (0x30) channels and sends gamepad @@ -252,8 +303,20 @@ fn spawn(state: Arc) -> Result { state.end_session("control stream disconnected"); } Event::Receive { - channel_id, packet, .. + peer: p, + channel_id, + packet, } => { + // Only the tracked session peer's input is honored. The owner-IP + // socket filter already drops non-owner datagrams once a launch is + // recorded; this is defense-in-depth for the window before the + // owner is captured (and mirrors the `Disconnect` arm's gate) so a + // peer that connected while `owner_ip` was `None` still cannot + // inject keyboard/mouse/gamepad after another peer became the + // session. security-review 2026-08-15 finding 2. + if peer != Some(p.id()) { + continue; + } on_receive( &state, channel_id, diff --git a/crates/punktfunk-host/src/gamestream/rtsp.rs b/crates/punktfunk-host/src/gamestream/rtsp.rs index 75c35ec9..0ce70ddb 100644 --- a/crates/punktfunk-host/src/gamestream/rtsp.rs +++ b/crates/punktfunk-host/src/gamestream/rtsp.rs @@ -286,6 +286,7 @@ fn handle_request(req: &Request, state: &Arc, peer: Option stream::GameLifetime { quit: state.quit.clone(), fingerprint: ls.owner_fp.map(hex::encode), + owner_ip: ls.peer_ip, on_game_exit: { let st = state.clone(); Arc::new(move || { @@ -310,6 +311,9 @@ fn handle_request(req: &Request, state: &Arc, peer: Option *state.audio_params.lock().unwrap(), state.audio_cap.clone(), on_lost, + // Same owner-IP bind as the video plane: only the launching peer's pings are + // honored at the audio endpoint. security-review 2026-08-15 finding 1. + ls.peer_ip, ); } response(&req.cseq, &[("Session", "DEADBEEFCAFE;timeout = 90")], None) diff --git a/crates/punktfunk-host/src/gamestream/stream.rs b/crates/punktfunk-host/src/gamestream/stream.rs index af4f857f..3b0668ed 100644 --- a/crates/punktfunk-host/src/gamestream/stream.rs +++ b/crates/punktfunk-host/src/gamestream/stream.rs @@ -66,6 +66,11 @@ pub struct GameLifetime { /// Hex cert fingerprint of the paired client that owns the launch, so only it can reclaim its own /// game. `None` when the peer cert couldn't be read. pub fingerprint: Option, + /// Source IP of the launching peer ([`super::LaunchSession::peer_ip`]), enforced when the video + /// thread learns its UDP endpoint so an off-path LAN peer cannot win the endpoint race and be + /// handed the (plaintext) video stream. `None` keeps the pre-owner behavior. security-review + /// 2026-08-15 finding 1. + pub owner_ip: Option, /// Ends the whole session, deliberately — the action for "the launched game exited". pub on_game_exit: super::OnSessionLost, } @@ -193,9 +198,27 @@ fn run( "video: awaiting client ping to learn endpoint" ); let mut probe = [0u8; 256]; - let (_, client) = sock - .recv_from(&mut probe) - .context("video: no client ping within 10s")?; + // Bind only to the launch owner's source IP (LaunchSession::peer_ip), the same owner the + // RTSP/ENet planes enforce. Video is plaintext by design, so without this an off-path LAN peer + // trickling UDP at this port wins the endpoint race in `recv_from` and is handed the desktop. + // `None` keeps the pre-owner behavior. security-review 2026-08-15 finding 1. + let client = { + let deadline = std::time::Instant::now() + Duration::from_secs(10); + loop { + let remaining = deadline.saturating_duration_since(std::time::Instant::now()); + if remaining.is_zero() { + anyhow::bail!("video: no client ping from the launch owner within 10s"); + } + sock.set_read_timeout(Some(remaining))?; + let (_, src) = sock + .recv_from(&mut probe) + .context("video: no client ping within 10s")?; + if life.owner_ip.is_some_and(|ip| ip != src.ip()) { + continue; + } + break src; + } + }; sock.connect(client) .context("connect client video endpoint")?; // Opt-in DSCP/QoS-tag this as the video class (PUNKTFUNK_DSCP=1); the guard keeps the