From 594e63805c2fa2e6b214c07dd7869dba52f4d627 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Mon, 10 Aug 2026 16:05:13 +0800 Subject: [PATCH] harden login request retry --- src/server/connection.rs | 144 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/src/server/connection.rs b/src/server/connection.rs index 25d9b6792..90729fb8d 100644 --- a/src/server/connection.rs +++ b/src/server/connection.rs @@ -363,6 +363,9 @@ pub struct Connection { server_audit_file: String, controlled_context: Option, lr: LoginRequest, + // Authentication retries may update credentials, but not the requested session scope. + // A digest, so no peer-controlled strings are retained. + login_scope: Option<[u8; 32]>, peer_argb: u32, session_last_recv_time: Option>>, chat_unanswered: bool, @@ -560,6 +563,7 @@ impl Connection { server_audit_file: "".to_owned(), controlled_context, lr: Default::default(), + login_scope: None, peer_argb: 0u32, session_last_recv_time: None, chat_unanswered: false, @@ -2621,6 +2625,90 @@ impl Connection { self.terminal_persistent = false; } + // Approval and whitelist decisions must stay bound to the same controller identity and + // session scope across authentication retries. + fn login_scope_digest(lr: &LoginRequest) -> [u8; 32] { + let mut hasher = Sha256::new(); + // Length-prefixed so adjacent fields cannot alias. + let mut push = |bytes: &[u8]| { + hasher.update((bytes.len() as u64).to_le_bytes()); + hasher.update(bytes); + }; + push(lr.my_id.as_bytes()); + // Payloads are destructured exhaustively: a new field fails to compile until it is + // either latched here or deliberately ignored. + match lr.union.as_ref() { + Some(login_request::Union::FileTransfer(ft)) => { + let FileTransfer { + dir, + show_hidden, + special_fields: _, + } = ft; + push(b"file_transfer"); + push(dir.as_bytes()); + push(&[*show_hidden as u8]); + } + Some(login_request::Union::ViewCamera(vc)) => { + let ViewCamera { special_fields: _ } = vc; + push(b"view_camera"); + } + Some(login_request::Union::Terminal(t)) => { + let Terminal { + service_id, + special_fields: _, + } = t; + push(b"terminal"); + push(service_id.as_bytes()); + } + Some(login_request::Union::PortForward(pf)) => { + let PortForward { + host, + port, + special_fields: _, + } = pf; + push(b"port_forward"); + push(host.as_bytes()); + push(&port.to_le_bytes()); + } + // Variants this build does not know execute as remote, so they latch as remote. + None | Some(_) => push(b"remote"), + } + hasher.finalize().into() + } + + // Logging only; security decisions compare digests. + fn login_scope_kind(lr: &LoginRequest) -> &'static str { + match lr.union.as_ref() { + Some(login_request::Union::FileTransfer(_)) => "file_transfer", + Some(login_request::Union::ViewCamera(_)) => "view_camera", + Some(login_request::Union::Terminal(_)) => "terminal", + Some(login_request::Union::PortForward(_)) => "port_forward", + _ => "remote", + } + } + + async fn check_login_scope(&mut self, lr: &LoginRequest) -> bool { + let requested = Self::login_scope_digest(lr); + match self.login_scope { + Some(initial) if initial != requested => { + // self.lr still holds the first accepted request, whose scope is the latched one. + log::warn!( + "Rejected login scope change: conn_id={}, initial={}, requested={}", + self.inner.id(), + Self::login_scope_kind(&self.lr), + Self::login_scope_kind(lr), + ); + self.send_login_error("Connection not allowed").await; + false + } + Some(_) => true, + None => { + self.login_scope = Some(requested); + true + } + } + } + async fn handle_login_request_without_validation(&mut self, lr: &LoginRequest) { self.lr = lr.clone(); self.peer_argb = crate::str2color(&format!("{}{}", &lr.my_id, &lr.my_platform), 0xff); @@ -2698,6 +2786,9 @@ impl Connection { } // After handling CloseReason messages, proceed to process other message types if let Some(message::Union::LoginRequest(lr)) = msg.union { + if !self.check_login_scope(&lr).await { + return false; + } self.awaiting_2fa = false; self.handle_login_request_without_validation(&lr).await; if self.authorized { @@ -7003,6 +7094,59 @@ mod test { #[allow(unused)] use super::*; + #[test] + fn login_scope_latches_session_scope_across_login_retries() { + let port_forward = |host: &str| { + let mut lr = LoginRequest::new(); + lr.my_id = "peer".to_owned(); + lr.set_port_forward(PortForward { + host: host.to_owned(), + port: 3389, + ..Default::default() + }); + lr + }; + let first = port_forward("localhost"); + let scope = |lr: &LoginRequest| Connection::login_scope_digest(lr); + + // A retry may carry new credentials, profile data, options, and unknown fields. + let mut retry = port_forward("localhost"); + retry.password = "secret".into(); + retry.hwid = "hwid".into(); + retry.os_login = Some(OSLogin { + username: "admin".to_owned(), + ..Default::default() + }) + .into(); + retry.my_name = "New Display Name".to_owned(); + retry.avatar = "data:image/png;base64,AAAA".to_owned(); + retry + .special_fields + .mut_unknown_fields() + .add_varint(9999, 1); + assert_eq!(scope(&first), scope(&retry)); + + // It may not change the controller identity, move the target, or switch type. + let mut rotated_id = first.clone(); + rotated_id.my_id = "rotated-id".to_owned(); + assert_ne!(scope(&first), scope(&rotated_id)); + assert_ne!(scope(&first), scope(&port_forward("10.0.0.5"))); + let mut moved_port = port_forward("localhost"); + moved_port.mut_port_forward().port = 22; + assert_ne!(scope(&first), scope(&moved_port)); + let terminal = |service_id: &str| { + let mut lr = LoginRequest::new(); + lr.my_id = "peer".to_owned(); + lr.set_terminal(Terminal { + service_id: service_id.to_owned(), + ..Default::default() + }); + lr + }; + assert_ne!(scope(&first), scope(&terminal(""))); + assert_ne!(scope(&terminal("a")), scope(&terminal("b"))); + } + #[test] fn test_wildcard_match() { // Exact match.