From 7b1c96585605e61ae1dcd52dbd332f1c0e2f57b2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 09:16:24 +0000 Subject: [PATCH 1/8] Initial plan From cac61e567769bbcf821444400f1629fed233c185 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 09:22:26 +0000 Subject: [PATCH 2/8] Change pub visibility to pub(crate) for CredSSP functions Co-authored-by: CBenoit <3809077+CBenoit@users.noreply.github.com> --- devolutions-gateway/src/rdp_proxy.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/devolutions-gateway/src/rdp_proxy.rs b/devolutions-gateway/src/rdp_proxy.rs index 388bbd7f5..657d19486 100644 --- a/devolutions-gateway/src/rdp_proxy.rs +++ b/devolutions-gateway/src/rdp_proxy.rs @@ -327,7 +327,7 @@ where } #[instrument(name = "server_credssp", level = "debug", ret, skip_all)] -pub async fn perform_credssp_with_server( +pub(crate) async fn perform_credssp_with_server( framed: &mut ironrdp_tokio::Framed, server_name: String, server_public_key: Vec, @@ -392,7 +392,7 @@ where } #[instrument(name = "client_credssp", level = "debug", ret, skip_all)] -pub async fn perform_credssp_with_client( +pub(crate) async fn perform_credssp_with_client( framed: &mut ironrdp_tokio::Framed, client_addr: IpAddr, gateway_public_key: Vec, @@ -483,7 +483,7 @@ where } } -pub async fn get_cached_gateway_public_key( +pub(crate) async fn get_cached_gateway_public_key( hostname: String, acceptor: tokio_rustls::TlsAcceptor, ) -> anyhow::Result> { @@ -533,7 +533,7 @@ async fn retrieve_gateway_public_key(hostname: String, acceptor: tokio_rustls::T Ok(public_key) } -pub fn extract_tls_server_public_key(tls_stream: &impl GetPeerCert) -> anyhow::Result> { +pub(crate) fn extract_tls_server_public_key(tls_stream: &impl GetPeerCert) -> anyhow::Result> { use x509_cert::der::Decode as _; let cert = tls_stream.get_peer_certificate().context("certificate is missing")?; @@ -551,7 +551,7 @@ pub fn extract_tls_server_public_key(tls_stream: &impl GetPeerCert) -> anyhow::R Ok(server_public_key) } -pub trait GetPeerCert { +pub(crate) trait GetPeerCert { fn get_peer_certificate(&self) -> Option<&tokio_rustls::rustls::pki_types::CertificateDer<'static>>; } From 55d8b5471db700d54bc4ca863a3cb90c262e6c0a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 09:28:20 +0000 Subject: [PATCH 3/8] Remove unnecessary 'static bound from handle function Co-authored-by: CBenoit <3809077+CBenoit@users.noreply.github.com> --- devolutions-gateway/src/rd_clean_path.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index e6615497f..8df2649c0 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -279,7 +279,7 @@ async fn process_cleanpath( /// Handle RDP connection with credential injection via CredSSP MITM #[allow(clippy::too_many_arguments)] async fn handle_with_credential_injection( - mut client_stream: impl AsyncRead + AsyncWrite + Unpin + Send + Sync + 'static, + mut client_stream: impl AsyncRead + AsyncWrite + Unpin + Send + Sync, client_addr: SocketAddr, conf: Arc, token_cache: &TokenCache, @@ -455,7 +455,7 @@ async fn handle_with_credential_injection( #[allow(clippy::too_many_arguments)] #[instrument("fwd", skip_all, fields(session_id = field::Empty, target = field::Empty))] pub async fn handle( - mut client_stream: impl AsyncRead + AsyncWrite + Unpin + Send + Sync + 'static, + mut client_stream: impl AsyncRead + AsyncWrite + Unpin + Send + Sync, client_addr: SocketAddr, conf: Arc, token_cache: &TokenCache, From 95c9e4a32d8f9d6f6fd123a4fdd3906d9a9d7915 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 09:30:32 +0000 Subject: [PATCH 4/8] Pass token to avoid double extraction in handle function Co-authored-by: CBenoit <3809077+CBenoit@users.noreply.github.com> --- devolutions-gateway/src/rd_clean_path.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index 8df2649c0..2ff398399 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -288,12 +288,11 @@ async fn handle_with_credential_injection( subscriber_tx: SubscriberSender, active_recordings: &ActiveRecordings, cleanpath_pdu: RDCleanPathPdu, + token: String, _credential_entry: crate::credential::ArcCredentialEntry, ) -> anyhow::Result<()> { - let token = cleanpath_pdu.proxy_auth.as_ref().context("missing token")?; - // Authorize the token - let claims = authorize(client_addr, token, &conf, token_cache, jrl, active_recordings, None) + let claims = authorize(client_addr, &token, &conf, token_cache, jrl, active_recordings, None) .map_err(|e| anyhow::anyhow!("authorization failed: {}", e))?; let crate::token::ConnectionMode::Fwd { targets: _ } = claims.jet_cm else { @@ -476,10 +475,10 @@ pub async fn handle( // Early credential detection: check if we should use RdpProxy instead let token = cleanpath_pdu .proxy_auth - .as_deref() + .clone() .ok_or_else(|| anyhow::anyhow!("missing token in RDCleanPath PDU"))?; - if let Some(entry) = crate::token::extract_jti(token) + if let Some(entry) = crate::token::extract_jti(&token) .ok() .and_then(|token_id| credential_store.get(token_id)) .filter(|entry| entry.mapping.is_some()) @@ -497,6 +496,7 @@ pub async fn handle( subscriber_tx, active_recordings, cleanpath_pdu, + token, entry, ) .await; From 534ef2e379512c27f6761a83671702c1a6ddbd31 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 09:32:18 +0000 Subject: [PATCH 5/8] Remove duplicate authorization code in handle_with_credential_injection Co-authored-by: CBenoit <3809077+CBenoit@users.noreply.github.com> --- devolutions-gateway/src/rd_clean_path.rs | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index 2ff398399..5b7ec5ce0 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -288,29 +288,17 @@ async fn handle_with_credential_injection( subscriber_tx: SubscriberSender, active_recordings: &ActiveRecordings, cleanpath_pdu: RDCleanPathPdu, - token: String, _credential_entry: crate::credential::ArcCredentialEntry, ) -> anyhow::Result<()> { - // Authorize the token - let claims = authorize(client_addr, &token, &conf, token_cache, jrl, active_recordings, None) - .map_err(|e| anyhow::anyhow!("authorization failed: {}", e))?; - - let crate::token::ConnectionMode::Fwd { targets: _ } = claims.jet_cm else { - anyhow::bail!("unexpected connection mode"); - }; - - let span = tracing::Span::current(); - span.record("session_id", claims.jet_aid.to_string()); - info!("Credential injection: performing CredSSP MITM"); - // Run normal RDCleanPath flow (this will handle server-side TLS and get certs) + // Run normal RDCleanPath flow (this will handle authorization, server-side TLS and get certs) let CleanPathResult { + claims, destination, server_addr, server_stream, x224_rsp, - .. } = process_cleanpath( cleanpath_pdu, client_addr, @@ -496,7 +484,6 @@ pub async fn handle( subscriber_tx, active_recordings, cleanpath_pdu, - token, entry, ) .await; From f333d7fa8cfaf0d28e50a8f0284dfafa877b7402 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 09:37:30 +0000 Subject: [PATCH 6/8] Refactor CredSSP MITM logic into rdp_proxy module Co-authored-by: CBenoit <3809077+CBenoit@users.noreply.github.com> --- devolutions-gateway/src/rd_clean_path.rs | 48 +++-------------- devolutions-gateway/src/rdp_proxy.rs | 66 ++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 40 deletions(-) diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index 5b7ec5ce0..0a34c7246 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -357,54 +357,22 @@ async fn handle_with_credential_injection( let server_public_key = crate::rdp_proxy::extract_tls_server_public_key(&server_stream).context("extract server TLS public key")?; - // Wrap streams in TokioFramed for CredSSP - let mut client_framed = ironrdp_tokio::TokioFramed::new(client_stream); - let mut server_framed = ironrdp_tokio::TokioFramed::new(server_stream); - // Use HYBRID_EX for client (web clients typically use this) let client_security_protocol = nego::SecurityProtocol::HYBRID_EX; - // Perform CredSSP MITM (in parallel) + // Perform CredSSP MITM // Note: Client expects server's public key (since we sent server certs in RDCleanPath response) - let client_credssp_fut = crate::rdp_proxy::perform_credssp_with_client( - &mut client_framed, + let (client_stream, server_stream) = crate::rdp_proxy::perform_credssp_mitm( + client_stream, + server_stream, client_addr.ip(), - server_public_key.clone(), - client_security_protocol, - &credential_mapping.proxy, - ); - - let server_credssp_fut = crate::rdp_proxy::perform_credssp_with_server( - &mut server_framed, destination.host().to_owned(), server_public_key, + client_security_protocol, server_security_protocol, - &credential_mapping.target, - ); - - let (client_res, server_res) = tokio::join!(client_credssp_fut, server_credssp_fut); - client_res.context("CredSSP with client failed")?; - server_res.context("CredSSP with server failed")?; - - info!("CredSSP MITM completed successfully"); - - // Extract streams and any leftover bytes - let (mut client_stream, client_leftover) = client_framed.into_inner(); - let (mut server_stream, server_leftover) = server_framed.into_inner(); - - // Forward any leftover bytes - if !server_leftover.is_empty() { - client_stream - .write_all(&server_leftover) - .await - .context("write server leftover to client")?; - } - if !client_leftover.is_empty() { - server_stream - .write_all(&client_leftover) - .await - .context("write client leftover to server")?; - } + credential_mapping, + ) + .await?; info!("RDP-TLS forwarding (credential injection)"); diff --git a/devolutions-gateway/src/rdp_proxy.rs b/devolutions-gateway/src/rdp_proxy.rs index 657d19486..44f227d7e 100644 --- a/devolutions-gateway/src/rdp_proxy.rs +++ b/devolutions-gateway/src/rdp_proxy.rs @@ -551,6 +551,72 @@ pub(crate) fn extract_tls_server_public_key(tls_stream: &impl GetPeerCert) -> an Ok(server_public_key) } +/// Perform CredSSP MITM between client and server, returning the unwrapped streams +#[instrument(name = "credssp_mitm", level = "debug", skip_all)] +pub(crate) async fn perform_credssp_mitm( + client_stream: C, + server_stream: S, + client_addr: IpAddr, + server_name: String, + server_public_key: Vec, + client_security_protocol: nego::SecurityProtocol, + server_security_protocol: nego::SecurityProtocol, + credential_mapping: &AppCredentialMapping, +) -> anyhow::Result<(C, S)> +where + C: AsyncRead + AsyncWrite + Unpin + Send + Sync, + S: AsyncRead + AsyncWrite + Unpin + Send + Sync, +{ + use tokio::io::AsyncWriteExt as _; + + // Wrap streams in TokioFramed for CredSSP + let mut client_framed = ironrdp_tokio::TokioFramed::new(client_stream); + let mut server_framed = ironrdp_tokio::TokioFramed::new(server_stream); + + // Perform CredSSP MITM (in parallel) + let client_credssp_fut = perform_credssp_with_client( + &mut client_framed, + client_addr, + server_public_key.clone(), + client_security_protocol, + &credential_mapping.proxy, + ); + + let server_credssp_fut = perform_credssp_with_server( + &mut server_framed, + server_name, + server_public_key, + server_security_protocol, + &credential_mapping.target, + ); + + let (client_res, server_res) = tokio::join!(client_credssp_fut, server_credssp_fut); + client_res.context("CredSSP with client failed")?; + server_res.context("CredSSP with server failed")?; + + info!("CredSSP MITM completed successfully"); + + // Extract streams and any leftover bytes + let (mut client_stream, client_leftover) = client_framed.into_inner(); + let (mut server_stream, server_leftover) = server_framed.into_inner(); + + // Forward any leftover bytes + if !server_leftover.is_empty() { + client_stream + .write_all(&server_leftover) + .await + .context("write server leftover to client")?; + } + if !client_leftover.is_empty() { + server_stream + .write_all(&client_leftover) + .await + .context("write client leftover to server")?; + } + + Ok((client_stream, server_stream)) +} + pub(crate) trait GetPeerCert { fn get_peer_certificate(&self) -> Option<&tokio_rustls::rustls::pki_types::CertificateDer<'static>>; } From b749e43d1cf8d0ce021e9589c00d21dc6a2b5889 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 09:46:02 +0000 Subject: [PATCH 7/8] Add clippy allow for too many arguments in perform_credssp_mitm Co-authored-by: CBenoit <3809077+CBenoit@users.noreply.github.com> --- devolutions-gateway/src/rdp_proxy.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/devolutions-gateway/src/rdp_proxy.rs b/devolutions-gateway/src/rdp_proxy.rs index 44f227d7e..d5d9e9137 100644 --- a/devolutions-gateway/src/rdp_proxy.rs +++ b/devolutions-gateway/src/rdp_proxy.rs @@ -552,6 +552,7 @@ pub(crate) fn extract_tls_server_public_key(tls_stream: &impl GetPeerCert) -> an } /// Perform CredSSP MITM between client and server, returning the unwrapped streams +#[allow(clippy::too_many_arguments)] #[instrument(name = "credssp_mitm", level = "debug", skip_all)] pub(crate) async fn perform_credssp_mitm( client_stream: C, From 11c2f5508c7d6403bffcd776f258808d77aaa338 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 09:48:00 +0000 Subject: [PATCH 8/8] Address code review feedback: avoid token clone and add clarifying comment Co-authored-by: CBenoit <3809077+CBenoit@users.noreply.github.com> --- devolutions-gateway/src/rd_clean_path.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index 0a34c7246..12ea8649d 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -293,6 +293,7 @@ async fn handle_with_credential_injection( info!("Credential injection: performing CredSSP MITM"); // Run normal RDCleanPath flow (this will handle authorization, server-side TLS and get certs) + // Note: process_cleanpath handles authorization and returns the claims let CleanPathResult { claims, destination, @@ -431,10 +432,10 @@ pub async fn handle( // Early credential detection: check if we should use RdpProxy instead let token = cleanpath_pdu .proxy_auth - .clone() + .as_deref() .ok_or_else(|| anyhow::anyhow!("missing token in RDCleanPath PDU"))?; - if let Some(entry) = crate::token::extract_jti(&token) + if let Some(entry) = crate::token::extract_jti(token) .ok() .and_then(|token_id| credential_store.get(token_id)) .filter(|entry| entry.mapping.is_some())