From d0cceafa8f6f7f4f29640f83e9907b2a708fb265 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Mon, 14 Sep 2026 22:17:47 +0800 Subject: [PATCH] webrtc: cap concurrent answerer setups, 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 64 slots before building the peer connection and gives it back once the data channel is open, or the wait for it fails; every earlier failure releases it through the guard's drop. The slot covers the setup an unauthenticated offer makes this machine pay for, ICE, DTLS and SCTP; with the channel open the session is a connection like any other. The cap is a guard against pathological concurrency, sized so a burst of legitimate controllers, slow ICE paths or a reconnect storm never meet it, and it stays inside the WebRTC rendezvous path: how many unauthenticated connections a machine allows is a question for the connection layer, where every transport shares it. 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. 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 Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns --- src/client.rs | 5 ++-- src/common.rs | 45 ++++++++++++++++++++++++++++ src/rendezvous_mediator.rs | 61 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/src/client.rs b/src/client.rs index 10330cfd6..b58d43272 100644 --- a/src/client.rs +++ b/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)"); } } diff --git a/src/common.rs b/src/common.rs index 00a409a63..95182e232 100644 --- a/src/common.rs +++ b/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)); + } } diff --git a/src/rendezvous_mediator.rs b/src/rendezvous_mediator.rs index 0436f060a..87516ac1e 100644 --- a/src/rendezvous_mediator.rs +++ b/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}, @@ -63,6 +63,36 @@ 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. A guard against pathological concurrency, sized so a burst of +/// legitimate controllers, slow ICE paths or a reconnect storm never meet it; once the channel +/// is open the connection is one like any other, and how many unauthenticated connections a +/// machine allows is a question for the connection layer, where every transport shares it. +const MAX_WEBRTC_ANSWERERS: usize = 64; +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 { + 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 +779,15 @@ impl RendezvousMediator { peer_addr: SocketAddr, meta: ConnectionMeta, ) -> ResultType { + 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(); @@ -849,6 +888,11 @@ impl RendezvousMediator { let session_key_for_cleanup = session_key.clone(); tokio::spawn(async move { let result = stream.wait_connected(CONNECT_TIMEOUT).await; + // The slot covers the setup an unauthenticated offer makes this machine pay for, ICE, + // DTLS and SCTP, and that wait is bounded by CONNECT_TIMEOUT. Release it here, before + // the cleanup and the close below, so their duration is never added to a slot's life; + // with the channel open the session is a connection like any other. + drop(slot); // Only evict our own route. The key is the offer's DTLS fingerprint, identical across // the controller's punch retries, so a retry that built a fresh answerer has already // replaced this entry — removing it blindly would delete the live session's sender and @@ -1488,7 +1532,10 @@ 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::net::SocketAddr; @@ -1736,4 +1783,14 @@ mod tests { "must return when the grace runs out, not a backoff later" ); } + + #[test] + fn test_answerer_slots_cap_and_release() { + let held: Vec<_> = (0..MAX_WEBRTC_ANSWERERS) + .map(|_| AnswererSlot::take().unwrap()) + .collect(); + assert!(AnswererSlot::take().is_none()); + drop(held); + assert!(AnswererSlot::take().is_some()); + } }