Compare commits

...

1 Commits

Author SHA1 Message Date
rustdesk
7ab1c34839 webrtc: cap in-flight answerers, 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 costs 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 and gives it back once the data channel opens or the wait fails.
Past the cap the offer is declined with an empty answer, the same reply the
controller gets from a peer without WebRTC, so it carries on over punch and
relay. Declines are logged through the same one-line-a-minute throttle as the
other peer-driven ICE sites.

The cap also covers a re-sent PunchHole for a live session: with all slots
held it gets an empty answer rather than the cached one, and that punch
attempt runs without WebRTC. 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns
2026-09-14 22:17:47 +08:00
3 changed files with 105 additions and 4 deletions

View File

@@ -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)");
}
}

View File

@@ -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));
}
}

View File

@@ -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,35 @@ 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);
static DECLINED_OFFER_LOG: hbb_common::log_throttle::LogThrottle =
hbb_common::log_throttle::LogThrottle::new(ICE_LOG_INTERVAL);
/// 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 +778,16 @@ impl RendezvousMediator {
peer_addr: SocketAddr,
meta: ConnectionMeta,
) -> ResultType<String> {
let Some(slot) = AnswererSlot::take() else {
if let Some(n) = DECLINED_OFFER_LOG.due() {
log::warn!(
"declined {} WebRTC offer(s): {} answerers already in flight",
n,
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,9 @@ 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 ICE and DTLS, the part an unauthenticated offer makes this machine
// pay for; from here 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 +1530,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 +1781,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());
}
}