From a800a3670e4955769cab4b6b0951d2f29206834e Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Wed, 29 Jul 2026 23:06:08 -0400 Subject: [PATCH 01/12] refactor(dgw): CredSSP enclosure, supermarket store, thin synthetic KDC Split rdp_proxy CredSSP into its own module. Delete CredentialService: DgwState holds ProvisioningStore + SyntheticKdcRegistry. from_provisioned builds PreparedCredentialInjection; register_if_kerberos publishes. take() consumes groceries once. Synthetic KDC keeps only fake-KDC runtime; credentials and target_kdc stay on the dish. --- devolutions-gateway/src/api/kdc_proxy.rs | 22 +- devolutions-gateway/src/api/preflight.rs | 15 +- devolutions-gateway/src/api/rdp.rs | 12 +- .../src/credential_injection.rs | 811 +++++++++++++ .../src/credential_injection_kdc.rs | 1073 ----------------- devolutions-gateway/src/generic_client.rs | 21 +- devolutions-gateway/src/lib.rs | 11 +- devolutions-gateway/src/listener.rs | 3 +- devolutions-gateway/src/ngrok.rs | 3 +- devolutions-gateway/src/provisioning.rs | 64 +- devolutions-gateway/src/rd_clean_path.rs | 38 +- devolutions-gateway/src/rdp_proxy.rs | 846 ------------- devolutions-gateway/src/rdp_proxy/credssp.rs | 571 +++++++++ devolutions-gateway/src/rdp_proxy/mod.rs | 278 +++++ devolutions-gateway/src/service.rs | 12 +- 15 files changed, 1762 insertions(+), 2018 deletions(-) create mode 100644 devolutions-gateway/src/credential_injection.rs delete mode 100644 devolutions-gateway/src/credential_injection_kdc.rs delete mode 100644 devolutions-gateway/src/rdp_proxy.rs create mode 100644 devolutions-gateway/src/rdp_proxy/credssp.rs create mode 100644 devolutions-gateway/src/rdp_proxy/mod.rs diff --git a/devolutions-gateway/src/api/kdc_proxy.rs b/devolutions-gateway/src/api/kdc_proxy.rs index 865fc369f..ad6aebd8c 100644 --- a/devolutions-gateway/src/api/kdc_proxy.rs +++ b/devolutions-gateway/src/api/kdc_proxy.rs @@ -5,9 +5,8 @@ use picky_krb::messages::KdcProxyMessage; use uuid::Uuid; use crate::DgwState; -use crate::credential_injection_kdc::{ - CredentialInjectionKdcInterception, CredentialInjectionKdcRequest, CredentialInjectionKdcResolveError, - kdc_proxy_message_realm, +use crate::credential_injection::{ + CredentialInjectionKdcInterception, CredentialInjectionKdcRequest, kdc_proxy_message_realm, }; use crate::extract::KdcToken; use crate::http::HttpError; @@ -22,7 +21,7 @@ pub fn make_router(state: DgwState) -> Router { async fn kdc_proxy( State(DgwState { conf_handle, - credentials, + synthetic_kdc_registry, agent_tunnel_handle, .. }): State, @@ -47,7 +46,9 @@ async fn kdc_proxy( KdcDestination::Inject { jti } => { enforce_credential_injection_enabled(jti, conf.debug.enable_unstable)?; - let kdc = credentials.kdc_for(jti).map_err(credential_injection_resolve_error)?; + let kdc = synthetic_kdc_registry + .get(jti) + .ok_or_else(|| HttpError::bad_request().msg("no live synthetic KDC published for this session"))?; debug!( jti = %kdc.jti(), @@ -92,17 +93,6 @@ async fn kdc_proxy( } } -fn credential_injection_resolve_error(error: CredentialInjectionKdcResolveError) -> HttpError { - match error { - CredentialInjectionKdcResolveError::BuildKdcConfig { .. } => HttpError::internal() - .with_msg("credential-injection KDC could not be initialized") - .build(error), - _ => HttpError::bad_request() - .with_msg("credential-injection state is not available") - .build(error), - } -} - // Forwards the request to the real KDC indicated by the token (or by the debug override) and // returns the response wrapped as a `KdcProxyMessage`. // diff --git a/devolutions-gateway/src/api/preflight.rs b/devolutions-gateway/src/api/preflight.rs index 5598d44ec..1477f737d 100644 --- a/devolutions-gateway/src/api/preflight.rs +++ b/devolutions-gateway/src/api/preflight.rs @@ -11,10 +11,9 @@ use uuid::Uuid; use crate::DgwState; use crate::config::Conf; -use crate::credential_injection_kdc::CredentialService; use crate::extract::PreflightScope; use crate::http::HttpError; -use crate::provisioning::InsertError; +use crate::provisioning::{InsertError, ProvisioningStore}; use crate::session::SessionMessageSender; const OP_GET_VERSION: &str = "get-version"; @@ -204,7 +203,7 @@ pub(super) async fn post_preflight( State(DgwState { conf_handle, sessions, - credentials, + provisioning, .. }): State, _scope: PreflightScope, @@ -231,13 +230,13 @@ pub(super) async fn post_preflight( let outputs = outputs.clone(); let conf = conf_handle.get_conf(); let sessions = sessions.clone(); - let credentials = credentials.clone(); + let provisioning = provisioning.clone(); async move { let operation_id = operation.id; trace!(%operation.id, "Process preflight operation"); - if let Err(error) = handle_operation(operation, &outputs, &conf, &sessions, &credentials).await { + if let Err(error) = handle_operation(operation, &outputs, &conf, &sessions, &provisioning).await { outputs.push(PreflightOutput { operation_id, kind: PreflightOutputKind::Alert { @@ -264,7 +263,7 @@ async fn handle_operation( outputs: &Outputs, conf: &Conf, sessions: &SessionMessageSender, - credentials: &CredentialService, + provisioning: &ProvisioningStore, ) -> Result<(), PreflightError> { match operation.kind.as_str() { OP_GET_VERSION => outputs.push(PreflightOutput { @@ -355,7 +354,7 @@ async fn handle_operation( })?; } - let replaced = credentials + let replaced = provisioning .insert_credentials(token, mapping, time_to_live) .inspect_err(|error| warn!(%operation.id, error = format!("{error:#}"), "Failed to insert credentials")) .map_err(|error| match error { @@ -397,7 +396,7 @@ async fn handle_operation( PreflightError::new(PreflightAlertStatus::InvalidParams, format!("invalid token: {error:#}")) })?; - let replaced = credentials.insert_connection_options(jti, connection_options, time_to_live); + let replaced = provisioning.insert_connection_options(jti, connection_options, time_to_live); if replaced { outputs.push(PreflightOutput { diff --git a/devolutions-gateway/src/api/rdp.rs b/devolutions-gateway/src/api/rdp.rs index b3d45dbcb..29e5d161e 100644 --- a/devolutions-gateway/src/api/rdp.rs +++ b/devolutions-gateway/src/api/rdp.rs @@ -25,7 +25,8 @@ pub async fn handler( subscriber_tx, recordings, shutdown_signal, - credentials, + provisioning, + synthetic_kdc_registry, agent_tunnel_handle, .. }): State, @@ -46,7 +47,8 @@ pub async fn handler( subscriber_tx, recordings.active_recordings, source_addr, - credentials, + provisioning, + synthetic_kdc_registry, agent_tunnel_handle, ) .instrument(span) @@ -66,7 +68,8 @@ async fn handle_socket( subscriber_tx: SubscriberSender, active_recordings: Arc, source_addr: SocketAddr, - credentials: crate::credential_injection_kdc::CredentialService, + provisioning: crate::provisioning::ProvisioningStore, + synthetic_kdc_registry: crate::credential_injection::SyntheticKdcRegistry, agent_tunnel_handle: Option>, ) { let (stream, close_handle) = crate::ws::handle( @@ -84,7 +87,8 @@ async fn handle_socket( sessions, subscriber_tx, &active_recordings, - &credentials, + &provisioning, + &synthetic_kdc_registry, agent_tunnel_handle, ) .await; diff --git a/devolutions-gateway/src/credential_injection.rs b/devolutions-gateway/src/credential_injection.rs new file mode 100644 index 000000000..7a2341a7f --- /dev/null +++ b/devolutions-gateway/src/credential_injection.rs @@ -0,0 +1,811 @@ +//! Credential-injection runtime: groceries → dish → synthetic KDC pass window. +//! +//! - Provisioned data lives in [`crate::provisioning::ProvisioningStore`] (supermarket). +//! - [`CredentialInjection`] is built by the RDP path from those groceries (chef). +//! - [`SyntheticKdcRegistry`] is the pass window: RDP publishes, `/jet/KdcProxy` looks up only. + +use std::collections::HashMap; +use std::fmt; +use std::net::SocketAddr; +use std::sync::Arc; +use std::time::Duration; + +use anyhow::Context as _; +use chacha20poly1305::aead::OsRng; +use chacha20poly1305::aead::rand_core::RngCore as _; +use ironrdp_connector::sspi; +use ironrdp_connector::sspi::generator::NetworkRequest; +use parking_lot::Mutex; +use picky_krb::messages::KdcProxyMessage; +use secrecy::{ExposeSecret as _, SecretBox, SecretString}; +use thiserror::Error; +use url::Url; +use uuid::Uuid; + +use crate::credential::{AppCredential, AppCredentialMapping}; +use crate::provisioning::ProvisioningEntry; +#[cfg(test)] +use crate::provisioning::ProvisioningStore; + +// The reserved `.invalid` TLD (RFC 6761) lets sspi-rs CredSSP server emit "KDC requests" that +// never leave the process: `intercept_network_request` recognises this hostname and dispatches +// the message into the in-process `kdc` server below. +// +// TODO(sspi-rs#664): replace this URL-trampoline with a pluggable KDC dispatcher trait once +// sspi-rs ships the API — see https://github.com/Devolutions/sspi-rs/issues/664. +const IN_PROCESS_KDC_HOST: &str = "cred.invalid"; + +/// In-process synthetic KDC for one Kerberos credential-injection session. +/// +/// Published to [`SyntheticKdcRegistry`] for `/jet/KdcProxy`. Holds only what the fake KDC and +/// CredSSP server-leg intercept need — not proxy/target passwords or routing bags. +pub(crate) struct CredentialInjectionKdc { + jti: Uuid, + target_hostname: String, + realm: String, + acceptor_principal_name: String, + acceptor_password: SecretString, + acceptor_long_term_key: SecretBox>, + // Built once from acceptor + proxy material; kdc crate API takes this by ref on each message. + kdc_config: kdc::config::KerberosServer, +} + +#[derive(Debug, Error)] +pub(crate) enum CredentialInjectionKdcResolveError { + #[error("credential-injection state is not available for {jti}")] + NonInjectionCredential { jti: Uuid }, + #[error("association token for {jti} is not valid for credential injection")] + InvalidAssociationToken { + jti: Uuid, + #[source] + source: anyhow::Error, + }, + #[error("credential-injection KDC config could not be initialized for {jti}")] + BuildKdcConfig { + jti: Uuid, + #[source] + source: anyhow::Error, + }, + #[error("Kerberos credential injection requires target connection option krb_kdc for {jti}")] + MissingKrbKdc { jti: Uuid }, +} + +#[derive(Debug, Clone, PartialEq, Eq, Error)] +#[error("expected: {expected}, got: {actual}")] +pub(crate) struct RealmMismatch { + pub(crate) expected: String, + pub(crate) actual: String, +} + +#[derive(Debug)] +pub(crate) enum CredentialInjectionKdcInterception { + Intercepted(Vec), + NotInjectionRequest, + NotInjectionRealm(RealmMismatch), +} + +/// Session-scoped credential injection. Holding [`CredentialInjection::Kerberos`] proves the +/// synthetic KDC is registered in [`SyntheticKdcRegistry`] for this connection. +/// +/// Build path: [`CredentialInjection::from_provisioned`] → [`PreparedCredentialInjection`] → +/// [`PreparedCredentialInjection::register_if_kerberos`]. +pub(crate) enum CredentialInjection { + Kerberos( + KerberosCredentialInjection, + #[expect(dead_code, reason = "RAII lease: Drop unpublishes the synthetic KDC")] SyntheticKdcRegistration, + ), + Ntlm(NtlmCredentialInjection), +} + +/// Kerberos dish: credentials + real KDC address + shared synthetic KDC. +pub(crate) struct KerberosCredentialInjection { + credential_mapping: AppCredentialMapping, + target_kdc: Url, + synthetic: Arc, +} + +/// Chef output: protocol chosen; synthetic KDC built if needed, not yet published. +#[derive(Debug)] +pub(crate) enum PreparedCredentialInjection { + Kerberos(KerberosCredentialInjection), + Ntlm(NtlmCredentialInjection), +} + +impl PreparedCredentialInjection { + /// Publish the synthetic KDC when this is Kerberos; NTLM is a no-op pass-through. + pub(crate) fn register_if_kerberos(self, registry: &SyntheticKdcRegistry) -> CredentialInjection { + match self { + Self::Kerberos(injection) => { + let registration = registry.register(Arc::clone(&injection.synthetic)); + debug!( + jti = %injection.synthetic.jti(), + "registered synthetic KDC for credential-injection session" + ); + CredentialInjection::Kerberos(injection, registration) + } + Self::Ntlm(injection) => CredentialInjection::Ntlm(injection), + } + } +} + +impl KerberosCredentialInjection { + pub(crate) fn synthetic_kdc(&self) -> &CredentialInjectionKdc { + &self.synthetic + } + + pub(crate) fn target_kdc(&self) -> &Url { + &self.target_kdc + } +} + +impl fmt::Debug for KerberosCredentialInjection { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("KerberosCredentialInjection") + .field("target_kdc", &self.target_kdc) + .field("synthetic", &self.synthetic) + .finish_non_exhaustive() + } +} + +/// NTLM injection carries credentials only — no synthetic KDC is published. +#[derive(Debug)] +pub(crate) struct NtlmCredentialInjection { + jti: Uuid, + credential_mapping: AppCredentialMapping, +} + +impl NtlmCredentialInjection { + pub(crate) fn jti(&self) -> Uuid { + self.jti + } + + pub(crate) fn proxy_credential(&self) -> &AppCredential { + &self.credential_mapping.proxy + } + + pub(crate) fn target_credential(&self) -> &AppCredential { + &self.credential_mapping.target + } +} + +impl CredentialInjection { + pub(crate) fn jti(&self) -> Uuid { + match self { + Self::Kerberos(k, _) => k.synthetic.jti(), + Self::Ntlm(ntlm) => ntlm.jti(), + } + } + + pub(crate) fn proxy_credential(&self) -> &AppCredential { + match self { + Self::Kerberos(k, _) => &k.credential_mapping.proxy, + Self::Ntlm(ntlm) => ntlm.proxy_credential(), + } + } + + pub(crate) fn target_credential(&self) -> &AppCredential { + match self { + Self::Kerberos(k, _) => &k.credential_mapping.target, + Self::Ntlm(ntlm) => ntlm.target_credential(), + } + } + + pub(crate) fn as_kerberos(&self) -> Option<&KerberosCredentialInjection> { + match self { + Self::Kerberos(k, _) => Some(k), + Self::Ntlm(_) => None, + } + } + + pub(crate) fn uses_kerberos(&self) -> bool { + matches!(self, Self::Kerberos(_, _)) + } + + /// RDP chef: owned groceries → prepared dish. Does not touch the registry. + pub(crate) fn from_provisioned( + jti: Uuid, + credential_entry: ProvisioningEntry, + kerberos_enabled: bool, + ) -> Result { + let ProvisioningEntry { + token, + mapping, + connection_options, + } = credential_entry; + + let mapping = mapping.ok_or_else(|| { + warn!(%jti, "credential-injection state has no mapping"); + CredentialInjectionKdcResolveError::NonInjectionCredential { jti } + })?; + + let target_hostname = crate::token::extract_credential_injection_target_hostname(&token).map_err(|source| { + warn!( + %jti, + error = format!("{source:#}"), + "invalid credential-injection association token" + ); + CredentialInjectionKdcResolveError::InvalidAssociationToken { jti, source } + })?; + + let target_username = match sspi::Username::parse(app_credential_username(&mapping.target)) { + Ok(u) => u, + Err(error) => { + warn!(%jti, error = format!("{error:#}"), "invalid target credential username"); + return Err(CredentialInjectionKdcResolveError::BuildKdcConfig { + jti, + source: anyhow::anyhow!("invalid target credential username: {error}"), + }); + } + }; + + let wants_kerberos = kerberos_enabled && target_username.domain_name().is_some(); + if !wants_kerberos { + return Ok(PreparedCredentialInjection::Ntlm(NtlmCredentialInjection { + jti, + credential_mapping: mapping, + })); + } + + let target_kdc = connection_options + .as_ref() + .and_then(|o| o.krb_kdc()) + .cloned() + .ok_or_else(|| { + warn!(%jti, "Kerberos credential injection requires krb_kdc"); + CredentialInjectionKdcResolveError::MissingKrbKdc { jti } + })?; + + let proxy_username = app_credential_username(&mapping.proxy).to_owned(); + let synthetic = CredentialInjectionKdc::new(jti, target_hostname, &proxy_username, &mapping.proxy) + .map_err(|source| CredentialInjectionKdcResolveError::BuildKdcConfig { jti, source })?; + + Ok(PreparedCredentialInjection::Kerberos(KerberosCredentialInjection { + credential_mapping: mapping, + target_kdc, + synthetic: Arc::new(synthetic), + })) + } +} + +pub(crate) struct CredentialInjectionKdcRequest { + message: KdcProxyMessage, +} + +impl CredentialInjectionKdcRequest { + pub(crate) fn from_token(message: KdcProxyMessage) -> Self { + Self { message } + } + + fn in_process(message: KdcProxyMessage) -> Self { + Self { message } + } +} + +impl fmt::Debug for CredentialInjectionKdc { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("CredentialInjectionKdc") + .field("jti", &self.jti) + .field("target_hostname", &self.target_hostname) + .field("realm", &self.realm) + .field("kdc_config", &"") + .finish() + } +} + +impl CredentialInjectionKdc { + fn new( + jti: Uuid, + target_hostname: String, + proxy_username: &str, + proxy_credential: &AppCredential, + ) -> anyhow::Result { + let realm = realm_from_proxy_username(proxy_username, jti); + let acceptor_principal_name = "jet".to_owned(); + let acceptor_password = SecretString::from(hex::encode(random_32_bytes())); + let acceptor_long_term_key = SecretBox::new(Box::new(random_32_bytes())); + let krbtgt_key = random_32_bytes(); + + let kdc_config = build_kdc_config( + &realm, + proxy_credential, + &acceptor_principal_name, + acceptor_password.expose_secret(), + &krbtgt_key, + acceptor_long_term_key.expose_secret(), + )?; + + Ok(Self { + jti, + target_hostname, + realm, + acceptor_principal_name, + acceptor_password, + acceptor_long_term_key, + kdc_config, + }) + } + + pub(crate) fn jti(&self) -> Uuid { + self.jti + } + + pub(crate) fn server_kerberos_config(&self, client_addr: SocketAddr) -> anyhow::Result { + let user = sspi::CredentialsBuffers::AuthIdentity(sspi::AuthIdentityBuffers::from_utf8( + &self.acceptor_principal_name, + &self.realm, + self.acceptor_password.expose_secret(), + )); + + let kdc_url = self.in_process_kdc_url()?; + + // The SPN that the client puts on its AP-REQ ticket is the one for the target RDP + // server (`TERMSRV/`). Gateway-as-CredSSP-server is impersonating that target, + // so ServerProperties must claim the same SPN or sspi-rs rejects the ticket. + Ok(sspi::KerberosServerConfig { + kerberos_config: sspi::KerberosConfig { + kdc_url: Some(kdc_url), + client_computer_name: client_addr.to_string(), + }, + server_properties: sspi::kerberos::ServerProperties::new( + &["TERMSRV", &self.target_hostname], + Some(user), + Duration::from_secs(300), + Some(sspi::Secret::new(self.acceptor_long_term_key.expose_secret().clone())), + )?, + }) + } + + pub(crate) fn intercept_network_request( + &self, + request: &NetworkRequest, + ) -> anyhow::Result { + if request.url.host_str() != Some(IN_PROCESS_KDC_HOST) { + return Ok(CredentialInjectionKdcInterception::NotInjectionRequest); + } + + let url_jti = request + .url + .path() + .trim_start_matches('/') + .parse::() + .context("malformed in-process KDC URL")?; + anyhow::ensure!( + url_jti == self.jti, + "in-process KDC URL JTI does not match current CredSSP session", + ); + + debug!( + jti = %self.jti, + scheme = %request.url.scheme(), + "Credential-injection KDC intercepted in-process request" + ); + + let kdc_message = KdcProxyMessage::from_raw(&request.data).context("malformed in-process KDC proxy payload")?; + self.handle_kdc_proxy_request(CredentialInjectionKdcRequest::in_process(kdc_message)) + } + + pub(crate) fn handle_kdc_proxy_request( + &self, + request: CredentialInjectionKdcRequest, + ) -> anyhow::Result { + let request_realm = self.resolve_message_realm(&request.message); + debug!( + jti = %self.jti, + resolved_realm = %request_realm, + "Credential-injection KDC realm resolved" + ); + + if let Some(mismatch) = realm_mismatch(&self.realm, &request_realm) { + return Ok(CredentialInjectionKdcInterception::NotInjectionRealm(mismatch)); + } + + let reply = self.handle_message(request.message)?; + Ok(CredentialInjectionKdcInterception::Intercepted(reply)) + } + + fn in_process_kdc_url(&self) -> anyhow::Result { + Url::parse(&format!("http://{}/{}", IN_PROCESS_KDC_HOST, self.jti)).context("build in-process KDC URL") + } + + fn resolve_message_realm(&self, kdc_proxy_message: &KdcProxyMessage) -> String { + kdc_proxy_message_realm(kdc_proxy_message).unwrap_or_else(|| self.realm.clone()) + } + + fn handle_message(&self, kdc_proxy_message: KdcProxyMessage) -> anyhow::Result> { + let reply = kdc::handle_kdc_proxy_message(kdc_proxy_message, &self.kdc_config, &self.target_hostname) + .context("handle credential-injection KDC message")?; + + reply.to_vec().context("encode credential-injection KDC reply") + } +} + +fn app_credential_username(credential: &AppCredential) -> &str { + match credential { + AppCredential::UsernamePassword { username, password: _ } => username, + } +} + +pub(crate) fn kdc_proxy_message_realm(kdc_proxy_message: &KdcProxyMessage) -> Option { + kdc_proxy_message + .target_domain + .0 + .as_ref() + .map(|realm| realm.0.to_string()) + .filter(|realm| !realm.is_empty()) +} + +fn realm_mismatch(expected: &str, actual: &str) -> Option { + if expected.eq_ignore_ascii_case(actual) { + None + } else { + Some(RealmMismatch { + expected: expected.to_owned(), + actual: actual.to_owned(), + }) + } +} + +fn realm_from_proxy_username(proxy_username: &str, jti: Uuid) -> String { + proxy_username + .split_once('@') + .map(|(_, realm)| realm) + .filter(|realm| !realm.is_empty()) + .map(str::to_owned) + .unwrap_or_else(|| synthetic_realm(jti)) +} + +fn build_kdc_config( + realm: &str, + proxy_credential: &AppCredential, + acceptor_principal_name: &str, + acceptor_password: &str, + krbtgt_key: &[u8], + acceptor_long_term_key: &[u8], +) -> anyhow::Result { + let (proxy_user_name, proxy_password) = proxy_credential.decrypt_password()?; + let proxy_user_name = principal_for_realm(&proxy_user_name, realm); + let acceptor_principal_name = principal_for_realm(acceptor_principal_name, realm); + + Ok(kdc::config::KerberosServer { + realm: realm.to_owned(), + users: vec![ + kdc::config::DomainUser { + username: proxy_user_name.clone(), + password: proxy_password.expose_secret().to_owned(), + salt: kerberos_salt(realm, &proxy_user_name), + }, + kdc::config::DomainUser { + username: acceptor_principal_name.clone(), + password: acceptor_password.to_owned(), + salt: kerberos_salt(realm, &acceptor_principal_name), + }, + ], + max_time_skew: 300, + krbtgt_key: krbtgt_key.to_vec(), + ticket_decryption_key: Some(acceptor_long_term_key.to_vec()), + service_user: Some(kdc::config::DomainUser { + username: acceptor_principal_name.clone(), + password: acceptor_password.to_owned(), + salt: kerberos_salt(realm, &acceptor_principal_name), + }), + }) +} + +fn principal_for_realm(user_name: &str, realm: &str) -> String { + if user_name.contains('@') { + user_name.to_owned() + } else { + format!("{user_name}@{realm}") + } +} + +fn kerberos_salt(realm: &str, principal: &str) -> String { + let local_name = principal.split('@').next().unwrap_or(principal); + format!("{}{local_name}", realm.to_ascii_uppercase()) +} + +fn synthetic_realm(jti: Uuid) -> String { + format!("CRED-{}.INVALID", jti.simple()).to_ascii_uppercase() +} + +fn random_32_bytes() -> Vec { + let mut bytes = vec![0u8; 32]; + OsRng.fill_bytes(&mut bytes); + bytes +} + +/// Live synthetic KDCs published by active RDP credential-injection sessions. +/// +/// Pass window between handlers: +/// - RDP path publishes when it starts a Kerberos injection +/// - `/jet/KdcProxy` only looks up; it never builds a KDC from provisioned groceries +/// +/// Entries are connection-scoped via [`SyntheticKdcRegistration`]. Reconnects `register` again +/// (replace + bump generation); a late drop of an older registration is a no-op. +#[derive(Debug, Clone)] +pub struct SyntheticKdcRegistry { + inner: Arc>, +} + +#[derive(Debug, Default)] +struct RegistryInner { + live: HashMap, + next_generation: HashMap, +} + +#[derive(Debug, Clone)] +struct PublishedSyntheticKdc { + generation: u64, + kdc: Arc, +} + +/// RAII lease for a published synthetic KDC. Dropping it unpublishes only this generation. +pub(crate) struct SyntheticKdcRegistration { + registry: SyntheticKdcRegistry, + jti: Uuid, + generation: u64, +} + +impl Drop for SyntheticKdcRegistration { + fn drop(&mut self) { + let mut inner = self.registry.inner.lock(); + let Some(current) = inner.live.get(&self.jti) else { + return; + }; + if current.generation == self.generation { + inner.live.remove(&self.jti); + debug!(jti = %self.jti, generation = self.generation, "unpublished synthetic KDC"); + } + } +} + +impl Default for SyntheticKdcRegistry { + fn default() -> Self { + Self::new() + } +} + +impl SyntheticKdcRegistry { + pub fn new() -> Self { + Self { + inner: Arc::new(Mutex::new(RegistryInner::default())), + } + } + + fn allocate_generation(inner: &mut RegistryInner, jti: Uuid) -> u64 { + let slot = inner.next_generation.entry(jti).or_insert(0); + *slot = slot.wrapping_add(1); + *slot + } + + pub(crate) fn register(&self, kdc: Arc) -> SyntheticKdcRegistration { + let jti = kdc.jti(); + let mut inner = self.inner.lock(); + let generation = Self::allocate_generation(&mut inner, jti); + inner.live.insert(jti, PublishedSyntheticKdc { generation, kdc }); + debug!(%jti, generation, "published synthetic KDC"); + SyntheticKdcRegistration { + registry: self.clone(), + jti, + generation, + } + } + + pub(crate) fn get(&self, jti: Uuid) -> Option> { + self.inner.lock().live.get(&jti).map(|e| Arc::clone(&e.kdc)) + } +} + +#[cfg(test)] +mod tests { + use base64::Engine as _; + use ironrdp_connector::sspi::network_client::NetworkProtocol; + use secrecy::SecretString; + + use super::*; + use crate::credential::{CleartextAppCredential, CleartextAppCredentialMapping}; + use crate::target_connection_options::TargetConnectionOptions; + + fn cleartext_mapping_with_target_username(target_username: &str) -> CleartextAppCredentialMapping { + CleartextAppCredentialMapping { + proxy: CleartextAppCredential::UsernamePassword { + username: "proxy@example.invalid".to_owned(), + password: SecretString::from("pwd"), + }, + target: CleartextAppCredential::UsernamePassword { + username: target_username.to_owned(), + password: SecretString::from("pwd"), + }, + } + } + + fn unsigned_jws(payload: serde_json::Value) -> String { + let engine = base64::engine::general_purpose::URL_SAFE_NO_PAD; + let header = engine.encode(r#"{"alg":"RS256"}"#); + let payload = engine.encode(serde_json::to_vec(&payload).expect("payload serializes")); + let signature = engine.encode(b"signature"); + format!("{header}.{payload}.{signature}") + } + + fn association_token(jti: Uuid) -> String { + unsigned_jws(serde_json::json!({ + "jti": jti, + "dst_hst": "target.example:3389" + })) + } + + fn kdc_options() -> TargetConnectionOptions { + serde_json::from_value(serde_json::json!({ "krb_kdc": "tcp://dc.example:88" })).expect("options") + } + + fn stock_with_mapping(jti: Uuid, target_username: &str) -> ProvisioningStore { + let store = ProvisioningStore::new(); + store + .insert_credentials( + association_token(jti), + Some(cleartext_mapping_with_target_username(target_username)), + time::Duration::minutes(5), + ) + .expect("insert"); + store + } + + fn dummy_entry(jti: Uuid, target_username: &str) -> ProvisioningEntry { + stock_with_mapping(jti, target_username).take(jti).expect("entry") + } + + fn dummy_kdc(jti: Uuid) -> CredentialInjectionKdc { + let entry = dummy_entry(jti, "target"); + let mapping = entry.mapping.expect("mapping"); + CredentialInjectionKdc::new( + jti, + "target.example".to_owned(), + app_credential_username(&mapping.proxy), + &mapping.proxy, + ) + .expect("valid KDC") + } + + fn network_request(url: &str) -> NetworkRequest { + NetworkRequest { + protocol: NetworkProtocol::Http, + url: Url::parse(url).expect("url"), + data: Vec::new(), + } + } + + #[test] + fn proxy_user_at_realm_is_used_as_realm() { + assert_eq!( + realm_from_proxy_username("proxy@example.invalid", Uuid::new_v4()), + "example.invalid" + ); + } + + #[test] + fn bare_proxy_username_yields_synthetic_realm() { + let jti = Uuid::new_v4(); + assert_eq!(realm_from_proxy_username("just-a-uuid", jti), synthetic_realm(jti)); + } + + #[test] + fn from_provisioned_selects_ntlm_when_kerberos_disabled() { + let jti = Uuid::new_v4(); + let entry = dummy_entry(jti, "administrator@example.invalid"); + let registry = SyntheticKdcRegistry::new(); + let injection = CredentialInjection::from_provisioned(jti, entry, false) + .expect("prepared") + .register_if_kerberos(®istry); + assert!(!injection.uses_kerberos()); + assert!(registry.get(jti).is_none()); + } + + #[test] + fn from_provisioned_selects_ntlm_for_domainless_target() { + let jti = Uuid::new_v4(); + let entry = dummy_entry(jti, "Administrator"); + let registry = SyntheticKdcRegistry::new(); + let injection = CredentialInjection::from_provisioned(jti, entry, true) + .expect("prepared") + .register_if_kerberos(®istry); + assert!(!injection.uses_kerberos()); + } + + #[test] + fn from_provisioned_requires_krb_kdc_for_kerberos() { + let jti = Uuid::new_v4(); + let entry = dummy_entry(jti, "administrator@example.invalid"); + let err = CredentialInjection::from_provisioned(jti, entry, true).expect_err("kdc"); + assert!(matches!(err, CredentialInjectionKdcResolveError::MissingKrbKdc { .. })); + } + + #[test] + fn from_provisioned_publishes_synthetic_kdc_for_kerberos() { + let jti = Uuid::new_v4(); + let store = stock_with_mapping(jti, "administrator@example.invalid"); + store.insert_connection_options(jti, kdc_options(), time::Duration::minutes(5)); + let entry = store.take(jti).expect("entry"); + assert!(store.take(jti).is_none(), "take consumes groceries"); + let registry = SyntheticKdcRegistry::new(); + let prepared = CredentialInjection::from_provisioned(jti, entry, true).expect("prepared"); + assert!(registry.get(jti).is_none(), "not published until register_if_kerberos"); + let injection = prepared.register_if_kerberos(®istry); + assert!(injection.uses_kerberos()); + assert!(registry.get(jti).is_some()); + assert_eq!( + registry.get(jti).expect("live kdc").jti(), + injection.as_kerberos().expect("kerberos").synthetic_kdc().jti() + ); + } + + #[test] + fn provisioned_krb_kdc_is_carried_on_kerberos_injection() { + // Pins provision → from_provisioned → target_kdc for the CredSSP client leg. + let jti = Uuid::new_v4(); + let store = stock_with_mapping(jti, "administrator@example.invalid"); + store.insert_connection_options(jti, kdc_options(), time::Duration::minutes(5)); + let entry = store.take(jti).expect("entry"); + let injection = CredentialInjection::from_provisioned(jti, entry, true) + .expect("prepared") + .register_if_kerberos(&SyntheticKdcRegistry::new()); + + assert_eq!( + injection.as_kerberos().expect("kerberos").target_kdc().as_str(), + "tcp://dc.example:88", + "provisioned krb_kdc must be the URL CredSSP will use as kdc_proxy_url", + ); + } + + #[test] + fn registry_replace_and_guarded_drop_keeps_successor() { + let registry = SyntheticKdcRegistry::new(); + let jti = Uuid::new_v4(); + let first = Arc::new(dummy_kdc(jti)); + let first_reg = registry.register(Arc::clone(&first)); + assert!(Arc::ptr_eq(®istry.get(jti).expect("first"), &first)); + let second = Arc::new(dummy_kdc(jti)); + let second_reg = registry.register(Arc::clone(&second)); + assert!(Arc::ptr_eq(®istry.get(jti).expect("second"), &second)); + drop(first_reg); + assert!(Arc::ptr_eq(®istry.get(jti).expect("still second"), &second)); + drop(second_reg); + assert!(registry.get(jti).is_none()); + } + + #[test] + fn kdc_proxy_cannot_invent_from_groceries() { + let jti = Uuid::new_v4(); + let _store = stock_with_mapping(jti, "administrator@example.invalid"); + let registry = SyntheticKdcRegistry::new(); + assert!(registry.get(jti).is_none()); + } + + #[test] + fn new_kdc_uses_jti_in_in_process_url() { + let jti = Uuid::new_v4(); + let kdc = dummy_kdc(jti); + let url = kdc.in_process_kdc_url().expect("url"); + assert!(url.path().contains(&jti.to_string())); + } + + #[test] + fn intercept_ignores_non_injection_host() { + let kdc = dummy_kdc(Uuid::new_v4()); + let result = kdc + .intercept_network_request(&network_request("http://kdc.real.example/path")) + .expect("intercept"); + assert!(matches!( + result, + CredentialInjectionKdcInterception::NotInjectionRequest + )); + } + + #[test] + fn intercept_rejects_malformed_url_path() { + let kdc = dummy_kdc(Uuid::new_v4()); + let err = kdc + .intercept_network_request(&network_request("http://cred.invalid/not-a-uuid")) + .expect_err("malformed path"); + assert!(format!("{err:#}").contains("malformed in-process KDC URL")); + } +} diff --git a/devolutions-gateway/src/credential_injection_kdc.rs b/devolutions-gateway/src/credential_injection_kdc.rs deleted file mode 100644 index 38031781c..000000000 --- a/devolutions-gateway/src/credential_injection_kdc.rs +++ /dev/null @@ -1,1073 +0,0 @@ -//! In-memory Kerberos KDC used by proxy-based credential injection. -//! -//! This module owns the Kerberos side of credential injection end-to-end: -//! per-session fake-KDC material, the session store, KDC proxy handling, and the -//! in-process KDC requests emitted by the server-side CredSSP acceptor. -//! Callers should only decide whether credential injection applies; once it does, this -//! component owns the Kerberos-specific behavior. - -use std::collections::HashMap; -use std::fmt; -use std::net::SocketAddr; -use std::sync::Arc; -use std::time::Duration; - -use anyhow::Context as _; -use async_trait::async_trait; -use chacha20poly1305::aead::OsRng; -use chacha20poly1305::aead::rand_core::RngCore as _; -use devolutions_gateway_task::{ShutdownSignal, Task}; -use ironrdp_connector::sspi; -use ironrdp_connector::sspi::generator::NetworkRequest; -use parking_lot::Mutex; -use picky_krb::messages::KdcProxyMessage; -use secrecy::{ExposeSecret as _, SecretBox, SecretString}; -use thiserror::Error; -use url::Url; -use uuid::Uuid; - -use crate::config::ConfHandle; -use crate::credential::{AppCredential, AppCredentialMapping}; -use crate::provisioning::{ArcProvisioningEntry, ProvisioningStore}; -use crate::target_connection_options::TargetConnectionOptions; - -// The reserved `.invalid` TLD (RFC 6761) lets sspi-rs CredSSP server emit "KDC requests" that -// never leave the process: `intercept_network_request` recognises this hostname and dispatches -// the message into the in-process `kdc` server below. -// -// TODO(sspi-rs#664): replace this URL-trampoline with a pluggable KDC dispatcher trait once -// sspi-rs ships the API — see https://github.com/Devolutions/sspi-rs/issues/664. -const IN_PROCESS_KDC_HOST: &str = "cred.invalid"; - -pub(crate) struct CredentialInjectionKdc { - jti: Uuid, - raw_token: String, - credential_mapping: AppCredentialMapping, - connection_options: Option, - // Client target hostname. It is not a hostname of the end machine, but a DGW hostname the client - // uses when connecting. - target_hostname: String, - session: Arc, - // The KDC crate models users with plaintext passwords, so this object owns those secrets - // for the lifetime of the credential-injection KDC. Keep Debug redacted. - kdc_config: kdc::config::KerberosServer, -} - -#[derive(Debug, Error)] -pub(crate) enum CredentialInjectionKdcResolveError { - #[error("credential-injection state is not available for {jti}")] - MissingCredential { jti: Uuid }, - #[error("credential-injection state is not available for {jti}")] - NonInjectionCredential { jti: Uuid }, - #[error("association token for {jti} is not valid for credential injection")] - InvalidAssociationToken { - jti: Uuid, - #[source] - source: anyhow::Error, - }, - #[error("credential-injection KDC config could not be initialized for {jti}")] - BuildKdcConfig { - jti: Uuid, - #[source] - source: anyhow::Error, - }, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct RealmMismatch { - pub(crate) expected: String, - pub(crate) actual: String, -} - -impl fmt::Display for RealmMismatch { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "expected: {}, got: {}", self.expected, self.actual) - } -} - -impl std::error::Error for RealmMismatch {} - -#[derive(Debug)] -pub(crate) enum CredentialInjectionKdcInterception { - Intercepted(Vec), - NotInjectionRequest, - NotInjectionRealm(RealmMismatch), -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum CredentialInjectionClientAcceptorProtocol { - Kerberos, - Ntlm, -} - -pub(crate) struct CredentialInjectionKdcRequest { - message: KdcProxyMessage, -} - -impl CredentialInjectionKdcRequest { - pub(crate) fn from_token(message: KdcProxyMessage) -> Self { - Self { message } - } - - fn in_process(message: KdcProxyMessage) -> Self { - Self { message } - } -} - -impl fmt::Debug for CredentialInjectionKdc { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("CredentialInjectionKdc") - .field("jti", &self.jti) - .field("target_hostname", &self.target_hostname) - .field("realm", &self.session.realm) - .field("kdc_config", &"") - .finish() - } -} - -impl CredentialInjectionKdc { - fn from_parts( - jti: Uuid, - credential_entry: ArcProvisioningEntry, - target_hostname: String, - session: Arc, - ) -> anyhow::Result { - let mapping = credential_entry - .mapping - .as_ref() - .context("credential entry has no credential-injection mapping")?; - anyhow::ensure!( - jti == session.jti, - "credential entry JTI does not match credential-injection KDC session JTI", - ); - - let kdc_config = build_kdc_config(&session, &mapping.proxy)?; - - Ok(Self { - jti, - raw_token: credential_entry.token.clone(), - credential_mapping: mapping.clone(), - connection_options: credential_entry.connection_options.clone(), - target_hostname, - session, - kdc_config, - }) - } - - pub(crate) fn krb_kdc(&self) -> Option<&Url> { - self.connection_options.as_ref()?.krb_kdc() - } - - pub(crate) fn jti(&self) -> Uuid { - self.jti - } - - pub(crate) fn raw_token(&self) -> &str { - &self.raw_token - } - - pub(crate) fn proxy_credential(&self) -> &AppCredential { - &self.credential_mapping.proxy - } - - pub(crate) fn target_credential(&self) -> &AppCredential { - &self.credential_mapping.target - } - - /// Selects the CredSSP acceptor backend Gateway should present to the RDP client. - /// - /// The acceptor side must mirror the target-side auth package. - /// Domainless target credentials cannot acquire Kerberos tickets. - /// Enabling the Kerberos acceptor for those sessions would make incoming NTLMSSP tokens fail in Kerberos parsing. - pub(crate) fn client_acceptor_protocol(&self) -> anyhow::Result { - let target_username = sspi::Username::parse(app_credential_username(self.target_credential())) - .context("invalid target credential username")?; - - if target_username.domain_name().is_some() { - Ok(CredentialInjectionClientAcceptorProtocol::Kerberos) - } else { - Ok(CredentialInjectionClientAcceptorProtocol::Ntlm) - } - } - - pub(crate) fn server_kerberos_config(&self, client_addr: SocketAddr) -> anyhow::Result { - let user = sspi::CredentialsBuffers::AuthIdentity(sspi::AuthIdentityBuffers::from_utf8( - &self.session.acceptor.principal_name, - &self.session.realm, - self.session.acceptor.password.expose_secret(), - )); - - let kdc_url = self.in_process_kdc_url()?; - - // The SPN that the client puts on its AP-REQ ticket is the one for the target RDP - // server (`TERMSRV/`). Gateway-as-CredSSP-server is impersonating that target, - // so `ServerProperties` must claim the same SPN as the gateway listener or sspi-rs - // rejects the ticket. - Ok(sspi::KerberosServerConfig { - kerberos_config: sspi::KerberosConfig { - kdc_url: Some(kdc_url), - client_computer_name: client_addr.to_string(), - }, - server_properties: sspi::kerberos::ServerProperties::new( - &["TERMSRV", &self.target_hostname], - Some(user), - Duration::from_secs(300), - Some(sspi::Secret::new( - self.session.acceptor.long_term_key.expose_secret().clone(), - )), - )?, - }) - } - - pub(crate) fn intercept_network_request( - &self, - request: &NetworkRequest, - ) -> anyhow::Result { - if request.url.host_str() != Some(IN_PROCESS_KDC_HOST) { - return Ok(CredentialInjectionKdcInterception::NotInjectionRequest); - } - - let url_jti = request - .url - .path() - .trim_start_matches('/') - .parse::() - .context("malformed in-process KDC URL")?; - anyhow::ensure!( - url_jti == self.jti, - "in-process KDC URL JTI does not match current CredSSP session", - ); - - debug!( - jti = %self.jti, - scheme = %request.url.scheme(), - "Credential-injection KDC intercepted in-process request" - ); - - let kdc_message = KdcProxyMessage::from_raw(&request.data).context("malformed in-process KDC proxy payload")?; - self.handle_kdc_proxy_request(CredentialInjectionKdcRequest::in_process(kdc_message)) - } - - pub(crate) fn handle_kdc_proxy_request( - &self, - request: CredentialInjectionKdcRequest, - ) -> anyhow::Result { - let request_realm = self.resolve_message_realm(&request.message); - debug!( - jti = %self.jti, - resolved_realm = %request_realm, - "Credential-injection KDC realm resolved" - ); - - if let Some(mismatch) = realm_mismatch(&self.session.realm, &request_realm) { - return Ok(CredentialInjectionKdcInterception::NotInjectionRealm(mismatch)); - } - - let reply = self.handle_message(request.message)?; - Ok(CredentialInjectionKdcInterception::Intercepted(reply)) - } - - fn in_process_kdc_url(&self) -> anyhow::Result { - Url::parse(&format!("http://{}/{}", IN_PROCESS_KDC_HOST, self.jti)).context("build in-process KDC URL") - } - - fn resolve_message_realm(&self, kdc_proxy_message: &KdcProxyMessage) -> String { - kdc_proxy_message_realm(kdc_proxy_message).unwrap_or_else(|| self.session.realm.clone()) - } - - fn handle_message(&self, kdc_proxy_message: KdcProxyMessage) -> anyhow::Result> { - let reply = kdc::handle_kdc_proxy_message(kdc_proxy_message, &self.kdc_config, &self.target_hostname) - .context("handle credential-injection KDC message")?; - - reply.to_vec().context("encode credential-injection KDC reply") - } -} - -fn app_credential_username(credential: &AppCredential) -> &str { - match credential { - AppCredential::UsernamePassword { username, password: _ } => username, - } -} - -pub(crate) fn kdc_proxy_message_realm(kdc_proxy_message: &KdcProxyMessage) -> Option { - kdc_proxy_message - .target_domain - .0 - .as_ref() - .map(|realm| realm.0.to_string()) - .filter(|realm| !realm.is_empty()) -} - -fn realm_mismatch(expected: &str, actual: &str) -> Option { - if expected.eq_ignore_ascii_case(actual) { - return None; - } - - Some(RealmMismatch { - expected: expected.to_owned(), - actual: actual.to_owned(), - }) -} - -/// Per-session Kerberos material for proxy-based credential injection. -/// -/// The key material and the acceptor PA-ENC-TIMESTAMP password are wrapped in [`SecretBox`] / -/// [`SecretString`] so they cannot be accidentally written to logs through structured tracing. -/// Access requires an explicit `expose_secret()` call, which is greppable and reviewable. -struct CredentialInjectionKdcSession { - jti: Uuid, - realm: String, - kdc: CredentialInjectionKdcState, - acceptor: CredentialInjectionAcceptorState, -} - -struct CredentialInjectionKdcState { - krbtgt_key: SecretBox>, -} - -struct CredentialInjectionAcceptorState { - principal_name: String, - password: SecretString, - long_term_key: SecretBox>, -} - -impl fmt::Debug for CredentialInjectionKdcSession { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("CredentialInjectionKdcSession") - .field("jti", &self.jti) - .field("realm", &self.realm) - .field("kdc", &self.kdc) - .field("acceptor", &self.acceptor) - .finish() - } -} - -impl fmt::Debug for CredentialInjectionKdcState { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("CredentialInjectionKdcState") - .field("krbtgt_key", &"<32 bytes redacted>") - .finish() - } -} - -impl fmt::Debug for CredentialInjectionAcceptorState { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("CredentialInjectionAcceptorState") - .field("principal_name", &self.principal_name) - .field("password", &"") - .field("long_term_key", &"<32 bytes redacted>") - .finish() - } -} - -/// Derive per-session Kerberos material from the proxy username and the association token's JTI. -/// -/// The proxy username's optional `@realm` suffix selects the realm DVLS supplied; otherwise -/// fall back to a per-session synthetic realm derived from the JTI. The two sides agree -/// because DVLS derives the synthetic value the same way. -fn derive_credential_injection_kdc_session(proxy_username: &str, jti: Uuid) -> CredentialInjectionKdcSession { - let realm = proxy_username - .split_once('@') - .map(|(_, realm)| realm) - .filter(|realm| !realm.is_empty()) - .map(str::to_owned) - .unwrap_or_else(|| synthetic_realm(jti)); - - CredentialInjectionKdcSession { - jti, - realm, - kdc: CredentialInjectionKdcState { - krbtgt_key: SecretBox::new(Box::new(random_32_bytes())), - }, - acceptor: CredentialInjectionAcceptorState { - principal_name: "jet".to_owned(), - password: SecretString::from(hex::encode(random_32_bytes())), - long_term_key: SecretBox::new(Box::new(random_32_bytes())), - }, - } -} - -fn build_kdc_config( - session: &CredentialInjectionKdcSession, - proxy_credential: &AppCredential, -) -> anyhow::Result { - let realm = &session.realm; - let (proxy_user_name, proxy_password) = proxy_credential.decrypt_password()?; - let proxy_user_name = principal_for_realm(&proxy_user_name, realm); - let acceptor_principal_name = principal_for_realm(&session.acceptor.principal_name, realm); - - let acceptor_password = session.acceptor.password.expose_secret().to_owned(); - Ok(kdc::config::KerberosServer { - realm: realm.to_owned(), - users: vec![ - kdc::config::DomainUser { - username: proxy_user_name.clone(), - password: proxy_password.expose_secret().to_owned(), - salt: kerberos_salt(realm, &proxy_user_name), - }, - kdc::config::DomainUser { - username: acceptor_principal_name.clone(), - password: acceptor_password.clone(), - salt: kerberos_salt(realm, &acceptor_principal_name), - }, - ], - max_time_skew: 300, - krbtgt_key: session.kdc.krbtgt_key.expose_secret().clone(), - ticket_decryption_key: Some(session.acceptor.long_term_key.expose_secret().clone()), - service_user: Some(kdc::config::DomainUser { - username: acceptor_principal_name.clone(), - password: acceptor_password, - salt: kerberos_salt(realm, &acceptor_principal_name), - }), - }) -} - -fn principal_for_realm(user_name: &str, realm: &str) -> String { - if user_name.contains('@') { - user_name.to_owned() - } else { - format!("{user_name}@{realm}") - } -} - -fn kerberos_salt(realm: &str, principal: &str) -> String { - let local_name = principal.split('@').next().unwrap_or(principal); - format!("{}{local_name}", realm.to_ascii_uppercase()) -} - -fn synthetic_realm(jti: Uuid) -> String { - format!("CRED-{}.INVALID", jti.simple()).to_ascii_uppercase() -} - -fn random_32_bytes() -> Vec { - let mut bytes = vec![0u8; 32]; - OsRng.fill_bytes(&mut bytes); - bytes -} - -/// One-stop service for credential storage and credential-injection KDC state. -/// -/// Wraps the protocol-neutral [`ProvisioningStore`] and adds a Kerberos session cache keyed by -/// association-token JTI. The credential store remains the single source of truth for entry -/// lifetime; the session cache piggybacks on it (Arc-cloned credentials at lookup time, with stale -/// sessions evicted on insert-replacement and by a periodic sweep). -/// -/// All credential reads/writes — provision-credentials, RDP mode detection, KDC dispatch — go -/// through this service, so callers see one handle instead of coordinating a store and a registry. -#[derive(Clone)] -pub struct CredentialService { - // The `ConfHandle` is needed to resolve the hostname for the KDC config, which is used to - // build the SPN for the CredSSP acceptor. The hostname cannot not be a plain `String`, because - // the config can be reloaded at runtime. - conf_handle: ConfHandle, - credentials: ProvisioningStore, - sessions: Arc>>>, -} - -impl fmt::Debug for CredentialService { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("CredentialService") - .field("conf_handle", &"") - .field("credentials", &self.credentials) - .field("sessions", &self.sessions) - .finish() - } -} - -impl CredentialService { - pub fn new(conf_handle: ConfHandle) -> Self { - Self { - conf_handle, - credentials: ProvisioningStore::new(), - sessions: Arc::new(Mutex::new(HashMap::new())), - } - } - - /// Insert (or replace) the credentials half keyed by the token's JTI. - /// - /// Any previously-cached Kerberos session for the same JTI is dropped: it was derived from - /// the prior provisioning and is no longer valid for the new entry. We invalidate even when - /// the store reports no replacement, because the prior entry may have already been evicted by - /// `provisioning::CleanupTask` while its session cache entry was still awaiting the next - /// `sweep_orphans` tick. - pub(crate) fn insert_credentials( - &self, - token: String, - mapping: Option, - time_to_live: time::Duration, - ) -> Result { - // Snapshot the JTI from the new token so we can invalidate the matching session entry - // regardless of whether the credential store reports a replacement. `ProvisioningStore::insert_credentials` - // re-extracts internally; both calls go through the same code path, so an invalid token - // here will surface as the same `InvalidToken` error downstream. - let jti = crate::token::extract_jti(&token) - .context("failed to extract token ID") - .map_err(crate::provisioning::InsertError::InvalidToken)?; - let replaced = self.credentials.insert_credentials(token, mapping, time_to_live)?; - self.sessions.lock().remove(&jti); - Ok(replaced) - } - - /// Insert (or replace) the connection-options half. Drops any cached Kerberos session for the - /// JTI because `krb_kdc` is part of the session's routing inputs. - pub(crate) fn insert_connection_options( - &self, - jti: Uuid, - connection_options: TargetConnectionOptions, - time_to_live: time::Duration, - ) -> bool { - let replaced = self - .credentials - .insert_connection_options(jti, connection_options, time_to_live); - self.sessions.lock().remove(&jti); - replaced - } - - /// Look up a credential entry by its association-token JTI. - pub(crate) fn get(&self, jti: Uuid) -> Option { - self.credentials.get(jti) - } - - /// Borrow the inner [`ProvisioningStore`] for plumbing that genuinely needs the - /// protocol-neutral primitive (e.g. wiring the background expiry task). - pub fn credential_store(&self) -> &ProvisioningStore { - &self.credentials - } - - /// Resolve the credential-injection KDC bound to the given association-token JTI. - /// - /// Returns the per-call KDC view; the underlying Kerberos session (krbtgt key, acceptor - /// long-term key, acceptor password) is cached so the in-process KDC and the CredSSP acceptor - /// see identical key material for the lifetime of the provisioned credentials. - pub(crate) fn kdc_for(&self, jti: Uuid) -> Result { - let credential_entry = self.credentials.get(jti).ok_or_else(|| { - warn!(%jti, "KDC token references missing credential-injection state"); - CredentialInjectionKdcResolveError::MissingCredential { jti } - })?; - - let mapping = credential_entry.mapping.as_ref().ok_or_else(|| { - warn!(%jti, "KDC token references non-injection credential state"); - CredentialInjectionKdcResolveError::NonInjectionCredential { jti } - })?; - - // Validate association-token shape for credential injection (dst_hst present, etc.). - // SPN / acceptor hostname comes from gateway config below (#1856), not dst_hst. - crate::token::extract_credential_injection_target_hostname(&credential_entry.token).map_err(|source| { - warn!( - %jti, - error = format!("{source:#}"), - "KDC token references invalid credential-injection association token" - ); - CredentialInjectionKdcResolveError::InvalidAssociationToken { jti, source } - })?; - - let proxy_username = app_credential_username(&mapping.proxy).to_owned(); - // Atomic get-or-insert: holds the lock long enough to guarantee a single Arc - // wins for this JTI even under concurrent `kdc_for` calls. The derivation is fast (a few - // hundred bytes of OsRng) so doing it under the lock is acceptable. - let session = { - let mut sessions = self.sessions.lock(); - let session = sessions - .entry(jti) - .or_insert_with(|| Arc::new(derive_credential_injection_kdc_session(&proxy_username, jti))); - Arc::clone(session) - }; - - let hostname = self.conf_handle.get_conf().hostname.clone(); - - CredentialInjectionKdc::from_parts(jti, credential_entry, hostname, session) - .map_err(|source| CredentialInjectionKdcResolveError::BuildKdcConfig { jti, source }) - } - - fn sweep_orphans(&self) { - let stale_jtis: Vec = { - let sessions = self.sessions.lock(); - sessions - .keys() - .copied() - .filter(|jti| self.credentials.get(*jti).is_none()) - .collect() - }; - - if stale_jtis.is_empty() { - return; - } - - let mut sessions = self.sessions.lock(); - for jti in stale_jtis { - sessions.remove(&jti); - } - } -} - -pub struct CleanupTask { - pub service: CredentialService, -} - -#[async_trait] -impl Task for CleanupTask { - type Output = anyhow::Result<()>; - - const NAME: &'static str = "credential injection kdc cleanup"; - - async fn run(self, shutdown_signal: ShutdownSignal) -> Self::Output { - cleanup_task(self.service, shutdown_signal).await; - Ok(()) - } -} - -#[instrument(skip_all)] -async fn cleanup_task(service: CredentialService, mut shutdown_signal: ShutdownSignal) { - use tokio::time::{Duration, sleep}; - - const TASK_INTERVAL: Duration = Duration::from_secs(60 * 15); // 15 minutes - - debug!("Task started"); - - loop { - tokio::select! { - _ = sleep(TASK_INTERVAL) => {} - _ = shutdown_signal.wait() => { - break; - } - } - - service.sweep_orphans(); - } - - debug!("Task terminated"); -} - -#[cfg(test)] -mod tests { - use base64::Engine as _; - use ironrdp_connector::sspi::network_client::NetworkProtocol; - use secrecy::SecretString; - - use super::*; - use crate::config::ConfHandle; - use crate::credential::{CleartextAppCredential, CleartextAppCredentialMapping}; - - const TEST_CONFIG: &str = r#"{ - "Hostname": "dgateway.localhost.com", - "ProvisionerPublicKeyData": { - "Value": "mMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA4vuqLOkl1pWobt6su1XO9VskgCAwevEGs6kkNjJQBwkGnPKYLmNF1E/af1yCocfVn/OnPf9e4x+lXVyZ6LMDJxFxu+axdgOq3Ld392J1iAEbfvwlyRFnEXFOJNyylqg3bY6LvnWHL/XZczVdMD9xYfq2sO9bg3xjRW4s7r9EEYOFjqVT3VFznH9iWJVtcSEKukmS/3uKoO6lGhacvu0HhjXXdgq0R8zvR4XRJ9Fcnf0f9Ypoc+i6L80NVjrRCeVOH+Ld/2fA9bocpfLarcVqG3RjS+qgOtpyCc0jWVFF4zaGQ7LUDFkEIYILkICeMMn2ll29hmZNzsJzZJ9s6NocgQIDAQAB" - }, - "Listeners": [ - { "InternalUrl": "http://*:7171", "ExternalUrl": "https://*:7171" } - ], - "__debug__": { "disable_token_validation": true } - }"#; - - fn mock_conf_handle() -> ConfHandle { - ConfHandle::mock(TEST_CONFIG).expect("test config is valid") - } - - fn cleartext_mapping_with_target_username(target_username: &str) -> CleartextAppCredentialMapping { - CleartextAppCredentialMapping { - proxy: CleartextAppCredential::UsernamePassword { - username: "proxy@example.invalid".to_owned(), - password: SecretString::from("pwd"), - }, - target: CleartextAppCredential::UsernamePassword { - username: target_username.to_owned(), - password: SecretString::from("pwd"), - }, - } - } - - fn unsigned_jws(payload: serde_json::Value) -> String { - let engine = base64::engine::general_purpose::URL_SAFE_NO_PAD; - let header = engine.encode(r#"{"alg":"RS256"}"#); - let payload = engine.encode(serde_json::to_vec(&payload).expect("payload serializes")); - let signature = engine.encode(b"signature"); - format!("{header}.{payload}.{signature}") - } - - fn association_token(jti: Uuid) -> String { - unsigned_jws(serde_json::json!({ - "jti": jti, - "dst_hst": "target.example:3389" - })) - } - - fn dummy_entry_with_target_username(jti: Uuid, target_username: &str) -> ArcProvisioningEntry { - let store = ProvisioningStore::new(); - store - .insert_credentials( - association_token(jti), - Some(cleartext_mapping_with_target_username(target_username)), - time::Duration::minutes(5), - ) - .expect("credential entry inserts"); - - store.get(jti).expect("credential entry is indexed by JTI") - } - - fn dummy_entry(jti: Uuid) -> ArcProvisioningEntry { - dummy_entry_with_target_username(jti, "target") - } - - fn dummy_kdc(jti: Uuid) -> CredentialInjectionKdc { - let entry = dummy_entry(jti); - let session = Arc::new(derive_credential_injection_kdc_session("proxy@example.invalid", jti)); - CredentialInjectionKdc::from_parts(jti, entry, "target.example".to_owned(), session) - .expect("valid credential-injection KDC") - } - - fn dummy_kdc_with_target_username(jti: Uuid, target_username: &str) -> CredentialInjectionKdc { - let entry = dummy_entry_with_target_username(jti, target_username); - let session = Arc::new(derive_credential_injection_kdc_session("proxy@example.invalid", jti)); - CredentialInjectionKdc::from_parts(jti, entry, "target.example".to_owned(), session) - .expect("valid credential-injection KDC") - } - - fn network_request(url: &str) -> NetworkRequest { - NetworkRequest { - protocol: NetworkProtocol::Http, - url: Url::parse(url).expect("test URL parses"), - data: Vec::new(), - } - } - - #[test] - fn proxy_user_at_realm_is_used_as_realm() { - let session = derive_credential_injection_kdc_session("proxy@example.invalid", Uuid::new_v4()); - assert_eq!(session.realm, "example.invalid"); - } - - #[test] - fn bare_proxy_username_yields_synthetic_realm() { - let jti = Uuid::new_v4(); - let session = derive_credential_injection_kdc_session("just-a-uuid", jti); - assert_eq!(session.realm, synthetic_realm(jti)); - assert!(!session.realm.is_empty()); - } - - #[test] - fn service_kdc_for_rejects_expired_credential_entry() { - let service = CredentialService::new(mock_conf_handle()); - let jti = Uuid::new_v4(); - - // Negative TTL: entry is born already expired. `ProvisioningStore::get` does not - // filter on expiry, so the service's own check is what guarantees we never build a KDC - // over stale credentials. - service - .insert_credentials( - association_token(jti), - Some(cleartext_mapping_with_target_username("target")), - time::Duration::seconds(-1), - ) - .expect("credential entry inserts"); - - assert!( - matches!( - service.kdc_for(jti), - Err(CredentialInjectionKdcResolveError::MissingCredential { .. }) - ), - "expired credentials must not yield a KDC" - ); - } - - #[test] - fn service_kdc_for_returns_same_session_under_concurrent_calls() { - let service = CredentialService::new(mock_conf_handle()); - let jti = Uuid::new_v4(); - - service - .insert_credentials( - association_token(jti), - Some(cleartext_mapping_with_target_username("target")), - time::Duration::minutes(5), - ) - .expect("credential entry inserts"); - - let first = service.kdc_for(jti).expect("first call resolves"); - let second = service.kdc_for(jti).expect("second call resolves"); - - // The Kerberos session is the piece that must be stable across calls; the per-call KDC - // view rebuilds the rest. Compare via the long-term acceptor key as a session-identity - // probe. - let first_key = first.session.acceptor.long_term_key.expose_secret().clone(); - let second_key = second.session.acceptor.long_term_key.expose_secret().clone(); - assert_eq!( - first_key, second_key, - "concurrent kdc_for must share one cached session per JTI" - ); - } - - #[test] - fn service_insert_drops_stale_session_even_without_credential_replacement() { - let service = CredentialService::new(mock_conf_handle()); - let jti = Uuid::new_v4(); - - // Simulate the race called out by Codex: a previous provisioning's session is still - // cached, but the credential entry has already been evicted (e.g. by - // `provisioning::cleanup_task`) and `sweep_orphans` has not run yet. A fresh provisioning - // under the same JTI must drop the stale session regardless of whether - // `ProvisioningStore::insert_credentials` reports a replacement, otherwise the next `kdc_for` - // would reuse the old key material. - let stale_session = Arc::new(derive_credential_injection_kdc_session("proxy@example.invalid", jti)); - service.sessions.lock().insert(jti, Arc::clone(&stale_session)); - - let replaced = service - .insert_credentials( - association_token(jti), - Some(cleartext_mapping_with_target_username("target")), - time::Duration::minutes(5), - ) - .expect("credential entry inserts"); - assert!(!replaced, "test precondition: no credential replacement"); - - assert!( - !service.sessions.lock().contains_key(&jti), - "insert must drop stale session even when no credential replacement occurred" - ); - } - - #[test] - fn service_insert_replacement_drops_cached_kerberos_material() { - let service = CredentialService::new(mock_conf_handle()); - let jti = Uuid::new_v4(); - - service - .insert_credentials( - association_token(jti), - Some(cleartext_mapping_with_target_username("target")), - time::Duration::minutes(5), - ) - .expect("credential entry inserts"); - - let first = service.kdc_for(jti).expect("first call resolves"); - let first_key = first.session.acceptor.long_term_key.expose_secret().clone(); - - // Re-insert under the same JTI: the cached session for the previous entry must be evicted - // automatically, otherwise the new KDC would carry stale key material that the freshly - // provisioned credentials no longer match. - service - .insert_credentials( - association_token(jti), - Some(cleartext_mapping_with_target_username("target")), - time::Duration::minutes(5), - ) - .expect("credential entry re-inserts"); - - let second = service.kdc_for(jti).expect("second call resolves with fresh session"); - let second_key = second.session.acceptor.long_term_key.expose_secret().clone(); - - assert_ne!( - first_key, second_key, - "insert-replacement must force a fresh session derivation" - ); - } - - #[test] - fn service_sweep_orphans_drops_sessions_with_no_credential_entry() { - let service = CredentialService::new(mock_conf_handle()); - let jti = Uuid::new_v4(); - - service - .insert_credentials( - association_token(jti), - Some(cleartext_mapping_with_target_username("target")), - time::Duration::minutes(5), - ) - .expect("credential entry inserts"); - - service.kdc_for(jti).expect("kdc_for populates session cache"); - assert!(service.sessions.lock().contains_key(&jti), "session cached"); - - // Simulate credential store eviction: build a parallel service whose credential store is - // empty but whose session cache is shared with the original. A more faithful test would - // drive `provisioning::cleanup_task` to expire the entry, but it sleeps for 15 minutes - // between ticks. Swapping the inner store is the deterministic equivalent. - let orphaned_service = CredentialService { - conf_handle: mock_conf_handle(), - credentials: ProvisioningStore::new(), - sessions: Arc::clone(&service.sessions), - }; - - orphaned_service.sweep_orphans(); - assert!( - !orphaned_service.sessions.lock().contains_key(&jti), - "sweep must drop sessions whose JTI is no longer in credential_store" - ); - } - - #[test] - fn client_acceptor_protocol_is_ntlm_for_domainless_target_credential() { - let kdc = dummy_kdc_with_target_username(Uuid::new_v4(), "Administrator"); - - assert_eq!( - kdc.client_acceptor_protocol().expect("protocol selected"), - CredentialInjectionClientAcceptorProtocol::Ntlm - ); - } - - #[test] - fn client_acceptor_protocol_is_kerberos_for_upn_target_credential() { - let kdc = dummy_kdc_with_target_username(Uuid::new_v4(), "administrator@example.invalid"); - - assert_eq!( - kdc.client_acceptor_protocol().expect("protocol selected"), - CredentialInjectionClientAcceptorProtocol::Kerberos - ); - } - - #[test] - fn client_acceptor_protocol_is_kerberos_for_downlevel_target_credential() { - let kdc = dummy_kdc_with_target_username(Uuid::new_v4(), "EXAMPLE\\Administrator"); - - assert_eq!( - kdc.client_acceptor_protocol().expect("protocol selected"), - CredentialInjectionClientAcceptorProtocol::Kerberos - ); - } - - #[test] - fn from_parts_rejects_mismatched_entry_and_session_jti() { - let entry_jti = Uuid::new_v4(); - let session_jti = Uuid::new_v4(); - assert_ne!(entry_jti, session_jti); - - let entry = dummy_entry(entry_jti); - let session = Arc::new(derive_credential_injection_kdc_session( - "proxy@example.invalid", - session_jti, - )); - - let err = CredentialInjectionKdc::from_parts(entry_jti, entry, "target.example".to_owned(), session) - .expect_err("mismatched entry/session JTI must fail closed"); - let msg = format!("{err:#}"); - assert!( - msg.contains("credential entry JTI does not match credential-injection KDC session JTI"), - "actual: {msg}" - ); - } - - #[test] - fn service_kdc_for_rejects_unknown_jti() { - let service = CredentialService::new(mock_conf_handle()); - - assert!( - matches!( - service.kdc_for(Uuid::new_v4()), - Err(CredentialInjectionKdcResolveError::MissingCredential { .. }) - ), - "KDC tokens with jet_cred_id must not fall back to real-KDC forwarding" - ); - } - - #[test] - fn service_kdc_for_rejects_non_injection_entry() { - let service = CredentialService::new(mock_conf_handle()); - let jti = Uuid::new_v4(); - - service - .insert_credentials(association_token(jti), None, time::Duration::minutes(5)) - .expect("provision-token entry inserts"); - - assert!( - matches!( - service.kdc_for(jti), - Err(CredentialInjectionKdcResolveError::NonInjectionCredential { .. }) - ), - "KDC tokens with jet_cred_id must require provision-credentials state" - ); - } - - #[test] - fn service_kdc_for_uses_gateway_hostname_for_spn() { - // #1856: SPN / acceptor hostname is the Gateway hostname from config, not dst_hst. - // Token dst_hst is still validated (missing/invalid shape fails kdc_for). - let service = CredentialService::new(mock_conf_handle()); - let jti = Uuid::new_v4(); - - service - .insert_credentials( - association_token(jti), - Some(cleartext_mapping_with_target_username("target")), - time::Duration::minutes(5), - ) - .expect("credential entry inserts"); - - let kdc = service.kdc_for(jti).expect("credential-injection KDC resolves"); - - assert_eq!(kdc.target_hostname, "dgateway.localhost.com"); - } - - #[test] - fn intercept_ignores_non_loopback_host() { - let jti = Uuid::new_v4(); - let kdc = dummy_kdc(jti); - - let request = network_request("http://kdc.real.example/path"); - let result = kdc - .intercept_network_request(&request) - .expect("non-loopback request dispatches"); - - assert!(matches!( - result, - CredentialInjectionKdcInterception::NotInjectionRequest - )); - } - - #[test] - fn intercept_rejects_malformed_url_path() { - let jti = Uuid::new_v4(); - let kdc = dummy_kdc(jti); - - let request = network_request("http://cred.invalid/not-a-uuid"); - let err = kdc - .intercept_network_request(&request) - .expect_err("non-UUID path must fail"); - let msg = format!("{err:#}"); - assert!(msg.contains("malformed in-process KDC URL"), "actual: {msg}"); - } - - #[test] - fn intercept_rejects_mismatched_jti() { - let entry_jti = Uuid::new_v4(); - let other_jti = Uuid::new_v4(); - assert_ne!(entry_jti, other_jti); - - let kdc = dummy_kdc(entry_jti); - - let request = network_request(&format!("http://cred.invalid/{}", other_jti)); - let err = kdc - .intercept_network_request(&request) - .expect_err("JTI mismatch must fail"); - let msg = format!("{err:#}"); - assert!(msg.contains("does not match current CredSSP session"), "actual: {msg}"); - } - - #[test] - fn intercept_accepts_matching_url_path_before_payload_decode() { - let jti = Uuid::new_v4(); - let kdc = dummy_kdc(jti); - - let request = network_request(&format!("http://cred.invalid/{jti}")); - let err = kdc - .intercept_network_request(&request) - .expect_err("empty KDC payload must fail after URL/JTI validation"); - let msg = format!("{err:#}"); - assert!(msg.contains("malformed in-process KDC proxy payload"), "actual: {msg}"); - } - - #[test] - fn realm_mismatch_is_reported_as_not_injection_realm() { - let mismatch = - realm_mismatch("cred-session.invalid", "evil.example").expect("different realms produce a mismatch"); - assert_eq!(mismatch.expected, "cred-session.invalid"); - assert_eq!(mismatch.actual, "evil.example"); - } - - #[test] - fn missing_kdc_proxy_envelope_realm_falls_back_to_session_realm() { - let jti = Uuid::new_v4(); - let kdc = dummy_kdc(jti); - let message = KdcProxyMessage::from_raw_kerb_message(&[]).expect("KDC proxy wrapper builds"); - - assert_eq!(kdc.resolve_message_realm(&message), "example.invalid"); - } -} diff --git a/devolutions-gateway/src/generic_client.rs b/devolutions-gateway/src/generic_client.rs index dfc31df7f..d7287a21f 100644 --- a/devolutions-gateway/src/generic_client.rs +++ b/devolutions-gateway/src/generic_client.rs @@ -8,7 +8,8 @@ use tracing::field; use typed_builder::TypedBuilder; use crate::config::Conf; -use crate::credential_injection_kdc::CredentialService; +use crate::credential_injection::{CredentialInjection, SyntheticKdcRegistry}; +use crate::provisioning::ProvisioningStore; use crate::proxy::Proxy; use crate::rdp_pcb::{extract_association_claims, read_pcb}; use crate::recording::ActiveRecordings; @@ -27,7 +28,8 @@ pub struct GenericClient { sessions: SessionMessageSender, subscriber_tx: SubscriberSender, active_recordings: Arc, - credentials: CredentialService, + provisioning: ProvisioningStore, + synthetic_kdc_registry: SyntheticKdcRegistry, #[builder(default)] agent_tunnel_handle: Option>, } @@ -51,7 +53,8 @@ where sessions, subscriber_tx, active_recordings, - credentials, + provisioning, + synthetic_kdc_registry, agent_tunnel_handle, } = self; @@ -152,14 +155,18 @@ where // The credential store is keyed on the association token's JTI, so a direct // lookup by `claims.jti` is the primary path. if is_rdp - && let Some(entry) = credentials.get(claims.jti) + && let Some(entry) = provisioning.take(claims.jti) && entry.mapping.is_some() { anyhow::ensure!(token == entry.token, "token mismatch"); - let credential_injection_kdc = credentials.kdc_for(claims.jti)?; + let kerberos_enabled = conf.debug.enable_unstable && conf.debug.kerberos_credential_injection; + let credential_injection = + CredentialInjection::from_provisioned(claims.jti, entry, kerberos_enabled)? + .register_if_kerberos(&synthetic_kdc_registry); info!( - jti = %credential_injection_kdc.jti(), + jti = %credential_injection.jti(), + kerberos = credential_injection.uses_kerberos(), "RDP-TLS forwarding with credential injection" ); @@ -179,7 +186,7 @@ where .server_stream(server_stream) .sessions(sessions) .subscriber_tx(subscriber_tx) - .credential_injection_kdc(credential_injection_kdc) + .credential_injection(credential_injection) .client_stream_leftover_bytes(leftover_bytes) .server_dns_name(selected_target.host().to_owned()) .disconnect_interest(disconnect_interest) diff --git a/devolutions-gateway/src/lib.rs b/devolutions-gateway/src/lib.rs index ae5ef4190..1fb08ff85 100644 --- a/devolutions-gateway/src/lib.rs +++ b/devolutions-gateway/src/lib.rs @@ -16,7 +16,7 @@ pub mod api; pub mod cli; pub mod config; pub mod credential; -pub mod credential_injection_kdc; +pub mod credential_injection; pub mod extract; pub mod generic_client; pub mod http; @@ -62,7 +62,8 @@ pub struct DgwState { pub shutdown_signal: devolutions_gateway_task::ShutdownSignal, pub recordings: recording::RecordingMessageSender, pub job_queue_handle: job_queue::JobQueueHandle, - pub credentials: credential_injection_kdc::CredentialService, + pub provisioning: provisioning::ProvisioningStore, + pub synthetic_kdc_registry: credential_injection::SyntheticKdcRegistry, pub monitoring_state: Arc, pub traffic_audit_handle: traffic_audit::TrafficAuditHandle, pub agent_tunnel_handle: Option>, @@ -90,7 +91,8 @@ impl DgwState { let (shutdown_handle, shutdown_signal) = devolutions_gateway_task::ShutdownHandle::new(); let (job_queue_handle, job_queue_rx) = job_queue::JobQueueHandle::new(); let (traffic_audit_handle, traffic_audit_rx) = traffic_audit::TrafficAuditHandle::new(); - let credentials = credential_injection_kdc::CredentialService::new(conf_handle.clone()); + let provisioning = provisioning::ProvisioningStore::new(); + let synthetic_kdc_registry = credential_injection::SyntheticKdcRegistry::new(); let monitoring_state = Arc::new(network_monitor::State::new(Arc::new(MockMonitorsCache))?); let state = Self { @@ -103,7 +105,8 @@ impl DgwState { recordings: recording_manager_handle, job_queue_handle, traffic_audit_handle, - credentials, + provisioning, + synthetic_kdc_registry, monitoring_state, agent_tunnel_handle: None, }; diff --git a/devolutions-gateway/src/listener.rs b/devolutions-gateway/src/listener.rs index 5e23f5f8a..6dd0b179b 100644 --- a/devolutions-gateway/src/listener.rs +++ b/devolutions-gateway/src/listener.rs @@ -158,7 +158,8 @@ 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) + .provisioning(state.provisioning) + .synthetic_kdc_registry(state.synthetic_kdc_registry) .agent_tunnel_handle(state.agent_tunnel_handle) .build() .serve() diff --git a/devolutions-gateway/src/ngrok.rs b/devolutions-gateway/src/ngrok.rs index 9e2e846bd..9adb561a7 100644 --- a/devolutions-gateway/src/ngrok.rs +++ b/devolutions-gateway/src/ngrok.rs @@ -237,7 +237,8 @@ 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) + .provisioning(state.provisioning) + .synthetic_kdc_registry(state.synthetic_kdc_registry) .agent_tunnel_handle(state.agent_tunnel_handle) .build() .serve() diff --git a/devolutions-gateway/src/provisioning.rs b/devolutions-gateway/src/provisioning.rs index 06b21c40b..76da7a671 100644 --- a/devolutions-gateway/src/provisioning.rs +++ b/devolutions-gateway/src/provisioning.rs @@ -44,8 +44,6 @@ pub struct ProvisioningEntry { pub(crate) connection_options: Option, } -pub type ArcProvisioningEntry = Arc; - #[derive(Debug, Clone)] struct CredentialsEntry { token: String, @@ -131,40 +129,41 @@ impl ProvisioningStore { self.connection_options.lock().insert(jti, entry).is_some() } - /// Assemble the provisioned view for a session. + /// Take the provisioned view for a session (one-shot). /// - /// Returns `None` unless the credentials half (token and/or mapping) is present and live. - /// Folds in connection options when that half is also present and live. - pub(crate) fn get(&self, jti: Uuid) -> Option { + /// Removes the credentials half (required) and any live connection-options half for `jti`. + /// Returns `None` if credentials are missing or expired. A second `take` for the same JTI + /// fails until preflight inserts again. + pub(crate) fn take(&self, jti: Uuid) -> Option { let now = time::OffsetDateTime::now_utc(); let (token, mapping) = { - let entries = self.credentials.lock(); - let entry = entries.get(&jti)?; + let mut entries = self.credentials.lock(); + let entry = entries.remove(&jti)?; if now >= entry.expires_at { warn!(%jti, "Provisioned credentials expired before the connection arrived"); return None; } - (entry.token.clone(), entry.mapping.clone()) + (entry.token, entry.mapping) }; - let connection_options = self.get_live_connection_options(jti, now); + let connection_options = { + let mut entries = self.connection_options.lock(); + match entries.remove(&jti) { + Some(entry) if now < entry.expires_at => Some(entry.connection_options), + Some(_) => { + warn!(%jti, "Provisioned connection options expired before the connection arrived"); + None + } + None => None, + } + }; - Some(Arc::new(ProvisioningEntry { + Some(ProvisioningEntry { token, mapping, connection_options, - })) - } - - fn get_live_connection_options(&self, jti: Uuid, now: time::OffsetDateTime) -> Option { - let entries = self.connection_options.lock(); - let entry = entries.get(&jti)?; - if now >= entry.expires_at { - warn!(%jti, "Provisioned connection options expired before the connection arrived"); - return None; - } - Some(entry.connection_options.clone()) + }) } } @@ -252,49 +251,54 @@ mod tests { } #[test] - fn get_returns_token_only_entry() { + fn take_returns_token_only_entry() { let store = ProvisioningStore::new(); let jti = Uuid::new_v4(); store .insert_credentials(association_token(jti), None, time::Duration::minutes(5)) .expect("insert"); - let entry = store.get(jti).expect("live entry"); + let entry = store.take(jti).expect("live entry"); assert!(entry.mapping.is_none()); assert!(entry.connection_options.is_none()); + assert!(store.take(jti).is_none(), "second take is empty"); } #[test] - fn get_returns_live_credentials_without_options() { + fn take_returns_live_credentials_without_options() { let store = ProvisioningStore::new(); let jti = Uuid::new_v4(); store .insert_credentials(association_token(jti), Some(mapping()), time::Duration::minutes(5)) .expect("insert"); - let entry = store.get(jti).expect("live entry"); + let entry = store.take(jti).expect("live entry"); assert!(entry.mapping.is_some()); assert!(entry.connection_options.is_none()); } #[test] - fn get_folds_in_live_connection_options() { + fn take_folds_in_and_consumes_connection_options() { let store = ProvisioningStore::new(); let jti = Uuid::new_v4(); store .insert_credentials(association_token(jti), Some(mapping()), time::Duration::minutes(5)) .expect("insert credentials"); assert!(!store.insert_connection_options(jti, options(), time::Duration::minutes(5))); - let entry = store.get(jti).expect("live entry"); + let entry = store.take(jti).expect("live entry"); assert!(entry.connection_options.is_some()); + assert!(store.take(jti).is_none()); + // options half was removed with take; re-insert options alone does not revive credentials + assert!(!store.insert_connection_options(jti, options(), time::Duration::minutes(5))); + assert!(store.take(jti).is_none()); } #[test] - fn get_treats_expired_credentials_as_absent() { + fn take_treats_expired_credentials_as_absent() { let store = ProvisioningStore::new(); let jti = Uuid::new_v4(); store .insert_credentials(association_token(jti), Some(mapping()), time::Duration::seconds(-1)) .expect("insert"); - assert!(store.get(jti).is_none()); + assert!(store.take(jti).is_none()); } #[test] diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index ca7376bd8..c4c104a7a 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -16,7 +16,8 @@ use tracing::field; const PCB_TRANSMIT_DEADLINE: Duration = Duration::from_secs(10); use crate::config::Conf; -use crate::credential_injection_kdc::{CredentialInjectionKdc, CredentialService}; +use crate::credential_injection::{CredentialInjection, SyntheticKdcRegistry}; +use crate::provisioning::ProvisioningStore; use crate::proxy::Proxy; use crate::recording::ActiveRecordings; use crate::session::{ConnectionModeDetails, DisconnectInterest, DisconnectedInfo, SessionInfo, SessionMessageSender}; @@ -437,7 +438,7 @@ async fn handle_with_credential_injection( subscriber_tx: SubscriberSender, active_recordings: &ActiveRecordings, cleanpath_pdu: RDCleanPathPdu, - credential_injection_kdc: CredentialInjectionKdc, + credential_injection: CredentialInjection, agent_tunnel_handle: Option>, ) -> anyhow::Result<()> { let tls_conf = conf.credssp_tls.get().context("CredSSP TLS configuration")?; @@ -538,24 +539,15 @@ async fn handle_with_credential_injection( let mut client_framed = ironrdp_tokio::MovableTokioFramed::new(client_stream); let mut server_framed = ironrdp_tokio::MovableTokioFramed::new(server_stream); - let krb_configs = crate::rdp_proxy::credential_injection_kerberos_configs( - &conf, - client_addr, - &gateway_hostname, - &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(), + client_addr, gateway_public_key, client_security_protocol, - credential_injection_kdc.proxy_credential(), - krb_configs.server, - &credential_injection_kdc, + &credential_injection, &kdc_connector, ); @@ -564,8 +556,8 @@ async fn handle_with_credential_injection( destination.host().to_owned(), server_public_key, server_security_protocol, - credential_injection_kdc.target_credential(), - krb_configs.client, + &credential_injection, + &gateway_hostname, &kdc_connector, ); @@ -639,7 +631,8 @@ pub async fn handle( sessions: SessionMessageSender, subscriber_tx: SubscriberSender, active_recordings: &ActiveRecordings, - credentials: &CredentialService, + provisioning: &ProvisioningStore, + synthetic_kdc_registry: &SyntheticKdcRegistry, agent_tunnel_handle: Option>, ) -> anyhow::Result<()> { // Special handshake of our RDP extension @@ -660,7 +653,7 @@ pub async fn handle( // proxy-based credential injection mode. Otherwise, we continue the usual // clean path procedure. The credential store is keyed on the association token's JTI. if let Some(jti) = crate::token::extract_jti(token).ok() - && let Some(entry) = credentials.get(jti) + && let Some(entry) = provisioning.take(jti) && entry.mapping.is_some() { // VMConnect needs pre-X.224 CredSSP against the Hyper-V host cert on the client. @@ -671,10 +664,13 @@ pub async fn handle( anyhow::bail!("credential injection is not supported for VMConnect RDCleanPath"); } - let credential_injection_kdc = credentials.kdc_for(jti)?; - anyhow::ensure!(token == credential_injection_kdc.raw_token(), "token mismatch"); + anyhow::ensure!(token == entry.token, "token mismatch"); + let kerberos_enabled = conf.debug.enable_unstable && conf.debug.kerberos_credential_injection; + let credential_injection = CredentialInjection::from_provisioned(jti, entry, kerberos_enabled)? + .register_if_kerberos(synthetic_kdc_registry); debug!( - jti = %credential_injection_kdc.jti(), + jti = %credential_injection.jti(), + kerberos = credential_injection.uses_kerberos(), "Switching to RdpProxy for credential injection (WebSocket)" ); @@ -688,7 +684,7 @@ pub async fn handle( subscriber_tx, active_recordings, cleanpath_pdu, - credential_injection_kdc, + credential_injection, agent_tunnel_handle.clone(), ) .await; diff --git a/devolutions-gateway/src/rdp_proxy.rs b/devolutions-gateway/src/rdp_proxy.rs deleted file mode 100644 index 254a71df8..000000000 --- a/devolutions-gateway/src/rdp_proxy.rs +++ /dev/null @@ -1,846 +0,0 @@ -use std::net::{IpAddr, SocketAddr}; -use std::sync::Arc; - -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; -use ironrdp_pdu::{mcs, nego, x224}; -use secrecy::ExposeSecret as _; -use tokio::io::{AsyncRead, AsyncWrite, AsyncWriteExt}; -use typed_builder::TypedBuilder; - -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; - -#[derive(TypedBuilder)] -pub struct RdpProxy { - conf: Arc, - session_info: SessionInfo, - client_stream: C, - client_addr: SocketAddr, - server_stream: S, - server_addr: SocketAddr, - credential_injection_kdc: CredentialInjectionKdc, - client_stream_leftover_bytes: bytes::BytesMut, - sessions: SessionMessageSender, - 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 -where - A: AsyncWrite + AsyncRead + Unpin + Send, - B: AsyncWrite + AsyncRead + Unpin + Send, -{ - pub async fn run(self) -> anyhow::Result<()> { - handle(self).await - } -} - -#[instrument("rdp_proxy", skip_all, fields(session_id = proxy.session_info.id.to_string(), target = proxy.server_addr.to_string()))] -async fn handle(proxy: RdpProxy) -> anyhow::Result<()> -where - C: AsyncRead + AsyncWrite + Unpin + Send, - S: AsyncRead + AsyncWrite + Unpin + Send, -{ - let RdpProxy { - conf, - session_info, - client_stream, - client_addr, - server_stream, - server_addr, - credential_injection_kdc, - client_stream_leftover_bytes, - sessions, - subscriber_tx, - server_dns_name, - disconnect_interest, - kdc_connector, - } = proxy; - - let tls_conf = conf.credssp_tls.get().context("CredSSP TLS configuration")?; - let gateway_hostname = conf.hostname.clone(); - - // -- Retrieve the Gateway TLS public key that must be used for client-proxy CredSSP later on -- // - - let gateway_cert_chain_handle = tokio::spawn(crate::tls::get_cert_chain_for_acceptor_cached( - gateway_hostname.clone(), - tls_conf.acceptor.clone(), - )); - - // -- Dual handshake with the client and the server until the TLS security upgrade -- // - - let mut client_framed = - ironrdp_tokio::MovableTokioFramed::new_with_leftover(client_stream, client_stream_leftover_bytes); - let mut server_framed = ironrdp_tokio::MovableTokioFramed::new(server_stream); - - let handshake_result = dual_handshake_until_tls_upgrade( - &mut client_framed, - &mut server_framed, - credential_injection_kdc.target_credential(), - ) - .await?; - - let client_stream = client_framed.into_inner_no_leftover(); - let server_stream = server_framed.into_inner_no_leftover(); - - // -- Perform the TLS upgrading for both the client and the server, effectively acting as a man-in-the-middle -- // - - let client_tls_upgrade_fut = tls_conf.acceptor.accept(client_stream); - let server_tls_upgrade_fut = crate::tls::dangerous_connect(server_dns_name.clone(), server_stream); - - let (client_stream, server_stream) = tokio::join!(client_tls_upgrade_fut, server_tls_upgrade_fut); - - let client_stream = client_stream.context("TLS upgrade with client failed")?; - let server_stream = server_stream.context("TLS upgrade with server failed")?; - - let server_public_key = - crate::tls::extract_stream_peer_public_key(&server_stream).context("extract target server TLS public key")?; - - let gateway_cert_chain = gateway_cert_chain_handle.await??; - let gateway_public_key = crate::tls::extract_public_key(gateway_cert_chain.first().context("no leaf")?) - .context("extract Gateway public key")?; - - // -- Perform the CredSSP authentication with the client (acting as a server) and the server (acting as a client) -- // - - let mut client_framed = ironrdp_tokio::MovableTokioFramed::new(client_stream); - let mut server_framed = ironrdp_tokio::MovableTokioFramed::new(server_stream); - - let krb_configs = - credential_injection_kerberos_configs(&conf, client_addr, &gateway_hostname, &credential_injection_kdc)?; - - let client_credssp_fut = perform_credssp_as_server( - &mut client_framed, - client_addr.ip(), - gateway_public_key, - handshake_result.client_security_protocol, - credential_injection_kdc.proxy_credential(), - krb_configs.server, - &credential_injection_kdc, - &kdc_connector, - ); - - let server_credssp_fut = perform_credssp_as_client( - &mut server_framed, - server_dns_name, - server_public_key, - handshake_result.server_security_protocol, - credential_injection_kdc.target_credential(), - krb_configs.client, - &kdc_connector, - ); - - let (client_credssp_res, server_credssp_res) = tokio::join!(client_credssp_fut, server_credssp_fut); - client_credssp_res.context("CredSSP with client")?; - server_credssp_res.context("CredSSP with server")?; - - // -- Intercept the Connect Confirm PDU, to override the server_security_protocol field -- // - - intercept_connect_confirm( - &mut client_framed, - &mut server_framed, - handshake_result.server_security_protocol, - ) - .await?; - - let (mut client_stream, client_leftover) = client_framed.into_inner(); - let (mut server_stream, server_leftover) = server_framed.into_inner(); - - // -- At this point, proceed to the usual two-way forwarding -- // - - info!("RDP-TLS forwarding (credential injection)"); - - client_stream - .write_all(&server_leftover) - .await - .context("write server leftover to client")?; - - server_stream - .write_all(&client_leftover) - .await - .context("write client leftover to server")?; - - Proxy::builder() - .conf(conf) - .session_info(session_info) - .address_a(client_addr) - .transport_a(client_stream) - .address_b(server_addr) - .transport_b(server_stream) - .sessions(sessions) - .subscriber_tx(subscriber_tx) - .disconnect_interest(disconnect_interest) - .build() - .select_dissector_and_forward() - .await - .context("RDP-TLS traffic proxying failed")?; - - Ok(()) -} - -#[derive(Debug)] -struct HandshakeResult { - client_security_protocol: nego::SecurityProtocol, - server_security_protocol: nego::SecurityProtocol, -} - -#[instrument(level = "debug", ret, skip_all)] -pub(crate) async fn intercept_connect_confirm( - client_framed: &mut ironrdp_tokio::MovableTokioFramed, - server_framed: &mut ironrdp_tokio::MovableTokioFramed, - server_security_protocol: nego::SecurityProtocol, -) -> anyhow::Result<()> -where - C: AsyncWrite + AsyncRead + Unpin + Send, - S: AsyncWrite + AsyncRead + Unpin + Send, -{ - let (_, received_frame) = client_framed - .read_pdu() - .await - .context("read MCS Connect Initial from client")?; - let received_connect_initial: x224::X224> = - ironrdp_core::decode(&received_frame).context("decode PDU from client")?; - let mut received_connect_initial: mcs::ConnectInitial = - ironrdp_core::decode(&received_connect_initial.0.data).context("decode Connect Initial PDU")?; - trace!(message = ?received_connect_initial, "Received Connect Initial PDU from client"); - - let mut gcc_blocks = received_connect_initial.conference_create_request.into_gcc_blocks(); - gcc_blocks.core.optional_data.server_selected_protocol = Some(server_security_protocol); - // Update the conference request with modified gcc_blocks. - received_connect_initial.conference_create_request = ironrdp_pdu::gcc::ConferenceCreateRequest::new(gcc_blocks)?; - trace!(message = ?received_connect_initial, "Send Connection Request PDU to server"); - let x224_msg_buf = ironrdp_core::encode_vec(&received_connect_initial)?; - let pdu = x224::X224Data { - data: std::borrow::Cow::Owned(x224_msg_buf), - }; - send_pdu(server_framed, &x224::X224(pdu)) - .await - .context("send connection request to server")?; - - Ok(()) -} - -#[instrument(name = "dual_handshake", level = "debug", ret, skip_all)] -async fn dual_handshake_until_tls_upgrade( - client_framed: &mut ironrdp_tokio::MovableTokioFramed, - server_framed: &mut ironrdp_tokio::MovableTokioFramed, - target_credential: &AppCredential, -) -> anyhow::Result -where - C: AsyncWrite + AsyncRead + Unpin + Send, - S: AsyncWrite + AsyncRead + Unpin + Send, -{ - let (_, received_frame) = client_framed.read_pdu().await.context("read PDU from client")?; - let received_connection_request: x224::X224 = - ironrdp_core::decode(&received_frame).context("decode PDU from client")?; - trace!(message = ?received_connection_request, "Received Connection Request PDU from client"); - - // Choose the security protocol to use with the client. - let received_connection_request_protocol = received_connection_request.0.protocol; - let client_security_protocol = if received_connection_request_protocol.contains(nego::SecurityProtocol::HYBRID_EX) { - nego::SecurityProtocol::HYBRID_EX - } else if received_connection_request - .0 - .protocol - .contains(nego::SecurityProtocol::HYBRID) - { - nego::SecurityProtocol::HYBRID - } else { - anyhow::bail!( - "client does not support CredSSP (received {})", - received_connection_request.0.protocol - ) - }; - - let connection_request_to_send = nego::ConnectionRequest { - nego_data: match target_credential { - AppCredential::UsernamePassword { username, .. } => { - Some(nego::NegoRequestData::cookie(username.to_owned())) - } - }, - flags: received_connection_request.0.flags, - // https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/902b090b-9cb3-4efc-92bf-ee13373371e3 - // - // The spec states that `PROTOCOL_SSL` "SHOULD" also be set when using `PROTOCOL_HYBRID`: - // - // > PROTOCOL_HYBRID (0x00000002) - // > Credential Security Support Provider protocol (CredSSP) (section 5.4.5.2). - // > If this flag is set, then the PROTOCOL_SSL (0x00000001) flag SHOULD also be set - // > because Transport Layer Security (TLS) is a subset of CredSSP. - // - // However, in practice `mstsc` is picky about these flags: it expects the - // SupportedProtocol bits in the ConnectionRequestPDU that reach the target - // server to match what the client originally sent. If the proxy modifies - // them (for example, forcing HYBRID | HYBRID_EX and/or clearing SSL), - // the connection can fail with an authentication error (Code: 0x609). - // - // We therefore *do not* synthesize a new protocol bitmask here anymore. - // Instead, we forward the client's SupportedProtocol flags as-is and - // enforce our policy by validating them: if HYBRID / HYBRID_EX are not - // present (i.e. NLA is not negotiated), we fail the connection rather - // than trying to "fix" the flags ourselves. - // - // See also: https://serverfault.com/a/720161 - protocol: received_connection_request_protocol, - }; - trace!(?connection_request_to_send, "Send Connection Request PDU to server"); - send_pdu(server_framed, &x224::X224(connection_request_to_send)) - .await - .context("send connection request to server")?; - - let (_, received_frame) = server_framed.read_pdu().await.context("read PDU from server")?; - let received_connection_confirm: x224::X224 = - ironrdp_core::decode(&received_frame).context("decode PDU from server")?; - trace!(message = ?received_connection_confirm, "Received Connection Confirm PDU from server"); - - let (connection_confirm_to_send, handshake_result) = match &received_connection_confirm.0 { - nego::ConnectionConfirm::Response { - flags, - protocol: server_security_protocol, - } => { - debug!(?server_security_protocol, ?flags, "Server confirmed connection"); - - let result = if !server_security_protocol - .intersects(nego::SecurityProtocol::HYBRID | nego::SecurityProtocol::HYBRID_EX) - { - Err(anyhow::anyhow!( - "server selected security protocol {server_security_protocol}, which is not supported for credential injection" - )) - } else { - Ok(HandshakeResult { - client_security_protocol, - server_security_protocol: *server_security_protocol, - }) - }; - - ( - x224::X224(nego::ConnectionConfirm::Response { - flags: *flags, - protocol: client_security_protocol, - }), - result, - ) - } - nego::ConnectionConfirm::Failure { code } => ( - x224::X224(received_connection_confirm.0.clone()), - Err(anyhow::anyhow!("RDP session initiation failed with code {code}")), - ), - }; - - trace!(?connection_confirm_to_send, "Send Connection Request PDU to client"); - send_pdu(client_framed, &connection_confirm_to_send) - .await - .context("send connection confirm to client")?; - - handshake_result -} - -/// Kerberos configs for the two CredSSP legs of a credential-injection session. -/// -/// `server` drives the client-facing acceptor (Gateway-as-server); `client` drives the -/// target-facing leg (Gateway-as-client). `None` on a leg means that leg authenticates over NTLM. -pub(crate) struct CredentialInjectionKerberosConfigs { - pub server: Option, - pub client: Option, -} - -/// Whether a credential-injection session speaks Kerberos (vs NTLM). Decided once so both CredSSP -/// legs agree — sspi's acceptor and initiator must speak the same package or the handshake fails -/// reading one as the other. Kerberos needs the experimental opt-in AND a domain-qualified target -/// (a domainless account can't get a ticket). -fn injection_uses_kerberos( - enable_unstable: bool, - kerberos_credential_injection: bool, - protocol: CredentialInjectionClientAcceptorProtocol, -) -> bool { - enable_unstable - && kerberos_credential_injection - && matches!(protocol, CredentialInjectionClientAcceptorProtocol::Kerberos) -} - -/// Build the Kerberos config for both CredSSP legs from the single [`injection_uses_kerberos`] -/// decision. Everything else is NTLM on both legs. -pub(crate) fn credential_injection_kerberos_configs( - conf: &Conf, - client_addr: SocketAddr, - gateway_hostname: &str, - credential_injection_kdc: &CredentialInjectionKdc, -) -> anyhow::Result { - let protocol = credential_injection_kdc.client_acceptor_protocol()?; - - if !injection_uses_kerberos( - conf.debug.enable_unstable, - conf.debug.kerberos_credential_injection, - protocol, - ) { - return Ok(CredentialInjectionKerberosConfigs { - server: None, - client: None, - }); - } - - let krb_kdc = credential_injection_kdc - .krb_kdc() - .context("kerberos credential injection requires the krb_kdc target connection option")?; - - Ok(CredentialInjectionKerberosConfigs { - server: Some(credential_injection_kdc.server_kerberos_config(client_addr)?), - client: Some(ironrdp_connector::credssp::KerberosConfig { - kdc_proxy_url: Some(krb_kdc.clone()), - hostname: gateway_hostname.to_owned(), - }), - }) -} - -#[instrument(name = "server_credssp", level = "debug", ret, skip_all)] -pub(crate) async fn perform_credssp_as_client( - framed: &mut ironrdp_tokio::Framed, - server_name: String, - server_public_key: Vec, - security_protocol: nego::SecurityProtocol, - credentials: &AppCredential, - kerberos_config: Option, - kdc_connector: &KdcConnector, -) -> anyhow::Result<()> -where - S: ironrdp_tokio::FramedRead + ironrdp_tokio::FramedWrite, -{ - use ironrdp_tokio::FramedWrite as _; - - // Decrypt password into short-lived buffer. - let (username, decrypted_password) = credentials - .decrypt_password() - .context("failed to decrypt credentials")?; - - let credentials = ironrdp_connector::Credentials::UsernamePassword { - username, - password: decrypted_password.expose_secret().to_owned(), - }; - // decrypted_password drops here, zeroizing its buffer; note: a copy of the plaintext - // remains in `credentials` above, which is a regular String (downstream API limitation). - - let (mut sequence, mut ts_request) = ironrdp_connector::credssp::CredsspSequence::init( - credentials, - None, - security_protocol, - ironrdp_connector::ServerName::new(server_name.clone()), - server_public_key, - kerberos_config, - )?; - - let mut buf = ironrdp_pdu::WriteBuf::new(); - - loop { - let client_state = { - let mut generator = sequence.process_ts_request(ts_request); - resolve_client_generator(&mut generator, kdc_connector).await? - }; // drop generator - - buf.clear(); - let written = sequence.handle_process_result(client_state, &mut buf)?; - - if let Some(response_len) = written.size() { - let response = &buf[..response_len]; - framed - .write_all(response) - .await - .map_err(|e| ironrdp_connector::custom_err!("write all", e))?; - } - - let Some(next_pdu_hint) = sequence.next_pdu_hint() else { - break; - }; - - let pdu = framed.read_by_hint(next_pdu_hint).await.context("read frame by hint")?; - - if let Some(next_request) = sequence.decode_server_message(&pdu)? { - ts_request = next_request; - } else { - break; - } - } - - Ok(()) -} - -async fn resolve_server_generator( - generator: &mut CredsspServerProcessGenerator<'_>, - credential_injection_kdc: &CredentialInjectionKdc, - kdc_connector: &KdcConnector, -) -> Result { - let mut state = generator.start(); - - loop { - match state { - GeneratorState::Suspended(request) => { - let response = match credential_injection_kdc.intercept_network_request(&request) { - Ok(CredentialInjectionKdcInterception::Intercepted(response)) => Ok(response), - 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}" - )), - Err(error) => Err(error), - } - .map_err(|err| sspi::credssp::ServerError { - ts_request: None, - error: sspi::Error::new(sspi::ErrorKind::InternalError, err), - })?; - - state = generator.resume(Ok(response)); - } - GeneratorState::Completed(client_state) => { - break client_state; - } - } - } -} - -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 = kdc_connector.send_network_request(&request).await?; - state = generator.resume(Ok(response)); - } - GeneratorState::Completed(client_state) => { - break Ok(client_state.map_err(|e| { - ironrdp_connector::ConnectorError::new("CredSSP", ironrdp_connector::ConnectorErrorKind::Credssp(e)) - })?); - } - }; - } -} - -#[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, - client_addr: IpAddr, - gateway_public_key: Vec, - security_protocol: nego::SecurityProtocol, - credentials: &AppCredential, - kerberos_server_config: Option, - credential_injection_kdc: &CredentialInjectionKdc, - kdc_connector: &KdcConnector, -) -> anyhow::Result<()> -where - S: ironrdp_tokio::FramedRead + ironrdp_tokio::FramedWrite, -{ - use ironrdp_connector::sspi::credssp::EarlyUserAuthResult; - use ironrdp_tokio::FramedWrite as _; - - let mut buf = ironrdp_pdu::WriteBuf::new(); - - // Are we supposed to use the actual computer name of the client? - // But this does not seem to matter so far, so we stringify the IP address of the client instead. - let client_computer_name = ironrdp_connector::ServerName::new(client_addr.to_string()); - - let result = credssp_loop( - framed, - &mut buf, - client_computer_name, - gateway_public_key, - credentials, - kerberos_server_config, - credential_injection_kdc, - kdc_connector, - ) - .await; - - if security_protocol.intersects(nego::SecurityProtocol::HYBRID_EX) { - trace!(?result, "HYBRID_EX"); - - let result = if result.is_ok() { - EarlyUserAuthResult::Success - } else { - EarlyUserAuthResult::AccessDenied - }; - - buf.clear(); - result.to_buffer(&mut buf).context("write early user auth result")?; - let response = &buf[..result.buffer_len()]; - framed.write_all(response).await.context("write_all")?; - } - - return result; - - async fn credssp_loop( - framed: &mut ironrdp_tokio::Framed, - buf: &mut ironrdp_pdu::WriteBuf, - client_computer_name: ironrdp_connector::ServerName, - public_key: Vec, - credentials: &AppCredential, - kerberos_server_config: Option, - credential_injection_kdc: &CredentialInjectionKdc, - kdc_connector: &KdcConnector, - ) -> anyhow::Result<()> - where - S: ironrdp_tokio::FramedRead + ironrdp_tokio::FramedWrite, - { - // Decrypt password into short-lived buffer. - let (username, decrypted_password) = credentials - .decrypt_password() - .context("failed to decrypt credentials")?; - - let username = sspi::Username::parse(&username).context("invalid username")?; - - let identity = sspi::AuthIdentity { - username, - password: decrypted_password.expose_secret().to_owned().into(), - }; - // decrypted_password drops here, zeroizing its buffer; note: a copy of the plaintext - // remains in `identity` above (downstream API limitation). - - let mut sequence = ironrdp_acceptor::credssp::CredsspSequence::init( - &identity, - client_computer_name, - public_key, - kerberos_server_config, - )?; - - loop { - let Some(next_pdu_hint) = sequence.next_pdu_hint()? else { - break; - }; - - let pdu = framed - .read_by_hint(next_pdu_hint) - .await - .map_err(|e| ironrdp_connector::custom_err!("read frame by hint", e))?; - - let Some(ts_request) = sequence.decode_client_message(&pdu)? else { - break; - }; - - let result = { - let mut generator = sequence.process_ts_request(ts_request); - resolve_server_generator(&mut generator, credential_injection_kdc, kdc_connector).await - }; // drop generator - - buf.clear(); - let written = sequence.handle_process_result(result, buf)?; - - if let Some(response_len) = written.size() { - let response = &buf[..response_len]; - framed - .write_all(response) - .await - .map_err(|e| ironrdp_connector::custom_err!("write all", e))?; - } - } - - Ok(()) - } -} - -async fn send_pdu(framed: &mut ironrdp_tokio::MovableTokioFramed, pdu: &P) -> anyhow::Result<()> -where - S: AsyncWrite + Unpin + Send, - P: ironrdp_core::Encode, -{ - use ironrdp_tokio::FramedWrite as _; - - let payload = ironrdp_core::encode_vec(pdu).context("failed to encode PDU")?; - framed.write_all(&payload).await.context("failed to write PDU")?; - Ok(()) -} - -#[cfg(test)] -mod tests { - use std::net::{Ipv4Addr, SocketAddr}; - use std::sync::Arc; - - use base64::Engine as _; - use secrecy::SecretString; - use uuid::Uuid; - - use super::*; - use crate::config::ConfHandle; - use crate::credential::{CleartextAppCredential, CleartextAppCredentialMapping}; - use crate::credential_injection_kdc::CredentialService; - use crate::target_connection_options::TargetConnectionOptions; - - const TEST_CONFIG: &str = r#"{ - "Hostname": "dgateway.localhost.com", - "ProvisionerPublicKeyData": { - "Value": "mMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA4vuqLOkl1pWobt6su1XO9VskgCAwevEGs6kkNjJQBwkGnPKYLmNF1E/af1yCocfVn/OnPf9e4x+lXVyZ6LMDJxFxu+axdgOq3Ld392J1iAEbfvwlyRFnEXFOJNyylqg3bY6LvnWHL/XZczVdMD9xYfq2sO9bg3xjRW4s7r9EEYOFjqVT3VFznH9iWJVtcSEKukmS/3uKoO6lGhacvu0HhjXXdgq0R8zvR4XRJ9Fcnf0f9Ypoc+i6L80NVjrRCeVOH+Ld/2fA9bocpfLarcVqG3RjS+qgOtpyCc0jWVFF4zaGQ7LUDFkEIYILkICeMMn2ll29hmZNzsJzZJ9s6NocgQIDAQAB" - }, - "Listeners": [ - { "InternalUrl": "http://*:7171", "ExternalUrl": "https://*:7171" } - ], - "__debug__": { "disable_token_validation": true } - }"#; - - const KERBEROS_CONFIG: &str = r#"{ - "Hostname": "dgateway.localhost.com", - "ProvisionerPublicKeyData": { - "Value": "mMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA4vuqLOkl1pWobt6su1XO9VskgCAwevEGs6kkNjJQBwkGnPKYLmNF1E/af1yCocfVn/OnPf9e4x+lXVyZ6LMDJxFxu+axdgOq3Ld392J1iAEbfvwlyRFnEXFOJNyylqg3bY6LvnWHL/XZczVdMD9xYfq2sO9bg3xjRW4s7r9EEYOFjqVT3VFznH9iWJVtcSEKukmS/3uKoO6lGhacvu0HhjXXdgq0R8zvR4XRJ9Fcnf0f9Ypoc+i6L80NVjrRCeVOH+Ld/2fA9bocpfLarcVqG3RjS+qgOtpyCc0jWVFF4zaGQ7LUDFkEIYILkICeMMn2ll29hmZNzsJzZJ9s6NocgQIDAQAB" - }, - "Listeners": [ - { "InternalUrl": "http://*:7171", "ExternalUrl": "https://*:7171" } - ], - "__debug__": { - "disable_token_validation": true, - "enable_unstable": true, - "kerberos_credential_injection": true - } - }"#; - - fn conf(json: &str) -> Arc { - ConfHandle::mock(json).expect("test config is valid").get_conf() - } - - fn client_addr() -> SocketAddr { - SocketAddr::from((Ipv4Addr::LOCALHOST, 33_889)) - } - - fn association_token(jti: Uuid) -> String { - let engine = base64::engine::general_purpose::URL_SAFE_NO_PAD; - let header = engine.encode(r#"{"alg":"RS256"}"#); - let payload = engine.encode( - serde_json::to_vec(&serde_json::json!({ - "jti": jti, - "dst_hst": "target.example:3389" - })) - .expect("payload serializes"), - ); - let signature = engine.encode(b"signature"); - format!("{header}.{payload}.{signature}") - } - - fn mapping(target_username: &str) -> CleartextAppCredentialMapping { - CleartextAppCredentialMapping { - proxy: CleartextAppCredential::UsernamePassword { - username: "proxy@example.invalid".to_owned(), - password: SecretString::from("pwd"), - }, - target: CleartextAppCredential::UsernamePassword { - username: target_username.to_owned(), - password: SecretString::from("pwd"), - }, - } - } - - /// Provision credentials (and optional `krb_kdc`) then resolve the injection KDC — the - /// in-process path RDP takes before building CredSSP Kerberos configs. - fn provisioned_kdc(target_username: &str, krb_kdc: Option<&str>) -> CredentialInjectionKdc { - let service = CredentialService::new(ConfHandle::mock(TEST_CONFIG).expect("test config is valid")); - let jti = Uuid::new_v4(); - service - .insert_credentials( - association_token(jti), - Some(mapping(target_username)), - time::Duration::minutes(5), - ) - .expect("credentials insert"); - if let Some(krb_kdc) = krb_kdc { - let options = TargetConnectionOptions::new(Some(krb_kdc)).expect("valid krb_kdc"); - service.insert_connection_options(jti, options, time::Duration::minutes(5)); - } - service.kdc_for(jti).expect("kdc_for resolves provisioned state") - } - - // The two CredSSP legs are built from this single decision, so agreement is guaranteed by - // construction. These cases pin the decision itself (the bug was the two legs deciding - // independently): Kerberos requires BOTH opt-in flags AND a domain-qualified target. - #[test] - fn injection_uses_kerberos_requires_optin_and_domain_qualified_target() { - use CredentialInjectionClientAcceptorProtocol::{Kerberos, Ntlm}; - - assert!(injection_uses_kerberos(true, true, Kerberos)); - - // Either opt-in off => NTLM, even for a Kerberos-capable target. - assert!(!injection_uses_kerberos(false, true, Kerberos)); - assert!(!injection_uses_kerberos(true, false, Kerberos)); - - // Domainless target can't get a ticket => NTLM regardless of the flags. - assert!(!injection_uses_kerberos(true, true, Ntlm)); - assert!(!injection_uses_kerberos(false, false, Ntlm)); - } - - #[test] - fn provisioned_krb_kdc_becomes_client_kdc_proxy_url() { - let conf = conf(KERBEROS_CONFIG); - let kdc = provisioned_kdc("administrator@example.invalid", Some("tcp://dc.example.com:88")); - - let configs = - credential_injection_kerberos_configs(conf.as_ref(), client_addr(), "dgateway.localhost.com", &kdc) - .expect("kerberos configs build when krb_kdc is provisioned"); - - let client = configs.client.expect("client leg speaks Kerberos"); - assert_eq!( - client.kdc_proxy_url.as_ref().map(url::Url::as_str), - Some("tcp://dc.example.com:88"), - "target-side CredSSP must use the provisioned KDC URL", - ); - assert_eq!(client.hostname, "dgateway.localhost.com"); - assert!(configs.server.is_some(), "both CredSSP legs must agree on Kerberos"); - } - - #[test] - fn kerberos_path_requires_provisioned_krb_kdc() { - let conf = conf(KERBEROS_CONFIG); - let kdc = provisioned_kdc("administrator@example.invalid", None); - - let error = - match credential_injection_kerberos_configs(conf.as_ref(), client_addr(), "dgateway.localhost.com", &kdc) { - Ok(_) => panic!("Kerberos without krb_kdc must fail before CredSSP starts"), - Err(error) => error, - }; - - assert!( - format!("{error:#}").contains("krb_kdc"), - "error should name the missing connection option, got: {error:#}", - ); - } - - #[test] - fn ntlm_path_does_not_require_krb_kdc() { - // Domainless target → NTLM decision even with Kerberos feature flags on. - let conf = conf(KERBEROS_CONFIG); - let kdc = provisioned_kdc("Administrator", None); - - let configs = - credential_injection_kerberos_configs(conf.as_ref(), client_addr(), "dgateway.localhost.com", &kdc) - .expect("NTLM path succeeds without connection options"); - - assert!(configs.client.is_none()); - assert!(configs.server.is_none()); - } - - #[test] - fn kerberos_flags_off_does_not_require_krb_kdc() { - // Domain-qualified target but feature flags off → NTLM on both legs. - let conf = conf(TEST_CONFIG); - let kdc = provisioned_kdc("administrator@example.invalid", None); - - let configs = - credential_injection_kerberos_configs(conf.as_ref(), client_addr(), "dgateway.localhost.com", &kdc) - .expect("flags off means NTLM without needing krb_kdc"); - - assert!(configs.client.is_none()); - assert!(configs.server.is_none()); - } -} diff --git a/devolutions-gateway/src/rdp_proxy/credssp.rs b/devolutions-gateway/src/rdp_proxy/credssp.rs new file mode 100644 index 000000000..db47beb87 --- /dev/null +++ b/devolutions-gateway/src/rdp_proxy/credssp.rs @@ -0,0 +1,571 @@ +//! CredSSP MITM for proxy-based RDP credential injection. +//! +//! Enclosed here so [`super::RdpProxy`] only orchestrates handshake and TLS upgrade. +//! The dual CredSSP legs, Kerberos config derivation, Connect Confirm intercept, and the +//! post-auth forward all live in [`CredsspSession::run`]. + +use std::net::SocketAddr; +use std::sync::Arc; + +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; +use ironrdp_pdu::{mcs, nego, x224}; +use secrecy::ExposeSecret as _; +use tokio::io::{AsyncRead, AsyncWrite, AsyncWriteExt as _}; +use typed_builder::TypedBuilder; + +use super::send_pdu; +use crate::config::Conf; +use crate::credential::AppCredential; +use crate::credential_injection::{CredentialInjection, CredentialInjectionKdc, CredentialInjectionKdcInterception}; +use crate::kdc_connector::KdcConnector; +use crate::proxy::Proxy; +use crate::session::{DisconnectInterest, SessionInfo, SessionMessageSender}; +use crate::subscriber::SubscriberSender; + +/// Long-lived inputs for the CredSSP MITM + forward phase. +#[derive(TypedBuilder)] +pub(crate) struct CredsspSession { + conf: Arc, + session_info: SessionInfo, + client_addr: SocketAddr, + server_addr: SocketAddr, + credential_injection: CredentialInjection, + sessions: SessionMessageSender, + subscriber_tx: SubscriberSender, + server_dns_name: String, + disconnect_interest: Option, + kdc_connector: KdcConnector, +} + +/// Streams and keys collected after TLS upgrade, ready for CredSSP. +#[derive(TypedBuilder)] +pub(crate) struct PreparedCredssp { + client_stream: C, + server_stream: S, + gateway_public_key: Vec, + server_public_key: Vec, + client_security_protocol: nego::SecurityProtocol, + server_security_protocol: nego::SecurityProtocol, +} + +impl CredsspSession { + pub(super) fn conf(&self) -> &Conf { + &self.conf + } + + pub(super) fn server_dns_name(&self) -> &str { + &self.server_dns_name + } + + pub(super) fn target_credential(&self) -> &AppCredential { + self.credential_injection.target_credential() + } + + /// Run both CredSSP legs, fix Connect Confirm, then forward RDP-TLS. + pub(crate) async fn run(self, prepared: PreparedCredssp) -> anyhow::Result<()> + where + C: AsyncRead + AsyncWrite + Unpin + Send, + S: AsyncRead + AsyncWrite + Unpin + Send, + { + let Self { + conf, + session_info, + client_addr, + server_addr, + credential_injection, + sessions, + subscriber_tx, + server_dns_name, + disconnect_interest, + kdc_connector, + } = self; + let PreparedCredssp { + client_stream, + server_stream, + gateway_public_key, + server_public_key, + client_security_protocol, + server_security_protocol, + } = prepared; + + let mut client_framed = ironrdp_tokio::MovableTokioFramed::new(client_stream); + let mut server_framed = ironrdp_tokio::MovableTokioFramed::new(server_stream); + + let client_credssp_fut = perform_credssp_as_server( + &mut client_framed, + client_addr, + gateway_public_key, + client_security_protocol, + &credential_injection, + &kdc_connector, + ); + + let server_credssp_fut = perform_credssp_as_client( + &mut server_framed, + server_dns_name, + server_public_key, + server_security_protocol, + &credential_injection, + &conf.hostname, + &kdc_connector, + ); + + let (client_credssp_res, server_credssp_res) = tokio::join!(client_credssp_fut, server_credssp_fut); + client_credssp_res.context("CredSSP with client")?; + server_credssp_res.context("CredSSP with server")?; + + intercept_connect_confirm(&mut client_framed, &mut server_framed, server_security_protocol).await?; + + let (mut client_stream, client_leftover) = client_framed.into_inner(); + let (mut server_stream, server_leftover) = server_framed.into_inner(); + + info!("RDP-TLS forwarding (credential injection)"); + + client_stream + .write_all(&server_leftover) + .await + .context("write server leftover to client")?; + + server_stream + .write_all(&client_leftover) + .await + .context("write client leftover to server")?; + + Proxy::builder() + .conf(conf) + .session_info(session_info) + .address_a(client_addr) + .transport_a(client_stream) + .address_b(server_addr) + .transport_b(server_stream) + .sessions(sessions) + .subscriber_tx(subscriber_tx) + .disconnect_interest(disconnect_interest) + .build() + .select_dissector_and_forward() + .await + .context("RDP-TLS traffic proxying failed")?; + + Ok(()) + } +} + +pub(crate) async fn intercept_connect_confirm( + client_framed: &mut ironrdp_tokio::MovableTokioFramed, + server_framed: &mut ironrdp_tokio::MovableTokioFramed, + server_security_protocol: nego::SecurityProtocol, +) -> anyhow::Result<()> +where + C: AsyncWrite + AsyncRead + Unpin + Send, + S: AsyncWrite + AsyncRead + Unpin + Send, +{ + let (_, received_frame) = client_framed + .read_pdu() + .await + .context("read MCS Connect Initial from client")?; + let received_connect_initial: x224::X224> = + ironrdp_core::decode(&received_frame).context("decode PDU from client")?; + let mut received_connect_initial: mcs::ConnectInitial = + ironrdp_core::decode(&received_connect_initial.0.data).context("decode Connect Initial PDU")?; + trace!(message = ?received_connect_initial, "Received Connect Initial PDU from client"); + + let mut gcc_blocks = received_connect_initial.conference_create_request.into_gcc_blocks(); + gcc_blocks.core.optional_data.server_selected_protocol = Some(server_security_protocol); + // Update the conference request with modified gcc_blocks. + received_connect_initial.conference_create_request = ironrdp_pdu::gcc::ConferenceCreateRequest::new(gcc_blocks)?; + trace!(message = ?received_connect_initial, "Send Connection Request PDU to server"); + let x224_msg_buf = ironrdp_core::encode_vec(&received_connect_initial)?; + let pdu = x224::X224Data { + data: std::borrow::Cow::Owned(x224_msg_buf), + }; + send_pdu(server_framed, &x224::X224(pdu)) + .await + .context("send connection request to server")?; + + Ok(()) +} + +fn server_kerberos_setup( + client_addr: SocketAddr, + injection: &CredentialInjection, +) -> anyhow::Result<(Option, Option<&CredentialInjectionKdc>)> { + let Some(kerberos) = injection.as_kerberos() else { + return Ok((None, None)); + }; + let synthetic = kerberos.synthetic_kdc(); + Ok((Some(synthetic.server_kerberos_config(client_addr)?), Some(synthetic))) +} + +fn client_kerberos_config( + gateway_hostname: &str, + injection: &CredentialInjection, +) -> anyhow::Result> { + let Some(kerberos) = injection.as_kerberos() else { + return Ok(None); + }; + Ok(Some(ironrdp_connector::credssp::KerberosConfig { + kdc_proxy_url: Some(kerberos.target_kdc().clone()), + hostname: gateway_hostname.to_owned(), + })) +} + +#[instrument(name = "server_credssp", level = "debug", ret, skip_all)] +pub(crate) async fn perform_credssp_as_client( + framed: &mut ironrdp_tokio::Framed, + server_name: String, + server_public_key: Vec, + security_protocol: nego::SecurityProtocol, + injection: &CredentialInjection, + gateway_hostname: &str, + kdc_connector: &KdcConnector, +) -> anyhow::Result<()> +where + S: ironrdp_tokio::FramedRead + ironrdp_tokio::FramedWrite, +{ + use ironrdp_tokio::FramedWrite as _; + + let credentials = injection.target_credential(); + let kerberos_config = client_kerberos_config(gateway_hostname, injection)?; + + // Decrypt password into short-lived buffer. + let (username, decrypted_password) = credentials + .decrypt_password() + .context("failed to decrypt credentials")?; + + let credentials = ironrdp_connector::Credentials::UsernamePassword { + username, + password: decrypted_password.expose_secret().to_owned(), + }; + // decrypted_password drops here, zeroizing its buffer; note: a copy of the plaintext + // remains in `credentials` above, which is a regular String (downstream API limitation). + + let (mut sequence, mut ts_request) = ironrdp_connector::credssp::CredsspSequence::init( + credentials, + None, + security_protocol, + ironrdp_connector::ServerName::new(server_name.clone()), + server_public_key, + kerberos_config, + )?; + + let mut buf = ironrdp_pdu::WriteBuf::new(); + + loop { + let client_state = { + let mut generator = sequence.process_ts_request(ts_request); + resolve_client_generator(&mut generator, kdc_connector).await? + }; // drop generator + + buf.clear(); + let written = sequence.handle_process_result(client_state, &mut buf)?; + + if let Some(response_len) = written.size() { + let response = &buf[..response_len]; + framed + .write_all(response) + .await + .map_err(|e| ironrdp_connector::custom_err!("write all", e))?; + } + + let Some(next_pdu_hint) = sequence.next_pdu_hint() else { + break; + }; + + let pdu = framed.read_by_hint(next_pdu_hint).await.context("read frame by hint")?; + + if let Some(next_request) = sequence.decode_server_message(&pdu)? { + ts_request = next_request; + } else { + break; + } + } + + Ok(()) +} + +async fn resolve_server_generator( + generator: &mut CredsspServerProcessGenerator<'_>, + credential_injection_kdc: Option<&CredentialInjectionKdc>, + kdc_connector: &KdcConnector, +) -> Result { + let mut state = generator.start(); + + loop { + match state { + GeneratorState::Suspended(request) => { + let kdc = credential_injection_kdc.ok_or_else(|| sspi::credssp::ServerError { + ts_request: None, + error: sspi::Error::new( + sspi::ErrorKind::InternalError, + "Kerberos CredSSP generator requires a synthetic KDC", + ), + })?; + let response = match kdc.intercept_network_request(&request) { + Ok(CredentialInjectionKdcInterception::Intercepted(response)) => Ok(response), + 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}" + )), + Err(error) => Err(error), + } + .map_err(|err| sspi::credssp::ServerError { + ts_request: None, + error: sspi::Error::new(sspi::ErrorKind::InternalError, err), + })?; + + state = generator.resume(Ok(response)); + } + GeneratorState::Completed(client_state) => { + break client_state; + } + } + } +} + +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 = kdc_connector.send_network_request(&request).await?; + state = generator.resume(Ok(response)); + } + GeneratorState::Completed(client_state) => { + break Ok(client_state.map_err(|e| { + ironrdp_connector::ConnectorError::new("CredSSP", ironrdp_connector::ConnectorErrorKind::Credssp(e)) + })?); + } + }; + } +} + +#[instrument(name = "client_credssp", level = "debug", ret, skip_all)] +pub(crate) async fn perform_credssp_as_server( + framed: &mut ironrdp_tokio::Framed, + client_addr: SocketAddr, + gateway_public_key: Vec, + security_protocol: nego::SecurityProtocol, + injection: &CredentialInjection, + kdc_connector: &KdcConnector, +) -> anyhow::Result<()> +where + S: ironrdp_tokio::FramedRead + ironrdp_tokio::FramedWrite, +{ + use ironrdp_connector::sspi::credssp::EarlyUserAuthResult; + use ironrdp_tokio::FramedWrite as _; + + let mut buf = ironrdp_pdu::WriteBuf::new(); + + // Are we supposed to use the actual computer name of the client? + // But this does not seem to matter so far, so we stringify the IP address of the client instead. + let client_computer_name = ironrdp_connector::ServerName::new(client_addr.ip().to_string()); + + let (kerberos_server_config, synthetic_kdc) = server_kerberos_setup(client_addr, injection)?; + let credentials = injection.proxy_credential(); + + let result = credssp_loop( + framed, + &mut buf, + client_computer_name, + gateway_public_key, + credentials, + kerberos_server_config, + synthetic_kdc, + kdc_connector, + ) + .await; + + if security_protocol.intersects(nego::SecurityProtocol::HYBRID_EX) { + trace!(?result, "HYBRID_EX"); + + let result = if result.is_ok() { + EarlyUserAuthResult::Success + } else { + EarlyUserAuthResult::AccessDenied + }; + + buf.clear(); + result.to_buffer(&mut buf).context("write early user auth result")?; + let response = &buf[..result.buffer_len()]; + framed.write_all(response).await.context("write_all")?; + } + + return result; + + #[allow(clippy::too_many_arguments)] + async fn credssp_loop( + framed: &mut ironrdp_tokio::Framed, + buf: &mut ironrdp_pdu::WriteBuf, + client_computer_name: ironrdp_connector::ServerName, + public_key: Vec, + credentials: &AppCredential, + kerberos_server_config: Option, + credential_injection_kdc: Option<&CredentialInjectionKdc>, + kdc_connector: &KdcConnector, + ) -> anyhow::Result<()> + where + S: ironrdp_tokio::FramedRead + ironrdp_tokio::FramedWrite, + { + // Decrypt password into short-lived buffer. + let (username, decrypted_password) = credentials + .decrypt_password() + .context("failed to decrypt credentials")?; + + let username = sspi::Username::parse(&username).context("invalid username")?; + + let identity = sspi::AuthIdentity { + username, + password: decrypted_password.expose_secret().to_owned().into(), + }; + // decrypted_password drops here, zeroizing its buffer; note: a copy of the plaintext + // remains in `identity` above (downstream API limitation). + + let mut sequence = ironrdp_acceptor::credssp::CredsspSequence::init( + &identity, + client_computer_name, + public_key, + kerberos_server_config, + )?; + + loop { + let Some(next_pdu_hint) = sequence.next_pdu_hint()? else { + break; + }; + + let pdu = framed + .read_by_hint(next_pdu_hint) + .await + .map_err(|e| ironrdp_connector::custom_err!("read frame by hint", e))?; + + let Some(ts_request) = sequence.decode_client_message(&pdu)? else { + break; + }; + + let result = { + let mut generator = sequence.process_ts_request(ts_request); + resolve_server_generator(&mut generator, credential_injection_kdc, kdc_connector).await + }; // drop generator + + buf.clear(); + let written = sequence.handle_process_result(result, buf)?; + + if let Some(response_len) = written.size() { + let response = &buf[..response_len]; + framed + .write_all(response) + .await + .map_err(|e| ironrdp_connector::custom_err!("write all", e))?; + } + } + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use secrecy::SecretString; + use uuid::Uuid; + + use super::*; + use crate::credential::{CleartextAppCredential, CleartextAppCredentialMapping}; + use crate::credential_injection::{CredentialInjection, SyntheticKdcRegistry}; + use crate::provisioning::ProvisioningStore; + use crate::target_connection_options::TargetConnectionOptions; + + fn association_token(jti: Uuid) -> String { + use base64::Engine as _; + let engine = base64::engine::general_purpose::URL_SAFE_NO_PAD; + let header = engine.encode(r#"{"alg":"RS256"}"#); + let payload = engine.encode( + serde_json::to_vec(&serde_json::json!({ + "jti": jti, + "dst_hst": "target.example:3389" + })) + .expect("payload serializes"), + ); + let signature = engine.encode(b"signature"); + format!("{header}.{payload}.{signature}") + } + + fn kerberos_injection() -> CredentialInjection { + let jti = Uuid::new_v4(); + let store = ProvisioningStore::new(); + store + .insert_credentials( + association_token(jti), + Some(CleartextAppCredentialMapping { + proxy: CleartextAppCredential::UsernamePassword { + username: "proxy@example.invalid".to_owned(), + password: SecretString::from("pwd"), + }, + target: CleartextAppCredential::UsernamePassword { + username: "administrator@example.invalid".to_owned(), + password: SecretString::from("pwd"), + }, + }), + time::Duration::minutes(5), + ) + .expect("credentials"); + let options = TargetConnectionOptions::new(Some("tcp://dc.example.com:88")).expect("kdc"); + store.insert_connection_options(jti, options, time::Duration::minutes(5)); + let entry = store.take(jti).expect("entry"); + CredentialInjection::from_provisioned(jti, entry, true) + .expect("prepared") + .register_if_kerberos(&SyntheticKdcRegistry::new()) + } + + #[test] + fn client_kerberos_config_uses_provisioned_target_kdc_url() { + let injection = kerberos_injection(); + let config = client_kerberos_config("dgateway.localhost.com", &injection) + .expect("config builds") + .expect("kerberos client leg"); + + assert_eq!( + config.kdc_proxy_url.as_ref().map(url::Url::as_str), + Some("tcp://dc.example.com:88"), + "CredSSP kdc_proxy_url must be the provisioned krb_kdc", + ); + assert_eq!(config.hostname, "dgateway.localhost.com"); + } + + #[test] + fn client_kerberos_config_is_none_for_ntlm() { + let jti = Uuid::new_v4(); + let store = ProvisioningStore::new(); + store + .insert_credentials( + association_token(jti), + Some(CleartextAppCredentialMapping { + proxy: CleartextAppCredential::UsernamePassword { + username: "proxy@example.invalid".to_owned(), + password: SecretString::from("pwd"), + }, + target: CleartextAppCredential::UsernamePassword { + username: "Administrator".to_owned(), + password: SecretString::from("pwd"), + }, + }), + time::Duration::minutes(5), + ) + .expect("credentials"); + let entry = store.take(jti).expect("entry"); + let injection = CredentialInjection::from_provisioned(jti, entry, true) + .expect("ntlm prepared") + .register_if_kerberos(&SyntheticKdcRegistry::new()); + + let config = client_kerberos_config("dgateway.localhost.com", &injection).expect("ntlm ok"); + assert!(config.is_none()); + } +} diff --git a/devolutions-gateway/src/rdp_proxy/mod.rs b/devolutions-gateway/src/rdp_proxy/mod.rs new file mode 100644 index 000000000..85ee1af48 --- /dev/null +++ b/devolutions-gateway/src/rdp_proxy/mod.rs @@ -0,0 +1,278 @@ +use std::net::SocketAddr; +use std::sync::Arc; + +use anyhow::Context as _; +use ironrdp_pdu::{nego, x224}; +use tokio::io::{AsyncRead, AsyncWrite}; +use typed_builder::TypedBuilder; + +mod credssp; + +use credssp::CredsspSession; +pub(crate) use credssp::{ + PreparedCredssp, intercept_connect_confirm, perform_credssp_as_client, perform_credssp_as_server, +}; + +use crate::config::Conf; +use crate::credential::AppCredential; +use crate::credential_injection::CredentialInjection; +use crate::kdc_connector::KdcConnector; +use crate::session::{DisconnectInterest, SessionInfo, SessionMessageSender}; +use crate::subscriber::SubscriberSender; + +/// RDP proxy for credential-injection sessions. +/// +/// The main path only orchestrates handshake and TLS upgrade. CredSSP MITM and the subsequent +/// forward live in [`CredsspSession`] / [`PreparedCredssp`]. +#[derive(TypedBuilder)] +pub struct RdpProxy { + conf: Arc, + session_info: SessionInfo, + client_stream: C, + client_addr: SocketAddr, + server_stream: S, + server_addr: SocketAddr, + credential_injection: CredentialInjection, + client_stream_leftover_bytes: bytes::BytesMut, + sessions: SessionMessageSender, + subscriber_tx: SubscriberSender, + server_dns_name: String, + disconnect_interest: Option, + kdc_connector: KdcConnector, +} + +impl RdpProxy +where + A: AsyncWrite + AsyncRead + Unpin + Send, + B: AsyncWrite + AsyncRead + Unpin + Send, +{ + pub async fn run(self) -> anyhow::Result<()> { + handle(self).await + } +} + +#[instrument("rdp_proxy", skip_all, fields(session_id = proxy.session_info.id.to_string(), target = proxy.server_addr.to_string()))] +async fn handle(proxy: RdpProxy) -> anyhow::Result<()> +where + C: AsyncRead + AsyncWrite + Unpin + Send, + S: AsyncRead + AsyncWrite + Unpin + Send, +{ + let RdpProxy { + conf, + session_info, + client_stream, + client_addr, + server_stream, + server_addr, + credential_injection, + client_stream_leftover_bytes, + sessions, + subscriber_tx, + server_dns_name, + disconnect_interest, + kdc_connector, + } = proxy; + + let session = CredsspSession::builder() + .conf(conf) + .session_info(session_info) + .client_addr(client_addr) + .server_addr(server_addr) + .credential_injection(credential_injection) + .sessions(sessions) + .subscriber_tx(subscriber_tx) + .server_dns_name(server_dns_name.clone()) + .disconnect_interest(disconnect_interest) + .kdc_connector(kdc_connector) + .build(); + + let tls_conf = session.conf().credssp_tls.get().context("CredSSP TLS configuration")?; + let gateway_hostname = session.conf().hostname.clone(); + + // -- Retrieve the Gateway TLS public key that must be used for client-proxy CredSSP later on -- // + + let gateway_cert_chain_handle = tokio::spawn(crate::tls::get_cert_chain_for_acceptor_cached( + gateway_hostname.clone(), + tls_conf.acceptor.clone(), + )); + + // -- Dual handshake with the client and the server until the TLS security upgrade -- // + + let mut client_framed = + ironrdp_tokio::MovableTokioFramed::new_with_leftover(client_stream, client_stream_leftover_bytes); + let mut server_framed = ironrdp_tokio::MovableTokioFramed::new(server_stream); + + let handshake_result = + dual_handshake_until_tls_upgrade(&mut client_framed, &mut server_framed, session.target_credential()).await?; + + let client_stream = client_framed.into_inner_no_leftover(); + let server_stream = server_framed.into_inner_no_leftover(); + + // -- Perform the TLS upgrading for both the client and the server, effectively acting as a man-in-the-middle -- // + + let client_tls_upgrade_fut = tls_conf.acceptor.accept(client_stream); + let server_tls_upgrade_fut = crate::tls::dangerous_connect(session.server_dns_name().to_owned(), server_stream); + + let (client_stream, server_stream) = tokio::join!(client_tls_upgrade_fut, server_tls_upgrade_fut); + + let client_stream = client_stream.context("TLS upgrade with client failed")?; + let server_stream = server_stream.context("TLS upgrade with server failed")?; + + let server_public_key = + crate::tls::extract_stream_peer_public_key(&server_stream).context("extract target server TLS public key")?; + + let gateway_cert_chain = gateway_cert_chain_handle.await??; + let gateway_public_key = crate::tls::extract_public_key(gateway_cert_chain.first().context("no leaf")?) + .context("extract Gateway public key")?; + + let prepared = PreparedCredssp::builder() + .client_stream(client_stream) + .server_stream(server_stream) + .gateway_public_key(gateway_public_key) + .server_public_key(server_public_key) + .client_security_protocol(handshake_result.client_security_protocol) + .server_security_protocol(handshake_result.server_security_protocol) + .build(); + + // CredSSP MITM + Connect Confirm intercept + bidirectional forward: owned by CredsspSession. + session.run(prepared).await +} + +#[derive(Debug)] +struct HandshakeResult { + client_security_protocol: nego::SecurityProtocol, + server_security_protocol: nego::SecurityProtocol, +} + +#[instrument(name = "dual_handshake", level = "debug", ret, skip_all)] +async fn dual_handshake_until_tls_upgrade( + client_framed: &mut ironrdp_tokio::MovableTokioFramed, + server_framed: &mut ironrdp_tokio::MovableTokioFramed, + target_credential: &AppCredential, +) -> anyhow::Result +where + C: AsyncWrite + AsyncRead + Unpin + Send, + S: AsyncWrite + AsyncRead + Unpin + Send, +{ + let (_, received_frame) = client_framed.read_pdu().await.context("read PDU from client")?; + let received_connection_request: x224::X224 = + ironrdp_core::decode(&received_frame).context("decode PDU from client")?; + trace!(message = ?received_connection_request, "Received Connection Request PDU from client"); + + // Choose the security protocol to use with the client. + let received_connection_request_protocol = received_connection_request.0.protocol; + let client_security_protocol = if received_connection_request_protocol.contains(nego::SecurityProtocol::HYBRID_EX) { + nego::SecurityProtocol::HYBRID_EX + } else if received_connection_request + .0 + .protocol + .contains(nego::SecurityProtocol::HYBRID) + { + nego::SecurityProtocol::HYBRID + } else { + anyhow::bail!( + "client does not support CredSSP (received {})", + received_connection_request.0.protocol + ) + }; + + let connection_request_to_send = nego::ConnectionRequest { + nego_data: match target_credential { + AppCredential::UsernamePassword { username, .. } => { + Some(nego::NegoRequestData::cookie(username.to_owned())) + } + }, + flags: received_connection_request.0.flags, + // https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/902b090b-9cb3-4efc-92bf-ee13373371e3 + // + // The spec states that `PROTOCOL_SSL` "SHOULD" also be set when using `PROTOCOL_HYBRID`: + // + // > PROTOCOL_HYBRID (0x00000002) + // > Credential Security Support Provider protocol (CredSSP) (section 5.4.5.2). + // > If this flag is set, then the PROTOCOL_SSL (0x00000001) flag SHOULD also be set + // > because Transport Layer Security (TLS) is a subset of CredSSP. + // + // However, in practice `mstsc` is picky about these flags: it expects the + // SupportedProtocol bits in the ConnectionRequestPDU that reach the target + // server to match what the client originally sent. If the proxy modifies + // them (for example, forcing HYBRID | HYBRID_EX and/or clearing SSL), + // the connection can fail with an authentication error (Code: 0x609). + // + // We therefore *do not* synthesize a new protocol bitmask here anymore. + // Instead, we forward the client's SupportedProtocol flags as-is and + // enforce our policy by validating them: if HYBRID / HYBRID_EX are not + // present (i.e. NLA is not negotiated), we fail the connection rather + // than trying to "fix" the flags ourselves. + // + // See also: https://serverfault.com/a/720161 + protocol: received_connection_request_protocol, + }; + trace!(?connection_request_to_send, "Send Connection Request PDU to server"); + send_pdu(server_framed, &x224::X224(connection_request_to_send)) + .await + .context("send connection request to server")?; + + let (_, received_frame) = server_framed.read_pdu().await.context("read PDU from server")?; + let received_connection_confirm: x224::X224 = + ironrdp_core::decode(&received_frame).context("decode PDU from server")?; + trace!(message = ?received_connection_confirm, "Received Connection Confirm PDU from server"); + + let (connection_confirm_to_send, handshake_result) = match &received_connection_confirm.0 { + nego::ConnectionConfirm::Response { + flags, + protocol: server_security_protocol, + } => { + debug!(?server_security_protocol, ?flags, "Server confirmed connection"); + + let result = if !server_security_protocol + .intersects(nego::SecurityProtocol::HYBRID | nego::SecurityProtocol::HYBRID_EX) + { + Err(anyhow::anyhow!( + "server selected security protocol {server_security_protocol}, which is not supported for credential injection" + )) + } else { + Ok(HandshakeResult { + client_security_protocol, + server_security_protocol: *server_security_protocol, + }) + }; + + ( + x224::X224(nego::ConnectionConfirm::Response { + flags: *flags, + protocol: client_security_protocol, + }), + result, + ) + } + nego::ConnectionConfirm::Failure { code } => ( + x224::X224(received_connection_confirm.0.clone()), + Err(anyhow::anyhow!("RDP session initiation failed with code {code}")), + ), + }; + + trace!(?connection_confirm_to_send, "Send Connection Request PDU to client"); + send_pdu(client_framed, &connection_confirm_to_send) + .await + .context("send connection confirm to client")?; + + handshake_result +} + +pub(super) async fn send_pdu(framed: &mut ironrdp_tokio::MovableTokioFramed, pdu: &P) -> anyhow::Result<()> +where + S: AsyncWrite + Unpin + Send, + P: ironrdp_core::Encode, +{ + use ironrdp_tokio::FramedWrite as _; + + let payload = ironrdp_core::encode_vec(pdu).context("failed to encode PDU")?; + framed.write_all(&payload).await.context("failed to write PDU")?; + Ok(()) +} + +#[cfg(test)] +mod tests { + // Protocol selection is owned by CredentialInjection (from_provisioned + register_if_kerberos). + // See credential_injection tests for Kerberos-vs-NTLM decision coverage. +} diff --git a/devolutions-gateway/src/service.rs b/devolutions-gateway/src/service.rs index 6602c9746..fa2f8b6c1 100644 --- a/devolutions-gateway/src/service.rs +++ b/devolutions-gateway/src/service.rs @@ -267,7 +267,8 @@ async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result { .await .context("failed to initialize traffic audit manager")?; - let credentials = devolutions_gateway::credential_injection_kdc::CredentialService::new(conf_handle.clone()); + let provisioning = devolutions_gateway::provisioning::ProvisioningStore::new(); + let synthetic_kdc_registry = devolutions_gateway::credential_injection::SyntheticKdcRegistry::new(); let filesystem_monitor_config_cache = devolutions_gateway::api::monitoring::FilesystemConfigCache::new( config::get_data_dir().join("monitors_cache.json"), @@ -315,7 +316,8 @@ async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result { shutdown_signal: tasks.shutdown_signal.clone(), recordings: recording_manager_handle.clone(), job_queue_handle: job_queue_ctx.job_queue_handle.clone(), - credentials: credentials.clone(), + provisioning: provisioning.clone(), + synthetic_kdc_registry: synthetic_kdc_registry.clone(), monitoring_state, traffic_audit_handle: traffic_audit_task.handle(), agent_tunnel_handle, @@ -350,11 +352,7 @@ async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result { tasks.register(devolutions_gateway::token::CleanupTask { token_cache }); - tasks.register(devolutions_gateway::provisioning::CleanupTask { - handle: credentials.credential_store().clone(), - }); - - tasks.register(devolutions_gateway::credential_injection_kdc::CleanupTask { service: credentials }); + tasks.register(devolutions_gateway::provisioning::CleanupTask { handle: provisioning }); tasks.register(devolutions_log::LogDeleterTask::::new( conf.log_file.clone(), From 459d3e5d252e6c7f59ef6f41eb39fd052a01aa94 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Fri, 31 Jul 2026 10:50:36 -0400 Subject: [PATCH 02/12] fix(dgw): address Copilot review on CredSSP/provisioning refactor Authorize CleanPath tokens before one-shot take, use a registry-wide generation counter, and replace bare clippy allow with expect. SPN remains association-token dst_hst for client-facing CredSSP. --- .../src/credential_injection.rs | 12 +++---- devolutions-gateway/src/provisioning.rs | 13 +++++++ devolutions-gateway/src/rd_clean_path.rs | 35 +++++++++++-------- devolutions-gateway/src/rdp_proxy/credssp.rs | 5 ++- 4 files changed, 44 insertions(+), 21 deletions(-) diff --git a/devolutions-gateway/src/credential_injection.rs b/devolutions-gateway/src/credential_injection.rs index 7a2341a7f..be7f19067 100644 --- a/devolutions-gateway/src/credential_injection.rs +++ b/devolutions-gateway/src/credential_injection.rs @@ -530,7 +530,8 @@ pub struct SyntheticKdcRegistry { #[derive(Debug, Default)] struct RegistryInner { live: HashMap, - next_generation: HashMap, + /// Registry-wide monotonic counter (not per-JTI) so generations stay unique without leaking map entries. + next_generation: u64, } #[derive(Debug, Clone)] @@ -572,16 +573,15 @@ impl SyntheticKdcRegistry { } } - fn allocate_generation(inner: &mut RegistryInner, jti: Uuid) -> u64 { - let slot = inner.next_generation.entry(jti).or_insert(0); - *slot = slot.wrapping_add(1); - *slot + fn allocate_generation(inner: &mut RegistryInner) -> u64 { + inner.next_generation = inner.next_generation.wrapping_add(1); + inner.next_generation } pub(crate) fn register(&self, kdc: Arc) -> SyntheticKdcRegistration { let jti = kdc.jti(); let mut inner = self.inner.lock(); - let generation = Self::allocate_generation(&mut inner, jti); + let generation = Self::allocate_generation(&mut inner); inner.live.insert(jti, PublishedSyntheticKdc { generation, kdc }); debug!(%jti, generation, "published synthetic KDC"); SyntheticKdcRegistration { diff --git a/devolutions-gateway/src/provisioning.rs b/devolutions-gateway/src/provisioning.rs index 76da7a671..e7cb50223 100644 --- a/devolutions-gateway/src/provisioning.rs +++ b/devolutions-gateway/src/provisioning.rs @@ -129,6 +129,19 @@ impl ProvisioningStore { self.connection_options.lock().insert(jti, entry).is_some() } + /// True when the credentials half is live and carries an injection mapping. + /// + /// Does not consume the entry — use before auth when deciding whether to take the + /// credential-injection path. + pub(crate) fn has_mapping(&self, jti: Uuid) -> bool { + let now = time::OffsetDateTime::now_utc(); + let entries = self.credentials.lock(); + match entries.get(&jti) { + Some(entry) if now < entry.expires_at => entry.mapping.is_some(), + _ => false, + } + } + /// Take the provisioned view for a session (one-shot). /// /// Removes the credentials half (required) and any live connection-options half for `jti`. diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index c4c104a7a..86e8e8f2d 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -438,7 +438,8 @@ async fn handle_with_credential_injection( subscriber_tx: SubscriberSender, active_recordings: &ActiveRecordings, cleanpath_pdu: RDCleanPathPdu, - credential_injection: CredentialInjection, + provisioning: &ProvisioningStore, + synthetic_kdc_registry: &SyntheticKdcRegistry, agent_tunnel_handle: Option>, ) -> anyhow::Result<()> { let tls_conf = conf.credssp_tls.get().context("CredSSP TLS configuration")?; @@ -482,6 +483,19 @@ async fn handle_with_credential_injection( .await .context("RDCleanPath authorization failed")?; + let token = cleanpath_pdu + .proxy_auth + .as_deref() + .context("missing token in RDCleanPath PDU")?; + let entry = provisioning + .take(claims.jti) + .context("provisioned credentials missing after authorization")?; + anyhow::ensure!(entry.mapping.is_some(), "provisioned entry has no credential mapping"); + anyhow::ensure!(token == entry.token, "token mismatch"); + let kerberos_enabled = conf.debug.enable_unstable && conf.debug.kerberos_credential_injection; + let credential_injection = CredentialInjection::from_provisioned(claims.jti, entry, kerberos_enabled)? + .register_if_kerberos(synthetic_kdc_registry); + let ConnectedRdpServer { tls_stream: server_stream, server_addr, @@ -651,10 +665,10 @@ pub async fn handle( // If a credential mapping has been pushed, we automatically switch to // proxy-based credential injection mode. Otherwise, we continue the usual - // clean path procedure. The credential store is keyed on the association token's JTI. + // clean path procedure. Peek only here — take after authorize_cleanpath so an + // unverified token cannot burn a victim JTI's one-shot groceries. if let Some(jti) = crate::token::extract_jti(token).ok() - && let Some(entry) = provisioning.take(jti) - && entry.mapping.is_some() + && provisioning.has_mapping(jti) { // VMConnect needs pre-X.224 CredSSP against the Hyper-V host cert on the client. // Proxy CredSSP MITM is X.224-first and is not supported for this ordering. @@ -664,15 +678,7 @@ pub async fn handle( anyhow::bail!("credential injection is not supported for VMConnect RDCleanPath"); } - anyhow::ensure!(token == entry.token, "token mismatch"); - let kerberos_enabled = conf.debug.enable_unstable && conf.debug.kerberos_credential_injection; - let credential_injection = CredentialInjection::from_provisioned(jti, entry, kerberos_enabled)? - .register_if_kerberos(synthetic_kdc_registry); - debug!( - jti = %credential_injection.jti(), - kerberos = credential_injection.uses_kerberos(), - "Switching to RdpProxy for credential injection (WebSocket)" - ); + debug!(%jti, "Switching to RdpProxy for credential injection (WebSocket)"); return handle_with_credential_injection( client_stream, @@ -684,7 +690,8 @@ pub async fn handle( subscriber_tx, active_recordings, cleanpath_pdu, - credential_injection, + provisioning, + synthetic_kdc_registry, agent_tunnel_handle.clone(), ) .await; diff --git a/devolutions-gateway/src/rdp_proxy/credssp.rs b/devolutions-gateway/src/rdp_proxy/credssp.rs index db47beb87..49d986652 100644 --- a/devolutions-gateway/src/rdp_proxy/credssp.rs +++ b/devolutions-gateway/src/rdp_proxy/credssp.rs @@ -402,7 +402,10 @@ where return result; - #[allow(clippy::too_many_arguments)] + #[expect( + clippy::too_many_arguments, + reason = "CredSSP loop needs framed IO, identity, optional synthetic KDC, and KdcConnector together", + )] async fn credssp_loop( framed: &mut ironrdp_tokio::Framed, buf: &mut ironrdp_pdu::WriteBuf, From 77d3c4c240da439c0d5c912e82bff9299e998efa Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Fri, 31 Jul 2026 10:58:21 -0400 Subject: [PATCH 03/12] style(dgw): rustfmt expect attribute --- devolutions-gateway/src/rdp_proxy/credssp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devolutions-gateway/src/rdp_proxy/credssp.rs b/devolutions-gateway/src/rdp_proxy/credssp.rs index 49d986652..5295b6e75 100644 --- a/devolutions-gateway/src/rdp_proxy/credssp.rs +++ b/devolutions-gateway/src/rdp_proxy/credssp.rs @@ -404,7 +404,7 @@ where #[expect( clippy::too_many_arguments, - reason = "CredSSP loop needs framed IO, identity, optional synthetic KDC, and KdcConnector together", + reason = "CredSSP loop needs framed IO, identity, optional synthetic KDC, and KdcConnector together" )] async fn credssp_loop( framed: &mut ironrdp_tokio::Framed, From f7900f60f7ec7a5d0547436c003bdc466e1ae679 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Fri, 14 Aug 2026 17:17:49 -0400 Subject: [PATCH 04/12] fix(dgw): pin injection destination to dst_hst Use association dst_hst for synthetic KDC SPN and target-leg Kerberos hostname instead of conf.hostname. Route RDCleanPath through CredsspSession, peek before one-shot take, and document checkout TTL. Issue: DGW review #1900 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/credential_injection.rs | 124 ++++++++++++----- devolutions-gateway/src/generic_client.rs | 21 ++- devolutions-gateway/src/openapi.rs | 8 +- devolutions-gateway/src/provisioning.rs | 11 +- devolutions-gateway/src/rd_clean_path.rs | 128 ++++++------------ devolutions-gateway/src/rdp_proxy/credssp.rs | 26 ++-- devolutions-gateway/src/rdp_proxy/mod.rs | 11 +- 7 files changed, 168 insertions(+), 161 deletions(-) diff --git a/devolutions-gateway/src/credential_injection.rs b/devolutions-gateway/src/credential_injection.rs index be7f19067..770d88886 100644 --- a/devolutions-gateway/src/credential_injection.rs +++ b/devolutions-gateway/src/credential_injection.rs @@ -1,8 +1,9 @@ -//! Credential-injection runtime: groceries → dish → synthetic KDC pass window. +//! Credential-injection runtime for RDP. //! -//! - Provisioned data lives in [`crate::provisioning::ProvisioningStore`] (supermarket). -//! - [`CredentialInjection`] is built by the RDP path from those groceries (chef). -//! - [`SyntheticKdcRegistry`] is the pass window: RDP publishes, `/jet/KdcProxy` looks up only. +//! - Provisioned material lives in [`crate::provisioning::ProvisioningStore`] until checkout. +//! - [`CredentialInjection::from_provisioned`] builds a session-scoped injection plan. +//! - Kerberos sessions publish a [`CredentialInjectionKdc`] into [`SyntheticKdcRegistry`]; +//! `/jet/KdcProxy` resolves only that registry (not the provisioning store). use std::collections::HashMap; use std::fmt; @@ -97,14 +98,14 @@ pub(crate) enum CredentialInjection { Ntlm(NtlmCredentialInjection), } -/// Kerberos dish: credentials + real KDC address + shared synthetic KDC. +/// Kerberos injection: credentials, target KDC URL, and the session synthetic KDC. pub(crate) struct KerberosCredentialInjection { credential_mapping: AppCredentialMapping, target_kdc: Url, synthetic: Arc, } -/// Chef output: protocol chosen; synthetic KDC built if needed, not yet published. +/// Protocol chosen; synthetic KDC built when Kerberos, not yet published to the registry. #[derive(Debug)] pub(crate) enum PreparedCredentialInjection { Kerberos(KerberosCredentialInjection), @@ -119,7 +120,7 @@ impl PreparedCredentialInjection { let registration = registry.register(Arc::clone(&injection.synthetic)); debug!( jti = %injection.synthetic.jti(), - "registered synthetic KDC for credential-injection session" + "Registered synthetic KDC for credential-injection session" ); CredentialInjection::Kerberos(injection, registration) } @@ -201,7 +202,10 @@ impl CredentialInjection { matches!(self, Self::Kerberos(_, _)) } - /// RDP chef: owned groceries → prepared dish. Does not touch the registry. + /// Build a session injection plan from a checked-out provisioning entry. + /// + /// Does not publish to [`SyntheticKdcRegistry`]; call + /// [`PreparedCredentialInjection::register_if_kerberos`] next. pub(crate) fn from_provisioned( jti: Uuid, credential_entry: ProvisioningEntry, @@ -214,7 +218,7 @@ impl CredentialInjection { } = credential_entry; let mapping = mapping.ok_or_else(|| { - warn!(%jti, "credential-injection state has no mapping"); + warn!(%jti, "Credential-injection state has no mapping"); CredentialInjectionKdcResolveError::NonInjectionCredential { jti } })?; @@ -222,30 +226,28 @@ impl CredentialInjection { warn!( %jti, error = format!("{source:#}"), - "invalid credential-injection association token" + "Invalid credential-injection association token" ); CredentialInjectionKdcResolveError::InvalidAssociationToken { jti, source } })?; - let target_username = match sspi::Username::parse(app_credential_username(&mapping.target)) { - Ok(u) => u, - Err(error) => { - warn!(%jti, error = format!("{error:#}"), "invalid target credential username"); - return Err(CredentialInjectionKdcResolveError::BuildKdcConfig { - jti, - source: anyhow::anyhow!("invalid target credential username: {error}"), - }); - } - }; - - let wants_kerberos = kerberos_enabled && target_username.domain_name().is_some(); - if !wants_kerberos { + let target_username = app_credential_username(&mapping.target); + if !select_kerberos_for_target(kerberos_enabled, target_username) { return Ok(PreparedCredentialInjection::Ntlm(NtlmCredentialInjection { jti, credential_mapping: mapping, })); } + // Kerberos path: username must parse (select_kerberos_for_target already required a domain). + if let Err(error) = sspi::Username::parse(target_username) { + warn!(%jti, error = format!("{error:#}"), "Invalid target credential username"); + return Err(CredentialInjectionKdcResolveError::BuildKdcConfig { + jti, + source: anyhow::anyhow!("invalid target credential username: {error}"), + }); + } + let target_kdc = connection_options .as_ref() .and_then(|o| o.krb_kdc()) @@ -267,6 +269,21 @@ impl CredentialInjection { } } +/// Unstable debug opt-in for Kerberos credential injection (both legs). +pub(crate) fn kerberos_injection_opt_in(conf: &crate::config::Conf) -> bool { + conf.debug.enable_unstable && conf.debug.kerberos_credential_injection +} + +/// Whether target username + opt-in select Kerberos injection (otherwise NTLM). +pub(crate) fn select_kerberos_for_target(kerberos_enabled: bool, target_username: &str) -> bool { + if !kerberos_enabled { + return false; + } + sspi::Username::parse(target_username) + .ok() + .is_some_and(|username| username.domain_name().is_some()) +} + pub(crate) struct CredentialInjectionKdcRequest { message: KdcProxyMessage, } @@ -329,6 +346,11 @@ impl CredentialInjectionKdc { self.jti } + /// Session destination host from association `dst_hst` (not Gateway `conf.hostname`). + pub(crate) fn target_hostname(&self) -> &str { + &self.target_hostname + } + pub(crate) fn server_kerberos_config(&self, client_addr: SocketAddr) -> anyhow::Result { let user = sspi::CredentialsBuffers::AuthIdentity(sspi::AuthIdentityBuffers::from_utf8( &self.acceptor_principal_name, @@ -338,9 +360,8 @@ impl CredentialInjectionKdc { let kdc_url = self.in_process_kdc_url()?; - // The SPN that the client puts on its AP-REQ ticket is the one for the target RDP - // server (`TERMSRV/`). Gateway-as-CredSSP-server is impersonating that target, - // so ServerProperties must claim the same SPN or sspi-rs rejects the ticket. + // Client AP-REQ SPN is TERMSRV/. Gateway-as-CredSSP-server impersonates that + // session destination, so ServerProperties must claim the same SPN. Ok(sspi::KerberosServerConfig { kerberos_config: sspi::KerberosConfig { kdc_url: Some(kdc_url), @@ -516,9 +537,9 @@ fn random_32_bytes() -> Vec { /// Live synthetic KDCs published by active RDP credential-injection sessions. /// -/// Pass window between handlers: -/// - RDP path publishes when it starts a Kerberos injection -/// - `/jet/KdcProxy` only looks up; it never builds a KDC from provisioned groceries +/// - The RDP path registers when a Kerberos injection session starts. +/// - `/jet/KdcProxy` only looks up published entries; it never builds a KDC from +/// [`crate::provisioning::ProvisioningStore`]. /// /// Entries are connection-scoped via [`SyntheticKdcRegistration`]. Reconnects `register` again /// (replace + bump generation); a late drop of an older registration is a no-op. @@ -674,6 +695,15 @@ mod tests { } } + #[test] + fn select_kerberos_for_target_matrix() { + assert!(!select_kerberos_for_target(false, "user@CORP.EXAMPLE")); + assert!(!select_kerberos_for_target(true, "Administrator")); + assert!(!select_kerberos_for_target(true, "")); + assert!(select_kerberos_for_target(true, "user@CORP.EXAMPLE")); + assert!(select_kerberos_for_target(true, r"CORP\user")); + } + #[test] fn proxy_user_at_realm_is_used_as_realm() { assert_eq!( @@ -725,7 +755,7 @@ mod tests { let store = stock_with_mapping(jti, "administrator@example.invalid"); store.insert_connection_options(jti, kdc_options(), time::Duration::minutes(5)); let entry = store.take(jti).expect("entry"); - assert!(store.take(jti).is_none(), "take consumes groceries"); + assert!(store.take(jti).is_none(), "take is one-shot"); let registry = SyntheticKdcRegistry::new(); let prepared = CredentialInjection::from_provisioned(jti, entry, true).expect("prepared"); assert!(registry.get(jti).is_none(), "not published until register_if_kerberos"); @@ -756,6 +786,38 @@ mod tests { ); } + #[test] + fn from_provisioned_uses_association_dst_hst_for_synthetic_kdc() { + // Destination is dynamic per token. conf.hostname is Gateway identity only and must not + // drive synthetic KDC SPN / service host (deliberate correction of #1856). + let jti = Uuid::new_v4(); + let store = ProvisioningStore::new(); + store + .insert_credentials( + unsigned_jws(serde_json::json!({ + "jti": jti, + "dst_hst": "it-help-dc.corp.example:3389" + })), + Some(cleartext_mapping_with_target_username("administrator@example.invalid")), + time::Duration::minutes(5), + ) + .expect("insert"); + store.insert_connection_options(jti, kdc_options(), time::Duration::minutes(5)); + let entry = store.take(jti).expect("entry"); + let injection = CredentialInjection::from_provisioned(jti, entry, true) + .expect("prepared") + .register_if_kerberos(&SyntheticKdcRegistry::new()); + + assert_eq!( + injection + .as_kerberos() + .expect("kerberos") + .synthetic_kdc() + .target_hostname(), + "it-help-dc.corp.example", + ); + } + #[test] fn registry_replace_and_guarded_drop_keeps_successor() { let registry = SyntheticKdcRegistry::new(); @@ -773,7 +835,7 @@ mod tests { } #[test] - fn kdc_proxy_cannot_invent_from_groceries() { + fn kdc_proxy_cannot_invent_from_provisioning_store() { let jti = Uuid::new_v4(); let _store = stock_with_mapping(jti, "administrator@example.invalid"); let registry = SyntheticKdcRegistry::new(); diff --git a/devolutions-gateway/src/generic_client.rs b/devolutions-gateway/src/generic_client.rs index d7287a21f..a824ceb17 100644 --- a/devolutions-gateway/src/generic_client.rs +++ b/devolutions-gateway/src/generic_client.rs @@ -146,20 +146,15 @@ where let disconnect_interest = DisconnectInterest::from_reconnection_policy(claims.jet_reuse); - // We support proxy-based credential injection for RDP. - // If a credential mapping has been pushed, we automatically switch to this mode. - // Otherwise, we continue the generic procedure. - // - // RdpProxy is generic over the server stream, so credential injection works - // regardless of whether the upstream is direct TCP or tunnelled via an agent. - // The credential store is keyed on the association token's JTI, so a direct - // lookup by `claims.jti` is the primary path. - if is_rdp - && let Some(entry) = provisioning.take(claims.jti) - && entry.mapping.is_some() - { + // RDP credential injection: peek for a mapping first so token-only provision rows are not + // consumed. take() is one-shot after the injection path is chosen. + if is_rdp && provisioning.has_mapping(claims.jti) { + let entry = provisioning + .take(claims.jti) + .context("provisioned credentials missing after has_mapping")?; + anyhow::ensure!(entry.mapping.is_some(), "provisioned entry has no credential mapping"); anyhow::ensure!(token == entry.token, "token mismatch"); - let kerberos_enabled = conf.debug.enable_unstable && conf.debug.kerberos_credential_injection; + let kerberos_enabled = crate::credential_injection::kerberos_injection_opt_in(&conf); let credential_injection = CredentialInjection::from_provisioned(claims.jti, entry, kerberos_enabled)? .register_if_kerberos(&synthetic_kdc_registry); diff --git a/devolutions-gateway/src/openapi.rs b/devolutions-gateway/src/openapi.rs index 73b22bf60..25590af34 100644 --- a/devolutions-gateway/src/openapi.rs +++ b/devolutions-gateway/src/openapi.rs @@ -393,10 +393,12 @@ struct PreflightOperation { /// /// Required for "resolve-host" kind. host_to_resolve: Option, - /// Minimum persistence duration in seconds for the data provisioned via this operation. + /// How long provisioned data may wait for first use, in seconds. /// - /// Optional parameter for "provision-token", "provision-credentials", and - /// "provision-connection-options" kinds. + /// Optional for "provision-token", "provision-credentials", and + /// "provision-connection-options". For credential-injection mappings this is the maximum + /// time until checkout: the injection path consumes the entry once (one-shot) when a + /// session starts, and does not put it back after a failed attempt. Re-provision to retry. time_to_live: Option, } diff --git a/devolutions-gateway/src/provisioning.rs b/devolutions-gateway/src/provisioning.rs index e7cb50223..48a3689e6 100644 --- a/devolutions-gateway/src/provisioning.rs +++ b/devolutions-gateway/src/provisioning.rs @@ -142,11 +142,16 @@ impl ProvisioningStore { } } - /// Take the provisioned view for a session (one-shot). + /// Take the provisioned view for a session (one-shot checkout). /// /// Removes the credentials half (required) and any live connection-options half for `jti`. - /// Returns `None` if credentials are missing or expired. A second `take` for the same JTI - /// fails until preflight inserts again. + /// Returns `None` if credentials are missing or expired. + /// + /// **Contract:** injection mappings are consumed when the injection path checks them out. + /// They are not restored after a failed TLS/CredSSP attempt. `time_to_live` is how long the + /// entry may wait for that first checkout, not a retry budget. Re-provision to try again. + /// Callers must [`Self::has_mapping`] (or equivalent) before taking so token-only rows are + /// not destroyed by unrelated RDP connections. pub(crate) fn take(&self, jti: Uuid) -> Option { let now = time::OffsetDateTime::now_utc(); diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index 86e8e8f2d..c5bb4314c 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -426,7 +426,7 @@ async fn connect_rdp_server( }) } -/// Handle RDP connection with credential injection via CredSSP MITM +/// Handle RDP connection with credential injection via CredSSP MITM. #[expect(clippy::too_many_arguments)] async fn handle_with_credential_injection( mut client_stream: impl AsyncRead + AsyncWrite + Unpin + Send, @@ -443,7 +443,6 @@ async fn handle_with_credential_injection( agent_tunnel_handle: Option>, ) -> anyhow::Result<()> { let tls_conf = conf.credssp_tls.get().context("CredSSP TLS configuration")?; - let gateway_hostname = conf.hostname.clone(); let x224_req = cleanpath_pdu @@ -453,7 +452,6 @@ async fn handle_with_credential_injection( let received_connection_request: ironrdp_pdu::x224::X224 = ironrdp_core::decode(x224_req.as_bytes()).context("decode X224 connection request PDU from client")?; - // Choose the security protocol to use with the client. let received_connection_request_protocol = received_connection_request.0.protocol; let client_security_protocol = if received_connection_request_protocol.contains(nego::SecurityProtocol::HYBRID_EX) { nego::SecurityProtocol::HYBRID_EX @@ -470,7 +468,6 @@ async fn handle_with_credential_injection( ) }; - // Authorize and connect to the RDP server. let CleanPathAuth { claims } = authorize_cleanpath( &cleanpath_pdu, client_addr, @@ -485,17 +482,10 @@ async fn handle_with_credential_injection( let token = cleanpath_pdu .proxy_auth - .as_deref() + .clone() .context("missing token in RDCleanPath PDU")?; - let entry = provisioning - .take(claims.jti) - .context("provisioned credentials missing after authorization")?; - anyhow::ensure!(entry.mapping.is_some(), "provisioned entry has no credential mapping"); - anyhow::ensure!(token == entry.token, "token mismatch"); - let kerberos_enabled = conf.debug.enable_unstable && conf.debug.kerberos_credential_injection; - let credential_injection = CredentialInjection::from_provisioned(claims.jti, entry, kerberos_enabled)? - .register_if_kerberos(synthetic_kdc_registry); + // Connect before checkout so a target connect failure does not consume one-shot credentials. let ConnectedRdpServer { tls_stream: server_stream, server_addr, @@ -506,13 +496,21 @@ async fn handle_with_credential_injection( .context("RDCleanPath connection failed")?; let x224_rsp = x224_rsp.context("RDCleanPath credential injection requires X.224")?; - // Retrieve the Gateway TLS public key that must be used for client-proxy CredSSP later on. + let entry = provisioning + .take(claims.jti) + .context("provisioned credentials missing after authorization")?; + anyhow::ensure!(entry.mapping.is_some(), "provisioned entry has no credential mapping"); + anyhow::ensure!(token == entry.token, "token mismatch"); + + let kerberos_enabled = crate::credential_injection::kerberos_injection_opt_in(&conf); + let credential_injection = CredentialInjection::from_provisioned(claims.jti, entry, kerberos_enabled)? + .register_if_kerberos(synthetic_kdc_registry); + let gateway_cert_chain_handle = tokio::spawn(crate::tls::get_cert_chain_for_acceptor_cached( - gateway_hostname.clone(), + gateway_hostname, tls_conf.acceptor.clone(), )); - // Extract server security protocol from X224 response (before x224_rsp is moved). let x224_confirm: ironrdp_pdu::x224::X224 = ironrdp_core::decode(&x224_rsp).context("decode X224 connection confirm")?; let server_security_protocol = match &x224_confirm.0 { @@ -536,8 +534,7 @@ async fn handle_with_credential_injection( let gateway_public_key = crate::tls::extract_public_key(gateway_cert_chain.first().context("no leaf")?) .context("extract Gateway public key")?; - // Send RDCleanPath response to client using Devolutions Gateway certification chain. - // (When performing credential injection, the client performs CredSSP against the Devolutions Gateway.) + // Client CredSSP runs against the Gateway certificate chain. trace!("Sending RDCleanPath response"); let rd_clean_path_rsp = RDCleanPathPdu::new_response( server_addr.to_string(), @@ -546,64 +543,8 @@ async fn handle_with_credential_injection( ) .context("couldn't build RDCleanPath response")?; send_clean_path_response(&mut client_stream, &rd_clean_path_rsp).await?; - debug!("RDCleanPath response sent, now performing CredSSP MITM"); + debug!("RDCleanPath response sent, starting CredSSP MITM"); - // -- Perform the CredSSP authentication with the client (acting as a server) and the server (acting as a client) -- // - - let mut client_framed = ironrdp_tokio::MovableTokioFramed::new(client_stream); - let mut server_framed = ironrdp_tokio::MovableTokioFramed::new(server_stream); - - 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, - gateway_public_key, - client_security_protocol, - &credential_injection, - &kdc_connector, - ); - - let server_credssp_fut = crate::rdp_proxy::perform_credssp_as_client( - &mut server_framed, - destination.host().to_owned(), - server_public_key, - server_security_protocol, - &credential_injection, - &gateway_hostname, - &kdc_connector, - ); - - let (client_credssp_res, server_credssp_res) = tokio::join!(client_credssp_fut, server_credssp_fut); - client_credssp_res.context("CredSSP with client")?; - server_credssp_res.context("CredSSP with server")?; - - debug!("CredSSP MITM completed successfully"); - - // -- Intercept the Connect Confirm PDU, to override the server_security_protocol field -- // - - crate::rdp_proxy::intercept_connect_confirm(&mut client_framed, &mut server_framed, server_security_protocol) - .await?; - - let (mut client_stream, client_leftover) = client_framed.into_inner(); - let (mut server_stream, server_leftover) = server_framed.into_inner(); - - // -- At this point, proceed to the usual two-way forwarding -- // - - info!("RDP-TLS forwarding (credential injection)"); - - client_stream - .write_all(&server_leftover) - .await - .context("write server leftover to client")?; - - server_stream - .write_all(&client_leftover) - .await - .context("write client leftover to server")?; - - // Build SessionInfo for forwarding let info = SessionInfo::builder() .id(claims.jet_aid) .application_protocol(claims.jet_ap) @@ -616,22 +557,32 @@ async fn handle_with_credential_injection( .build(); let disconnect_interest = DisconnectInterest::from_reconnection_policy(claims.jet_reuse); + let kdc_connector = + crate::kdc_connector::KdcConnector::new(claims.jet_aid, claims.jet_agent_id, agent_tunnel_handle.clone()); - // Plain forwarding for now - Proxy::builder() + let session = crate::rdp_proxy::CredsspSession::builder() .conf(conf) .session_info(info) - .address_a(client_addr) - .transport_a(client_stream) - .address_b(server_addr) - .transport_b(server_stream) + .client_addr(client_addr) + .server_addr(server_addr) + .credential_injection(credential_injection) .sessions(sessions) .subscriber_tx(subscriber_tx) + .server_dns_name(destination.host().to_owned()) .disconnect_interest(disconnect_interest) - .build() - .select_dissector_and_forward() - .await - .context("proxy failed") + .kdc_connector(kdc_connector) + .build(); + + let prepared = crate::rdp_proxy::PreparedCredssp::builder() + .client_stream(client_stream) + .server_stream(server_stream) + .gateway_public_key(gateway_public_key) + .server_public_key(server_public_key) + .client_security_protocol(client_security_protocol) + .server_security_protocol(server_security_protocol) + .build(); + + session.run(prepared).await } #[expect(clippy::too_many_arguments)] @@ -663,10 +614,9 @@ pub async fn handle( .as_deref() .context("missing token in RDCleanPath PDU")?; - // If a credential mapping has been pushed, we automatically switch to - // proxy-based credential injection mode. Otherwise, we continue the usual - // clean path procedure. Peek only here — take after authorize_cleanpath so an - // unverified token cannot burn a victim JTI's one-shot groceries. + // If a credential mapping has been pushed, switch to proxy-based credential injection. + // Peek only here — take after authorize + server connect so an unverified token cannot + // burn a victim JTI, and a failed target connect does not consume the one-shot entry. if let Some(jti) = crate::token::extract_jti(token).ok() && provisioning.has_mapping(jti) { diff --git a/devolutions-gateway/src/rdp_proxy/credssp.rs b/devolutions-gateway/src/rdp_proxy/credssp.rs index 5295b6e75..a49a0b155 100644 --- a/devolutions-gateway/src/rdp_proxy/credssp.rs +++ b/devolutions-gateway/src/rdp_proxy/credssp.rs @@ -110,7 +110,6 @@ impl CredsspSession { server_public_key, server_security_protocol, &credential_injection, - &conf.hostname, &kdc_connector, ); @@ -154,7 +153,7 @@ impl CredsspSession { } } -pub(crate) async fn intercept_connect_confirm( +async fn intercept_connect_confirm( client_framed: &mut ironrdp_tokio::MovableTokioFramed, server_framed: &mut ironrdp_tokio::MovableTokioFramed, server_security_protocol: nego::SecurityProtocol, @@ -201,26 +200,26 @@ fn server_kerberos_setup( } fn client_kerberos_config( - gateway_hostname: &str, injection: &CredentialInjection, ) -> anyhow::Result> { let Some(kerberos) = injection.as_kerberos() else { return Ok(None); }; + // Target-leg Kerberos uses the same session destination as the synthetic KDC (association + // `dst_hst`). conf.hostname is Gateway identity only and is not the RDP destination. Ok(Some(ironrdp_connector::credssp::KerberosConfig { kdc_proxy_url: Some(kerberos.target_kdc().clone()), - hostname: gateway_hostname.to_owned(), + hostname: kerberos.synthetic_kdc().target_hostname().to_owned(), })) } #[instrument(name = "server_credssp", level = "debug", ret, skip_all)] -pub(crate) async fn perform_credssp_as_client( +async fn perform_credssp_as_client( framed: &mut ironrdp_tokio::Framed, server_name: String, server_public_key: Vec, security_protocol: nego::SecurityProtocol, injection: &CredentialInjection, - gateway_hostname: &str, kdc_connector: &KdcConnector, ) -> anyhow::Result<()> where @@ -229,7 +228,7 @@ where use ironrdp_tokio::FramedWrite as _; let credentials = injection.target_credential(); - let kerberos_config = client_kerberos_config(gateway_hostname, injection)?; + let kerberos_config = client_kerberos_config(injection)?; // Decrypt password into short-lived buffer. let (username, decrypted_password) = credentials @@ -350,7 +349,7 @@ async fn resolve_client_generator( } #[instrument(name = "client_credssp", level = "debug", ret, skip_all)] -pub(crate) async fn perform_credssp_as_server( +async fn perform_credssp_as_server( framed: &mut ironrdp_tokio::Framed, client_addr: SocketAddr, gateway_public_key: Vec, @@ -529,9 +528,9 @@ mod tests { } #[test] - fn client_kerberos_config_uses_provisioned_target_kdc_url() { + fn client_kerberos_config_uses_provisioned_target_kdc_url_and_dst_hst() { let injection = kerberos_injection(); - let config = client_kerberos_config("dgateway.localhost.com", &injection) + let config = client_kerberos_config(&injection) .expect("config builds") .expect("kerberos client leg"); @@ -540,7 +539,10 @@ mod tests { Some("tcp://dc.example.com:88"), "CredSSP kdc_proxy_url must be the provisioned krb_kdc", ); - assert_eq!(config.hostname, "dgateway.localhost.com"); + assert_eq!( + config.hostname, "target.example", + "target-leg Kerberos hostname is association dst_hst, not conf.hostname", + ); } #[test] @@ -568,7 +570,7 @@ mod tests { .expect("ntlm prepared") .register_if_kerberos(&SyntheticKdcRegistry::new()); - let config = client_kerberos_config("dgateway.localhost.com", &injection).expect("ntlm ok"); + let config = client_kerberos_config(&injection).expect("ntlm ok"); assert!(config.is_none()); } } diff --git a/devolutions-gateway/src/rdp_proxy/mod.rs b/devolutions-gateway/src/rdp_proxy/mod.rs index 85ee1af48..3eae2e84e 100644 --- a/devolutions-gateway/src/rdp_proxy/mod.rs +++ b/devolutions-gateway/src/rdp_proxy/mod.rs @@ -8,10 +8,7 @@ use typed_builder::TypedBuilder; mod credssp; -use credssp::CredsspSession; -pub(crate) use credssp::{ - PreparedCredssp, intercept_connect_confirm, perform_credssp_as_client, perform_credssp_as_server, -}; +pub(crate) use credssp::{CredsspSession, PreparedCredssp}; use crate::config::Conf; use crate::credential::AppCredential; @@ -270,9 +267,3 @@ where framed.write_all(&payload).await.context("failed to write PDU")?; Ok(()) } - -#[cfg(test)] -mod tests { - // Protocol selection is owned by CredentialInjection (from_provisioned + register_if_kerberos). - // See credential_injection tests for Kerberos-vs-NTLM decision coverage. -} From 312004f55579fa75d2dfe3074af67796401cf962 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Mon, 17 Aug 2026 11:21:32 -0400 Subject: [PATCH 05/12] ci(package): retry Windows tool installs Keep the preinstalled WiX toolset until Chocolatey successfully installs the pinned version. Retry transient feed failures, validate candle.exe, and expose WIXSHARP_WIXDIR so installer builds cannot continue with an empty WiX path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 36 ++++++++++++++++++++----------- .github/workflows/package.yml | 10 ++++++--- ci/install-chocolatey-package.ps1 | 34 +++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 16 deletions(-) create mode 100644 ci/install-chocolatey-package.ps1 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 113ebd31d..946746f73 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -646,15 +646,20 @@ jobs: - name: Configure Windows runner if: ${{ matrix.os == 'windows' }} run: | - # https://github.com/actions/runner-images/issues/9667 - choco uninstall wixtoolset - choco install wixtoolset --version 3.14.0 --allow-downgrade --no-progress --force - - # WiX is installed on Windows runners but not in the PATH - Write-Output "C:\Program Files (x86)\WiX Toolset v3.14\bin" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append + ./ci/install-chocolatey-package.ps1 ` + -Package wixtoolset ` + -Version 3.14.0 ` + -AdditionalArguments @('--allow-downgrade', '--force') + + $WixBinPath = "C:\Program Files (x86)\WiX Toolset v3.14\bin" + if (-not (Test-Path (Join-Path $WixBinPath 'candle.exe'))) { + throw "WiX installation is missing candle.exe at $WixBinPath" + } + Write-Output $WixBinPath | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append + Write-Output "WIXSHARP_WIXDIR=$WixBinPath" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append # NASM is required by aws-lc-rs (used as rustls crypto backend) - choco install nasm + ./ci/install-chocolatey-package.ps1 -Package nasm # Install Visual Studio Developer PowerShell Module for cmdlets such as Enter-VsDevShell Install-Module VsDevShell -Force @@ -909,18 +914,23 @@ jobs: - name: Configure Windows runner if: ${{ matrix.os == 'windows' }} run: | - # https://github.com/actions/runner-images/issues/9667 - choco uninstall wixtoolset - choco install wixtoolset --version 3.14.0 --allow-downgrade --force + ./ci/install-chocolatey-package.ps1 ` + -Package wixtoolset ` + -Version 3.14.0 ` + -AdditionalArguments @('--allow-downgrade', '--force') # Devolutions PEDM needs MakeAppx.exe Write-Output "C:\Program Files (x86)\Windows Kits\10\bin\10.0.17763.0\x64" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append - # WiX is installed on Windows runners but not in the PATH - Write-Output "C:\Program Files (x86)\WiX Toolset v3.14\bin" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append + $WixBinPath = "C:\Program Files (x86)\WiX Toolset v3.14\bin" + if (-not (Test-Path (Join-Path $WixBinPath 'candle.exe'))) { + throw "WiX installation is missing candle.exe at $WixBinPath" + } + Write-Output $WixBinPath | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append + Write-Output "WIXSHARP_WIXDIR=$WixBinPath" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append # NASM is required by aws-lc-rs (used as rustls crypto backend) - choco install nasm + ./ci/install-chocolatey-package.ps1 -Package nasm # We need to add the NASM binary folder to the PATH manually. Write-Output "$Env:ProgramFiles\NASM" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append diff --git a/.github/workflows/package.yml b/.github/workflows/package.yml index 27ee014f9..4d4fe71da 100644 --- a/.github/workflows/package.yml +++ b/.github/workflows/package.yml @@ -249,10 +249,14 @@ jobs: run: | echo "C:\Program Files (x86)\Windows Kits\10\bin\10.0.17763.0\x64" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append - # https://github.com/actions/runner-images/issues/9667 - choco uninstall wixtoolset - choco install wixtoolset --version 3.14.0 --allow-downgrade --no-progress --force + ./ci/install-chocolatey-package.ps1 ` + -Package wixtoolset ` + -Version 3.14.0 ` + -AdditionalArguments @('--allow-downgrade', '--force') $WixBinPath = "C:\Program Files (x86)\WiX Toolset v3.14\bin" + if (-not (Test-Path (Join-Path $WixBinPath 'candle.exe'))) { + throw "WiX installation is missing candle.exe at $WixBinPath" + } echo $WixBinPath | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append # WixSharp reads WIXSHARP_WIXDIR as a fallback when WixSharpBinPath MSBuild property # is empty (happens when /t:restore,build runs on a fresh runner without cached NuGet diff --git a/ci/install-chocolatey-package.ps1 b/ci/install-chocolatey-package.ps1 new file mode 100644 index 000000000..16a67f7aa --- /dev/null +++ b/ci/install-chocolatey-package.ps1 @@ -0,0 +1,34 @@ +param( + [Parameter(Mandatory = $true)] + [string]$Package, + + [string]$Version, + + [string[]]$AdditionalArguments = @(), + + [ValidateRange(1, 10)] + [int]$Attempts = 3 +) + +$ErrorActionPreference = 'Stop' + +$arguments = @('install', $Package, '--yes', '--no-progress') +if ($Version) { + $arguments += @('--version', $Version) +} +$arguments += $AdditionalArguments + +for ($attempt = 1; $attempt -le $Attempts; $attempt++) { + & choco @arguments + if ($LASTEXITCODE -eq 0) { + exit 0 + } + + if ($attempt -eq $Attempts) { + throw "Chocolatey failed to install $Package after $Attempts attempts" + } + + $delaySeconds = 10 * $attempt + Write-Warning "Chocolatey failed to install $Package (attempt $attempt/$Attempts); retrying in $delaySeconds seconds" + Start-Sleep -Seconds $delaySeconds +} From 2c9ec6fffb1c0a294d433697a7b23a94727bd585 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Mon, 17 Aug 2026 15:17:13 -0400 Subject: [PATCH 06/12] chore: remove unrelated package CI changes Keep PR #1900 scoped to the Gateway provisioning and CredSSP refactor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 36 +++++++++++-------------------- .github/workflows/package.yml | 10 +++------ ci/install-chocolatey-package.ps1 | 34 ----------------------------- 3 files changed, 16 insertions(+), 64 deletions(-) delete mode 100644 ci/install-chocolatey-package.ps1 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 946746f73..113ebd31d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -646,20 +646,15 @@ jobs: - name: Configure Windows runner if: ${{ matrix.os == 'windows' }} run: | - ./ci/install-chocolatey-package.ps1 ` - -Package wixtoolset ` - -Version 3.14.0 ` - -AdditionalArguments @('--allow-downgrade', '--force') - - $WixBinPath = "C:\Program Files (x86)\WiX Toolset v3.14\bin" - if (-not (Test-Path (Join-Path $WixBinPath 'candle.exe'))) { - throw "WiX installation is missing candle.exe at $WixBinPath" - } - Write-Output $WixBinPath | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append - Write-Output "WIXSHARP_WIXDIR=$WixBinPath" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append + # https://github.com/actions/runner-images/issues/9667 + choco uninstall wixtoolset + choco install wixtoolset --version 3.14.0 --allow-downgrade --no-progress --force + + # WiX is installed on Windows runners but not in the PATH + Write-Output "C:\Program Files (x86)\WiX Toolset v3.14\bin" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append # NASM is required by aws-lc-rs (used as rustls crypto backend) - ./ci/install-chocolatey-package.ps1 -Package nasm + choco install nasm # Install Visual Studio Developer PowerShell Module for cmdlets such as Enter-VsDevShell Install-Module VsDevShell -Force @@ -914,23 +909,18 @@ jobs: - name: Configure Windows runner if: ${{ matrix.os == 'windows' }} run: | - ./ci/install-chocolatey-package.ps1 ` - -Package wixtoolset ` - -Version 3.14.0 ` - -AdditionalArguments @('--allow-downgrade', '--force') + # https://github.com/actions/runner-images/issues/9667 + choco uninstall wixtoolset + choco install wixtoolset --version 3.14.0 --allow-downgrade --force # Devolutions PEDM needs MakeAppx.exe Write-Output "C:\Program Files (x86)\Windows Kits\10\bin\10.0.17763.0\x64" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append - $WixBinPath = "C:\Program Files (x86)\WiX Toolset v3.14\bin" - if (-not (Test-Path (Join-Path $WixBinPath 'candle.exe'))) { - throw "WiX installation is missing candle.exe at $WixBinPath" - } - Write-Output $WixBinPath | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append - Write-Output "WIXSHARP_WIXDIR=$WixBinPath" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append + # WiX is installed on Windows runners but not in the PATH + Write-Output "C:\Program Files (x86)\WiX Toolset v3.14\bin" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append # NASM is required by aws-lc-rs (used as rustls crypto backend) - ./ci/install-chocolatey-package.ps1 -Package nasm + choco install nasm # We need to add the NASM binary folder to the PATH manually. Write-Output "$Env:ProgramFiles\NASM" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append diff --git a/.github/workflows/package.yml b/.github/workflows/package.yml index 4d4fe71da..27ee014f9 100644 --- a/.github/workflows/package.yml +++ b/.github/workflows/package.yml @@ -249,14 +249,10 @@ jobs: run: | echo "C:\Program Files (x86)\Windows Kits\10\bin\10.0.17763.0\x64" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append - ./ci/install-chocolatey-package.ps1 ` - -Package wixtoolset ` - -Version 3.14.0 ` - -AdditionalArguments @('--allow-downgrade', '--force') + # https://github.com/actions/runner-images/issues/9667 + choco uninstall wixtoolset + choco install wixtoolset --version 3.14.0 --allow-downgrade --no-progress --force $WixBinPath = "C:\Program Files (x86)\WiX Toolset v3.14\bin" - if (-not (Test-Path (Join-Path $WixBinPath 'candle.exe'))) { - throw "WiX installation is missing candle.exe at $WixBinPath" - } echo $WixBinPath | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append # WixSharp reads WIXSHARP_WIXDIR as a fallback when WixSharpBinPath MSBuild property # is empty (happens when /t:restore,build runs on a fresh runner without cached NuGet diff --git a/ci/install-chocolatey-package.ps1 b/ci/install-chocolatey-package.ps1 deleted file mode 100644 index 16a67f7aa..000000000 --- a/ci/install-chocolatey-package.ps1 +++ /dev/null @@ -1,34 +0,0 @@ -param( - [Parameter(Mandatory = $true)] - [string]$Package, - - [string]$Version, - - [string[]]$AdditionalArguments = @(), - - [ValidateRange(1, 10)] - [int]$Attempts = 3 -) - -$ErrorActionPreference = 'Stop' - -$arguments = @('install', $Package, '--yes', '--no-progress') -if ($Version) { - $arguments += @('--version', $Version) -} -$arguments += $AdditionalArguments - -for ($attempt = 1; $attempt -le $Attempts; $attempt++) { - & choco @arguments - if ($LASTEXITCODE -eq 0) { - exit 0 - } - - if ($attempt -eq $Attempts) { - throw "Chocolatey failed to install $Package after $Attempts attempts" - } - - $delaySeconds = 10 * $attempt - Write-Warning "Chocolatey failed to install $Package (attempt $attempt/$Attempts); retrying in $delaySeconds seconds" - Start-Sleep -Seconds $delaySeconds -} From ccb3963f87d5fd6905eb380fa9b003a2b75ced83 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Mon, 17 Aug 2026 15:17:27 -0400 Subject: [PATCH 07/12] fix(dgw): make injection checkout fail closed Keep consumed credential mappings visible until their original expiry so a reused JTI fails explicitly instead of silently falling back to ordinary forwarding. Centralize atomic checkout and CredSSP orchestration, preserve token-only provisioning, and simplify KDC error handling. Record the one-shot contract in PR history without regenerating unchanged OpenAPI artifacts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/credential_injection.rs | 121 +++++------ devolutions-gateway/src/generic_client.rs | 106 ++++++---- devolutions-gateway/src/openapi.rs | 8 +- devolutions-gateway/src/provisioning.rs | 198 +++++++++++++++--- devolutions-gateway/src/rd_clean_path.rs | 36 ++-- 5 files changed, 312 insertions(+), 157 deletions(-) diff --git a/devolutions-gateway/src/credential_injection.rs b/devolutions-gateway/src/credential_injection.rs index 770d88886..d21ae0a06 100644 --- a/devolutions-gateway/src/credential_injection.rs +++ b/devolutions-gateway/src/credential_injection.rs @@ -24,9 +24,7 @@ use url::Url; use uuid::Uuid; use crate::credential::{AppCredential, AppCredentialMapping}; -use crate::provisioning::ProvisioningEntry; -#[cfg(test)] -use crate::provisioning::ProvisioningStore; +use crate::provisioning::{ProvisioningEntry, ProvisioningStore}; // The reserved `.invalid` TLD (RFC 6761) lets sspi-rs CredSSP server emit "KDC requests" that // never leave the process: `intercept_network_request` recognises this hostname and dispatches @@ -51,26 +49,6 @@ pub(crate) struct CredentialInjectionKdc { kdc_config: kdc::config::KerberosServer, } -#[derive(Debug, Error)] -pub(crate) enum CredentialInjectionKdcResolveError { - #[error("credential-injection state is not available for {jti}")] - NonInjectionCredential { jti: Uuid }, - #[error("association token for {jti} is not valid for credential injection")] - InvalidAssociationToken { - jti: Uuid, - #[source] - source: anyhow::Error, - }, - #[error("credential-injection KDC config could not be initialized for {jti}")] - BuildKdcConfig { - jti: Uuid, - #[source] - source: anyhow::Error, - }, - #[error("Kerberos credential injection requires target connection option krb_kdc for {jti}")] - MissingKrbKdc { jti: Uuid }, -} - #[derive(Debug, Clone, PartialEq, Eq, Error)] #[error("expected: {expected}, got: {actual}")] pub(crate) struct RealmMismatch { @@ -170,6 +148,19 @@ impl NtlmCredentialInjection { } impl CredentialInjection { + pub(crate) fn checkout( + provisioning: &ProvisioningStore, + registry: &SyntheticKdcRegistry, + jti: Uuid, + token: &str, + kerberos_enabled: bool, + ) -> anyhow::Result { + let entry = provisioning + .take_mapping(jti, token) + .with_context(|| format!("checkout credential-injection material for {jti}"))?; + Ok(Self::from_provisioned(jti, entry, kerberos_enabled)?.register_if_kerberos(registry)) + } + pub(crate) fn jti(&self) -> Uuid { match self { Self::Kerberos(k, _) => k.synthetic.jti(), @@ -210,26 +201,17 @@ impl CredentialInjection { jti: Uuid, credential_entry: ProvisioningEntry, kerberos_enabled: bool, - ) -> Result { + ) -> anyhow::Result { let ProvisioningEntry { token, mapping, connection_options, } = credential_entry; - let mapping = mapping.ok_or_else(|| { - warn!(%jti, "Credential-injection state has no mapping"); - CredentialInjectionKdcResolveError::NonInjectionCredential { jti } - })?; + let mapping = mapping.context("credential-injection state has no mapping")?; - let target_hostname = crate::token::extract_credential_injection_target_hostname(&token).map_err(|source| { - warn!( - %jti, - error = format!("{source:#}"), - "Invalid credential-injection association token" - ); - CredentialInjectionKdcResolveError::InvalidAssociationToken { jti, source } - })?; + let target_hostname = crate::token::extract_credential_injection_target_hostname(&token) + .with_context(|| format!("association token for {jti} is not valid for credential injection"))?; let target_username = app_credential_username(&mapping.target); if !select_kerberos_for_target(kerberos_enabled, target_username) { @@ -240,26 +222,20 @@ impl CredentialInjection { } // Kerberos path: username must parse (select_kerberos_for_target already required a domain). - if let Err(error) = sspi::Username::parse(target_username) { - warn!(%jti, error = format!("{error:#}"), "Invalid target credential username"); - return Err(CredentialInjectionKdcResolveError::BuildKdcConfig { - jti, - source: anyhow::anyhow!("invalid target credential username: {error}"), - }); - } + sspi::Username::parse(target_username) + .with_context(|| format!("invalid target credential username for credential-injection session {jti}"))?; let target_kdc = connection_options .as_ref() .and_then(|o| o.krb_kdc()) .cloned() - .ok_or_else(|| { - warn!(%jti, "Kerberos credential injection requires krb_kdc"); - CredentialInjectionKdcResolveError::MissingKrbKdc { jti } + .with_context(|| { + format!("Kerberos credential injection requires target connection option krb_kdc for {jti}") })?; let proxy_username = app_credential_username(&mapping.proxy).to_owned(); let synthetic = CredentialInjectionKdc::new(jti, target_hostname, &proxy_username, &mapping.proxy) - .map_err(|source| CredentialInjectionKdcResolveError::BuildKdcConfig { jti, source })?; + .with_context(|| format!("credential-injection KDC config could not be initialized for {jti}"))?; Ok(PreparedCredentialInjection::Kerberos(KerberosCredentialInjection { credential_mapping: mapping, @@ -270,8 +246,8 @@ impl CredentialInjection { } /// Unstable debug opt-in for Kerberos credential injection (both legs). -pub(crate) fn kerberos_injection_opt_in(conf: &crate::config::Conf) -> bool { - conf.debug.enable_unstable && conf.debug.kerberos_credential_injection +pub(crate) fn kerberos_injection_opt_in(enable_unstable: bool, kerberos_credential_injection: bool) -> bool { + enable_unstable && kerberos_credential_injection } /// Whether target username + opt-in select Kerberos injection (otherwise NTLM). @@ -541,8 +517,9 @@ fn random_32_bytes() -> Vec { /// - `/jet/KdcProxy` only looks up published entries; it never builds a KDC from /// [`crate::provisioning::ProvisioningStore`]. /// -/// Entries are connection-scoped via [`SyntheticKdcRegistration`]. Reconnects `register` again -/// (replace + bump generation); a late drop of an older registration is a no-op. +/// Entries are connection-scoped via [`SyntheticKdcRegistration`] and removed when the owning session ends. +/// Generations prevent an older session from unpublishing a replacement. +/// Re-provisioning the same JTI can register that replacement before the older session ends. #[derive(Debug, Clone)] pub struct SyntheticKdcRegistry { inner: Arc>, @@ -551,7 +528,6 @@ pub struct SyntheticKdcRegistry { #[derive(Debug, Default)] struct RegistryInner { live: HashMap, - /// Registry-wide monotonic counter (not per-JTI) so generations stay unique without leaking map entries. next_generation: u64, } @@ -561,7 +537,7 @@ struct PublishedSyntheticKdc { kdc: Arc, } -/// RAII lease for a published synthetic KDC. Dropping it unpublishes only this generation. +/// RAII lease for a published synthetic KDC. pub(crate) struct SyntheticKdcRegistration { registry: SyntheticKdcRegistry, jti: Uuid, @@ -576,7 +552,7 @@ impl Drop for SyntheticKdcRegistration { }; if current.generation == self.generation { inner.live.remove(&self.jti); - debug!(jti = %self.jti, generation = self.generation, "unpublished synthetic KDC"); + debug!(jti = %self.jti, generation = self.generation, "Unpublished synthetic KDC"); } } } @@ -594,17 +570,13 @@ impl SyntheticKdcRegistry { } } - fn allocate_generation(inner: &mut RegistryInner) -> u64 { - inner.next_generation = inner.next_generation.wrapping_add(1); - inner.next_generation - } - pub(crate) fn register(&self, kdc: Arc) -> SyntheticKdcRegistration { let jti = kdc.jti(); let mut inner = self.inner.lock(); - let generation = Self::allocate_generation(&mut inner); + inner.next_generation = inner.next_generation.wrapping_add(1); + let generation = inner.next_generation; inner.live.insert(jti, PublishedSyntheticKdc { generation, kdc }); - debug!(%jti, generation, "published synthetic KDC"); + debug!(%jti, generation, "Published synthetic KDC"); SyntheticKdcRegistration { registry: self.clone(), jti, @@ -613,7 +585,7 @@ impl SyntheticKdcRegistry { } pub(crate) fn get(&self, jti: Uuid) -> Option> { - self.inner.lock().live.get(&jti).map(|e| Arc::clone(&e.kdc)) + self.inner.lock().live.get(&jti).map(|entry| Arc::clone(&entry.kdc)) } } @@ -695,6 +667,14 @@ mod tests { } } + #[test] + fn kerberos_injection_opt_in_requires_both_flags() { + assert!(!kerberos_injection_opt_in(false, false)); + assert!(!kerberos_injection_opt_in(false, true)); + assert!(!kerberos_injection_opt_in(true, false)); + assert!(kerberos_injection_opt_in(true, true)); + } + #[test] fn select_kerberos_for_target_matrix() { assert!(!select_kerberos_for_target(false, "user@CORP.EXAMPLE")); @@ -746,7 +726,7 @@ mod tests { let jti = Uuid::new_v4(); let entry = dummy_entry(jti, "administrator@example.invalid"); let err = CredentialInjection::from_provisioned(jti, entry, true).expect_err("kdc"); - assert!(matches!(err, CredentialInjectionKdcResolveError::MissingKrbKdc { .. })); + assert!(format!("{err:#}").contains("requires target connection option krb_kdc")); } #[test] @@ -819,18 +799,19 @@ mod tests { } #[test] - fn registry_replace_and_guarded_drop_keeps_successor() { + fn older_registration_drop_keeps_reprovisioned_successor() { let registry = SyntheticKdcRegistry::new(); let jti = Uuid::new_v4(); let first = Arc::new(dummy_kdc(jti)); - let first_reg = registry.register(Arc::clone(&first)); + let first_registration = registry.register(Arc::clone(&first)); assert!(Arc::ptr_eq(®istry.get(jti).expect("first"), &first)); + let second = Arc::new(dummy_kdc(jti)); - let second_reg = registry.register(Arc::clone(&second)); - assert!(Arc::ptr_eq(®istry.get(jti).expect("second"), &second)); - drop(first_reg); - assert!(Arc::ptr_eq(®istry.get(jti).expect("still second"), &second)); - drop(second_reg); + let second_registration = registry.register(Arc::clone(&second)); + drop(first_registration); + assert!(Arc::ptr_eq(®istry.get(jti).expect("successor"), &second)); + + drop(second_registration); assert!(registry.get(jti).is_none()); } diff --git a/devolutions-gateway/src/generic_client.rs b/devolutions-gateway/src/generic_client.rs index a824ceb17..ef9121ec4 100644 --- a/devolutions-gateway/src/generic_client.rs +++ b/devolutions-gateway/src/generic_client.rs @@ -9,7 +9,7 @@ use typed_builder::TypedBuilder; use crate::config::Conf; use crate::credential_injection::{CredentialInjection, SyntheticKdcRegistry}; -use crate::provisioning::ProvisioningStore; +use crate::provisioning::{MappingStatus, ProvisioningStore}; use crate::proxy::Proxy; use crate::rdp_pcb::{extract_association_claims, read_pcb}; use crate::recording::ActiveRecordings; @@ -146,50 +146,66 @@ where let disconnect_interest = DisconnectInterest::from_reconnection_policy(claims.jet_reuse); - // RDP credential injection: peek for a mapping first so token-only provision rows are not - // consumed. take() is one-shot after the injection path is chosen. - if is_rdp && provisioning.has_mapping(claims.jti) { - let entry = provisioning - .take(claims.jti) - .context("provisioned credentials missing after has_mapping")?; - anyhow::ensure!(entry.mapping.is_some(), "provisioned entry has no credential mapping"); - anyhow::ensure!(token == entry.token, "token mismatch"); - let kerberos_enabled = crate::credential_injection::kerberos_injection_opt_in(&conf); - let credential_injection = - CredentialInjection::from_provisioned(claims.jti, entry, kerberos_enabled)? - .register_if_kerberos(&synthetic_kdc_registry); - - info!( - jti = %credential_injection.jti(), - kerberos = credential_injection.uses_kerberos(), - "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) - .session_info(info) - .client_addr(client_addr) - .client_stream(client_stream) - .server_addr(server_addr) - .server_stream(server_stream) - .sessions(sessions) - .subscriber_tx(subscriber_tx) - .credential_injection(credential_injection) - .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 - .context("encountered a failure during RDP proxying (credential injection)"); + // Peek first so token-only provision rows are not consumed. + // Fail explicitly for consumed mappings instead of silently downgrading. + let mapping_status = if is_rdp { + provisioning.mapping_status(claims.jti) + } else { + MappingStatus::Absent + }; + match mapping_status { + MappingStatus::Consumed => { + anyhow::bail!( + "credential-injection material for {} was already consumed; re-provision to retry", + claims.jti + ); + } + MappingStatus::Absent => {} + MappingStatus::Available => { + let kerberos_enabled = crate::credential_injection::kerberos_injection_opt_in( + conf.debug.enable_unstable, + conf.debug.kerberos_credential_injection, + ); + let credential_injection = CredentialInjection::checkout( + &provisioning, + &synthetic_kdc_registry, + claims.jti, + token, + kerberos_enabled, + )?; + + info!( + jti = %credential_injection.jti(), + kerberos = credential_injection.uses_kerberos(), + "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) + .session_info(info) + .client_addr(client_addr) + .client_stream(client_stream) + .server_addr(server_addr) + .server_stream(server_stream) + .sessions(sessions) + .subscriber_tx(subscriber_tx) + .credential_injection(credential_injection) + .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 + .context("encountered a failure during RDP proxying (credential injection)"); + } } info!("Upstream forwarding"); diff --git a/devolutions-gateway/src/openapi.rs b/devolutions-gateway/src/openapi.rs index 25590af34..73b22bf60 100644 --- a/devolutions-gateway/src/openapi.rs +++ b/devolutions-gateway/src/openapi.rs @@ -393,12 +393,10 @@ struct PreflightOperation { /// /// Required for "resolve-host" kind. host_to_resolve: Option, - /// How long provisioned data may wait for first use, in seconds. + /// Minimum persistence duration in seconds for the data provisioned via this operation. /// - /// Optional for "provision-token", "provision-credentials", and - /// "provision-connection-options". For credential-injection mappings this is the maximum - /// time until checkout: the injection path consumes the entry once (one-shot) when a - /// session starts, and does not put it back after a failed attempt. Re-provision to retry. + /// Optional parameter for "provision-token", "provision-credentials", and + /// "provision-connection-options" kinds. time_to_live: Option, } diff --git a/devolutions-gateway/src/provisioning.rs b/devolutions-gateway/src/provisioning.rs index 48a3689e6..bc72b877b 100644 --- a/devolutions-gateway/src/provisioning.rs +++ b/devolutions-gateway/src/provisioning.rs @@ -51,6 +51,19 @@ struct CredentialsEntry { expires_at: time::OffsetDateTime, } +#[derive(Debug, Default)] +struct CredentialsState { + entries: HashMap, + consumed: HashMap, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum MappingStatus { + Available, + Consumed, + Absent, +} + #[derive(Debug, Clone)] struct ConnectionOptionsEntry { connection_options: TargetConnectionOptions, @@ -68,7 +81,7 @@ struct ConnectionOptionsEntry { /// operations and may arrive, expire, or be replaced independently. #[derive(Debug, Clone)] pub struct ProvisioningStore { - credentials: Arc>>, + credentials: Arc>, connection_options: Arc>>, } @@ -81,7 +94,7 @@ impl Default for ProvisioningStore { impl ProvisioningStore { pub fn new() -> Self { Self { - credentials: Arc::new(Mutex::new(HashMap::new())), + credentials: Arc::new(Mutex::new(CredentialsState::default())), connection_options: Arc::new(Mutex::new(HashMap::new())), } } @@ -111,7 +124,9 @@ impl ProvisioningStore { expires_at: time::OffsetDateTime::now_utc() + time_to_live, }; - Ok(self.credentials.lock().insert(jti, entry).is_some()) + let mut credentials = self.credentials.lock(); + credentials.consumed.remove(&jti); + Ok(credentials.entries.insert(jti, entry).is_some()) } /// Insert or replace the connection-options half. Returns whether a prior entry was replaced. @@ -129,39 +144,45 @@ impl ProvisioningStore { self.connection_options.lock().insert(jti, entry).is_some() } - /// True when the credentials half is live and carries an injection mapping. + /// State of the credential-injection mapping for `jti`. /// - /// Does not consume the entry — use before auth when deciding whether to take the - /// credential-injection path. - pub(crate) fn has_mapping(&self, jti: Uuid) -> bool { + /// Does not consume the entry. + /// A consumed tombstone remains until the original provisioning expiry. + /// This makes reconnects fail explicitly instead of silently falling back to non-injected forwarding. + pub(crate) fn mapping_status(&self, jti: Uuid) -> MappingStatus { let now = time::OffsetDateTime::now_utc(); - let entries = self.credentials.lock(); - match entries.get(&jti) { - Some(entry) if now < entry.expires_at => entry.mapping.is_some(), - _ => false, + let mut credentials = self.credentials.lock(); + + if credentials + .consumed + .get(&jti) + .is_some_and(|expires_at| now < *expires_at) + { + return MappingStatus::Consumed; + } + credentials.consumed.remove(&jti); + + match credentials.entries.get(&jti) { + Some(entry) if now < entry.expires_at && entry.mapping.is_some() => MappingStatus::Available, + _ => MappingStatus::Absent, } } - /// Take the provisioned view for a session (one-shot checkout). - /// - /// Removes the credentials half (required) and any live connection-options half for `jti`. - /// Returns `None` if credentials are missing or expired. - /// - /// **Contract:** injection mappings are consumed when the injection path checks them out. - /// They are not restored after a failed TLS/CredSSP attempt. `time_to_live` is how long the - /// entry may wait for that first checkout, not a retry budget. Re-provision to try again. - /// Callers must [`Self::has_mapping`] (or equivalent) before taking so token-only rows are - /// not destroyed by unrelated RDP connections. + /// Test helper that takes either a token-only or mapped entry. + #[cfg(test)] pub(crate) fn take(&self, jti: Uuid) -> Option { let now = time::OffsetDateTime::now_utc(); let (token, mapping) = { - let mut entries = self.credentials.lock(); - let entry = entries.remove(&jti)?; + let mut credentials = self.credentials.lock(); + let entry = credentials.entries.remove(&jti)?; if now >= entry.expires_at { warn!(%jti, "Provisioned credentials expired before the connection arrived"); return None; } + if entry.mapping.is_some() { + credentials.consumed.insert(jti, entry.expires_at); + } (entry.token, entry.mapping) }; @@ -183,6 +204,64 @@ impl ProvisioningStore { connection_options, }) } + + /// Atomically validate and consume an injection mapping (one-shot checkout). + /// + /// The mapping is not restored after a failed TLS/CredSSP attempt. + /// `time_to_live` is how long it may wait for first checkout, not a retry budget. + /// A consumed tombstone makes subsequent attempts fail explicitly until expiry or re-provisioning. + pub(crate) fn take_mapping(&self, jti: Uuid, token: &str) -> anyhow::Result { + let now = time::OffsetDateTime::now_utc(); + + let (token, mapping) = { + let mut credentials = self.credentials.lock(); + + if credentials + .consumed + .get(&jti) + .is_some_and(|expires_at| now < *expires_at) + { + anyhow::bail!("credential-injection material for {jti} was already consumed; re-provision to retry"); + } + credentials.consumed.remove(&jti); + + let entry = credentials + .entries + .get(&jti) + .context("provisioned credential-injection material is missing")?; + anyhow::ensure!( + now < entry.expires_at, + "provisioned credential-injection material expired" + ); + anyhow::ensure!(entry.mapping.is_some(), "provisioned entry has no credential mapping"); + anyhow::ensure!(token == entry.token, "token mismatch"); + + let entry = credentials + .entries + .remove(&jti) + .expect("entry exists while credential state lock is held"); + credentials.consumed.insert(jti, entry.expires_at); + (entry.token, entry.mapping) + }; + + let connection_options = { + let mut entries = self.connection_options.lock(); + match entries.remove(&jti) { + Some(entry) if now < entry.expires_at => Some(entry.connection_options), + Some(_) => { + warn!(%jti, "Provisioned connection options expired before the connection arrived"); + None + } + None => None, + } + }; + + Ok(ProvisioningEntry { + token, + mapping, + connection_options, + }) + } } pub struct CleanupTask { @@ -218,7 +297,10 @@ async fn cleanup_task(handle: ProvisioningStore, mut shutdown_signal: ShutdownSi } let now = time::OffsetDateTime::now_utc(); - handle.credentials.lock().retain(|_, entry| now < entry.expires_at); + let mut credentials = handle.credentials.lock(); + credentials.entries.retain(|_, entry| now < entry.expires_at); + credentials.consumed.retain(|_, expires_at| now < *expires_at); + drop(credentials); handle .connection_options .lock() @@ -337,4 +419,72 @@ mod tests { assert!(!store.insert_connection_options(jti, options(), time::Duration::minutes(5))); assert!(store.insert_connection_options(jti, options(), time::Duration::minutes(5))); } + + #[test] + fn consumed_mapping_is_explicit_until_reprovisioned() { + let store = ProvisioningStore::new(); + let jti = Uuid::new_v4(); + let token = association_token(jti); + store + .insert_credentials(token.clone(), Some(mapping()), time::Duration::minutes(5)) + .expect("insert"); + + assert_eq!(store.mapping_status(jti), MappingStatus::Available); + store.take_mapping(jti, &token).expect("first checkout"); + assert_eq!(store.mapping_status(jti), MappingStatus::Consumed); + let error = store.take_mapping(jti, &token).expect_err("second checkout fails"); + assert!(format!("{error:#}").contains("already consumed")); + + store + .insert_credentials(token, Some(mapping()), time::Duration::minutes(5)) + .expect("re-provision"); + assert_eq!(store.mapping_status(jti), MappingStatus::Available); + } + + #[test] + fn token_mismatch_does_not_consume_mapping() { + let store = ProvisioningStore::new(); + let jti = Uuid::new_v4(); + let token = association_token(jti); + store + .insert_credentials(token.clone(), Some(mapping()), time::Duration::minutes(5)) + .expect("insert"); + + let error = store.take_mapping(jti, "different token").expect_err("mismatch"); + assert!(format!("{error:#}").contains("token mismatch")); + assert_eq!(store.mapping_status(jti), MappingStatus::Available); + store.take_mapping(jti, &token).expect("valid checkout"); + } + + #[test] + fn concurrent_mapping_checkout_has_one_winner() { + let store = ProvisioningStore::new(); + let jti = Uuid::new_v4(); + let token = association_token(jti); + store + .insert_credentials(token.clone(), Some(mapping()), time::Duration::minutes(5)) + .expect("insert"); + + let barrier = Arc::new(std::sync::Barrier::new(3)); + let handles: Vec<_> = (0..2) + .map(|_| { + let store = store.clone(); + let token = token.clone(); + let barrier = Arc::clone(&barrier); + std::thread::spawn(move || { + barrier.wait(); + store.take_mapping(jti, &token) + }) + }) + .collect(); + barrier.wait(); + + let results: Vec<_> = handles + .into_iter() + .map(|handle| handle.join().expect("thread")) + .collect(); + assert_eq!(results.iter().filter(|result| result.is_ok()).count(), 1); + let error = results.into_iter().find_map(Result::err).expect("one failure"); + assert!(format!("{error:#}").contains("already consumed")); + } } diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index c5bb4314c..e30d635cc 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -17,7 +17,7 @@ const PCB_TRANSMIT_DEADLINE: Duration = Duration::from_secs(10); use crate::config::Conf; use crate::credential_injection::{CredentialInjection, SyntheticKdcRegistry}; -use crate::provisioning::ProvisioningStore; +use crate::provisioning::{MappingStatus, ProvisioningStore}; use crate::proxy::Proxy; use crate::recording::ActiveRecordings; use crate::session::{ConnectionModeDetails, DisconnectInterest, DisconnectedInfo, SessionInfo, SessionMessageSender}; @@ -484,6 +484,11 @@ async fn handle_with_credential_injection( .proxy_auth .clone() .context("missing token in RDCleanPath PDU")?; + anyhow::ensure!( + provisioning.mapping_status(claims.jti) != MappingStatus::Consumed, + "credential-injection material for {} was already consumed; re-provision to retry", + claims.jti, + ); // Connect before checkout so a target connect failure does not consume one-shot credentials. let ConnectedRdpServer { @@ -496,15 +501,17 @@ async fn handle_with_credential_injection( .context("RDCleanPath connection failed")?; let x224_rsp = x224_rsp.context("RDCleanPath credential injection requires X.224")?; - let entry = provisioning - .take(claims.jti) - .context("provisioned credentials missing after authorization")?; - anyhow::ensure!(entry.mapping.is_some(), "provisioned entry has no credential mapping"); - anyhow::ensure!(token == entry.token, "token mismatch"); - - let kerberos_enabled = crate::credential_injection::kerberos_injection_opt_in(&conf); - let credential_injection = CredentialInjection::from_provisioned(claims.jti, entry, kerberos_enabled)? - .register_if_kerberos(synthetic_kdc_registry); + let kerberos_enabled = crate::credential_injection::kerberos_injection_opt_in( + conf.debug.enable_unstable, + conf.debug.kerberos_credential_injection, + ); + let credential_injection = CredentialInjection::checkout( + provisioning, + synthetic_kdc_registry, + claims.jti, + &token, + kerberos_enabled, + )?; let gateway_cert_chain_handle = tokio::spawn(crate::tls::get_cert_chain_for_acceptor_cached( gateway_hostname, @@ -615,10 +622,13 @@ pub async fn handle( .context("missing token in RDCleanPath PDU")?; // If a credential mapping has been pushed, switch to proxy-based credential injection. - // Peek only here — take after authorize + server connect so an unverified token cannot - // burn a victim JTI, and a failed target connect does not consume the one-shot entry. + // Peek here without consuming. + // Checkout happens after authorization and target connection to protect the JTI from invalid requests and failures. if let Some(jti) = crate::token::extract_jti(token).ok() - && provisioning.has_mapping(jti) + && matches!( + provisioning.mapping_status(jti), + MappingStatus::Available | MappingStatus::Consumed + ) { // VMConnect needs pre-X.224 CredSSP against the Hyper-V host cert on the client. // Proxy CredSSP MITM is X.224-first and is not supported for this ordering. From cfd91407b8e38f057e46eec4a83f8ee2d7afe48f Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Mon, 17 Aug 2026 15:31:24 -0400 Subject: [PATCH 08/12] docs(openapi): document one-shot provisioning TTL Describe time_to_live as the first-checkout window for credential-injection mappings and state that failed attempts require re-provisioning. Regenerate the published specification and clients. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- devolutions-gateway/openapi/doc/index.adoc | 2 +- .../openapi/dotnet-client/docs/PreflightOperation.md | 3 +-- .../Model/PreflightOperation.cs | 6 +++--- devolutions-gateway/openapi/gateway-api.yaml | 9 ++++++--- .../ts-angular-client/model/preflightOperation.ts | 2 +- devolutions-gateway/src/openapi.rs | 9 ++++++--- 6 files changed, 18 insertions(+), 13 deletions(-) diff --git a/devolutions-gateway/openapi/doc/index.adoc b/devolutions-gateway/openapi/doc/index.adoc index 6910ffa17..42bd35a87 100644 --- a/devolutions-gateway/openapi/doc/index.adoc +++ b/devolutions-gateway/openapi/doc/index.adoc @@ -4445,7 +4445,7 @@ Current auto-update schedule for Devolutions Agent. | | X | Integer -| Minimum persistence duration in seconds for the data provisioned via this operation. Optional parameter for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds. +| How long provisioned data may wait for first use, in seconds. Optional for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\". Credential-injection mappings are consumed once when a session starts and are not restored after a failed attempt. Re-provision to retry. | int32 | token diff --git a/devolutions-gateway/openapi/dotnet-client/docs/PreflightOperation.md b/devolutions-gateway/openapi/dotnet-client/docs/PreflightOperation.md index 4c16436b1..aadff24ba 100644 --- a/devolutions-gateway/openapi/dotnet-client/docs/PreflightOperation.md +++ b/devolutions-gateway/openapi/dotnet-client/docs/PreflightOperation.md @@ -10,8 +10,7 @@ Name | Type | Description | Notes **Kind** | **PreflightOperationKind** | | **ProxyCredential** | [**AppCredential**](AppCredential.md) | | [optional] **TargetCredential** | [**AppCredential**](AppCredential.md) | | [optional] -**TimeToLive** | **int?** | Minimum persistence duration in seconds for the data provisioned via this operation. Optional parameter for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds. | [optional] +**TimeToLive** | **int?** | How long provisioned data may wait for first use, in seconds. Optional for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\". Credential-injection mappings are consumed once when a session starts and are not restored after a failed attempt. Re-provision to retry. | [optional] **Token** | **string** | The token to be stored on the proxy-side. Required for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds. | [optional] [[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md) - diff --git a/devolutions-gateway/openapi/dotnet-client/src/Devolutions.Gateway.Client/Model/PreflightOperation.cs b/devolutions-gateway/openapi/dotnet-client/src/Devolutions.Gateway.Client/Model/PreflightOperation.cs index 59212a095..6b90ff714 100644 --- a/devolutions-gateway/openapi/dotnet-client/src/Devolutions.Gateway.Client/Model/PreflightOperation.cs +++ b/devolutions-gateway/openapi/dotnet-client/src/Devolutions.Gateway.Client/Model/PreflightOperation.cs @@ -53,7 +53,7 @@ protected PreflightOperation() { } /// kind (required). /// proxyCredential. /// targetCredential. - /// Minimum persistence duration in seconds for the data provisioned via this operation. Optional parameter for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds.. + /// How long provisioned data may wait for first use, in seconds. Optional for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\". Credential-injection mappings are consumed once when a session starts and are not restored after a failed attempt. Re-provision to retry.. /// The token to be stored on the proxy-side. Required for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds.. public PreflightOperation(TargetConnectionOptions connectionOptions = default(TargetConnectionOptions), string hostToResolve = default(string), Guid id = default(Guid), PreflightOperationKind kind = default(PreflightOperationKind), AppCredential proxyCredential = default(AppCredential), AppCredential targetCredential = default(AppCredential), int? timeToLive = default(int?), string token = default(string)) { @@ -100,9 +100,9 @@ protected PreflightOperation() { } public AppCredential TargetCredential { get; set; } /// - /// Minimum persistence duration in seconds for the data provisioned via this operation. Optional parameter for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds. + /// How long provisioned data may wait for first use, in seconds. Optional for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\". Credential-injection mappings are consumed once when a session starts and are not restored after a failed attempt. Re-provision to retry. /// - /// Minimum persistence duration in seconds for the data provisioned via this operation. Optional parameter for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds. + /// How long provisioned data may wait for first use, in seconds. Optional for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\". Credential-injection mappings are consumed once when a session starts and are not restored after a failed attempt. Re-provision to retry. [DataMember(Name = "time_to_live", EmitDefaultValue = true)] public int? TimeToLive { get; set; } diff --git a/devolutions-gateway/openapi/gateway-api.yaml b/devolutions-gateway/openapi/gateway-api.yaml index 851fdd6af..53cfbd9c6 100644 --- a/devolutions-gateway/openapi/gateway-api.yaml +++ b/devolutions-gateway/openapi/gateway-api.yaml @@ -1837,10 +1837,13 @@ components: type: integer format: int32 description: |- - Minimum persistence duration in seconds for the data provisioned via this operation. + How long provisioned data may wait for first use, in seconds. - Optional parameter for "provision-token", "provision-credentials", and - "provision-connection-options" kinds. + Optional for "provision-token", "provision-credentials", and + "provision-connection-options". + Credential-injection mappings are consumed once when a session starts and are not restored + after a failed attempt. + Re-provision to retry. nullable: true minimum: 0 token: diff --git a/devolutions-gateway/openapi/ts-angular-client/model/preflightOperation.ts b/devolutions-gateway/openapi/ts-angular-client/model/preflightOperation.ts index 76d51046a..7a115b294 100644 --- a/devolutions-gateway/openapi/ts-angular-client/model/preflightOperation.ts +++ b/devolutions-gateway/openapi/ts-angular-client/model/preflightOperation.ts @@ -26,7 +26,7 @@ export interface PreflightOperation { proxy_credential?: AppCredential | null; target_credential?: AppCredential | null; /** - * Minimum persistence duration in seconds for the data provisioned via this operation. Optional parameter for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds. + * How long provisioned data may wait for first use, in seconds. Optional for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\". Credential-injection mappings are consumed once when a session starts and are not restored after a failed attempt. Re-provision to retry. */ time_to_live?: number | null; /** diff --git a/devolutions-gateway/src/openapi.rs b/devolutions-gateway/src/openapi.rs index 73b22bf60..506ac8589 100644 --- a/devolutions-gateway/src/openapi.rs +++ b/devolutions-gateway/src/openapi.rs @@ -393,10 +393,13 @@ struct PreflightOperation { /// /// Required for "resolve-host" kind. host_to_resolve: Option, - /// Minimum persistence duration in seconds for the data provisioned via this operation. + /// How long provisioned data may wait for first use, in seconds. /// - /// Optional parameter for "provision-token", "provision-credentials", and - /// "provision-connection-options" kinds. + /// Optional for "provision-token", "provision-credentials", and + /// "provision-connection-options". + /// Credential-injection mappings are consumed once when a session starts and are not restored + /// after a failed attempt. + /// Re-provision to retry. time_to_live: Option, } From 5c8b4129514ef0af850bf4632e57da429bd86b23 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Mon, 17 Aug 2026 15:46:45 -0400 Subject: [PATCH 09/12] docs(dgw): clarify Kerberos SPN contract State that supported credential-injection clients retain association dst_hst as their logical TERMSRV service name even when the transport endpoint is a Gateway listener. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- devolutions-gateway/src/api/kdc_proxy.rs | 7 +++---- devolutions-gateway/src/credential_injection.rs | 5 +++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/devolutions-gateway/src/api/kdc_proxy.rs b/devolutions-gateway/src/api/kdc_proxy.rs index ad6aebd8c..f90eee2ae 100644 --- a/devolutions-gateway/src/api/kdc_proxy.rs +++ b/devolutions-gateway/src/api/kdc_proxy.rs @@ -183,10 +183,9 @@ mod tests { #[test] fn enforce_realm_mismatch_passes_under_bypass() { - // `bypass=true` is the `__debug__.disable_token_validation` downgrade. CBenoit asked - // for explicit coverage of this branch because it is the only place the realm - // authorization is intentionally weakened, and slipping the gate (e.g. by inverting the - // condition) would only surface in production. + // `bypass=true` is the `__debug__.disable_token_validation` downgrade. + // This is the only branch where realm authorization is intentionally weakened, so pin it + // explicitly to catch an inverted gate. assert!(enforce_realm_token_match("ad.example", "evil.example", true).is_ok()); } diff --git a/devolutions-gateway/src/credential_injection.rs b/devolutions-gateway/src/credential_injection.rs index d21ae0a06..45c1dfe75 100644 --- a/devolutions-gateway/src/credential_injection.rs +++ b/devolutions-gateway/src/credential_injection.rs @@ -323,6 +323,11 @@ impl CredentialInjectionKdc { } /// Session destination host from association `dst_hst` (not Gateway `conf.hostname`). + /// + /// Supported clients retain this logical destination when forming their `TERMSRV` SPN, even + /// when the transport endpoint is a Gateway listener. + /// Clients that derive the SPN from the Gateway transport hostname are not supported by the + /// unstable Kerberos credential-injection path. pub(crate) fn target_hostname(&self) -> &str { &self.target_hostname } From 29248b24e90a75533bfbf2931b983de51d614def Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Mon, 17 Aug 2026 17:03:59 -0400 Subject: [PATCH 10/12] revert: remove one-shot OpenAPI documentation Restore the pre-existing provisioning TTL wording and generated artifacts. The OpenAPI documentation update was outside the requested PR scope. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- devolutions-gateway/openapi/doc/index.adoc | 2 +- .../openapi/dotnet-client/docs/PreflightOperation.md | 3 ++- .../Model/PreflightOperation.cs | 6 +++--- devolutions-gateway/openapi/gateway-api.yaml | 9 +++------ .../ts-angular-client/model/preflightOperation.ts | 2 +- devolutions-gateway/src/openapi.rs | 9 +++------ 6 files changed, 13 insertions(+), 18 deletions(-) diff --git a/devolutions-gateway/openapi/doc/index.adoc b/devolutions-gateway/openapi/doc/index.adoc index 42bd35a87..6910ffa17 100644 --- a/devolutions-gateway/openapi/doc/index.adoc +++ b/devolutions-gateway/openapi/doc/index.adoc @@ -4445,7 +4445,7 @@ Current auto-update schedule for Devolutions Agent. | | X | Integer -| How long provisioned data may wait for first use, in seconds. Optional for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\". Credential-injection mappings are consumed once when a session starts and are not restored after a failed attempt. Re-provision to retry. +| Minimum persistence duration in seconds for the data provisioned via this operation. Optional parameter for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds. | int32 | token diff --git a/devolutions-gateway/openapi/dotnet-client/docs/PreflightOperation.md b/devolutions-gateway/openapi/dotnet-client/docs/PreflightOperation.md index aadff24ba..4c16436b1 100644 --- a/devolutions-gateway/openapi/dotnet-client/docs/PreflightOperation.md +++ b/devolutions-gateway/openapi/dotnet-client/docs/PreflightOperation.md @@ -10,7 +10,8 @@ Name | Type | Description | Notes **Kind** | **PreflightOperationKind** | | **ProxyCredential** | [**AppCredential**](AppCredential.md) | | [optional] **TargetCredential** | [**AppCredential**](AppCredential.md) | | [optional] -**TimeToLive** | **int?** | How long provisioned data may wait for first use, in seconds. Optional for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\". Credential-injection mappings are consumed once when a session starts and are not restored after a failed attempt. Re-provision to retry. | [optional] +**TimeToLive** | **int?** | Minimum persistence duration in seconds for the data provisioned via this operation. Optional parameter for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds. | [optional] **Token** | **string** | The token to be stored on the proxy-side. Required for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds. | [optional] [[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md) + diff --git a/devolutions-gateway/openapi/dotnet-client/src/Devolutions.Gateway.Client/Model/PreflightOperation.cs b/devolutions-gateway/openapi/dotnet-client/src/Devolutions.Gateway.Client/Model/PreflightOperation.cs index 6b90ff714..59212a095 100644 --- a/devolutions-gateway/openapi/dotnet-client/src/Devolutions.Gateway.Client/Model/PreflightOperation.cs +++ b/devolutions-gateway/openapi/dotnet-client/src/Devolutions.Gateway.Client/Model/PreflightOperation.cs @@ -53,7 +53,7 @@ protected PreflightOperation() { } /// kind (required). /// proxyCredential. /// targetCredential. - /// How long provisioned data may wait for first use, in seconds. Optional for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\". Credential-injection mappings are consumed once when a session starts and are not restored after a failed attempt. Re-provision to retry.. + /// Minimum persistence duration in seconds for the data provisioned via this operation. Optional parameter for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds.. /// The token to be stored on the proxy-side. Required for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds.. public PreflightOperation(TargetConnectionOptions connectionOptions = default(TargetConnectionOptions), string hostToResolve = default(string), Guid id = default(Guid), PreflightOperationKind kind = default(PreflightOperationKind), AppCredential proxyCredential = default(AppCredential), AppCredential targetCredential = default(AppCredential), int? timeToLive = default(int?), string token = default(string)) { @@ -100,9 +100,9 @@ protected PreflightOperation() { } public AppCredential TargetCredential { get; set; } /// - /// How long provisioned data may wait for first use, in seconds. Optional for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\". Credential-injection mappings are consumed once when a session starts and are not restored after a failed attempt. Re-provision to retry. + /// Minimum persistence duration in seconds for the data provisioned via this operation. Optional parameter for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds. /// - /// How long provisioned data may wait for first use, in seconds. Optional for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\". Credential-injection mappings are consumed once when a session starts and are not restored after a failed attempt. Re-provision to retry. + /// Minimum persistence duration in seconds for the data provisioned via this operation. Optional parameter for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds. [DataMember(Name = "time_to_live", EmitDefaultValue = true)] public int? TimeToLive { get; set; } diff --git a/devolutions-gateway/openapi/gateway-api.yaml b/devolutions-gateway/openapi/gateway-api.yaml index 53cfbd9c6..851fdd6af 100644 --- a/devolutions-gateway/openapi/gateway-api.yaml +++ b/devolutions-gateway/openapi/gateway-api.yaml @@ -1837,13 +1837,10 @@ components: type: integer format: int32 description: |- - How long provisioned data may wait for first use, in seconds. + Minimum persistence duration in seconds for the data provisioned via this operation. - Optional for "provision-token", "provision-credentials", and - "provision-connection-options". - Credential-injection mappings are consumed once when a session starts and are not restored - after a failed attempt. - Re-provision to retry. + Optional parameter for "provision-token", "provision-credentials", and + "provision-connection-options" kinds. nullable: true minimum: 0 token: diff --git a/devolutions-gateway/openapi/ts-angular-client/model/preflightOperation.ts b/devolutions-gateway/openapi/ts-angular-client/model/preflightOperation.ts index 7a115b294..76d51046a 100644 --- a/devolutions-gateway/openapi/ts-angular-client/model/preflightOperation.ts +++ b/devolutions-gateway/openapi/ts-angular-client/model/preflightOperation.ts @@ -26,7 +26,7 @@ export interface PreflightOperation { proxy_credential?: AppCredential | null; target_credential?: AppCredential | null; /** - * How long provisioned data may wait for first use, in seconds. Optional for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\". Credential-injection mappings are consumed once when a session starts and are not restored after a failed attempt. Re-provision to retry. + * Minimum persistence duration in seconds for the data provisioned via this operation. Optional parameter for \"provision-token\", \"provision-credentials\", and \"provision-connection-options\" kinds. */ time_to_live?: number | null; /** diff --git a/devolutions-gateway/src/openapi.rs b/devolutions-gateway/src/openapi.rs index 506ac8589..73b22bf60 100644 --- a/devolutions-gateway/src/openapi.rs +++ b/devolutions-gateway/src/openapi.rs @@ -393,13 +393,10 @@ struct PreflightOperation { /// /// Required for "resolve-host" kind. host_to_resolve: Option, - /// How long provisioned data may wait for first use, in seconds. + /// Minimum persistence duration in seconds for the data provisioned via this operation. /// - /// Optional for "provision-token", "provision-credentials", and - /// "provision-connection-options". - /// Credential-injection mappings are consumed once when a session starts and are not restored - /// after a failed attempt. - /// Re-provision to retry. + /// Optional parameter for "provision-token", "provision-credentials", and + /// "provision-connection-options" kinds. time_to_live: Option, } From eb4587a4d8dc054f58d2a1a0f9c10efff92f724a Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Thu, 20 Aug 2026 14:38:28 -0400 Subject: [PATCH 11/12] fix(dgw): keep injection mappings across reconnects Token validation already accepts the same association JWT inside jet_reuse, but checkout consumed the mapping on first use. Native RDM reconnects reuse that JWT without DVLS, so injection failed or silently forwarded. Keep encrypted mappings until the token acceptance deadline, authorize before choosing injection, fail closed when required material is gone, and reuse one synthetic KDC per provisioning generation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/credential_injection.rs | 297 +++++++++++++++-- devolutions-gateway/src/generic_client.rs | 125 +++---- devolutions-gateway/src/provisioning.rs | 304 ++++++++++++------ devolutions-gateway/src/rd_clean_path.rs | 133 ++++---- devolutions-gateway/src/rdp_proxy/credssp.rs | 7 +- devolutions-gateway/src/token.rs | 17 + 6 files changed, 621 insertions(+), 262 deletions(-) diff --git a/devolutions-gateway/src/credential_injection.rs b/devolutions-gateway/src/credential_injection.rs index 45c1dfe75..825e187c1 100644 --- a/devolutions-gateway/src/credential_injection.rs +++ b/devolutions-gateway/src/credential_injection.rs @@ -1,9 +1,10 @@ //! Credential-injection runtime for RDP. //! -//! - Provisioned material lives in [`crate::provisioning::ProvisioningStore`] until checkout. +//! - Provisioned mappings live in [`crate::provisioning::ProvisioningStore`]. //! - [`CredentialInjection::from_provisioned`] builds a session-scoped injection plan. -//! - Kerberos sessions publish a [`CredentialInjectionKdc`] into [`SyntheticKdcRegistry`]; -//! `/jet/KdcProxy` resolves only that registry (not the provisioning store). +//! - Kerberos sessions reuse one synthetic KDC per provisioning generation, then publish a +//! [`CredentialInjectionKdc`] into [`SyntheticKdcRegistry`] for the connection. +//! - `/jet/KdcProxy` resolves only that registry (not the provisioning store). use std::collections::HashMap; use std::fmt; @@ -92,10 +93,14 @@ pub(crate) enum PreparedCredentialInjection { impl PreparedCredentialInjection { /// Publish the synthetic KDC when this is Kerberos; NTLM is a no-op pass-through. - pub(crate) fn register_if_kerberos(self, registry: &SyntheticKdcRegistry) -> CredentialInjection { + pub(crate) fn register_if_kerberos( + self, + registry: &SyntheticKdcRegistry, + provision_generation: u64, + ) -> CredentialInjection { match self { Self::Kerberos(injection) => { - let registration = registry.register(Arc::clone(&injection.synthetic)); + let registration = registry.register(Arc::clone(&injection.synthetic), provision_generation); debug!( jti = %injection.synthetic.jti(), "Registered synthetic KDC for credential-injection session" @@ -156,9 +161,21 @@ impl CredentialInjection { kerberos_enabled: bool, ) -> anyhow::Result { let entry = provisioning - .take_mapping(jti, token) + .get_mapping(jti, token) .with_context(|| format!("checkout credential-injection material for {jti}"))?; - Ok(Self::from_provisioned(jti, entry, kerberos_enabled)?.register_if_kerberos(registry)) + let generation = entry.generation; + let kdc_expires_at = entry.kdc_expires_at; + registry.discard_stale_session_kdc(jti, generation); + let prepared = Self::from_provisioned(jti, entry, kerberos_enabled)?; + let prepared = match prepared { + PreparedCredentialInjection::Kerberos(mut injection) => { + let expires_at = kdc_expires_at.context("mapped Kerberos row has no token deadline")?; + injection.synthetic = registry.intern_session_kdc(jti, generation, expires_at, injection.synthetic); + PreparedCredentialInjection::Kerberos(injection) + } + ntlm @ PreparedCredentialInjection::Ntlm(_) => ntlm, + }; + Ok(prepared.register_if_kerberos(registry, generation)) } pub(crate) fn jti(&self) -> Uuid { @@ -206,6 +223,8 @@ impl CredentialInjection { token, mapping, connection_options, + generation: _, + kdc_expires_at: _, } = credential_entry; let mapping = mapping.context("credential-injection state has no mapping")?; @@ -522,9 +541,9 @@ fn random_32_bytes() -> Vec { /// - `/jet/KdcProxy` only looks up published entries; it never builds a KDC from /// [`crate::provisioning::ProvisioningStore`]. /// -/// Entries are connection-scoped via [`SyntheticKdcRegistration`] and removed when the owning session ends. -/// Generations prevent an older session from unpublishing a replacement. -/// Re-provisioning the same JTI can register that replacement before the older session ends. +/// Connection leases publish to `/jet/KdcProxy`. The same provisioning generation is +/// reference-counted; a newer generation replaces an older one. An older lease cannot unpublish +/// or overwrite a newer generation. #[derive(Debug, Clone)] pub struct SyntheticKdcRegistry { inner: Arc>, @@ -533,31 +552,51 @@ pub struct SyntheticKdcRegistry { #[derive(Debug, Default)] struct RegistryInner { live: HashMap, - next_generation: u64, + session: HashMap, } #[derive(Debug, Clone)] struct PublishedSyntheticKdc { - generation: u64, + provision_generation: u64, + leases: u32, + kdc: Arc, +} + +#[derive(Debug, Clone)] +struct SessionSyntheticKdc { + provision_generation: u64, + expires_at: time::OffsetDateTime, kdc: Arc, } +fn generation_is_newer(candidate: u64, than: u64) -> bool { + candidate != than && candidate.wrapping_sub(than) < than.wrapping_sub(candidate) +} + /// RAII lease for a published synthetic KDC. pub(crate) struct SyntheticKdcRegistration { registry: SyntheticKdcRegistry, jti: Uuid, - generation: u64, + provision_generation: u64, } impl Drop for SyntheticKdcRegistration { fn drop(&mut self) { let mut inner = self.registry.inner.lock(); - let Some(current) = inner.live.get(&self.jti) else { + let Some(current) = inner.live.get_mut(&self.jti) else { return; }; - if current.generation == self.generation { + if current.provision_generation != self.provision_generation { + return; + } + current.leases = current.leases.saturating_sub(1); + if current.leases == 0 { inner.live.remove(&self.jti); - debug!(jti = %self.jti, generation = self.generation, "Unpublished synthetic KDC"); + debug!( + jti = %self.jti, + provision_generation = self.provision_generation, + "Unpublished synthetic KDC" + ); } } } @@ -575,23 +614,112 @@ impl SyntheticKdcRegistry { } } - pub(crate) fn register(&self, kdc: Arc) -> SyntheticKdcRegistration { + pub(crate) fn register( + &self, + kdc: Arc, + provision_generation: u64, + ) -> SyntheticKdcRegistration { let jti = kdc.jti(); let mut inner = self.inner.lock(); - inner.next_generation = inner.next_generation.wrapping_add(1); - let generation = inner.next_generation; - inner.live.insert(jti, PublishedSyntheticKdc { generation, kdc }); - debug!(%jti, generation, "Published synthetic KDC"); + match inner.live.get_mut(&jti) { + Some(current) if current.provision_generation == provision_generation => { + current.leases = current.leases.saturating_add(1); + } + Some(current) if generation_is_newer(current.provision_generation, provision_generation) => {} + _ => { + inner.live.insert( + jti, + PublishedSyntheticKdc { + provision_generation, + leases: 1, + kdc, + }, + ); + debug!(%jti, provision_generation, "Published synthetic KDC"); + } + } SyntheticKdcRegistration { registry: self.clone(), jti, - generation, + provision_generation, } } pub(crate) fn get(&self, jti: Uuid) -> Option> { self.inner.lock().live.get(&jti).map(|entry| Arc::clone(&entry.kdc)) } + + /// Drop interned KDCs that are expired or older than this provisioning generation. + pub(crate) fn discard_stale_session_kdc(&self, jti: Uuid, provision_generation: u64) { + let now = time::OffsetDateTime::now_utc(); + let mut inner = self.inner.lock(); + inner.session.retain(|_, entry| now < entry.expires_at); + if inner + .session + .get(&jti) + .is_some_and(|entry| generation_is_newer(provision_generation, entry.provision_generation)) + { + inner.session.remove(&jti); + } + } + + /// Reuse the synthetic KDC for this provisioning generation until `expires_at`. + /// + /// A later `provision-credentials` bumps the generation and replaces the cached KDC. + /// An older generation never overwrites a newer interned KDC. + pub(crate) fn intern_session_kdc( + &self, + jti: Uuid, + provision_generation: u64, + expires_at: time::OffsetDateTime, + kdc: Arc, + ) -> Arc { + let now = time::OffsetDateTime::now_utc(); + let mut inner = self.inner.lock(); + inner.session.retain(|_, entry| now < entry.expires_at); + if now >= expires_at { + if inner + .session + .get(&jti) + .is_some_and(|entry| entry.provision_generation == provision_generation) + { + inner.session.remove(&jti); + } + return kdc; + } + if let Some(existing) = inner.session.get(&jti) { + if existing.provision_generation == provision_generation { + return Arc::clone(&existing.kdc); + } + if generation_is_newer(existing.provision_generation, provision_generation) { + return kdc; + } + } + inner.session.insert( + jti, + SessionSyntheticKdc { + provision_generation, + expires_at, + kdc: Arc::clone(&kdc), + }, + ); + kdc + } + + #[cfg(test)] + pub(crate) fn session_kdc_live(&self, jti: Uuid) -> bool { + self.interned_kdc(jti).is_some() + } + + #[cfg(test)] + fn interned_kdc(&self, jti: Uuid) -> Option> { + let now = time::OffsetDateTime::now_utc(); + self.inner + .lock() + .session + .get(&jti) + .and_then(|entry| (now < entry.expires_at).then(|| Arc::clone(&entry.kdc))) + } } #[cfg(test)] @@ -628,7 +756,8 @@ mod tests { fn association_token(jti: Uuid) -> String { unsigned_jws(serde_json::json!({ "jti": jti, - "dst_hst": "target.example:3389" + "dst_hst": "target.example:3389", + "exp": time::OffsetDateTime::now_utc().unix_timestamp() + 3600 })) } @@ -710,7 +839,7 @@ mod tests { let registry = SyntheticKdcRegistry::new(); let injection = CredentialInjection::from_provisioned(jti, entry, false) .expect("prepared") - .register_if_kerberos(®istry); + .register_if_kerberos(®istry, 1); assert!(!injection.uses_kerberos()); assert!(registry.get(jti).is_none()); } @@ -722,7 +851,7 @@ mod tests { let registry = SyntheticKdcRegistry::new(); let injection = CredentialInjection::from_provisioned(jti, entry, true) .expect("prepared") - .register_if_kerberos(®istry); + .register_if_kerberos(®istry, 1); assert!(!injection.uses_kerberos()); } @@ -740,11 +869,11 @@ mod tests { let store = stock_with_mapping(jti, "administrator@example.invalid"); store.insert_connection_options(jti, kdc_options(), time::Duration::minutes(5)); let entry = store.take(jti).expect("entry"); - assert!(store.take(jti).is_none(), "take is one-shot"); + assert!(store.take(jti).is_none(), "test helper take removes the row"); let registry = SyntheticKdcRegistry::new(); let prepared = CredentialInjection::from_provisioned(jti, entry, true).expect("prepared"); assert!(registry.get(jti).is_none(), "not published until register_if_kerberos"); - let injection = prepared.register_if_kerberos(®istry); + let injection = prepared.register_if_kerberos(®istry, 1); assert!(injection.uses_kerberos()); assert!(registry.get(jti).is_some()); assert_eq!( @@ -753,6 +882,80 @@ mod tests { ); } + #[test] + fn checkout_reuses_synthetic_kdc_for_the_same_generation() { + let jti = Uuid::new_v4(); + let token = association_token(jti); + let store = ProvisioningStore::new(); + store + .insert_credentials( + token.clone(), + Some(cleartext_mapping_with_target_username("administrator@example.invalid")), + time::Duration::minutes(5), + ) + .expect("insert"); + store.insert_connection_options(jti, kdc_options(), time::Duration::minutes(5)); + let registry = SyntheticKdcRegistry::new(); + + let first = CredentialInjection::checkout(&store, ®istry, jti, &token, true).expect("first"); + let first_ptr = std::ptr::from_ref(first.as_kerberos().expect("kerberos").synthetic_kdc()); + drop(first); + + let second = CredentialInjection::checkout(&store, ®istry, jti, &token, true).expect("second"); + let second_ptr = std::ptr::from_ref(second.as_kerberos().expect("kerberos").synthetic_kdc()); + assert_eq!(first_ptr, second_ptr); + + store + .insert_credentials( + token.clone(), + Some(cleartext_mapping_with_target_username("administrator@example.invalid")), + time::Duration::minutes(5), + ) + .expect("re-provision"); + store.insert_connection_options(jti, kdc_options(), time::Duration::minutes(5)); + let third = CredentialInjection::checkout(&store, ®istry, jti, &token, true).expect("third"); + let third_ptr = std::ptr::from_ref(third.as_kerberos().expect("kerberos").synthetic_kdc()); + assert_ne!(first_ptr, third_ptr); + } + + #[test] + fn interned_kdc_is_not_kept_past_deadline() { + let jti = Uuid::new_v4(); + let registry = SyntheticKdcRegistry::new(); + let kdc = Arc::new(dummy_kdc(jti)); + let expired = time::OffsetDateTime::now_utc() - time::Duration::seconds(1); + registry.intern_session_kdc(jti, 1, expired, kdc); + assert!(!registry.session_kdc_live(jti)); + } + + #[test] + fn ntlm_checkout_discards_previous_generation_kdc() { + let jti = Uuid::new_v4(); + let token = association_token(jti); + let store = ProvisioningStore::new(); + store + .insert_credentials( + token.clone(), + Some(cleartext_mapping_with_target_username("administrator@example.invalid")), + time::Duration::minutes(5), + ) + .expect("insert"); + store.insert_connection_options(jti, kdc_options(), time::Duration::minutes(5)); + let registry = SyntheticKdcRegistry::new(); + let _kerberos = CredentialInjection::checkout(&store, ®istry, jti, &token, true).expect("kerberos"); + assert!(registry.session_kdc_live(jti)); + + store + .insert_credentials( + token.clone(), + Some(cleartext_mapping_with_target_username("Administrator")), + time::Duration::minutes(5), + ) + .expect("ntlm re-provision"); + let _ntlm = CredentialInjection::checkout(&store, ®istry, jti, &token, true).expect("ntlm"); + assert!(!registry.session_kdc_live(jti)); + } + #[test] fn provisioned_krb_kdc_is_carried_on_kerberos_injection() { // Pins provision → from_provisioned → target_kdc for the CredSSP client leg. @@ -762,7 +965,7 @@ mod tests { let entry = store.take(jti).expect("entry"); let injection = CredentialInjection::from_provisioned(jti, entry, true) .expect("prepared") - .register_if_kerberos(&SyntheticKdcRegistry::new()); + .register_if_kerberos(&SyntheticKdcRegistry::new(), 1); assert_eq!( injection.as_kerberos().expect("kerberos").target_kdc().as_str(), @@ -781,7 +984,8 @@ mod tests { .insert_credentials( unsigned_jws(serde_json::json!({ "jti": jti, - "dst_hst": "it-help-dc.corp.example:3389" + "dst_hst": "it-help-dc.corp.example:3389", + "exp": time::OffsetDateTime::now_utc().unix_timestamp() + 3600 })), Some(cleartext_mapping_with_target_username("administrator@example.invalid")), time::Duration::minutes(5), @@ -791,7 +995,7 @@ mod tests { let entry = store.take(jti).expect("entry"); let injection = CredentialInjection::from_provisioned(jti, entry, true) .expect("prepared") - .register_if_kerberos(&SyntheticKdcRegistry::new()); + .register_if_kerberos(&SyntheticKdcRegistry::new(), 1); assert_eq!( injection @@ -808,11 +1012,11 @@ mod tests { let registry = SyntheticKdcRegistry::new(); let jti = Uuid::new_v4(); let first = Arc::new(dummy_kdc(jti)); - let first_registration = registry.register(Arc::clone(&first)); + let first_registration = registry.register(Arc::clone(&first), 1); assert!(Arc::ptr_eq(®istry.get(jti).expect("first"), &first)); let second = Arc::new(dummy_kdc(jti)); - let second_registration = registry.register(Arc::clone(&second)); + let second_registration = registry.register(Arc::clone(&second), 2); drop(first_registration); assert!(Arc::ptr_eq(®istry.get(jti).expect("successor"), &second)); @@ -820,6 +1024,35 @@ mod tests { assert!(registry.get(jti).is_none()); } + #[test] + fn same_generation_leases_unpublish_on_last_drop() { + let registry = SyntheticKdcRegistry::new(); + let jti = Uuid::new_v4(); + let kdc = Arc::new(dummy_kdc(jti)); + let first = registry.register(Arc::clone(&kdc), 1); + let second = registry.register(Arc::clone(&kdc), 1); + drop(first); + assert!(registry.get(jti).is_some()); + drop(second); + assert!(registry.get(jti).is_none()); + } + + #[test] + fn stale_generation_does_not_replace_interned_kdc() { + let jti = Uuid::new_v4(); + let registry = SyntheticKdcRegistry::new(); + let newer = Arc::new(dummy_kdc(jti)); + let older = Arc::new(dummy_kdc(jti)); + let deadline = time::OffsetDateTime::now_utc() + time::Duration::minutes(5); + let interned = registry.intern_session_kdc(jti, 2, deadline, Arc::clone(&newer)); + assert!(Arc::ptr_eq(&interned, &newer)); + let rejected = registry.intern_session_kdc(jti, 1, deadline, Arc::clone(&older)); + assert!(Arc::ptr_eq(&rejected, &older)); + assert!(Arc::ptr_eq(®istry.interned_kdc(jti).expect("kept"), &newer)); + registry.discard_stale_session_kdc(jti, 1); + assert!(Arc::ptr_eq(®istry.interned_kdc(jti).expect("still kept"), &newer)); + } + #[test] fn kdc_proxy_cannot_invent_from_provisioning_store() { let jti = Uuid::new_v4(); diff --git a/devolutions-gateway/src/generic_client.rs b/devolutions-gateway/src/generic_client.rs index ef9121ec4..cd5646006 100644 --- a/devolutions-gateway/src/generic_client.rs +++ b/devolutions-gateway/src/generic_client.rs @@ -116,6 +116,21 @@ where RecordingPolicy::Proxy => anyhow::bail!("can't meet recording policy"), } + let is_rdp = claims.jet_ap == token::ApplicationProtocol::Known(token::Protocol::Rdp); + let mapping_status = if is_rdp { + provisioning.mapping_status(claims.jti) + } else { + MappingStatus::Absent + }; + let inject = match mapping_status { + MappingStatus::RequiredMissing => anyhow::bail!( + "credential-injection material for {} is missing or expired; re-provision to retry", + claims.jti + ), + MappingStatus::Available => true, + MappingStatus::Absent => false, + }; + let ConnectedUpstream { leg: mut server_stream, server_addr, @@ -131,8 +146,6 @@ where span.record("target", selected_target.to_string()); - let is_rdp = claims.jet_ap == token::ApplicationProtocol::Known(token::Protocol::Rdp); - let info = SessionInfo::builder() .id(claims.jet_aid) .application_protocol(claims.jet_ap) @@ -146,66 +159,56 @@ where let disconnect_interest = DisconnectInterest::from_reconnection_policy(claims.jet_reuse); - // Peek first so token-only provision rows are not consumed. - // Fail explicitly for consumed mappings instead of silently downgrading. - let mapping_status = if is_rdp { - provisioning.mapping_status(claims.jti) - } else { - MappingStatus::Absent - }; - match mapping_status { - MappingStatus::Consumed => { - anyhow::bail!( - "credential-injection material for {} was already consumed; re-provision to retry", + if inject { + let kerberos_enabled = crate::credential_injection::kerberos_injection_opt_in( + conf.debug.enable_unstable, + conf.debug.kerberos_credential_injection, + ); + let credential_injection = CredentialInjection::checkout( + &provisioning, + &synthetic_kdc_registry, + claims.jti, + token, + kerberos_enabled, + ) + .with_context(|| { + format!( + "credential-injection material for {} is missing or expired; re-provision to retry", claims.jti - ); - } - MappingStatus::Absent => {} - MappingStatus::Available => { - let kerberos_enabled = crate::credential_injection::kerberos_injection_opt_in( - conf.debug.enable_unstable, - conf.debug.kerberos_credential_injection, - ); - let credential_injection = CredentialInjection::checkout( - &provisioning, - &synthetic_kdc_registry, - claims.jti, - token, - kerberos_enabled, - )?; - - info!( - jti = %credential_injection.jti(), - kerberos = credential_injection.uses_kerberos(), - "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) - .session_info(info) - .client_addr(client_addr) - .client_stream(client_stream) - .server_addr(server_addr) - .server_stream(server_stream) - .sessions(sessions) - .subscriber_tx(subscriber_tx) - .credential_injection(credential_injection) - .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 - .context("encountered a failure during RDP proxying (credential injection)"); - } + ) + })?; + + info!( + jti = %credential_injection.jti(), + kerberos = credential_injection.uses_kerberos(), + "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) + .session_info(info) + .client_addr(client_addr) + .client_stream(client_stream) + .server_addr(server_addr) + .server_stream(server_stream) + .sessions(sessions) + .subscriber_tx(subscriber_tx) + .credential_injection(credential_injection) + .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 + .context("encountered a failure during RDP proxying (credential injection)"); } info!("Upstream forwarding"); diff --git a/devolutions-gateway/src/provisioning.rs b/devolutions-gateway/src/provisioning.rs index bc72b877b..991badf5b 100644 --- a/devolutions-gateway/src/provisioning.rs +++ b/devolutions-gateway/src/provisioning.rs @@ -42,6 +42,8 @@ pub struct ProvisioningEntry { pub(crate) token: String, pub(crate) mapping: Option, pub(crate) connection_options: Option, + pub(crate) generation: u64, + pub(crate) kdc_expires_at: Option, } #[derive(Debug, Clone)] @@ -49,18 +51,15 @@ struct CredentialsEntry { token: String, mapping: Option, expires_at: time::OffsetDateTime, -} - -#[derive(Debug, Default)] -struct CredentialsState { - entries: HashMap, - consumed: HashMap, + /// `Some` for `provision-credentials`: fail closed until this JWT acceptance deadline. + required_until: Option, + generation: u64, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum MappingStatus { Available, - Consumed, + RequiredMissing, Absent, } @@ -81,7 +80,7 @@ struct ConnectionOptionsEntry { /// operations and may arrive, expire, or be replaced independently. #[derive(Debug, Clone)] pub struct ProvisioningStore { - credentials: Arc>, + credentials: Arc>>, connection_options: Arc>>, } @@ -94,15 +93,18 @@ impl Default for ProvisioningStore { impl ProvisioningStore { pub fn new() -> Self { Self { - credentials: Arc::new(Mutex::new(CredentialsState::default())), + credentials: Arc::new(Mutex::new(HashMap::new())), connection_options: Arc::new(Mutex::new(HashMap::new())), } } /// Insert or replace the credentials half (token-only or with a mapping). /// - /// Same contract as master: `provision-token` passes `mapping = None`; - /// `provision-credentials` passes `Some(mapping)`. + /// `provision-token` passes `mapping = None`; `provision-credentials` passes `Some(mapping)`. + /// + /// For mapped rows, `time_to_live` is a staging wait for first checkout, capped to the token + /// acceptance deadline. The first successful [`Self::get_mapping`] then keeps the mapping until + /// that deadline. pub(crate) fn insert_credentials( &self, token: String, @@ -112,21 +114,48 @@ impl ProvisioningStore { let jti = crate::token::extract_jti(&token) .context("failed to extract token ID") .map_err(InsertError::InvalidToken)?; + let now = time::OffsetDateTime::now_utc(); + let staging_expires = now + time_to_live; + let required_until = if mapping.is_some() { + let exp = crate::token::extract_exp(&token) + .context("failed to extract token expiration") + .map_err(InsertError::InvalidToken)?; + Some(crate::token::token_acceptance_deadline(exp)) + } else { + None + }; + let expires_at = required_until.map_or(staging_expires, |deadline| staging_expires.min(deadline)); let mapping = mapping .map(CleartextAppCredentialMapping::encrypt) .transpose() .context("encrypt provisioned credentials") .map_err(InsertError::CredentialEncryption)?; - let entry = CredentialsEntry { - token, - mapping, - expires_at: time::OffsetDateTime::now_utc() + time_to_live, - }; - let mut credentials = self.credentials.lock(); - credentials.consumed.remove(&jti); - Ok(credentials.entries.insert(jti, entry).is_some()) + let generation = credentials + .get(&jti) + .map_or(1, |entry| entry.generation.wrapping_add(1)); + let replaced = credentials + .insert( + jti, + CredentialsEntry { + token, + mapping, + expires_at, + required_until, + generation, + }, + ) + .is_some(); + + if let Some(deadline) = required_until + && let Some(options) = self.connection_options.lock().get_mut(&jti) + && options.expires_at > deadline + { + options.expires_at = deadline; + } + + Ok(replaced) } /// Insert or replace the connection-options half. Returns whether a prior entry was replaced. @@ -136,9 +165,15 @@ impl ProvisioningStore { connection_options: TargetConnectionOptions, time_to_live: time::Duration, ) -> bool { + let now = time::OffsetDateTime::now_utc(); + let mut expires_at = now + time_to_live; + if let Some(deadline) = self.credentials.lock().get(&jti).and_then(|entry| entry.required_until) { + expires_at = expires_at.min(deadline); + } + let entry = ConnectionOptionsEntry { connection_options, - expires_at: time::OffsetDateTime::now_utc() + time_to_live, + expires_at, }; self.connection_options.lock().insert(jti, entry).is_some() @@ -146,25 +181,24 @@ impl ProvisioningStore { /// State of the credential-injection mapping for `jti`. /// - /// Does not consume the entry. - /// A consumed tombstone remains until the original provisioning expiry. - /// This makes reconnects fail explicitly instead of silently falling back to non-injected forwarding. + /// `RequiredMissing` means injection was provisioned but the mapping is gone or expired while + /// the token could still be accepted. Callers must fail closed instead of ordinary forwarding. pub(crate) fn mapping_status(&self, jti: Uuid) -> MappingStatus { let now = time::OffsetDateTime::now_utc(); - let mut credentials = self.credentials.lock(); + let credentials = self.credentials.lock(); - if credentials - .consumed - .get(&jti) - .is_some_and(|expires_at| now < *expires_at) - { - return MappingStatus::Consumed; - } - credentials.consumed.remove(&jti); - - match credentials.entries.get(&jti) { - Some(entry) if now < entry.expires_at && entry.mapping.is_some() => MappingStatus::Available, - _ => MappingStatus::Absent, + let Some(entry) = credentials.get(&jti) else { + return MappingStatus::Absent; + }; + let Some(deadline) = entry.required_until else { + return MappingStatus::Absent; + }; + if now >= deadline { + MappingStatus::Absent + } else if entry.mapping.is_some() && now < entry.expires_at { + MappingStatus::Available + } else { + MappingStatus::RequiredMissing } } @@ -173,17 +207,14 @@ impl ProvisioningStore { pub(crate) fn take(&self, jti: Uuid) -> Option { let now = time::OffsetDateTime::now_utc(); - let (token, mapping) = { + let (token, mapping, generation, kdc_expires_at) = { let mut credentials = self.credentials.lock(); - let entry = credentials.entries.remove(&jti)?; + let entry = credentials.remove(&jti)?; if now >= entry.expires_at { warn!(%jti, "Provisioned credentials expired before the connection arrived"); return None; } - if entry.mapping.is_some() { - credentials.consumed.insert(jti, entry.expires_at); - } - (entry.token, entry.mapping) + (entry.token, entry.mapping, entry.generation, entry.required_until) }; let connection_options = { @@ -202,52 +233,55 @@ impl ProvisioningStore { token, mapping, connection_options, + generation, + kdc_expires_at, }) } - /// Atomically validate and consume an injection mapping (one-shot checkout). + /// Clone injection material for this `jti`. /// - /// The mapping is not restored after a failed TLS/CredSSP attempt. - /// `time_to_live` is how long it may wait for first checkout, not a retry budget. - /// A consumed tombstone makes subsequent attempts fail explicitly until expiry or re-provisioning. - pub(crate) fn take_mapping(&self, jti: Uuid, token: &str) -> anyhow::Result { + /// The first successful lookup extends retention to the token acceptance deadline so reconnects + /// authorized by `jet_reuse` can still inject. + pub(crate) fn get_mapping(&self, jti: Uuid, token: &str) -> anyhow::Result { let now = time::OffsetDateTime::now_utc(); - let (token, mapping) = { + let (token, mapping, generation, required_until) = { let mut credentials = self.credentials.lock(); - - if credentials - .consumed - .get(&jti) - .is_some_and(|expires_at| now < *expires_at) - { - anyhow::bail!("credential-injection material for {jti} was already consumed; re-provision to retry"); - } - credentials.consumed.remove(&jti); - let entry = credentials - .entries - .get(&jti) + .get_mut(&jti) .context("provisioned credential-injection material is missing")?; - anyhow::ensure!( - now < entry.expires_at, - "provisioned credential-injection material expired" - ); - anyhow::ensure!(entry.mapping.is_some(), "provisioned entry has no credential mapping"); + anyhow::ensure!(token == entry.token, "token mismatch"); + let Some(deadline) = entry.required_until else { + anyhow::bail!("provisioned entry has no credential mapping"); + }; + anyhow::ensure!(entry.mapping.is_some(), "provisioned entry has no credential mapping"); - let entry = credentials - .entries - .remove(&jti) - .expect("entry exists while credential state lock is held"); - credentials.consumed.insert(jti, entry.expires_at); - (entry.token, entry.mapping) + if now >= deadline || now >= entry.expires_at { + anyhow::bail!("credential-injection material for {jti} is missing or expired; re-provision to retry"); + } + + entry.expires_at = deadline; + + ( + entry.token.clone(), + entry.mapping.clone(), + entry.generation, + entry.required_until, + ) }; let connection_options = { let mut entries = self.connection_options.lock(); - match entries.remove(&jti) { - Some(entry) if now < entry.expires_at => Some(entry.connection_options), + match entries.get_mut(&jti) { + Some(entry) if now < entry.expires_at => { + if let Some(deadline) = required_until + && entry.expires_at < deadline + { + entry.expires_at = deadline; + } + Some(entry.connection_options.clone()) + } Some(_) => { warn!(%jti, "Provisioned connection options expired before the connection arrived"); None @@ -260,8 +294,15 @@ impl ProvisioningStore { token, mapping, connection_options, + generation, + kdc_expires_at: required_until, }) } + + #[cfg(test)] + pub(crate) fn credentials_expires_at(&self, jti: Uuid) -> Option { + self.credentials.lock().get(&jti).map(|entry| entry.expires_at) + } } pub struct CleanupTask { @@ -298,8 +339,13 @@ async fn cleanup_task(handle: ProvisioningStore, mut shutdown_signal: ShutdownSi let now = time::OffsetDateTime::now_utc(); let mut credentials = handle.credentials.lock(); - credentials.entries.retain(|_, entry| now < entry.expires_at); - credentials.consumed.retain(|_, expires_at| now < *expires_at); + for entry in credentials.values_mut() { + if now >= entry.expires_at { + entry.mapping = None; + } + } + credentials + .retain(|_, entry| now < entry.expires_at || entry.required_until.is_some_and(|deadline| now < deadline)); drop(credentials); handle .connection_options @@ -331,14 +377,15 @@ mod tests { } } - fn association_token(jti: Uuid) -> String { + fn association_token_with_exp(jti: Uuid, exp: i64) -> String { use base64::Engine as _; let engine = base64::engine::general_purpose::URL_SAFE_NO_PAD; let header = engine.encode(r#"{"alg":"RS256"}"#); let payload = engine.encode( serde_json::to_vec(&serde_json::json!({ "jti": jti, - "dst_hst": "target.example:3389" + "dst_hst": "target.example:3389", + "exp": exp })) .expect("payload serializes"), ); @@ -346,6 +393,10 @@ mod tests { format!("{header}.{payload}.{signature}") } + fn association_token(jti: Uuid) -> String { + association_token_with_exp(jti, time::OffsetDateTime::now_utc().unix_timestamp() + 3600) + } + fn options() -> TargetConnectionOptions { serde_json::from_value(serde_json::json!({ "krb_kdc": "tcp://dc.example:88" })).expect("options") } @@ -421,7 +472,7 @@ mod tests { } #[test] - fn consumed_mapping_is_explicit_until_reprovisioned() { + fn get_mapping_is_reusable_until_reprovisioned() { let store = ProvisioningStore::new(); let jti = Uuid::new_v4(); let token = association_token(jti); @@ -430,19 +481,19 @@ mod tests { .expect("insert"); assert_eq!(store.mapping_status(jti), MappingStatus::Available); - store.take_mapping(jti, &token).expect("first checkout"); - assert_eq!(store.mapping_status(jti), MappingStatus::Consumed); - let error = store.take_mapping(jti, &token).expect_err("second checkout fails"); - assert!(format!("{error:#}").contains("already consumed")); + store.get_mapping(jti, &token).expect("first checkout"); + assert_eq!(store.mapping_status(jti), MappingStatus::Available); + store.get_mapping(jti, &token).expect("second checkout"); store - .insert_credentials(token, Some(mapping()), time::Duration::minutes(5)) + .insert_credentials(token.clone(), Some(mapping()), time::Duration::minutes(5)) .expect("re-provision"); - assert_eq!(store.mapping_status(jti), MappingStatus::Available); + let first = store.get_mapping(jti, &token).expect("after replace"); + assert_eq!(first.generation, 2); } #[test] - fn token_mismatch_does_not_consume_mapping() { + fn token_mismatch_does_not_drop_mapping() { let store = ProvisioningStore::new(); let jti = Uuid::new_v4(); let token = association_token(jti); @@ -450,14 +501,14 @@ mod tests { .insert_credentials(token.clone(), Some(mapping()), time::Duration::minutes(5)) .expect("insert"); - let error = store.take_mapping(jti, "different token").expect_err("mismatch"); + let error = store.get_mapping(jti, "different token").expect_err("mismatch"); assert!(format!("{error:#}").contains("token mismatch")); assert_eq!(store.mapping_status(jti), MappingStatus::Available); - store.take_mapping(jti, &token).expect("valid checkout"); + store.get_mapping(jti, &token).expect("valid checkout"); } #[test] - fn concurrent_mapping_checkout_has_one_winner() { + fn concurrent_mapping_checkout_all_succeed() { let store = ProvisioningStore::new(); let jti = Uuid::new_v4(); let token = association_token(jti); @@ -473,7 +524,7 @@ mod tests { let barrier = Arc::clone(&barrier); std::thread::spawn(move || { barrier.wait(); - store.take_mapping(jti, &token) + store.get_mapping(jti, &token) }) }) .collect(); @@ -483,8 +534,77 @@ mod tests { .into_iter() .map(|handle| handle.join().expect("thread")) .collect(); - assert_eq!(results.iter().filter(|result| result.is_ok()).count(), 1); - let error = results.into_iter().find_map(Result::err).expect("one failure"); - assert!(format!("{error:#}").contains("already consumed")); + assert_eq!(results.iter().filter(|result| result.is_ok()).count(), 2); + } + + #[test] + fn staging_expiry_before_first_use_is_required_missing() { + let store = ProvisioningStore::new(); + let jti = Uuid::new_v4(); + let token = association_token(jti); + store + .insert_credentials(token.clone(), Some(mapping()), time::Duration::seconds(-1)) + .expect("insert"); + + assert_eq!(store.mapping_status(jti), MappingStatus::RequiredMissing); + let error = store.get_mapping(jti, &token).expect_err("expired staging"); + assert!(format!("{error:#}").contains("missing or expired")); + } + + #[test] + fn first_get_extends_expiry_to_token_deadline() { + let store = ProvisioningStore::new(); + let jti = Uuid::new_v4(); + let exp = time::OffsetDateTime::now_utc().unix_timestamp() + 3600; + let token = association_token_with_exp(jti, exp); + store + .insert_credentials(token.clone(), Some(mapping()), time::Duration::seconds(30)) + .expect("insert"); + + let before = store.credentials_expires_at(jti).expect("inserted"); + store.get_mapping(jti, &token).expect("activate"); + let after = store.credentials_expires_at(jti).expect("activated"); + assert!(after > before); + assert_eq!(after, crate::token::token_acceptance_deadline(exp)); + } + + #[test] + fn insert_caps_caller_ttl_to_token_acceptance_deadline() { + let store = ProvisioningStore::new(); + let jti = Uuid::new_v4(); + let exp = time::OffsetDateTime::now_utc().unix_timestamp(); + let token = association_token_with_exp(jti, exp); + store + .insert_credentials(token, Some(mapping()), time::Duration::hours(2)) + .expect("insert"); + + let expires_at = store.credentials_expires_at(jti).expect("inserted"); + let deadline = crate::token::token_acceptance_deadline(exp); + let delta = (expires_at - deadline).abs(); + assert!(delta <= time::Duration::seconds(1)); + } + + #[test] + fn mapped_insert_requires_exp() { + let store = ProvisioningStore::new(); + let jti = Uuid::new_v4(); + use base64::Engine as _; + let engine = base64::engine::general_purpose::URL_SAFE_NO_PAD; + let token = format!( + "{}.{}.{}", + engine.encode(r#"{"alg":"RS256"}"#), + engine.encode( + serde_json::to_vec(&serde_json::json!({ + "jti": jti, + "dst_hst": "target.example:3389" + })) + .expect("payload") + ), + engine.encode(b"signature") + ); + let error = store + .insert_credentials(token, Some(mapping()), time::Duration::minutes(5)) + .expect_err("missing exp"); + assert!(format!("{error:#}").contains("exp")); } } diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index e30d635cc..7cd4cce25 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -432,12 +432,10 @@ async fn handle_with_credential_injection( mut client_stream: impl AsyncRead + AsyncWrite + Unpin + Send, client_addr: SocketAddr, conf: Arc, - token_cache: &TokenCache, - jrl: &CurrentJrl, sessions: SessionMessageSender, subscriber_tx: SubscriberSender, - active_recordings: &ActiveRecordings, cleanpath_pdu: RDCleanPathPdu, + claims: AssociationTokenClaims, provisioning: &ProvisioningStore, synthetic_kdc_registry: &SyntheticKdcRegistry, agent_tunnel_handle: Option>, @@ -468,29 +466,11 @@ async fn handle_with_credential_injection( ) }; - let CleanPathAuth { claims } = authorize_cleanpath( - &cleanpath_pdu, - client_addr, - &conf, - token_cache, - jrl, - active_recordings, - &sessions, - ) - .await - .context("RDCleanPath authorization failed")?; - let token = cleanpath_pdu .proxy_auth .clone() .context("missing token in RDCleanPath PDU")?; - anyhow::ensure!( - provisioning.mapping_status(claims.jti) != MappingStatus::Consumed, - "credential-injection material for {} was already consumed; re-provision to retry", - claims.jti, - ); - // Connect before checkout so a target connect failure does not consume one-shot credentials. let ConnectedRdpServer { tls_stream: server_stream, server_addr, @@ -615,69 +595,74 @@ pub async fn handle( .await .context("couldn't read cleanpath PDU")?; - // Early credential detection: check if we should use RdpProxy instead. - let token = cleanpath_pdu - .proxy_auth - .as_deref() - .context("missing token in RDCleanPath PDU")?; + let auth = match authorize_cleanpath( + &cleanpath_pdu, + client_addr, + &conf, + token_cache, + jrl, + active_recordings, + &sessions, + ) + .await + { + Ok(auth) => auth, + Err(error) => { + let response = RDCleanPathPdu::from(&error); + send_clean_path_response(&mut client_stream, &response).await?; + return anyhow::Error::new(error) + .context("an error occurred when processing cleanpath PDU") + .pipe(Err)?; + } + }; - // If a credential mapping has been pushed, switch to proxy-based credential injection. - // Peek here without consuming. - // Checkout happens after authorization and target connection to protect the JTI from invalid requests and failures. - if let Some(jti) = crate::token::extract_jti(token).ok() + let mapping_status = provisioning.mapping_status(auth.claims.jti); + if is_vmconnect_request(&cleanpath_pdu) && matches!( - provisioning.mapping_status(jti), - MappingStatus::Available | MappingStatus::Consumed + mapping_status, + MappingStatus::Available | MappingStatus::RequiredMissing ) { - // VMConnect needs pre-X.224 CredSSP against the Hyper-V host cert on the client. - // Proxy CredSSP MITM is X.224-first and is not supported for this ordering. - if is_vmconnect_request(&cleanpath_pdu) { - let response = RDCleanPathPdu::new_http_error(400); + let response = RDCleanPathPdu::new_http_error(400); + send_clean_path_response(&mut client_stream, &response).await?; + anyhow::bail!("credential injection is not supported for VMConnect RDCleanPath"); + } + + match mapping_status { + MappingStatus::Available => { + debug!(jti = %auth.claims.jti, "Switching to RdpProxy for credential injection (WebSocket)"); + return handle_with_credential_injection( + client_stream, + client_addr, + conf, + sessions, + subscriber_tx, + cleanpath_pdu, + auth.claims, + provisioning, + synthetic_kdc_registry, + agent_tunnel_handle.clone(), + ) + .await; + } + MappingStatus::RequiredMissing => { + let error = CleanPathError::BadRequest(anyhow::anyhow!( + "credential-injection material for {} is missing or expired; re-provision to retry", + auth.claims.jti + )); + let response = RDCleanPathPdu::from(&error); send_clean_path_response(&mut client_stream, &response).await?; - anyhow::bail!("credential injection is not supported for VMConnect RDCleanPath"); + return anyhow::Error::new(error) + .context("an error occurred when processing cleanpath PDU") + .pipe(Err)?; } - - debug!(%jti, "Switching to RdpProxy for credential injection (WebSocket)"); - - return handle_with_credential_injection( - client_stream, - client_addr, - conf, - token_cache, - jrl, - sessions, - subscriber_tx, - active_recordings, - cleanpath_pdu, - provisioning, - synthetic_kdc_registry, - agent_tunnel_handle.clone(), - ) - .await; + MappingStatus::Absent => {} } trace!("Processing RDCleanPath"); - let (auth, connected) = match async { - let auth = authorize_cleanpath( - &cleanpath_pdu, - client_addr, - &conf, - token_cache, - jrl, - active_recordings, - &sessions, - ) - .await?; - - let connected = connect_rdp_server(&auth.claims, cleanpath_pdu, agent_tunnel_handle.as_ref()).await?; - - Ok::<_, CleanPathError>((auth, connected)) - } - .await - { - Ok(result) => result, + let connected = match connect_rdp_server(&auth.claims, cleanpath_pdu, agent_tunnel_handle.as_ref()).await { + Ok(connected) => connected, Err(error) => { let response = RDCleanPathPdu::from(&error); send_clean_path_response(&mut client_stream, &response).await?; diff --git a/devolutions-gateway/src/rdp_proxy/credssp.rs b/devolutions-gateway/src/rdp_proxy/credssp.rs index a49a0b155..5a7053664 100644 --- a/devolutions-gateway/src/rdp_proxy/credssp.rs +++ b/devolutions-gateway/src/rdp_proxy/credssp.rs @@ -492,7 +492,8 @@ mod tests { let payload = engine.encode( serde_json::to_vec(&serde_json::json!({ "jti": jti, - "dst_hst": "target.example:3389" + "dst_hst": "target.example:3389", + "exp": time::OffsetDateTime::now_utc().unix_timestamp() + 3600 })) .expect("payload serializes"), ); @@ -524,7 +525,7 @@ mod tests { let entry = store.take(jti).expect("entry"); CredentialInjection::from_provisioned(jti, entry, true) .expect("prepared") - .register_if_kerberos(&SyntheticKdcRegistry::new()) + .register_if_kerberos(&SyntheticKdcRegistry::new(), 1) } #[test] @@ -568,7 +569,7 @@ mod tests { let entry = store.take(jti).expect("entry"); let injection = CredentialInjection::from_provisioned(jti, entry, true) .expect("ntlm prepared") - .register_if_kerberos(&SyntheticKdcRegistry::new()); + .register_if_kerberos(&SyntheticKdcRegistry::new(), 1); let config = client_kerberos_config(&injection).expect("ntlm ok"); assert!(config.is_none()); diff --git a/devolutions-gateway/src/token.rs b/devolutions-gateway/src/token.rs index a3f6e0e7f..18e6e08cb 100644 --- a/devolutions-gateway/src/token.rs +++ b/devolutions-gateway/src/token.rs @@ -1279,6 +1279,23 @@ pub fn extract_jti(token: &str) -> anyhow::Result { extract_uuid(token, "jti").context("extract jti") } +/// Extract the JWT `exp` claim without verifying the signature. +pub fn extract_exp(token: &str) -> anyhow::Result { + let payload = extract_payload(token)?; + let exp = payload.get("exp").context("exp is missing from the token")?; + exp.as_i64() + .or_else(|| exp.as_u64().and_then(|value| i64::try_from(value).ok())) + .context("exp is malformed") +} + +/// Latest instant at which Gateway will still accept a token with this `exp`. +/// +/// Includes the hardcoded JWT clock-skew leeway. +pub(crate) fn token_acceptance_deadline(exp: i64) -> time::OffsetDateTime { + let timestamp = exp.saturating_add(i64::from(LEEWAY_SECS)); + time::OffsetDateTime::from_unix_timestamp(timestamp).unwrap_or(time::OffsetDateTime::UNIX_EPOCH) +} + pub fn extract_session_id(token: &str) -> anyhow::Result { extract_uuid(token, "jet_aid").context("extract jet_aid") } From c2e42c67aaca22670d60e851ef4f6b0a282a634f Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Thu, 20 Aug 2026 15:33:32 -0400 Subject: [PATCH 12/12] test(dgw): require exp on provision-credentials fixtures Mapped insert now caps retention to the association token acceptance deadline, so unsigned preflight fixtures without exp fail as invalid-parameters. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- devolutions-gateway/tests/preflight.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/devolutions-gateway/tests/preflight.rs b/devolutions-gateway/tests/preflight.rs index 1d0de8866..1cbcf8480 100644 --- a/devolutions-gateway/tests/preflight.rs +++ b/devolutions-gateway/tests/preflight.rs @@ -46,6 +46,10 @@ fn preflight_request(operations: serde_json::Value) -> anyhow::Result i64 { + time::OffsetDateTime::now_utc().unix_timestamp() + 3600 +} + fn unsigned_jws(payload: serde_json::Value) -> anyhow::Result { let engine = base64::engine::general_purpose::URL_SAFE_NO_PAD; let header = engine.encode(r#"{"alg":"RS256"}"#); @@ -84,7 +88,8 @@ async fn test_provision_credentials_success() -> anyhow::Result<()> { let jti = Uuid::new_v4(); let token = unsigned_jws(json!({ "jti": jti, - "dst_hst": "target.example:3389" + "dst_hst": "target.example:3389", + "exp": token_exp() }))?; let op_id = Uuid::new_v4(); @@ -124,7 +129,8 @@ async fn test_provision_credentials_success_when_unstable_disabled() -> anyhow:: let jti = Uuid::new_v4(); let token = unsigned_jws(json!({ "jti": jti, - "dst_hst": "target.example:3389" + "dst_hst": "target.example:3389", + "exp": token_exp() }))?; let op_id = Uuid::new_v4(); @@ -318,7 +324,8 @@ async fn test_provision_credentials_and_connection_options_fold() -> anyhow::Res let jti = Uuid::new_v4(); let token = unsigned_jws(json!({ "jti": jti, - "dst_hst": "target.example:3389" + "dst_hst": "target.example:3389", + "exp": token_exp() }))?; let ops = json!([