Skip to content

Commit c3f2066

Browse files
Revert "feat(dgw): route KDC traffic through agent tunnel"
This reverts commit 6e80f71 (PR #1781). Reason: the merged change reshaped `KdcToken` from a tuple struct (`pub struct KdcToken(pub KdcTokenClaims)`) to a named-fields struct (`pub struct KdcToken { pub claims, pub jti }`) without prior consult. This is a breaking change to a public API surface. The correct shape for surfacing the KDC token's `jti` is to add it to `KdcTokenClaims` (and `KdcClaimsHelper`'s serde) so it lives next to the other token fields, keeping `KdcToken` itself unchanged. That follow-up will be done in a separate PR. The listener.rs `Arc::clone` drive-by fix from #1781 is also reverted here; it should be re-landed in its own hotfix PR independent of DGW-384. Issue: DGW-384
1 parent 6e80f71 commit c3f2066

7 files changed

Lines changed: 28 additions & 213 deletions

File tree

crates/agent-tunnel/src/listener.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ impl AgentTunnelListener {
152152
let handle = AgentTunnelHandle {
153153
registry: Arc::clone(&registry),
154154
agent_connections: Arc::clone(&agent_connections),
155-
ca_manager: Arc::clone(&ca_manager),
155+
ca_manager,
156156
};
157157

158158
let listener = Self {

crates/agent-tunnel/src/routing.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
//! Shared routing pipeline for agent tunnel.
22
//!
33
//! Consumed by the upstream connection paths (forwarding, RDP clean path,
4-
//! generic client) and by the KDC proxy (HTTP endpoint plus the CredSSP/NLA
5-
//! sub-flow inside `rdp_proxy.rs`) to ensure consistent routing behavior and
6-
//! error messages.
4+
//! generic client) to ensure consistent routing behavior and error messages.
75
86
use std::net::IpAddr;
97
use std::sync::Arc;

devolutions-gateway/src/api/kdc_proxy.rs

Lines changed: 11 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use axum::routing::post;
77
use picky_krb::messages::KdcProxyMessage;
88
use tokio::io::{AsyncReadExt, AsyncWriteExt};
99
use tokio::net::{TcpStream, UdpSocket};
10-
use uuid::Uuid;
1110

1211
use crate::DgwState;
1312
use crate::credential_injection_kdc::{
@@ -27,13 +26,9 @@ async fn kdc_proxy(
2726
State(DgwState {
2827
conf_handle,
2928
credentials,
30-
agent_tunnel_handle,
3129
..
3230
}): State<DgwState>,
33-
KdcToken {
34-
claims: KdcTokenClaims { destination },
35-
jti: token_jti,
36-
}: KdcToken,
31+
KdcToken(KdcTokenClaims { destination }): KdcToken,
3732
body: axum::body::Bytes,
3833
) -> Result<Vec<u8>, HttpError> {
3934
let conf = conf_handle.get_conf();
@@ -82,8 +77,6 @@ async fn kdc_proxy(
8277
&krb_kdc,
8378
conf.debug.override_kdc.as_ref(),
8479
conf.debug.disable_token_validation,
85-
agent_tunnel_handle.as_deref(),
86-
token_jti,
8780
)
8881
.await
8982
}
@@ -107,21 +100,13 @@ fn credential_injection_resolve_error(error: CredentialInjectionKdcResolveError)
107100
// The forward path requires the envelope realm to be set: there is no fallback since this is
108101
// not a credential-injection session. After resolving, validates the realm against the
109102
// token's `krb_realm` claim before forwarding anything.
110-
#[expect(clippy::too_many_arguments)]
111103
async fn forward_to_real_kdc(
112104
kdc_proxy_message: KdcProxyMessage,
113105
envelope_realm: Option<String>,
114106
token_realm: &str,
115107
token_kdc_addr: &TargetAddr,
116108
override_kdc: Option<&TargetAddr>,
117109
bypass_realm_check: bool,
118-
agent_tunnel_handle: Option<&agent_tunnel::AgentTunnelHandle>,
119-
// The HTTP /jet/KdcProxy endpoint has no parent association token, so we use the KDC
120-
// token's own `jti` for log/agent-side correlation. It is persistent for the lifetime of
121-
// the KDC token (which can be reused) rather than per-request, but it is the most stable
122-
// identifier we have here. The RDP CredSSP/NLA caller (rdp_proxy.rs::send_network_request)
123-
// passes `claims.jet_aid` instead so KDC sub-traffic correlates with its RDP session.
124-
session_id: Uuid,
125110
) -> Result<Vec<u8>, HttpError> {
126111
let realm = envelope_realm.ok_or_else(|| HttpError::bad_request().msg("realm is missing from KDC request"))?;
127112
debug!(resolved_realm = %realm, "Forward-to-real-KDC realm resolved");
@@ -135,19 +120,7 @@ async fn forward_to_real_kdc(
135120
None => token_kdc_addr,
136121
};
137122

138-
// No parent association token here, so no `jet_agent_id` to enforce. The HTTP
139-
// /jet/KdcProxy endpoint stands on its own — let the routing pipeline pick any
140-
// matching agent (or fall back to direct connect).
141-
let explicit_agent_id = None;
142-
143-
let kdc_reply_bytes = send_krb_message(
144-
kdc_addr,
145-
&kdc_proxy_message.kerb_message.0.0,
146-
agent_tunnel_handle,
147-
session_id,
148-
explicit_agent_id,
149-
)
150-
.await?;
123+
let kdc_reply_bytes = send_krb_message(kdc_addr, &kdc_proxy_message.kerb_message.0.0).await?;
151124

152125
let reply = KdcProxyMessage::from_raw_kerb_message(&kdc_reply_bytes)
153126
.map_err(HttpError::internal().with_msg("couldn't create KDC proxy reply").err())?;
@@ -157,7 +130,7 @@ async fn forward_to_real_kdc(
157130
reply.to_vec().map_err(HttpError::internal().err())
158131
}
159132

160-
fn enforce_credential_injection_enabled(jet_cred_id: Uuid, enable_unstable: bool) -> Result<(), HttpError> {
133+
fn enforce_credential_injection_enabled(jet_cred_id: uuid::Uuid, enable_unstable: bool) -> Result<(), HttpError> {
161134
if enable_unstable {
162135
return Ok(());
163136
}
@@ -192,33 +165,11 @@ fn enforce_realm_token_match(token_realm: &str, request_realm: &str, bypass: boo
192165
.err()(format!("expected: {token_realm}, got: {request_realm}")))
193166
}
194167

195-
/// Hard ceiling on the announced length of a TCP-framed KDC reply.
196-
///
197-
/// The KDC TCP transport prefixes its message with a 4-byte big-endian length.
198-
/// A misbehaving (or malicious) peer can claim up to `u32::MAX` bytes, which
199-
/// without a cap would have us pre-allocate ~4 GiB on a single reply. 64 KiB
200-
/// is well above any realistic Kerberos reply size while keeping the worst
201-
/// case bounded.
202-
const MAX_KDC_REPLY_MESSAGE_LEN: u32 = 64 * 1024;
203-
204-
async fn read_kdc_reply_message<R: AsyncReadExt + Unpin>(reader: &mut R) -> io::Result<Vec<u8>> {
205-
let len = reader.read_u32().await?;
206-
207-
if len > MAX_KDC_REPLY_MESSAGE_LEN {
208-
return Err(io::Error::new(
209-
io::ErrorKind::InvalidData,
210-
format!("KDC reply too large: announced {len} bytes, maximum is {MAX_KDC_REPLY_MESSAGE_LEN}"),
211-
));
212-
}
213-
214-
let total_len = len
215-
.checked_add(4)
216-
.and_then(|n| usize::try_from(n).ok())
217-
.ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "KDC reply length prefix overflowed"))?;
218-
219-
let mut buf = vec![0; total_len];
220-
buf[0..4].copy_from_slice(&len.to_be_bytes());
221-
reader.read_exact(&mut buf[4..]).await?;
168+
async fn read_kdc_reply_message(connection: &mut TcpStream) -> io::Result<Vec<u8>> {
169+
let len = connection.read_u32().await?;
170+
let mut buf = vec![0; (len + 4).try_into().expect("u32-to-usize")];
171+
buf[0..4].copy_from_slice(&(len.to_be_bytes()));
172+
connection.read_exact(&mut buf[4..]).await?;
222173
Ok(buf)
223174
}
224175

@@ -247,67 +198,7 @@ fn unable_to_reach_kdc_server_err(error: io::Error) -> HttpError {
247198
}
248199

249200
/// Sends the Kerberos message to the specified KDC address.
250-
///
251-
/// Uses the same routing pipeline as connection forwarding:
252-
/// if an agent claims the KDC's domain/subnet, traffic goes through the tunnel.
253-
/// Falls back to direct connect when no agent matches.
254-
///
255-
/// `session_id` is forwarded to the agent as the QUIC stream's session ID for
256-
/// log correlation. Callers that have a parent association (RDP CredSSP) should
257-
/// pass the parent's `jet_aid`; the HTTP `/jet/KdcProxy` endpoint passes the KDC
258-
/// token's own `jti` (no parent association exists for that path).
259-
///
260-
/// `explicit_agent_id` honors the same routing contract as every other proxy path:
261-
/// when the parent association token pins the session to a specific agent via
262-
/// `jet_agent_id`, that pin is enforced here too (route via that agent or fail —
263-
/// do **not** silently fall back to another agent or to direct connect).
264-
/// Callers with no parent association (HTTP `/jet/KdcProxy`) pass `None`.
265-
pub async fn send_krb_message(
266-
kdc_addr: &TargetAddr,
267-
message: &[u8],
268-
agent_tunnel_handle: Option<&agent_tunnel::AgentTunnelHandle>,
269-
session_id: Uuid,
270-
explicit_agent_id: Option<Uuid>,
271-
) -> Result<Vec<u8>, HttpError> {
272-
// Route through agent tunnel using the SAME pipeline as connection forwarding,
273-
// but only for `tcp` KDC targets. The agent tunnel currently has a single
274-
// `ConnectRequest::tcp` shape, so a `udp://` KDC routed this way would be
275-
// delivered to the agent as a TCP target — wrong protocol semantics that can
276-
// silently break UDP Kerberos deployments. Fall through to the direct path
277-
// (which honors the scheme) until an explicit UDP tunnel hop exists.
278-
//
279-
// `as_addr()` returns `host:port` (with IPv6 brackets), which is what the agent
280-
// tunnel target parser expects — unlike `to_string()` which includes the scheme.
281-
let kdc_target = kdc_addr.as_addr();
282-
let tunnel_handle = if kdc_addr.scheme().eq_ignore_ascii_case("tcp") {
283-
agent_tunnel_handle
284-
} else {
285-
None
286-
};
287-
288-
let route_target = match kdc_addr.host_ip() {
289-
Some(ip) => agent_tunnel::routing::RouteTarget::ip(ip),
290-
None => agent_tunnel::routing::RouteTarget::hostname(kdc_addr.host()),
291-
};
292-
293-
if let Some((mut stream, _agent)) =
294-
agent_tunnel::routing::try_route(tunnel_handle, explicit_agent_id, &route_target, session_id, kdc_target)
295-
.await
296-
.map_err(|e| HttpError::bad_gateway().build(format!("KDC routing through agent tunnel failed: {e:#}")))?
297-
{
298-
stream.write_all(message).await.map_err(
299-
HttpError::bad_gateway()
300-
.with_msg("unable to send KDC message through agent tunnel")
301-
.err(),
302-
)?;
303-
304-
return read_kdc_reply_message(&mut stream).await.map_err(
305-
HttpError::bad_gateway()
306-
.with_msg("unable to read KDC reply through agent tunnel")
307-
.err(),
308-
);
309-
}
310-
201+
pub async fn send_krb_message(kdc_addr: &TargetAddr, message: &[u8]) -> Result<Vec<u8>, HttpError> {
311202
let protocol = kdc_addr.scheme();
312203

313204
debug!("Connecting to KDC server located at {kdc_addr} using protocol {protocol}...");
@@ -397,11 +288,11 @@ mod tests {
397288

398289
#[test]
399290
fn credential_injection_gate_allows_jet_cred_id_when_enabled() {
400-
assert!(enforce_credential_injection_enabled(Uuid::new_v4(), true).is_ok());
291+
assert!(enforce_credential_injection_enabled(uuid::Uuid::new_v4(), true).is_ok());
401292
}
402293

403294
#[test]
404295
fn credential_injection_gate_rejects_jet_cred_id_when_disabled() {
405-
assert!(enforce_credential_injection_enabled(Uuid::new_v4(), false).is_err());
296+
assert!(enforce_credential_injection_enabled(uuid::Uuid::new_v4(), false).is_err());
406297
}
407298
}

devolutions-gateway/src/extract.rs

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::DgwState;
88
use crate::http::HttpError;
99
use crate::token::{
1010
AccessScope, AccessTokenClaims, AssociationTokenClaims, BridgeTokenClaims, JmuxTokenClaims, JrecTokenClaims,
11-
JrlTokenClaims, KdcTokenClaims, ScopeTokenClaims, WebAppTokenClaims, extract_jti,
11+
JrlTokenClaims, KdcTokenClaims, ScopeTokenClaims, WebAppTokenClaims,
1212
};
1313

1414
#[derive(Clone)]
@@ -109,13 +109,7 @@ where
109109
/// the path, runs it through the same `authenticate()` routine the middleware would, and
110110
/// unwraps the `Kdc` variant so handlers receive `KdcTokenClaims` directly.
111111
#[derive(Clone)]
112-
pub struct KdcToken {
113-
pub claims: KdcTokenClaims,
114-
/// The KDC token's own `jti`. Carried alongside the claims so the KDC proxy handler can
115-
/// use it as a persistent session-correlation identifier (the JWT standard `jti` claim
116-
/// is not threaded through [`KdcTokenClaims`] itself).
117-
pub jti: uuid::Uuid,
118-
}
112+
pub struct KdcToken(pub KdcTokenClaims);
119113

120114
impl FromRequestParts<DgwState> for KdcToken {
121115
type Rejection = HttpError;
@@ -141,10 +135,7 @@ impl FromRequestParts<DgwState> for KdcToken {
141135
.map_err(HttpError::unauthorized().err())?;
142136

143137
match claims {
144-
AccessTokenClaims::Kdc(claims) => {
145-
let jti = extract_jti(&token).map_err(HttpError::internal().with_msg("KDC token missing jti").err())?;
146-
Ok(Self { claims, jti })
147-
}
138+
AccessTokenClaims::Kdc(claims) => Ok(Self(claims)),
148139
_ => Err(HttpError::forbidden().msg("token not allowed (expected KDC token)")),
149140
}
150141
}

devolutions-gateway/src/generic_client.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,6 @@ where
177177
.client_stream_leftover_bytes(leftover_bytes)
178178
.server_dns_name(selected_target.host().to_owned())
179179
.disconnect_interest(disconnect_interest)
180-
.agent_tunnel_handle(agent_tunnel_handle)
181-
.explicit_agent_id(claims.jet_agent_id)
182180
.build()
183181
.run()
184182
.await

devolutions-gateway/src/rd_clean_path.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -425,9 +425,6 @@ async fn handle_with_credential_injection(
425425
credential_injection_kdc.proxy_credential(),
426426
krb_server_config,
427427
&credential_injection_kdc,
428-
agent_tunnel_handle.as_deref(),
429-
claims.jet_aid,
430-
claims.jet_agent_id,
431428
);
432429

433430
let krb_client_config = if conf.debug.enable_unstable
@@ -451,9 +448,6 @@ async fn handle_with_credential_injection(
451448
server_security_protocol,
452449
credential_injection_kdc.target_credential(),
453450
krb_client_config,
454-
agent_tunnel_handle.as_deref(),
455-
claims.jet_aid,
456-
claims.jet_agent_id,
457451
);
458452

459453
let (client_credssp_res, server_credssp_res) = tokio::join!(client_credssp_fut, server_credssp_fut);

0 commit comments

Comments
 (0)