Harden terminal reconnect packet handling

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<Mutex<ServerState>>, 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();