From 15bad97d50b0188bf047c4f0dd63588f8138df8e Mon Sep 17 00:00:00 2001 From: rustdesk Date: Fri, 4 Sep 2026 18:02:17 +0800 Subject: [PATCH] port forward: the login's target travels with the accept, not the handler `listen()` wrote `lc.port_forward` (and, on this branch, `port_forward_mux`) into the window's shared `LoginConfigHandler` before connecting, and `create_login_msg` read them back only when the peer's `Hash` arrived. Two mappings logging in at the same time could therefore swap targets: on master that bridged a local socket to the wrong target, and with a tunnel bound to its login's target it also left the mapping refusing every later accept until it was recreated. The target is now a `PortForward` carried by the interface clone that handles one accept, passed explicitly down to `create_login_msg`; the handler no longer has a field to race on. No lock spans the login. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01EZ49AbZJYfm8NTp5yDPMab --- src/client.rs | 55 +++++++++++++++++++++++++++---------- src/port_forward.rs | 28 ++++++++++++------- src/port_forward_mux.rs | 3 ++ src/ui_session_interface.rs | 12 +++++++- 4 files changed, 73 insertions(+), 25 deletions(-) diff --git a/src/client.rs b/src/client.rs index a464f53a6..b8ab0f1c2 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1752,10 +1752,6 @@ pub struct LoginConfigHandler { password: Vec, // remember password for reconnect pub remember: bool, config: PeerConfig, - pub port_forward: (String, i32), - /// Whether the next port-forward login asks for the multiplexed tunnel. - /// `port_forward::listen` sets it per accept. - pub port_forward_mux: bool, pub version: i64, features: Option, pub session_id: u64, // used for local <-> server communication @@ -2669,6 +2665,7 @@ impl LoginConfigHandler { os_username: String, os_password: String, password: Vec, + port_forward: PortForward, ) -> Message { let my_id = Config::get_id(); let (my_id, pure_id) = if let Some((id, _, _)) = self.other_server.as_ref() { @@ -2765,12 +2762,7 @@ impl LoginConfigHandler { ..Default::default() }), ConnType::VIEW_CAMERA => lr.set_view_camera(Default::default()), - ConnType::PORT_FORWARD | ConnType::RDP => lr.set_port_forward(PortForward { - host: self.port_forward.0.clone(), - port: self.port_forward.1, - multiplex: self.port_forward_mux, - ..Default::default() - }), + ConnType::PORT_FORWARD | ConnType::RDP => lr.set_port_forward(port_forward), ConnType::TERMINAL => { let mut terminal = Terminal::new(); terminal.service_id = self.get_option(self.get_key_terminal_service_id()); @@ -3504,6 +3496,7 @@ pub async fn handle_hash( lc: Arc>, password_preset: &str, hash: Hash, + port_forward: PortForward, interface: &impl Interface, peer: &mut Stream, ) -> bool { @@ -3623,7 +3616,7 @@ pub async fn handle_hash( hasher.finalize()[..].into() }; - send_login(lc.clone(), String::new(), String::new(), password, peer).await; + send_login(lc.clone(), String::new(), String::new(), password, port_forward, peer).await; lc.write().unwrap().hash = hash; true } @@ -3661,18 +3654,20 @@ fn try_get_password_from_personal_ab(lc: Arc>, passwo /// * `os_username` - OS username. /// * `os_password` - OS password. /// * `password` - Password. +/// * `port_forward` - Target of a port-forward login; ignored by other types. /// * `peer` - [`Stream`] for communicating with peer. async fn send_login( lc: Arc>, os_username: String, os_password: String, password: Vec, + port_forward: PortForward, peer: &mut Stream, ) { let msg_out = lc .read() .unwrap() - .create_login_msg(os_username, os_password, password); + .create_login_msg(os_username, os_password, password, port_forward); allow_err!(peer.send(&msg_out).await); } @@ -3685,6 +3680,7 @@ async fn send_login( /// * `os_password` - OS password. /// * `password` - Password. /// * `remember` - Whether to remember password. +/// * `port_forward` - Target of a port-forward login; ignored by other types. /// * `peer` - [`Stream`] for communicating with peer. pub async fn handle_login_from_ui( lc: Arc>, @@ -3692,6 +3688,7 @@ pub async fn handle_login_from_ui( os_password: String, password: String, remember: bool, + port_forward: PortForward, peer: &mut Stream, ) { let mut hash_password = if password.is_empty() { @@ -3718,7 +3715,7 @@ pub async fn handle_login_from_ui( hasher2.update(&lc.read().unwrap().hash.challenge); hash_password = hasher2.finalize()[..].to_vec(); - send_login(lc.clone(), os_username, os_password, hash_password, peer).await; + send_login(lc.clone(), os_username, os_password, hash_password, port_forward, peer).await; } async fn send_switch_login_request( @@ -3732,7 +3729,7 @@ async fn send_switch_login_request( lr: hbb_common::protobuf::MessageField::some( lc.read() .unwrap() - .create_login_msg("".to_owned(), "".to_owned(), vec![]) + .create_login_msg("".to_owned(), "".to_owned(), vec![], Default::default()) .login_request() .to_owned(), ), @@ -3765,6 +3762,11 @@ pub trait Interface: Send + Clone + 'static + Sized { async fn handle_test_delay(&self, t: TestDelay, peer: &mut Stream); fn get_lch(&self) -> Arc>; + /// A clone whose port-forward login asks for this target. The target + /// travels with the clone, never through the shared + /// `LoginConfigHandler`, so mappings logging in at the same time cannot + /// overwrite each other's target between the accept and the peer's `Hash`. + fn with_port_forward(&self, port_forward: PortForward) -> Self; fn get_id(&self) -> String { self.get_lch().read().unwrap().id.clone() @@ -4066,6 +4068,31 @@ mod retry_tests { } } +#[cfg(test)] +mod login_scope_tests { + use super::*; + + /// The target rides in the call, never in the shared handler: two accepts + /// building their logins off one handler cannot see each other's target. + #[test] + fn a_port_forward_login_carries_the_callers_target() { + let mut lc = LoginConfigHandler::default(); + lc.conn_type = ConnType::PORT_FORWARD; + let target = |host: &str, port: i32| PortForward { + host: host.to_owned(), + port, + multiplex: true, + ..Default::default() + }; + let a = lc.create_login_msg(String::new(), String::new(), vec![], target("a", 1)); + let b = lc.create_login_msg(String::new(), String::new(), vec![], target("b", 2)); + let pf = |m: &Message| m.login_request().port_forward().clone(); + assert_eq!((pf(&a).host.as_str(), pf(&a).port), ("a", 1)); + assert_eq!((pf(&b).host.as_str(), pf(&b).port), ("b", 2)); + assert!(pf(&a).multiplex); + } +} + pub async fn hc_connection( feedback: i32, rendezvous_server: String, diff --git a/src/port_forward.rs b/src/port_forward.rs index 9a02f62e5..0fba9fe29 100644 --- a/src/port_forward.rs +++ b/src/port_forward.rs @@ -108,11 +108,11 @@ pub async fn listen( // The claiming accept negotiates: it asks for the tunnel, and // the peer's answer fixes this listener's mode until it closes. Claim::Claimed => { - { - let mut lc = lc.write().unwrap(); - lc.port_forward = (remote_host.clone(), remote_port); - lc.port_forward_mux = crate::common::get_port_forward_mux_enabled(); - } + let interface = interface.with_port_forward(login_target( + &remote_host, + remote_port, + crate::common::get_port_forward_mux_enabled(), + )); let mut forward = Framed::new(forward, BytesCodec::new()); let mut close_port_forward = false; match connect_and_login(&id, &password, &mut ui_receiver, interface.clone(), &mut forward, key, token, is_rdp, &mut close_port_forward).await { @@ -150,11 +150,7 @@ pub async fn listen( // the window, is how a user picks up an upgraded peer; nothing // switches modes underneath live connections. Claim::Legacy => { - { - let mut lc = lc.write().unwrap(); - lc.port_forward = (remote_host.clone(), remote_port); - lc.port_forward_mux = false; - } + let interface = interface.with_port_forward(login_target(&remote_host, remote_port, false)); let mut forward = Framed::new(forward, BytesCodec::new()); let mut close_port_forward = false; match connect_and_login(&id, &password, &mut ui_receiver, interface.clone(), &mut forward, key, token, is_rdp, &mut close_port_forward).await { @@ -218,6 +214,18 @@ pub(crate) struct LoginOutcome { local_eof: bool, } +/// The target this accept's login asks for. It travels with the interface +/// clone rather than through the shared `LoginConfigHandler`, so mappings +/// logging in at the same time cannot overwrite each other's target. +fn login_target(host: &str, port: i32, multiplex: bool) -> PortForward { + PortForward { + host: host.to_owned(), + port, + multiplex, + ..Default::default() + } +} + fn peer_supports_mux(pi: &PeerInfo) -> bool { pi.features.as_ref().map(|f| f.port_forward_mux).unwrap_or(false) } diff --git a/src/port_forward_mux.rs b/src/port_forward_mux.rs index ded2508bc..6ac418605 100644 --- a/src/port_forward_mux.rs +++ b/src/port_forward_mux.rs @@ -1342,6 +1342,9 @@ mod tests { fn handle_login_error(&self, _err: &str) -> bool { false } + fn with_port_forward(&self, _port_forward: PortForward) -> Self { + self.clone() + } fn handle_peer_info(&self, _pi: PeerInfo) {} fn set_multiple_windows_session(&self, _sessions: Vec) {} async fn handle_hash(&self, _pass: &str, _hash: Hash, _peer: &mut Stream) -> bool { diff --git a/src/ui_session_interface.rs b/src/ui_session_interface.rs index 03b59a497..b170b0f05 100644 --- a/src/ui_session_interface.rs +++ b/src/ui_session_interface.rs @@ -57,6 +57,9 @@ const CHANGE_RESOLUTION_VALID_TIMEOUT_SECS: u64 = 15; pub struct Session { pub password: String, pub args: Vec, + /// Per clone, set by `Interface::with_port_forward`: the target a + /// port-forward login asks for. + pub port_forward: PortForward, pub lc: Arc>, pub sender: Arc>>>, pub thread: Arc>>>, @@ -1868,8 +1871,14 @@ impl Interface for Session { } } + fn with_port_forward(&self, port_forward: PortForward) -> Self { + let mut scoped = self.clone(); + scoped.port_forward = port_forward; + scoped + } + async fn handle_hash(&self, pass: &str, hash: Hash, peer: &mut Stream) -> bool { - handle_hash(self.lc.clone(), pass, hash, self, peer).await + handle_hash(self.lc.clone(), pass, hash, self.port_forward.clone(), self, peer).await } async fn handle_login_from_ui( @@ -1886,6 +1895,7 @@ impl Interface for Session { os_password, password, remember, + self.port_forward.clone(), peer, ) .await;