From e272fede36fafa63e6f6b8c71b918267a7ea4195 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Mon, 14 Sep 2026 22:22:44 +0800 Subject: [PATCH 01/13] tcp: key exchange version 1, one stream key per direction Version 1 of the key exchange splits the exchanged key into a subkey per direction, the way Noise splits its cipher states after the handshake, and leaves the nonce layout and the handshake messages as they are. Each subkey is a keyed BLAKE2b over the handshake transcript: both ephemeral public keys, the version the responder advertised and the one the initiator picked, so a change to any of it in transit leaves the two sides with different keys. The original scheme is version 0, which is also what an absent field means. Negotiated through new fields old peers do not see: the side that speaks first advertises the newest version it has (`KeyExchange.version` from the server, `IdPk.kx_version` inside the signed identity from the controlled side), the other side answers with the highest both have, and the pair runs that. Any old peer on either end keeps today's stream byte for byte. The controlled side's advertisement sits inside the signature; the server's is repeated in `RendezvousMessage.kx_advertised` on its first message after the exchange, inside the encrypted channel, and a client that was shown a different value in the clear refuses the stream. `Encrypt::new` is unchanged; `new_split`, `set_key_split` and `check_kx_advertised` are added beside it, and `box_pk_of` for a responder that kept only the secret half of its ephemeral pair. The tests pin the version 0 output as well as the split and the echo. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns --- protos/rendezvous.proto | 11 ++ src/stream.rs | 32 +++++ src/tcp.rs | 269 +++++++++++++++++++++++++++++++++++++++- src/websocket.rs | 20 ++- 4 files changed, 327 insertions(+), 5 deletions(-) diff --git a/protos/rendezvous.proto b/protos/rendezvous.proto index f87882385..e8c7d6c2a 100644 --- a/protos/rendezvous.proto +++ b/protos/rendezvous.proto @@ -8,6 +8,9 @@ message IdPk { // a WebRTC peer's DTLS channel can be bound to its verified identity (defeats a rendezvous/relay // that swaps SDP fingerprints). Empty for non-WebRTC handshakes. string dtls_fingerprint = 3; + // Newest key exchange version the signer speaks; absent, i.e. 0, is the original scheme. + // Inside the signed identity so a rendezvous or relay cannot lower it. + uint32 kx_version = 4; } message RegisterPeer { @@ -232,6 +235,9 @@ message OnlineResponse { message KeyExchange { repeated bytes keys = 1; + // From the server: the newest key exchange version it speaks. From the client: the version it + // picked, the highest both speak. Absent, i.e. 0, is the original scheme. + uint32 version = 2; } message HealthCheck { @@ -291,4 +297,9 @@ message RendezvousMessage { HttpProxyResponse http_proxy_response = 28; IceCandidate ice_candidate = 29; } + // From the server, on its first message after a key exchange: the version it advertised, + // repeated inside the encrypted channel where nothing in transit can alter it. A client that + // was shown a different value in the clear knows the handshake was altered and refuses the + // stream. Absent from servers that predate versions. + uint32 kx_advertised = 30; } diff --git a/src/stream.rs b/src/stream.rs index e651be554..8702828da 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -80,6 +80,38 @@ impl Stream { } } + /// Per-direction keys bound to the handshake; see `tcp::KX_VERSION_LATEST`. `is_initiator` + /// is the side that sent the sealed key. + #[inline] + pub fn set_key_split( + &mut self, + key: Key, + is_initiator: bool, + t: &tcp::KxTranscript, + ) -> ResultType<()> { + match self { + #[cfg(feature = "webrtc")] + Stream::WebRTC(s) => { + s.set_key(key); + Ok(()) + } + Stream::WebSocket(s) => s.set_key_split(key, is_initiator, t), + Stream::Tcp(s) => s.set_key_split(key, is_initiator, t), + } + } + + /// Refuse the stream if the server's first encrypted message says it advertised a key + /// exchange version other than `seen`, the one this side received in the clear. + #[inline] + pub fn check_kx_advertised(&mut self, seen: u32) { + match self { + #[cfg(feature = "webrtc")] + Stream::WebRTC(_) => {} + Stream::WebSocket(s) => s.check_kx_advertised(seen), + Stream::Tcp(s) => s.check_kx_advertised(seen), + } + } + /// An opaque token that changes whenever bytes have arrived from the peer, a fragment of a /// message that is still incomplete included. Compare successive samples; the value itself /// means nothing. `None` where the transport reports no such thing. diff --git a/src/tcp.rs b/src/tcp.rs index 2296edb1d..6673327ac 100644 --- a/src/tcp.rs +++ b/src/tcp.rs @@ -4,7 +4,7 @@ use bytes::{BufMut, Bytes, BytesMut}; use futures::{SinkExt, StreamExt}; use protobuf::Message; use sodiumoxide::crypto::{ - box_, + box_, generichash, scalarmult, secretbox::{self, Key, Nonce}, }; use std::{ @@ -24,8 +24,47 @@ use tokio_util::codec::Framed; pub trait TcpStreamTrait: AsyncRead + AsyncWrite + Unpin {} pub struct DynTcpStream(pub Box); +/// The newest key exchange version this build speaks. Version 0 is the original scheme, and +/// what an absent field means: one key for both directions, each direction counting its own +/// nonce from 1, so the n-th message each way is sealed under the same (key, nonce). Version 1 +/// splits the exchanged key into one per direction, the way Noise splits its cipher states +/// after the handshake, binds the handshake transcript into both, and keeps the nonce layout. +pub const KX_VERSION_LATEST: u32 = 1; + +/// The version to run against a peer that advertised `advertised`: the highest both sides +/// support. +#[inline] +pub fn kx_version_for(advertised: u32) -> u32 { + advertised.min(KX_VERSION_LATEST) +} + +const KX_SPLIT_CONTEXT: &[u8] = b"rdkx-spl"; +const KX_SPLIT_INITIATOR: u8 = 1; +const KX_SPLIT_RESPONDER: u8 = 2; + +/// What both sides saw during the key exchange, each from its own side of the wire. Mixed into +/// the split keys, so a change to any of it in transit leaves the two sides with different keys +/// and the first message undecryptable, without a check per field. +pub struct KxTranscript<'a> { + /// The ephemeral public key of the side that sent the sealed key. + pub initiator_pk: &'a [u8], + /// The ephemeral public key the sealed key was sealed to. + pub responder_pk: &'a [u8], + /// The version the responder advertised: what it sent, what the initiator received. + pub advertised: u32, + /// The version the initiator picked. + pub picked: u32, +} + +/// The public half of an X25519 secret key, for a responder that kept only the secret half. +pub fn box_pk_of(sk: &box_::SecretKey) -> box_::PublicKey { + box_::PublicKey(scalarmult::scalarmult_base(&scalarmult::Scalar(sk.0)).0) +} + +/// The sending key, the send and receive counters, the receiving key where it differs, and +/// the server's advertised version as this side saw it, while its echo is still awaited. #[derive(Clone)] -pub struct Encrypt(pub Key, pub u64, pub u64); +pub struct Encrypt(pub Key, pub u64, pub u64, Option, Option); pub struct FramedStream( pub Framed, @@ -200,6 +239,22 @@ impl FramedStream { self.2 = Some(Encrypt::new(key)); } + pub fn set_key_split( + &mut self, + key: Key, + is_initiator: bool, + t: &KxTranscript, + ) -> ResultType<()> { + self.2 = Some(Encrypt::new_split(key, is_initiator, t)?); + Ok(()) + } + + pub fn check_kx_advertised(&mut self, seen: u32) { + if let Some(enc) = self.2.as_mut() { + enc.check_kx_advertised(seen); + } + } + fn get_nonce(seqnum: u64) -> Nonce { let mut nonce = Nonce([0u8; secretbox::NONCEBYTES]); nonce.0[..std::mem::size_of_val(&seqnum)].copy_from_slice(&seqnum.to_le_bytes()); @@ -295,7 +350,69 @@ impl TcpStreamTrait for R {} impl Encrypt { pub fn new(key: Key) -> Self { - Self(key, 0, 0) + Self(key, 0, 0, None, None) + } + + /// Arm the check of the server's advertisement echo, `seen` being the version this side + /// received in the clear. Call once the key is set and before the first frame is read. + pub fn check_kx_advertised(&mut self, seen: u32) { + self.4 = Some(seen); + } + + /// The server repeats what it advertised inside its first encrypted message, where it can + /// be neither forged nor stripped. A different value in the clear means the handshake was + /// altered on the way, and the stream is refused rather than run at the version it was + /// pushed down to. No echo is a server from before versions, whose advertisement was none. + fn check_kx_advertised_echo(plain: &[u8], seen: u32) -> Result<(), Error> { + let echoed = crate::rendezvous_proto::RendezvousMessage::parse_from_bytes(plain) + .map(|m| m.kx_advertised) + .unwrap_or(0); + if echoed != 0 && echoed != seen { + return Err(Error::new( + ErrorKind::Other, + format!( + "key exchange advertised {} but {} arrived: altered in transit", + echoed, seen + ), + )); + } + Ok(()) + } + + /// Version 1: the initiator, the side that sent the sealed key, sends under one subkey and + /// receives under the other; the responder the reverse. + pub fn new_split(key: Key, is_initiator: bool, t: &KxTranscript) -> ResultType { + let initiator = Self::derive_subkey(&key, KX_SPLIT_INITIATOR, t)?; + let responder = Self::derive_subkey(&key, KX_SPLIT_RESPONDER, t)?; + Ok(if is_initiator { + Self(initiator, 0, 0, Some(responder), None) + } else { + Self(responder, 0, 0, Some(initiator), None) + }) + } + + fn derive_subkey(key: &Key, direction: u8, t: &KxTranscript) -> ResultType { + if t.initiator_pk.len() != box_::PUBLICKEYBYTES + || t.responder_pk.len() != box_::PUBLICKEYBYTES + { + bail!("key exchange transcript has a public key of the wrong length"); + } + let failed = |_| anyhow::anyhow!("key derivation failed"); + let mut h = + generichash::State::new(Some(secretbox::KEYBYTES), Some(&key.0)).map_err(failed)?; + for part in [ + KX_SPLIT_CONTEXT, + &[direction], + &t.advertised.to_le_bytes(), + &t.picked.to_le_bytes(), + t.initiator_pk, + t.responder_pk, + ] { + h.update(part).map_err(failed)?; + } + let mut subkey = [0u8; secretbox::KEYBYTES]; + subkey.copy_from_slice(h.finalize().map_err(failed)?.as_ref()); + Ok(Key(subkey)) } pub fn dec(&mut self, bytes: &mut BytesMut) -> Result<(), Error> { @@ -304,8 +421,11 @@ impl Encrypt { } self.2 += 1; let nonce = FramedStream::get_nonce(self.2); - match secretbox::open(bytes, &nonce, &self.0) { + match secretbox::open(bytes, &nonce, self.3.as_ref().unwrap_or(&self.0)) { Ok(res) => { + if let Some(seen) = self.4.take() { + Self::check_kx_advertised_echo(&res, seen)?; + } bytes.clear(); bytes.put_slice(&res); Ok(()) @@ -342,3 +462,144 @@ impl Encrypt { Ok(Key(key)) } } + +#[cfg(test)] +mod tests { + use super::*; + + const INITIATOR_PK: [u8; 32] = [1u8; 32]; + const RESPONDER_PK: [u8; 32] = [2u8; 32]; + + fn transcript(advertised: u32) -> KxTranscript<'static> { + KxTranscript { + initiator_pk: &INITIATOR_PK, + responder_pk: &RESPONDER_PK, + advertised, + picked: KX_VERSION_LATEST, + } + } + + fn seal_and_open(a: &mut Encrypt, b: &mut Encrypt, msg: &[u8]) -> Vec { + let sealed = a.enc(msg); + let mut buf = BytesMut::from(&sealed[..]); + b.dec(&mut buf).unwrap(); + assert_eq!(&buf[..], msg); + sealed + } + + #[test] + fn test_v0_shares_one_key_across_directions() { + let key = secretbox::gen_key(); + let (mut initiator, mut responder) = (Encrypt::new(key.clone()), Encrypt::new(key)); + let sent = seal_and_open(&mut initiator, &mut responder, b"hello"); + let back = seal_and_open(&mut responder, &mut initiator, b"hello"); + // The n-th message each way is sealed under the same (key, nonce): the defect the split + // fixes, pinned here so the path a peer without versions relies on stays byte-for-byte. + assert_eq!(sent, back); + } + + #[test] + fn test_split_directions_use_different_keys() { + let key = secretbox::gen_key(); + let t = transcript(KX_VERSION_LATEST); + let mut initiator = Encrypt::new_split(key.clone(), true, &t).unwrap(); + let mut responder = Encrypt::new_split(key.clone(), false, &t).unwrap(); + let sent = seal_and_open(&mut initiator, &mut responder, b"hello"); + let back = seal_and_open(&mut responder, &mut initiator, b"hello"); + assert_ne!(sent, back); + // Neither direction's key is the exchanged key itself, so a version 0 peer cannot be + // paired with a split one by accident. + let mut v0 = Encrypt::new(key); + let mut buf = BytesMut::from(&sent[..]); + assert!(v0.dec(&mut buf).is_err()); + } + + #[test] + fn test_split_roles_must_differ() { + let key = secretbox::gen_key(); + let t = transcript(KX_VERSION_LATEST); + let mut a = Encrypt::new_split(key.clone(), true, &t).unwrap(); + let mut b = Encrypt::new_split(key, true, &t).unwrap(); + let sealed = a.enc(b"hello"); + let mut buf = BytesMut::from(&sealed[..]); + assert!(b.dec(&mut buf).is_err()); + } + + #[test] + fn test_kx_version_for_picks_the_highest_shared() { + assert_eq!(kx_version_for(0), 0); + assert_eq!(kx_version_for(1), 1); + assert_eq!(kx_version_for(KX_VERSION_LATEST), KX_VERSION_LATEST); + assert_eq!(kx_version_for(KX_VERSION_LATEST + 7), KX_VERSION_LATEST); + } + + #[test] + fn test_split_binds_the_transcript() { + let key = secretbox::gen_key(); + // The responder advertised LATEST; the initiator was handed something else in transit. + let mut initiator = + Encrypt::new_split(key.clone(), true, &transcript(KX_VERSION_LATEST + 1)).unwrap(); + let mut responder = Encrypt::new_split(key, false, &transcript(KX_VERSION_LATEST)).unwrap(); + let sealed = initiator.enc(b"hello"); + let mut buf = BytesMut::from(&sealed[..]); + assert!(responder.dec(&mut buf).is_err()); + } + + #[test] + fn test_split_rejects_a_short_public_key() { + let t = KxTranscript { + initiator_pk: &[1u8; 31], + responder_pk: &RESPONDER_PK, + advertised: KX_VERSION_LATEST, + picked: KX_VERSION_LATEST, + }; + assert!(Encrypt::new_split(secretbox::gen_key(), true, &t).is_err()); + } + + fn rendezvous_frame(kx_advertised: u32) -> Vec { + let mut msg = crate::rendezvous_proto::RendezvousMessage::new(); + msg.set_hc(Default::default()); + msg.kx_advertised = kx_advertised; + msg.write_to_bytes().unwrap() + } + + fn first_frame_after(seen: u32, echoed: u32) -> Result<(), Error> { + let key = secretbox::gen_key(); + let mut server = Encrypt::new(key.clone()); + let mut client = Encrypt::new(key); + client.check_kx_advertised(seen); + // An empty heartbeat is not a frame, and must not use up the check. + let mut empty = BytesMut::new(); + client.dec(&mut empty).unwrap(); + let mut buf = BytesMut::from(&server.enc(&rendezvous_frame(echoed))[..]); + client.dec(&mut buf) + } + + #[test] + fn test_kx_advertised_echo_must_match_what_was_seen() { + assert!(first_frame_after(KX_VERSION_LATEST, KX_VERSION_LATEST).is_ok()); + // A server from before versions echoes nothing, and advertised nothing. + assert!(first_frame_after(0, 0).is_ok()); + // The advertisement was lowered on the way; the echo says otherwise. + assert!(first_frame_after(0, KX_VERSION_LATEST).is_err()); + assert!(first_frame_after(KX_VERSION_LATEST, KX_VERSION_LATEST + 1).is_err()); + } + + #[test] + fn test_kx_advertised_check_is_one_shot() { + let key = secretbox::gen_key(); + let mut server = Encrypt::new(key.clone()); + let mut client = Encrypt::new(key); + client.check_kx_advertised(KX_VERSION_LATEST); + let mut first = BytesMut::from(&server.enc(&rendezvous_frame(KX_VERSION_LATEST))[..]); + client.dec(&mut first).unwrap(); + let mut later = BytesMut::from(&server.enc(&rendezvous_frame(KX_VERSION_LATEST + 9))[..]); + client.dec(&mut later).unwrap(); + } + + #[test] + fn test_box_pk_of_matches_the_generated_pair() { + let (pk, sk) = box_::gen_keypair(); + assert_eq!(box_pk_of(&sk), pk); + } +} diff --git a/src/websocket.rs b/src/websocket.rs index b2901e322..d4dc85883 100644 --- a/src/websocket.rs +++ b/src/websocket.rs @@ -5,7 +5,7 @@ use crate::{ protobuf::Message, socket_client::split_host_port, sodiumoxide::crypto::secretbox::Key, - tcp::Encrypt, + tcp::{Encrypt, KxTranscript}, tls::{get_cached_tls_accept_invalid_cert, get_cached_tls_type, upsert_tls_cache, TlsType}, ResultType, }; @@ -257,6 +257,24 @@ impl WsFramedStream { self.encrypt = Some(Encrypt::new(key)); } + #[inline] + pub fn set_key_split( + &mut self, + key: Key, + is_initiator: bool, + t: &KxTranscript, + ) -> ResultType<()> { + self.encrypt = Some(Encrypt::new_split(key, is_initiator, t)?); + Ok(()) + } + + #[inline] + pub fn check_kx_advertised(&mut self, seen: u32) { + if let Some(enc) = self.encrypt.as_mut() { + enc.check_kx_advertised(seen); + } + } + #[inline] pub fn is_secured(&self) -> bool { self.encrypt.is_some() From 955621899e2baef0e38a722f0925bcb5614c521b Mon Sep 17 00:00:00 2001 From: rustdesk Date: Mon, 21 Sep 2026 12:29:11 +0800 Subject: [PATCH 02/13] tcp: keep the advertisement check armed until an echo arrives The check was spent on the first frame that decrypted, whatever it held. A server tags the messages it chooses to, and a frame carrying no echo reads as `echoed == 0`, which passes; on the way out it also disarmed the check for good, so an attacker who rewrote the advertised version only had to let one untagged frame through to be rid of the only thing that would have caught it. A zero now says nothing either way and leaves the check standing. Only a non-zero echo settles the question, matching or refusing the stream. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EZ49AbZJYfm8NTp5yDPMab --- src/tcp.rs | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/src/tcp.rs b/src/tcp.rs index 6673327ac..f53302b37 100644 --- a/src/tcp.rs +++ b/src/tcp.rs @@ -359,15 +359,21 @@ impl Encrypt { self.4 = Some(seen); } - /// The server repeats what it advertised inside its first encrypted message, where it can - /// be neither forged nor stripped. A different value in the clear means the handshake was + /// The server repeats what it advertised inside an encrypted message, where it can be + /// neither forged nor stripped. A different value in the clear means the handshake was /// altered on the way, and the stream is refused rather than run at the version it was - /// pushed down to. No echo is a server from before versions, whose advertisement was none. - fn check_kx_advertised_echo(plain: &[u8], seen: u32) -> Result<(), Error> { + /// pushed down to. Returns whether the echo settled the question: a frame that carries + /// none says nothing either way, and must leave the check armed for the frames after it, + /// since a server tags what it chooses to and an attacker would otherwise only have to + /// let one untagged frame through to be rid of the check. + fn check_kx_advertised_echo(plain: &[u8], seen: u32) -> Result { let echoed = crate::rendezvous_proto::RendezvousMessage::parse_from_bytes(plain) .map(|m| m.kx_advertised) .unwrap_or(0); - if echoed != 0 && echoed != seen { + if echoed == 0 { + return Ok(false); + } + if echoed != seen { return Err(Error::new( ErrorKind::Other, format!( @@ -376,7 +382,7 @@ impl Encrypt { ), )); } - Ok(()) + Ok(true) } /// Version 1: the initiator, the side that sent the sealed key, sends under one subkey and @@ -423,8 +429,10 @@ impl Encrypt { let nonce = FramedStream::get_nonce(self.2); match secretbox::open(bytes, &nonce, self.3.as_ref().unwrap_or(&self.0)) { Ok(res) => { - if let Some(seen) = self.4.take() { - Self::check_kx_advertised_echo(&res, seen)?; + if let Some(seen) = self.4 { + if Self::check_kx_advertised_echo(&res, seen)? { + self.4 = None; + } } bytes.clear(); bytes.put_slice(&res); @@ -585,6 +593,20 @@ mod tests { assert!(first_frame_after(KX_VERSION_LATEST, KX_VERSION_LATEST + 1).is_err()); } + #[test] + fn test_kx_advertised_check_survives_an_untagged_frame() { + let key = secretbox::gen_key(); + let mut server = Encrypt::new(key.clone()); + let mut client = Encrypt::new(key); + client.check_kx_advertised(KX_VERSION_LATEST); + // A message the server does not tag says nothing about the advertisement. + let mut untagged = BytesMut::from(&server.enc(&rendezvous_frame(0))[..]); + client.dec(&mut untagged).unwrap(); + // So the frame that does carry an echo is still held to it. + let mut tagged = BytesMut::from(&server.enc(&rendezvous_frame(KX_VERSION_LATEST + 1))[..]); + assert!(client.dec(&mut tagged).is_err()); + } + #[test] fn test_kx_advertised_check_is_one_shot() { let key = secretbox::gen_key(); From 445486ccaae6cbe9c45adca11916a04969cdefd1 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Mon, 21 Sep 2026 12:30:10 +0800 Subject: [PATCH 03/13] tcp: drop box_pk_of until something calls it It has no caller here or in the servers; the responder that would keep only the secret half does not exist yet, so it belongs to that change. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EZ49AbZJYfm8NTp5yDPMab --- src/tcp.rs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/tcp.rs b/src/tcp.rs index f53302b37..2204c1c18 100644 --- a/src/tcp.rs +++ b/src/tcp.rs @@ -4,7 +4,7 @@ use bytes::{BufMut, Bytes, BytesMut}; use futures::{SinkExt, StreamExt}; use protobuf::Message; use sodiumoxide::crypto::{ - box_, generichash, scalarmult, + box_, generichash, secretbox::{self, Key, Nonce}, }; use std::{ @@ -56,11 +56,6 @@ pub struct KxTranscript<'a> { pub picked: u32, } -/// The public half of an X25519 secret key, for a responder that kept only the secret half. -pub fn box_pk_of(sk: &box_::SecretKey) -> box_::PublicKey { - box_::PublicKey(scalarmult::scalarmult_base(&scalarmult::Scalar(sk.0)).0) -} - /// The sending key, the send and receive counters, the receiving key where it differs, and /// the server's advertised version as this side saw it, while its echo is still awaited. #[derive(Clone)] @@ -618,10 +613,4 @@ mod tests { let mut later = BytesMut::from(&server.enc(&rendezvous_frame(KX_VERSION_LATEST + 9))[..]); client.dec(&mut later).unwrap(); } - - #[test] - fn test_box_pk_of_matches_the_generated_pair() { - let (pk, sk) = box_::gen_keypair(); - assert_eq!(box_pk_of(&sk), pk); - } } From 1266b56150da889dfec318e68b9b20d7c30c0bb6 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Mon, 21 Sep 2026 14:19:34 +0800 Subject: [PATCH 04/13] kx_advertised: every message after the exchange, not the first alone The contract named the server's first message after the key exchange. That frame sits at a position an attacker needs no key to find, so one who had lowered the advertisement in the clear only had to drop it to be rid of the check for the whole session. With every message carrying the echo, being rid of it means dropping them all, which is a plain DoS. The client already kept the check armed until an echo arrived; this is the wording the server side is written to. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01EZ49AbZJYfm8NTp5yDPMab --- protos/rendezvous.proto | 6 ++++-- src/stream.rs | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/protos/rendezvous.proto b/protos/rendezvous.proto index e8c7d6c2a..fe0ef637a 100644 --- a/protos/rendezvous.proto +++ b/protos/rendezvous.proto @@ -297,9 +297,11 @@ message RendezvousMessage { HttpProxyResponse http_proxy_response = 28; IceCandidate ice_candidate = 29; } - // From the server, on its first message after a key exchange: the version it advertised, + // From the server, on every message after a key exchange: the version it advertised, // repeated inside the encrypted channel where nothing in transit can alter it. A client that // was shown a different value in the clear knows the handshake was altered and refuses the - // stream. Absent from servers that predate versions. + // stream. Every message and not the first alone, so that dropping one frame at a position + // that needs no key to find does not rid an attacker of the check; dropping them all is a + // plain DoS. Absent from servers that predate versions. uint32 kx_advertised = 30; } diff --git a/src/stream.rs b/src/stream.rs index 8702828da..e10c7e3d2 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -100,7 +100,7 @@ impl Stream { } } - /// Refuse the stream if the server's first encrypted message says it advertised a key + /// Refuse the stream if an encrypted message from the server says it advertised a key /// exchange version other than `seen`, the one this side received in the clear. #[inline] pub fn check_kx_advertised(&mut self, seen: u32) { From d9f519e91581686466fa625fa4edb61341dbe886 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Tue, 22 Sep 2026 13:18:45 +0800 Subject: [PATCH 05/13] KeyExchange: signed_version, the server's signature over its version The echo catches an advertisement lowered in the clear only once a tagged frame arrives, so the initiator's first frame has already gone out under the lower version by then. A server that signs its version instead sets bit 255 of the ephemeral X25519 key inside the signed keys[0] and puts sign("rdkx-ver" || key || version LE) here. RFC 7748 masks that bit, so the box opens as before on either side, and a legacy server, whose keys are canonical, never sets it. A client that sees the bit, or the field, verifies before trusting the version, so the pair settles it at the handshake rather than one frame pair later. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns --- protos/rendezvous.proto | 3 +++ 1 file changed, 3 insertions(+) diff --git a/protos/rendezvous.proto b/protos/rendezvous.proto index fe0ef637a..f126714a8 100644 --- a/protos/rendezvous.proto +++ b/protos/rendezvous.proto @@ -238,6 +238,9 @@ message KeyExchange { // From the server: the newest key exchange version it speaks. From the client: the version it // picked, the highest both speak. Absent, i.e. 0, is the original scheme. uint32 version = 2; + // Server signature of "rdkx-ver" || on-wire ephemeral public key || little-endian uint32 version. + // Bit 255 of the signed X25519 public key requires this field; legacy keys leave it clear. + bytes signed_version = 3; } message HealthCheck { From e3452acf599135bf889070b0aff166b2d570eb87 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Tue, 22 Sep 2026 15:02:44 +0800 Subject: [PATCH 06/13] kx: comments say what the fields and functions are Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns --- protos/rendezvous.proto | 9 ++------- src/stream.rs | 2 +- src/tcp.rs | 31 ++++++++++++------------------- 3 files changed, 15 insertions(+), 27 deletions(-) diff --git a/protos/rendezvous.proto b/protos/rendezvous.proto index f126714a8..ccdde0e79 100644 --- a/protos/rendezvous.proto +++ b/protos/rendezvous.proto @@ -9,7 +9,6 @@ message IdPk { // that swaps SDP fingerprints). Empty for non-WebRTC handshakes. string dtls_fingerprint = 3; // Newest key exchange version the signer speaks; absent, i.e. 0, is the original scheme. - // Inside the signed identity so a rendezvous or relay cannot lower it. uint32 kx_version = 4; } @@ -300,11 +299,7 @@ message RendezvousMessage { HttpProxyResponse http_proxy_response = 28; IceCandidate ice_candidate = 29; } - // From the server, on every message after a key exchange: the version it advertised, - // repeated inside the encrypted channel where nothing in transit can alter it. A client that - // was shown a different value in the clear knows the handshake was altered and refuses the - // stream. Every message and not the first alone, so that dropping one frame at a position - // that needs no key to find does not rid an attacker of the check; dropping them all is a - // plain DoS. Absent from servers that predate versions. + // From the server, on every message after a key exchange: the version it advertised. + // Absent from servers that predate versions. uint32 kx_advertised = 30; } diff --git a/src/stream.rs b/src/stream.rs index e10c7e3d2..ad08f0549 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -101,7 +101,7 @@ impl Stream { } /// Refuse the stream if an encrypted message from the server says it advertised a key - /// exchange version other than `seen`, the one this side received in the clear. + /// exchange version other than `seen`, the one this side read from the key exchange. #[inline] pub fn check_kx_advertised(&mut self, seen: u32) { match self { diff --git a/src/tcp.rs b/src/tcp.rs index 2204c1c18..0f2e27ed0 100644 --- a/src/tcp.rs +++ b/src/tcp.rs @@ -25,10 +25,9 @@ pub trait TcpStreamTrait: AsyncRead + AsyncWrite + Unpin {} pub struct DynTcpStream(pub Box); /// The newest key exchange version this build speaks. Version 0 is the original scheme, and -/// what an absent field means: one key for both directions, each direction counting its own -/// nonce from 1, so the n-th message each way is sealed under the same (key, nonce). Version 1 -/// splits the exchanged key into one per direction, the way Noise splits its cipher states -/// after the handshake, binds the handshake transcript into both, and keeps the nonce layout. +/// what an absent field means: one key for both directions. Version 1 splits the exchanged key +/// into one per direction, the way Noise splits its cipher states after the handshake, binds +/// the handshake transcript into both, and keeps the nonce layout. pub const KX_VERSION_LATEST: u32 = 1; /// The version to run against a peer that advertised `advertised`: the highest both sides @@ -42,9 +41,8 @@ const KX_SPLIT_CONTEXT: &[u8] = b"rdkx-spl"; const KX_SPLIT_INITIATOR: u8 = 1; const KX_SPLIT_RESPONDER: u8 = 2; -/// What both sides saw during the key exchange, each from its own side of the wire. Mixed into -/// the split keys, so a change to any of it in transit leaves the two sides with different keys -/// and the first message undecryptable, without a check per field. +/// What both sides saw during the key exchange, each from its own side of the wire, mixed +/// into the split keys. pub struct KxTranscript<'a> { /// The ephemeral public key of the side that sent the sealed key. pub initiator_pk: &'a [u8], @@ -349,18 +347,14 @@ impl Encrypt { } /// Arm the check of the server's advertisement echo, `seen` being the version this side - /// received in the clear. Call once the key is set and before the first frame is read. + /// read from the key exchange. Call once the key is set and before the first frame is read. pub fn check_kx_advertised(&mut self, seen: u32) { self.4 = Some(seen); } - /// The server repeats what it advertised inside an encrypted message, where it can be - /// neither forged nor stripped. A different value in the clear means the handshake was - /// altered on the way, and the stream is refused rather than run at the version it was - /// pushed down to. Returns whether the echo settled the question: a frame that carries - /// none says nothing either way, and must leave the check armed for the frames after it, - /// since a server tags what it chooses to and an attacker would otherwise only have to - /// let one untagged frame through to be rid of the check. + /// The server repeats what it advertised inside its encrypted messages. Returns whether the + /// echo settled the question: a frame that carries none says nothing either way, and leaves + /// the check armed for the frames after it. fn check_kx_advertised_echo(plain: &[u8], seen: u32) -> Result { let echoed = crate::rendezvous_proto::RendezvousMessage::parse_from_bytes(plain) .map(|m| m.kx_advertised) @@ -496,8 +490,7 @@ mod tests { let (mut initiator, mut responder) = (Encrypt::new(key.clone()), Encrypt::new(key)); let sent = seal_and_open(&mut initiator, &mut responder, b"hello"); let back = seal_and_open(&mut responder, &mut initiator, b"hello"); - // The n-th message each way is sealed under the same (key, nonce): the defect the split - // fixes, pinned here so the path a peer without versions relies on stays byte-for-byte. + // Pinned so the path a peer without versions relies on stays byte-for-byte. assert_eq!(sent, back); } @@ -539,7 +532,7 @@ mod tests { #[test] fn test_split_binds_the_transcript() { let key = secretbox::gen_key(); - // The responder advertised LATEST; the initiator was handed something else in transit. + // The two sides put different advertisements into the transcript. let mut initiator = Encrypt::new_split(key.clone(), true, &transcript(KX_VERSION_LATEST + 1)).unwrap(); let mut responder = Encrypt::new_split(key, false, &transcript(KX_VERSION_LATEST)).unwrap(); @@ -583,7 +576,7 @@ mod tests { assert!(first_frame_after(KX_VERSION_LATEST, KX_VERSION_LATEST).is_ok()); // A server from before versions echoes nothing, and advertised nothing. assert!(first_frame_after(0, 0).is_ok()); - // The advertisement was lowered on the way; the echo says otherwise. + // The echo differs from what this side saw. assert!(first_frame_after(0, KX_VERSION_LATEST).is_err()); assert!(first_frame_after(KX_VERSION_LATEST, KX_VERSION_LATEST + 1).is_err()); } From 5194159858b7b92846658b15f9c64045ce5f745e Mon Sep 17 00:00:00 2001 From: rustdesk Date: Tue, 22 Sep 2026 17:22:01 +0800 Subject: [PATCH 07/13] kx: drop the advertisement echo signed_version settles the server's version inside the key exchange, so an echo of it on the messages after has nothing left to confirm. With it go RendezvousMessage.kx_advertised, the check the transport ran on each decrypted frame, and the tests of both. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns --- protos/rendezvous.proto | 3 -- src/stream.rs | 12 ----- src/tcp.rs | 105 ++-------------------------------------- src/websocket.rs | 7 --- 4 files changed, 5 insertions(+), 122 deletions(-) diff --git a/protos/rendezvous.proto b/protos/rendezvous.proto index ccdde0e79..b1e405f94 100644 --- a/protos/rendezvous.proto +++ b/protos/rendezvous.proto @@ -299,7 +299,4 @@ message RendezvousMessage { HttpProxyResponse http_proxy_response = 28; IceCandidate ice_candidate = 29; } - // From the server, on every message after a key exchange: the version it advertised. - // Absent from servers that predate versions. - uint32 kx_advertised = 30; } diff --git a/src/stream.rs b/src/stream.rs index ad08f0549..6260f39a5 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -100,18 +100,6 @@ impl Stream { } } - /// Refuse the stream if an encrypted message from the server says it advertised a key - /// exchange version other than `seen`, the one this side read from the key exchange. - #[inline] - pub fn check_kx_advertised(&mut self, seen: u32) { - match self { - #[cfg(feature = "webrtc")] - Stream::WebRTC(_) => {} - Stream::WebSocket(s) => s.check_kx_advertised(seen), - Stream::Tcp(s) => s.check_kx_advertised(seen), - } - } - /// An opaque token that changes whenever bytes have arrived from the peer, a fragment of a /// message that is still incomplete included. Compare successive samples; the value itself /// means nothing. `None` where the transport reports no such thing. diff --git a/src/tcp.rs b/src/tcp.rs index 0f2e27ed0..d04ed1e66 100644 --- a/src/tcp.rs +++ b/src/tcp.rs @@ -54,10 +54,9 @@ pub struct KxTranscript<'a> { pub picked: u32, } -/// The sending key, the send and receive counters, the receiving key where it differs, and -/// the server's advertised version as this side saw it, while its echo is still awaited. +/// The sending key, the send and receive counters, and the receiving key where it differs. #[derive(Clone)] -pub struct Encrypt(pub Key, pub u64, pub u64, Option, Option); +pub struct Encrypt(pub Key, pub u64, pub u64, Option); pub struct FramedStream( pub Framed, @@ -242,12 +241,6 @@ impl FramedStream { Ok(()) } - pub fn check_kx_advertised(&mut self, seen: u32) { - if let Some(enc) = self.2.as_mut() { - enc.check_kx_advertised(seen); - } - } - fn get_nonce(seqnum: u64) -> Nonce { let mut nonce = Nonce([0u8; secretbox::NONCEBYTES]); nonce.0[..std::mem::size_of_val(&seqnum)].copy_from_slice(&seqnum.to_le_bytes()); @@ -343,35 +336,7 @@ impl TcpStreamTrait for R {} impl Encrypt { pub fn new(key: Key) -> Self { - Self(key, 0, 0, None, None) - } - - /// Arm the check of the server's advertisement echo, `seen` being the version this side - /// read from the key exchange. Call once the key is set and before the first frame is read. - pub fn check_kx_advertised(&mut self, seen: u32) { - self.4 = Some(seen); - } - - /// The server repeats what it advertised inside its encrypted messages. Returns whether the - /// echo settled the question: a frame that carries none says nothing either way, and leaves - /// the check armed for the frames after it. - fn check_kx_advertised_echo(plain: &[u8], seen: u32) -> Result { - let echoed = crate::rendezvous_proto::RendezvousMessage::parse_from_bytes(plain) - .map(|m| m.kx_advertised) - .unwrap_or(0); - if echoed == 0 { - return Ok(false); - } - if echoed != seen { - return Err(Error::new( - ErrorKind::Other, - format!( - "key exchange advertised {} but {} arrived: altered in transit", - echoed, seen - ), - )); - } - Ok(true) + Self(key, 0, 0, None) } /// Version 1: the initiator, the side that sent the sealed key, sends under one subkey and @@ -380,9 +345,9 @@ impl Encrypt { let initiator = Self::derive_subkey(&key, KX_SPLIT_INITIATOR, t)?; let responder = Self::derive_subkey(&key, KX_SPLIT_RESPONDER, t)?; Ok(if is_initiator { - Self(initiator, 0, 0, Some(responder), None) + Self(initiator, 0, 0, Some(responder)) } else { - Self(responder, 0, 0, Some(initiator), None) + Self(responder, 0, 0, Some(initiator)) }) } @@ -418,11 +383,6 @@ impl Encrypt { let nonce = FramedStream::get_nonce(self.2); match secretbox::open(bytes, &nonce, self.3.as_ref().unwrap_or(&self.0)) { Ok(res) => { - if let Some(seen) = self.4 { - if Self::check_kx_advertised_echo(&res, seen)? { - self.4 = None; - } - } bytes.clear(); bytes.put_slice(&res); Ok(()) @@ -551,59 +511,4 @@ mod tests { }; assert!(Encrypt::new_split(secretbox::gen_key(), true, &t).is_err()); } - - fn rendezvous_frame(kx_advertised: u32) -> Vec { - let mut msg = crate::rendezvous_proto::RendezvousMessage::new(); - msg.set_hc(Default::default()); - msg.kx_advertised = kx_advertised; - msg.write_to_bytes().unwrap() - } - - fn first_frame_after(seen: u32, echoed: u32) -> Result<(), Error> { - let key = secretbox::gen_key(); - let mut server = Encrypt::new(key.clone()); - let mut client = Encrypt::new(key); - client.check_kx_advertised(seen); - // An empty heartbeat is not a frame, and must not use up the check. - let mut empty = BytesMut::new(); - client.dec(&mut empty).unwrap(); - let mut buf = BytesMut::from(&server.enc(&rendezvous_frame(echoed))[..]); - client.dec(&mut buf) - } - - #[test] - fn test_kx_advertised_echo_must_match_what_was_seen() { - assert!(first_frame_after(KX_VERSION_LATEST, KX_VERSION_LATEST).is_ok()); - // A server from before versions echoes nothing, and advertised nothing. - assert!(first_frame_after(0, 0).is_ok()); - // The echo differs from what this side saw. - assert!(first_frame_after(0, KX_VERSION_LATEST).is_err()); - assert!(first_frame_after(KX_VERSION_LATEST, KX_VERSION_LATEST + 1).is_err()); - } - - #[test] - fn test_kx_advertised_check_survives_an_untagged_frame() { - let key = secretbox::gen_key(); - let mut server = Encrypt::new(key.clone()); - let mut client = Encrypt::new(key); - client.check_kx_advertised(KX_VERSION_LATEST); - // A message the server does not tag says nothing about the advertisement. - let mut untagged = BytesMut::from(&server.enc(&rendezvous_frame(0))[..]); - client.dec(&mut untagged).unwrap(); - // So the frame that does carry an echo is still held to it. - let mut tagged = BytesMut::from(&server.enc(&rendezvous_frame(KX_VERSION_LATEST + 1))[..]); - assert!(client.dec(&mut tagged).is_err()); - } - - #[test] - fn test_kx_advertised_check_is_one_shot() { - let key = secretbox::gen_key(); - let mut server = Encrypt::new(key.clone()); - let mut client = Encrypt::new(key); - client.check_kx_advertised(KX_VERSION_LATEST); - let mut first = BytesMut::from(&server.enc(&rendezvous_frame(KX_VERSION_LATEST))[..]); - client.dec(&mut first).unwrap(); - let mut later = BytesMut::from(&server.enc(&rendezvous_frame(KX_VERSION_LATEST + 9))[..]); - client.dec(&mut later).unwrap(); - } } diff --git a/src/websocket.rs b/src/websocket.rs index d4dc85883..6c506250b 100644 --- a/src/websocket.rs +++ b/src/websocket.rs @@ -268,13 +268,6 @@ impl WsFramedStream { Ok(()) } - #[inline] - pub fn check_kx_advertised(&mut self, seen: u32) { - if let Some(enc) = self.encrypt.as_mut() { - enc.check_kx_advertised(seen); - } - } - #[inline] pub fn is_secured(&self) -> bool { self.encrypt.is_some() From e74a188f4fa13e78f8ff0c51383d758f8a62ab59 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Tue, 22 Sep 2026 17:43:38 +0800 Subject: [PATCH 08/13] KeyExchange: sign KxParams, a structure, in place of a fixed byte string The server's second signature covered "rdkx-ver" || key || version, a layout that could take nothing more. It now covers KxParams, a message holding the key as it went out and the advertised version, so anything the server needs signed later is a field added to it, and a client that does not know a field parses past it, as with IdPk on the peer side. Field 3 keeps its number under the name signed_params. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns --- protos/rendezvous.proto | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/protos/rendezvous.proto b/protos/rendezvous.proto index b1e405f94..3cee04e2e 100644 --- a/protos/rendezvous.proto +++ b/protos/rendezvous.proto @@ -237,9 +237,16 @@ message KeyExchange { // From the server: the newest key exchange version it speaks. From the client: the version it // picked, the highest both speak. Absent, i.e. 0, is the original scheme. uint32 version = 2; - // Server signature of "rdkx-ver" || on-wire ephemeral public key || little-endian uint32 version. - // Bit 255 of the signed X25519 public key requires this field; legacy keys leave it clear. - bytes signed_version = 3; + // Server signature of KxParams. Bit 255 of the signed X25519 public key requires this field; + // legacy keys leave it clear. + bytes signed_params = 3; +} + +// Signed by the server beside its ephemeral public key: that key as it went out, so the signature +// belongs to this exchange alone, and the newest key exchange version the server speaks. +message KxParams { + bytes pk = 1; + uint32 version = 2; } message HealthCheck { From 26582e163d7bb581d00f02094c68a739d01e157a Mon Sep 17 00:00:00 2001 From: rustdesk Date: Tue, 22 Sep 2026 17:59:11 +0800 Subject: [PATCH 09/13] tcp: KX_PARAMS_DOMAIN, the prefix under which KxParams is signed The server signs its ephemeral key, IdPk records and KxParams with one key, and a protobuf carries no type of its own: a signed IdPk parses as a KxParams whose pk is the id and whose version is 0. The prefix keeps the three apart; the client requires it before parsing. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns --- src/tcp.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tcp.rs b/src/tcp.rs index d04ed1e66..35d22d639 100644 --- a/src/tcp.rs +++ b/src/tcp.rs @@ -30,6 +30,10 @@ pub struct DynTcpStream(pub Box); /// the handshake transcript into both, and keeps the nonce layout. pub const KX_VERSION_LATEST: u32 = 1; +/// Prefix of what the server signs as `KxParams`. The same key signs other messages, and a +/// protobuf carries no type of its own, so the prefix is what keeps them apart. +pub const KX_PARAMS_DOMAIN: &[u8] = b"rdkx-params"; + /// The version to run against a peer that advertised `advertised`: the highest both sides /// support. #[inline] From cbc8772d4ab749c70634b2f1c7e76e509e69f363 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Wed, 23 Sep 2026 09:53:34 +0800 Subject: [PATCH 10/13] stream: set_negotiated_key, one place that maps the picked version to a key Every handshake picked a version and then repeated the same choice: at least 1 splits the key, 0 sets it as before. Stream now makes that choice from the transcript it is already given, so callers make one call and a later version is handled here rather than at each of them. Version 0 still goes through set_key. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns --- src/stream.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/stream.rs b/src/stream.rs index 6260f39a5..6571520c3 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -80,15 +80,20 @@ impl Stream { } } - /// Per-direction keys bound to the handshake; see `tcp::KX_VERSION_LATEST`. `is_initiator` - /// is the side that sent the sealed key. + /// The stream key for the key exchange version in `t.picked`: version 0 is `set_key`, and + /// later versions derive one key per direction from the transcript. `is_initiator` is the + /// side that sent the sealed key. #[inline] - pub fn set_key_split( + pub fn set_negotiated_key( &mut self, key: Key, is_initiator: bool, t: &tcp::KxTranscript, ) -> ResultType<()> { + if t.picked == 0 { + self.set_key(key); + return Ok(()); + } match self { #[cfg(feature = "webrtc")] Stream::WebRTC(s) => { From cee8cf3dc3b3119622ca1d2adabef27eb3833143 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Wed, 23 Sep 2026 13:20:36 +0800 Subject: [PATCH 11/13] stream: refuse a key exchange version set_negotiated_key does not know Version 1 is matched explicitly and any other picked version is an error, so adding a version without its arm here fails instead of running as version 1. WebRTC takes only version 0, as both handshakes already require. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns --- src/stream.rs | 47 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/src/stream.rs b/src/stream.rs index 6571520c3..a486302d4 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -1,4 +1,4 @@ -use crate::{config, tcp, websocket, ResultType}; +use crate::{bail, config, tcp, websocket, ResultType}; #[cfg(feature = "webrtc")] use crate::webrtc; use sodiumoxide::crypto::secretbox::Key; @@ -81,8 +81,8 @@ impl Stream { } /// The stream key for the key exchange version in `t.picked`: version 0 is `set_key`, and - /// later versions derive one key per direction from the transcript. `is_initiator` is the - /// side that sent the sealed key. + /// version 1 derives one key per direction from the transcript. Any other version is refused + /// rather than run as one of these. `is_initiator` is the side that sent the sealed key. #[inline] pub fn set_negotiated_key( &mut self, @@ -90,18 +90,18 @@ impl Stream { is_initiator: bool, t: &tcp::KxTranscript, ) -> ResultType<()> { - if t.picked == 0 { - self.set_key(key); - return Ok(()); - } - match self { - #[cfg(feature = "webrtc")] - Stream::WebRTC(s) => { - s.set_key(key); + match t.picked { + 0 => { + self.set_key(key); Ok(()) } - Stream::WebSocket(s) => s.set_key_split(key, is_initiator, t), - Stream::Tcp(s) => s.set_key_split(key, is_initiator, t), + 1 => match self { + #[cfg(feature = "webrtc")] + Stream::WebRTC(_) => bail!("key exchange version 1 does not apply to WebRTC"), + Stream::WebSocket(s) => s.set_key_split(key, is_initiator, t), + Stream::Tcp(s) => s.set_key_split(key, is_initiator, t), + }, + v => bail!("unsupported key exchange version {}", v), } } @@ -313,3 +313,24 @@ impl Drop for Stream { self.close_webrtc(); } } + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn test_set_negotiated_key_refuses_unknown_versions() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let mut stream = Stream::from(TcpStream::connect(addr).await.unwrap(), addr); + let t = |picked| tcp::KxTranscript { + initiator_pk: &[1u8; 32], + responder_pk: &[2u8; 32], + advertised: 1, + picked, + }; + let mut set = |picked| stream.set_negotiated_key(Key([0u8; 32]), true, &t(picked)); + assert!(set(2).is_err()); + assert!(set(1).is_ok()); + } +} From 28492584606edf699cdd3294bd2e0e6e4cec7788 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Wed, 23 Sep 2026 13:20:36 +0800 Subject: [PATCH 12/13] tcp: pin the version 0 frame and the version 1 subkeys The existing tests run both ends on the same code, so they pass even if the derivation changes. Fixed bytes catch that: the version 0 frame released peers send, and the same subkey vector hbbs's copy of this code pins. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns --- src/tcp.rs | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/tcp.rs b/src/tcp.rs index 35d22d639..332e404d7 100644 --- a/src/tcp.rs +++ b/src/tcp.rs @@ -454,10 +454,36 @@ mod tests { let (mut initiator, mut responder) = (Encrypt::new(key.clone()), Encrypt::new(key)); let sent = seal_and_open(&mut initiator, &mut responder, b"hello"); let back = seal_and_open(&mut responder, &mut initiator, b"hello"); - // Pinned so the path a peer without versions relies on stays byte-for-byte. assert_eq!(sent, back); } + // Fixed bytes: the version 0 frame is what released peers send, and the subkeys are the ones + // hbbs's own copy of this code pins, so a change here breaks the wire, not just this test. + #[test] + fn test_wire_vectors() { + let hex = |b: &[u8]| b.iter().map(|b| format!("{:02x}", b)).collect::(); + let key = Key([0x11u8; 32]); + assert_eq!( + hex(&Encrypt::new(key.clone()).enc(b"hello")), + "3b768f827fdcc4af555b9e42533be6611e2c333481" + ); + let t = KxTranscript { + initiator_pk: &[0x22u8; 32], + responder_pk: &[0x33u8; 32], + advertised: 1, + picked: 1, + }; + let Encrypt(send, _, _, recv) = Encrypt::new_split(key, true, &t).unwrap(); + assert_eq!( + hex(&send.0), + "c189ec6e1935c0751cfc85a0f075405cae507d7925bb19687caf56cbb54e0ecf" + ); + assert_eq!( + hex(&recv.unwrap().0), + "8a62648e9195cb10ea900c0a24d2a166c1e982347a2dc6833c3e756bf5715d36" + ); + } + #[test] fn test_split_directions_use_different_keys() { let key = secretbox::gen_key(); From 42c77b9a3b91bb4b651d9e506bccae5f508202d8 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Wed, 23 Sep 2026 14:27:51 +0800 Subject: [PATCH 13/13] tcp: pin the subkeys for an advertisement above the pick With advertised and picked both 1, swapping their order in the derivation leaves the vector unchanged. Advertised 3 with picked 1 is what a client meets at a newer server, and tells the two apart. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns --- src/tcp.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/tcp.rs b/src/tcp.rs index 332e404d7..b457bbe03 100644 --- a/src/tcp.rs +++ b/src/tcp.rs @@ -467,13 +467,13 @@ mod tests { hex(&Encrypt::new(key.clone()).enc(b"hello")), "3b768f827fdcc4af555b9e42533be6611e2c333481" ); - let t = KxTranscript { + let mut t = KxTranscript { initiator_pk: &[0x22u8; 32], responder_pk: &[0x33u8; 32], advertised: 1, picked: 1, }; - let Encrypt(send, _, _, recv) = Encrypt::new_split(key, true, &t).unwrap(); + let Encrypt(send, _, _, recv) = Encrypt::new_split(key.clone(), true, &t).unwrap(); assert_eq!( hex(&send.0), "c189ec6e1935c0751cfc85a0f075405cae507d7925bb19687caf56cbb54e0ecf" @@ -482,6 +482,17 @@ mod tests { hex(&recv.unwrap().0), "8a62648e9195cb10ea900c0a24d2a166c1e982347a2dc6833c3e756bf5715d36" ); + // With the two versions equal, swapping their order in the derivation would go unseen. + t.advertised = 3; + let Encrypt(send, _, _, recv) = Encrypt::new_split(key, true, &t).unwrap(); + assert_eq!( + hex(&send.0), + "25b0778ae600f9731c7fa457d1cc5d40071b0aa3d34c90b53e1d5344b0e7123a" + ); + assert_eq!( + hex(&recv.unwrap().0), + "5bbde0ac6a708d4cd1ff807125d79cb6030bde62f7a66b6cc1a79e31bd657756" + ); } #[test]