mirror of
https://github.com/rustdesk/rustdesk.git
synced 2026-11-05 16:38:17 +00:00
server: hold an unauthenticated connection to a small message (#16254)
* server: hold an unauthenticated connection to a small message Until a peer authorizes it sends only a public key, a login request, a test delay and a close reason, none of them large. Nothing said so: a frame header could declare up to whatever the transport allowed, 1 GiB on TCP and WebRTC, and a connection holds its place for up to LOGIN_GRACE before it has to authorize. With MAX_UNAUTHORIZED_CONNS places to fill, that is 64 GiB of header-declared payload one peer could make us hold - or, on WebSocket, 1 GiB bought outright with a few hundred bytes of frame headers, because tungstenite reserves a frame's declared payload as soon as it passes max_frame_size. The cap goes on in create_tcp_connection, before the identity handshake, so that read is bounded too, and comes off once the login is settled. It comes off before connect_port_forward_if_needed rather than beside the rest of authorization: a multiplexed tunnel narrows the same knob again for its own framing and has to have the last word. 128 KiB is several times the largest login request anyone sends - a long hostname, an os_login, an avatar URL, a file-transfer path - and is also the read buffer tungstenite allocates per WebSocket connection whatever we do, so on that transport the bound costs nothing beyond a floor already paid. A server hands that avatar out as a URL; only a custom client that inlines an image into the avatar option instead can reach the bound at all. Together with MAX_UNAUTHORIZED_CONNS it holds every unauthorized connection to 8 MiB. Redis answered this same shape in CVE-2021-32675 with 16 KiB, tighter because a per-message bound is the only one it has; here the connection count is the other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EZ49AbZJYfm8NTp5yDPMab * server: test that the unauthenticated cap is on before the handshake reads hbb_common covers each transport's cap; nothing covered where the connection layer puts it. A header one byte over MAX_UNAUTHORIZED_MESSAGE, written to a connection stalled in the identity handshake, has to end the handshake on the header alone and release the connection's place: the test fails with the cap moved past the handshake or dropped. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EZ49AbZJYfm8NTp5yDPMab * client: hold the clipboard until this round's login is accepted The clipboard listener is one per process, started by the first session to log in, and its broadcast went to every session - one whose login was still waiting on a password, 2FA or the peer's consent included. What the user copied meanwhile went to a machine that had not admitted them; the peer drops it unread before authorization, but it is in that peer's hands. The check goes on the round, not the session: a session-level one reads a state and later a sender that a reconnect can have swapped in between, so a broadcast that passed for the round before could still queue on the next. Remote is the round - its queue, and is_connected set once its own PeerInfo is in - so a Clipboard or MultiClipboards that reaches handle_msg_from_ui before then is dropped there, on the line before it would go out. What the login itself sends through the same queue, Auth2FA among it, goes as before. The unauthenticated cap on the other side is how this came up: a clipboard over 128 KiB ended such a login with "Reset by the peer". The small case had always gone through quietly. A test drives a round's Remote over a loopback pair before any login: a clipboard does not reach the far end, a 2FA code does, and once the round is connected the clipboard does too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EZ49AbZJYfm8NTp5yDPMab --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
97811acbdd
commit
dd9b21cf86
3 files changed
+131
No files matched your search
@@ -641,6 +641,19 @@ impl<T: InvokeUiSession> Remote<T> {
|
||||
self.check_clipboard_file_context();
|
||||
}
|
||||
Data::Message(msg) => {
|
||||
// The Flutter clipboard broadcast is process-wide, so a clipboard can reach this
|
||||
// round's queue before the round has logged in; it is dropped here, on the round
|
||||
// itself.
|
||||
#[cfg(feature = "flutter")]
|
||||
if !self.is_connected
|
||||
&& matches!(
|
||||
msg.union.as_ref(),
|
||||
Some(message::Union::Clipboard(_))
|
||||
| Some(message::Union::MultiClipboards(_))
|
||||
)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
match &msg.union {
|
||||
Some(message::Union::Misc(misc)) => match misc.union {
|
||||
Some(misc::Union::RefreshVideo(_)) => {
|
||||
@@ -2643,3 +2656,72 @@ impl Drop for VideoThread {
|
||||
*self.discard_queue.write().unwrap() = true;
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[cfg(feature = "flutter")]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::flutter::FlutterHandler;
|
||||
|
||||
/// A round's `Remote` over a loopback pair, before any login: what it sends to `peer`
|
||||
/// arrives at `far`.
|
||||
async fn remote_and_peer() -> (Remote<FlutterHandler>, Stream, Stream) {
|
||||
let listener = hbb_common::tcp::new_listener("127.0.0.1:0", false)
|
||||
.await
|
||||
.unwrap();
|
||||
let addr = listener.local_addr().unwrap();
|
||||
let (peer, accepted) = tokio::join!(
|
||||
hbb_common::socket_client::connect_tcp(addr.to_string(), 3000),
|
||||
listener.accept()
|
||||
);
|
||||
let (accepted, far_addr) = accepted.unwrap();
|
||||
let far = Stream::Tcp(hbb_common::tcp::FramedStream::from(accepted, far_addr));
|
||||
let (sender, receiver) = mpsc::unbounded_channel::<Data>();
|
||||
let remote = Remote::new(Session::<FlutterHandler>::default(), receiver, sender);
|
||||
(remote, peer.unwrap(), far)
|
||||
}
|
||||
|
||||
async fn arrives(far: &mut Stream) -> bool {
|
||||
matches!(hbb_common::timeout(300, far.next()).await, Ok(Some(Ok(_))))
|
||||
}
|
||||
|
||||
fn clipboard() -> Data {
|
||||
let mut msg = Message::new();
|
||||
msg.set_clipboard(Clipboard {
|
||||
content: b"copied while this login was pending".to_vec().into(),
|
||||
..Default::default()
|
||||
});
|
||||
Data::Message(msg)
|
||||
}
|
||||
|
||||
fn auth_2fa() -> Data {
|
||||
let mut msg = Message::new();
|
||||
msg.set_auth_2fa(Auth2FA {
|
||||
code: "123456".to_owned(),
|
||||
..Default::default()
|
||||
});
|
||||
Data::Message(msg)
|
||||
}
|
||||
|
||||
// A clipboard queued before this round's login stays here; what the login itself sends
|
||||
// through the same queue does not.
|
||||
#[tokio::test]
|
||||
async fn a_clipboard_queued_before_this_rounds_login_is_dropped() {
|
||||
let (mut remote, mut peer, mut far) = remote_and_peer().await;
|
||||
assert!(!remote.is_connected);
|
||||
assert!(remote.handle_msg_from_ui(clipboard(), &mut peer).await);
|
||||
assert!(
|
||||
!arrives(&mut far).await,
|
||||
"a clipboard went out before the login"
|
||||
);
|
||||
assert!(remote.handle_msg_from_ui(auth_2fa(), &mut peer).await);
|
||||
assert!(arrives(&mut far).await, "the 2FA code was held back");
|
||||
|
||||
remote.is_connected = true;
|
||||
assert!(remote.handle_msg_from_ui(clipboard(), &mut peer).await);
|
||||
assert!(
|
||||
arrives(&mut far).await,
|
||||
"a clipboard after the login was held back"
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user