feat(rdcleanpath): support Hyper-V connection ordering by irvingoujAtDevolution · Pull Request #1505 · Devolutions/IronRDP

irvingouj@Devolutions (irvingoujAtDevolution) · 2026-07-31T16:23:11Z

Hyper-V over RDCleanPath needs PCB → TLS on the proxy, then CredSSP → X.224 on the client. Ordinary RDCleanPath (proxy does X.224 + TLS) stays as-is.

Still VERSION_1. Reuse preconnection_blob. X.224 optional in the typed model: present = ordinary front, absent = PCB front. Native client, web, and .NET/FFI all take that path; .NET matches connect_front (HYBRID-only X.224 + selected-protocol check).

Gateway reference implementation: Devolutions/devolutions-gateway#1372

Tested locally against nested Hyper-V through Gateway. Native, .NET Avalonia, and Web all connect and render.

Add RDCleanPath version 2 as a strict extension of version 1. It carries an opaque server preconnection PDU and lets the proxy establish TLS before the client runs CredSSP and X.224, matching Hyper-V VMConnect ordering across the native client, web client, and .NET bindings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Preserve the in-progress RDCleanPath v2 integration against the extracted ironrdp-vmconnect sequencing before rebasing the stack onto the updated core branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Preserve the v1 Rust interface, bind responses to their request version, keep web PCB payloads opaque, and align the FFI/.NET sequence with pre-X.224 CredSSP. Add fixed v2 wire tests and remove secret-bearing request logs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Use the existing preconnection_blob field for VMConnect instead of introducing a second binary field and protocol version. The presence of X.224 now selects the ordinary front; its absence selects PCB followed by TLS. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Collapse PcbFront* into optional X.224 on Request/Response, drop handshake structs and extra vmconnect tests, and keep only the encode/proxy helpers plus client/web/FFI call paths.

Enforce HYBRID-only X.224 and selected-protocol checks on the .NET path, clear host credentials after CredSSP, and lowercase RDCleanPath/vmconnect error messages.

Avoid a second clone when encoding opaque preconnection blob payloads into RDCleanPath.

Allow PCB-first RDCleanPath requests to omit X.224 so Gateway sends the complete preconnection blob and establishes TLS before handing CredSSP and X.224 back to the client. Keep the ordinary and credential-injection fronts on their existing X.224-first sequence. Depends-on: Devolutions/IronRDP#1505 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Adds Hyper-V PCB-first connection ordering over RDCleanPath across native, web, and .NET clients.

Changes:

Makes X.224 optional in PCB-front RDCleanPath requests and responses.
Reuses VMConnect’s CredSSP → X.224 flow after proxy TLS.
Extends FFI/.NET APIs and serialization tests.

Reviewed changes

Copilot reviewed 14 out of 21 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`Cargo.lock`	Records new VMConnect dependencies.
`crates/ironrdp-async/src/connector.rs`	Relaxes the stream `Sync` bound.
`crates/ironrdp-client/src/config.rs`	Allows VMConnect with RDCleanPath.
`crates/ironrdp-client/src/rdp.rs`	Implements native PCB-front handshake ordering.
`crates/ironrdp-rdcleanpath/src/lib.rs`	Models optional X.224 payloads.
`crates/ironrdp-testsuite-core/tests/rdcleanpath.rs`	Adds PCB-front DER fixtures.
`crates/ironrdp-vmconnect/src/lib.rs`	Adds proxy PCB receipts and encoding helpers.
`crates/ironrdp-web/Cargo.toml`	Adds VMConnect dependency.
`crates/ironrdp-web/src/session.rs`	Implements web PCB-front ordering.
`ffi/Cargo.toml`	Adds VMConnect dependency.
`ffi/src/connector/mod.rs`	Exposes VMConnect connector operations.
`ffi/src/credssp/mod.rs`	Supports explicit CredSSP protocols.
`ffi/src/rdcleanpath.rs`	Exposes PCB-front PDU APIs.
`ffi/dotnet/Devolutions.IronRdp/src/ConnectionHelpers.cs`	Supports explicit CredSSP protocols.
`ffi/dotnet/Devolutions.IronRdp/src/RDCleanPathConnection.cs`	Implements .NET PCB-front ordering.
`ffi/dotnet/Devolutions.IronRdp/Generated/ClientConnector.cs`	Adds generated connector bindings.
`ffi/dotnet/Devolutions.IronRdp/Generated/CredsspSequence.cs`	Adds generated CredSSP binding.
`ffi/dotnet/Devolutions.IronRdp/Generated/RDCleanPathPdu.cs`	Adds generated PCB-front bindings.
`ffi/dotnet/Devolutions.IronRdp/Generated/RawClientConnector.cs`	Adds native connector declarations.
`ffi/dotnet/Devolutions.IronRdp/Generated/RawCredsspSequence.cs`	Adds native CredSSP declaration.
`ffi/dotnet/Devolutions.IronRdp/Generated/RawRDCleanPathPdu.cs`	Adds native RDCleanPath declarations.

Suppressed comments (1)

ffi/src/rdcleanpath.rs:98

When both accepted alternatives are absent, reporting only x224_connection_pdu is misleading now that PCB-front requests are valid. Match the typed-model diagnostic so callers know either field can satisfy the request.


                if self.0.x224_connection_pdu.is_none() && self.0.preconnection_blob.is_none() {

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

Marc-André Moreau (mamoreau-devolutions) · 2026-08-08T12:31:13Z

+        /// True when the PDU has no X.224 payload (PCB-front request or response).
+        pub fn has_x224(&self) -> bool {
+            self.0.x224_connection_pdu.is_some()
+        }


Still open after 9cfa482.

Agreed — the doc comment (and the generated .NET binding text) currently says the opposite of the implementation. has_x224 returns true when x224_connection_pdu is present.

Proposed follow-up (docs-only, no API change):

Fix the rustdoc on both FFI getters so it matches the implementation

Regenerate .NET bindings so the summary text stays in sync

Optionally rename to something like has_x224_connection_pdu only if we want the name to carry the meaning; not required if docs are corrected

Leaving unresolved until that lands.

Marc-André Moreau (mamoreau-devolutions) · 2026-08-08T12:31:15Z

        string authToken,
        string destination,
-        string? pcb = null,
+        string? pcbPayload = null,


Still open after 9cfa482.

Agreed this is a source-breaking rename for named-argument callers (pcb: value), even though the CLR signature is unchanged.

Options for a follow-up:

Preferred if no external consumers rely on the new name yet: restore the parameter name to pcb and document the PCB-front semantics in the XML doc / param description.

If pcbPayload is intentional, treat it as a deliberate breaking API change with a short migration note in the PR/changelog (named arg pcb: → pcbPayload:).

Need an explicit decision before merge; leaving unresolved.

Marc-André Moreau (mamoreau-devolutions) · 2026-08-08T12:31:18Z

+/// Receipt after an RDCleanPath proxy has written the PCB and established TLS.
+pub fn pcb_sent_via_proxy() -> PcbSent {
+    PcbSent


Still open after 9cfa482.

Agreed — prepare_connector, the RDCleanPath/proxy PCB-front flow, and the encoding/receipt helpers are public surface now, but the crate README (included as crate docs) still describes only the direct send_preconnection_blob path.

Proposed follow-up (docs-only):

Extend the README API table / flow section for RDCleanPath PCB-front ordering

Document prepare_connector as the shared TLS+CredSSP prerequisite choke point (also used from FFI init_with_protocol)

Mention encode_preconnection_blob_string / related helpers used by proxy clients

Leaving unresolved until docs catch up.

Bundle RDCleanPath handshake args, gate client PCB on vmconnect, reject request/response front-mode mismatches, and enforce TLS+CredSSP via prepare_connector for FFI pre-X.224 CredSSP init.

github-actions

The PCB-front sequencing itself (proxy writes PCB + TLS, client runs CredSSP then a HYBRID-only X.224) is coherent and matches ironrdp-vmconnect across all three embedders. Two issues should block. First, the new preconnection-blob string helpers push binary PDU bytes through String::from_utf8, which cannot represent any payload of 54+ characters or containing non-ASCII — the Hyper-V default sits three characters under that cliff, and web/FFI callers pass unconstrained text. Second, the web and .NET clients now treat any preconnection blob as a request for the Direct Approach and stop forwarding it on the ordinary path, silently changing the wire sequence for existing non-Hyper-V PCB users with no opt-out. Also blocking: ffi has_x224's doc comment states the inverse of its behaviour and ships that way to .NET. One open question on the RDCleanPath proxy contract, plus notes on duplication, test coverage, and bundled unrelated edits.

Protocol analysis: partially_accepted — Verified both UTF-8 conflicts against ironrdp-pdu pcb.rs/utf16.rs: cbSize = 20 + 2*utf16_units, little-endian first, so the 54-unit and non-ASCII boundaries are exact. Confirmed and sharpened the web mode-coupling conflict: the ordinary branch now passes None, so the prior PCB behaviour is unreachable and .NET regresses identically; the V1-vs-V2 item folds into it. Kept the double-wrapping item, reclassified as a question since it needs the Gateway contract. Accepted every 'conforms' assessment. Rejected two discrepancies as out of scope: the missing PROTOCOL_SSL flag is pre-existing and unchanged, and the PCB deadline is inherently the proxy's once it owns the socket. Missed: inverted has_x224 doc, empty-string PCB divergence, duplicated ensure_selected_credssp.

blocking / high — crates/ironrdp-web/src/session.rs
request_pcb_front = pcb.is_some() makes any preconnection blob select the MS-RDPBCGR Direct Approach (RDCleanPath request with no X.224, then CredSSP before X.224), and the ordinary branch now passes None instead of pcb, so the previous behaviour is unreachable. The pcb extension is a generic 'Pre Connection Blob' (web-client/iron-remote-desktop-rdp/src/main.ts, free-text field in login.svelte); MS-RDPEPS describes the preconnection PDU as a general RDP-source selection mechanism, so a PCB implies nothing about which security-enhanced connection sequence applies. An existing web consumer routing a non-Hyper-V PCB target silently moves from negotiation-based to Direct Approach, the payload is now re-wrapped as a V2 PDU (V1 becomes unreachable), and pcb + enable_credssp=false now hard-fails in prepare_connector. ffi/dotnet/.../RDCleanPathConnection.cs has the same regression on its public pcbPayload parameter. Gate PCB-front on an explicit vmconnect/Hyper-V opt-in, as the Rust client does via vm_id.

github-actions · 2026-08-09T17:23:23Z

+pub fn encode_preconnection_blob_payload_string(payload: String) -> ConnectorResult<String> {
+    String::from_utf8(encode_preconnection_blob_payload(payload)?)
+        .map_err(|e| custom_err!("encode preconnection blob as RDCleanPath string", e))
+}


blocking / high: encode_preconnection_blob_string / _payload_string tunnel a binary RDP_PRECONNECTION_PDU_V2 through String::from_utf8. Per crates/ironrdp-pdu/src/pcb.rs, cbSize = 20 + 2*utf16_units and is written little-endian first, so any payload of >= 54 BMP units makes byte 0 land in 0x80..0xFE, followed by 0x00 — never valid UTF-8 — and the call returns an encode error. Any non-ASCII payload character fails too ('e' with acute = U+00E9 -> bytes E9 00). MS-RDPEPS defines wszPCB as an opaque Unicode string, so long and non-ASCII payloads are legal. The Hyper-V default (36-char GUID + ";EnhancedMode=1" = 51 units, cbSize 0x7A) sits three characters under the cliff; adding one more key=value pair breaks it, and the web/FFI entry points accept unconstrained user text. Carrying arbitrary bytes in an ASN.1 Utf8String is the root problem: use an OCTET STRING field, or keep sending the payload and let the proxy build the PDU.

github-actions · 2026-08-09T17:23:24Z

+            preconnection_blob: Some(preconnection_blob),
+            ..Self::default()
+        }
+    }


question / high: new_request_with_pcb changes what the preconnection_blob field carries: previously IronRDP put the raw wszPCB payload there (web pcb, .NET pcb), now every caller puts a fully serialized RDP_PRECONNECTION_PDU_V2 in it, under the same VERSION_1 (0x0D3E) with no discriminator and no doc update — the field doc at line 187 still just says 'The RDP PCB forwarded by the proxy to the RDP server'. If the proxy still wraps that field into a PDU, the server reads the outer 16-byte header as cchPCB/wszPCB and fails the MS-RDPEPS 3.2.5.1 cbSize check. RDCleanPath is a Devolutions construct with no public spec, so this cannot be settled from the repo. Please state which Gateway version implements the PCB-front contract (verbatim field bytes, response without x224_connection_pdu) and how a client talking to an older proxy is expected to fail; then document the two meanings on the field itself.

github-actions · 2026-08-09T17:23:24Z

+        /// True when the PDU has no X.224 payload (PCB-front request or response).
+        pub fn has_x224(&self) -> bool {
+            self.0.x224_connection_pdu.is_some()
+        }


blocking / medium: The doc comment reads 'True when the PDU has no X.224 payload (PCB-front request or response)' but the body returns x224_connection_pdu.is_some(), i.e. the exact opposite. Diplomat copies this into two shipped bindings (ffi/dotnet/.../Generated/RDCleanPathPdu.cs and RawRDCleanPathPdu.cs), so a .NET consumer following the documentation inverts its branch and takes the PCB-front path on an ordinary response — running CredSSP before an X.224 exchange that already happened. The in-tree caller happens to use the correct sense, which is why nothing fails today; the hazard is entirely on external FFI users. Fix the comment to 'True when the PDU carries an X.224 payload' and regenerate.

github-actions · 2026-08-09T17:23:24Z

+const RESPONSE_WITH_PCB_DER: &[u8] = &[
+    0x30, 0x2C, 0xA0, 0x4, 0x2, 0x2, 0xD, 0x3E, 0xA7, 0x14, 0x30, 0x12, 0x4, 0x4, 0xDE, 0xAD, 0xBE, 0xFF, 0x4, 0x4,
+    0xDE, 0xAD, 0xBE, 0xFF, 0x4, 0x4, 0xDE, 0xAD, 0xBE, 0xFF, 0xA9, 0xE, 0xC, 0xC, 0x31, 0x39, 0x32, 0x2E, 0x31, 0x36,
+    0x38, 0x2E, 0x37, 0x2E, 0x39, 0x35,


non_blocking / medium: The new fixtures use the literal three-character string "PCB", so they only exercise RDCleanPath DER round-tripping — the place the PR actually introduces risk (encode_preconnection_blob_string / _payload_string) has no test at all. Worth pinning: exact bytes for a GUID + ';EnhancedMode=1' payload (cbSize 0x7A, Version 2, Id 0, cchPCB 0x34, UTF-16LE with NUL terminator); payload lengths 53 and 54 and a non-ASCII payload to pin the UTF-8 boundary; a decode round-trip back through PreconnectionBlob. The new decode-side validation is also untested: a Request with neither x224_connection_pdu nor preconnection_blob must be rejected, a PCB-front Request must decode with x224_connection_request absent, and ensure_rdcleanpath_front_mode should be covered in all four combinations.

github-actions · 2026-08-09T17:23:24Z

+                )
+                .into()),
+            }
+        }


non_blocking / medium: ensure_selected_hybrid is a hand-copy of the private ensure_selected_credssp in crates/ironrdp-vmconnect/src/lib.rs:195, down to both message strings ('server must select HYBRID for a Hyper-V console, but it selected {}' and 'expected EnhancedSecurityUpgrade after Hyper-V X.224 initiation, got {}'). The PR already made prepare_connector pub for exactly this reason, so the same treatment applies here: export ensure_selected_credssp and call it. As written the two copies will drift, and the FFI copy silently loses any future tightening (for example HYBRID_EX handling) made in the crate that owns the invariant.

github-actions · 2026-08-09T17:23:24Z

        string pcb)
    {
        var writeBuf = WriteBuf.New();
+        var pcbFront = !string.IsNullOrEmpty(pcb);


non_blocking / low: The .NET path treats an empty PCB as 'not PCB-front' (!string.IsNullOrEmpty), while crates/ironrdp-web/src/session.rs:1691 uses pcb.is_some(). The web pcb extension stores whatever it is handed (session.rs:247), so preConnectionBlob('') puts the web client into PCB-front mode and sends a PCB whose payload is just a NUL terminator, whereas the identical .NET input keeps the ordinary path. The bundled Svelte UI guards with pcb !== '' but the published extension API does not. Pick one rule and apply it in both places.

github-actions · 2026-08-09T17:23:24Z

-use ironrdp_session::{ActiveStage, ActiveStageBuilder, ActiveStageOutput, GracefulDisconnectReason, SessionResult};
+#[cfg(feature = "clipboard")]
+use ironrdp_session::ActiveStage;
+use ironrdp_session::{ActiveStageBuilder, ActiveStageOutput, GracefulDisconnectReason, SessionResult};


non_blocking / low: Several edits here are unrelated to RDCleanPath PCB-front: the clipboard-feature build fixes (cfg-gated ActiveStage import, expect(dead_code) on clipboard_sender at line 130, `let _ = clipboard_event` in active_session) fix a non-default feature combination, and in crates/ironrdp-rdcleanpath/src/lib.rs the PR also deletes the commented-out ocsp_response field, rewords MissingRDCleanPathField's Display, and rewrites `version: VERSION_1` to `version: version.value` (a no-op under the existing match guard). crates/ironrdp-web/src/session.rs additionally strips explanatory comments in the NegotiationErr arm that still describe live logic. None of this is wrong, but it enlarges the review surface of a wire-affecting change; splitting it out would let the protocol change be judged on its own.

github-actions Bot added scope/web Affects the web/WASM ecosystem scope/core Touches the core architectural tier A-extra size/L Size: 400-799 lines of code labels Jul 31, 2026

Benoît Cortier (CBenoit) removed rust labels Aug 4, 2026

irvingouj@Devolutions (irvingoujAtDevolution) force-pushed the feat/rdcleanpath-v2 branch from 9b0dcff to 7ec3da1 Compare August 4, 2026 14:24

Benoît Cortier (CBenoit) removed the A-extra label Aug 4, 2026

irvingouj@Devolutions (irvingoujAtDevolution) force-pushed the feat/rdcleanpath-v2 branch from 7ec3da1 to dd55aee Compare August 4, 2026 21:07

Base automatically changed from feat/vmconnect-on-toolchain to master August 7, 2026 19:04

irvingouj@Devolutions (irvingoujAtDevolution) and others added 4 commits August 7, 2026 15:27

irvingouj@Devolutions (irvingoujAtDevolution) force-pushed the feat/rdcleanpath-v2 branch from e745d2a to de26fa3 Compare August 7, 2026 19:52

irvingouj@Devolutions (irvingoujAtDevolution) added 3 commits August 7, 2026 16:20

refactor(rdcleanpath): shrink Hyper-V PCB front

b850e8a

Collapse PcbFront* into optional X.224 on Request/Response, drop handshake structs and extra vmconnect tests, and keep only the encode/proxy helpers plus client/web/FFI call paths.

fix(rdcleanpath): align .NET PCB front with connect_front

d1b4b26

Enforce HYBRID-only X.224 and selected-protocol checks on the .NET path, clear host credentials after CredSSP, and lowercase RDCleanPath/vmconnect error messages.

perf(vmconnect): take owned PCB payload strings

6af4d08

Avoid a second clone when encoding opaque preconnection blob payloads into RDCleanPath.

irvingouj@Devolutions (irvingoujAtDevolution) mentioned this pull request Aug 7, 2026

feat(dgw): support VMConnect through RDCleanPath Devolutions/devolutions-gateway#1372

Draft

irvingouj@Devolutions (irvingoujAtDevolution) marked this pull request as ready for review August 7, 2026 20:53

Copilot AI balanced review requested due to automatic review settings August 7, 2026 20:53

chore: drop session DECISIONS.md from PR

eae13d5

Copilot started reviewing on behalf of irvingouj@Devolutions (irvingoujAtDevolution) August 7, 2026 20:54 View session

github-actions Bot added the maintainer-required Maintainer review or intervention is required label Aug 7, 2026

style: rustfmt

7461bb4

Copilot AI reviewed Aug 7, 2026

View reviewed changes

github-actions Bot added risk/unknown scope/ffi Affects native or .NET bindings size/XL Size: 800 or more lines of code and removed size/L Size: 400-799 lines of code labels Aug 7, 2026

fix(rdcleanpath): harden Hyper-V front handshake paths

9cfa482

Bundle RDCleanPath handshake args, gate client PCB on vmconnect, reject request/response front-mode mismatches, and enforce TLS+CredSSP via prepare_connector for FFI pre-X.224 CredSSP init.

github-actions Bot reviewed Aug 9, 2026

View reviewed changes

Uh oh!

Conversation

irvingouj@Devolutions (irvingoujAtDevolution) commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Marc-André Moreau (mamoreau-devolutions) Aug 8, 2026

Choose a reason for hiding this comment

Uh oh!

Marc-André Moreau (mamoreau-devolutions) Aug 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Marc-André Moreau (mamoreau-devolutions) Aug 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 9, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 9, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 9, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 9, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 9, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 9, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

irvingouj@Devolutions (irvingoujAtDevolution) commented Jul 31, 2026 •

edited

Loading