feat(connector): implement multitransport bootstrapping handshake#1098
feat(connector): implement multitransport bootstrapping handshake#1098Greg Lamberson (glamberson) wants to merge 3 commits into
Conversation
|
I really need feedback on this so I'll take the draft status off and see where we get. |
|
Hi! Good job. I need a little bit more time to dig into this and craft a well thought answer. I’ll come back to you in the next 24h. |
Thanks! Great! Yes this is pretty important so I knew it would take some time. Much appreciated. |
3585d1b to
6ab896d
Compare
|
Hi! I’m sorry for the delay. Monday is a holiday for me, so I may only come back to you on Tuesday! FYI, I think I may be leaning on b), because the overall architecture stays "sans-IO", it can be thought as of "state-machine responsibility" where the “pause” is part of the protocol state machine. If the state machine consumes the bytes that triggered the pause, it could thus own deferring/replaying them; we avoid duplicating subtle handshake logic in every driver. Otherwise we’ll need the same “connect_finalize must stash last PDU and replay it later” logic in ironrdp-async, ironrdp-blocking, and any third-party driver using the connector directly. Last, this makes the API more misuse resistant, because with a) it’s easy for the caller to forget to buffer, or buffer the wrong input (e.g. after a faulty framing), and so on. This also keeps the API consistent with the existing should_perform_X / complete_X, etc. Once we call complete_multitransport() or skip_multitransport(), the connector can immediately continue processing without requiring extra steps. But I’m still thinking a bit on this, and double checking other parts of the spec. |
That's fine. I relocated for now back to Illinois in the USA, and I'm just now getting up and running here. Regarding your inclinations, (b) makes sense. Keeps replay logic in one place and the driver API stays clean. I'll rework to buffer the Demand Active internally once you confirm. Two questions that would help me get the rework right:
No rush, happy to wait until Tuesday. Greg |
7815daa to
c9947d4
Compare
There was a problem hiding this comment.
Pull request overview
Implements the multitransport bootstrapping portion of the RDP connector state machine by collecting optional server Initiate Multitransport Request PDU(s) after licensing and exposing an application-driven pause point before proceeding to capabilities exchange.
Changes:
- Added
MultitransportBootstrappingrequest collection and a newMultitransportPendingpause state. - Introduced a connector API for multitransport handling:
should_perform_multitransport(),multitransport_requests(),complete_multitransport(), andskip_multitransport(). - Updated
next_pdu_hint()andstep()logic to route post-licensing traffic into the multitransport flow.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
Coming back to the design discussion from February. Two questions are still open and they affect how I address the new Copilot review comments, so I'd like to settle them first.
If you confirm (b) on both points I'll rework accordingly. The detection logic (distinguishing multitransport from Demand Active via BasicSecurityHeader flags) and the state extraction pattern are independent of this choice and I can address those either way. |
c9947d4 to
89ff5d6
Compare
|
I went ahead and forged ahead despite my last comment. If you want to revise jsut let me know. In summary:
Thanks. |
89ff5d6 to
130bbc8
Compare
|
Benoît Cortier (@CBenoit), polite ping on this one. The option-b reshape (connector-internal Demand Active replay, MultitransportResult enum, try-decode detection, mem::replace for state extraction) has been in place since 2026-02-23. Just rebased on master to keep CI fresh; happy to address any new review whenever you have time. |
Makes MultitransportBootstrapping functional: reads the server's optional Initiate Multitransport Request PDUs (0, 1, or 2), then pauses in MultitransportPending for the application to establish UDP transport before proceeding to capabilities exchange. Follows the existing should_perform_X() / mark_X_as_done() pattern used by TLS upgrade and CredSSP.
The doc-comment on MultitransportPending described the connector as "paused waiting for the application to establish UDP transport," which reads as a spec-mandated TCP gate on UDP completion. Per review feedback, real RDP traffic shows TCP and UDP negotiating in parallel, with UDP completion functioning as a DVC migration signal rather than a TCP gate. Rewrite the comment to describe the state as the connector's API yield point (what it is) and add a paragraph describing the actual wire behavior. State machine semantics unchanged. Signed-off-by: Greg Lamberson <greg@lamco.io>
eee839f to
e1d0396
Compare
|
Rebased onto current master. Force-pushed `eee839fa` -> `e1d03960`. 20 days behind master since the 2026-05-19 doc-comment fix; rebase was clean (no conflicts, despite master having picked up #1254 / #1250 / #1273 / #1272 in the connector vicinity). David Fort's review thread (`discussion_r3269401135`) remains resolved. All five xtask gates green on the new head:
|
…ip in drivers Enforce the MS-RDPBCGR 2.2.15.1 0-1-2 limit on incoming Initiate Multitransport Request PDUs at the connector layer by rejecting any third request with a ConnectorError; the requests Vec is preallocated to the spec cap. Wire both connect_finalize loops (ironrdp-async and ironrdp-blocking) to auto-skip multitransport when the connector reports should_perform_multitransport(). Existing driver consumers no longer abort when a server advertises multitransport; the connection succeeds TCP-only. Applications that want to participate in multitransport setup must drive the connector directly via complete_multitransport() instead of calling connect_finalize.
|
Per improved internal prechecks I'm running on my PRs, took the following tightening actions:
|
Summary
Makes the MultitransportBootstrapping state functional instead of a no-op
pass-through. After licensing the server may send 0, 1, or 2 Initiate
Multitransport Request PDUs before capabilities exchange. The connector
reads those, pauses in a new MultitransportPending state, and surfaces
the requests for the application to establish UDP transport (RDPEUDP2 +
TLS + RDPEMT) or decline.
API
Mirrors the existing
should_perform_X()pause-point pattern used by TLSupgrade and CredSSP, but uses
complete_X()/skip_X()rather thanmark_X_as_done()because completion carries result data:should_perform_multitransport(): true when requests have been collectedmultitransport_requests(): access the server's request PDUscomplete_multitransport(results, output): send response PDUs, advanceskip_multitransport(output): decline, advancecomplete_multitransportaccepts&[MultitransportResult](aSuccess/Failure(hresult)enum) rather than caller-built response PDUs. Theconnector builds the response PDUs internally using the stored request
IDs and validates that the result count matches the request count.
Approach
Detection. The state machine attempts to decode the incoming PDU as
MultitransportRequestPdufirst. The decoder validatesSEC_TRANSPORT_REQinternally, so a Demand Active PDU fails cleanly without false positives.
No flag-peeking on
BasicSecurityHeader.Demand Active buffering. The connector buffers the Demand Active PDU
internally in
MultitransportPendingand replays it throughConnectionActivationSequence::step()whencomplete_multitransport()or
skip_multitransport()is called.Driver wiring. Both
ironrdp-asyncandironrdp-blockingconnect_finalizeloops now auto-skip multitransport: when the connectorreports
should_perform_multitransport(), the driver callsskip_multitransport()on the caller's behalf so the connection succeedsTCP-only. Applications that want to participate in multitransport setup
must drive the connector directly rather than calling
connect_finalize.Request count. Per MS-RDPBCGR 2.2.15.1 the server may send 0, 1, or 2
requests (one per transport protocol). The connector rejects subsequent
requests with a
ConnectorError.Wire behavior. Per the doc-comment on
MultitransportPending, TCP andUDP negotiation happen in parallel. The UDP transport is established
alongside the ongoing TCP handshake; its completion is a signal to the
dynamic-channel layer that subsequent channels may migrate to UDP. The
connector's API yield point here is a Rust affordance, not a
spec-mandated TCP pause. Thanks to David Fort (@hardening) for the correction.
Tests
State-machine tests for this PR would require server-side
MultitransportRequestPduemission, whichironrdp::server::RdpServerdoes not implement today. Integration coverage at
ironrdp-testsuite-extrawill follow once server-side multitransport support is in place; the
connector-side state surface added here is the prerequisite.
Verification
cargo xtask check fmt -vgreencargo xtask check lints -vgreencargo xtask check tests -vgreencargo xtask check typos -vgreencargo xtask check locks -vgreenReferences
Builds on: #1091
Related: #140