From 27c6446dfffb28370984b75756da2a860d2b19a9 Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Sun, 31 May 2026 20:02:52 -0500 Subject: [PATCH] fix(pdu)!: send NetworkAutoDetect over the MCS message channel NetworkAutoDetect was modeled as a slow-path Share Data PDU (ShareDataPduType::AutoDetect, a fabricated 0x3B) carried on the I/O channel. Per [MS-RDPBCGR] 2.2.14.3 / 2.2.14.4 the auto-detect PDUs are framed by a Basic Security Header (SEC_AUTODETECT_REQ / SEC_AUTODETECT_RSP) and ride the MCS message channel, so neither mstsc nor xfreerdp answered the requests. ironrdp-pdu: add AutoDetectReqPdu / AutoDetectRspPdu, which wrap the request and response data with the security header (mirroring the multitransport PDUs). Remove the ShareDataPdu::AutoDetectReq / AutoDetectRsp variants and the ShareDataPduType::AutoDetect discriminant, which did not correspond to any real PDUTYPE2. ironrdp-server: send auto-detect requests and receive responses on the message channel surfaced by the acceptor, with the new framing. ironrdp-connector and ironrdp-session: the client now requests the message channel (Client Message Channel Data), advertises SUPPORT_NET_CHAR_AUTODETECT, joins the channel, and answers RTT requests on it. The connector handles connect-time auto-detection (previously a no-op) and the session handles continuous auto-detection. Resolves the client side of the message-channel TODO (#140). Builds on the acceptor message-channel negotiation merged in #1347 (efa57328). --- crates/ironrdp-connector/src/connection.rs | 148 ++++++++++++++-- crates/ironrdp-pdu/src/rdp/autodetect.rs | 162 ++++++++++++++++++ crates/ironrdp-pdu/src/rdp/headers.rs | 28 --- crates/ironrdp-server/src/server.rs | 112 +++++++----- crates/ironrdp-session/src/active_stage.rs | 1 + crates/ironrdp-session/src/x224/mod.rs | 68 +++++--- .../tests/session/autodetect.rs | 75 ++++---- 7 files changed, 443 insertions(+), 151 deletions(-) diff --git a/crates/ironrdp-connector/src/connection.rs b/crates/ironrdp-connector/src/connection.rs index 55ab284bb..23e99f715 100644 --- a/crates/ironrdp-connector/src/connection.rs +++ b/crates/ironrdp-connector/src/connection.rs @@ -21,6 +21,8 @@ use crate::{ pub struct ConnectionResult { pub io_channel_id: u16, pub user_channel_id: u16, + /// MCS channel ID of the message channel, when one was negotiated. + pub message_channel_id: Option, pub share_id: u32, pub static_channels: StaticChannelSet, pub desktop_size: DesktopSize, @@ -126,6 +128,8 @@ pub struct ClientConnector { /// The client address to be used in the Client Info PDU. pub client_addr: SocketAddr, pub static_channels: StaticChannelSet, + /// MCS message channel ID assigned by the server, once negotiated. + pub message_channel_id: Option, } impl ClientConnector { @@ -135,6 +139,7 @@ impl ClientConnector { state: ClientConnectorState::ConnectionInitiationSendRequest, client_addr, static_channels: StaticChannelSet::new(), + message_channel_id: None, } } @@ -212,7 +217,15 @@ impl Sequence for ClientConnector { ClientConnectorState::BasicSettingsExchangeWaitResponse { .. } => Some(&ironrdp_pdu::X224_HINT), ClientConnectorState::ChannelConnection { channel_connection, .. } => channel_connection.next_pdu_hint(), ClientConnectorState::SecureSettingsExchange { .. } => None, - ClientConnectorState::ConnectTimeAutoDetection { .. } => None, + ClientConnectorState::ConnectTimeAutoDetection { .. } => { + // Wait for input only when a message channel was negotiated, so + // we can receive connect-time auto-detect requests there. + if self.message_channel_id.is_some() { + Some(&ironrdp_pdu::X224_HINT) + } else { + None + } + } ClientConnectorState::LicensingExchange { license_exchange, .. } => license_exchange.next_pdu_hint(), ClientConnectorState::MultitransportBootstrapping { .. } => None, ClientConnectorState::CapabilitiesExchange { @@ -382,9 +395,10 @@ impl Sequence for ClientConnector { return Err(general_err!("can't satisfy server security settings")); } - if server_gcc_blocks.message_channel.is_some() { - warn!("Unexpected ServerMessageChannelData GCC block (not supported)"); - } + self.message_channel_id = server_gcc_blocks + .message_channel + .as_ref() + .map(|data| data.mcs_message_channel_id); if server_gcc_blocks.multi_transport_channel.is_some() { warn!("Unexpected MultiTransportChannelData GCC block (not supported)"); @@ -418,7 +432,9 @@ impl Sequence for ClientConnector { channel_connection: if skip_channel_join { ChannelConnectionSequence::skip_channel_join() } else { - ChannelConnectionSequence::new(io_channel_id, static_channel_ids) + let mut join_channel_ids = static_channel_ids; + join_channel_ids.extend(self.message_channel_id); + ChannelConnectionSequence::new(io_channel_id, join_channel_ids) }, }, ) @@ -485,12 +501,57 @@ impl Sequence for ClientConnector { ClientConnectorState::ConnectTimeAutoDetection { io_channel_id, user_channel_id, - } => ( - Written::Nothing, - ClientConnectorState::LicensingExchange { - io_channel_id, - user_channel_id, - license_exchange: LicenseExchangeSequence::new( + } => { + // The server may run Optional Connect-Time Auto-Detection on the + // message channel before licensing ([MS-RDPBCGR] 1.3.8). When a + // message channel was negotiated we wait for a PDU here and demux + // by MCS channel: a PDU on the message channel is never a licensing + // PDU, so it must not be handed to the licensing sequence. An + // auto-detect request is answered and we keep listening; any other + // message-channel PDU is not ours to act on in this phase and is + // ignored. The first PDU that is not on the message channel (the + // licensing PDU on the I/O channel) ends the phase. Without a + // message channel nothing is read and we go straight to licensing, + // as before. + let autodetect = self.message_channel_id.and_then(|message_channel_id| { + let mcs = decode::>>(input).ok()?; + match mcs.0 { + mcs::McsMessage::SendDataIndication(data) if data.channel_id == message_channel_id => { + decode::(&data.user_data).ok() + } + _ => None, + } + }); + let on_message_channel = self.message_channel_id.is_some_and(|message_channel_id| { + decode::>>(input).is_ok_and(|mcs| { + matches!(mcs.0, mcs::McsMessage::SendDataIndication(data) if data.channel_id == message_channel_id) + }) + }); + + if let (Some(message_channel_id), Some(pdu)) = (self.message_channel_id, autodetect) { + let written = + respond_to_connect_time_autodetect(pdu.request, message_channel_id, user_channel_id, output)?; + ( + written, + ClientConnectorState::ConnectTimeAutoDetection { + io_channel_id, + user_channel_id, + }, + ) + } else if on_message_channel { + // A message-channel PDU we do not handle in this phase (per the + // canonical sequence multitransport bootstrap is Phase 8 and + // heartbeat is post-connection, both after licensing). Ignore it + // and keep listening rather than decoding it as a licensing PDU. + ( + Written::Nothing, + ClientConnectorState::ConnectTimeAutoDetection { + io_channel_id, + user_channel_id, + }, + ) + } else { + let mut license_exchange = LicenseExchangeSequence::new( io_channel_id, self.config.credentials.username().unwrap_or("").to_owned(), self.config.domain.clone(), @@ -499,9 +560,39 @@ impl Sequence for ClientConnector { .license_cache .clone() .unwrap_or_else(|| Arc::new(NoopLicenseCache)), - ), - }, - ), + ); + // If a PDU was read (message channel present) it is the first + // licensing PDU; process it here and advance the same way the + // LicensingExchange state would, so it is not stepped twice. + // Otherwise nothing was read and the licensing sequence runs + // from its first step as before. + if self.message_channel_id.is_some() { + let written = license_exchange.step(input, output)?; + let next_state = if license_exchange.state.is_terminal() { + ClientConnectorState::MultitransportBootstrapping { + io_channel_id, + user_channel_id, + } + } else { + ClientConnectorState::LicensingExchange { + io_channel_id, + user_channel_id, + license_exchange, + } + }; + (written, next_state) + } else { + ( + Written::Nothing, + ClientConnectorState::LicensingExchange { + io_channel_id, + user_channel_id, + license_exchange, + }, + ) + } + } + } //== Licensing ==// // Server is sending information regarding licensing. @@ -593,6 +684,7 @@ impl Sequence for ClientConnector { result: ConnectionResult { io_channel_id, user_channel_id, + message_channel_id: self.message_channel_id, share_id, static_channels: mem::take(&mut self.static_channels), desktop_size, @@ -639,6 +731,27 @@ pub fn encode_send_data_request( Ok(written) } +fn respond_to_connect_time_autodetect( + request: rdp::autodetect::AutoDetectRequest, + message_channel_id: u16, + user_channel_id: u16, + output: &mut WriteBuf, +) -> ConnectorResult { + use ironrdp_pdu::rdp::autodetect::{AutoDetectRequest, AutoDetectResponse, AutoDetectRspPdu}; + + match request { + AutoDetectRequest::RttRequest { sequence_number, .. } => { + let response = AutoDetectRspPdu::new(AutoDetectResponse::RttResponse { sequence_number }); + let written = encode_send_data_request(user_channel_id, message_channel_id, &response, output)?; + Written::from_size(written) + } + // The Network Characteristics Result is informational, and bandwidth + // measurement is driven implicitly by the connect-time payload exchange. + // Neither requires an immediate reply. + _ => Ok(Written::Nothing), + } +} + #[expect(single_use_lifetimes)] // anonymous lifetimes in `impl Trait` are unstable fn create_gcc_blocks<'a>( config: &Config, @@ -703,6 +816,7 @@ fn create_gcc_blocks<'a>( let mut early_capability_flags = ClientEarlyCapabilityFlags::VALID_CONNECTION_TYPE | ClientEarlyCapabilityFlags::SUPPORT_ERR_INFO_PDU | ClientEarlyCapabilityFlags::STRONG_ASYMMETRIC_KEYS + | ClientEarlyCapabilityFlags::SUPPORT_NET_CHAR_AUTODETECT | ClientEarlyCapabilityFlags::SUPPORT_SKIP_CHANNELJOIN; // TODO(#136): support for ClientEarlyCapabilityFlags::SUPPORT_STATUS_INFO_PDU @@ -743,8 +857,10 @@ fn create_gcc_blocks<'a>( // TODO(#139): support for Some(ClientClusterData { flags: RedirectionFlags::REDIRECTION_SUPPORTED, redirection_version: RedirectionVersion::V4, redirected_session_id: 0, }), cluster: None, monitor: None, - // TODO(#140): support for Client Message Channel Data (https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/f50e791c-de03-4b25-b17e-e914c9020bc3) - message_channel: None, + // Request the MCS message channel, which carries network auto-detect + // ([MS-RDPBCGR] 2.2.14) and the multitransport / heartbeat PDUs. The + // server assigns its ID in Server Message Channel Data. + message_channel: Some(gcc::ClientMessageChannelData), multi_transport_channel: config .multitransport_flags .map(|flags| gcc::MultiTransportChannelData { flags }), diff --git a/crates/ironrdp-pdu/src/rdp/autodetect.rs b/crates/ironrdp-pdu/src/rdp/autodetect.rs index fc49c54c6..630ab00ed 100644 --- a/crates/ironrdp-pdu/src/rdp/autodetect.rs +++ b/crates/ironrdp-pdu/src/rdp/autodetect.rs @@ -15,6 +15,8 @@ use ironrdp_core::{ Decode, DecodeResult, Encode, EncodeResult, ReadCursor, WriteCursor, ensure_size, invalid_field_err, }; +use crate::rdp::headers::{BasicSecurityHeader, BasicSecurityHeaderFlags}; + // ============================================================================ // Constants // ============================================================================ @@ -676,10 +678,170 @@ impl<'de> Decode<'de> for AutoDetectResponse { } } +// ============================================================================ +// MCS message channel framing +// ============================================================================ +// +// Auto-detect is not a Share Data PDU. Per [MS-RDPBCGR] 2.2.14.3 / 2.2.14.4 it +// rides the MCS message channel framed by a Basic Security Header whose +// SEC_AUTODETECT_REQ / SEC_AUTODETECT_RSP flag identifies it, the same dispatch +// mechanism used by multitransport (see `rdp::multitransport`). + +/// Server Auto-Detect Request PDU ([MS-RDPBCGR] 2.2.14.3). +/// +/// Wraps an [`AutoDetectRequest`] with the `SEC_AUTODETECT_REQ` security header. +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] +pub struct AutoDetectReqPdu { + pub security_header: BasicSecurityHeader, + pub request: AutoDetectRequest, +} + +impl AutoDetectReqPdu { + const NAME: &'static str = "AutoDetectReqPdu"; + + /// Wrap a request with the `SEC_AUTODETECT_REQ` security header. + pub fn new(request: AutoDetectRequest) -> Self { + Self { + security_header: BasicSecurityHeader { + flags: BasicSecurityHeaderFlags::AUTODETECT_REQ, + }, + request, + } + } +} + +impl Encode for AutoDetectReqPdu { + fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> { + ensure_size!(in: dst, size: self.size()); + + self.security_header.encode(dst)?; + self.request.encode(dst)?; + + Ok(()) + } + + fn name(&self) -> &'static str { + Self::NAME + } + + fn size(&self) -> usize { + BasicSecurityHeader::FIXED_PART_SIZE + self.request.size() + } +} + +impl<'de> Decode<'de> for AutoDetectReqPdu { + fn decode(src: &mut ReadCursor<'de>) -> DecodeResult { + let security_header = BasicSecurityHeader::decode(src)?; + + if !security_header.flags.contains(BasicSecurityHeaderFlags::AUTODETECT_REQ) { + return Err(invalid_field_err!("securityHeader", "expected SEC_AUTODETECT_REQ flag")); + } + + let request = AutoDetectRequest::decode(src)?; + + Ok(Self { + security_header, + request, + }) + } +} + +/// Client Auto-Detect Response PDU ([MS-RDPBCGR] 2.2.14.4). +/// +/// Wraps an [`AutoDetectResponse`] with the `SEC_AUTODETECT_RSP` security header. +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] +pub struct AutoDetectRspPdu { + pub security_header: BasicSecurityHeader, + pub response: AutoDetectResponse, +} + +impl AutoDetectRspPdu { + const NAME: &'static str = "AutoDetectRspPdu"; + + /// Wrap a response with the `SEC_AUTODETECT_RSP` security header. + pub fn new(response: AutoDetectResponse) -> Self { + Self { + security_header: BasicSecurityHeader { + flags: BasicSecurityHeaderFlags::AUTODETECT_RSP, + }, + response, + } + } +} + +impl Encode for AutoDetectRspPdu { + fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> { + ensure_size!(in: dst, size: self.size()); + + self.security_header.encode(dst)?; + self.response.encode(dst)?; + + Ok(()) + } + + fn name(&self) -> &'static str { + Self::NAME + } + + fn size(&self) -> usize { + BasicSecurityHeader::FIXED_PART_SIZE + self.response.size() + } +} + +impl<'de> Decode<'de> for AutoDetectRspPdu { + fn decode(src: &mut ReadCursor<'de>) -> DecodeResult { + let security_header = BasicSecurityHeader::decode(src)?; + + if !security_header.flags.contains(BasicSecurityHeaderFlags::AUTODETECT_RSP) { + return Err(invalid_field_err!("securityHeader", "expected SEC_AUTODETECT_RSP flag")); + } + + let response = AutoDetectResponse::decode(src)?; + + Ok(Self { + security_header, + response, + }) + } +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn req_pdu_round_trip() { + let original = AutoDetectReqPdu::new(AutoDetectRequest::RttRequest { + sequence_number: 7, + request_type: RTT_REQUEST_CONTINUOUS, + }); + assert_eq!(original.security_header.flags, BasicSecurityHeaderFlags::AUTODETECT_REQ); + + let encoded = ironrdp_core::encode_vec(&original).unwrap(); + let decoded = ironrdp_core::decode::(&encoded).unwrap(); + assert_eq!(decoded, original); + } + + #[test] + fn rsp_pdu_round_trip() { + let original = AutoDetectRspPdu::new(AutoDetectResponse::RttResponse { sequence_number: 7 }); + assert_eq!(original.security_header.flags, BasicSecurityHeaderFlags::AUTODETECT_RSP); + + let encoded = ironrdp_core::encode_vec(&original).unwrap(); + let decoded = ironrdp_core::decode::(&encoded).unwrap(); + assert_eq!(decoded, original); + } + + #[test] + fn req_pdu_rejects_response_flag() { + // A response-flagged frame must not decode as a request PDU. + let rsp = AutoDetectRspPdu::new(AutoDetectResponse::RttResponse { sequence_number: 1 }); + let encoded = ironrdp_core::encode_vec(&rsp).unwrap(); + assert!(ironrdp_core::decode::(&encoded).is_err()); + } + // ======================================================================== // Request encoding/decoding tests // ======================================================================== diff --git a/crates/ironrdp-pdu/src/rdp/headers.rs b/crates/ironrdp-pdu/src/rdp/headers.rs index 2e4557d05..0373e1f54 100644 --- a/crates/ironrdp-pdu/src/rdp/headers.rs +++ b/crates/ironrdp-pdu/src/rdp/headers.rs @@ -8,7 +8,6 @@ use num_traits::FromPrimitive as _; use crate::codecs::rfx::FrameAcknowledgePdu; use crate::input::InputEventPdu; -use crate::rdp::autodetect::{AutoDetectRequest, AutoDetectResponse}; use crate::rdp::capability_sets::{ClientConfirmActive, ServerDemandActive}; use crate::rdp::client_info; use crate::rdp::finalization_messages::{ControlPdu, FontPdu, MonitorLayoutPdu, SynchronizePdu}; @@ -336,10 +335,6 @@ pub enum ShareDataPdu { DrawGdiPusErrorPdu(Vec), ArcStatusPdu(Vec), StatusInfoPdu(Vec), - /// Auto-Detect Request (server to client) - AutoDetectReq(AutoDetectRequest), - /// Auto-Detect Response (client to server) - AutoDetectRsp(AutoDetectResponse), } impl ShareDataPdu { @@ -372,8 +367,6 @@ impl ShareDataPdu { ShareDataPdu::DrawGdiPusErrorPdu(_) => "Draw GDI PUS Error PDU", ShareDataPdu::ArcStatusPdu(_) => "Arc Status PDU", ShareDataPdu::StatusInfoPdu(_) => "Status Info PDU", - ShareDataPdu::AutoDetectReq(_) => "Auto-Detect Request PDU", - ShareDataPdu::AutoDetectRsp(_) => "Auto-Detect Response PDU", } } @@ -404,7 +397,6 @@ impl ShareDataPdu { ShareDataPdu::DrawGdiPusErrorPdu(_) => ShareDataPduType::DrawGdiPusErrorPdu, ShareDataPdu::ArcStatusPdu(_) => ShareDataPduType::ArcStatusPdu, ShareDataPdu::StatusInfoPdu(_) => ShareDataPduType::StatusInfoPdu, - ShareDataPdu::AutoDetectReq(_) | ShareDataPdu::AutoDetectRsp(_) => ShareDataPduType::AutoDetect, } } @@ -445,15 +437,6 @@ impl ShareDataPdu { ShareDataPduType::DrawGdiPusErrorPdu => Ok(ShareDataPdu::DrawGdiPusErrorPdu(src.remaining().to_vec())), ShareDataPduType::ArcStatusPdu => Ok(ShareDataPdu::ArcStatusPdu(src.remaining().to_vec())), ShareDataPduType::StatusInfoPdu => Ok(ShareDataPdu::StatusInfoPdu(src.remaining().to_vec())), - ShareDataPduType::AutoDetect => { - ensure_size!(in: src, size: 2); - let type_id = src.remaining()[1]; - if type_id == crate::rdp::autodetect::TYPE_ID_AUTODETECT_REQUEST { - Ok(ShareDataPdu::AutoDetectReq(AutoDetectRequest::decode(src)?)) - } else { - Ok(ShareDataPdu::AutoDetectRsp(AutoDetectResponse::decode(src)?)) - } - } } } } @@ -472,8 +455,6 @@ impl Encode for ShareDataPdu { ShareDataPdu::ShutdownRequest | ShareDataPdu::ShutdownDenied => Ok(()), ShareDataPdu::SuppressOutput(pdu) => pdu.encode(dst), ShareDataPdu::RefreshRectangle(pdu) => pdu.encode(dst), - ShareDataPdu::AutoDetectReq(pdu) => pdu.encode(dst), - ShareDataPdu::AutoDetectRsp(pdu) => pdu.encode(dst), _ => Err(other_err!("Encoding not implemented")), } } @@ -507,8 +488,6 @@ impl Encode for ShareDataPdu { | ShareDataPdu::DrawGdiPusErrorPdu(buffer) | ShareDataPdu::ArcStatusPdu(buffer) | ShareDataPdu::StatusInfoPdu(buffer) => buffer.len(), - ShareDataPdu::AutoDetectReq(pdu) => pdu.size(), - ShareDataPdu::AutoDetectRsp(pdu) => pdu.size(), } } } @@ -606,13 +585,6 @@ pub enum ShareDataPduType { StatusInfoPdu = 0x36, MonitorLayoutPdu = 0x37, FrameAcknowledgePdu = 0x38, - /// Auto-Detect Request or Response ([MS-RDPBCGR 2.2.14]). - /// - /// The headerTypeId field within the PDU body discriminates direction: - /// 0x00 for server-to-client requests, 0x01 for client-to-server responses. - /// - /// [MS-RDPBCGR 2.2.14]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/dc672839-4f4e-40b1-a71c-cd6a959baa38 - AutoDetect = 0x3b, } impl ShareDataPduType { diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 2e57ab105..6c25bc74e 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -881,6 +881,7 @@ impl RdpServer { writer: &mut impl FramedWrite, io_channel_id: u16, user_channel_id: u16, + message_channel_id: Option, ) -> Result { match action { Action::FastPath => { @@ -890,7 +891,7 @@ impl RdpServer { Action::X224 => { if self - .handle_x224(writer, io_channel_id, user_channel_id, &bytes) + .handle_x224(writer, io_channel_id, user_channel_id, message_channel_id, &bytes) .await .context("X224 input error")? { @@ -944,8 +945,8 @@ impl RdpServer { &mut self, events: &mut Vec, writer: &mut impl FramedWrite, - io_channel_id: u16, user_channel_id: u16, + message_channel_id: Option, ) -> Result { // Avoid wave messages queuing up and causing extra delay. When a // batch carries more than `WAVE_KEEP` waves, drop the OLDEST ones @@ -1069,14 +1070,13 @@ impl RdpServer { } }, ServerEvent::AutoDetectRttRequest => { - if let Some(ref mut ad) = self.autodetect { + // Auto-detect requests ride the MCS message channel + // ([MS-RDPBCGR] 2.2.14.3). With none negotiated (the client + // did not request it), there is nowhere to send them. + if let (Some(ad), Some(message_channel_id)) = (self.autodetect.as_mut(), message_channel_id) { ad.expire_stale_probes(crate::autodetect::RTT_PROBE_MAX_AGE); let request = ad.send_rtt_request(); - let data = encode_share_data_pdu( - rdp::headers::ShareDataPdu::AutoDetectReq(request), - io_channel_id, - user_channel_id, - )?; + let data = encode_autodetect_request(request, message_channel_id, user_channel_id)?; writer.write_all(&data).await?; } } @@ -1092,6 +1092,7 @@ impl RdpServer { writer: &mut Framed, io_channel_id: u16, user_channel_id: u16, + message_channel_id: Option, mut encoder: UpdateEncoder, ) -> Result where @@ -1112,7 +1113,14 @@ impl RdpServer { let (action, bytes) = reader.read_pdu().await?; let mut this = this.lock().await; match this - .dispatch_pdu(action, bytes, &mut writer, io_channel_id, user_channel_id) + .dispatch_pdu( + action, + bytes, + &mut writer, + io_channel_id, + user_channel_id, + message_channel_id, + ) .await? { RunState::Continue => continue, @@ -1171,7 +1179,7 @@ impl RdpServer { } let mut this = this.lock().await; match this - .dispatch_server_events(&mut events, &mut event_writer, io_channel_id, user_channel_id) + .dispatch_server_events(&mut events, &mut event_writer, user_channel_id, message_channel_id) .await? { RunState::Continue => continue, @@ -1234,6 +1242,7 @@ impl RdpServer { writer, result.io_channel_id, result.user_channel_id, + result.message_channel_id, result.input_events, ) .await?; @@ -1342,7 +1351,14 @@ impl RdpServer { .context("failed to initialize update encoder")?; let state = self - .client_loop(reader, writer, result.io_channel_id, result.user_channel_id, encoder) + .client_loop( + reader, + writer, + result.io_channel_id, + result.user_channel_id, + result.message_channel_id, + encoder, + ) .await .context("client loop failure")?; @@ -1354,6 +1370,7 @@ impl RdpServer { writer: &mut impl FramedWrite, io_channel_id: u16, user_channel_id: u16, + message_channel_id: Option, frames: Vec>, ) -> Result<()> { for frame in frames { @@ -1364,7 +1381,9 @@ impl RdpServer { } Ok(Action::X224) => { - let _ = self.handle_x224(writer, io_channel_id, user_channel_id, &frame).await; + let _ = self + .handle_x224(writer, io_channel_id, user_channel_id, message_channel_id, &frame) + .await; } // the frame here is always valid, because otherwise it would @@ -1424,17 +1443,6 @@ impl RdpServer { return Ok(true); } - rdp::headers::ShareDataPdu::AutoDetectRsp(response) => { - if let Some(ref mut ad) = self.autodetect { - if let Some(rtt_ms) = ad.handle_response(&response) { - self.autodetect_rtt.store(rtt_ms, Ordering::Relaxed); - debug!(rtt_ms, seq = response.sequence_number(), "RTT measured"); - } else { - trace!(seq = response.sequence_number(), "Unmatched auto-detect response"); - } - } - } - // Client requests the server stop or resume sending display // updates. mstsc sends `desktop_rect: None` on minimize and // `desktop_rect: Some(rect)` on refocus. Without honoring @@ -1477,11 +1485,33 @@ impl RdpServer { Ok(false) } + fn handle_message_channel_data(&mut self, data: SendDataRequest<'_>) { + // The MCS message channel currently carries only the auto-detect + // response. It is framed by a Basic Security Header (SEC_AUTODETECT_RSP), + // not a Share Control header. + match decode::(data.user_data.as_ref()) { + Ok(pdu) => { + if let Some(ref mut ad) = self.autodetect { + if let Some(rtt_ms) = ad.handle_response(&pdu.response) { + self.autodetect_rtt.store(rtt_ms, Ordering::Relaxed); + debug!(rtt_ms, seq = pdu.response.sequence_number(), "RTT measured"); + } else { + trace!(seq = pdu.response.sequence_number(), "Unmatched auto-detect response"); + } + } + } + Err(error) => { + warn!(error = format!("{error:#}"), "Unhandled MCS message channel PDU"); + } + } + } + async fn handle_x224( &mut self, writer: &mut impl FramedWrite, io_channel_id: u16, user_channel_id: u16, + message_channel_id: Option, frame: &[u8], ) -> Result { let message = decode::>>(frame)?; @@ -1497,6 +1527,11 @@ impl RdpServer { return self.handle_io_channel_data(data).await; } + if message_channel_id == Some(data.channel_id) { + self.handle_message_channel_data(data); + return Ok(false); + } + if let Some(svc) = self.static_channels.get_by_channel_id_mut(data.channel_id) { let response_pdus = svc.process(&data.user_data)?; let response = server_encode_svc_messages(response_pdus, data.channel_id, user_channel_id)?; @@ -1595,32 +1630,23 @@ impl RdpServer { } } -/// Encode a server-initiated Share Data PDU for the IO channel. +/// Encode a server-initiated Auto-Detect Request PDU for the MCS message channel. /// -/// `share_id` is hard-coded to 0, matching the existing convention in -/// `deactivate_all()`. In practice, RDP clients do not validate `share_id` -/// on server-initiated PDUs, but a future refactor could thread the -/// negotiated value from the Demand Active exchange if needed. -fn encode_share_data_pdu( - share_data_pdu: rdp::headers::ShareDataPdu, - io_channel_id: u16, +/// The request is framed by a Basic Security Header (SEC_AUTODETECT_REQ) per +/// [MS-RDPBCGR] 2.2.14.3 and carried in an MCS Send Data Indication on the +/// negotiated message channel, not as a Share Data PDU on the I/O channel. +fn encode_autodetect_request( + request: rdp::autodetect::AutoDetectRequest, + message_channel_id: u16, user_channel_id: u16, ) -> Result> { - let header = rdp::headers::ShareDataHeader { - share_data_pdu, - stream_priority: rdp::headers::StreamPriority::Medium, - compression_flags: rdp::headers::CompressionFlags::empty(), - compression_type: rdp::client_info::CompressionType::K8, - }; - let pdu = rdp::headers::ShareControlHeader { - share_id: 0, - pdu_source: user_channel_id, - share_control_pdu: ShareControlPdu::Data(header), - }; + // Auto-detect rides the MCS message channel framed by a Basic Security + // Header (SEC_AUTODETECT_REQ), not a Share Control / Share Data header. + let pdu = rdp::autodetect::AutoDetectReqPdu::new(request); let user_data = encode_vec(&pdu)?.into(); let mcs_pdu = SendDataIndication { initiator_id: user_channel_id, - channel_id: io_channel_id, + channel_id: message_channel_id, user_data, }; Ok(encode_vec(&X224(mcs_pdu))?) diff --git a/crates/ironrdp-session/src/active_stage.rs b/crates/ironrdp-session/src/active_stage.rs index b7bb82446..6f2a01437 100644 --- a/crates/ironrdp-session/src/active_stage.rs +++ b/crates/ironrdp-session/src/active_stage.rs @@ -44,6 +44,7 @@ impl ActiveStage { connection_result.static_channels, connection_result.user_channel_id, connection_result.io_channel_id, + connection_result.message_channel_id, connection_result.share_id, connection_result.connection_activation, ); diff --git a/crates/ironrdp-session/src/x224/mod.rs b/crates/ironrdp-session/src/x224/mod.rs index 529aa397d..c9df1527f 100644 --- a/crates/ironrdp-session/src/x224/mod.rs +++ b/crates/ironrdp-session/src/x224/mod.rs @@ -1,9 +1,9 @@ use ironrdp_connector::connection_activation::ConnectionActivationSequence; use ironrdp_connector::legacy::SendDataIndicationCtx; -use ironrdp_core::WriteBuf; +use ironrdp_core::{WriteBuf, decode}; use ironrdp_dvc::{DrdynvcClient, DvcProcessor, DynamicVirtualChannel}; use ironrdp_pdu::mcs::{DisconnectProviderUltimatum, DisconnectReason, McsMessage}; -use ironrdp_pdu::rdp::autodetect::{AutoDetectRequest, AutoDetectResponse}; +use ironrdp_pdu::rdp::autodetect::{AutoDetectReqPdu, AutoDetectRequest, AutoDetectResponse, AutoDetectRspPdu}; use ironrdp_pdu::rdp::headers::ShareDataPdu; use ironrdp_pdu::rdp::multitransport::MultitransportRequestPdu; use ironrdp_pdu::rdp::server_error_info::{ErrorInfo, ProtocolIndependentCode, ServerSetErrorInfoPdu}; @@ -65,6 +65,7 @@ pub struct Processor { static_channels: StaticChannelSet, user_channel_id: u16, io_channel_id: u16, + message_channel_id: Option, share_id: u32, connection_activation: ConnectionActivationSequence, } @@ -74,6 +75,7 @@ impl Processor { static_channels: StaticChannelSet, user_channel_id: u16, io_channel_id: u16, + message_channel_id: Option, share_id: u32, connection_activation: ConnectionActivationSequence, ) -> Self { @@ -81,6 +83,7 @@ impl Processor { static_channels, user_channel_id, io_channel_id, + message_channel_id, share_id, connection_activation, } @@ -134,6 +137,8 @@ impl Processor { if channel_id == self.io_channel_id { self.process_io_channel(data_ctx) + } else if self.message_channel_id == Some(channel_id) { + self.process_message_channel(data_ctx) } else if let Some(svc) = self.static_channels.get_by_channel_id_mut(channel_id) { let response_pdus = svc.process(data_ctx.user_data).map_err(SessionError::pdu)?; process_svc_messages(response_pdus, channel_id, data_ctx.initiator_id) @@ -195,28 +200,6 @@ impl Processor { )), ]) } - ShareDataPdu::AutoDetectReq(AutoDetectRequest::RttRequest { sequence_number, .. }) => { - let response = AutoDetectResponse::RttResponse { sequence_number }; - let mut frame = WriteBuf::new(); - ironrdp_connector::legacy::encode_share_data( - self.user_channel_id, - self.io_channel_id, - self.share_id, - ShareDataPdu::AutoDetectRsp(response), - &mut frame, - ) - .map_err(crate::legacy::map_error)?; - debug!(sequence_number, "Responded to auto-detect RTT request"); - Ok(vec![ProcessorOutput::ResponseFrame(frame.into_inner())]) - } - ShareDataPdu::AutoDetectReq(req @ AutoDetectRequest::NetworkCharacteristicsResult { .. }) => { - debug!(?req, "Received network characteristics from server"); - Ok(vec![ProcessorOutput::AutoDetect(req)]) - } - ShareDataPdu::AutoDetectReq(_) => { - debug!(pdu = %ctx.pdu.as_short_name(), "Auto-detect request not yet implemented"); - Ok(Vec::new()) - } // TODO: slow-path payloads may be bulk-compressed when // ClientInfoFlags::COMPRESSION is negotiated. Decompression // should happen here before passing data downstream. Currently @@ -249,6 +232,43 @@ impl Processor { } } + /// Process an auto-detect request received on the MCS message channel. + /// + /// During continuous auto-detection ([MS-RDPBCGR] 2.2.14) the server sends + /// RTT (and bandwidth) requests on the message channel; the client answers + /// RTT requests and surfaces the final Network Characteristics Result. + fn process_message_channel(&self, data_ctx: SendDataIndicationCtx<'_>) -> SessionResult> { + let Some(message_channel_id) = self.message_channel_id else { + return Err(reason_err!("message channel", "no message channel negotiated")); + }; + + let req = decode::(data_ctx.user_data).map_err(SessionError::decode)?; + + match req.request { + AutoDetectRequest::RttRequest { sequence_number, .. } => { + let response = AutoDetectRspPdu::new(AutoDetectResponse::RttResponse { sequence_number }); + let mut frame = WriteBuf::new(); + ironrdp_connector::legacy::encode_send_data_request( + self.user_channel_id, + message_channel_id, + &response, + &mut frame, + ) + .map_err(crate::legacy::map_error)?; + debug!(sequence_number, "Responded to auto-detect RTT request"); + Ok(vec![ProcessorOutput::ResponseFrame(frame.into_inner())]) + } + req @ AutoDetectRequest::NetworkCharacteristicsResult { .. } => { + debug!(?req, "Received network characteristics from server"); + Ok(vec![ProcessorOutput::AutoDetect(req)]) + } + req => { + debug!(?req, "Auto-detect request not yet implemented"); + Ok(Vec::new()) + } + } + } + /// Send a pdu on the static global channel. Typically used to send input events pub fn encode_static(&self, output: &mut WriteBuf, pdu: ShareDataPdu) -> SessionResult { let written = ironrdp_connector::legacy::encode_share_data( diff --git a/crates/ironrdp-testsuite-core/tests/session/autodetect.rs b/crates/ironrdp-testsuite-core/tests/session/autodetect.rs index 9e931deae..290053c3b 100644 --- a/crates/ironrdp-testsuite-core/tests/session/autodetect.rs +++ b/crates/ironrdp-testsuite-core/tests/session/autodetect.rs @@ -5,18 +5,15 @@ use ironrdp_connector::{Credentials, DesktopSize}; use ironrdp_core::encode_vec; use ironrdp_pdu::gcc; use ironrdp_pdu::mcs::{McsMessage, SendDataIndication}; -use ironrdp_pdu::rdp::autodetect::{AutoDetectRequest, AutoDetectResponse}; +use ironrdp_pdu::rdp::autodetect::{AutoDetectReqPdu, AutoDetectRequest, AutoDetectResponse, AutoDetectRspPdu}; use ironrdp_pdu::rdp::capability_sets::MajorPlatformType; -use ironrdp_pdu::rdp::client_info::CompressionType; -use ironrdp_pdu::rdp::headers::{ - CompressionFlags, ShareControlHeader, ShareControlPdu, ShareDataHeader, ShareDataPdu, StreamPriority, -}; use ironrdp_pdu::x224::X224; use ironrdp_session::x224::Processor; use ironrdp_svc::StaticChannelSet; const USER_CHANNEL_ID: u16 = 1002; const IO_CHANNEL_ID: u16 = 1003; +const MESSAGE_CHANNEL_ID: u16 = 1004; const SHARE_ID: u32 = 0x0001_0000; fn test_config() -> ironrdp_connector::Config { @@ -63,29 +60,26 @@ fn test_config() -> ironrdp_connector::Config { fn make_processor() -> Processor { let config = test_config(); let cas = ConnectionActivationSequence::new(config, IO_CHANNEL_ID, USER_CHANNEL_ID); - Processor::new(StaticChannelSet::new(), USER_CHANNEL_ID, IO_CHANNEL_ID, SHARE_ID, cas) + Processor::new( + StaticChannelSet::new(), + USER_CHANNEL_ID, + IO_CHANNEL_ID, + Some(MESSAGE_CHANNEL_ID), + SHARE_ID, + cas, + ) } -/// Encode a ShareDataPdu as a server-to-client SendDataIndication frame. -fn encode_server_share_data(pdu: ShareDataPdu) -> Vec { - let share_data_header = ShareDataHeader { - share_data_pdu: pdu, - stream_priority: StreamPriority::Medium, - compression_flags: CompressionFlags::empty(), - compression_type: CompressionType::K8, - }; - - let share_control_header = ShareControlHeader { - share_control_pdu: ShareControlPdu::Data(share_data_header), - pdu_source: USER_CHANNEL_ID, - share_id: SHARE_ID, - }; - - let user_data = encode_vec(&share_control_header).unwrap(); +/// Encode an Auto-Detect Request as a server-to-client SendDataIndication on the +/// MCS message channel ([MS-RDPBCGR] 2.2.14.3): the auto-detect data is framed by +/// a Basic Security Header (SEC_AUTODETECT_REQ), not a Share Data header. +fn encode_server_autodetect(request: AutoDetectRequest) -> Vec { + let pdu = AutoDetectReqPdu::new(request); + let user_data = encode_vec(&pdu).unwrap(); let indication = McsMessage::SendDataIndication(SendDataIndication { initiator_id: USER_CHANNEL_ID, - channel_id: IO_CHANNEL_ID, + channel_id: MESSAGE_CHANNEL_ID, user_data: Cow::Owned(user_data), }); @@ -96,7 +90,7 @@ fn encode_server_share_data(pdu: ShareDataPdu) -> Vec { fn rtt_request_produces_response_frame() { let mut processor = make_processor(); let request = AutoDetectRequest::rtt_continuous(42); - let frame = encode_server_share_data(ShareDataPdu::AutoDetectReq(request)); + let frame = encode_server_autodetect(request); let outputs = processor.process(&frame).unwrap(); @@ -114,7 +108,7 @@ fn rtt_response_preserves_sequence_number() { let mut processor = make_processor(); let sequence_number = 0x1234; let request = AutoDetectRequest::rtt_connect_time(sequence_number); - let frame = encode_server_share_data(ShareDataPdu::AutoDetectReq(request)); + let frame = encode_server_autodetect(request); let outputs = processor.process(&frame).unwrap(); @@ -123,24 +117,25 @@ fn rtt_response_preserves_sequence_number() { panic!("expected ResponseFrame"); }; - // The response frame wraps X224 > MCS SendDataRequest > ShareControl > ShareData > AutoDetectRsp. - // Decode the MCS layer to extract user data, then decode the share headers. + // The response is a Client Auto-Detect Response PDU on the message channel: + // X224 > MCS SendDataRequest > BasicSecurityHeader(SEC_AUTODETECT_RSP) > data. let mcs_msg = ironrdp_core::decode::>>(response_data).unwrap(); let McsMessage::SendDataRequest(send_data) = mcs_msg.0 else { panic!("expected SendDataRequest in response frame"); }; - let share_control = ironrdp_core::decode::(&send_data.user_data).unwrap(); - let ShareControlPdu::Data(share_data) = share_control.share_control_pdu else { - panic!("expected Data PDU in ShareControl"); - }; - - match share_data.share_data_pdu { - ShareDataPdu::AutoDetectRsp(AutoDetectResponse::RttResponse { + assert_eq!( + send_data.channel_id, MESSAGE_CHANNEL_ID, + "response must be sent on the message channel" + ); + + let response = ironrdp_core::decode::(&send_data.user_data).unwrap(); + match response.response { + AutoDetectResponse::RttResponse { sequence_number: rsp_seq, - }) => { + } => { assert_eq!(rsp_seq, sequence_number, "sequence number must be echoed"); } - other => panic!("expected AutoDetectRsp(RttResponse), got {other:?}"), + other => panic!("expected RttResponse, got {other:?}"), } } @@ -148,7 +143,7 @@ fn rtt_response_preserves_sequence_number() { fn network_characteristics_result_surfaces_as_autodetect() { let mut processor = make_processor(); let request = AutoDetectRequest::netchar_result(7, 10, 50000, 20); - let frame = encode_server_share_data(ShareDataPdu::AutoDetectReq(request.clone())); + let frame = encode_server_autodetect(request.clone()); let outputs = processor.process(&frame).unwrap(); @@ -165,7 +160,7 @@ fn network_characteristics_result_surfaces_as_autodetect() { fn bandwidth_measure_start_does_not_crash() { let mut processor = make_processor(); let request = AutoDetectRequest::bw_start_connect_time(100); - let frame = encode_server_share_data(ShareDataPdu::AutoDetectReq(request)); + let frame = encode_server_autodetect(request); let outputs = processor.process(&frame).unwrap(); assert!(outputs.is_empty(), "BW start should produce no output"); @@ -175,7 +170,7 @@ fn bandwidth_measure_start_does_not_crash() { fn bandwidth_measure_stop_does_not_crash() { let mut processor = make_processor(); let request = AutoDetectRequest::bw_stop_continuous(200); - let frame = encode_server_share_data(ShareDataPdu::AutoDetectReq(request)); + let frame = encode_server_autodetect(request); let outputs = processor.process(&frame).unwrap(); assert!(outputs.is_empty(), "BW stop should produce no output"); @@ -185,7 +180,7 @@ fn bandwidth_measure_stop_does_not_crash() { fn bandwidth_measure_payload_does_not_crash() { let mut processor = make_processor(); let request = AutoDetectRequest::bw_payload(300, vec![0xAA; 64]); - let frame = encode_server_share_data(ShareDataPdu::AutoDetectReq(request)); + let frame = encode_server_autodetect(request); let outputs = processor.process(&frame).unwrap(); assert!(outputs.is_empty(), "BW payload should produce no output");