refactor(dgw): rename KDC service and validate target options at construction

Rename `CredentialService` to `CredentialInjectionKdcService` (it now holds only
the Kerberos session cache, no credentials) and the `DgwState` field likewise.
Validate the KDC scheme when a `TargetConnectionOptions` is built, not only when
deserialized, so in-crate callers can't construct an unsupported address. Document
that `TokenKeyedStore::get` may return an expired-but-not-yet-swept entry.

Author: irvingoujAtDevolution

devolutions-gateway/src/api/kdc_proxy.rs

@@ -22,7 +22,7 @@ pub fn make_router<S>(state: DgwState) -> Router<S> {
 async fn kdc_proxy(
     State(DgwState {
         conf_handle,
-        credentials,
+        credential_injection,
         provisioning,
         agent_tunnel_handle,
         ..
@@ -48,7 +48,7 @@ async fn kdc_proxy(
         KdcDestination::Inject { jti } => {
             enforce_credential_injection_enabled(jti, conf.debug.enable_unstable)?;
 
-            let kdc = credentials
+            let kdc = credential_injection
                 .kdc_for(&provisioning, jti)
                 .map_err(credential_injection_resolve_error)?;
 

devolutions-gateway/src/api/rdp.rs

@@ -25,7 +25,7 @@ pub async fn handler(
         subscriber_tx,
         recordings,
         shutdown_signal,
-        credentials,
+        credential_injection,
         provisioning,
         agent_tunnel_handle,
         ..
@@ -47,7 +47,7 @@ pub async fn handler(
             subscriber_tx,
             recordings.active_recordings,
             source_addr,
-            credentials,
+            credential_injection,
             provisioning,
             agent_tunnel_handle,
         )
@@ -68,7 +68,7 @@ async fn handle_socket(
     subscriber_tx: SubscriberSender,
     active_recordings: Arc<ActiveRecordings>,
     source_addr: SocketAddr,
-    credentials: crate::credential_injection_kdc::CredentialService,
+    credential_injection: crate::credential_injection_kdc::CredentialInjectionKdcService,
     provisioning: crate::provisioning::ProvisioningStore,
     agent_tunnel_handle: Option<Arc<agent_tunnel::AgentTunnelHandle>>,
 ) {
@@ -87,7 +87,7 @@ async fn handle_socket(
         sessions,
         subscriber_tx,
         &active_recordings,
-        &credentials,
+        &credential_injection,
         &provisioning,
         agent_tunnel_handle,
     )

devolutions-gateway/src/credential_injection_kdc.rs

@@ -165,7 +165,7 @@ impl CredentialInjectionKdc {
     }
 
     pub(crate) fn krb_kdc(&self) -> Option<&crate::target_addr::TargetAddr> {
-        self.connection_options.as_ref()?.krb_kdc.as_ref()
+        self.connection_options.as_ref()?.krb_kdc()
     }
 
     /// Selects the CredSSP acceptor backend Gateway should present to the RDP client.
@@ -445,7 +445,7 @@ fn random_32_bytes() -> Vec<u8> {
 /// live here, and the two are keyed by the same JTI but never reach into each other. Resolution
 /// reads the provisioning store passed in by the caller.
 #[derive(Debug, Clone)]
-pub struct CredentialService {
+pub struct CredentialInjectionKdcService {
     sessions: Arc<Mutex<HashMap<Uuid, CachedSession>>>,
 }
 
@@ -460,13 +460,13 @@ struct CachedSession {
     session: Arc<CredentialInjectionKdcSession>,
 }
 
-impl Default for CredentialService {
+impl Default for CredentialInjectionKdcService {
     fn default() -> Self {
         Self::new()
     }
 }
 
-impl CredentialService {
+impl CredentialInjectionKdcService {
     pub fn new() -> Self {
         Self {
             sessions: Arc::new(Mutex::new(HashMap::new())),
@@ -541,11 +541,8 @@ impl CredentialService {
             })?;
 
         let proxy_username = app_credential_username(&mapping.proxy).to_owned();
-        // Atomic get-or-derive under the lock: guarantees a single session wins per JTI even under
-        // concurrent calls. A cached session is reused only when it was derived for this exact
-        // provisioning entry — a re-provisioning produces a new entry, so its stale session is
-        // dropped and re-derived here. Derivation is fast (a few hundred bytes of OsRng), so holding
-        // the lock across it is acceptable.
+        // Hold the lock across derive so one session wins per JTI; reuse only if the cached session
+        // was derived for this exact entry (re-provisioning yields a new entry, forcing a re-derive).
         let session = {
             let mut sessions = self.sessions.lock();
             let reuse = sessions.get(&jti).and_then(|cached| {
@@ -584,7 +581,7 @@ impl CredentialService {
 }
 
 pub struct CleanupTask {
-    pub service: CredentialService,
+    pub service: CredentialInjectionKdcService,
 }
 
 #[async_trait]
@@ -600,7 +597,7 @@ impl Task for CleanupTask {
 }
 
 #[instrument(skip_all)]
-async fn cleanup_task(service: CredentialService, mut shutdown_signal: ShutdownSignal) {
+async fn cleanup_task(service: CredentialInjectionKdcService, mut shutdown_signal: ShutdownSignal) {
     use tokio::time::{Duration, sleep};
 
     const TASK_INTERVAL: Duration = Duration::from_secs(60 * 15); // 15 minutes
@@ -719,7 +716,7 @@ mod tests {
 
     #[test]
     fn service_kdc_for_rejects_expired_credential_entry() {
-        let service = CredentialService::new();
+        let service = CredentialInjectionKdcService::new();
         let jti = Uuid::new_v4();
 
         // Negative TTL: entry is born already expired. `TokenKeyedStore::get` does not
@@ -738,7 +735,7 @@ mod tests {
 
     #[test]
     fn service_kdc_for_returns_same_session_under_concurrent_calls() {
-        let service = CredentialService::new();
+        let service = CredentialInjectionKdcService::new();
         let jti = Uuid::new_v4();
         let provisioning = provisioned_store(jti, "target", time::Duration::minutes(5));
 
@@ -758,7 +755,7 @@ mod tests {
 
     #[test]
     fn service_kdc_for_ignores_cached_session_from_a_gone_entry() {
-        let service = CredentialService::new();
+        let service = CredentialInjectionKdcService::new();
         let jti = Uuid::new_v4();
 
         // Simulate the race called out by Codex: a previous provisioning's session is still cached,
@@ -784,7 +781,7 @@ mod tests {
 
     #[test]
     fn service_kdc_for_rederives_session_after_reprovisioning() {
-        let service = CredentialService::new();
+        let service = CredentialInjectionKdcService::new();
         let jti = Uuid::new_v4();
         let provisioning = provisioned_store(jti, "target", time::Duration::minutes(5));
 
@@ -816,7 +813,7 @@ mod tests {
 
     #[test]
     fn service_sweep_orphans_drops_sessions_with_no_credential_entry() {
-        let service = CredentialService::new();
+        let service = CredentialInjectionKdcService::new();
         let jti = Uuid::new_v4();
 
         {
@@ -889,7 +886,7 @@ mod tests {
 
     #[test]
     fn service_kdc_for_rejects_unknown_jti() {
-        let service = CredentialService::new();
+        let service = CredentialInjectionKdcService::new();
         let provisioning = ProvisioningStore::new();
 
         assert!(
@@ -903,7 +900,7 @@ mod tests {
 
     #[test]
     fn service_kdc_for_rejects_non_injection_entry() {
-        let service = CredentialService::new();
+        let service = CredentialInjectionKdcService::new();
         let provisioning = ProvisioningStore::new();
         let jti = Uuid::new_v4();
 
@@ -922,7 +919,7 @@ mod tests {
 
     #[test]
     fn service_kdc_for_lazily_extracts_target_hostname_from_entry_token() {
-        let service = CredentialService::new();
+        let service = CredentialInjectionKdcService::new();
         let jti = Uuid::new_v4();
         let provisioning = provisioned_store(jti, "target", time::Duration::minutes(5));
 
@@ -936,7 +933,7 @@ mod tests {
     #[test]
     fn provisioning_and_credential_service_share_target_options() {
         let provisioning = ProvisioningStore::new();
-        let service = CredentialService::new();
+        let service = CredentialInjectionKdcService::new();
         let jti = Uuid::new_v4();
         let token = association_token(jti);
         let krb_kdc = crate::target_addr::TargetAddr::parse("tcp://kdc.example.invalid:88", Some(88))
@@ -946,9 +943,7 @@ mod tests {
             .insert(
                 token.clone(),
                 Some(cleartext_mapping_with_target_username("target@example.invalid")),
-                Some(TargetConnectionOptions {
-                    krb_kdc: Some(krb_kdc.clone()),
-                }),
+                Some(TargetConnectionOptions::new(Some(krb_kdc.clone())).expect("supported KDC scheme")),
                 time::Duration::minutes(5),
             )
             .expect("provisioning entry inserts");
@@ -959,7 +954,7 @@ mod tests {
                 .value
                 .connection_options
                 .as_ref()
-                .and_then(|options| options.krb_kdc.as_ref()),
+                .and_then(|options| options.krb_kdc()),
             Some(&krb_kdc)
         );
 

devolutions-gateway/src/generic_client.rs

@@ -8,7 +8,7 @@ use tracing::field;
 use typed_builder::TypedBuilder;
 
 use crate::config::Conf;
-use crate::credential_injection_kdc::CredentialService;
+use crate::credential_injection_kdc::CredentialInjectionKdcService;
 use crate::provisioning::ProvisioningStore;
 use crate::proxy::Proxy;
 use crate::rdp_pcb::{extract_association_claims, read_pcb};
@@ -28,7 +28,7 @@ pub struct GenericClient<S> {
     sessions: SessionMessageSender,
     subscriber_tx: SubscriberSender,
     active_recordings: Arc<ActiveRecordings>,
-    credentials: CredentialService,
+    credential_injection: CredentialInjectionKdcService,
     provisioning: ProvisioningStore,
     #[builder(default)]
     agent_tunnel_handle: Option<Arc<AgentTunnelHandle>>,
@@ -53,7 +53,7 @@ where
             sessions,
             subscriber_tx,
             active_recordings,
-            credentials,
+            credential_injection,
             provisioning,
             agent_tunnel_handle,
         } = self;
@@ -156,7 +156,7 @@ where
                 // lookup by `claims.jti` is the primary path.
                 if is_rdp
                     && let Some(credential_injection_kdc) =
-                        credentials.resolve_injection_kdc(&provisioning, claims.jti, token)?
+                        credential_injection.resolve_injection_kdc(&provisioning, claims.jti, token)?
                 {
                     info!(
                         jti = %credential_injection_kdc.jti(),

devolutions-gateway/src/lib.rs

@@ -65,7 +65,7 @@ pub struct DgwState {
     pub recordings: recording::RecordingMessageSender,
     pub job_queue_handle: job_queue::JobQueueHandle,
     pub provisioning: provisioning::ProvisioningStore,
-    pub credentials: credential_injection_kdc::CredentialService,
+    pub credential_injection: credential_injection_kdc::CredentialInjectionKdcService,
     pub monitoring_state: Arc<network_monitor::State>,
     pub traffic_audit_handle: traffic_audit::TrafficAuditHandle,
     pub agent_tunnel_handle: Option<Arc<agent_tunnel::AgentTunnelHandle>>,
@@ -94,7 +94,7 @@ impl DgwState {
         let (job_queue_handle, job_queue_rx) = job_queue::JobQueueHandle::new();
         let (traffic_audit_handle, traffic_audit_rx) = traffic_audit::TrafficAuditHandle::new();
         let provisioning = provisioning::ProvisioningStore::new();
-        let credentials = credential_injection_kdc::CredentialService::new();
+        let credential_injection = credential_injection_kdc::CredentialInjectionKdcService::new();
         let monitoring_state = Arc::new(network_monitor::State::new(Arc::new(MockMonitorsCache))?);
 
         let state = Self {
@@ -108,7 +108,7 @@ impl DgwState {
             job_queue_handle,
             traffic_audit_handle,
             provisioning,
-            credentials,
+            credential_injection,
             monitoring_state,
             agent_tunnel_handle: None,
         };

devolutions-gateway/src/listener.rs

@@ -158,7 +158,7 @@ async fn handle_tcp_peer(stream: TcpStream, state: DgwState, peer_addr: SocketAd
                 .sessions(state.sessions)
                 .subscriber_tx(state.subscriber_tx)
                 .active_recordings(state.recordings.active_recordings)
-                .credentials(state.credentials)
+                .credential_injection(state.credential_injection)
                 .provisioning(state.provisioning)
                 .agent_tunnel_handle(state.agent_tunnel_handle)
                 .build()

devolutions-gateway/src/ngrok.rs

@@ -237,7 +237,7 @@ async fn run_tcp_tunnel(mut tunnel: ngrok::tunnel::TcpTunnel, state: DgwState) {
                         .sessions(state.sessions)
                         .subscriber_tx(state.subscriber_tx)
                         .active_recordings(state.recordings.active_recordings)
-                        .credentials(state.credentials)
+                        .credential_injection(state.credential_injection)
                         .provisioning(state.provisioning)
                         .agent_tunnel_handle(state.agent_tunnel_handle)
                         .build()

devolutions-gateway/src/rd_clean_path.rs

@@ -11,7 +11,7 @@ use tokio::io::{AsyncRead, AsyncReadExt as _, AsyncWrite, AsyncWriteExt as _};
 use tracing::field;
 
 use crate::config::Conf;
-use crate::credential_injection_kdc::{CredentialInjectionKdc, CredentialService};
+use crate::credential_injection_kdc::{CredentialInjectionKdc, CredentialInjectionKdcService};
 use crate::provisioning::ProvisioningStore;
 use crate::proxy::Proxy;
 use crate::recording::ActiveRecordings;
@@ -521,7 +521,7 @@ pub async fn handle(
     sessions: SessionMessageSender,
     subscriber_tx: SubscriberSender,
     active_recordings: &ActiveRecordings,
-    credentials: &CredentialService,
+    credential_injection: &CredentialInjectionKdcService,
     provisioning: &ProvisioningStore,
     agent_tunnel_handle: Option<Arc<agent_tunnel::AgentTunnelHandle>>,
 ) -> anyhow::Result<()> {
@@ -543,7 +543,7 @@ pub async fn handle(
     // proxy-based credential injection mode. Otherwise, we continue the usual
     // clean path procedure. The provisioning store is keyed on the association token's JTI.
     if let Some(jti) = crate::token::extract_jti(token).ok()
-        && let Some(credential_injection_kdc) = credentials.resolve_injection_kdc(provisioning, jti, token)?
+        && let Some(credential_injection_kdc) = credential_injection.resolve_injection_kdc(provisioning, jti, token)?
     {
         debug!(
             jti = %credential_injection_kdc.jti(),

devolutions-gateway/src/service.rs

@@ -268,7 +268,7 @@ async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result<Tasks> {
             .context("failed to initialize traffic audit manager")?;
 
     let provisioning = devolutions_gateway::provisioning::ProvisioningStore::new();
-    let credentials = devolutions_gateway::credential_injection_kdc::CredentialService::new();
+    let credential_injection = devolutions_gateway::credential_injection_kdc::CredentialInjectionKdcService::new();
 
     let filesystem_monitor_config_cache = devolutions_gateway::api::monitoring::FilesystemConfigCache::new(
         config::get_data_dir().join("monitors_cache.json"),
@@ -317,7 +317,7 @@ async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result<Tasks> {
         recordings: recording_manager_handle.clone(),
         job_queue_handle: job_queue_ctx.job_queue_handle.clone(),
         provisioning: provisioning.clone(),
-        credentials: credentials.clone(),
+        credential_injection: credential_injection.clone(),
         monitoring_state,
         traffic_audit_handle: traffic_audit_task.handle(),
         agent_tunnel_handle,
@@ -354,7 +354,9 @@ async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result<Tasks> {
 
     tasks.register(provisioning.cleanup_task());
 
-    tasks.register(devolutions_gateway::credential_injection_kdc::CleanupTask { service: credentials });
+    tasks.register(devolutions_gateway::credential_injection_kdc::CleanupTask {
+        service: credential_injection,
+    });
 
     tasks.register(devolutions_log::LogDeleterTask::<GatewayLog>::new(
         conf.log_file.clone(),