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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns
This commit is contained in:
rustdesk
2026-09-22 17:22:01 +08:00
co-authored by Claude Fable 5.1
parent e3452acf59
commit 5194159858
4 changed files with 5 additions and 122 deletions
-3
View File
@@ -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;
}
-12
View File
@@ -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.
+5 -100
View File
@@ -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<Key>, Option<u32>);
pub struct Encrypt(pub Key, pub u64, pub u64, Option<Key>);
pub struct FramedStream(
pub Framed<DynTcpStream, BytesCodec>,
@@ -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<R: AsyncRead + AsyncWrite + Unpin> 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<bool, Error> {
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<u8> {
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();
}
}
-7
View File
@@ -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()