Skip to content

feat(dgw): support vmconnect in RDCleanPath #1372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

feat(dgw): support vmconnect in RDCleanPath #1372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c53b76
WIP
irvingoujAtDevolution Jun 2, 2025
b238b71
works
irvingoujAtDevolution Jun 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion devolutions-gateway/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ job-queue = { path = "../crates/job-queue" }
job-queue-libsql = { path = "../crates/job-queue-libsql" }
ironrdp-pdu = { version = "0.5", features = ["std"] }
ironrdp-core = { version = "0.1", features = ["std"] }
ironrdp-rdcleanpath = "0.1"
ironrdp-tokio = { version = "0.4", default-features = false }
ironrdp-connector = { version = "0.5" }
ironrdp-acceptor = { version = "0.5" }
ironrdp-rdcleanpath = { path = "C:\\dev\\IronRDP\\crates\\ironrdp-rdcleanpath" }
ceviche = "0.6.1"
picky-krb = "0.9"
network-scanner = { version = "0.0.0", path = "../crates/network-scanner" }
Expand Down
105 changes: 74 additions & 31 deletions devolutions-gateway/src/rd_clean_path.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ struct CleanPathResult {
destination: TargetAddr,
server_addr: SocketAddr,
server_stream: tokio_rustls::client::TlsStream<tokio::net::TcpStream>,
x224_rsp: Vec<u8>,
x224_rsp: Option<Vec<u8>>,
}

async fn process_cleanpath(
Expand Down Expand Up @@ -212,45 +212,88 @@ async fn process_cleanpath(
debug!(%selected_target, "Connected to destination server");
span.record("target", selected_target.to_string());

// Send preconnection blob if applicable
if let Some(pcb) = cleanpath_pdu.preconnection_blob {
server_stream.write_all(pcb.as_bytes()).await?;
}

// Send X224 connection request
let x224_req = cleanpath_pdu
.x224_connection_pdu
.context("request is missing X224 connection PDU")
.map_err(CleanPathError::BadRequest)?;
server_stream.write_all(x224_req.as_bytes()).await?;

// Receive server X224 connection response

trace!("Receiving X224 response");
// Preconnection Blob (PCB) is currently only used for Hyper-V VMs.
//
// Connection sequence with Hyper-V VMs (PCB enabled):
// ┌─────────────────────┐ ┌─────────────────────────────────────────────────────────────┐
// │ handled by │ │ handled by IronRDP client │
// │ Gateway │ │ │
// └─────────────────────┘ └─────────────────────────────────────────────────────────────┘
// │PCB → TLS handshake │ → │CredSSP → X224 connection request → X224 connection response │
// └─────────────────────┘ └─────────────────────────────────────────────────────────────┘
//
// Connection sequence without Hyper-V VMs (PCB disabled):
// ┌─────────────────────────────────────────────────────────────┐ ┌──────────────────────┐
// │ handled by Gateway │ │ handled by IronRDP │
// │ │ │ client │
// └─────────────────────────────────────────────────────────────┘ └──────────────────────┘
// │X224 connection request → X224 connection response → TLS hs │ → │CredSSP → ... │
// └─────────────────────────────────────────────────────────────┘ └──────────────────────┘
//
// Summary:
// - With PCB: Gateway handles (1) sending PCB, (2) TLS handshake, then leaves CredSSP
// and X224 connection request/response to IronRDP client
// - Without PCB: Gateway handles (1) X224 connection request, (2) X224 connection response,
// then leaves TLS handshake and CredSSP to IronRDP client
Comment on lines +215 to +237
Copy link
Member

@CBenoit CBenoit Jun 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

praise: Good summary and explanation.

issue: However, this implementation assumes that the presence of a Preconnection Blob (PCB) implies the use of the Hyper-V VMConnect flow, where X.224 messages are exchanged after the CredSSP sequence. However, this assumption is overly restrictive.
While the PCB is indeed utilized in Hyper-V scenarios, particularly with VMConnect, which employs a unique connection sequence involving CredSSP prior to X.224 negotiation, it's important to recognize that the PCB is a general-purpose mechanism that does not necessarily imply an altered connection sequence. RDP clients, such as MSTSC and FreeRDP, support specifying a PCB without altering the conventional connection sequence, where X.224 negotiation precedes CredSSP authentication.

suggestion: The implementation should not default to the Hyper-V VMConnect flow solely based on the presence of a PCB. For instance, you could verify whether cleanpath_pdu.x224_connection_pdu is Some or None. If it’s None, it seems fair to assume the Hyper-V VMConnect flow.

let (server_stream, x224_rsp) = if let Some(pcb_string) = cleanpath_pdu.preconnection_blob {
debug!("Sending preconnection blob to server");
let pcb = ironrdp_pdu::pcb::PreconnectionBlob {
version: ironrdp_pdu::pcb::PcbVersion::V2,
id: 0,
v2_payload: Some(pcb_string),
};

let encoded = ironrdp_core::encode_vec(&pcb)
.context("failed to encode preconnection blob")
.map_err(CleanPathError::BadRequest)?;

server_stream.write_all(&encoded).await?;

let server_stream = crate::tls::connect(selected_target.host().to_owned(), server_stream)
.await
.map_err(|source| CleanPathError::TlsHandshake {
source,
target_server: selected_target.to_owned(),
})?;

let x224_rsp = read_x224_response(&mut server_stream)
.await
.with_context(|| format!("read X224 response from {selected_target}"))
.map_err(CleanPathError::BadRequest)?;
(server_stream, None)
} else {
// Send X224 connection request
let x224_req = cleanpath_pdu
.x224_connection_pdu
.context("request is missing X224 connection PDU")
.map_err(CleanPathError::BadRequest)?;

server_stream.write_all(x224_req.as_bytes()).await?;

let x224_rsp = read_x224_response(&mut server_stream)
.await
.with_context(|| format!("read X224 response from {selected_target}"))
.map_err(CleanPathError::BadRequest)?;
trace!("Receiving X224 response");

let server_stream = crate::tls::connect(selected_target.host().to_owned(), server_stream)
.await
.map_err(|source| CleanPathError::TlsHandshake {
source,
target_server: selected_target.to_owned(),
})?;
debug!("X224 connection request sent");

trace!("Establishing TLS connection with server");
// Receive server X224 connection response

// Establish TLS connection with server
trace!("Establishing TLS connection with server");

let server_stream = crate::tls::connect(selected_target.host().to_owned(), server_stream)
.await
.map_err(|source| CleanPathError::TlsHandshake {
source,
target_server: selected_target.to_owned(),
})?;
(server_stream, Some(x224_rsp))
};

Ok(CleanPathResult {
return Ok(CleanPathResult {
destination: selected_target.to_owned(),
claims,
server_addr,
server_stream,
x224_rsp,
})
});
}

#[allow(clippy::too_many_arguments)]
Expand All @@ -273,7 +316,7 @@ pub async fn handle(
.await
.context("couldn’t read clean cleanpath PDU")?;

trace!("Processing RDCleanPath");
trace!(RDCleanPath = ?cleanpath_pdu,"Processing RDCleanPath");

let CleanPathResult {
claims,
Expand Down
4 changes: 4 additions & 0 deletions devolutions-gateway/src/rdp_pcb.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ fn decode_pcb(buf: &[u8]) -> Result<Option<(PreconnectionBlob, usize)>, io::Erro
Ok(pcb) => {
let pdu_size = ironrdp_core::size(&pcb);
let read_len = cursor.pos();
info!(
pdu_size,
read_len, "read preconnection blob (size: {}, read: {})", pdu_size, read_len
);

// NOTE: sanity check (reporting the wrong number will corrupt the communication)
if read_len != pdu_size {
Expand Down
Loading