From 6b12b3dfe0a3177673765bfde2dbaffcfc4c0d9d Mon Sep 17 00:00:00 2001 From: DuProcess <273172371+DuProcess@users.noreply.github.com> Date: Tue, 7 Jul 2026 11:48:59 -0400 Subject: [PATCH] Harden terminal reconnect packet handling --- src/bin/dosh-client.rs | 26 +++++++++++++---- src/bin/dosh-server.rs | 65 ++++++++++++++++++++++++++++++++++++++++++ src/protocol.rs | 2 +- tests/protocol_auth.rs | 13 +++++++++ 4 files changed, 100 insertions(+), 6 deletions(-) diff --git a/src/bin/dosh-client.rs b/src/bin/dosh-client.rs index e45aa85..4fbd6aa 100644 --- a/src/bin/dosh-client.rs +++ b/src/bin/dosh-client.rs @@ -5679,19 +5679,21 @@ fn stale_mouse_report_maybe_incomplete(bytes: &[u8], offset: usize) -> bool { [0x1b] | [0x1b, b'['] | [0x1b, b'[', b'<'] => true, [0x1b, b'[', b'M', ..] if rest.len() < 6 => true, [0x1b, b'[', b'<', params @ ..] | [0x1b, b'[', params @ ..] - if params_are_mouse_prefix(params) => + if params_are_mouse_prefix(params, 2) => { true } [0x9b] | [0x9b, b'<'] => true, [0x9b, b'M', ..] if rest.len() < 5 => true, - [0x9b, b'<', params @ ..] | [0x9b, params @ ..] if params_are_mouse_prefix(params) => true, - params if params_are_mouse_prefix(params) => true, + [0x9b, b'<', params @ ..] | [0x9b, params @ ..] if params_are_mouse_prefix(params, 2) => { + true + } + params if params_are_mouse_prefix(params, 2) => true, _ => false, } } -fn params_are_mouse_prefix(params: &[u8]) -> bool { +fn params_are_mouse_prefix(params: &[u8], min_semicolons: usize) -> bool { let mut semicolons = 0usize; let mut saw_digit = false; for byte in params { @@ -5701,7 +5703,7 @@ fn params_are_mouse_prefix(params: &[u8]) -> bool { _ => return false, } } - saw_digit && semicolons >= 1 + saw_digit && semicolons >= min_semicolons } async fn flush_startup_input_if_ready( @@ -8529,6 +8531,20 @@ mod tests { assert_eq!(strip_stale_mouse_reports(b"10m"), b""); } + #[test] + fn numeric_semicolon_input_survives_stale_filter() { + assert_eq!(strip_stale_mouse_reports(b"123;"), b"123;"); + assert_eq!(strip_stale_mouse_reports(b"12;34"), b"12;34"); + assert_eq!(strip_stale_mouse_reports(b"echo 1;"), b"echo 1;"); + } + + #[test] + fn incomplete_three_field_mouse_prefix_is_suppressed() { + assert_eq!(strip_stale_mouse_reports(b"35;152;"), b""); + assert_eq!(strip_stale_mouse_reports(b"\x1b[<35;152;"), b""); + assert_eq!(strip_stale_mouse_reports(b"\x9b35;152;"), b""); + } + #[test] fn stale_focus_reports_are_stripped_from_pending_input() { assert_eq!(strip_stale_mouse_reports(b"\x1b[Ihello\x1b[O"), b"hello"); diff --git a/src/bin/dosh-server.rs b/src/bin/dosh-server.rs index 6a031a1..86873ac 100644 --- a/src/bin/dosh-server.rs +++ b/src/bin/dosh-server.rs @@ -865,6 +865,7 @@ async fn handle_packet( | PacketKind::StreamEof | PacketKind::StreamWindowAdjust | PacketKind::RekeyAck + | PacketKind::Detach if find_client_decrypt_key(state, &packet.header).is_err() => { send_reject_to_client(socket, peer, packet.header.conn_id, "unknown client").await @@ -1734,7 +1735,14 @@ async fn handle_ping( } async fn handle_detach(state: &Arc>, packet: &protocol::Packet) -> Result<()> { + let (key, _) = find_client_decrypt_key(state, &packet.header)?; + let _ = protocol::decrypt_body(packet, &key, CLIENT_TO_SERVER)?; let mut locked = state.lock().expect("server state poisoned"); + if let Some(client) = locked.client_mut(&packet.header.conn_id) { + if !client.replay.accept(packet.header.seq) { + return Ok(()); + } + } locked.remove_client_everywhere(&packet.header.conn_id); Ok(()) } @@ -4105,6 +4113,63 @@ mod tests { assert!(locked.lookup_client(&client_id).is_none()); } + #[tokio::test] + async fn forged_plaintext_detach_does_not_remove_client() { + let (pty_tx, _pty_rx) = mpsc::unbounded_channel(); + let mut state = ServerState::new(ServerConfig::default(), [0u8; 32], pty_tx); + state + .ensure_session("work", 80, 24, "forward-only", &[]) + .unwrap(); + let client_id = [21u8; 16]; + let session_key = [22u8; 32]; + state.insert_client("work", client_id, test_client_state(session_key)); + let state = Arc::new(Mutex::new(state)); + let mut packet = protocol::decode( + &protocol::encode_plain(PacketKind::Detach, client_id, 1, 0, b"").unwrap(), + ) + .unwrap(); + packet.header.session_key_id = protocol::session_key_id(&session_key); + + assert!(handle_detach(&state, &packet).await.is_err()); + + let locked = state.lock().unwrap(); + assert!( + locked.lookup_client(&client_id).is_some(), + "plaintext detach with a copied key id must not remove a live client" + ); + } + + #[tokio::test] + async fn authenticated_detach_removes_client() { + let (pty_tx, _pty_rx) = mpsc::unbounded_channel(); + let mut state = ServerState::new(ServerConfig::default(), [0u8; 32], pty_tx); + state + .ensure_session("work", 80, 24, "forward-only", &[]) + .unwrap(); + let client_id = [23u8; 16]; + let session_key = [24u8; 32]; + state.insert_client("work", client_id, test_client_state(session_key)); + let state = Arc::new(Mutex::new(state)); + let packet = protocol::decode( + &protocol::encode_encrypted( + PacketKind::Detach, + client_id, + 1, + 0, + &session_key, + CLIENT_TO_SERVER, + b"", + ) + .unwrap(), + ) + .unwrap(); + + handle_detach(&state, &packet).await.unwrap(); + + let locked = state.lock().unwrap(); + assert!(locked.lookup_client(&client_id).is_none()); + } + #[tokio::test] async fn duplicate_stream_open_for_open_stream_resends_ok() { let (pty_tx, _pty_rx) = mpsc::unbounded_channel(); diff --git a/src/protocol.rs b/src/protocol.rs index 437cb79..816c331 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -280,7 +280,7 @@ pub fn decode(input: &[u8]) -> Result { pub fn decrypt_body(packet: &Packet, key: &[u8; 32], direction: u32) -> Result> { if packet.header.flags & 1 == 0 { - return Ok(packet.body.clone()); + bail!("packet body is not encrypted"); } let nonce = crypto::nonce_from(direction, packet.header.seq); let aad = packet.header.aad(); diff --git a/tests/protocol_auth.rs b/tests/protocol_auth.rs index 1166359..169bd8f 100644 --- a/tests/protocol_auth.rs +++ b/tests/protocol_auth.rs @@ -58,6 +58,19 @@ fn encrypted_packet_round_trips() { assert_eq!(plain, b"hello"); } +#[test] +fn plaintext_packet_never_decrypts_as_authenticated_body() { + let key = crypto::random_32(); + let mut decoded = protocol::decode( + &protocol::encode_plain(PacketKind::Input, crypto::random_16(), 1, 0, b"hello").unwrap(), + ) + .unwrap(); + decoded.header.session_key_id = protocol::session_key_id(&key); + + let err = protocol::decrypt_body(&decoded, &key, CLIENT_TO_SERVER).unwrap_err(); + assert!(err.to_string().contains("not encrypted")); +} + #[test] fn encrypted_packet_rejects_wrong_session_key_id_before_decrypt() { let key = crypto::random_32();