Skip to content

Commit a2b2ff5

Browse files
refactor(pr2): defer KDC tunnel routing to DGW-384
Revert kdc_proxy.rs to master. KDC tunnel routing covers two callers (the /jet/KdcProxy HTTP handler and the CredSSP/NLA path in rdp_proxy.rs::send_network_request), and both need to agree on how send_krb_message takes a session_id. Doing only the HTTP handler here forced a Uuid::new_v4() at the routing site for a "session" the KDC token has no notion of -- meaningless on the wire and on the agent log. Move the whole KDC-via-tunnel story (HTTP path + CredSSP path, plus the read_kdc_reply_message DoS cap) into DGW-384 where the API can be designed once with both call sites in view. Also drop the kdc_proxy.rs reference from routing.rs's module doc since this crate's caller is now only the upstream module family.
1 parent 1188fbe commit a2b2ff5

3 files changed

Lines changed: 14 additions & 96 deletions

File tree

crates/agent-tunnel/src/routing.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Shared routing pipeline for agent tunnel.
22
//!
3-
//! Used by both connection forwarding (`fwd.rs`) and KDC proxy (`kdc_proxy.rs`)
4-
//! to ensure consistent routing behavior and error messages.
3+
//! Consumed by the upstream connection paths (forwarding, RDP clean path,
4+
//! generic client) to ensure consistent routing behavior and error messages.
55
66
use std::net::IpAddr;
77
use std::sync::Arc;

devolutions-gateway/src/api/kdc_proxy.rs

Lines changed: 7 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ async fn kdc_proxy(
2525
token_cache,
2626
jrl,
2727
recordings,
28-
agent_tunnel_handle,
2928
..
3029
}): State<DgwState>,
3130
extract::Path(token): extract::Path<String>,
@@ -106,12 +105,7 @@ async fn kdc_proxy(
106105
&claims.krb_kdc
107106
};
108107

109-
let kdc_reply_message = send_krb_message(
110-
kdc_addr,
111-
&kdc_proxy_message.kerb_message.0.0,
112-
agent_tunnel_handle.as_deref(),
113-
)
114-
.await?;
108+
let kdc_reply_message = send_krb_message(kdc_addr, &kdc_proxy_message.kerb_message.0.0).await?;
115109

116110
let kdc_reply_message = KdcProxyMessage::from_raw_kerb_message(&kdc_reply_message)
117111
.map_err(HttpError::internal().with_msg("couldn't create KDC proxy reply").err())?;
@@ -121,33 +115,11 @@ async fn kdc_proxy(
121115
kdc_reply_message.to_vec().map_err(HttpError::internal().err())
122116
}
123117

124-
/// Hard ceiling on the announced length of a TCP-framed KDC reply.
125-
///
126-
/// The KDC TCP transport prefixes its message with a 4-byte big-endian length.
127-
/// A misbehaving (or malicious) peer can claim up to `u32::MAX` bytes, which
128-
/// without a cap would have us pre-allocate ~4 GiB on a single reply. 64 KiB
129-
/// is well above any realistic Kerberos reply size while keeping the worst
130-
/// case bounded.
131-
const MAX_KDC_REPLY_MESSAGE_LEN: u32 = 64 * 1024;
132-
133-
async fn read_kdc_reply_message<R: AsyncReadExt + Unpin>(reader: &mut R) -> io::Result<Vec<u8>> {
134-
let len = reader.read_u32().await?;
135-
136-
if len > MAX_KDC_REPLY_MESSAGE_LEN {
137-
return Err(io::Error::new(
138-
io::ErrorKind::InvalidData,
139-
format!("KDC reply too large: announced {len} bytes, maximum is {MAX_KDC_REPLY_MESSAGE_LEN}"),
140-
));
141-
}
142-
143-
let total_len = len
144-
.checked_add(4)
145-
.and_then(|n| usize::try_from(n).ok())
146-
.ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "KDC reply length prefix overflowed"))?;
147-
148-
let mut buf = vec![0; total_len];
149-
buf[0..4].copy_from_slice(&len.to_be_bytes());
150-
reader.read_exact(&mut buf[4..]).await?;
118+
async fn read_kdc_reply_message(connection: &mut TcpStream) -> io::Result<Vec<u8>> {
119+
let len = connection.read_u32().await?;
120+
let mut buf = vec![0; (len + 4).try_into().expect("u32-to-usize")];
121+
buf[0..4].copy_from_slice(&(len.to_be_bytes()));
122+
connection.read_exact(&mut buf[4..]).await?;
151123
Ok(buf)
152124
}
153125

@@ -176,59 +148,7 @@ fn unable_to_reach_kdc_server_err(error: io::Error) -> HttpError {
176148
}
177149

178150
/// Sends the Kerberos message to the specified KDC address.
179-
///
180-
/// Uses the same routing pipeline as connection forwarding:
181-
/// if an agent claims the KDC's domain/subnet, traffic goes through the tunnel.
182-
/// Falls back to direct connect only when no agent matches.
183-
pub async fn send_krb_message(
184-
kdc_addr: &TargetAddr,
185-
message: &[u8],
186-
agent_tunnel_handle: Option<&agent_tunnel::AgentTunnelHandle>,
187-
) -> Result<Vec<u8>, HttpError> {
188-
// Route through agent tunnel using the SAME pipeline as connection forwarding,
189-
// but only for `tcp` KDC targets. The agent tunnel currently has a single
190-
// `ConnectRequest::tcp` shape, so a `udp://` KDC routed this way would be
191-
// delivered to the agent as a TCP target — wrong protocol semantics that can
192-
// silently break UDP Kerberos deployments. Fall through to the direct path
193-
// (which honors the scheme) until an explicit UDP tunnel hop exists.
194-
//
195-
// `as_addr()` returns `host:port` (with IPv6 brackets), which is what the agent
196-
// tunnel target parser expects — unlike `to_string()` which includes the scheme.
197-
let kdc_target = kdc_addr.as_addr();
198-
let agent_tunnel_handle = if kdc_addr.scheme().eq_ignore_ascii_case("tcp") {
199-
agent_tunnel_handle
200-
} else {
201-
None
202-
};
203-
204-
let route_target = match kdc_addr.host_ip() {
205-
Some(ip) => agent_tunnel::routing::RouteTarget::ip(ip),
206-
None => agent_tunnel::routing::RouteTarget::hostname(kdc_addr.host()),
207-
};
208-
209-
if let Some((mut stream, _agent)) = agent_tunnel::routing::try_route(
210-
agent_tunnel_handle,
211-
None,
212-
&route_target,
213-
uuid::Uuid::new_v4(),
214-
kdc_target,
215-
)
216-
.await
217-
.map_err(|e| HttpError::bad_gateway().build(format!("KDC routing through agent tunnel failed: {e:#}")))?
218-
{
219-
stream.write_all(message).await.map_err(
220-
HttpError::bad_gateway()
221-
.with_msg("unable to send KDC message through agent tunnel")
222-
.err(),
223-
)?;
224-
225-
return read_kdc_reply_message(&mut stream).await.map_err(
226-
HttpError::bad_gateway()
227-
.with_msg("unable to read KDC reply through agent tunnel")
228-
.err(),
229-
);
230-
}
231-
151+
pub async fn send_krb_message(kdc_addr: &TargetAddr, message: &[u8]) -> Result<Vec<u8>, HttpError> {
232152
let protocol = kdc_addr.scheme();
233153

234154
debug!("Connecting to KDC server located at {kdc_addr} using protocol {protocol}...");

devolutions-gateway/src/rdp_proxy.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -637,13 +637,11 @@ where
637637
async fn send_network_request(request: &NetworkRequest) -> anyhow::Result<Vec<u8>> {
638638
let target_addr = TargetAddr::parse(request.url.as_str(), Some(88))?;
639639

640-
// TODO(agent-tunnel): plumb `agent_tunnel_handle` through `RdpProxy` and pass it here
641-
// so CredSSP-originated Kerberos network requests can traverse the agent tunnel when
642-
// the KDC is only reachable from an enrolled agent's network. Currently these requests
643-
// always go out direct from the gateway host, which bypasses the transparent routing
644-
// pipeline used by every other proxy path (fwd / rd_clean_path / generic_client /
645-
// kdc_proxy). Edge case: KDC behind a NAT'd site with no direct path from the gateway.
646-
send_krb_message(&target_addr, &request.data, None)
640+
// TODO(DGW-384): plumb `agent_tunnel_handle` through `RdpProxy` so
641+
// CredSSP-originated Kerberos requests can traverse the agent tunnel.
642+
// Currently these go direct from the gateway host, bypassing the
643+
// routing pipeline used by every other proxy path.
644+
send_krb_message(&target_addr, &request.data)
647645
.await
648646
.map_err(|err| anyhow::Error::msg("failed to send KDC message").context(err))
649647
}

0 commit comments

Comments
 (0)