From 5194159858b7b92846658b15f9c64045ce5f745e Mon Sep 17 00:00:00 2001 From: rustdesk Date: Tue, 22 Sep 2026 17:22:01 +0800 Subject: [PATCH] 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()