From 2dc8a62300bc04c2ba323259ab85e12b6e7ae7c2 Mon Sep 17 00:00:00 2001 From: irving ou Date: Thu, 21 May 2026 17:18:20 -0400 Subject: [PATCH 1/2] feat(dgw): route KDC traffic through agent tunnel When an agent advertises the KDC's subnet or DNS domain, route Kerberos traffic through the QUIC tunnel just like every other proxy path. Closes the last gap left after the transparent routing PR (#1741). Two paths now use the same routing pipeline as connection forwarding: - `/jet/KdcProxy` HTTP endpoint -- the handler builds a `KdcConnector` and forwards through it. When an agent advertises the KDC subnet, the request goes through the agent tunnel; otherwise it falls back to a direct TCP/UDP connection. - RDP CredSSP/NLA -- `rdp_proxy.rs::send_network_request` previously hard-coded `None` for the agent handle. `RdpProxy` now carries a `KdcConnector` field that the CredSSP machinery (`perform_credssp_as_*` -> `resolve_*_generator` -> `send_network_request`) uses for every Kerberos sub-request. The same change reaches the credential-injection clean path (`rd_clean_path.rs`). `KdcConnector` (new `src/kdc_connector.rs`) bundles the three inputs the routing pipeline needs (`session_id`, `explicit_agent_id`, `agent_tunnel_handle`) into a single value and always defers the routing decision to `agent_tunnel::routing::try_route`. Callers never pre-decide "direct" vs "via tunnel": the routing pipeline does, and its existing `explicit_agent_id` enforcement (pin without tunnel handle must error, never silently fall back to direct) is preserved end-to-end. Session correlation: - RDP CredSSP callers pass the parent association's `claims.jet_aid` as `session_id`, so KDC sub-traffic ties back to its parent RDP session in agent-side logs. - The HTTP `/jet/KdcProxy` handler passes the KDC token's own `jti`, the most persistent identifier available without a parent association. `KdcTokenClaims` now exposes `jti` through its serde helper, matching how every other `*TokenClaims` type surfaces `jti`. Explicit-agent routing (matches every other proxy path): - The parent association's `jet_agent_id`, when set, is forwarded to `try_route`. KDC traffic must route via that agent or fail -- never silently fall back to a different agent or to direct connect. The HTTP handler passes `None` (no parent association). - A new UDP-via-agent guard rejects `udp://` KDC targets whenever the routing pipeline selects an agent. Without it, an explicit `jet_agent_id` pin could be downgraded to direct UDP, since the agent tunnel currently carries only TCP. Hardening (came along since they share the file): - 64 KiB `MAX_KDC_REPLY_MESSAGE_LEN` DoS cap on the announced TCP-framed KDC reply length, with overflow-safe length math. - UDP scheme guard at the direct-connect branch (preserved). Tests: - `kdc_connector` unit tests cover the two cases that don't need a live `AgentTunnelHandle`: pin-without-tunnel must error, no-pin-no-tunnel falls through to direct. The remaining cases (pin-with-missing-agent, no-match-falls-back, tunnel success, UDP-via-agent guard) need an integration-style listener fixture and are left as a follow-up. Issue: DGW-384 --- devolutions-gateway/src/api/kdc_proxy.rs | 132 ++-------- devolutions-gateway/src/api/webapp.rs | 1 + devolutions-gateway/src/generic_client.rs | 7 + devolutions-gateway/src/kdc_connector.rs | 291 ++++++++++++++++++++++ devolutions-gateway/src/lib.rs | 1 + devolutions-gateway/src/rd_clean_path.rs | 5 + devolutions-gateway/src/rdp_proxy.rs | 54 ++-- devolutions-gateway/src/token.rs | 15 +- devolutions-gateway/src/upstream.rs | 2 +- 9 files changed, 365 insertions(+), 143 deletions(-) create mode 100644 devolutions-gateway/src/kdc_connector.rs diff --git a/devolutions-gateway/src/api/kdc_proxy.rs b/devolutions-gateway/src/api/kdc_proxy.rs index df5a09c5c..865fc369f 100644 --- a/devolutions-gateway/src/api/kdc_proxy.rs +++ b/devolutions-gateway/src/api/kdc_proxy.rs @@ -1,12 +1,8 @@ -use std::io; - use axum::Router; use axum::extract::State; -use axum::http::StatusCode; use axum::routing::post; use picky_krb::messages::KdcProxyMessage; -use tokio::io::{AsyncReadExt, AsyncWriteExt}; -use tokio::net::{TcpStream, UdpSocket}; +use uuid::Uuid; use crate::DgwState; use crate::credential_injection_kdc::{ @@ -14,7 +10,8 @@ use crate::credential_injection_kdc::{ kdc_proxy_message_realm, }; use crate::extract::KdcToken; -use crate::http::{HttpError, HttpErrorBuilder}; +use crate::http::HttpError; +use crate::kdc_connector::KdcConnector; use crate::target_addr::TargetAddr; use crate::token::{KdcDestination, KdcTokenClaims}; @@ -26,9 +23,13 @@ async fn kdc_proxy( State(DgwState { conf_handle, credentials, + agent_tunnel_handle, .. }): State, - KdcToken(KdcTokenClaims { destination }): KdcToken, + KdcToken(KdcTokenClaims { + destination, + jti: token_jti, + }): KdcToken, body: axum::body::Bytes, ) -> Result, HttpError> { let conf = conf_handle.get_conf(); @@ -70,6 +71,13 @@ async fn kdc_proxy( } KdcDestination::Real { krb_realm, krb_kdc } => { let envelope_realm = kdc_proxy_message_realm(&kdc_proxy_message); + + // session_id: the HTTP /jet/KdcProxy endpoint has no parent association token, so we + // use the KDC token's own `jti` for log correlation (the RDP CredSSP/NLA caller + // passes `claims.jet_aid` so KDC sub-traffic correlates with its parent RDP session). + // explicit_agent_id: HTTP has no parent association, hence no `jet_agent_id` pin. + let kdc_connector = KdcConnector::new(token_jti, None, agent_tunnel_handle); + forward_to_real_kdc( kdc_proxy_message, envelope_realm, @@ -77,6 +85,7 @@ async fn kdc_proxy( &krb_kdc, conf.debug.override_kdc.as_ref(), conf.debug.disable_token_validation, + &kdc_connector, ) .await } @@ -107,6 +116,7 @@ async fn forward_to_real_kdc( token_kdc_addr: &TargetAddr, override_kdc: Option<&TargetAddr>, bypass_realm_check: bool, + kdc_connector: &KdcConnector, ) -> Result, HttpError> { let realm = envelope_realm.ok_or_else(|| HttpError::bad_request().msg("realm is missing from KDC request"))?; debug!(resolved_realm = %realm, "Forward-to-real-KDC realm resolved"); @@ -120,7 +130,9 @@ async fn forward_to_real_kdc( None => token_kdc_addr, }; - let kdc_reply_bytes = send_krb_message(kdc_addr, &kdc_proxy_message.kerb_message.0.0).await?; + let kdc_reply_bytes = kdc_connector + .send(kdc_addr, &kdc_proxy_message.kerb_message.0.0) + .await?; let reply = KdcProxyMessage::from_raw_kerb_message(&kdc_reply_bytes) .map_err(HttpError::internal().with_msg("couldn't create KDC proxy reply").err())?; @@ -130,7 +142,7 @@ async fn forward_to_real_kdc( reply.to_vec().map_err(HttpError::internal().err()) } -fn enforce_credential_injection_enabled(jet_cred_id: uuid::Uuid, enable_unstable: bool) -> Result<(), HttpError> { +fn enforce_credential_injection_enabled(jet_cred_id: Uuid, enable_unstable: bool) -> Result<(), HttpError> { if enable_unstable { return Ok(()); } @@ -165,104 +177,6 @@ fn enforce_realm_token_match(token_realm: &str, request_realm: &str, bypass: boo .err()(format!("expected: {token_realm}, got: {request_realm}"))) } -async fn read_kdc_reply_message(connection: &mut TcpStream) -> io::Result> { - let len = connection.read_u32().await?; - let mut buf = vec![0; (len + 4).try_into().expect("u32-to-usize")]; - buf[0..4].copy_from_slice(&(len.to_be_bytes())); - connection.read_exact(&mut buf[4..]).await?; - Ok(buf) -} - -#[track_caller] -fn unable_to_reach_kdc_server_err(error: io::Error) -> HttpError { - use io::ErrorKind; - - let builder = match error.kind() { - ErrorKind::TimedOut => HttpErrorBuilder::new(StatusCode::GATEWAY_TIMEOUT), - ErrorKind::ConnectionRefused => HttpError::bad_gateway(), - ErrorKind::ConnectionAborted => HttpError::bad_gateway(), - ErrorKind::ConnectionReset => HttpError::bad_gateway(), - ErrorKind::BrokenPipe => HttpError::bad_gateway(), - ErrorKind::OutOfMemory => HttpError::internal(), - // FIXME: once stabilized use new IO error variants - // - https://github.com/rust-lang/rust/pull/106375 - // - https://github.com/rust-lang/rust/issues/86442 - // ErrorKind::NetworkDown => HttpErrorBuilder::new(StatusCode::SERVICE_UNAVAILABLE), - // ErrorKind::NetworkUnreachable => HttpError::bad_gateway(), - // ErrorKind::HostUnreachable => HttpError::bad_gateway(), - // TODO: When the above is applied, we can return an internal error in the fallback branch. - _ => HttpError::bad_gateway(), - }; - - builder.with_msg("unable to reach KDC server").build(error) -} - -/// Sends the Kerberos message to the specified KDC address. -pub async fn send_krb_message(kdc_addr: &TargetAddr, message: &[u8]) -> Result, HttpError> { - let protocol = kdc_addr.scheme(); - - debug!("Connecting to KDC server located at {kdc_addr} using protocol {protocol}..."); - - if protocol == "tcp" { - #[allow(clippy::redundant_closure)] // We get a better caller location for the error by using a closure. - let mut connection = TcpStream::connect(kdc_addr.as_addr()).await.map_err(|e| { - error!(%kdc_addr, "failed to connect to KDC server"); - unable_to_reach_kdc_server_err(e) - })?; - - trace!("Connected! Forwarding KDC message..."); - - connection.write_all(message).await.map_err( - HttpError::bad_gateway() - .with_msg("unable to send the message to the KDC server") - .err(), - )?; - - trace!("Reading KDC reply..."); - - Ok(read_kdc_reply_message(&mut connection).await.map_err( - HttpError::bad_gateway() - .with_msg("unable to read KDC reply message") - .err(), - )?) - } else { - // We assume that ticket length is not bigger than 2048 bytes. - let mut buf = [0; 2048]; - - let udp_socket = UdpSocket::bind("127.0.0.1:0") - .await - .map_err(HttpError::internal().with_msg("unable to bind UDP socket").err())?; - - let port = udp_socket - .local_addr() - .map_err(HttpError::internal().with_msg("unable to get UDP socket address").err())? - .port(); - - trace!("Binded UDP listener to 127.0.0.1:{port}, forwarding KDC message..."); - - // First 4 bytes contains message length. We don't need it for UDP. - #[allow(clippy::redundant_closure)] // We get a better caller location for the error by using a closure. - udp_socket - .send_to(&message[4..], kdc_addr.as_addr()) - .await - .map_err(|e| unable_to_reach_kdc_server_err(e))?; - - trace!("Reading KDC reply..."); - - let n = udp_socket.recv(&mut buf).await.map_err( - HttpError::bad_gateway() - .with_msg("unable to read reply from the KDC server") - .err(), - )?; - - let mut reply_buf = Vec::new(); - reply_buf.extend_from_slice(&u32::try_from(n).expect("n not too big").to_be_bytes()); - reply_buf.extend_from_slice(&buf[0..n]); - - Ok(reply_buf) - } -} - #[cfg(test)] mod tests { use super::*; @@ -288,11 +202,11 @@ mod tests { #[test] fn credential_injection_gate_allows_jet_cred_id_when_enabled() { - assert!(enforce_credential_injection_enabled(uuid::Uuid::new_v4(), true).is_ok()); + assert!(enforce_credential_injection_enabled(Uuid::new_v4(), true).is_ok()); } #[test] fn credential_injection_gate_rejects_jet_cred_id_when_disabled() { - assert!(enforce_credential_injection_enabled(uuid::Uuid::new_v4(), false).is_err()); + assert!(enforce_credential_injection_enabled(Uuid::new_v4(), false).is_err()); } } diff --git a/devolutions-gateway/src/api/webapp.rs b/devolutions-gateway/src/api/webapp.rs index d11f253c9..4e632a61e 100644 --- a/devolutions-gateway/src/api/webapp.rs +++ b/devolutions-gateway/src/api/webapp.rs @@ -390,6 +390,7 @@ pub(crate) async fn sign_session_token( krb_realm: krb_realm.into(), krb_kdc: krb_kdc.clone(), }, + jti, } .pipe(serde_json::to_value) .map(|mut claims| { diff --git a/devolutions-gateway/src/generic_client.rs b/devolutions-gateway/src/generic_client.rs index d44595864..dfc31df7f 100644 --- a/devolutions-gateway/src/generic_client.rs +++ b/devolutions-gateway/src/generic_client.rs @@ -163,6 +163,12 @@ where "RDP-TLS forwarding with credential injection" ); + let kdc_connector = crate::kdc_connector::KdcConnector::new( + claims.jet_aid, + claims.jet_agent_id, + agent_tunnel_handle.clone(), + ); + // NOTE: In the future, we could imagine performing proxy-based recording as well using RdpProxy. return crate::rdp_proxy::RdpProxy::builder() .conf(conf) @@ -177,6 +183,7 @@ where .client_stream_leftover_bytes(leftover_bytes) .server_dns_name(selected_target.host().to_owned()) .disconnect_interest(disconnect_interest) + .kdc_connector(kdc_connector) .build() .run() .await diff --git a/devolutions-gateway/src/kdc_connector.rs b/devolutions-gateway/src/kdc_connector.rs new file mode 100644 index 000000000..380f13056 --- /dev/null +++ b/devolutions-gateway/src/kdc_connector.rs @@ -0,0 +1,291 @@ +//! Outbound dispatcher for KDC messages. +//! +//! Bundles the three inputs the agent-tunnel routing pipeline needs (`session_id`, +//! `explicit_agent_id`, `agent_tunnel_handle`) into a single value so callers do not +//! thread those primitives through every layer of CredSSP machinery — and so the +//! routing decision is always taken by `agent_tunnel::routing::try_route`, never +//! pre-decided by the caller. + +use std::io; +use std::sync::Arc; + +use axum::http::StatusCode; +use ironrdp_connector::sspi::generator::NetworkRequest; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::{TcpStream, UdpSocket}; +use uuid::Uuid; + +use crate::http::{HttpError, HttpErrorBuilder}; +use crate::target_addr::TargetAddr; +use crate::upstream::route_target_from_target_addr; + +/// Sends Kerberos messages to a KDC, consulting the agent-tunnel routing pipeline. +/// +/// All three fields are always passed to [`agent_tunnel::routing::try_route`]; the +/// routing pipeline decides whether to route via an agent, fail, or fall through to a +/// direct connection. Callers do not pre-decide between "direct" and "via tunnel". +/// +/// Field semantics: +/// +/// - `session_id` — tag sent to the agent for log correlation. RDP CredSSP/NLA callers +/// pass `claims.jet_aid` so KDC sub-traffic correlates with its parent RDP session; +/// the HTTP `/jet/KdcProxy` endpoint passes the KDC token's own `jti` (no parent). +/// +/// - `explicit_agent_id` — the parent association token's `jet_agent_id`, if any. When +/// set, traffic must route via that specific agent (or fail). The routing pipeline +/// enforces this even when `agent_tunnel_handle` is `None`. +/// +/// - `agent_tunnel_handle` — `Some` whenever the Gateway is running an agent tunnel +/// listener. Whether *this* particular request goes through it still depends on the +/// target matching an advertised agent route. +#[derive(Clone)] +pub struct KdcConnector { + session_id: Uuid, + explicit_agent_id: Option, + agent_tunnel_handle: Option>, +} + +impl KdcConnector { + pub fn new( + session_id: Uuid, + explicit_agent_id: Option, + agent_tunnel_handle: Option>, + ) -> Self { + Self { + session_id, + explicit_agent_id, + agent_tunnel_handle, + } + } + + /// Send a Kerberos message to `kdc_addr` and return the reply bytes. + pub async fn send(&self, kdc_addr: &TargetAddr, message: &[u8]) -> Result, HttpError> { + let kdc_target = kdc_addr.as_addr(); + let route_target = route_target_from_target_addr(kdc_addr); + + let route_result = agent_tunnel::routing::try_route( + self.agent_tunnel_handle.as_deref(), + self.explicit_agent_id, + &route_target, + self.session_id, + kdc_target, + ) + .await + .map_err(|e| HttpError::bad_gateway().build(format!("KDC routing through agent tunnel failed: {e:#}")))?; + + if let Some((mut stream, _agent)) = route_result { + // The agent tunnel currently carries only TCP (`ConnectRequest::tcp`). If the + // routing pipeline picked an agent for a udp:// KDC target — either by subnet + // match or by explicit pin — we must reject explicitly. Silently falling + // through to direct UDP here would bypass an explicit `jet_agent_id` pin and + // bypass the routing decision in general. + if kdc_addr.scheme().eq_ignore_ascii_case("udp") { + return Err(HttpError::bad_gateway().build( + "agent tunnel does not yet support UDP; udp:// KDC requests cannot be routed through an agent", + )); + } + + stream.write_all(message).await.map_err( + HttpError::bad_gateway() + .with_msg("unable to send KDC message through agent tunnel") + .err(), + )?; + + return read_kdc_reply_message(&mut stream).await.map_err( + HttpError::bad_gateway() + .with_msg("unable to read KDC reply through agent tunnel") + .err(), + ); + } + + // Direct fallback. `try_route` returning `Ok(None)` means: no matching agent and + // no explicit pin — the caller is allowed to direct-connect with the scheme it + // chose. + let protocol = kdc_addr.scheme(); + + debug!("Connecting to KDC server located at {kdc_addr} using protocol {protocol}..."); + + if protocol == "tcp" { + #[allow(clippy::redundant_closure)] // We get a better caller location for the error by using a closure. + let mut connection = TcpStream::connect(kdc_addr.as_addr()).await.map_err(|e| { + error!(%kdc_addr, "failed to connect to KDC server"); + unable_to_reach_kdc_server_err(e) + })?; + + trace!("Connected! Forwarding KDC message..."); + + connection.write_all(message).await.map_err( + HttpError::bad_gateway() + .with_msg("unable to send the message to the KDC server") + .err(), + )?; + + trace!("Reading KDC reply..."); + + Ok(read_kdc_reply_message(&mut connection).await.map_err( + HttpError::bad_gateway() + .with_msg("unable to read KDC reply message") + .err(), + )?) + } else { + // We assume that ticket length is not bigger than 2048 bytes. + let mut buf = [0; 2048]; + + let udp_socket = UdpSocket::bind("127.0.0.1:0") + .await + .map_err(HttpError::internal().with_msg("unable to bind UDP socket").err())?; + + let port = udp_socket + .local_addr() + .map_err(HttpError::internal().with_msg("unable to get UDP socket address").err())? + .port(); + + trace!("Binded UDP listener to 127.0.0.1:{port}, forwarding KDC message..."); + + // First 4 bytes contains message length. We don't need it for UDP. + #[allow(clippy::redundant_closure)] // We get a better caller location for the error by using a closure. + udp_socket + .send_to(&message[4..], kdc_addr.as_addr()) + .await + .map_err(|e| unable_to_reach_kdc_server_err(e))?; + + trace!("Reading KDC reply..."); + + let n = udp_socket.recv(&mut buf).await.map_err( + HttpError::bad_gateway() + .with_msg("unable to read reply from the KDC server") + .err(), + )?; + + let mut reply_buf = Vec::new(); + reply_buf.extend_from_slice(&u32::try_from(n).expect("n not too big").to_be_bytes()); + reply_buf.extend_from_slice(&buf[0..n]); + + Ok(reply_buf) + } + } + + /// Adapter for `sspi-rs` CredSSP: drain one `NetworkRequest` to its KDC and return the + /// reply bytes as an `anyhow::Result`. + /// + /// Only handles real-network schemes (`tcp` / `udp`); credential-injection loopback + /// requests are intercepted by `CredentialInjectionKdc` before reaching this point. + /// + /// TODO(sspi-rs#664): once sspi-rs ships a pluggable KDC dispatcher API, this adapter + /// goes away entirely. + pub async fn send_network_request(&self, request: &NetworkRequest) -> anyhow::Result> { + match request.url.scheme() { + "tcp" | "udp" => { + let target_addr = TargetAddr::parse(request.url.as_str(), Some(88))?; + + self.send(&target_addr, &request.data) + .await + .map_err(|err| anyhow::anyhow!("failed to send KDC message: {err}")) + } + unsupported => anyhow::bail!("unsupported KDC request scheme: {unsupported}"), + } + } +} + +/// Hard ceiling on the announced length of a TCP-framed KDC reply. +/// +/// The KDC TCP transport prefixes its message with a 4-byte big-endian length. +/// A misbehaving (or malicious) peer can claim up to `u32::MAX` bytes, which +/// without a cap would have us pre-allocate ~4 GiB on a single reply. 64 KiB +/// is well above any realistic Kerberos reply size while keeping the worst +/// case bounded. +const MAX_KDC_REPLY_MESSAGE_LEN: u32 = 64 * 1024; + +async fn read_kdc_reply_message(reader: &mut R) -> io::Result> { + let len = reader.read_u32().await?; + + if len > MAX_KDC_REPLY_MESSAGE_LEN { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("KDC reply too large: announced {len} bytes, maximum is {MAX_KDC_REPLY_MESSAGE_LEN}"), + )); + } + + let total_len = len + .checked_add(4) + .and_then(|n| usize::try_from(n).ok()) + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "KDC reply length prefix overflowed"))?; + + let mut buf = vec![0; total_len]; + buf[0..4].copy_from_slice(&len.to_be_bytes()); + reader.read_exact(&mut buf[4..]).await?; + Ok(buf) +} + +#[track_caller] +fn unable_to_reach_kdc_server_err(error: io::Error) -> HttpError { + use io::ErrorKind; + + let builder = match error.kind() { + ErrorKind::TimedOut => HttpErrorBuilder::new(StatusCode::GATEWAY_TIMEOUT), + ErrorKind::ConnectionRefused => HttpError::bad_gateway(), + ErrorKind::ConnectionAborted => HttpError::bad_gateway(), + ErrorKind::ConnectionReset => HttpError::bad_gateway(), + ErrorKind::BrokenPipe => HttpError::bad_gateway(), + ErrorKind::OutOfMemory => HttpError::internal(), + _ => HttpError::bad_gateway(), + }; + + builder.with_msg("unable to reach KDC server").build(error) +} + +#[cfg(test)] +mod tests { + //! Behavioral contract of [`KdcConnector::send`]. + //! + //! These tests pin down the routing decision matrix — see the table in the file-level + //! docs. The success path (agent matched + connection succeeded) and the new UDP-via- + //! agent guard cannot be reached without a live QUIC connection to a fake agent, so + //! Only the two cases that don't require an [`AgentTunnelHandle`] are covered here: + //! pin-without-tunnel (must error) and no-pin-no-tunnel (falls through to direct). + //! The four cases that need a real handle (pin-with-missing-agent, no-match-falls-back, + //! tunnel success, UDP-via-agent guard) are observable today only via integration tests + //! that stand up an actual agent-tunnel listener — left as a follow-up. + use uuid::Uuid; + + use super::*; + + fn unreachable_kdc_addr() -> TargetAddr { + // Loopback + a port that is not listening produces ConnectionRefused on every supported + // platform, which `unable_to_reach_kdc_server_err` maps to a bad-gateway `HttpError`. + // Avoids a real network round-trip while still exercising the direct-connect branch. + TargetAddr::parse("tcp://127.0.0.1:1", Some(88)).expect("static target addr is valid") + } + + /// No tunnel handle + explicit agent pin → must error. + /// + /// `jet_agent_id` declares a routing requirement; with no agent tunnel listener + /// configured, falling back to a direct connection would silently bypass that + /// requirement. `try_route` rejects this combination and we surface the error. + #[tokio::test] + async fn explicit_pin_without_tunnel_handle_errors() { + let connector = KdcConnector::new(Uuid::new_v4(), Some(Uuid::new_v4()), None); + let result = connector.send(&unreachable_kdc_addr(), b"\x00\x00\x00\x00").await; + let err = result.expect_err("explicit pin must reject when no tunnel handle is configured"); + assert!( + format!("{err}").contains("requires agent tunnel routing"), + "error message should explain the pin/tunnel mismatch, got: {err}", + ); + } + + /// No tunnel handle, no pin → falls through to direct connect. + /// + /// We point at an unreachable loopback port; the only thing the test asserts is that + /// we *got* to the direct-connect path (any error from there shape-matches the + /// "unable to reach KDC server" wrapping). + #[tokio::test] + async fn no_pin_no_tunnel_handle_attempts_direct() { + let connector = KdcConnector::new(Uuid::new_v4(), None, None); + let result = connector.send(&unreachable_kdc_addr(), b"\x00\x00\x00\x00").await; + let err = result.expect_err("loopback:1 should be unreachable"); + assert!( + format!("{err}").contains("unable to reach KDC server"), + "should have reached the direct-connect branch, got: {err}", + ); + } +} diff --git a/devolutions-gateway/src/lib.rs b/devolutions-gateway/src/lib.rs index 5c1777e6e..a55556b59 100644 --- a/devolutions-gateway/src/lib.rs +++ b/devolutions-gateway/src/lib.rs @@ -24,6 +24,7 @@ pub mod http; pub mod interceptor; pub mod jmux; pub mod job_queue; +pub mod kdc_connector; pub mod listener; pub mod log; pub mod middleware; diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index bbe69290c..642b516f8 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -417,6 +417,9 @@ async fn handle_with_credential_injection( let krb_server_config = crate::rdp_proxy::credential_injection_kerberos_server_config(&conf, client_addr, &credential_injection_kdc)?; + let kdc_connector = + crate::kdc_connector::KdcConnector::new(claims.jet_aid, claims.jet_agent_id, agent_tunnel_handle.clone()); + let client_credssp_fut = crate::rdp_proxy::perform_credssp_as_server( &mut client_framed, client_addr.ip(), @@ -425,6 +428,7 @@ async fn handle_with_credential_injection( credential_injection_kdc.proxy_credential(), krb_server_config, &credential_injection_kdc, + &kdc_connector, ); let krb_client_config = if conf.debug.enable_unstable @@ -448,6 +452,7 @@ async fn handle_with_credential_injection( server_security_protocol, credential_injection_kdc.target_credential(), krb_client_config, + &kdc_connector, ); let (client_credssp_res, server_credssp_res) = tokio::join!(client_credssp_fut, server_credssp_fut); diff --git a/devolutions-gateway/src/rdp_proxy.rs b/devolutions-gateway/src/rdp_proxy.rs index 17a37e757..3649b2285 100644 --- a/devolutions-gateway/src/rdp_proxy.rs +++ b/devolutions-gateway/src/rdp_proxy.rs @@ -5,22 +5,21 @@ use anyhow::Context as _; use ironrdp_acceptor::credssp::CredsspProcessGenerator as CredsspServerProcessGenerator; use ironrdp_connector::credssp::CredsspProcessGenerator as CredsspClientProcessGenerator; use ironrdp_connector::sspi; -use ironrdp_connector::sspi::generator::{GeneratorState, NetworkRequest}; +use ironrdp_connector::sspi::generator::GeneratorState; use ironrdp_pdu::{mcs, nego, x224}; use secrecy::ExposeSecret as _; use tokio::io::{AsyncRead, AsyncWrite, AsyncWriteExt}; use typed_builder::TypedBuilder; -use crate::api::kdc_proxy::send_krb_message; use crate::config::Conf; use crate::credential::AppCredential; use crate::credential_injection_kdc::{ CredentialInjectionClientAcceptorProtocol, CredentialInjectionKdc, CredentialInjectionKdcInterception, }; +use crate::kdc_connector::KdcConnector; use crate::proxy::Proxy; use crate::session::{DisconnectInterest, SessionInfo, SessionMessageSender}; use crate::subscriber::SubscriberSender; -use crate::target_addr::TargetAddr; #[derive(TypedBuilder)] pub struct RdpProxy { @@ -36,6 +35,10 @@ pub struct RdpProxy { subscriber_tx: SubscriberSender, server_dns_name: String, disconnect_interest: Option, + /// Outbound dispatcher for CredSSP-originated KDC traffic. Encapsulates whether KDC + /// requests should attempt agent-tunnel routing (and any `jet_agent_id` pin from the + /// parent association token) or always go direct. + kdc_connector: KdcConnector, } impl RdpProxy @@ -67,6 +70,7 @@ where subscriber_tx, server_dns_name, disconnect_interest, + kdc_connector, } = proxy; let tls_conf = conf.credssp_tls.get().context("CredSSP TLS configuration")?; @@ -127,6 +131,7 @@ where credential_injection_kdc.proxy_credential(), krb_server_config, &credential_injection_kdc, + &kdc_connector, ); let krb_client_config = if conf.debug.enable_unstable @@ -150,6 +155,7 @@ where handshake_result.server_security_protocol, credential_injection_kdc.target_credential(), krb_client_config, + &kdc_connector, ); let (client_credssp_res, server_credssp_res) = tokio::join!(client_credssp_fut, server_credssp_fut); @@ -388,6 +394,7 @@ pub(crate) async fn perform_credssp_as_client( security_protocol: nego::SecurityProtocol, credentials: &AppCredential, kerberos_config: Option, + kdc_connector: &KdcConnector, ) -> anyhow::Result<()> where S: ironrdp_tokio::FramedRead + ironrdp_tokio::FramedWrite, @@ -420,7 +427,7 @@ where loop { let client_state = { let mut generator = sequence.process_ts_request(ts_request); - resolve_client_generator(&mut generator).await? + resolve_client_generator(&mut generator, kdc_connector).await? }; // drop generator buf.clear(); @@ -453,6 +460,7 @@ where async fn resolve_server_generator( generator: &mut CredsspServerProcessGenerator<'_>, credential_injection_kdc: &CredentialInjectionKdc, + kdc_connector: &KdcConnector, ) -> Result { let mut state = generator.start(); @@ -461,7 +469,9 @@ async fn resolve_server_generator( GeneratorState::Suspended(request) => { let response = match credential_injection_kdc.intercept_network_request(&request) { Ok(CredentialInjectionKdcInterception::Intercepted(response)) => Ok(response), - Ok(CredentialInjectionKdcInterception::NotInjectionRequest) => send_network_request(&request).await, + Ok(CredentialInjectionKdcInterception::NotInjectionRequest) => { + kdc_connector.send_network_request(&request).await + } Ok(CredentialInjectionKdcInterception::NotInjectionRealm(mismatch)) => Err(anyhow::anyhow!( "kdc request realm does not match credential-injection session realm: {mismatch}" )), @@ -483,13 +493,14 @@ async fn resolve_server_generator( async fn resolve_client_generator( generator: &mut CredsspClientProcessGenerator<'_>, + kdc_connector: &KdcConnector, ) -> anyhow::Result { let mut state = generator.start(); loop { match state { GeneratorState::Suspended(request) => { - let response = send_network_request(&request).await?; + let response = kdc_connector.send_network_request(&request).await?; state = generator.resume(Ok(response)); } GeneratorState::Completed(client_state) => { @@ -501,6 +512,7 @@ async fn resolve_client_generator( } } +#[expect(clippy::too_many_arguments)] #[instrument(name = "client_credssp", level = "debug", ret, skip_all)] pub(crate) async fn perform_credssp_as_server( framed: &mut ironrdp_tokio::Framed, @@ -510,6 +522,7 @@ pub(crate) async fn perform_credssp_as_server( credentials: &AppCredential, kerberos_server_config: Option, credential_injection_kdc: &CredentialInjectionKdc, + kdc_connector: &KdcConnector, ) -> anyhow::Result<()> where S: ironrdp_tokio::FramedRead + ironrdp_tokio::FramedWrite, @@ -531,6 +544,7 @@ where credentials, kerberos_server_config, credential_injection_kdc, + kdc_connector, ) .await; @@ -559,6 +573,7 @@ where credentials: &AppCredential, kerberos_server_config: Option, credential_injection_kdc: &CredentialInjectionKdc, + kdc_connector: &KdcConnector, ) -> anyhow::Result<()> where S: ironrdp_tokio::FramedRead + ironrdp_tokio::FramedWrite, @@ -600,7 +615,7 @@ where let result = { let mut generator = sequence.process_ts_request(ts_request); - resolve_server_generator(&mut generator, credential_injection_kdc).await + resolve_server_generator(&mut generator, credential_injection_kdc, kdc_connector).await }; // drop generator buf.clear(); @@ -630,28 +645,3 @@ where framed.write_all(&payload).await.context("failed to write PDU")?; Ok(()) } - -/// Generic Kerberos network-request dispatcher. -/// -/// Only handles real-network schemes (`tcp` / `udp`); credential-injection loopback requests -/// are intercepted by [`CredentialInjectionKdc`] before reaching this function. -/// -/// TODO(sspi-rs#664): when sspi-rs ships a pluggable KDC dispatcher API, the URL trick for -/// credential injection goes away entirely and this helper can be inlined back into the -/// CredSSP loops. -async fn send_network_request(request: &NetworkRequest) -> anyhow::Result> { - match request.url.scheme() { - "tcp" | "udp" => { - let target_addr = TargetAddr::parse(request.url.as_str(), Some(88))?; - - // TODO(DGW-384): plumb `agent_tunnel_handle` through `RdpProxy` so - // CredSSP-originated Kerberos requests can traverse the agent tunnel. - // Currently these go direct from the gateway host, bypassing the - // routing pipeline used by every other proxy path. - send_krb_message(&target_addr, &request.data) - .await - .map_err(|err| anyhow::Error::msg("failed to send KDC message").context(err)) - } - unsupported => anyhow::bail!("unsupported KDC request scheme: {unsupported}"), - } -} diff --git a/devolutions-gateway/src/token.rs b/devolutions-gateway/src/token.rs index b390fbf5c..a27382c41 100644 --- a/devolutions-gateway/src/token.rs +++ b/devolutions-gateway/src/token.rs @@ -609,6 +609,13 @@ pub struct JrecTokenClaims { pub struct KdcTokenClaims { /// Where the KDC traffic for this session is routed. pub destination: KdcDestination, + + /// JWT "JWT ID" claim, the unique ID for this token. + /// + /// Used as the agent-tunnel session ID when the HTTP `/jet/KdcProxy` endpoint forwards + /// through an agent — it has no parent association token to inherit `jet_aid` from, so the + /// KDC token's own `jti` provides a persistent log-correlation identifier instead. + pub jti: Uuid, } /// Destination for a KDC session token. @@ -1482,6 +1489,7 @@ mod serde_impl { krb_kdc: Option, #[serde(default, skip_serializing_if = "Option::is_none")] jet_cred_id: Option, + jti: Uuid, } impl ser::Serialize for SessionTtl { @@ -1733,11 +1741,13 @@ mod serde_impl { krb_realm: Some(krb_realm.clone()), krb_kdc: Some(SmolStr::new(krb_kdc.as_str())), jet_cred_id: None, + jti: self.jti, }, KdcDestination::Inject { jti } => KdcClaimsHelper { krb_realm: None, krb_kdc: None, jet_cred_id: Some(*jti), + jti: self.jti, }, }; @@ -1789,7 +1799,10 @@ mod serde_impl { } }; - Ok(Self { destination }) + Ok(Self { + destination, + jti: claims.jti, + }) } } } diff --git a/devolutions-gateway/src/upstream.rs b/devolutions-gateway/src/upstream.rs index 94e853026..3716d5d33 100644 --- a/devolutions-gateway/src/upstream.rs +++ b/devolutions-gateway/src/upstream.rs @@ -298,7 +298,7 @@ impl<'a> RoutePlan<'a> { } } -fn route_target_from_target_addr(target: &TargetAddr) -> RouteTarget { +pub(crate) fn route_target_from_target_addr(target: &TargetAddr) -> RouteTarget { match target.host_ip() { Some(ip) => RouteTarget::ip(ip), None => RouteTarget::hostname(target.host()), From a9b3512234c4f6fe4606fa94d9083b84b1e5bd0b Mon Sep 17 00:00:00 2001 From: irving ou Date: Fri, 22 May 2026 13:25:06 -0400 Subject: [PATCH 2/2] fix(dgw): address KDC connector review feedback --- devolutions-gateway/src/kdc_connector.rs | 63 +++++++++++++++++++++--- devolutions-gateway/src/token.rs | 28 +++++++++++ 2 files changed, 85 insertions(+), 6 deletions(-) diff --git a/devolutions-gateway/src/kdc_connector.rs b/devolutions-gateway/src/kdc_connector.rs index 380f13056..deb97f3db 100644 --- a/devolutions-gateway/src/kdc_connector.rs +++ b/devolutions-gateway/src/kdc_connector.rs @@ -7,6 +7,7 @@ //! pre-decided by the caller. use std::io; +use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr}; use std::sync::Arc; use axum::http::StatusCode; @@ -128,24 +129,30 @@ impl KdcConnector { .err(), )?) } else { + let udp_payload = message.get(4..).ok_or_else(|| { + HttpError::bad_request().msg("KDC UDP message is too short to contain a length prefix") + })?; + + let destination_addr = resolve_udp_destination(kdc_addr).await?; + let bind_addr = udp_bind_addr_for(destination_addr); + // We assume that ticket length is not bigger than 2048 bytes. let mut buf = [0; 2048]; - let udp_socket = UdpSocket::bind("127.0.0.1:0") + let udp_socket = UdpSocket::bind(bind_addr) .await .map_err(HttpError::internal().with_msg("unable to bind UDP socket").err())?; - let port = udp_socket + let local_addr = udp_socket .local_addr() - .map_err(HttpError::internal().with_msg("unable to get UDP socket address").err())? - .port(); + .map_err(HttpError::internal().with_msg("unable to get UDP socket address").err())?; - trace!("Binded UDP listener to 127.0.0.1:{port}, forwarding KDC message..."); + trace!(%local_addr, %destination_addr, "Bound UDP listener, forwarding KDC message"); // First 4 bytes contains message length. We don't need it for UDP. #[allow(clippy::redundant_closure)] // We get a better caller location for the error by using a closure. udp_socket - .send_to(&message[4..], kdc_addr.as_addr()) + .send_to(udp_payload, destination_addr) .await .map_err(|e| unable_to_reach_kdc_server_err(e))?; @@ -187,6 +194,24 @@ impl KdcConnector { } } +async fn resolve_udp_destination(kdc_addr: &TargetAddr) -> Result { + let mut addrs = tokio::net::lookup_host(kdc_addr.as_addr()) + .await + .map_err(unable_to_reach_kdc_server_err)?; + + addrs.next().ok_or_else(|| { + unable_to_reach_kdc_server_err(io::Error::new(io::ErrorKind::NotFound, "KDC address resolved empty")) + }) +} + +fn udp_bind_addr_for(destination_addr: SocketAddr) -> SocketAddr { + if destination_addr.is_ipv4() { + SocketAddr::from((Ipv4Addr::UNSPECIFIED, 0)) + } else { + SocketAddr::from((Ipv6Addr::UNSPECIFIED, 0)) + } +} + /// Hard ceiling on the announced length of a TCP-framed KDC reply. /// /// The KDC TCP transport prefixes its message with a 4-byte big-endian length. @@ -257,6 +282,10 @@ mod tests { TargetAddr::parse("tcp://127.0.0.1:1", Some(88)).expect("static target addr is valid") } + fn udp_kdc_addr() -> TargetAddr { + TargetAddr::parse("udp://127.0.0.1:88", Some(88)).expect("static target addr is valid") + } + /// No tunnel handle + explicit agent pin → must error. /// /// `jet_agent_id` declares a routing requirement; with no agent tunnel listener @@ -288,4 +317,26 @@ mod tests { "should have reached the direct-connect branch, got: {err}", ); } + + #[tokio::test] + async fn udp_message_shorter_than_length_prefix_errors() { + let connector = KdcConnector::new(Uuid::new_v4(), None, None); + let result = connector.send(&udp_kdc_addr(), b"\x00\x01\x02").await; + let err = result.expect_err("UDP message shorter than the TCP length prefix must be rejected"); + assert!( + format!("{err}").contains("too short"), + "error message should explain the malformed UDP payload, got: {err}", + ); + } + + #[test] + fn udp_bind_addr_matches_destination_family() { + let v4_bind = udp_bind_addr_for(SocketAddr::from((Ipv4Addr::LOCALHOST, 88))); + assert!(v4_bind.is_ipv4()); + assert_eq!(v4_bind.port(), 0); + + let v6_bind = udp_bind_addr_for(SocketAddr::from((Ipv6Addr::LOCALHOST, 88))); + assert!(v6_bind.is_ipv6()); + assert_eq!(v6_bind.port(), 0); + } } diff --git a/devolutions-gateway/src/token.rs b/devolutions-gateway/src/token.rs index a27382c41..c3f3d381c 100644 --- a/devolutions-gateway/src/token.rs +++ b/devolutions-gateway/src/token.rs @@ -1489,9 +1489,14 @@ mod serde_impl { krb_kdc: Option, #[serde(default, skip_serializing_if = "Option::is_none")] jet_cred_id: Option, + #[serde(default = "legacy_kdc_token_jti")] jti: Uuid, } + fn legacy_kdc_token_jti() -> Uuid { + Uuid::new_v4() + } + impl ser::Serialize for SessionTtl { fn serialize(&self, serializer: S) -> Result where @@ -1834,4 +1839,27 @@ mod tests { vec!["secondary.example:3389".to_owned()] ); } + + #[test] + fn kdc_real_claims_without_jti_deserialize_for_legacy_tokens() { + let claims: KdcTokenClaims = serde_json::from_value(serde_json::json!({ + "krb_realm": "example.com", + "krb_kdc": "tcp://dc.example.com:88", + })) + .expect("legacy KDC token without jti should still deserialize"); + + assert_ne!(claims.jti, Uuid::nil()); + assert!(matches!(claims.destination, KdcDestination::Real { .. })); + } + + #[test] + fn kdc_injection_claims_without_jti_deserialize_for_legacy_tokens() { + let claims: KdcTokenClaims = serde_json::from_value(serde_json::json!({ + "jet_cred_id": Uuid::new_v4(), + })) + .expect("legacy injection KDC token without jti should still deserialize"); + + assert_ne!(claims.jti, Uuid::nil()); + assert!(matches!(claims.destination, KdcDestination::Inject { .. })); + } }