Skip to content

feat(connector): implement multitransport bootstrapping handshake#1098

Open
Greg Lamberson (glamberson) wants to merge 3 commits into
Devolutions:masterfrom
glamberson:multitransport-connector
Open

feat(connector): implement multitransport bootstrapping handshake#1098
Greg Lamberson (glamberson) wants to merge 3 commits into
Devolutions:masterfrom
glamberson:multitransport-connector

Conversation

@glamberson

@glamberson Greg Lamberson (glamberson) commented Feb 12, 2026

Copy link
Copy Markdown
Contributor

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 TLS
upgrade and CredSSP, but uses complete_X() / skip_X() rather than
mark_X_as_done() because completion carries result data:

  • should_perform_multitransport(): true when requests have been collected
  • multitransport_requests(): access the server's request PDUs
  • complete_multitransport(results, output): send response PDUs, advance
  • skip_multitransport(output): decline, advance

complete_multitransport accepts &[MultitransportResult] (a Success /
Failure(hresult) enum) rather than caller-built response PDUs. The
connector 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
MultitransportRequestPdu first. The decoder validates SEC_TRANSPORT_REQ
internally, 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 MultitransportPending and replays it through
ConnectionActivationSequence::step() when complete_multitransport()
or skip_multitransport() is called.

Driver wiring. Both ironrdp-async and ironrdp-blocking
connect_finalize loops now auto-skip multitransport: when the connector
reports should_perform_multitransport(), the driver calls
skip_multitransport() on the caller's behalf so the connection succeeds
TCP-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 and
UDP 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
MultitransportRequestPdu emission, which ironrdp::server::RdpServer
does not implement today. Integration coverage at ironrdp-testsuite-extra
will follow once server-side multitransport support is in place; the
connector-side state surface added here is the prerequisite.

Verification

  • cargo xtask check fmt -v green
  • cargo xtask check lints -v green
  • cargo xtask check tests -v green
  • cargo xtask check typos -v green
  • cargo xtask check locks -v green

References

Builds on: #1091
Related: #140

@glamberson

Copy link
Copy Markdown
Contributor Author

I really need feedback on this so I'll take the draft status off and see where we get.

@glamberson Greg Lamberson (glamberson) marked this pull request as ready for review February 14, 2026 14:35
@CBenoit

Copy link
Copy Markdown
Member

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.

@glamberson

Copy link
Copy Markdown
Contributor Author

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.

@CBenoit

Benoît Cortier (CBenoit) commented Feb 22, 2026

Copy link
Copy Markdown
Member

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.

@glamberson

Copy link
Copy Markdown
Contributor Author

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:

  1. Response PDU ownership: should complete_multitransport() accept the caller's MultitransportResponsePdus (connector writes them to output), or should the connector generate them from a success/failure result per request?
  2. Scope of feat(connector,session): dispatch multitransport PDUs on IO channel #1108: if the connector handles multitransport bootstrapping internally during initial connection, does the session layer still need to surface MultitransportRequest for server-initiated multitransport during an active session (MS-RDPBCGR 2.2.15.1), or is that out of scope for now?

No rush, happy to wait until Tuesday.

Greg

@glamberson Greg Lamberson (glamberson) force-pushed the multitransport-connector branch 2 times, most recently from 7815daa to c9947d4 Compare March 17, 2026 10:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 MultitransportBootstrapping request collection and a new MultitransportPending pause state.
  • Introduced a connector API for multitransport handling: should_perform_multitransport(), multitransport_requests(), complete_multitransport(), and skip_multitransport().
  • Updated next_pdu_hint() and step() 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.

Comment thread crates/ironrdp-connector/src/connection.rs Outdated
Comment thread crates/ironrdp-connector/src/connection.rs Outdated
Comment thread crates/ironrdp-connector/src/connection.rs Outdated
Comment thread crates/ironrdp-connector/src/connection.rs
Comment thread crates/ironrdp-connector/src/connection.rs
@glamberson

Copy link
Copy Markdown
Contributor Author

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.

  1. Response ownership: the current implementation takes caller-constructed MultitransportResponsePdus. Given your preference for option (b) where the connector owns more state internally, it might be cleaner for complete_multitransport() to accept a success/failure enum per request and let the connector build the response PDUs (it already has the request_id and cookie). This also naturally validates the response count (Copilot thread on line 287).

  2. Demand Active buffering: the current code requires the caller to re-feed the Demand Active PDU after calling complete/skip_multitransport(). Option (b) would have the connector buffer it internally and replay it on state transition. This would eliminate the driver regression risk Copilot flagged (line 762) since existing drivers wouldn't need updating.

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.

@glamberson

Copy link
Copy Markdown
Contributor Author

I went ahead and forged ahead despite my last comment. If you want to revise jsut let me know.

In summary:

  1. Demand Active buffered internally. MultitransportPending stores the raw PDU bytes and replays them through ConnectionActivationSequence::step() when the app calls complete_multitransport() or skip_multitransport(). Drivers don't
    need re-feed logic.

  2. MultitransportResult enum. Callers pass Success or Failure(hresult) instead of building response PDUs. The connector pairs each with the stored request ID/cookie and validates the count.

  3. Try-decode detection. Instead of peeking at BasicSecurityHeaderFlags bits (which could false-positive on a Demand Active totalLength), we attempt decode::(). The decoder validates SEC_TRANSPORT_REQ internally, so failure cleanly means "not multitransport".

  4. mem::replace for state extraction. Both complete_multitransport() and skip_multitransport() use mem::replace(&mut self.state, Consumed), matching step()'s pattern. Returns ConnectorError on wrong state instead of panicking.

Thanks.

Comment thread crates/ironrdp-connector/src/connection.rs Outdated
@glamberson

Copy link
Copy Markdown
Contributor Author

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>
@glamberson

Copy link
Copy Markdown
Contributor Author

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:

  • `cargo xtask check fmt -v`
  • `cargo xtask check lints -v` (workspace, all-targets)
  • `cargo xtask check tests -v`
  • `cargo xtask check typos -v`
  • `cargo xtask check locks -v`

…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.
@glamberson

Greg Lamberson (glamberson) commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

Per improved internal prechecks I'm running on my PRs, took the following tightening actions:

  • Cap incoming multitransport requests at the MS-RDPBCGR 2.2.15.1 limit of 2 with a clean ConnectorError on overflow.
  • Drivers (ironrdp-async, ironrdp-blocking) connect_finalize now auto-skip multitransport instead of aborting when a server advertises it.
  • Rewrote the PR body for accuracy on the complete_X/skip_X naming and the new driver behavior.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants