feat(client): support HyperV VM Connect #796

irvingoujAtDevolution · 2025-05-28T19:07:40Z

The main changes primarily involves changing in sequence of the connector state. With vmconnect, our connection goes from
Preconnection Blob -> TLS -> CredSSP -> Cookie/InitRequest

where as oppose to original sequence, the cookie is sent before TLS

Copilot

Pull Request Overview

This PR adds support for Hyper-V VM Connect by introducing a Preconnection Blob exchange before TLS, updating connector state handling, and exposing a vmconnect option throughout FFI, connector, CLI, and web session layers.

Exposed a new vmconnect flag (String) in configs and FFI APIs
Introduced a PreconnectionBlob state to send a VM Connect Pcb before TLS, then reordered the sequence
Added --vmconnect CLI argument and propagated the flag through async/blocking connectors

Reviewed Changes

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

Show a summary per file

File	Description
ffi/src/credssp/mod.rs	Added `vmconnect: bool` to `sequence_init` signature
ffi/src/connector/state.rs	Updated match to handle `ConnectionInitiationSendRequest { .. }`
ffi/src/connector/config.rs	Introduced `vmconnect` field and `set_vm_connect` setter
crates/ironrdp-web/src/session.rs	Added `vmconnect: None` placeholder with TODO
crates/ironrdp-testsuite-extra/tests/tests.rs	Updated `default_client_config` to include `vmconnect: None`
crates/ironrdp-testsuite-core/tests/pcb.rs	Added Hyper-V GUID case in `encode_decode_test!`
crates/ironrdp-pdu/src/rdp/client_info.rs	Wrote reserved1/2 fields in `ExtendedClientOptionalInfo` encode
crates/ironrdp-core/src/cursor.rs	Added `bytes_written()` method
crates/ironrdp-connector/src/lib.rs	Added `vmconnect` to `Config` struct
crates/ironrdp-connector/src/credssp.rs	Stored `vmconnect` in `CredsspSequence`
crates/ironrdp-connector/src/connection.rs	Added `PreconnectionBlob` state and updated sequence logic
crates/ironrdp-client/src/rdp.rs	Enhanced debug logging and state matching patterns
crates/ironrdp-client/src/config.rs	Added `--vmconnect` arg and refactored credential prompts
crates/ironrdp-blocking/src/connector.rs	Passed `vmconnect` flag into blocking `connect_begin`
crates/ironrdp-async/src/connector.rs	Passed `vmconnect` flag into async `connect_begin`

Comments suppressed due to low confidence (2)

ffi/src/connector/config.rs:45

[nitpick] The field is named vmconnect, but the setter is set_vm_connect. Consider renaming either the field to vm_connect or the setter to set_vmconnect for consistent snake_case naming.

pub vmconnect: Option<String>,

crates/ironrdp-client/src/config.rs:305

Defaulting to a literal password ("password") on prompt failure may introduce security risks. Consider returning an error or requiring explicit user input instead of using an insecure fallback.

.unwrap_or_else(|_| "password".to_owned())

crates/ironrdp-web/src/session.rs

crates/ironrdp-connector/src/connection.rs

crates/ironrdp-pdu/src/rdp/client_info.rs

irvingoujAtDevolution · 2025-05-29T19:06:01Z

I don't know why but there's file is blocking fmt from working

error: expected item, found `..`
 --> \\?\C:\dev\IronRDP\crates\ironrdp-testsuite-core\tests\server\fast_path.rs:1:1
  |
1 | ../../../ironrdp-server/src/encoder/fast_path.rs
  | ^^ expected item
  |
  = note: for a full list of items that can appear in modules, see <https://doc.rust-lang.org/reference/items.html>

I'm not sure if it suppose to be like this?

CBenoit · 2025-06-02T10:08:43Z

I don't know why but there's file is blocking fmt from working

error: expected item, found `..`
 --> \\?\C:\dev\IronRDP\crates\ironrdp-testsuite-core\tests\server\fast_path.rs:1:1
  |
1 | ../../../ironrdp-server/src/encoder/fast_path.rs
  | ^^ expected item
  |
  = note: for a full list of items that can appear in modules, see <https://doc.rust-lang.org/reference/items.html>

I'm not sure if it suppose to be like this?

Looks like the symlink was not handled properly. I think you can actually enable an option in git for that, verify you enabled it:

git config --global core.symlinks true

You may need re-clone the repository, though there may be alternate ways of fixing this.

CBenoit · 2025-06-02T10:09:38Z

crates/ironrdp-connector/src/credssp.rs

@@ -70,6 +70,7 @@ pub struct CredsspSequence {
    client: CredSspClient,
    state: CredsspState,
    selected_protocol: nego::SecurityProtocol,
+    vmconnect: bool,


question: What is the purpose of this field? I don’t see where it is used

CBenoit · 2025-06-02T10:13:43Z

crates/ironrdp-core/src/cursor.rs

+    /// Returns the number of bytes written.
+    #[inline]
+    pub const fn bytes_written(&self) -> usize {
+        self.pos
+    }


issue: Remove this method. You can use pos() instead which is the same thing. Also, strictly speaking, it’s not necessarily the number of bytes written because it’s possible to "rewind" the cursor.

CBenoit · 2025-06-02T10:16:17Z

crates/ironrdp-pdu/src/rdp/client_info.rs

+        dst.write_u16(0); // reserved1
+        dst.write_u16(0); // reserved2


question: Did you fix a bug? Could you elaborate on that? Please, open a separate PR dedicated to this with a fix commit type.

CBenoit · 2025-06-02T10:18:18Z

crates/ironrdp-testsuite-core/tests/pcb.rs

+    v2_hyperv_guid :
+        PreconnectionBlob {
+            version: PcbVersion::V2,
+            id: 0,
+            v2_payload: Some(String::from("ff995b48-a34f-404a-938f-303e4bb5bf31")),
+        },
+         hex::decode(concat!(
+            "5c0000000000000002000000000000002500660066003900390035006200340038002d0061003300340066002d0034003000340061002d0039003300380066002d003300300033006500340062006200350062006600330031000000"
+        )).expect("v2_hyperv_guid payload");
 }


issue: I think this test case does nothing more than the one just above, so let’s not add it

CBenoit · 2025-06-02T10:21:30Z

crates/ironrdp-web/src/session.rs

+        // TODO: implement this
+        vmconnect: None,


issue: We already support a pcb extension. My understanding is that all you need to do is te place a UUID into it. No need to add extra parameters.

CBenoit · 2025-06-02T10:22:17Z

crates/ironrdp-connector/src/lib.rs

+
+    pub vmconnect: Option<String>,


issue: I don’t think we should expose a vmconnect option. Also, since the PCB is something outside of the RDP protocol itself, let’s not handle it in the connector at all.

CBenoit · 2025-06-02T10:27:09Z

crates/ironrdp-client/src/rdp.rs

@@ -148,7 +148,7 @@ async fn connect(

    let should_upgrade = ironrdp_tokio::connect_begin(&mut framed, &mut connector).await?;

-    debug!("TLS upgrade");
+    debug!(destination = ?config.destination,"TLS upgrade");


nitpick: Add a debug! log before TcpStream::connect instead

CBenoit · 2025-06-02T10:28:01Z

crates/ironrdp-client/src/config.rs

+
+    /// The Virtual Machine ID for Hyper-V, used as Pre Connection Blob
+    #[clap(long)]
+    vmconnect: Option<String>,


suggestion: I think you could use a Uuid type here

CBenoit

General review: do not add knowledge of the PCB into ironrdp-connector, since it’s outside of the RDP connection sequence itself. Instead, I think it’s fine to have this code in ironrdp-client directly.

Also, it would be worth adding a --pcb parameter that would cover much more use cases. It does not seems necessary to have a "vmconnect" parameter that is a special case of the pcb string, but if you really want to add a parameter, configure clap so that these options are "conflicting".

irvingoujAtDevolution · 2025-06-02T15:12:04Z

You might be right, let me draft this PR, I need to make some experiment on another branch

irvingoujAtDevolution · 2025-07-04T18:57:57Z

Closed in favor of #841

irvingoujAtDevolution added 6 commits May 28, 2025 15:04

WIP add PDU support

f79954c

cargo conflict

41ad74d

WIP, not working

64f4f3c

clean up

9ebe578

clean up

84745e5

clean up

4989e42

irvingoujAtDevolution changed the title ~~Hypervm connect~~ feat(client): support HyperV VM Connect May 29, 2025

clean up

43f09e6

irvingoujAtDevolution requested review from CBenoit, Copilot and devolutionsbot May 29, 2025 18:02

fmt

ba4537c

irvingoujAtDevolution marked this pull request as ready for review May 29, 2025 18:02

Copilot AI reviewed May 29, 2025

View reviewed changes

crates/ironrdp-web/src/session.rs Show resolved Hide resolved

crates/ironrdp-connector/src/connection.rs Show resolved Hide resolved

crates/ironrdp-pdu/src/rdp/client_info.rs Show resolved Hide resolved

WIP

36a6217

CBenoit reviewed Jun 2, 2025

View reviewed changes

WIP

4376c6c

irvingoujAtDevolution marked this pull request as draft June 2, 2025 15:12

irvingoujAtDevolution closed this Jul 4, 2025

		dst.write_u16(0); // reserved1
		dst.write_u16(0); // reserved2

feat(client): support HyperV VM Connect #796

feat(client): support HyperV VM Connect #796

Uh oh!

Conversation

irvingoujAtDevolution commented May 28, 2025 • 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!

Uh oh!

Uh oh!

irvingoujAtDevolution commented May 29, 2025

Uh oh!

CBenoit commented Jun 2, 2025

Uh oh!

CBenoit Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

CBenoit Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

CBenoit Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CBenoit Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

CBenoit Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

CBenoit Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

CBenoit Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

CBenoit Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

CBenoit left a comment

Choose a reason for hiding this comment

Uh oh!

irvingoujAtDevolution commented Jun 2, 2025

Uh oh!

irvingoujAtDevolution commented Jul 4, 2025

Uh oh!

Uh oh!

irvingoujAtDevolution commented May 28, 2025 •

edited

Loading

CBenoit Jun 2, 2025 •

edited

Loading