Skip to content

fix(rdpeusb)!: add the capability exchange on each per-device channel - #1711

Open
uchouT (uchouT) wants to merge 1 commit into
Devolutions:masterfrom
uchouT:fix/caps-exchange
Open

fix(rdpeusb)!: add the capability exchange on each per-device channel#1711
uchouT (uchouT) wants to merge 1 commit into
Devolutions:masterfrom
uchouT:fix/caps-exchange

Conversation

@uchouT

Copy link
Copy Markdown
Contributor

Reported by clintcan in #1417:

The per-device channel needs its own capability exchange, or mstsc closes it. Your per-device processor's start() (UrbdrcDeviceServer) returns CHANNEL_CREATED directly, with no preceding RIM_EXCHANGE_CAPABILITY_REQUEST — your main UrbdrcControlServer::start() sends one, the per-device one doesn't. That's fine on FreeRDP (its readiness is global to the main channel), but we found live (mstsc, mid-2026) that mstsc closes a per-device channel that gets CHANNEL_CREATED / RIMCALL_RELEASE without a preceding capability exchange — it keeps the channel open-but-silent waiting for the caps request, so the device never emits ADD_DEVICE. Our per-device processor therefore runs the full caps → CHANNEL_CREATED handshake on each per-device channel, same as the main one.

Changes

  • UrbdrcDeviceServer::start() emits RIM_EXCHANGE_CAPABILITY_REQUEST; on the response it releases the capabilities interface and sends CHANNEL_CREATED, mirroring UrbdrcControlServer.
  • UrbdrcDeviceClient answers a capability request on the per-device channel.
  • The capabilities interface is decodable on the per-device channel in both directions.
  • Device handshake state is now AwaitingCaps → AwaitingChanCreated → AwaitingDevice → Ready, so ADD_DEVICE is processed only after the handshake completes per 3.2.5.2.2.

BREAKING CHANGE: UrbdrcServerDevicePdu and UrbdrcClientDevicePdu are public and not #[non_exhaustive], so the new Caps variant breaks exhaustive matches.

BREAKING CHANGE: `UrbdrcServerDevicePdu` and `UrbdrcClientDevicePdu` are
public and not `#[non_exhaustive]`, so the new `Caps` variant breaks
exhaustive matches.

Signed-off-by: uchouT <i@uchout.moe>
Copilot AI balanced review requested due to automatic review settings August 20, 2026 01:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added kind/protocol Affects RDP or related protocol behavior maintainer-required Maintainer review or intervention is required risk/medium Behavioral change that does not substantially alter a core public API scope/core Touches the core architectural tier size/S Size: up to 199 counted lines and 5 files; exceeds XS in either measure ai-reviewed/1 One automated review completed labels Aug 20, 2026
@clintcan

Copy link
Copy Markdown
Contributor

Thanks for picking this up, uchouT (@uchouT) — and for the credit. macrdp carries the server-direction per-device handshake as our divergence 16, so I can confirm the core of your fix: having the per-device start() send RimExchangeCapabilityRequest before CHANNEL_CREATED is exactly the direction that unblocks mstsc. We verified that live (mstsc, 2026-07-06): with the per-device caps exchange, mstsc completes the handshake and emits ADD_DEVICE; without it, it never announces the device (it closes the per-device channel). FreeRDP tolerates skipping it — its readiness is global to the main channel — which is why it wasn't caught there.

One sequencing detail worth double-checking against mstsc, since it's strict about the per-device handshake order and I can't be 100% sure from the diff alone. macrdp's mstsc-verified sequence after the caps response is:

caps-response → CHANNEL_CREATED (alone) → [client acks] → RIMCALL_RELEASE (alone)

i.e. we send CHANNEL_CREATED by itself, and only send RIMCALL_RELEASE afterward as a separate step — it's the barrier that actually makes mstsc emit ADD_DEVICE. Your AwaitingCaps arm looks like it emits InterfaceRelease + ChannelCreated together in one response; if mstsc needs them separated/ordered the way we found, the symptom would be the channel completing but no ADD_DEVICE arriving. I'm not certain the bundled form fails — just flagging it as the exact spot mstsc was picky for us.

Happy to settle it definitively: I have an mstsc + entitled-macrdp rig with a real redirected device (the path that found this gap), so if you point me at your branch I'll run your actual code against mstsc and report whether the device enumerates end-to-end (gamepad / mass-storage). That's the one check neither the spec nor FreeRDP can stand in for here.

@uchouT

Copy link
Copy Markdown
Contributor Author

clintcan:

One sequencing detail worth double-checking against mstsc, since it's strict about the per-device handshake order and I can't be 100% sure from the diff alone. macrdp's mstsc-verified sequence after the caps response is:

caps-response → CHANNEL_CREATED (alone) → [client acks] → RIMCALL_RELEASE (alone)

i.e. we send CHANNEL_CREATED by itself, and only send RIMCALL_RELEASE afterward as a separate step — it's the barrier that actually makes mstsc emit ADD_DEVICE. Your AwaitingCaps arm looks like it emits InterfaceRelease + ChannelCreated together in one response; if mstsc needs them separated/ordered the way we found, the symptom would be the channel completing but no ADD_DEVICE arriving. I'm not certain the bundled form fails — just flagging it as the exact spot mstsc was picky for us.

Thanks for your remind. On the sequencing: the Ok(resp) vec isn't a bundle. encode_dvc_messages emits one DrdynvcDataPdu per DvcMessage, so those go out as two separate messages. The current sequence:

caps-response → RIMCALL_RELEASE(CAPABILITIES) → CHANNEL_CREATED → [client ack] → RIMCALL_RELEASE(NOTIFY_CLIENT)

So the shape you verified is all present; the only delta is that I release the capabilities interface before CHANNEL_CREATED.

Happy to settle it definitively: I have an mstsc + entitled-macrdp rig with a real redirected device (the path that found this gap), so if you point me at your branch I'll run your actual code against mstsc and report whether the device enumerates end-to-end (gamepad / mass-storage). That's the one check neither the spec nor FreeRDP can stand in for here.

Thanks, that's the confirmation I couldn't get myself. Branch: Branch: https://github.com/uchouT/IronRDP/tree/fix/caps-exchange

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-reviewed/1 One automated review completed kind/protocol Affects RDP or related protocol behavior maintainer-required Maintainer review or intervention is required risk/medium Behavioral change that does not substantially alter a core public API scope/core Touches the core architectural tier size/S Size: up to 199 counted lines and 5 files; exceeds XS in either measure

Development

Successfully merging this pull request may close these issues.

3 participants