Skip to content

Refactor RDCleanPath credential injection to reduce duplication and improve encapsulation #1618

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
Copilot wants to merge 8 commits into
base: dev/rdcleanpath-credential-injection
Choose a base branch
from
Draft

Refactor RDCleanPath credential injection to reduce duplication and improve encapsulation #1618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7b1c965
Initial plan
Copilot Dec 9, 2025
cac61e5
Change pub visibility to pub(crate) for CredSSP functions
Copilot Dec 9, 2025
55d8b54
Remove unnecessary 'static bound from handle function
Copilot Dec 9, 2025
95c9e4a
Pass token to avoid double extraction in handle function
Copilot Dec 9, 2025
534ef2e
Remove duplicate authorization code in handle_with_credential_injection
Copilot Dec 9, 2025
f333d7f
Refactor CredSSP MITM logic into rdp_proxy module
Copilot Dec 9, 2025
b749e43
Add clippy allow for too many arguments in perform_credssp_mitm
Copilot Dec 9, 2025
11c2f55
Address code review feedback: avoid token clone and add clarifying co…
Copilot Dec 9, 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
70 changes: 13 additions & 57 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 @@ -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<Conf>,
token_cache: &TokenCache,
Expand All @@ -290,28 +290,16 @@ async fn handle_with_credential_injection(
cleanpath_pdu: RDCleanPathPdu,
_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)
.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)
// Note: process_cleanpath handles authorization and returns the claims
let CleanPathResult {
claims,
destination,
server_addr,
server_stream,
x224_rsp,
..
} = process_cleanpath(
cleanpath_pdu,
client_addr,
Expand Down Expand Up @@ -370,54 +358,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)");

Expand Down Expand Up @@ -455,7 +411,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<Conf>,
token_cache: &TokenCache,
Expand Down
77 changes: 72 additions & 5 deletions devolutions-gateway/src/rdp_proxy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ where
}

#[instrument(name = "server_credssp", level = "debug", ret, skip_all)]
pub async fn perform_credssp_with_server<S>(
pub(crate) async fn perform_credssp_with_server<S>(
framed: &mut ironrdp_tokio::Framed<S>,
server_name: String,
server_public_key: Vec<u8>,
Expand Down Expand Up @@ -392,7 +392,7 @@ where
}

#[instrument(name = "client_credssp", level = "debug", ret, skip_all)]
pub async fn perform_credssp_with_client<S>(
pub(crate) async fn perform_credssp_with_client<S>(
framed: &mut ironrdp_tokio::Framed<S>,
client_addr: IpAddr,
gateway_public_key: Vec<u8>,
Expand Down Expand Up @@ -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<Vec<u8>> {
Expand Down Expand Up @@ -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<Vec<u8>> {
pub(crate) fn extract_tls_server_public_key(tls_stream: &impl GetPeerCert) -> anyhow::Result<Vec<u8>> {
use x509_cert::der::Decode as _;

let cert = tls_stream.get_peer_certificate().context("certificate is missing")?;
Expand All @@ -551,7 +551,74 @@ pub fn extract_tls_server_public_key(tls_stream: &impl GetPeerCert) -> anyhow::R
Ok(server_public_key)
}

pub trait GetPeerCert {
/// 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<C, S>(
client_stream: C,
server_stream: S,
client_addr: IpAddr,
server_name: String,
server_public_key: Vec<u8>,
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>>;
}

Expand Down