feat(server): add server-side UDP multitransport bootstrapping - #1951
Open
Greg Lamberson (glamberson) wants to merge 4 commits into
Open
Greg Lamberson (glamberson) wants to merge 4 commits into
Greg Lamberson (glamberson) wants to merge 4 commits into
Conversation
Add a MultitransportBootstrapping state to the acceptor sequence, entered right after licensing. It advertises UDP multitransport support in the GCC Server MultiTransportChannelData block when configured (set_multitransport_offer), and, once the client reciprocates the reliable-UDP flag, sends the Initiate Multitransport Request (MS-RDPBCGR 2.2.15.1) on the MCS message channel before moving straight on to capability negotiation. The acceptor does not wait for the client's Initiate Multitransport Response before continuing: MS-RDPBCGR 3.2.5.15.1 only obliges the client to send one when Soft-Sync is negotiated or the sideband attempt failed, so blocking on it would stall the handshake on the common successful path. multitransport_request() surfaces the sent request so the caller can establish the sideband UDP transport in parallel. A response that does arrive lands before the mandatory Confirm Active (the client sends it, if at all, before it ever reads Demand Active), so CapabilitiesWaitConfirm recognizes and drops it by channel rather than erroring on the unexpected payload. Adds ironrdp-testsuite-core coverage for the offer/no-offer/no-client- support paths and for the response-before-Confirm-Active ordering.
Greg Lamberson (glamberson)
temporarily deployed
to
llm-providers
September 10, 2026 19:08 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
github-actions
Bot
added
breaking-change
This was referenced Sep 10, 2026
Copilot started reviewing on behalf of
Benoît Cortier (CBenoit)
September 11, 2026 15:14
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Late multitransport responses can corrupt or abort connection finalization, and one test sends a protocol-invalid success response.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds server-side UDP multitransport negotiation and exposes bootstrap requests to callers.
Changes:
- Advertises configurable UDP multitransport support.
- Sends reliable-UDP bootstrap requests after licensing.
- Adds handshake integration tests.
File summaries
| File | Description |
|---|---|
crates/ironrdp-acceptor/src/connection.rs |
Implements negotiation and bootstrap state handling. |
crates/ironrdp-acceptor/Cargo.toml |
Adds randomness dependency. |
crates/ironrdp-testsuite-core/tests/server/acceptor.rs |
Tests multitransport handshake paths. |
Cargo.lock |
Records the dependency update. |
Review details
Suppressed comments (1)
crates/ironrdp-acceptor/src/connection.rs:1131
- This only accepts the response before Confirm Active, but the specification does not impose that ordering: a failure or Soft-Sync response is generated after the out-of-band sideband attempt finishes, which may be during connection finalization or later. Once this state advances,
FinalizationSequencetreats the same message-channel PDU as a finalization input (potentially advancing the wrong state or returning a decode error), so interoperable third-party clients can corrupt or abort the handshake. Route matching message-channel responses throughout the remaining acceptor/finalization lifecycle instead of relying on IronRDP client scheduling.
// An Initiate Multitransport Response can legitimately land
// here: it travels on the message channel, and the client
// sends it (when it sends one at all) while resolving its
// own multitransport bootstrapping, strictly before it ever
// reads the Demand Active that leads to Confirm Active. So
- Files reviewed: 3/4 changed files
- Comments generated: 2
- Review effort level: Balanced
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Adds opt-in server-side UDP multitransport bootstrapping: a GCC Server MultiTransportChannelData block, a post-licensing Initiate Multitransport Request on the MCS message channel, and tolerance of a late Initiate Multitransport Response during capability wait. The core flow, default-off behavior, and rand usage mirror the merged client connector and are sound. Published: the advertisement is not gated on the client's multitransport block (MS-RDPBCGR 2.2.1.4 MUST NOT); message-channel traffic is swallowed by channel-only classification in CapabilitiesWaitConfirm; a response after Confirm Active is not tolerated in finalization (abort or injection into input events) despite the 'guaranteed ordering' comment; plus one doc fix and one style simplification. Two compression-only suggestions were rejected.
github-actions
Bot
added
ai-reviewed/1
The Server MultiTransportChannelData block was advertised whenever the server's own offer was configured, regardless of whether the client actually populated its own Client MultiTransportChannelData block. MS-RDPBCGR 2.2.1.4 requires the server block to be omitted when the client did not send one. Added a client_offered_multitransport field tracking presence separately from multitransport_flags, which alone can't distinguish "absent" from "present but empty", and gated the offer on it. The Initiate Multitransport Response has no fixed position relative to the rest of the handshake (3.2.5.15.1): a conforming client can send it after Confirm Active, during ConnectionFinalization, not only before it. None of FinalizationSequence's own PDU decoders expect it, so depending which sub-state was active a late response was silently swallowed while advancing a state, propagated as a connection-ending decode error, or surfaced to the embedding application as a raw input event. Added the same tolerance CapabilitiesWaitConfirm already had to ConnectionFinalization. The late-response guard itself only checked channel and outstanding- request, not whether the payload actually decoded as a response. Since the message channel also carries Auto-Detect Response and Heartbeat PDUs (2.2.1.4.5, 2.2.8.1.1.2.1), that traffic was misclassified and dropped instead of falling through to its own handling. The guard now requires a successful strict decode, mirroring how ClientConnectorState::ConnectTimeAutoDetection demuxes the same channel client-side. Also: corrected the multitransport_request() doc, which claimed it returns None on reactivation when the carried-forward request in fact keeps it Some (intentional, needed for the late-response guard to keep working across reactivation); simplified an Option<u32> round-trip in the response-logging path down to a direct comparison, since the calling guard already guarantees a request is outstanding; and fixed two test cases constructing an S_OK response without the server advertising Soft-Sync, which 2.2.15.2 disallows. Regression tests added for the finalization tolerance and the non-response message-channel traffic case; both verified to fail against the prior behavior and pass with the fix.
Greg Lamberson (glamberson)
deployed
to
llm-providers
September 11, 2026 19:40 — with
GitHub Actions
Active
GitHub Actions
Active
github-actions
Bot
added
the
triage/overlap
Contributor
|
This pull request may overlap with #1953. Both center on acceptor-side UDP multitransport: this PR adds the acceptor's MultitransportBootstrapping state, offer configuration, and Initiate Multitransport Request exposed via multitransport_request() for out-of-band RDPEUDP2 establishment; PR This notice is advisory only. Automated review continues as usual, and how these pull requests relate is for maintainers and authors to decide. Note LLM-assisted content (no human feedback). |
Contributor
There was a problem hiding this comment.
PR 1951 adds opt-in server-side UDP multitransport bootstrapping to the acceptor: GCC advertisement gated on the client's own MultiTransportChannelData block, a write-only MultitransportBootstrapping state after licensing that sends the Initiate Multitransport Request on the message channel, and tolerant log-and-drop handling of late responses in both CapabilitiesWaitConfirm and ConnectionFinalization, demuxed by channel plus strict decode. Independent inspection of pr-head confirms the state flow, reactivation skip, default-off behavior, and demux guard; FinalizationSequence's WaitRequestControl does propagate decode errors, so the finalization tolerance is necessary. No protocol or correctness defects found. All six published findings are low-severity API-shape or maintainability items, each verified against the head code and accepted substantially as filed.
github-actions
Bot
added
ai-reviewed/2
…ping Documents the interim limitation of set_multitransport_offer: this acceptor only sends the request, it does not establish the sideband UDP transport itself. Marks AcceptorState non_exhaustive, matching ClientConnectorState's convention. Changes multitransport_soft_sync_negotiated to return Option<bool>, None before a request was actually sent, rather than deriving from GCC flags alone which could report true with nothing sent. Replaces the client_offered_multitransport bool with a single multitransport_flags: Option<MultiTransportFlags> field, removing the duplicated absent-vs-empty distinction. Merges log_multitransport_response into late_multitransport_response, removing the panic-prone two-step coupling, and folds the CapabilitiesWaitConfirm pre-check into the main match arm. Adds a multitransport_acceptor(offer) test factory, removing repeated setup across four tests.
Greg Lamberson (glamberson)
deployed
to
llm-providers
September 11, 2026 23:01 — with
GitHub Actions
Active
GitHub Actions
Active
github-actions
Bot
added
maintainer-required
Greg Lamberson (glamberson)
added a commit
to lamco-admin/IronRDP
that referenced
this pull request
Sep 11, 2026
Four of the eight findings were already resolved by the rebase onto Devolutions#1951's own review-response commit: the late-response tolerance now applies uniformly to every FinalizationSequence sub-state, the server MultiTransportChannelData block is filtered on the client's own block presence, and two stale doc comments were already corrected. For the remaining four, deduplicated the MCS SendDataRequest encoder and the client GCC-block builder between acceptor.rs and multitransport_finalize.rs (both made pub(super) and reused), extracted a shared play_confirm_active_and_finalization helper covering the Confirm Active plus four-PDU finalization exchange that play_client and play_reactivation_round both repeated, and extracted a recording_handler factory removing duplicated Arc::clone-into-closure plumbing across the two handler tests. play_client now also returns the request_id it decoded so the first handler test can assert the handler received the exact request the acceptor sent, rather than recording it unread. Added the missing assertion to multitransport_not_offered_by_default, whose doc comment claimed the server's GCC advertisement was checked when nothing actually was.
…ad of calling it inline The security cookie and request ID for the Initiate Multitransport Request were generated with a direct rand::rng() call inside MultitransportBootstrapping's step() arm, a hidden side channel to global RNG state in what is otherwise a sans-I/O, deterministic sequence. Nothing was actually broken by this; it is a testability and architecture concern, not a bug, so this is a refactor rather than a fix. Added a MultitransportSecurityRng trait (fill_security_cookie, next_request_id) stored as a boxed trait object on Acceptor, defaulting to an OS-backed implementation and overridable via set_multitransport_security_rng(), matching the existing set_multitransport_offer()/set_honor_client_desktop_size() builder idiom. step() now reads through the injected source. Carried through new_deactivation_reactivation() alongside the acceptor's other injected state. Added a test injecting a fixed source and asserting the exact bytes reach the encoded wire PDU.
Greg Lamberson (glamberson)
deployed
to
llm-providers
September 17, 2026 00:14 — with
GitHub Actions
Active
GitHub Actions
Active
github-actions
Bot
added
risk/medium
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MultitransportBootstrappingstate to the acceptor sequence, entered right after licensing.set_multitransport_offer()config (previously alwaysNone, so a server could never enable it), gated on the client having populated its own Client MultiTransportChannelData block (MS-RDPBCGR 2.2.1.4).multitransport_request()surfaces the sent request so the caller can establish the sideband UDP transport (RDPEUDP2 + TLS + RDPEMT) in parallel, without the acceptor waiting for the client's response first.Validation
cargo xtask check fmt/lints/tests/typos/locksall pass.6 tests in
ironrdp-testsuite-core/tests/server/acceptor.rsdriving the full handshake through a real MCS channel join sequence: offered-and-reciprocated (request sent on the message channel, response tolerated before Confirm Active), disabled by default, client-does-not-reciprocate, a late response tolerated during ConnectionFinalization (not just before Confirm Active), and non-response message-channel traffic (Auto-Detect Response, Heartbeat) not misclassified as a multitransport response.Notes
The acceptor does not wait for the client's Initiate Multitransport Response before continuing: MS-RDPBCGR 3.2.5.15.1 only obliges the client to send one when Soft-Sync is negotiated or the sideband attempt failed, so blocking on it would stall the handshake on the plain successful path. A response can legitimately arrive either before the mandatory Confirm Active or after it, during ConnectionFinalization: both
CapabilitiesWaitConfirmandConnectionFinalizationrecognize it (by channel and a successful strict decode of the payload, so other message-channel traffic like Auto-Detect Response or Heartbeat correctly falls through instead) and drop it rather than erroring or desyncing the finalization sequence.Only reliable UDP (
TRANSPORT_TYPE_UDP_FECR) is requested; lossy UDP is accepted in the offered flags for advertisement but never requested. Server-side wiring (accept_finalize_with_multitransportandironrdp-serverintegration) is a follow-up.One PR is stacked on this
#1953 (
accept_finalize_with_multitransport, an async driver consuming this PR's API) is stacked on this branch. Its diff is filed againstmasterand is cumulative with this one; see its own body for the incremental compare.