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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EZ49AbZJYfm8NTp5yDPMab
This commit is contained in:
rustdesk
2026-09-04 18:02:17 +08:00
parent 8e2cb56a53
commit 15bad97d50
4 changed files with 73 additions and 25 deletions

View File

@@ -1752,10 +1752,6 @@ pub struct LoginConfigHandler {
password: Vec<u8>, // 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<Features>,
pub session_id: u64, // used for local <-> server communication
@@ -2669,6 +2665,7 @@ impl LoginConfigHandler {
os_username: String,
os_password: String,
password: Vec<u8>,
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<RwLock<LoginConfigHandler>>,
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<RwLock<LoginConfigHandler>>, 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<RwLock<LoginConfigHandler>>,
os_username: String,
os_password: String,
password: Vec<u8>,
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<RwLock<LoginConfigHandler>>,
@@ -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<RwLock<LoginConfigHandler>>;
/// 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,

View File

@@ -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)
}

View File

@@ -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<WindowsSession>) {}
async fn handle_hash(&self, _pass: &str, _hash: Hash, _peer: &mut Stream) -> bool {

View File

@@ -57,6 +57,9 @@ const CHANGE_RESOLUTION_VALID_TIMEOUT_SECS: u64 = 15;
pub struct Session<T: InvokeUiSession> {
pub password: String,
pub args: Vec<String>,
/// Per clone, set by `Interface::with_port_forward`: the target a
/// port-forward login asks for.
pub port_forward: PortForward,
pub lc: Arc<RwLock<LoginConfigHandler>>,
pub sender: Arc<RwLock<Option<mpsc::UnboundedSender<Data>>>>,
pub thread: Arc<Mutex<Option<std::thread::JoinHandle<()>>>>,
@@ -1868,8 +1871,14 @@ impl<T: InvokeUiSession> Interface for Session<T> {
}
}
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<T: InvokeUiSession> Interface for Session<T> {
os_password,
password,
remember,
self.port_forward.clone(),
peer,
)
.await;