fix(host/security): close audit findings S1,#1,#4,#10,#12,#7,#6,S2-S6 (Linux/cross-platform)

Remediations from design/security-review-2026-06-28.md verified on Linux (cargo check/clippy/test green; Windows-gated paths verify in CI): - S1 [HIGH]: bump quinn-proto 0.11.14 -> 0.11.15 (RUSTSEC-2026-0185, pre-auth out-of-order STREAM reassembly memory exhaustion on the always-on default QUIC listener). - #1 [HIGH]: remove the unauthenticated nvhttp `GET /pin` endpoint; the GameStream PIN is delivered ONLY via the bearer-gated mgmt API, so a network client can no longer submit its own displayed PIN and self-pair. - #4 [HIGH->MED]: gate the unauthenticated RTSP/UDP media plane on a paired `/launch` and bind it to the launching client's source IP (threaded through the HTTPS handler), so an unpaired peer can neither start capture on an idle host nor ride a paired client's active launch. - #12: bound concurrent parked pairing waiters (MAX_PARKED_WAITERS) so a pre-auth peer can't pin unbounded 300s handshakes. +regression test. - #10: throttle the per-packet ENet control GCM-decrypt-failed warn (exponential backoff) so a junk flood can't spam the log. - #7 [MED->LOW]: serialize all process-global env mutation on the session-setup path under a new vdisplay::ENV_LOCK (apply_session_env / apply_input_env / the launch-cmd set_var / the gamescope env read), so concurrent native sessions can't race set_var/getenv (data-race UB -> host-wide DoS). Full per-session SessionContext threading remains a follow-up for cross-session value confusion. - #6 [MED]: move the gamescope EIS socket relay from world-writable /tmp to $XDG_RUNTIME_DIR (per-user 0700) and reject a symlinked relay file, so a local user can't intercept (keylog) or deny the remote session's input. - S2: a malformed client Opus mic frame now drops that frame instead of tearing down the shared host-lifetime virtual mic (cross-session DoS). - S3: track held buttons/keys in capped HashSets (was unbounded Vec with O(n) scans) so a paired client can't grow per-session input state. - S5: reject fps==0/absurd at the open_video chokepoint (covers Hello, ANNOUNCE, Reconfigure) so the encoder time_base/pts math can't div-by-0. - S6: bound the shared mic mpsc (drop-newest when full). - S4: cap Epic launcher-cache reads (catcache.bin/.item) so a planted giant can't OOM the host during library enumeration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-28 22:06:24 +00:00
parent 6b846913f5
commit 3532e35b75
14 changed files with 280 additions and 98 deletions
@@ -56,6 +56,9 @@ pub fn spawn(state: Arc<AppState>) -> Result<()> {
        .spawn(move || {
            // GCM scheme detected from the first authenticating packet; reused thereafter.
            let mut detected: Option<Scheme> = None;
+            // Consecutive control-decrypt failures for this peer — throttles the warn log so a
+            // junk-packet flood can't spam unbounded lines (security-review 2026-06-28 #10).
+            let mut decrypt_fails: u64 = 0;
            // Decoded keyboard/mouse is forwarded to a dedicated host-lifetime injector thread —
            // NEVER injected inline, so a slow Wayland/libei/SendInput call can't head-block ENet
            // keepalive/retransmit servicing on this thread. The injector owns non-Send compositor
@@ -77,6 +80,7 @@ pub fn spawn(state: Arc<AppState>) -> Result<()> {
                            Event::Disconnect { .. } => {
                                tracing::info!("control: client disconnected");
                                detected = None;
+                                decrypt_fails = 0;
                                peer = None;
                                // Unplug the session's virtual pads.
                                pads = GamepadManager::new();
@@ -89,6 +93,7 @@ pub fn spawn(state: Arc<AppState>) -> Result<()> {
                                    channel_id,
                                    packet.data(),
                                    &mut detected,
+                                    &mut decrypt_fails,
                                    &inj_tx,
                                    &mut pads,
                                );
@@ -163,6 +168,7 @@ fn on_receive(
    _channel_id: u8,
    d: &[u8],
    detected: &mut Option<Scheme>,
+    decrypt_fails: &mut u64,
    inj_tx: &Sender<InputEvent>,
    pads: &mut GamepadManager,
 ) {
@@ -180,10 +186,20 @@ fn on_receive(
                tracing::info!(?scheme, "control: GCM scheme locked in");
            }
            *detected = Some(scheme);
+            *decrypt_fails = 0;
            pt
        }
        None => {
-            tracing::warn!(len = d.len(), "control: GCM decrypt failed");
+            // Throttle: a junk-packet flood must not spam one warn line per packet. Log the first
+            // failure, then only at exponentially-spaced counts (1, 2, 4, 8, …).
+            *decrypt_fails += 1;
+            if decrypt_fails.is_power_of_two() {
+                tracing::warn!(
+                    len = d.len(),
+                    fails = *decrypt_fails,
+                    "control: GCM decrypt failed"
+                );
+            }
            return;
        }
    };
@@ -90,6 +90,11 @@ pub struct LaunchSession {
    pub fps: u32,
    /// `/launch?appid=N` — selects the app-catalog entry (session recipe).
    pub appid: u32,
+    /// Source IP of the paired HTTPS client that issued `/launch`. The unauthenticated RTSP/UDP
+    /// media plane binds to this so only the launching peer can start/own the stream — an
+    /// unpaired RTSP peer cannot ride a paired client's launch (security-review 2026-06-28 #4).
+    /// `None` if the address could not be captured (then RTSP falls back to launch-present only).
+    pub peer_ip: Option<std::net::IpAddr>,
 }

 /// Shared control-plane state used as the axum app state.
@@ -1,9 +1,14 @@
 //! The nvhttp servers: plain HTTP on 47989 and mutual-TLS on 47984. Serves `/serverinfo`,
-//! the `/pair` flow, `/applist`, and `/launch`/`/resume`/`/cancel`, plus a punktfunk-only
-//! `/pin` endpoint to deliver the Moonlight-displayed PIN. Over HTTPS the client is
+//! the `/pair` flow, `/applist`, and `/launch`/`/resume`/`/cancel`. Over HTTPS the client is
 //! mutual-TLS-authenticated, so `/serverinfo` reports `PairStatus=1` there.
+//!
+//! The pairing PIN is delivered out-of-band ONLY through the bearer-authenticated management
+//! API (`POST /api/v1/pair/pin`): the operator reads the PIN off the Moonlight client and
+//! types it into the host console. There is deliberately NO unauthenticated nvhttp PIN
+//! endpoint — one would let a network client submit its own displayed PIN and drive the whole
+//! ceremony to a pinned cert with no operator consent (security-review 2026-06-28 #1).

-use super::tls::PeerCertFingerprint;
+use super::tls::{PeerAddr, PeerCertFingerprint};
 use super::{serverinfo, AppState, LaunchSession, HTTPS_PORT, HTTP_PORT, RTSP_PORT};
 use anyhow::{anyhow, Context, Result};
 use axum::{
@@ -58,7 +63,6 @@ fn router(state: Arc<AppState>, https: bool) -> Router {
    Router::new()
        .route("/serverinfo", get(h_serverinfo))
        .route("/pair", get(h_pair))
-        .route("/pin", get(h_pin))
        .route("/applist", get(h_applist))
        .route("/launch", get(h_launch))
        .route("/resume", get(h_resume))
@@ -82,19 +86,6 @@ async fn h_serverinfo(
    xml(serverinfo::serverinfo_xml(&st.host, https, paired))
 }

-async fn h_pin(
-    State(st): State<Arc<AppState>>,
-    Query(q): Query<HashMap<String, String>>,
-) -> impl IntoResponse {
-    match q.get("pin").filter(|p| !p.is_empty()) {
-        Some(pin) => {
-            st.pairing.pin.submit(pin.clone());
-            "PIN accepted\n".to_string()
-        }
-        None => "usage: GET /pin?pin=NNNN\n".to_string(),
-    }
-}
-
 async fn h_applist(
    State(st): State<Arc<AppState>>,
    peer: Option<Extension<PeerCertFingerprint>>,
@@ -110,6 +101,7 @@ async fn h_applist(
 async fn h_launch(
    State(st): State<Arc<AppState>>,
    peer: Option<Extension<PeerCertFingerprint>>,
+    addr: Option<Extension<PeerAddr>>,
    Query(q): Query<HashMap<String, String>>,
 ) -> impl IntoResponse {
    if !peer_is_paired(&peer, &st) {
@@ -117,7 +109,9 @@ async fn h_launch(
        return xml(error_xml());
    }
    match launch(&st, &q) {
-        Ok(session) => {
+        Ok(mut session) => {
+            // Bind the (unauthenticated) RTSP/UDP media plane to this paired client's source IP.
+            session.peer_ip = addr.map(|Extension(PeerAddr(a))| a.ip());
            *st.launch.lock().unwrap() = Some(session);
            tracing::info!(
                w = session.width,
@@ -193,6 +187,7 @@ fn launch(_st: &AppState, q: &HashMap<String, String>) -> Result<LaunchSession>
        height,
        fps,
        appid,
+        peer_ip: None, // set by `h_launch` from the verified HTTPS peer address
    })
 }

@@ -17,9 +17,14 @@ use std::sync::Mutex;
 use std::time::Duration;
 use tokio::sync::Notify;

-/// Out-of-band PIN delivery. Moonlight generates + displays a PIN; the user submits it
-/// (via the management API's `POST /api/v1/pair/pin` or nvhttp's `GET /pin?pin=NNNN`).
-/// `getservercert` parks until a PIN arrives.
+/// Out-of-band PIN delivery. Moonlight generates + displays a PIN; the operator submits it
+/// via the bearer-authenticated management API (`POST /api/v1/pair/pin`) only — there is no
+/// unauthenticated nvhttp delivery path (a network client must never be able to submit its
+/// own PIN; security-review 2026-06-28 #1). `getservercert` parks until a PIN arrives.
+/// Max pairing handshakes parked in [`PinGate::take`] at once (each holds a slot for up to
+/// 300s), bounding a pre-auth waiter flood. Real pairing is one operator-driven client at a time.
+const MAX_PARKED_WAITERS: usize = 4;
+
 pub struct PinGate {
    pin: Mutex<Option<String>>,
    notify: Notify,
@@ -48,7 +53,20 @@ impl PinGate {
    }

    async fn take(&self, timeout: Duration) -> Option<String> {
-        self.waiters.fetch_add(1, Ordering::SeqCst);
+        // Bound the number of pairing handshakes parked at once: each `getservercert` is
+        // pre-auth and parks for up to 300s, so without a cap an unpaired LAN peer could pin
+        // unbounded tasks + keep `awaiting_pin` asserted (security-review 2026-06-28 #12).
+        // Reserve a slot atomically; refuse (treated as "no PIN") once the cap is reached.
+        if self
+            .waiters
+            .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |n| {
+                (n < MAX_PARKED_WAITERS).then_some(n + 1)
+            })
+            .is_err()
+        {
+            tracing::warn!("pairing: too many handshakes awaiting a PIN — refusing");
+            return None;
+        }
        // Decrement on every exit path (PIN delivered, timeout, or future cancellation).
        struct WaiterGuard<'a>(&'a AtomicUsize);
        impl Drop for WaiterGuard<'_> {
@@ -117,7 +135,8 @@ impl Pairing {

        tracing::info!(
            uniqueid,
-            "pairing phase 1 (getservercert) — awaiting PIN: submit `GET /pin?pin=NNNN`"
+            "pairing phase 1 (getservercert) — awaiting PIN: deliver it via the management \
+             API `POST /api/v1/pair/pin` (operator reads the PIN off the Moonlight client)"
        );
        let pin = self
            .pin
@@ -304,4 +323,28 @@ mod tests {
        assert_eq!(pairing.pin.take(Duration::from_millis(10)).await, None);
        assert!(!pairing.pin.awaiting_pin());
    }
+
+    /// A pre-auth peer flood can park at most `MAX_PARKED_WAITERS` pairing handshakes; the next
+    /// `take` is refused immediately (returns `None` without parking), bounding the 300s-waiter DoS
+    /// (security-review 2026-06-28 #12).
+    #[tokio::test]
+    async fn pin_gate_caps_parked_waiters() {
+        let pairing = Arc::new(Pairing::new());
+        let mut handles = Vec::new();
+        for _ in 0..MAX_PARKED_WAITERS {
+            let p = pairing.clone();
+            handles.push(tokio::spawn(async move {
+                p.pin.take(Duration::from_secs(5)).await
+            }));
+        }
+        // Wait until all the slots are taken.
+        while pairing.pin.waiters.load(Ordering::SeqCst) < MAX_PARKED_WAITERS {
+            tokio::time::sleep(Duration::from_millis(2)).await;
+        }
+        // One more is refused right away (no parking), even with a long timeout.
+        assert_eq!(pairing.pin.take(Duration::from_secs(5)).await, None);
+        for h in handles {
+            h.abort();
+        }
+    }
 }
@@ -14,7 +14,7 @@ use crate::encode::Codec;
 use anyhow::{Context, Result};
 use std::collections::HashMap;
 use std::io::{Read, Write};
-use std::net::{TcpListener, TcpStream};
+use std::net::{SocketAddr, TcpListener, TcpStream};
 use std::sync::atomic::{AtomicUsize, Ordering};
 use std::sync::Arc;
 use std::time::Duration;
@@ -102,13 +102,12 @@ fn handle_conn(mut stream: TcpStream, state: Arc<AppState>) -> Result<()> {
            "RTSP {} | {}", req.head.replace("\r\n", " | "),
            if req.body.is_empty() { String::new() } else { format!("body: {}", req.body.replace("\r\n", " | ")) }
        );
-        let resp = handle_request(&req, &state);
+        let resp = handle_request(&req, &state, peer);
        stream.write_all(resp.as_bytes()).context("RTSP write")?;
        stream.flush().ok();
        // Close (FIN after the flushed response) so the client detects end-of-response.
        let _ = stream.shutdown(std::net::Shutdown::Both);
    }
-    let _ = peer;
    Ok(())
 }

@@ -171,7 +170,7 @@ fn parse_request(head: &str, body: String) -> Request {
    }
 }

-fn handle_request(req: &Request, state: &AppState) -> String {
+fn handle_request(req: &Request, state: &AppState, peer: Option<SocketAddr>) -> String {
    match req.method.as_str() {
        "OPTIONS" => response(
            &req.cseq,
@@ -216,16 +215,30 @@ fn handle_request(req: &Request, state: &AppState) -> String {
            response(&req.cseq, &[], None)
        }
        "PLAY" => {
+            // The RTSP/UDP media plane is UNAUTHENTICATED. A stream may start only for the paired
+            // client that completed the pairing-gated `/launch` (which set `state.launch`), and —
+            // when the launching IP is known — only from that same source IP. So an unpaired RTSP
+            // peer can neither start a stream on an idle host nor ride a paired client's active
+            // launch (security-review 2026-06-28 #4). `nvhttp` gates `/launch` on a pinned cert.
+            let launch = *state.launch.lock().unwrap();
+            let Some(ls) = launch else {
+                tracing::warn!(?peer, "RTSP PLAY — refused: no paired `/launch` session");
+                return response_status("401 Unauthorized", &req.cseq, &[], None);
+            };
+            if let (Some(want), Some(got)) = (ls.peer_ip, peer.map(|p| p.ip())) {
+                if want != got {
+                    tracing::warn!(
+                        %want, %got,
+                        "RTSP PLAY — refused: peer IP does not match the launching client"
+                    );
+                    return response_status("401 Unauthorized", &req.cseq, &[], None);
+                }
+            }
            let cfg = *state.stream.lock().unwrap();
            match cfg {
                Some(cfg) if !state.streaming.swap(true, Ordering::SeqCst) => {
                    // Resolve the launched catalog entry (session recipe) for the stream.
-                    let app = state
-                        .launch
-                        .lock()
-                        .unwrap()
-                        .map(|l| l.appid)
-                        .and_then(super::apps::by_id);
+                    let app = super::apps::by_id(ls.appid);
                    tracing::info!(app = ?app.as_ref().map(|a| &a.title), "RTSP PLAY — starting video stream");
                    stream::start(
                        cfg,
@@ -243,18 +256,15 @@ fn handle_request(req: &Request, state: &AppState) -> String {
            // Audio runs independently (Opus on UDP 48000, stereo or 5.1/7.1 multistream per
            // the ANNOUNCE); it needs the launch key for the AES-CBC payload encryption the
            // client expects.
-            let launch = *state.launch.lock().unwrap();
-            if let Some(ls) = launch {
-                if !state.audio_streaming.swap(true, Ordering::SeqCst) {
-                    tracing::info!("RTSP PLAY — starting audio stream");
-                    audio::start(
-                        state.audio_streaming.clone(),
-                        ls.gcm_key,
-                        ls.rikeyid,
-                        *state.audio_params.lock().unwrap(),
-                        state.audio_cap.clone(),
-                    );
-                }
+            if !state.audio_streaming.swap(true, Ordering::SeqCst) {
+                tracing::info!("RTSP PLAY — starting audio stream");
+                audio::start(
+                    state.audio_streaming.clone(),
+                    ls.gcm_key,
+                    ls.rikeyid,
+                    *state.audio_params.lock().unwrap(),
+                    state.audio_cap.clone(),
+                );
            }
            response(&req.cseq, &[("Session", "DEADBEEFCAFE;timeout = 90")], None)
        }
@@ -24,6 +24,12 @@ use std::sync::Arc;
 #[derive(Clone)]
 pub(crate) struct PeerCertFingerprint(pub Option<String>);

+/// The TCP source address of an HTTPS request, injected per-connection by [`serve_https`]. Used by
+/// `/launch` to record which paired client owns the session so the unauthenticated RTSP/UDP media
+/// plane can bind to that peer's IP (security-review 2026-06-28 #4).
+#[derive(Clone, Copy)]
+pub(crate) struct PeerAddr(pub SocketAddr);
+
 /// HTTPS server that surfaces the verified client cert to handlers. `axum_server` can't expose the
 /// peer cert, so this runs the rustls handshake itself (tokio-rustls), reads the peer certificate,
 /// and serves the axum `Router` over hyper with the peer's fingerprint attached to every request as
@@ -39,7 +45,7 @@ pub(crate) async fn serve_https(
        .await
        .with_context(|| format!("bind HTTPS {bind}"))?;
    loop {
-        let (tcp, _peer) = match listener.accept().await {
+        let (tcp, peer) = match listener.accept().await {
            Ok(v) => v,
            Err(e) => {
                tracing::warn!(error = %e, "HTTPS accept failed");
@@ -63,14 +69,16 @@ pub(crate) async fn serve_https(
                .peer_certificates()
                .and_then(|c| c.first())
                .map(|c| hex::encode(punktfunk_core::quic::endpoint::cert_fingerprint(c.as_ref())));
-            let peer = PeerCertFingerprint(fp);
+            let fp = PeerCertFingerprint(fp);
+            let addr = PeerAddr(peer);
            let svc =
                hyper::service::service_fn(move |req: hyper::Request<hyper::body::Incoming>| {
                    let app = app.clone();
-                    let peer = peer.clone();
+                    let fp = fp.clone();
                    async move {
                        let mut req = req.map(axum::body::Body::new);
-                        req.extensions_mut().insert(peer);
+                        req.extensions_mut().insert(fp);
+                        req.extensions_mut().insert(addr);
                        app.oneshot(req).await // Router error is Infallible
                    }
                });