webrtc: cap unauthenticated answerer connections, pin the DTLS fingerprint binding with a test

A WebRTC offer reaches the controlled side before any password or accept
prompt, and answering one builds a peer connection that binds a socket per
interface and runs ICE for up to CONNECT_TIMEOUT. A forged TCP punch reuses
the mediator's local port for one connect; a forged offer costs all of that,
and nothing bounded how many could be in flight at once. SESSIONS dedups by
offer fingerprint, which only stops replays of one offer.

spawn_webrtc_answerer now takes one of eight slots before building the peer
connection. The slot is not given back when the data channel opens: Open is
not an authenticated peer, since create_tcp_connection still runs the
SignedId/PublicKey handshake, up to CONNECT_TIMEOUT, before the peer proves
who it is. The slot travels into the connection through ConnectionMeta,
type-erased so the shared code only carries and drops it, and is released
when the connection authorizes or when it ends first. The cap therefore
bounds unauthenticated WebRTC peer connections in every state from offer to
authorization, not merely concurrent handshakes.

Past the cap the offer is declined with an empty answer, the reply the
controller already gets from a peer without WebRTC, so it carries on over
punch and relay. Declines log through the throttled-log macro. At the cap a
re-sent PunchHole for a live session also gets an empty answer rather than
the cached one, since the slot is taken before the cache is consulted; only
reachable at the cap, where degrading is the point.

Tests pin the counter's cap and release, and that a slot handed over
type-erased still holds its place until that handle drops.

The other change is a test. The controller's defence against a rendezvous or
relay that swaps SDP fingerprints is the fingerprint the controlled side signs
into IdPk and the comparison in secure_connection, and neither had a test.
The comparison moves into dtls_fingerprint_bound so it can have one, along
with decode_id_pk_dtls: the fingerprint round-trips under the signature,
another key or an edited payload yields nothing, empty never binds, and
decode_id_pk still sees the same id and pk.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns

src/client.rs

@@ -30,7 +30,8 @@ use uuid::Uuid;
 use crate::{
     check_port,
     common::input::{MOUSE_BUTTON_LEFT, MOUSE_BUTTON_RIGHT, MOUSE_TYPE_DOWN, MOUSE_TYPE_UP},
-    create_symmetric_key_msg, decode_id_pk, decode_id_pk_dtls, get_rs_pk, is_keyboard_mode_supported,
+    create_symmetric_key_msg, decode_id_pk, decode_id_pk_dtls, dtls_fingerprint_bound, get_rs_pk,
+    is_keyboard_mode_supported,
     kcp_stream::KcpStream,
     secure_tcp,
     ui_interface::{get_builtin_option, resolve_avatar_url, use_texture_render},
@@ -1648,7 +1649,7 @@ impl Client {
                                     let actual_fp = conn.dtls_fingerprint(false).await.ok_or_else(
                                         || anyhow!("WebRTC DTLS fingerprint unavailable"),
                                     )?;
-                                    if signed_fp.is_empty() || signed_fp != actual_fp {
+                                    if !dtls_fingerprint_bound(&signed_fp, &actual_fp) {
                                         bail!("WebRTC DTLS fingerprint not bound to peer identity (possible MITM)");
                                     }
                                 }

src/common.rs

@@ -2161,6 +2161,13 @@ pub fn decode_id_pk_dtls(
     }
 }
 
+/// Whether the DTLS fingerprint a WebRTC peer signed into its identity is the one of the channel
+/// actually negotiated. An empty signed value binds nothing: on a WebRTC channel it is either a
+/// peer that could not sign one or a rendezvous/relay that stripped it, and both fail closed.
+pub fn dtls_fingerprint_bound(signed_fp: &str, actual_fp: &str) -> bool {
+    !signed_fp.is_empty() && signed_fp == actual_fp
+}
+
 pub fn create_symmetric_key_msg(their_pk_b: [u8; 32]) -> (Bytes, Bytes, secretbox::Key) {
     let their_pk_b = box_::PublicKey(their_pk_b);
     let (our_pk_b, out_sk_b) = box_::gen_keypair();
@@ -3258,4 +3265,42 @@ mod tests {
         assert_eq!(combined_mask & MOUSE_TYPE_MASK, MOUSE_TYPE_DOWN);
         assert_eq!(combined_mask >> 3, MOUSE_BUTTON_LEFT | MOUSE_BUTTON_RIGHT);
     }
+
+    #[test]
+    fn test_dtls_fingerprint_travels_signed_and_binds() {
+        let (pk, sk) = sign::gen_keypair();
+        let fp = "sha-256 0A:1B:2C";
+        let signed = sign::sign(
+            &IdPk {
+                id: "123456789".to_owned(),
+                pk: Bytes::from(vec![7u8; 32]),
+                dtls_fingerprint: fp.to_owned(),
+                ..Default::default()
+            }
+            .write_to_bytes()
+            .unwrap(),
+            &sk,
+        );
+
+        let (id, their_pk, signed_fp) = decode_id_pk_dtls(&signed, &pk).unwrap();
+        assert_eq!(id, "123456789");
+        assert_eq!(their_pk, [7u8; 32]);
+        assert_eq!(signed_fp, fp);
+        assert!(dtls_fingerprint_bound(&signed_fp, fp));
+        assert!(!dtls_fingerprint_bound(&signed_fp, "sha-256 0A:1B:2D"));
+        assert!(!dtls_fingerprint_bound("", ""));
+
+        // The fingerprint is under the signature: a blob verified with another key yields
+        // nothing, and one whose payload was edited in transit fails verification.
+        let (other_pk, _) = sign::gen_keypair();
+        assert!(decode_id_pk_dtls(&signed, &other_pk).is_err());
+        let mut tampered = signed.clone();
+        let last = tampered.len() - 1;
+        tampered[last] ^= 1;
+        assert!(decode_id_pk_dtls(&tampered, &pk).is_err());
+
+        // `decode_id_pk` is the same blob minus the fingerprint, so the field is invisible to
+        // non-WebRTC handshakes.
+        assert_eq!(decode_id_pk(&signed, &pk).unwrap(), (id, their_pk));
+    }
 }

src/rendezvous_mediator.rs

@@ -3,7 +3,7 @@ use std::{
     hash::BuildHasher,
     net::SocketAddr,
     sync::{
-        atomic::{AtomicBool, Ordering},
+        atomic::{AtomicBool, AtomicUsize, Ordering},
         Arc, RwLock,
     },
     time::{Duration, Instant},
@@ -46,6 +46,7 @@ fn connection_meta(
     ConnectionMeta {
         control_permissions,
         controlled_context,
+        webrtc_pre_auth_hold: None,
     }
 }
 
@@ -63,6 +64,33 @@ const MAX_PENDING_REMOTE_ICE: usize = 64;
 /// Queued candidates remembered so the controller's re-send is skipped instead of taking a slot
 /// of its own. Far more than an honest peer gathers, at eight bytes each.
 const ICE_DEDUP_WINDOW: usize = 256;
+/// Answerers between an offer and an open data channel. An offer arrives before any password or
+/// accept prompt, and each one builds a peer connection that binds a socket per interface and
+/// runs ICE for up to `CONNECT_TIMEOUT`, where a forged TCP punch costs one connect. Past this
+/// many the offer is declined, and the controller carries on over punch and relay as it does
+/// for a peer without WebRTC.
+const MAX_WEBRTC_ANSWERERS: usize = 8;
+static WEBRTC_ANSWERERS: AtomicUsize = AtomicUsize::new(0);
+
+/// One of the `MAX_WEBRTC_ANSWERERS` slots, given back on drop.
+struct AnswererSlot;
+
+impl AnswererSlot {
+    fn take() -> Option<Self> {
+        WEBRTC_ANSWERERS
+            .fetch_update(Ordering::AcqRel, Ordering::Acquire, |n| {
+                (n < MAX_WEBRTC_ANSWERERS).then(|| n + 1)
+            })
+            .ok()
+            .map(|_| Self)
+    }
+}
+
+impl Drop for AnswererSlot {
+    fn drop(&mut self) {
+        WEBRTC_ANSWERERS.fetch_sub(1, Ordering::AcqRel);
+    }
+}
 // The rendezvous ICE route is reachable without a prior punch and the peer decides how many
 // candidates it sends, so these sites would let someone else set how much this machine writes to
 // its log file. One line a minute each, carrying the suppressed count.
@@ -749,6 +777,15 @@ impl RendezvousMediator {
         peer_addr: SocketAddr,
         meta: ConnectionMeta,
     ) -> ResultType<String> {
+        let Some(slot) = AnswererSlot::take() else {
+            hbb_common::throttled_log!(
+                ICE_LOG_INTERVAL,
+                warn,
+                "declined a WebRTC offer: {} answerers already in flight",
+                MAX_WEBRTC_ANSWERERS
+            );
+            return Ok(String::new());
+        };
         let mut stream =
             WebRTCStream::new(&ph.webrtc_sdp_offer, relay_only_ice, CONNECT_TIMEOUT).await?;
         let answer = stream.local_endpoint().to_owned();
@@ -875,6 +912,13 @@ impl RendezvousMediator {
             // SESSIONS (its state handler only fires on a terminal ICE state, which a cleanly
             // closed session may never reach) leaking the pc, channels, and socket fds.
             let stream_for_cleanup = stream.clone();
+            // A data channel at Open is not yet an authenticated peer: create_tcp_connection still
+            // runs the SignedId/PublicKey handshake, up to CONNECT_TIMEOUT, before the peer proves
+            // who it is. Carry the slot into that connection so a forged offer that opens a channel
+            // and then stalls keeps occupying one until the handshake closes it out, rather than
+            // freeing it here and letting the next batch of eight begin.
+            let mut meta = meta;
+            meta.webrtc_pre_auth_hold = Some(std::sync::Arc::new(slot));
             if let Err(err) = crate::server::create_tcp_connection(
                 server,
                 Stream::WebRTC(stream),
@@ -1488,9 +1532,14 @@ impl Drop for CheckIfResendPk {
 
 #[cfg(test)]
 mod tests {
-    use super::{mpsc, socket_client, tokio, IceRoute, ICE_DEDUP_WINDOW, MAX_PENDING_REMOTE_ICE};
+    use super::{
+        mpsc, socket_client, tokio, AnswererSlot, IceRoute, ICE_DEDUP_WINDOW,
+        MAX_PENDING_REMOTE_ICE, MAX_WEBRTC_ANSWERERS,
+    };
     use hbb_common::tcp::new_listener;
+    use std::any::Any;
     use std::net::SocketAddr;
+    use std::sync::Arc;
 
     // A SOCKS proxy makes `connect_tcp_local` dial the proxy and ignore the local address, so
     // nothing these two assert can hold. Read once, from the same global config production reads.
@@ -1736,4 +1785,36 @@ mod tests {
             "must return when the grace runs out, not a backoff later"
         );
     }
+
+    // The slot counter is process-global and the test harness runs tests in parallel threads,
+    // so every test that takes slots holds this first; a poisoned lock is still a lock.
+    static SLOT_TESTS: std::sync::Mutex<()> = std::sync::Mutex::new(());
+
+    #[test]
+    fn test_answerer_slots_cap_and_release() {
+        let _serial = SLOT_TESTS.lock().unwrap_or_else(|e| e.into_inner());
+        let held: Vec<_> = (0..MAX_WEBRTC_ANSWERERS)
+            .map(|_| AnswererSlot::take().unwrap())
+            .collect();
+        assert!(AnswererSlot::take().is_none());
+        drop(held);
+        assert!(AnswererSlot::take().is_some());
+    }
+
+    // The answerer hands its slot into the connection type-erased, exactly as it reaches the
+    // authenticated peer; this pins that the erased hold still occupies a slot and that dropping
+    // it — as authorization or a dropped Connection does — is what releases it. If the slot were
+    // freed at data-channel Open instead, this connection could not keep the last one occupied.
+    #[test]
+    fn test_erased_slot_holds_until_dropped() {
+        let _serial = SLOT_TESTS.lock().unwrap_or_else(|e| e.into_inner());
+        let held: Vec<_> = (0..MAX_WEBRTC_ANSWERERS - 1)
+            .map(|_| AnswererSlot::take().unwrap())
+            .collect();
+        let erased: Arc<dyn Any + Send + Sync> = Arc::new(AnswererSlot::take().unwrap());
+        assert!(AnswererSlot::take().is_none());
+        drop(erased);
+        assert!(AnswererSlot::take().is_some());
+        drop(held);
+    }
 }

src/server.rs

@@ -88,6 +88,11 @@ type ConnMap = HashMap<i32, ConnInner>;
 pub struct ConnectionMeta {
     pub control_permissions: Option<ControlPermissions>,
     pub controlled_context: Option<ControlledContext>,
+    /// A resource permit the answerer path attaches to a WebRTC connection so it stays counted
+    /// until RustDesk's own authentication, not merely until the data channel opens. Type-erased
+    /// so this shared struct need not know the answerer's slot type; opaque everywhere else, it is
+    /// only carried and dropped. `None` on every other transport.
+    pub webrtc_pre_auth_hold: Option<std::sync::Arc<dyn std::any::Any + Send + Sync>>,
 }
 
 #[cfg(any(target_os = "macos", target_os = "linux"))]

src/server/connection.rs

@@ -264,6 +264,9 @@ pub struct Connection {
     port_forward_address: String,
     tx_to_cm: mpsc::UnboundedSender<ipc::Data>,
     authorized: bool,
+    // Held for a WebRTC answerer until authorization, then dropped to free its resource slot;
+    // see ConnectionMeta::webrtc_pre_auth_hold. None on every other transport.
+    webrtc_pre_auth_hold: Option<std::sync::Arc<dyn std::any::Any + Send + Sync>>,
     require_2fa: Option<totp_rs::TOTP>,
     awaiting_2fa: bool,
     keyboard: bool,
@@ -429,6 +432,7 @@ impl Connection {
         let super::ConnectionMeta {
             control_permissions,
             controlled_context,
+            webrtc_pre_auth_hold,
         } = meta;
         // Android is not supported yet, so we always set control_permissions to None.
         #[cfg(target_os = "android")]
@@ -483,6 +487,7 @@ impl Connection {
             port_forward_address: "".to_owned(),
             tx_to_cm,
             authorized: false,
+            webrtc_pre_auth_hold,
             keyboard: Self::permission(keys::OPTION_ENABLE_KEYBOARD, &control_permissions),
             clipboard: Self::permission(keys::OPTION_ENABLE_CLIPBOARD, &control_permissions),
             audio: Self::permission(keys::OPTION_ENABLE_AUDIO, &control_permissions),
@@ -1792,6 +1797,9 @@ impl Connection {
             return false;
         }
         self.authorized = true;
+        // The peer is authenticated now, so release any WebRTC pre-auth resource slot; a no-op
+        // on other transports.
+        self.webrtc_pre_auth_hold = None;
         // Releases the budget `check_id_whitelist` charges against this address: only a peer
         // that got this far proved more than a self-reported id.
         self.clear_id_whitelist_failures();