fix(pdu)!: send NetworkAutoDetect over the MCS message channel#1348
Open
Greg Lamberson (glamberson) wants to merge 2 commits into
Open
fix(pdu)!: send NetworkAutoDetect over the MCS message channel#1348Greg Lamberson (glamberson) wants to merge 2 commits into
Greg Lamberson (glamberson) wants to merge 2 commits into
Conversation
Set the EXTENDED_CLIENT_DATA_SUPPORTED flag in the RDP Negotiation Response so the client sends Client Message Channel Data (section 2.2.1.3.7). When the client requests the message channel, allocate an MCS channel ID for it after the I/O channel and any static virtual channels, return it in Server Message Channel Data (section 2.2.1.4.5), and add it to the set of channels joined during the MCS channel join phase. The allocated ID is surfaced on AcceptorResult so the server can route PDUs that ride the message channel. The immediate consumer is server network auto-detect (section 2.2.14); the channel is also the transport for multitransport bootstrap and heartbeat. Previously the acceptor never advertised the channel, so a spec-compliant client withheld its Client Message Channel Data and these features had no transport.
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 (Devolutions#140). Depends on the acceptor message-channel negotiation in the parent PR.
There was a problem hiding this comment.
Pull request overview
This PR corrects Network Auto-Detect framing and routing to match MS-RDPBCGR by moving it off the I/O channel slow-path Share Data PDUs and onto the MCS message channel with the required Basic Security Header (SEC_AUTODETECT_REQ / SEC_AUTODETECT_RSP). This aligns IronRDP with mstsc/xfreerdp behavior and enables both connect-time and continuous auto-detection to actually function.
Changes:
- Add
AutoDetectReqPdu/AutoDetectRspPduwrappers (Basic Security Header framing) and remove the fabricated Share Data PDU representation for auto-detect. - Negotiate, join, and plumb the MCS message channel ID through acceptor/connector/session and use it for auto-detect traffic (client + server).
- Update and expand tests to cover message-channel negotiation and auto-detect request/response processing on that channel.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/ironrdp-testsuite-core/tests/session/autodetect.rs | Migrates session auto-detect tests to use message-channel framing and validates channel usage. |
| crates/ironrdp-testsuite-core/tests/server/acceptor.rs | Adds coverage ensuring the acceptor advertises Extended Client Data support and allocates a deterministic message channel ID. |
| crates/ironrdp-session/src/x224/mod.rs | Routes message-channel traffic to new auto-detect decoder/encoder path and stops handling auto-detect as Share Data. |
| crates/ironrdp-session/src/active_stage.rs | Wires negotiated message-channel ID into the session processor. |
| crates/ironrdp-server/src/server.rs | Sends auto-detect requests on message channel and decodes auto-detect responses from message channel. |
| crates/ironrdp-pdu/src/rdp/headers.rs | Removes auto-detect from ShareDataPdu / ShareDataPduType since it is not a real slow-path Share Data PDU. |
| crates/ironrdp-pdu/src/rdp/autodetect.rs | Introduces AutoDetectReqPdu / AutoDetectRspPdu with Basic Security Header framing + round-trip tests. |
| crates/ironrdp-connector/src/connection.rs | Requests message channel in GCC, joins it, enables connect-time auto-detect handling on the message channel, and propagates message_channel_id. |
| crates/ironrdp-acceptor/src/connection.rs | Negotiates and joins the message channel when requested and surfaces message_channel_id in AcceptorResult. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+513
to
+533
| let autodetect = self.message_channel_id.and_then(|message_channel_id| { | ||
| let mcs = decode::<X224<mcs::McsMessage<'_>>>(input).ok()?; | ||
| match mcs.0 { | ||
| mcs::McsMessage::SendDataIndication(data) if data.channel_id == message_channel_id => { | ||
| decode::<rdp::autodetect::AutoDetectReqPdu>(&data.user_data).ok() | ||
| } | ||
| _ => None, | ||
| } | ||
| }); | ||
|
|
||
| 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 { |
Comment on lines
1423
to
1427
| /// Encode a server-initiated Share Data PDU for the IO 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #1347 (acceptor message-channel negotiation). Until that merges,
the diff here also contains its commit; review the top commit
(
fix(pdu)!: send NetworkAutoDetect over the MCS message channel). I willrebase this to master-only once #1347 lands.
NetworkAutoDetect was modeled as a slow-path Share Data PDU
(ShareDataPduType::AutoDetect, a fabricated 0x3B value) 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 ever answered the requests.
ironrdp-pdu: add AutoDetectReqPdu / AutoDetectRspPdu, wrapping 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. This is the breaking change.
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 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).
Tested: the in-process client/server tests in ironrdp-testsuite-core exercise
the full negotiation and continuous auto-detect; new ironrdp-pdu round-trip
tests cover the security-header framing; the session auto-detect tests were
migrated to the message channel.