feat(rdpeusb)!: carry the full configuration descriptor in UsbConfigDesc by clintcan · Pull Request #1420 · Devolutions/IronRDP

clintcan · 2026-07-08T21:42:31Z

What

TS_URB_SELECT_CONFIGURATION sets ConfigurationDescriptorIsValid and then encodes a UsbConfigDesc, but only the 9-byte USB_CONFIGURATION_DESCRIPTOR header was modelled — wTotalLength would claim e.g. 759 bytes with only 9 following.

Per [MS-RDPEUSB] 2.2.9.2 the field must carry the full configuration descriptor (all interface / endpoint / class-specific bytes) when the flag is set. Real Windows (mstsc) walks wTotalLength and rejects a header-only descriptor with 0x80070057 (E_INVALIDARG), so URB_SELECT_CONFIGURATION fails against a real Windows peer. FreeRDP uses the interface array and ignores the descriptor bytes, which masks this.

Change

Adds a trailing: Vec<u8> field holding bytes 9..wTotalLength:

encode writes it after the 9-byte header.
decode reads wTotalLength - 9 bytes, clamped to what is present — so a header-only descriptor still decodes (with trailing empty), and a spec-conformant full descriptor's trailing bytes are now consumed from the cursor instead of left dangling.

BREAKING CHANGE: UsbConfigDesc gains the public field trailing: Vec<u8> (all fields are public, so struct-literal constructors need it).

Tests

Adds tests/rdpeusb/ts_urb.rs (wired into the rdpeusb testsuite): full_config_descriptor_round_trips, header_only_descriptor_still_decodes, and select_configuration_carries_full_descriptor.

Context

Found while building a server-direction (client-redirected-USB) consumer of this crate, and verified live: mstsc SelectConfiguration succeeds (pipe handles returned) for a redirected camera + audio/HID device; FreeRDP mass storage is unaffected. Independent of #1418 (a different file/type in the same crate) — this branch is rebased on current master, not stacked on it.

Gates run locally: cargo fmt --all -- --check, cargo clippy -p ironrdp-rdpeusb --all-targets --locked -D warnings, and the rdpeusb testsuite (all green).

Copilot

Pull request overview

This PR fixes RDPEUSB TS_URB_SELECT_CONFIGURATION configuration-descriptor handling by modelling and encoding/decoding the full USB configuration descriptor (not just the 9-byte header) so wTotalLength matches the actual payload Windows peers expect.

Changes:

Extend UsbConfigDesc with trailing: Vec<u8> and encode/decode it as bytes 9..wTotalLength (clamped on decode to tolerate header-only inputs).
Update size calculation for UsbConfigDesc so parent PDUs account for the full descriptor length.
Add rdpeusb tests covering full-descriptor round-trip, header-only tolerance, and containment within TS_URB_SELECT_CONFIGURATION.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
crates/ironrdp-rdpeusb/src/pdu/usb_dev/ts_urb/utils.rs	Adds `trailing` bytes to `UsbConfigDesc` and updates encode/decode/size to carry the full configuration descriptor.
crates/ironrdp-testsuite-core/tests/rdpeusb/ts_urb.rs	Adds unit tests validating full vs header-only configuration descriptor behavior and URB round-tripping.
crates/ironrdp-testsuite-core/tests/rdpeusb/mod.rs	Wires the new `ts_urb` test module into the rdpeusb testsuite.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

    fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
-        ensure_fixed_part_size!(in: dst);
+        ensure_size!(in: dst, size: self.size());

        dst.write_u8(self.length);
        dst.write_u8(self.descriptor_type);


clintcan · 2026-07-09T19:20:58Z

Addressed the review point in 2cde264f: UsbConfigDesc::encode now validates the header against the payload before writing — bLength must be the 9-byte header length and wTotalLength must equal the header plus the trailing bytes (which also rejects a trailing too large for wTotalLength to address). So a caller can no longer emit a descriptor whose wTotalLength disagrees with the bytes that follow — the exact shape a real Windows peer rejects with 0x80070057. Added inconsistent_header_fails_to_encode covering both a mismatched wTotalLength and a wrong bLength.

…values `UsbDeviceCaps::decode` rejected any value it did not name for the four device-reported capability fields of USB_DEVICE_CAPABILITIES (UsbBusInterfaceVersion, USBDI_Version, SupportedUsbVersion, DeviceIsHighSpeed). A real USB 3.2 device reports SupportedUsbVersion = 0x320, which was not named, so decoding its ADD_DEVICE failed and tore down the URBDRC channel. Model the four fields as newtype structs wrapping the raw `u32` (the `http::StatusCode` shape the crate's README recommends): named associated constants cover the documented values (SupportedUsbVer also names USB 3.0/3.1/3.2), while any other device-reported value is preserved verbatim instead of failing the decode. Unlike an `Other(u32)` enum fallback, a newtype cannot alias a named value, so round-trip identity and `Eq` stay exact. The framing constants (CbSize, HcdCapabilities) stay strict — they validate the PDU layout, not device data. The added tests decode a real 0x320 device's raw 28-byte USB_DEVICE_CAPABILITIES directly (independent of this crate's own encode, and pinning the field offset) and round-trip every named version plus an unnamed value. BREAKING CHANGE: SupportedUsbVer/UsbdiVer/UsbBusIfaceVer/DeviceSpeed are now newtype structs over u32 with associated constants (e.g. SupportedUsbVer::USB_20, DeviceSpeed::HIGH_SPEED) instead of fieldless enums; construct an arbitrary value with `from_u32` and read the wire value with `to_u32`.

…on 0 Per [MS-RDPEUSB] 2.2.11, DeviceIsHighSpeed MUST be 0x00000000 when UsbBusInterfaceVersion is 0x00000000. DeviceSpeed is now a lenient newtype that preserves any device-reported u32, so the constraint must reject every non-zero speed at version 0, not only the named HIGH_SPEED (0x1) — a reported value such as 0x2 was previously accepted. Compare against FULL_SPEED (0x0) instead. Add a raw-decode test pinning the constraint across the fixed and unaffected cases.

TS_URB_SELECT_CONFIGURATION sets ConfigurationDescriptorIsValid and then encodes a UsbConfigDesc, but only the 9-byte USB_CONFIGURATION_DESCRIPTOR header was modelled — wTotalLength would claim e.g. 759 bytes with only 9 following. [MS-RDPEUSB] 2.2.9.2 requires the full configuration descriptor (all interface/endpoint/class-specific bytes) when the flag is set; real Windows (mstsc) walks wTotalLength and rejects a header-only descriptor with 0x80070057 (E_INVALIDARG), so URB_SELECT_CONFIGURATION failed against a real Windows peer. (FreeRDP uses the interface array and ignores the descriptor bytes, which masked this.) Add a `trailing: Vec<u8>` field holding bytes 9..wTotalLength: encode writes it after the header, decode reads `wTotalLength - 9` bytes clamped to what is present — so a header-only descriptor still decodes, with `trailing` empty. Decode previously also left the trailing bytes unconsumed in the cursor when a peer sent a spec-conformant full descriptor; they are now consumed as part of the descriptor. Encode now validates the header against the payload: bLength must be the 9-byte header length and wTotalLength must equal the header plus the trailing bytes (which also rejects a `trailing` too large for wTotalLength to address). Otherwise a caller could emit a descriptor whose wTotalLength disagrees with the bytes that follow — exactly what a real Windows peer rejects. BREAKING CHANGE: UsbConfigDesc gains the public field `trailing: Vec<u8>` (all fields are public, so struct-literal constructors need the new field).

Benoît Cortier (CBenoit) requested a review from Copilot July 9, 2026 13:49

Copilot started reviewing on behalf of Benoît Cortier (CBenoit) July 9, 2026 13:49 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

clintcan force-pushed the feat/rdpeusb-full-config-descriptor branch from 1bf7e38 to 2cde264 Compare July 9, 2026 19:20

clintcan force-pushed the feat/rdpeusb-full-config-descriptor branch from 2cde264 to d24dc16 Compare July 13, 2026 21:16

Clint Christopher Canada added 2 commits July 17, 2026 03:13

clintcan force-pushed the feat/rdpeusb-full-config-descriptor branch from d24dc16 to 0fc2c28 Compare July 16, 2026 19:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(rdpeusb)!: carry the full configuration descriptor in UsbConfigDesc#1420

feat(rdpeusb)!: carry the full configuration descriptor in UsbConfigDesc#1420
clintcan wants to merge 3 commits into
Devolutions:masterfrom
clintcan:feat/rdpeusb-full-config-descriptor

clintcan commented Jul 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

clintcan commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

clintcan commented Jul 8, 2026

What

Change

Tests

Context

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

clintcan commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants