diff --git a/src/bin/dosh-server.rs b/src/bin/dosh-server.rs index 8da270e..6a031a1 100644 --- a/src/bin/dosh-server.rs +++ b/src/bin/dosh-server.rs @@ -433,6 +433,7 @@ struct PendingStreamOpen { attempts: u32, } +#[derive(Clone)] struct PendingNativeAuth { client: dosh::native::NativeClientHello, server: NativeServerHello, @@ -992,8 +993,8 @@ async fn handle_native_user_auth( packet: &protocol::Packet, ) -> Result<()> { let pending = { - let mut locked = state.lock().expect("server state poisoned"); - locked.pending_native.remove(&packet.header.conn_id) + let locked = state.lock().expect("server state poisoned"); + locked.pending_native.get(&packet.header.conn_id).cloned() }; let Some(pending) = pending else { return send_reject_to_client(socket, peer, packet.header.conn_id, "unknown native auth") @@ -1009,11 +1010,38 @@ async fn handle_native_user_auth( .await; } if pending.created_at.elapsed() > Duration::from_secs(30) { + state + .lock() + .expect("server state poisoned") + .pending_native + .remove(&packet.header.conn_id); return send_reject_to_client(socket, peer, packet.header.conn_id, "native auth expired") .await; } - let body = protocol::decrypt_body(packet, &pending.session_key, CLIENT_TO_SERVER)?; - let req: NativeUserAuthBody = protocol::from_body(&body)?; + let body = match protocol::decrypt_body(packet, &pending.session_key, CLIENT_TO_SERVER) { + Ok(body) => body, + Err(_) => { + return send_reject_to_client( + socket, + peer, + packet.header.conn_id, + "native auth decrypt failed", + ) + .await; + } + }; + let req: NativeUserAuthBody = match protocol::from_body(&body) { + Ok(req) => req, + Err(_) => { + return send_reject_to_client( + socket, + peer, + packet.header.conn_id, + "invalid native auth body", + ) + .await; + } + }; if pending.client.requested_mode == "doctor" { let check_result = { let locked = state.lock().expect("server state poisoned"); @@ -1049,6 +1077,11 @@ async fn handle_native_user_auth( .await; } }; + state + .lock() + .expect("server state poisoned") + .pending_native + .remove(&packet.header.conn_id); let body = protocol::to_body(&check)?; let out = protocol::encode_encrypted( PacketKind::NativeAuthCheckOk, @@ -1229,6 +1262,11 @@ async fn handle_native_user_auth( if let Some((listener, path)) = agent_listener { spawn_agent_forward(state, socket, client_id, listener, path); } + state + .lock() + .expect("server state poisoned") + .pending_native + .remove(&packet.header.conn_id); let ok = NativeAuthOk { client_id, diff --git a/src/server.rs b/src/server.rs index b295a96..9fd4816 100644 --- a/src/server.rs +++ b/src/server.rs @@ -13,6 +13,7 @@ use crate::transport::{ DoshTransport, SessionEvent, SessionRole, SessionTransportConfig, TransportConfig, service_name_from_target, }; +use crate::udp::is_transient_udp_send_error; use anyhow::{Context, Result, anyhow, bail}; use ed25519_dalek::SigningKey; use std::collections::{HashMap, HashSet}; @@ -97,6 +98,7 @@ pub enum DoshServerEvent { Ignored, } +#[derive(Debug, Clone)] struct PendingServerAuth { client: native::NativeClientHello, server: NativeServerHello, @@ -255,7 +257,7 @@ impl DoshServer { }; let body = protocol::to_body(&NativeServerHelloBody { hello })?; let out = protocol::encode_plain(PacketKind::NativeServerHello, pending_id, 1, 0, &body)?; - self.socket.send_to(&out, peer).await?; + let _ = send_udp(&self.socket, &out, peer).await?; Ok(()) } @@ -327,7 +329,7 @@ impl DoshServer { peer: SocketAddr, packet: &protocol::Packet, ) -> Result { - let Some(pending) = self.pending.remove(&packet.header.conn_id) else { + let Some(pending) = self.pending.get(&packet.header.conn_id).cloned() else { self.send_reject(peer, packet.header.conn_id, "unknown native auth") .await?; return Ok(DoshServerEvent::Ignored); @@ -338,13 +340,28 @@ impl DoshServer { return Ok(DoshServerEvent::Ignored); } if pending.created_at.elapsed() > self.config.auth_timeout { + self.pending.remove(&packet.header.conn_id); self.send_reject(peer, packet.header.conn_id, "native auth expired") .await?; return Ok(DoshServerEvent::Ignored); } - let body = protocol::decrypt_body(packet, &pending.session_key, CLIENT_TO_SERVER)?; - let req: NativeUserAuthBody = protocol::from_body(&body)?; + let body = match protocol::decrypt_body(packet, &pending.session_key, CLIENT_TO_SERVER) { + Ok(body) => body, + Err(_) => { + self.send_reject(peer, packet.header.conn_id, "native auth decrypt failed") + .await?; + return Ok(DoshServerEvent::Ignored); + } + }; + let req: NativeUserAuthBody = match protocol::from_body(&body) { + Ok(req) => req, + Err(_) => { + self.send_reject(peer, packet.header.conn_id, "invalid native auth body") + .await?; + return Ok(DoshServerEvent::Ignored); + } + }; let services = match self.verify_auth_and_services(&pending, &req.auth) { Ok(services) => services, Err(err) => { @@ -353,6 +370,7 @@ impl DoshServer { return Ok(DoshServerEvent::Ignored); } }; + self.pending.remove(&packet.header.conn_id); let conn_id = crypto::random_16(); let key_id = protocol::session_key_id(&pending.session_key); @@ -381,7 +399,7 @@ impl DoshServer { SERVER_TO_CLIENT, &body, )?; - self.socket.send_to(&out, peer).await?; + let _ = send_udp(&self.socket, &out, peer).await?; let transport = DoshTransport::new( Arc::clone(&self.socket), @@ -428,7 +446,7 @@ impl DoshServer { reason: reason.to_string(), })?; let out = protocol::encode_plain(PacketKind::AttachReject, conn_id, 0, 0, &body)?; - self.socket.send_to(&out, peer).await?; + let _ = send_udp(&self.socket, &out, peer).await?; Ok(()) } @@ -439,6 +457,14 @@ impl DoshServer { } } +async fn send_udp(socket: &UdpSocket, packet: &[u8], peer: SocketAddr) -> Result { + match socket.send_to(packet, peer).await { + Ok(_) => Ok(true), + Err(err) if is_transient_udp_send_error(&err) => Ok(false), + Err(err) => Err(err).with_context(|| format!("send UDP packet to {peer}")), + } +} + fn validate_requested_services( allowed_services: &HashSet, requests: &[ForwardingRequest], @@ -477,6 +503,7 @@ mod tests { use super::*; use crate::client::DoshClient; use crate::config::{ClientConfig, HostsConfig}; + use crate::protocol::{CLIENT_TO_SERVER, NativeUserAuthBody}; use crate::transport::TransportEvent; use ed25519_dalek::SigningKey; @@ -588,4 +615,91 @@ mod tests { } } } + + #[tokio::test] + async fn bad_native_auth_does_not_consume_pending_challenge() { + let dir = tempfile::tempdir().unwrap(); + let host_key = dir.path().join("host_key"); + let authorized_keys = dir.path().join("authorized_keys"); + let signing = SigningKey::from_bytes(&[93u8; 32]); + let keypair = ssh_key::private::Ed25519Keypair::from(&signing); + let private = + ssh_key::PrivateKey::new(ssh_key::private::KeypairData::from(keypair), "").unwrap(); + std::fs::write( + &authorized_keys, + format!("{}\n", private.public_key().to_openssh().unwrap()), + ) + .unwrap(); + + let server_config = ServerConfig { + host_key: host_key.to_string_lossy().to_string(), + authorized_keys: vec![authorized_keys.to_string_lossy().to_string()], + ..ServerConfig::default() + }; + let server_config = DoshServerConfig::new(server_config) + .bind_addr("127.0.0.1:0".parse().unwrap()) + .require_current_user(false); + let mut server = DoshServer::bind(server_config).await.unwrap(); + let peer: SocketAddr = "127.0.0.1:9".parse().unwrap(); + let (client_secret, client_public) = native::generate_native_ephemeral(); + let hello = native::NativeClientHello { + protocol_version: native::NATIVE_PROTOCOL_VERSION, + client_random: crypto::random_32(), + client_ephemeral_public: client_public, + requested_host: "127.0.0.1".to_string(), + requested_user: "sdk-user".to_string(), + requested_session: "test".to_string(), + requested_mode: "forward-only".to_string(), + terminal_size: (80, 24), + supported_aead: vec!["chacha20poly1305".to_string()], + supported_user_key_algorithms: vec!["ssh-ed25519".to_string()], + cached_host_key_fingerprint: None, + attach_ticket_envelope: None, + requested_env: Vec::new(), + }; + let (pending_id, server_hello) = server.build_server_hello(hello.clone(), peer).unwrap(); + let session_key = native::derive_native_session_key( + &client_secret, + server_hello.server_ephemeral_public, + &hello, + &server_hello, + ) + .unwrap(); + let auth = native::sign_user_auth(&signing, &hello, &server_hello, Vec::new()).unwrap(); + let body = protocol::to_body(&NativeUserAuthBody { auth }).unwrap(); + let bad = protocol::encode_encrypted( + PacketKind::NativeUserAuth, + pending_id, + 2, + 1, + &[7u8; 32], + CLIENT_TO_SERVER, + &body, + ) + .unwrap(); + let bad = protocol::decode(&bad).unwrap(); + + assert!(matches!( + server.handle_user_auth(peer, &bad).await.unwrap(), + DoshServerEvent::Ignored + )); + assert!(server.pending.contains_key(&pending_id)); + + let valid = protocol::encode_encrypted( + PacketKind::NativeUserAuth, + pending_id, + 2, + 1, + &session_key, + CLIENT_TO_SERVER, + &body, + ) + .unwrap(); + let valid = protocol::decode(&valid).unwrap(); + assert!(matches!( + server.handle_user_auth(peer, &valid).await.unwrap(), + DoshServerEvent::Accepted(_) + )); + assert!(!server.pending.contains_key(&pending_id)); + } } diff --git a/src/transport.rs b/src/transport.rs index 6da1c51..a522b1d 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -737,11 +737,17 @@ impl DoshTransport { if packet.header.conn_id != self.conn_id { continue; } + let plain = match protocol::decrypt_body( + &packet, + &self.session_key, + self.role.recv_direction(), + ) { + Ok(plain) => plain, + Err(_) => continue, + }; if !self.replay.accept(packet.header.seq) { continue; } - let plain = - protocol::decrypt_body(&packet, &self.session_key, self.role.recv_direction())?; self.peer_addr = peer; self.ack_seq = self.ack_seq.max(packet.header.seq); self.last_contact = Instant::now(); @@ -756,14 +762,21 @@ impl DoshTransport { datagram: &[u8], peer: SocketAddr, ) -> Result { - let packet = protocol::decode(datagram)?; + let packet = match protocol::decode(datagram) { + Ok(packet) => packet, + Err(_) => return Ok(SessionEvent::Ignored), + }; if packet.header.conn_id != self.conn_id { return Ok(SessionEvent::Ignored); } + let plain = + match protocol::decrypt_body(&packet, &self.session_key, self.role.recv_direction()) { + Ok(plain) => plain, + Err(_) => return Ok(SessionEvent::Ignored), + }; if !self.replay.accept(packet.header.seq) { return Ok(SessionEvent::Ignored); } - let plain = protocol::decrypt_body(&packet, &self.session_key, self.role.recv_direction())?; self.peer_addr = peer; self.ack_seq = self.ack_seq.max(packet.header.seq); self.last_contact = Instant::now(); @@ -1130,4 +1143,61 @@ mod tests { assert!(matches!(server.recv().await.unwrap(), SessionEvent::Ping)); assert_eq!(server.peer_addr(), roaming_addr); } + + #[tokio::test] + async fn unauthenticated_datagram_does_not_poison_replay_window() { + let socket = UdpSocket::bind("127.0.0.1:0").await.unwrap(); + let peer: SocketAddr = "127.0.0.1:9".parse().unwrap(); + let key = [11u8; 32]; + let wrong_key = [12u8; 32]; + let conn_id = [4u8; 16]; + let mut transport = DoshTransport::new_owned( + socket, + SessionTransportConfig { + role: SessionRole::Client, + conn_id, + session_key: key, + peer_addr: peer, + initial_send_seq: 1, + initial_ack: 0, + stream: TransportConfig::default(), + }, + ); + let bad = protocol::encode_encrypted( + PacketKind::Pong, + conn_id, + 9, + 0, + &wrong_key, + SERVER_TO_CLIENT, + b"", + ) + .unwrap(); + let valid = protocol::encode_encrypted( + PacketKind::Pong, + conn_id, + 9, + 0, + &key, + SERVER_TO_CLIENT, + b"", + ) + .unwrap(); + + assert!(matches!( + transport + .handle_datagram(b"not a dosh packet", peer) + .await + .unwrap(), + SessionEvent::Ignored + )); + assert!(matches!( + transport.handle_datagram(&bad, peer).await.unwrap(), + SessionEvent::Ignored + )); + assert!(matches!( + transport.handle_datagram(&valid, peer).await.unwrap(), + SessionEvent::Pong + )); + } }