[Marc-André Moreau (mamoreau-devolutions)]

Add RDP-UDP and RDPEMT wire codecs for the transport foundation.

Support FEC, ACK-of-ACK, correlation ID, and extended SYN headers while preserving source-payload framing boundaries and rejecting invalid RDPEMT create-PDU subheaders.

[Copilot]

Pull request overview

Adds foundational RDP-UDP and RDPEMT wire codecs to ironrdp-pdu.

Changes:

• Adds reliable RDP-UDP headers, handshake data, ACK vectors, and payload codecs.
• Adds RDPEMT tunnel creation/data codecs with validation.
• Exposes the new UDP module and adds unit tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
crates/ironrdp-pdu/src/rdp/udp.rs	Implements RDP-UDP wire structures.
crates/ironrdp-pdu/src/rdp/multitransport.rs	Implements RDPEMT tunnel PDUs.
crates/ironrdp-pdu/src/rdp/mod.rs	Exports the UDP module.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Wire layouts are largely correct: the tunnel-header nibble split, header/payload length rules, MTU bounds, big-endian RDPEUDP fields, and ACK-vector alignment all match the cited spec text. Four issues warrant changes. The four new tunnel Encode impls omit the ensure_size guard every neighbouring PDU uses, so encoding into an undersized caller buffer panics instead of returning NotEnoughBytes. RdpUdpFlags omits the documented 0x0080 SACK_OPTION bit, and since decode uses from_bits, a peer setting it makes the common header undecodable. TunnelHeader::decode reports truncation as an invalid field and rejects trailing bytes while exposing no framing helper. The ACK-vector decoder rejects non-zero padding the spec does not constrain. Non-blocking: two deep clones per encoded data PDU on the tunnel hot path, an orphan RdpUdpProtocolVersion, self-round-trip-only tests with no spec fixtures, RDP-UDP layered under the rdp module, and a dead length check.

Protocol analysis: partially_accepted — Three of four discrepancies survive verification: the missing RDPUDP_FLAG_SACK_OPTION 0x0080 (confirmed independently by walking the declared bit sequence, which jumps 0x0040 to 0x0100), the zero-padding rejection in RdpUdpAckVector::decode, and truncated tunnel PDUs collapsing into invalid-field. For the last I checked the supporting facts in-tree: ironrdp-core models NotEnoughBytes separately, and this crate already exposes PduHint/find_size framing helpers. read_padding ignoring content likewise supports the ACK-vector point. I downgrade the fourth: is_success comparing to zero deviates from HRESULT severity semantics but matches the existing convention in the same file and Windows behaviour, so it is low. The conforming assessments check out against the diff bytes.

[Greg Lamberson (glamberson)]

I have a complete RDPEUDP and RDPEUDP2 implementation with RDPEMT tunnelling and an async driver, sans-I/O throughout, around 13,000 lines with 364 passing tests. I raised it on #140 in February along with the five upstream seams it needs and a design question about MultitransportBootstrapping, and said I would bring it whenever there was appetite. That offer stands. A heads-up that this work was starting would have been welcome.

To the review. I am reading it against MS-RDPEUDP and against an implementation that has run against real clients, which is where the findings come from.

The common header rejects a spec-defined flag. 2.2.2.1 defines RDPUDP_FLAG_SACK_OPTION 0x0080. RdpUdpFlags omits it and decode uses from_bits(...).ok_or_else(...), so a peer that sets it makes the header of every datagram undecodable rather than that one bit unknown. from_bits_truncate or from_bits_retain would also let the type survive flags added later, which is the direction #1489 just took for unknown GCC blocks.

V3 is advertisable and unspeakable. RdpUdpProtocolVersion::V3 = 0x0101 is declared and decoded, but 1.3.2.2 says "If the UDP Transport Extension version negotiated in the UDP connection initialization phase is version 3 or higher, the UDP data transfer is defined in [MS-RDPEUDP2]. The UDP data transfer messages described in this document MUST be used only when the version negotiated in the UDP connection initialization phase is version 1 or version 2." None of that framing is here. Neither is 2.2.2.9 RDPUDP_SYNDATAEX_PAYLOAD, which is the structure carrying uUdpVer in the first place and whose cookieHash MUST be present in a client SYN when uUdpVer is 0x0101. The version can be named, not negotiated, and not spoken. Windows 11 mstsc negotiates it by default.

Four flags announce structures that are not defined. FEC points at 2.2.2.2, ACK_OF_ACKS at 2.2.2.6, CORRELATION_ID at 2.2.2.8, SYN_EX at 2.2.2.9. None are present, and the module doc claims the file covers "2.2.2.1, 2.2.2.4-2.2.2.7", which includes 2.2.2.6.

RdpUdpSourcePayload::decode consumes the rest of the cursor. 2.2.2.4 is an 8-byte header. read_remaining() means the type cannot be decoded as part of a datagram carrying anything after it, and the flag set models exactly those combinations.

Placement. Every per-protocol crate here owns its PDUs: ironrdp-egfx/src/pdu, ironrdp-cliprdr/src/pdu, ironrdp-rdpdr/src/pdu, ironrdp-rdpsnd/src/pdu, ironrdp-displaycontrol/src/pdu, ironrdp-echo/src/pdu.rs, and ironrdp-rdpeusb already exists for its own spec. #1303 moved deliberately in that direction by removing the egfx duplicates from ironrdp-pdu. RDPEUDP is its own spec family on its own transport, so udp.rs reads like it wants to be ironrdp-rdpeudp. multitransport.rs is different and belongs where you put it, since 2.2.15 rides the main channel next to nego and gcc.

What is right, since a review that only lists problems is not much use: network byte order is correct, the 2048 byte ACK vector cap matches 2.2.2.7, the DWORD padding is computed against the structure rather than the datagram which is what 2.2.2.7 asks for, and the 1132 to 1232 MTU range is a MUST on both fields so rejecting out-of-range values on decode is defensible.

The pieces missing above are the pieces I already have, including the RDPEUDP2 framing, SYNDATAEX, the ACK-of-acks and correlation ID payloads, and the transport state machine underneath all of it. Say the word and I will bring them.

[github-actions]

This pull request is size/XL, so automated review is disabled for it: a change this large is hard to review well in one piece, whether by a human or a model.

Please split it into focused pull requests that can each be reviewed on their own. When the parts build on each other, stacked pull requests let you open each one on top of the last without waiting for the one below to merge. Stacks require every branch to live in this repository, so from a fork, please open separate pull requests instead.

Automated review resumes once the change is below the size/XL threshold.