From 4b91ac4c73bace5cd1b4087dc0ce4d8a94351275 Mon Sep 17 00:00:00 2001 From: irving ou Date: Fri, 15 May 2026 15:51:48 -0400 Subject: [PATCH 1/3] fix(DGW-378): support Kerberos credential injection --- Cargo.lock | 1 + devolutions-gateway/Cargo.toml | 1 + devolutions-gateway/src/api/kdc_proxy.rs | 235 +++-- devolutions-gateway/src/api/preflight.rs | 90 +- devolutions-gateway/src/api/rdp.rs | 4 + devolutions-gateway/src/api/webapp.rs | 1 + devolutions-gateway/src/credential/mod.rs | 147 +-- devolutions-gateway/src/credential/tests.rs | 131 +++ .../src/credential_injection_kdc.rs | 835 ++++++++++++++++++ devolutions-gateway/src/extract.rs | 47 +- devolutions-gateway/src/generic_client.rs | 65 +- devolutions-gateway/src/lib.rs | 4 + devolutions-gateway/src/listener.rs | 1 + devolutions-gateway/src/ngrok.rs | 1 + devolutions-gateway/src/rd_clean_path.rs | 87 +- devolutions-gateway/src/rdp_proxy.rs | 168 ++-- devolutions-gateway/src/service.rs | 7 + devolutions-gateway/src/token.rs | 100 ++- devolutions-gateway/tests/preflight.rs | 113 ++- tools/tokengen/src/lib.rs | 10 +- tools/tokengen/src/main.rs | 12 +- tools/tokengen/src/server/server_impl.rs | 2 + .../src/AssociationClaims.cs | 2 +- .../src/KdcClaims.cs | 5 +- .../src/ProvisionCredentialsRequest.cs | 57 ++ 25 files changed, 1792 insertions(+), 334 deletions(-) create mode 100644 devolutions-gateway/src/credential/tests.rs create mode 100644 devolutions-gateway/src/credential_injection_kdc.rs create mode 100644 utils/dotnet/Devolutions.Gateway.Utils/src/ProvisionCredentialsRequest.cs diff --git a/Cargo.lock b/Cargo.lock index d8a02aa66..7efc390e9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1618,6 +1618,7 @@ dependencies = [ "axum 0.8.9", "axum-extra", "backoff", + "base64 0.22.1", "bitflags 2.11.1", "bytes 1.11.1", "cadeau", diff --git a/devolutions-gateway/Cargo.toml b/devolutions-gateway/Cargo.toml index a4465f93d..0149d19a6 100644 --- a/devolutions-gateway/Cargo.toml +++ b/devolutions-gateway/Cargo.toml @@ -140,6 +140,7 @@ windows-sys = { version = "0.61", features = ["Win32_Storage_FileSystem", "Win32 embed-resource = "3.0" [dev-dependencies] +base64 = "0.22" tokio-test = "0.4" proptest = "1.7" tempfile = "3" diff --git a/devolutions-gateway/src/api/kdc_proxy.rs b/devolutions-gateway/src/api/kdc_proxy.rs index 8847009d0..eec557222 100644 --- a/devolutions-gateway/src/api/kdc_proxy.rs +++ b/devolutions-gateway/src/api/kdc_proxy.rs @@ -1,19 +1,22 @@ use std::io; -use std::net::SocketAddr; use axum::Router; -use axum::extract::{self, ConnectInfo, State}; +use axum::extract::State; use axum::http::StatusCode; use axum::routing::post; -use kdc::handle_kdc_proxy_message; use picky_krb::messages::KdcProxyMessage; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::{TcpStream, UdpSocket}; use crate::DgwState; +use crate::credential_injection_kdc::{ + CredentialInjectionKdc, CredentialInjectionKdcInterception, CredentialInjectionKdcRequest, + CredentialInjectionKdcResolveError, kdc_proxy_message_realm, +}; +use crate::extract::KdcToken; use crate::http::{HttpError, HttpErrorBuilder}; use crate::target_addr::TargetAddr; -use crate::token::AccessTokenClaims; +use crate::token::KdcTokenClaims; pub fn make_router(state: DgwState) -> Router { Router::new().route("/{token}", post(kdc_proxy)).with_state(state) @@ -22,97 +25,160 @@ pub fn make_router(state: DgwState) -> Router { async fn kdc_proxy( State(DgwState { conf_handle, - token_cache, - jrl, - recordings, + credential_store, + kerberos_session_store, .. }): State, - extract::Path(token): extract::Path, - ConnectInfo(source_addr): ConnectInfo, + KdcToken(KdcTokenClaims { + krb_realm, + krb_kdc, + jet_cred_id, + }): KdcToken, body: axum::body::Bytes, ) -> Result, HttpError> { let conf = conf_handle.get_conf(); - let claims = crate::middleware::auth::authenticate( - source_addr, - &token, - &conf, - &token_cache, - &jrl, - &recordings.active_recordings, - None, - ) - .map_err(HttpError::unauthorized().err())?; - - let AccessTokenClaims::Kdc(claims) = claims else { - return Err(HttpError::forbidden().msg("token not allowed (expected KDC token)")); - }; - let kdc_proxy_message = KdcProxyMessage::from_raw(&body).map_err(HttpError::bad_request().err())?; trace!(?kdc_proxy_message, "Received KDC message"); - debug!( ?kdc_proxy_message.target_domain, ?kdc_proxy_message.dclocator_hint, "KDC message", ); - let realm = if let Some(realm) = &kdc_proxy_message.target_domain.0 { - realm.0.to_string() - } else { - return Err(HttpError::bad_request().msg("realm is missing from KDC request")); - }; - - debug!("Request is for realm (target_domain): {realm}"); + enforce_credential_injection_enabled(jet_cred_id, conf.debug.enable_unstable)?; - if !claims.krb_realm.eq_ignore_ascii_case(&realm) { - if conf.debug.disable_token_validation { - warn!( - token_realm = %claims.krb_realm, - request_realm = %realm, - "**DEBUG OPTION** Allowed a KDC request towards a KDC whose Kerberos realm differs from what's inside the KDC token" + match CredentialInjectionKdc::resolve(jet_cred_id, &credential_store, &kerberos_session_store) + .map_err(credential_injection_resolve_error)? + { + Some(kdc) => { + debug!( + jti = %kdc.jti(), + "Proxy-based credential injection with Kerberos. Processing KdcProxy message internally" ); - } else { - let error_message = format!("expected: {}, got: {}", claims.krb_realm, realm); - return Err(HttpError::bad_request() - .with_msg("requested domain is not allowed") - .err()(error_message)); + match kdc + .handle_kdc_proxy_request(CredentialInjectionKdcRequest::from_token( + kdc_proxy_message, + &krb_realm, + conf.debug.disable_token_validation, + )) + .map_err(HttpError::internal().err())? + { + CredentialInjectionKdcInterception::Intercepted(reply) => Ok(reply), + CredentialInjectionKdcInterception::NotInjectionRealm(mismatch) => { + Err(HttpError::bad_request() + .with_msg("requested domain is not allowed") + .err()(mismatch)) + } + CredentialInjectionKdcInterception::NotInjectionRequest => { + Err(HttpError::internal().msg("credential-injection KDC did not handle the KDC proxy request")) + } + } + } + None => { + let envelope_realm = kdc_proxy_message_realm(&kdc_proxy_message); + forward_to_real_kdc( + kdc_proxy_message, + envelope_realm, + &krb_realm, + &krb_kdc, + conf.debug.override_kdc.as_ref(), + conf.debug.disable_token_validation, + ) + .await } } +} - let gateway_id = conf - .id - .ok_or_else(|| HttpError::internal().build("Gateway ID is missing"))?; - if let Some(krb_config) = &conf.debug.kerberos - && realm.eq_ignore_ascii_case(&krb_config.kerberos_server.realm(gateway_id)) - && conf.debug.enable_unstable - { - debug!("Proxy-based credential injection with Kerberos. Processing KdcProxy message internally..."); +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`. +// +// The forward path requires the envelope realm to be set: there is no fallback since this is +// not a credential-injection session. After resolving, validates the realm against the +// token's `krb_realm` claim before forwarding anything. +async fn forward_to_real_kdc( + kdc_proxy_message: KdcProxyMessage, + envelope_realm: Option, + token_realm: &str, + token_kdc_addr: &TargetAddr, + override_kdc: Option<&TargetAddr>, + bypass_realm_check: bool, +) -> Result, HttpError> { + let realm = envelope_realm.ok_or_else(|| HttpError::bad_request().msg("realm is missing from KDC request"))?; + debug!(resolved_realm = %realm, "Forward-to-real-KDC realm resolved"); + enforce_realm_token_match(token_realm, &realm, bypass_realm_check)?; + + let kdc_addr = match override_kdc { + Some(override_addr) => { + warn!(%override_addr, "**DEBUG OPTION** KDC address has been overridden"); + override_addr + } + None => token_kdc_addr, + }; + + let kdc_reply_bytes = send_krb_message(kdc_addr, &kdc_proxy_message.kerb_message.0.0).await?; + + let reply = KdcProxyMessage::from_raw_kerb_message(&kdc_reply_bytes) + .map_err(HttpError::internal().with_msg("couldn't create KDC proxy reply").err())?; + + trace!(?reply, "Sending back KDC reply"); - let config = krb_config.kerberos_server.clone().into_kdc_kerberos_config(gateway_id); - let kdc_reply_message = handle_kdc_proxy_message(kdc_proxy_message, &config, &conf.hostname) - .map_err(HttpError::internal().err())?; + reply.to_vec().map_err(HttpError::internal().err()) +} - return kdc_reply_message.to_vec().map_err(HttpError::internal().err()); +fn enforce_credential_injection_enabled( + jet_cred_id: Option, + enable_unstable: bool, +) -> Result<(), HttpError> { + if enable_unstable { + return Ok(()); } - let kdc_addr = if let Some(kdc_addr) = &conf.debug.override_kdc { - warn!("**DEBUG OPTION** KDC address has been overridden with {kdc_addr}"); - kdc_addr - } else { - &claims.krb_kdc + let Some(jet_cred_id) = jet_cred_id else { + return Ok(()); }; - let kdc_reply_message = send_krb_message(kdc_addr, &kdc_proxy_message.kerb_message.0.0).await?; + warn!( + %jet_cred_id, + "Credential-injection KDC token rejected because unstable Kerberos injection is disabled" + ); + Err(HttpError::bad_request().msg("credential-injection KDC proxy is not enabled")) +} - let kdc_reply_message = KdcProxyMessage::from_raw_kerb_message(&kdc_reply_message) - .map_err(HttpError::internal().with_msg("couldn't create KDC proxy reply").err())?; +/// Refuses to forward a KDC request whose realm disagrees with the realm the token was issued for. +/// +/// `bypass=true` (only when `__debug__.disable_token_validation` is on) downgrades the mismatch +/// to a warning. Production never opts into this. +fn enforce_realm_token_match(token_realm: &str, request_realm: &str, bypass: bool) -> Result<(), HttpError> { + if token_realm.eq_ignore_ascii_case(request_realm) { + return Ok(()); + } - trace!(?kdc_reply_message, "Sending back KDC reply"); + if bypass { + warn!( + %token_realm, + %request_realm, + "**DEBUG OPTION** Allowed a KDC request towards a KDC whose Kerberos realm differs from what's inside the KDC token" + ); + return Ok(()); + } - kdc_reply_message.to_vec().map_err(HttpError::internal().err()) + Err(HttpError::bad_request() + .with_msg("requested domain is not allowed") + .err()(format!("expected: {token_realm}, got: {request_realm}"))) } async fn read_kdc_reply_message(connection: &mut TcpStream) -> io::Result> { @@ -212,3 +278,42 @@ pub async fn send_krb_message(kdc_addr: &TargetAddr, message: &[u8]) -> Result, _scope: PreflightScope, @@ -223,12 +225,21 @@ pub(super) async fn post_preflight( let conf = conf_handle.get_conf(); let sessions = sessions.clone(); let credential_store = credential_store.clone(); + let kerberos_session_store = kerberos_session_store.clone(); async move { let operation_id = operation.id; trace!(%operation.id, "Process preflight operation"); - if let Err(error) = handle_operation(operation, &outputs, &conf, &sessions, &credential_store).await + if let Err(error) = handle_operation( + operation, + &outputs, + &conf, + &sessions, + &credential_store, + &kerberos_session_store, + ) + .await { outputs.push(PreflightOutput { operation_id, @@ -257,6 +268,7 @@ async fn handle_operation( conf: &Conf, sessions: &SessionMessageSender, credential_store: &CredentialStoreHandle, + kerberos_session_store: &CredentialInjectionKdcSessionStoreHandle, ) -> Result<(), PreflightError> { match operation.kind.as_str() { OP_GET_VERSION => outputs.push(PreflightOutput { @@ -337,17 +349,77 @@ async fn handle_operation( }); } + // Token signature isn't validated at the preflight scope (DVLS signs and pushes; + // gateway only re-uses what DVLS already verified). We do parse JTI / dst_hst here + // because the credential store no longer touches JWT shape — see the contract on + // `CredentialStoreHandle::insert`. + let jti = crate::token::extract_jti(&token).map_err(|error| { + PreflightError::new(PreflightAlertStatus::InvalidParams, format!("invalid token: {error:#}")) + })?; + + // For `provision-credentials`: resolve `target_hostname` from `dst_hst` *before* + // calling either store, so an invalid target is reported as `invalid-parameters` + // at preflight rather than mid-CredSSP. Also derive per-session Kerberos material + // from the proxy username; both stores receive their entries paired and keyed by + // the same JTI. + let injection_payload = if let Some(mapping) = mapping { + const DEFAULT_DST_PORT: u16 = 3389; + let raw_dst_hst = crate::token::extract_dst_hst(&token) + .map_err(|error| { + PreflightError::new( + PreflightAlertStatus::InvalidParams, + format!("read dst_hst from association token: {error:#}"), + ) + })? + .ok_or_else(|| { + PreflightError::new( + PreflightAlertStatus::InvalidParams, + "association token has no dst_hst, required for credential injection", + ) + })?; + + let dst_alt = crate::token::extract_dst_alt(&token).map_err(|error| { + PreflightError::new( + PreflightAlertStatus::InvalidParams, + format!("read dst_alt from association token: {error:#}"), + ) + })?; + if !dst_alt.is_empty() { + return Err(PreflightError::new( + PreflightAlertStatus::InvalidParams, + "association token dst_alt is not supported for credential injection", + )); + } + + let target_hostname = crate::target_addr::TargetAddr::parse(&raw_dst_hst, DEFAULT_DST_PORT) + .map_err(|error| { + PreflightError::new( + PreflightAlertStatus::InvalidParams, + format!("parse dst_hst as target address: {error:#}"), + ) + })? + .host() + .to_owned(); + + let kdc_session = crate::credential_injection_kdc::derive_credential_injection_kdc_session( + mapping.proxy_username(), + jti, + ); + kerberos_session_store.insert(kdc_session, time_to_live); + + Some((mapping, target_hostname)) + } else { + None + }; + let previous_entry = credential_store - .insert(token, mapping, time_to_live) + .insert(jti, token, injection_payload, time_to_live) .inspect_err(|error| warn!(%operation.id, error = format!("{error:#}"), "Failed to insert credentials")) - .map_err(|e| match e { - InsertError::InvalidToken(_) => { - PreflightError::new(PreflightAlertStatus::InvalidParams, format!("{e:#}")) - } - InsertError::Internal(_) => PreflightError::new( + .map_err(|_| { + PreflightError::new( PreflightAlertStatus::InternalServerError, "an internal error occurred".to_owned(), - ), + ) })?; if previous_entry.is_some() { diff --git a/devolutions-gateway/src/api/rdp.rs b/devolutions-gateway/src/api/rdp.rs index 6129a776c..b684a28d5 100644 --- a/devolutions-gateway/src/api/rdp.rs +++ b/devolutions-gateway/src/api/rdp.rs @@ -26,6 +26,7 @@ pub async fn handler( recordings, shutdown_signal, credential_store, + kerberos_session_store, agent_tunnel_handle, .. }): State, @@ -47,6 +48,7 @@ pub async fn handler( recordings.active_recordings, source_addr, credential_store, + kerberos_session_store, agent_tunnel_handle, ) .instrument(span) @@ -67,6 +69,7 @@ async fn handle_socket( active_recordings: Arc, source_addr: SocketAddr, credential_store: crate::credential::CredentialStoreHandle, + kerberos_session_store: crate::credential_injection_kdc::CredentialInjectionKdcSessionStoreHandle, agent_tunnel_handle: Option>, ) { let (stream, close_handle) = crate::ws::handle( @@ -85,6 +88,7 @@ async fn handle_socket( subscriber_tx, &active_recordings, &credential_store, + &kerberos_session_store, agent_tunnel_handle, ) .await; diff --git a/devolutions-gateway/src/api/webapp.rs b/devolutions-gateway/src/api/webapp.rs index f266f4207..0eacadd39 100644 --- a/devolutions-gateway/src/api/webapp.rs +++ b/devolutions-gateway/src/api/webapp.rs @@ -388,6 +388,7 @@ pub(crate) async fn sign_session_token( KdcTokenClaims { krb_realm: krb_realm.into(), krb_kdc: krb_kdc.clone(), + jet_cred_id: None, } .pipe(serde_json::to_value) .map(|mut claims| { diff --git a/devolutions-gateway/src/credential/mod.rs b/devolutions-gateway/src/credential/mod.rs index 166be9412..fea6829d6 100644 --- a/devolutions-gateway/src/credential/mod.rs +++ b/devolutions-gateway/src/credential/mod.rs @@ -4,40 +4,16 @@ mod crypto; pub use crypto::EncryptedPassword; use std::collections::HashMap; -use std::fmt; use std::sync::Arc; -use anyhow::Context; use async_trait::async_trait; use devolutions_gateway_task::{ShutdownSignal, Task}; use parking_lot::Mutex; -use secrecy::ExposeSecret as _; +use secrecy::{ExposeSecret as _, SecretString}; use uuid::Uuid; use self::crypto::MASTER_KEY; -/// Error returned by [`CredentialStoreHandle::insert`]. -#[derive(Debug)] -pub enum InsertError { - /// The provided token is invalid (e.g., missing or malformed JTI). - /// - /// This is a client-side error: the caller supplied bad input. - InvalidToken(anyhow::Error), - /// An internal error occurred (e.g., encryption failure). - Internal(anyhow::Error), -} - -impl fmt::Display for InsertError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::InvalidToken(e) => e.fmt(f), - Self::Internal(e) => e.fmt(f), - } - } -} - -impl std::error::Error for InsertError {} - /// Credential at the application protocol level #[derive(Debug, Clone)] pub enum AppCredential { @@ -48,10 +24,16 @@ pub enum AppCredential { } impl AppCredential { + pub(crate) fn username(&self) -> &str { + match self { + AppCredential::UsernamePassword { username, password: _ } => username, + } + } + /// Decrypt the password using the global master key. /// /// Returns the username and a short-lived decrypted password that zeroizes on drop. - pub fn decrypt_password(&self) -> anyhow::Result<(String, secrecy::SecretString)> { + pub fn decrypt_password(&self) -> anyhow::Result<(String, SecretString)> { match self { AppCredential::UsernamePassword { username, password } => { let decrypted = MASTER_KEY.lock().decrypt(password)?; @@ -74,12 +56,9 @@ pub struct AppCredentialMapping { /// This type is never stored directly — hand it to [`CredentialStoreHandle::insert`]. #[derive(Debug, Deserialize)] #[serde(tag = "kind")] -pub enum CleartextAppCredential { +pub(crate) enum CleartextAppCredential { #[serde(rename = "username-password")] - UsernamePassword { - username: String, - password: secrecy::SecretString, - }, + UsernamePassword { username: String, password: SecretString }, } impl CleartextAppCredential { @@ -100,20 +79,30 @@ impl CleartextAppCredential { /// /// Passwords are encrypted on write. Hand this directly to [`CredentialStoreHandle::insert`]. #[derive(Debug, Deserialize)] -pub struct CleartextAppCredentialMapping { +pub(crate) struct CleartextAppCredentialMapping { #[serde(rename = "proxy_credential")] - pub proxy: CleartextAppCredential, + pub(crate) proxy: CleartextAppCredential, #[serde(rename = "target_credential")] - pub target: CleartextAppCredential, + pub(crate) target: CleartextAppCredential, } impl CleartextAppCredentialMapping { - fn encrypt(self) -> anyhow::Result { + pub(crate) fn encrypt(self) -> anyhow::Result { Ok(AppCredentialMapping { proxy: self.proxy.encrypt()?, target: self.target.encrypt()?, }) } + + /// Expose the proxy-side username without dropping ownership of the rest of the mapping. + /// + /// Used by the preflight layer to derive per-session Kerberos material before the mapping + /// is moved into the credential store. The password is not exposed. + pub(crate) fn proxy_username(&self) -> &str { + match &self.proxy { + CleartextAppCredential::UsernamePassword { username, password: _ } => username, + } + } } #[derive(Debug, Clone)] @@ -130,21 +119,32 @@ impl CredentialStoreHandle { Self(Arc::new(Mutex::new(CredentialStore::new()))) } - pub fn insert( + /// Insert a credential entry for the association token whose JTI is `jti`. + /// + /// The caller is responsible for extracting `jti` from a token whose signature has already + /// been validated upstream. `injection` carries the optional `provision-credentials` payload: + /// the cleartext mapping and the parsed RDP target hostname. The store has no other + /// dependency on the token's JWT shape. + pub(crate) fn insert( &self, + jti: Uuid, token: String, - mapping: Option, + injection: Option<(CleartextAppCredentialMapping, String)>, time_to_live: time::Duration, - ) -> Result, InsertError> { - let mapping = mapping - .map(CleartextAppCredentialMapping::encrypt) - .transpose() - .map_err(InsertError::Internal)?; - self.0.lock().insert(token, mapping, time_to_live) + ) -> anyhow::Result> { + let injection = injection + .map(|(mapping, target_hostname)| -> anyhow::Result { + Ok(InjectionState { + mapping: mapping.encrypt()?, + target_hostname, + }) + }) + .transpose()?; + Ok(self.0.lock().insert(jti, token, injection, time_to_live)) } - pub fn get(&self, token_id: Uuid) -> Option { - self.0.lock().get(token_id) + pub(crate) fn get(&self, jti: Uuid) -> Option { + self.0.lock().get(jti) } } @@ -154,13 +154,31 @@ struct CredentialStore { } #[derive(Debug)] -pub struct CredentialEntry { - pub token: String, - pub mapping: Option, - pub expires_at: time::OffsetDateTime, +pub(crate) struct CredentialEntry { + /// The association token's JTI. Doubles as the credential-store key and as the value the + /// matching KDC token's `jet_cred_id` claim points back to. + pub(crate) jti: Uuid, + pub(crate) token: String, + pub(crate) expires_at: time::OffsetDateTime, + /// Credential-injection state for this entry, set when (and only when) the entry was + /// provisioned via `provision-credentials`. Plain `provision-token` entries leave this `None` + /// and are inert from the routing layer's perspective. Grouping the three correlated fields + /// into one option captures the "all set or all unset" invariant in the type system. + pub(crate) injection: Option, +} + +#[derive(Debug)] +pub(crate) struct InjectionState { + pub(crate) mapping: AppCredentialMapping, + /// Hostname of the target RDP server, parsed from the association token's `dst_hst` claim + /// at preflight. Fake-KDC validates client TGS-REQ sname against `TERMSRV/`; + /// without it the SPN check would silently fall back to Gateway's own hostname, so the + /// preflight handler rejects `provision-credentials` requests with missing/malformed `dst_hst` + /// or with alternate targets (`dst_alt`) until credential injection supports target failover. + pub(crate) target_hostname: String, } -pub type ArcCredentialEntry = Arc; +pub(crate) type ArcCredentialEntry = Arc; impl CredentialStore { fn new() -> Self { @@ -171,27 +189,29 @@ impl CredentialStore { fn insert( &mut self, + jti: Uuid, token: String, - mapping: Option, + injection: Option, time_to_live: time::Duration, - ) -> Result, InsertError> { - let jti = crate::token::extract_jti(&token) - .context("failed to extract token ID") - .map_err(InsertError::InvalidToken)?; - + ) -> Option { let entry = CredentialEntry { + jti, token, - mapping, expires_at: time::OffsetDateTime::now_utc() + time_to_live, + injection, }; - let previous_entry = self.entries.insert(jti, Arc::new(entry)); - - Ok(previous_entry) + self.entries.insert(jti, Arc::new(entry)) } - fn get(&self, token_id: Uuid) -> Option { - self.entries.get(&token_id).map(Arc::clone) + fn get(&self, jti: Uuid) -> Option { + // Filter expired entries at lookup. The 15-minute cleanup task is best-effort; + // without this filter an expired entry remains usable until the next sweep, which + // makes `time_to_live` a soft hint rather than a hard limit. + self.entries + .get(&jti) + .filter(|entry| time::OffsetDateTime::now_utc() < entry.expires_at) + .map(Arc::clone) } } @@ -234,3 +254,6 @@ async fn cleanup_task(handle: CredentialStoreHandle, mut shutdown_signal: Shutdo debug!("Task terminated"); } + +#[cfg(test)] +mod tests; diff --git a/devolutions-gateway/src/credential/tests.rs b/devolutions-gateway/src/credential/tests.rs new file mode 100644 index 000000000..f970db617 --- /dev/null +++ b/devolutions-gateway/src/credential/tests.rs @@ -0,0 +1,131 @@ +use super::*; + +fn cleartext_mapping(proxy_username: &str) -> CleartextAppCredentialMapping { + CleartextAppCredentialMapping { + proxy: CleartextAppCredential::UsernamePassword { + username: proxy_username.to_owned(), + password: SecretString::from("proxy-password"), + }, + target: CleartextAppCredential::UsernamePassword { + username: "target@example.invalid".to_owned(), + password: SecretString::from("target-password"), + }, + } +} + +#[test] +fn insert_indexes_by_jti() { + let store = CredentialStoreHandle::new(); + let jti = Uuid::new_v4(); + + let previous = store + .insert( + jti, + "raw-token".to_owned(), + Some((cleartext_mapping("proxy@example.invalid"), "target.example".to_owned())), + time::Duration::minutes(5), + ) + .expect("insert succeeds"); + + assert!(previous.is_none()); + let entry = store.get(jti).expect("entry is indexed by JTI"); + assert_eq!(entry.jti, jti); + assert_eq!(entry.token, "raw-token"); + assert_eq!( + entry + .injection + .as_ref() + .expect("injection state present") + .target_hostname, + "target.example" + ); +} + +#[test] +fn re_insert_under_same_jti_replaces_entry() { + let store = CredentialStoreHandle::new(); + let jti = Uuid::new_v4(); + + store + .insert( + jti, + "token-a".to_owned(), + Some((cleartext_mapping("proxy@example.invalid"), "host-a".to_owned())), + time::Duration::minutes(5), + ) + .expect("first insert succeeds"); + let previous = store + .insert( + jti, + "token-b".to_owned(), + Some((cleartext_mapping("proxy@example.invalid"), "host-b".to_owned())), + time::Duration::minutes(5), + ) + .expect("second insert succeeds"); + + let previous = previous.expect("replacement must report previous entry"); + assert_eq!(previous.jti, jti); + assert_eq!(previous.token, "token-a"); + + let current = store.get(jti).expect("replacement entry present"); + assert_eq!(current.token, "token-b"); +} + +#[test] +fn distinct_jtis_coexist() { + let store = CredentialStoreHandle::new(); + let first_jti = Uuid::new_v4(); + let second_jti = Uuid::new_v4(); + + store + .insert( + first_jti, + "token-1".to_owned(), + Some((cleartext_mapping("proxy@example.invalid"), "h1".to_owned())), + time::Duration::minutes(5), + ) + .expect("first insert succeeds"); + store + .insert( + second_jti, + "token-2".to_owned(), + Some((cleartext_mapping("proxy@example.invalid"), "h2".to_owned())), + time::Duration::minutes(5), + ) + .expect("second insert succeeds"); + + assert_eq!(store.get(first_jti).expect("first entry").jti, first_jti); + assert_eq!(store.get(second_jti).expect("second entry").jti, second_jti); +} + +#[test] +fn lookup_filters_expired_entries() { + let store = CredentialStoreHandle::new(); + let jti = Uuid::new_v4(); + + store + .insert( + jti, + "raw-token".to_owned(), + Some((cleartext_mapping("proxy@example.invalid"), "host".to_owned())), + // Negative TTL: entry is born already expired. Validates the lookup-time filter + // without depending on real elapsed time in tests. + time::Duration::seconds(-1), + ) + .expect("insert succeeds"); + + assert!(store.get(jti).is_none(), "expired entry must not be returned"); +} + +#[test] +fn provision_token_entry_has_no_injection_state() { + let store = CredentialStoreHandle::new(); + let jti = Uuid::new_v4(); + + store + .insert(jti, "raw-token".to_owned(), None, time::Duration::minutes(5)) + .expect("insert succeeds"); + + let entry = store.get(jti).expect("entry exists"); + assert!(entry.injection.is_none()); +} diff --git a/devolutions-gateway/src/credential_injection_kdc.rs b/devolutions-gateway/src/credential_injection_kdc.rs new file mode 100644 index 000000000..aead87ade --- /dev/null +++ b/devolutions-gateway/src/credential_injection_kdc.rs @@ -0,0 +1,835 @@ +//! 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::credential::{ + AppCredential, AppCredentialMapping, ArcCredentialEntry, CredentialStoreHandle, InjectionState, +}; + +const IN_PROCESS_KDC_HOST: &str = "cred.invalid"; + +pub(crate) struct CredentialInjectionKdc { + jti: Uuid, + raw_token: String, + credential_mapping: AppCredentialMapping, + 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, +} + +pub(crate) type CredentialInjectionKdcResolution = Option>; + +#[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("credential-injection Kerberos state is not available for {jti}")] + MissingKerberosSession { jti: Uuid }, + #[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<'a> { + message: KdcProxyMessage, + realm_policy: CredentialInjectionKdcRealmPolicy<'a>, +} + +enum CredentialInjectionKdcRealmPolicy<'a> { + InProcess, + Token { realm: &'a str, bypass_for_debug: bool }, +} + +impl<'a> CredentialInjectionKdcRequest<'a> { + pub(crate) fn from_token(message: KdcProxyMessage, realm: &'a str, bypass_for_debug: bool) -> Self { + Self { + message, + realm_policy: CredentialInjectionKdcRealmPolicy::Token { + realm, + bypass_for_debug, + }, + } + } + + fn in_process(message: KdcProxyMessage) -> Self { + Self { + message, + realm_policy: CredentialInjectionKdcRealmPolicy::InProcess, + } + } +} + +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 { + pub(crate) fn from_parts( + credential_entry: ArcCredentialEntry, + session: Arc, + ) -> anyhow::Result { + let InjectionState { + mapping, + target_hostname, + } = credential_entry + .injection + .as_ref() + .context("credential entry has no credential-injection state")?; + anyhow::ensure!( + credential_entry.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: credential_entry.jti, + raw_token: credential_entry.token.clone(), + credential_mapping: mapping.clone(), + target_hostname: target_hostname.clone(), + session, + kdc_config, + }) + } + + pub(crate) fn resolve( + jet_cred_id: Option, + credential_store: &CredentialStoreHandle, + session_store: &CredentialInjectionKdcSessionStoreHandle, + ) -> Result { + let Some(jti) = jet_cred_id else { + return Ok(None); + }; + + let credential_entry = credential_store.get(jti).ok_or_else(|| { + warn!(%jti, "KDC token references missing credential-injection state"); + CredentialInjectionKdcResolveError::MissingCredential { jti } + })?; + + if credential_entry.injection.is_none() { + warn!(%jti, "KDC token references non-injection credential state"); + return Err(CredentialInjectionKdcResolveError::NonInjectionCredential { jti }); + } + + let session = session_store.get(jti).ok_or_else(|| { + warn!(%jti, "KDC token references missing credential-injection Kerberos state"); + CredentialInjectionKdcResolveError::MissingKerberosSession { jti } + })?; + + let kdc = Self::from_parts(credential_entry, session) + .map_err(|source| CredentialInjectionKdcResolveError::BuildKdcConfig { jti, source })?; + + Ok(Some(Box::new(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(self.target_credential().username()).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 or sspi-rs rejects the ticket. + Ok(sspi::KerberosServerConfig { + kerberos_config: sspi::KerberosConfig { + kdc_url: Some(kdc_url), + client_computer_name: Some(client_addr.to_string()), + }, + server_properties: sspi::kerberos::ServerProperties::new( + &["TERMSRV", &self.target_hostname], + Some(user), + Duration::from_secs(300), + Some(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" + ); + + match request.realm_policy { + CredentialInjectionKdcRealmPolicy::InProcess => {} + CredentialInjectionKdcRealmPolicy::Token { + realm, + bypass_for_debug, + } => { + if let Some(mismatch) = token_realm_mismatch(realm, &request_realm, bypass_for_debug) { + return Ok(CredentialInjectionKdcInterception::NotInjectionRealm(mismatch)); + } + } + } + + 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") + } +} + +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 token_realm_mismatch(token_realm: &str, request_realm: &str, bypass: bool) -> Option { + if token_realm.eq_ignore_ascii_case(request_realm) { + return None; + } + + if bypass { + warn!( + %token_realm, + %request_realm, + "**DEBUG OPTION** Allowed a KDC request towards a KDC whose Kerberos realm differs from what's inside the KDC token" + ); + return None; + } + + Some(RealmMismatch { + expected: token_realm.to_owned(), + actual: request_realm.to_owned(), + }) +} + +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. +pub(crate) struct CredentialInjectionKdcSession { + jti: Uuid, + pub(crate) 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. +pub(crate) 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()) +} + +pub(crate) 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 +} + +/// Parallel store of [`CredentialInjectionKdcSession`] entries keyed by association-token JTI. +/// +/// Pairs 1:1 with [`crate::credential::CredentialStoreHandle`]: a credential entry provisioned +/// with `provision-credentials` has a matching entry here under the same JTI. The two stores +/// share neither lock nor map so that the credential store stays Kerberos-unaware. +#[derive(Debug, Clone, Default)] +pub struct CredentialInjectionKdcSessionStoreHandle(Arc>>); + +#[derive(Debug)] +struct Entry { + session: Arc, + expires_at: time::OffsetDateTime, +} + +impl CredentialInjectionKdcSessionStoreHandle { + pub fn new() -> Self { + Self(Arc::new(Mutex::new(HashMap::new()))) + } + + pub(crate) fn insert(&self, session: CredentialInjectionKdcSession, time_to_live: time::Duration) { + let jti = session.jti; + let entry = Entry { + session: Arc::new(session), + expires_at: time::OffsetDateTime::now_utc() + time_to_live, + }; + self.0.lock().insert(jti, entry); + } + + pub(crate) fn get(&self, jti: Uuid) -> Option> { + // Lookup-time TTL enforcement mirrors `CredentialStoreHandle::get`: the cleanup task is + // best-effort, and we don't want to hand out Kerberos material whose paired credential + // entry has already expired. + self.0 + .lock() + .get(&jti) + .filter(|entry| time::OffsetDateTime::now_utc() < entry.expires_at) + .map(|entry| Arc::clone(&entry.session)) + } +} + +pub struct CleanupTask { + pub handle: CredentialInjectionKdcSessionStoreHandle, +} + +#[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.handle, shutdown_signal).await; + Ok(()) + } +} + +#[instrument(skip_all)] +async fn cleanup_task(handle: CredentialInjectionKdcSessionStoreHandle, 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; + } + } + + let now = time::OffsetDateTime::now_utc(); + handle.0.lock().retain(|_, entry| now < entry.expires_at); + } + + debug!("Task terminated"); +} + +#[cfg(test)] +mod tests { + use ironrdp_connector::sspi::network_client::NetworkProtocol; + use secrecy::SecretString; + + use super::*; + use crate::credential::{ + AppCredentialMapping, CleartextAppCredential, CleartextAppCredentialMapping, CredentialEntry, + }; + + 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 dummy_entry_with_target_username(jti: Uuid, target_username: &str) -> ArcCredentialEntry { + let mapping: AppCredentialMapping = cleartext_mapping_with_target_username(target_username) + .encrypt() + .expect("encrypt mapping"); + Arc::new(CredentialEntry { + jti, + token: "raw-token".to_owned(), + expires_at: time::OffsetDateTime::now_utc() + time::Duration::minutes(5), + injection: Some(InjectionState { + mapping, + target_hostname: "target.example".to_owned(), + }), + }) + } + + fn dummy_entry(jti: Uuid) -> ArcCredentialEntry { + 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(entry, 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(entry, 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 store_lookup_filters_expired_entries() { + let store = CredentialInjectionKdcSessionStoreHandle::new(); + let jti = Uuid::new_v4(); + let session = derive_credential_injection_kdc_session("proxy@example.invalid", jti); + + // Negative TTL: entry is born already expired. + store.insert(session, time::Duration::seconds(-1)); + + assert!(store.get(jti).is_none(), "expired entry must not be returned"); + } + + #[test] + fn store_returns_fresh_entry() { + let store = CredentialInjectionKdcSessionStoreHandle::new(); + let jti = Uuid::new_v4(); + let session = derive_credential_injection_kdc_session("proxy@example.invalid", jti); + + store.insert(session, time::Duration::minutes(5)); + + assert_eq!(store.get(jti).expect("fresh entry returned").realm, "example.invalid"); + } + + #[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 resolve_with_no_jet_cred_id_forwards_to_real_kdc() { + let credential_store = CredentialStoreHandle::new(); + let session_store = CredentialInjectionKdcSessionStoreHandle::new(); + + let dispatch = CredentialInjectionKdc::resolve(None, &credential_store, &session_store) + .expect("plain KDC token should dispatch"); + + assert!(dispatch.is_none()); + } + + #[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, 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 resolve_with_missing_jet_cred_id_fails_closed() { + let credential_store = CredentialStoreHandle::new(); + let session_store = CredentialInjectionKdcSessionStoreHandle::new(); + + assert!( + CredentialInjectionKdc::resolve(Some(Uuid::new_v4()), &credential_store, &session_store).is_err(), + "KDC tokens with jet_cred_id must not fall back to real-KDC forwarding" + ); + } + + #[test] + fn resolve_with_non_injection_entry_fails_closed() { + let credential_store = CredentialStoreHandle::new(); + let session_store = CredentialInjectionKdcSessionStoreHandle::new(); + let jti = Uuid::new_v4(); + + credential_store + .insert(jti, "raw-token".to_owned(), None, time::Duration::minutes(5)) + .expect("provision-token entry inserts"); + + assert!( + CredentialInjectionKdc::resolve(Some(jti), &credential_store, &session_store).is_err(), + "KDC tokens with jet_cred_id must require provision-credentials state" + ); + } + + #[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"); + } +} diff --git a/devolutions-gateway/src/extract.rs b/devolutions-gateway/src/extract.rs index bbf3be4e4..227029aba 100644 --- a/devolutions-gateway/src/extract.rs +++ b/devolutions-gateway/src/extract.rs @@ -1,11 +1,14 @@ +use std::net::SocketAddr; + use axum::Extension; -use axum::extract::{FromRequest, FromRequestParts, RawQuery, Request}; +use axum::extract::{ConnectInfo, FromRequest, FromRequestParts, Path, RawQuery, Request}; use axum::http::request::Parts; +use crate::DgwState; use crate::http::HttpError; use crate::token::{ AccessScope, AccessTokenClaims, AssociationTokenClaims, BridgeTokenClaims, JmuxTokenClaims, JrecTokenClaims, - JrlTokenClaims, ScopeTokenClaims, WebAppTokenClaims, + JrlTokenClaims, KdcTokenClaims, ScopeTokenClaims, WebAppTokenClaims, }; #[derive(Clone)] @@ -98,6 +101,46 @@ where } } +/// Extractor for the KDC proxy route's path-bound token. +/// +/// `/jet/KdcProxy/{token}` carries the token in the URL path rather than the standard +/// `Authorization: Bearer` header or `?token=` query parameter, so the global auth middleware +/// (`middleware/auth.rs`) skips it (see `AUTH_EXCEPTIONS`). This extractor reads the token from +/// the path, runs it through the same `authenticate()` routine the middleware would, and +/// unwraps the `Kdc` variant so handlers receive `KdcTokenClaims` directly. +#[derive(Clone)] +pub struct KdcToken(pub KdcTokenClaims); + +impl FromRequestParts for KdcToken { + type Rejection = HttpError; + + async fn from_request_parts(parts: &mut Parts, state: &DgwState) -> Result { + let Path(token) = Path::::from_request_parts(parts, state) + .await + .map_err(HttpError::bad_request().with_msg("KDC token missing from path").err())?; + let ConnectInfo(source_addr) = ConnectInfo::::from_request_parts(parts, state) + .await + .map_err(HttpError::internal().with_msg("source address unavailable").err())?; + + let conf = state.conf_handle.get_conf(); + let claims = crate::middleware::auth::authenticate( + source_addr, + &token, + &conf, + &state.token_cache, + &state.jrl, + &state.recordings.active_recordings, + None, + ) + .map_err(HttpError::unauthorized().err())?; + + match claims { + AccessTokenClaims::Kdc(claims) => Ok(Self(claims)), + _ => Err(HttpError::forbidden().msg("token not allowed (expected KDC token)")), + } + } +} + #[derive(Clone)] pub struct ScopeToken(pub ScopeTokenClaims); diff --git a/devolutions-gateway/src/generic_client.rs b/devolutions-gateway/src/generic_client.rs index 7b5e6c47b..7966b946e 100644 --- a/devolutions-gateway/src/generic_client.rs +++ b/devolutions-gateway/src/generic_client.rs @@ -9,6 +9,7 @@ use typed_builder::TypedBuilder; use crate::config::Conf; use crate::credential::CredentialStoreHandle; +use crate::credential_injection_kdc::{CredentialInjectionKdc, CredentialInjectionKdcSessionStoreHandle}; use crate::proxy::Proxy; use crate::rdp_pcb::{extract_association_claims, read_pcb}; use crate::recording::ActiveRecordings; @@ -28,6 +29,7 @@ pub struct GenericClient { subscriber_tx: SubscriberSender, active_recordings: Arc, credential_store: CredentialStoreHandle, + kerberos_session_store: CredentialInjectionKdcSessionStoreHandle, #[builder(default)] agent_tunnel_handle: Option>, } @@ -52,6 +54,7 @@ where subscriber_tx, active_recordings, credential_store, + kerberos_session_store, agent_tunnel_handle, } = self; @@ -147,35 +150,41 @@ where // 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 now works + // RdpProxy is generic over the server stream, so credential injection works // regardless of whether the upstream is direct TCP or tunnelled via an agent. - if is_rdp { - let token_id = token::extract_jti(token).context("failed to extract jti claim from token")?; - - if let Some(entry) = credential_store.get(token_id) { - anyhow::ensure!(token == entry.token, "token mismatch"); - - // NOTE: In the future, we could imagine performing proxy-based recording as well using RdpProxy. - if entry.mapping.is_some() { - 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_entry(entry) - .client_stream_leftover_bytes(leftover_bytes) - .server_dns_name(selected_target.host().to_owned()) - .disconnect_interest(disconnect_interest) - .build() - .run() - .await - .context("encountered a failure during RDP proxying (credential injection)"); - } - } + // 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) = credential_store.get(claims.jti) + && entry.injection.is_some() + && let Some(kdc_session) = kerberos_session_store.get(claims.jti) + { + anyhow::ensure!(token == entry.token, "token mismatch"); + let credential_injection_kdc = CredentialInjectionKdc::from_parts(entry, kdc_session)?; + + info!( + jti = %credential_injection_kdc.jti(), + "RDP-TLS forwarding with credential injection" + ); + + // 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_kdc(credential_injection_kdc) + .client_stream_leftover_bytes(leftover_bytes) + .server_dns_name(selected_target.host().to_owned()) + .disconnect_interest(disconnect_interest) + .build() + .run() + .await + .context("encountered a failure during RDP proxying (credential injection)"); } info!("Upstream forwarding"); diff --git a/devolutions-gateway/src/lib.rs b/devolutions-gateway/src/lib.rs index 3af9f7999..019252365 100644 --- a/devolutions-gateway/src/lib.rs +++ b/devolutions-gateway/src/lib.rs @@ -17,6 +17,7 @@ pub mod api; pub mod cli; pub mod config; pub mod credential; +pub mod credential_injection_kdc; pub mod extract; pub mod generic_client; pub mod http; @@ -60,6 +61,7 @@ pub struct DgwState { pub recordings: recording::RecordingMessageSender, pub job_queue_handle: job_queue::JobQueueHandle, pub credential_store: credential::CredentialStoreHandle, + pub kerberos_session_store: credential_injection_kdc::CredentialInjectionKdcSessionStoreHandle, pub monitoring_state: Arc, pub traffic_audit_handle: traffic_audit::TrafficAuditHandle, pub agent_tunnel_handle: Option>, @@ -88,6 +90,7 @@ impl DgwState { let (job_queue_handle, job_queue_rx) = job_queue::JobQueueHandle::new(); let (traffic_audit_handle, traffic_audit_rx) = traffic_audit::TrafficAuditHandle::new(); let credential_store = credential::CredentialStoreHandle::new(); + let kerberos_session_store = credential_injection_kdc::CredentialInjectionKdcSessionStoreHandle::new(); let monitoring_state = Arc::new(network_monitor::State::new(Arc::new(MockMonitorsCache))?); let state = Self { @@ -101,6 +104,7 @@ impl DgwState { job_queue_handle, traffic_audit_handle, credential_store, + kerberos_session_store, monitoring_state, agent_tunnel_handle: None, }; diff --git a/devolutions-gateway/src/listener.rs b/devolutions-gateway/src/listener.rs index 0b7ce2740..5898b738a 100644 --- a/devolutions-gateway/src/listener.rs +++ b/devolutions-gateway/src/listener.rs @@ -159,6 +159,7 @@ async fn handle_tcp_peer(stream: TcpStream, state: DgwState, peer_addr: SocketAd .subscriber_tx(state.subscriber_tx) .active_recordings(state.recordings.active_recordings) .credential_store(state.credential_store) + .kerberos_session_store(state.kerberos_session_store) .agent_tunnel_handle(state.agent_tunnel_handle) .build() .serve() diff --git a/devolutions-gateway/src/ngrok.rs b/devolutions-gateway/src/ngrok.rs index 71c0c005f..58115db89 100644 --- a/devolutions-gateway/src/ngrok.rs +++ b/devolutions-gateway/src/ngrok.rs @@ -238,6 +238,7 @@ async fn run_tcp_tunnel(mut tunnel: ngrok::tunnel::TcpTunnel, state: DgwState) { .subscriber_tx(state.subscriber_tx) .active_recordings(state.recordings.active_recordings) .credential_store(state.credential_store) + .kerberos_session_store(state.kerberos_session_store) .agent_tunnel_handle(state.agent_tunnel_handle) .build() .serve() diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index 41a118a02..3f790974e 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -3,17 +3,16 @@ use std::net::SocketAddr; use std::sync::Arc; use anyhow::Context as _; -use ironrdp_connector::sspi; use ironrdp_pdu::nego; use ironrdp_rdcleanpath::RDCleanPathPdu; -use secrecy::ExposeSecret as _; use tap::prelude::*; use thiserror::Error; use tokio::io::{AsyncRead, AsyncReadExt as _, AsyncWrite, AsyncWriteExt as _}; use tracing::field; use crate::config::Conf; -use crate::credential::{CredentialEntry, CredentialStoreHandle}; +use crate::credential::CredentialStoreHandle; +use crate::credential_injection_kdc::{CredentialInjectionKdc, CredentialInjectionKdcSessionStoreHandle}; use crate::proxy::Proxy; use crate::recording::ActiveRecordings; use crate::session::{ConnectionModeDetails, DisconnectInterest, DisconnectedInfo, SessionInfo, SessionMessageSender}; @@ -316,15 +315,13 @@ async fn handle_with_credential_injection( subscriber_tx: SubscriberSender, active_recordings: &ActiveRecordings, cleanpath_pdu: RDCleanPathPdu, - credential_entry: Arc, + credential_injection_kdc: CredentialInjectionKdc, agent_tunnel_handle: Option>, ) -> anyhow::Result<()> { let tls_conf = conf.credssp_tls.get().context("CredSSP TLS configuration")?; let gateway_hostname = conf.hostname.clone(); - let credential_mapping = credential_entry.mapping.as_ref().context("no credential mapping")?; - let x224_req = cleanpath_pdu .x224_connection_pdu .as_ref() @@ -418,57 +415,17 @@ 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_server_config = if conf.debug.enable_unstable - && let Some(crate::config::dto::KerberosConfig { - kerberos_server: - crate::config::dto::KerberosServer { - max_time_skew, - ticket_decryption_key, - service_user, - .. - }, - kdc_url: _, - }) = conf.debug.kerberos.as_ref() - { - let user = service_user.as_ref().map(|user| { - let crate::config::dto::DomainUser { - fqdn, - password, - salt: _, - } = user; - - // The username is in the FQDN format. Thus, the domain field can be empty. - sspi::CredentialsBuffers::AuthIdentity(sspi::AuthIdentityBuffers::from_utf8( - fqdn, - "", - password.expose_secret(), - )) - }); - - Some(sspi::KerberosServerConfig { - kerberos_config: sspi::KerberosConfig { - // The sspi-rs can automatically resolve the KDC host via DNS and/or env variable. - kdc_url: None, - client_computer_name: Some(client_addr.to_string()), - }, - server_properties: sspi::kerberos::ServerProperties::new( - &["TERMSRV", &gateway_hostname], - user, - std::time::Duration::from_secs(*max_time_skew), - ticket_decryption_key.clone(), - )?, - }) - } else { - None - }; + let krb_server_config = + crate::rdp_proxy::credential_injection_kerberos_server_config(&conf, client_addr, &credential_injection_kdc)?; - let client_credssp_fut = crate::rdp_proxy::perform_credssp_with_client( + let client_credssp_fut = crate::rdp_proxy::perform_credssp_as_server( &mut client_framed, client_addr.ip(), gateway_public_key, client_security_protocol, - &credential_mapping.proxy, + credential_injection_kdc.proxy_credential(), krb_server_config, + &credential_injection_kdc, ); let krb_client_config = if conf.debug.enable_unstable @@ -485,12 +442,12 @@ async fn handle_with_credential_injection( None }; - let server_credssp_fut = crate::rdp_proxy::perform_credssp_with_server( + 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_mapping.target, + credential_injection_kdc.target_credential(), krb_client_config, ); @@ -565,6 +522,7 @@ pub async fn handle( subscriber_tx: SubscriberSender, active_recordings: &ActiveRecordings, credential_store: &CredentialStoreHandle, + kerberos_session_store: &CredentialInjectionKdcSessionStoreHandle, agent_tunnel_handle: Option>, ) -> anyhow::Result<()> { // Special handshake of our RDP extension @@ -583,14 +541,19 @@ 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. - if let Some(entry) = crate::token::extract_jti(token) - .ok() - .and_then(|token_id| credential_store.get(token_id)) - .filter(|entry| entry.mapping.is_some()) - { - anyhow::ensure!(token == entry.token, "token mismatch"); - debug!("Switching to RdpProxy for credential injection (WebSocket)"); + // clean path procedure. The credential store is keyed on the association token's JTI, + // and the per-session Kerberos material lives in a parallel store under the same JTI. + if let Some(credential_injection_kdc) = crate::token::extract_jti(token).ok().and_then(|jti| { + let entry = credential_store.get(jti)?; + entry.injection.as_ref()?; + let kdc_session = kerberos_session_store.get(jti)?; + CredentialInjectionKdc::from_parts(entry, kdc_session).ok() + }) { + anyhow::ensure!(token == credential_injection_kdc.raw_token(), "token mismatch"); + debug!( + jti = %credential_injection_kdc.jti(), + "Switching to RdpProxy for credential injection (WebSocket)" + ); return handle_with_credential_injection( client_stream, @@ -602,7 +565,7 @@ pub async fn handle( subscriber_tx, active_recordings, cleanpath_pdu, - entry, + credential_injection_kdc, agent_tunnel_handle.clone(), ) .await; diff --git a/devolutions-gateway/src/rdp_proxy.rs b/devolutions-gateway/src/rdp_proxy.rs index af7d5f090..1ab4382f1 100644 --- a/devolutions-gateway/src/rdp_proxy.rs +++ b/devolutions-gateway/src/rdp_proxy.rs @@ -13,7 +13,10 @@ use typed_builder::TypedBuilder; use crate::api::kdc_proxy::send_krb_message; use crate::config::Conf; -use crate::credential::{AppCredentialMapping, ArcCredentialEntry}; +use crate::credential::AppCredential; +use crate::credential_injection_kdc::{ + CredentialInjectionClientAcceptorProtocol, CredentialInjectionKdc, CredentialInjectionKdcInterception, +}; use crate::proxy::Proxy; use crate::session::{DisconnectInterest, SessionInfo, SessionMessageSender}; use crate::subscriber::SubscriberSender; @@ -27,7 +30,7 @@ pub struct RdpProxy { client_addr: SocketAddr, server_stream: S, server_addr: SocketAddr, - credential_entry: ArcCredentialEntry, + credential_injection_kdc: CredentialInjectionKdc, client_stream_leftover_bytes: bytes::BytesMut, sessions: SessionMessageSender, subscriber_tx: SubscriberSender, @@ -58,7 +61,7 @@ where client_addr, server_stream, server_addr, - credential_entry, + credential_injection_kdc, client_stream_leftover_bytes, sessions, subscriber_tx, @@ -69,8 +72,6 @@ where let tls_conf = conf.credssp_tls.get().context("CredSSP TLS configuration")?; let gateway_hostname = conf.hostname.clone(); - let credential_mapping = credential_entry.mapping.as_ref().context("no credential mapping")?; - // -- 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( @@ -84,8 +85,12 @@ where 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_mapping).await?; + 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(); @@ -112,57 +117,16 @@ where let mut client_framed = ironrdp_tokio::MovableTokioFramed::new(client_stream); let mut server_framed = ironrdp_tokio::MovableTokioFramed::new(server_stream); - let krb_server_config = if conf.debug.enable_unstable - && let Some(crate::config::dto::KerberosConfig { - kerberos_server: - crate::config::dto::KerberosServer { - max_time_skew, - ticket_decryption_key, - service_user, - .. - }, - kdc_url: _, - }) = conf.debug.kerberos.as_ref() - { - let user = service_user.as_ref().map(|user| { - let crate::config::dto::DomainUser { - fqdn, - password, - salt: _, - } = user; - - // The username is in the FQDN format. Thus, the domain field can be empty. - sspi::CredentialsBuffers::AuthIdentity(sspi::AuthIdentityBuffers::from_utf8( - fqdn, - "", - password.expose_secret(), - )) - }); - - Some(sspi::KerberosServerConfig { - kerberos_config: sspi::KerberosConfig { - // The sspi-rs can automatically resolve the KDC host via DNS and/or env variable. - kdc_url: None, - client_computer_name: Some(client_addr.to_string()), - }, - server_properties: sspi::kerberos::ServerProperties::new( - &["TERMSRV", &gateway_hostname], - user, - std::time::Duration::from_secs(*max_time_skew), - ticket_decryption_key.clone(), - )?, - }) - } else { - None - }; + let krb_server_config = credential_injection_kerberos_server_config(&conf, client_addr, &credential_injection_kdc)?; - let client_credssp_fut = perform_credssp_with_client( + let client_credssp_fut = perform_credssp_as_server( &mut client_framed, client_addr.ip(), gateway_public_key, handshake_result.client_security_protocol, - &credential_mapping.proxy, + credential_injection_kdc.proxy_credential(), krb_server_config, + &credential_injection_kdc, ); let krb_client_config = if conf.debug.enable_unstable @@ -179,12 +143,12 @@ where None }; - let server_credssp_fut = perform_credssp_with_server( + let server_credssp_fut = perform_credssp_as_client( &mut server_framed, server_dns_name, server_public_key, handshake_result.server_security_protocol, - &credential_mapping.target, + credential_injection_kdc.target_credential(), krb_client_config, ); @@ -282,7 +246,7 @@ where async fn dual_handshake_until_tls_upgrade( client_framed: &mut ironrdp_tokio::MovableTokioFramed, server_framed: &mut ironrdp_tokio::MovableTokioFramed, - mapping: &AppCredentialMapping, + target_credential: &AppCredential, ) -> anyhow::Result where C: AsyncWrite + AsyncRead + Unpin + Send, @@ -311,8 +275,8 @@ where }; let connection_request_to_send = nego::ConnectionRequest { - nego_data: match &mapping.target { - crate::credential::AppCredential::UsernamePassword { username, .. } => { + nego_data: match target_credential { + AppCredential::UsernamePassword { username, .. } => { Some(nego::NegoRequestData::cookie(username.to_owned())) } }, @@ -393,13 +357,36 @@ where handshake_result } +pub(crate) fn credential_injection_kerberos_server_config( + conf: &Conf, + client_addr: SocketAddr, + credential_injection_kdc: &CredentialInjectionKdc, +) -> anyhow::Result> { + if !conf.debug.enable_unstable || conf.debug.kerberos.is_none() { + return Ok(None); + } + + match credential_injection_kdc.client_acceptor_protocol()? { + CredentialInjectionClientAcceptorProtocol::Kerberos => { + credential_injection_kdc.server_kerberos_config(client_addr).map(Some) + } + CredentialInjectionClientAcceptorProtocol::Ntlm => { + debug!( + jti = %credential_injection_kdc.jti(), + "Credential-injection Kerberos acceptor disabled for NTLM target credential" + ); + Ok(None) + } + } +} + #[instrument(name = "server_credssp", level = "debug", ret, skip_all)] -pub(crate) async fn perform_credssp_with_server( +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: &crate::credential::AppCredential, + credentials: &AppCredential, kerberos_config: Option, ) -> anyhow::Result<()> where @@ -423,7 +410,7 @@ where credentials, None, security_protocol, - ironrdp_connector::ServerName::new(server_name), + ironrdp_connector::ServerName::new(server_name.clone()), server_public_key, kerberos_config, )?; @@ -465,18 +452,25 @@ where async fn resolve_server_generator( generator: &mut CredsspServerProcessGenerator<'_>, + credential_injection_kdc: &CredentialInjectionKdc, ) -> Result { let mut state = generator.start(); loop { match state { GeneratorState::Suspended(request) => { - let response = send_network_request(&request) - .await - .map_err(|err| sspi::credssp::ServerError { - ts_request: None, - error: sspi::Error::new(sspi::ErrorKind::InternalError, err), - })?; + let response = match credential_injection_kdc.intercept_network_request(&request) { + Ok(CredentialInjectionKdcInterception::Intercepted(response)) => Ok(response), + Ok(CredentialInjectionKdcInterception::NotInjectionRequest) => send_network_request(&request).await, + Ok(CredentialInjectionKdcInterception::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)); } @@ -508,13 +502,14 @@ async fn resolve_client_generator( } #[instrument(name = "client_credssp", level = "debug", ret, skip_all)] -pub(crate) async fn perform_credssp_with_client( +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: &crate::credential::AppCredential, + credentials: &AppCredential, kerberos_server_config: Option, + credential_injection_kdc: &CredentialInjectionKdc, ) -> anyhow::Result<()> where S: ironrdp_tokio::FramedRead + ironrdp_tokio::FramedWrite, @@ -535,6 +530,7 @@ where gateway_public_key, credentials, kerberos_server_config, + credential_injection_kdc, ) .await; @@ -560,8 +556,9 @@ where buf: &mut ironrdp_pdu::WriteBuf, client_computer_name: ironrdp_connector::ServerName, public_key: Vec, - credentials: &crate::credential::AppCredential, + credentials: &AppCredential, kerberos_server_config: Option, + credential_injection_kdc: &CredentialInjectionKdc, ) -> anyhow::Result<()> where S: ironrdp_tokio::FramedRead + ironrdp_tokio::FramedWrite, @@ -603,7 +600,7 @@ where let result = { let mut generator = sequence.process_ts_request(ts_request); - resolve_server_generator(&mut generator).await + resolve_server_generator(&mut generator, credential_injection_kdc).await }; // drop generator buf.clear(); @@ -634,14 +631,27 @@ where Ok(()) } +/// Generic Kerberos network-request dispatcher. +/// +/// Only handles real-network schemes (`tcp` / `udp`); credential-injection loopback requests +/// are intercepted by [`CredentialInjectionKdc`] before reaching this function. +/// +/// TODO(sspi-rs#NNN): when sspi-rs ships a pluggable KDC dispatcher API, the URL trick for +/// credential injection goes away entirely and this helper can be inlined back into the +/// CredSSP loops. async fn send_network_request(request: &NetworkRequest) -> anyhow::Result> { - let target_addr = TargetAddr::parse(request.url.as_str(), Some(88))?; - - // TODO(DGW-384): plumb `agent_tunnel_handle` through `RdpProxy` so - // CredSSP-originated Kerberos requests can traverse the agent tunnel. - // Currently these go direct from the gateway host, bypassing the - // routing pipeline used by every other proxy path. - send_krb_message(&target_addr, &request.data) - .await - .map_err(|err| anyhow::Error::msg("failed to send KDC message").context(err)) + match request.url.scheme() { + "tcp" | "udp" => { + let target_addr = TargetAddr::parse(request.url.as_str(), Some(88))?; + + // TODO(DGW-384): plumb `agent_tunnel_handle` through `RdpProxy` so + // CredSSP-originated Kerberos requests can traverse the agent tunnel. + // Currently these go direct from the gateway host, bypassing the + // routing pipeline used by every other proxy path. + send_krb_message(&target_addr, &request.data) + .await + .map_err(|err| anyhow::Error::msg("failed to send KDC message").context(err)) + } + unsupported => anyhow::bail!("unsupported KDC request scheme: {unsupported}"), + } } diff --git a/devolutions-gateway/src/service.rs b/devolutions-gateway/src/service.rs index f3de73ccf..47823b872 100644 --- a/devolutions-gateway/src/service.rs +++ b/devolutions-gateway/src/service.rs @@ -269,6 +269,8 @@ async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result { .context("failed to initialize traffic audit manager")?; let credential_store = CredentialStoreHandle::new(); + let kerberos_session_store = + devolutions_gateway::credential_injection_kdc::CredentialInjectionKdcSessionStoreHandle::new(); let filesystem_monitor_config_cache = devolutions_gateway::api::monitoring::FilesystemConfigCache::new( config::get_data_dir().join("monitors_cache.json"), @@ -317,6 +319,7 @@ async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result { recordings: recording_manager_handle.clone(), job_queue_handle: job_queue_ctx.job_queue_handle.clone(), credential_store: credential_store.clone(), + kerberos_session_store: kerberos_session_store.clone(), monitoring_state, traffic_audit_handle: traffic_audit_task.handle(), agent_tunnel_handle, @@ -355,6 +358,10 @@ async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result { handle: credential_store, }); + tasks.register(devolutions_gateway::credential_injection_kdc::CleanupTask { + handle: kerberos_session_store, + }); + tasks.register(devolutions_log::LogDeleterTask::::new( conf.log_file.clone(), )); diff --git a/devolutions-gateway/src/token.rs b/devolutions-gateway/src/token.rs index e188da969..1e29b7471 100644 --- a/devolutions-gateway/src/token.rs +++ b/devolutions-gateway/src/token.rs @@ -616,6 +616,11 @@ pub struct KdcTokenClaims { /// Default scheme is `tcp`. /// Default port is `88`. pub krb_kdc: TargetAddr, + + /// JTI of the association token whose credential-injection state this KDC token should + /// dispatch through. Present only on KDC tokens minted alongside an injection session; + /// plain Kerberos proxying leaves this `None` and forwards to the real KDC. + pub jet_cred_id: Option, } // ----- jrl claims ----- // @@ -1204,11 +1209,15 @@ fn validate_token_impl( Ok(claims) } -fn extract_uuid(token: &str, field: &str) -> anyhow::Result { +fn extract_payload(token: &str) -> anyhow::Result { let jws = RawJws::decode(token) .context("failed to parse the provided JWS")? .discard_signature(); - let payload = serde_json::from_slice::(&jws.payload).context("parse JWS payload")?; + serde_json::from_slice::(&jws.payload).context("parse JWS payload") +} + +fn extract_uuid(token: &str, field: &str) -> anyhow::Result { + let payload = extract_payload(token)?; let uuid = payload.get(field).context("claim is missing from the token")?; let uuid = uuid.as_str().context("value is malformed")?; let uuid = Uuid::from_str(uuid).context("value is not a valid UUID string")?; @@ -1224,6 +1233,36 @@ pub fn extract_session_id(token: &str) -> anyhow::Result { extract_uuid(token, "jet_aid").context("extract jet_aid") } +/// Extract the destination host claim (`dst_hst`) from an association token without verifying its +/// signature. Returns `None` if the claim is missing. +/// +/// Used by the credential store to remember the target server's hostname for a credential-injection +/// session, so that fake-KDC can validate the client's TGS-REQ sname against it (the SPN the client +/// actually requested is `TERMSRV/`, not Gateway's own hostname). +pub fn extract_dst_hst(token: &str) -> anyhow::Result> { + let payload = extract_payload(token)?; + let Some(value) = payload.get("dst_hst") else { + return Ok(None); + }; + let dst_hst = value.as_str().context("dst_hst is malformed")?; + Ok(Some(dst_hst.to_owned())) +} + +/// Extract alternate destination hosts (`dst_alt`) from an association token without verifying its +/// signature. +pub fn extract_dst_alt(token: &str) -> anyhow::Result> { + let payload = extract_payload(token)?; + let Some(value) = payload.get("dst_alt") else { + return Ok(Vec::new()); + }; + + let dst_alt = value.as_array().context("dst_alt is malformed")?; + dst_alt + .iter() + .map(|value| value.as_str().context("dst_alt entry is malformed").map(str::to_owned)) + .collect() +} + #[deprecated = "make sure this is never used without a deliberate action"] pub mod unsafe_debug { // Any function in this module should only be used at development stage when deliberately @@ -1391,6 +1430,8 @@ mod serde_impl { struct KdcClaimsHelper { krb_realm: SmolStr, krb_kdc: SmolStr, + #[serde(default, skip_serializing_if = "Option::is_none")] + jet_cred_id: Option, } impl ser::Serialize for SessionTtl { @@ -1640,6 +1681,7 @@ mod serde_impl { KdcClaimsHelper { krb_realm: self.krb_realm.clone(), krb_kdc: SmolStr::new(self.krb_kdc.as_str()), + jet_cred_id: self.jet_cred_id, } .serialize(serializer) } @@ -1654,11 +1696,9 @@ mod serde_impl { let claims = KdcClaimsHelper::deserialize(deserializer)?; - // Validate krb_realm value - - if claims.krb_realm.chars().any(char::is_uppercase) { - return Err(de::Error::custom("krb_realm field contains uppercases")); - } + // Normalize `krb_realm` for token producers that follow the Kerberos uppercase realm + // convention. Realm authorization remains case-insensitive at use sites. + let krb_realm = SmolStr::new(claims.krb_realm.to_ascii_lowercase()); // Validate krb_kdc field @@ -1673,9 +1713,53 @@ mod serde_impl { } Ok(Self { - krb_realm: claims.krb_realm, + krb_realm, krb_kdc, + jet_cred_id: claims.jet_cred_id, }) } } } + +#[cfg(test)] +mod tests { + use base64::Engine as _; + + use super::*; + + 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}") + } + + #[test] + fn kdc_claims_normalizes_uppercase_realm() { + let claims: KdcTokenClaims = serde_json::from_value(serde_json::json!({ + "krb_realm": "CRED-D0089FC1456D4A7E84AEEB6D7C6C59B7.INVALID", + "krb_kdc": "tcp://dc.example:88" + })) + .expect("uppercase realm should deserialize"); + + assert_eq!( + claims.krb_realm.as_str(), + "cred-d0089fc1456d4a7e84aeeb6d7c6c59b7.invalid" + ); + } + + #[test] + fn extract_dst_alt_returns_alternate_targets() { + let token = unsigned_jws(serde_json::json!({ + "jti": "5e3e833f-84c7-4541-b676-acc3299e39b8", + "dst_hst": "primary.example:3389", + "dst_alt": ["secondary.example:3389"] + })); + + assert_eq!( + extract_dst_alt(&token).expect("dst_alt parses"), + vec!["secondary.example:3389".to_owned()] + ); + } +} diff --git a/devolutions-gateway/tests/preflight.rs b/devolutions-gateway/tests/preflight.rs index 0f3f175ae..1f78d3094 100644 --- a/devolutions-gateway/tests/preflight.rs +++ b/devolutions-gateway/tests/preflight.rs @@ -2,13 +2,12 @@ #![allow(clippy::unwrap_used)] use std::net::SocketAddr; -use std::str::FromStr as _; use axum::Router; use axum::body::Body; use axum::extract::connect_info::MockConnectInfo; use axum::http::{self, Request, StatusCode}; -use devolutions_gateway::credential::AppCredential; +use base64::Engine as _; use devolutions_gateway::{DgwState, MockHandles}; use http_body_util::BodyExt as _; use serde_json::json; @@ -31,7 +30,8 @@ const CONFIG: &str = r#"{ } ], "__debug__": { - "disable_token_validation": true + "disable_token_validation": true, + "enable_unstable": true } }"#; @@ -46,6 +46,14 @@ fn preflight_request(operations: serde_json::Value) -> anyhow::Result anyhow::Result { + 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)?); + let signature = engine.encode(b"signature"); + Ok(format!("{header}.{payload}.{signature}")) +} + fn make_router() -> anyhow::Result<(Router, DgwState, MockHandles)> { let (state, handles) = DgwState::mock(CONFIG)?; let app = devolutions_gateway::make_http_service(state.clone()) @@ -53,6 +61,13 @@ fn make_router() -> anyhow::Result<(Router, DgwState, MockHandles)> { Ok((app, state, handles)) } +fn make_router_with_config(config: &str) -> anyhow::Result<(Router, DgwState, MockHandles)> { + let (state, handles) = DgwState::mock(config)?; + let app = devolutions_gateway::make_http_service(state.clone()) + .layer(MockConnectInfo(SocketAddr::from(([0, 0, 0, 0], 3000)))); + Ok((app, state, handles)) +} + fn init_logger() -> tracing::subscriber::DefaultGuard { tracing_subscriber::fmt() .with_test_writer() @@ -64,10 +79,11 @@ fn init_logger() -> tracing::subscriber::DefaultGuard { async fn test_provision_credentials_success() -> anyhow::Result<()> { let _guard = init_logger(); - let (app, state, _handles) = make_router()?; + let (app, _state, _handles) = make_router()?; - let token_id = Uuid::from_str("5e3e833f-84c7-4541-b676-acc3299e39b8").unwrap(); - let token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJqdGkiOiI1ZTNlODMzZi04NGM3LTQ1NDEtYjY3Ni1hY2MzMjk5ZTM5YjgifQ.1qECGlrW7y9HWFArc6GPHLGTOY7PhAvzKJ5XMRBg4k4"; + // JWT payload includes `dst_hst` because credential injection requires a target hostname + // (fake-KDC validates TGS-REQ sname against `TERMSRV/`); insert rejects tokens without it. + let token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJqdGkiOiI1ZTNlODMzZi04NGM3LTQ1NDEtYjY3Ni1hY2MzMjk5ZTM5YjgiLCJkc3RfaHN0IjoidGFyZ2V0LmV4YW1wbGU6MzM4OSJ9.1qECGlrW7y9HWFArc6GPHLGTOY7PhAvzKJ5XMRBg4k4"; let op_id = Uuid::new_v4(); @@ -89,18 +105,85 @@ async fn test_provision_credentials_success() -> anyhow::Result<()> { let body: serde_json::Value = serde_json::from_slice(&body)?; assert_eq!(body.as_array().expect("an array").len(), 1); assert_eq!(body[0]["operation_id"], op_id.to_string()); - assert_eq!(body[0]["kind"], "ack", "{:?}", body[1]); + assert_eq!(body[0]["kind"], "ack", "{:?}", body[0]); + + Ok(()) +} - let entry = state.credential_store.get(token_id).expect("the provisioned entry"); - assert_eq!(entry.token, token); +#[tokio::test] +async fn test_provision_credentials_success_when_unstable_disabled() -> anyhow::Result<()> { + let _guard = init_logger(); - let now = time::OffsetDateTime::now_utc(); - assert!(now + time::Duration::seconds(10) < entry.expires_at); - assert!(entry.expires_at < now + time::Duration::seconds(20)); + let config = CONFIG.replace("\"enable_unstable\": true", "\"enable_unstable\": false"); + let (app, _state, _handles) = make_router_with_config(&config)?; - let mapping = entry.mapping.as_ref().expect("the provisioned mapping"); - assert!(matches!(mapping.proxy, AppCredential::UsernamePassword { .. })); - assert!(matches!(mapping.target, AppCredential::UsernamePassword { .. })); + // `provision-credentials` is protocol-neutral: NTLM credential injection relies on this + // preflight state even when the Kerberos injection path is disabled. + let token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJqdGkiOiI1ZTNlODMzZi04NGM3LTQ1NDEtYjY3Ni1hY2MzMjk5ZTM5YjgiLCJkc3RfaHN0IjoidGFyZ2V0LmV4YW1wbGU6MzM4OSJ9.1qECGlrW7y9HWFArc6GPHLGTOY7PhAvzKJ5XMRBg4k4"; + + let op_id = Uuid::new_v4(); + + let op = json!([{ + "id": op_id, + "kind": "provision-credentials", + "token": token, + "proxy_credential": { "kind": "username-password", "username": "proxy_user", "password": "secret1" }, + "target_credential": { "kind": "username-password", "username": "target_user", "password": "secret2" }, + "time_to_live": 15 + }]); + + let response = app.oneshot(preflight_request(op)?).await.unwrap(); + assert_eq!(response.status(), StatusCode::OK); + + let body = response.into_body().collect().await?.to_bytes(); + let body: serde_json::Value = serde_json::from_slice(&body)?; + assert_eq!(body.as_array().expect("an array").len(), 1); + assert_eq!(body[0]["operation_id"], op_id.to_string()); + assert_eq!(body[0]["kind"], "ack", "{:?}", body[0]); + + Ok(()) +} + +#[tokio::test] +async fn test_provision_credentials_rejects_alternate_targets() -> anyhow::Result<()> { + let _guard = init_logger(); + + let (app, _state, _handles) = make_router()?; + + let token = unsigned_jws(json!({ + "jti": "5e3e833f-84c7-4541-b676-acc3299e39b8", + "dst_hst": "target-primary.example:3389", + "dst_alt": ["target-secondary.example:3389"] + }))?; + + let op_id = Uuid::new_v4(); + + let op = json!([{ + "id": op_id, + "kind": "provision-credentials", + "token": token, + "proxy_credential": { "kind": "username-password", "username": "proxy_user", "password": "secret1" }, + "target_credential": { "kind": "username-password", "username": "target_user", "password": "secret2" }, + "time_to_live": 15 + }]); + + let response = app.oneshot(preflight_request(op)?).await?; + assert_eq!(response.status(), StatusCode::OK); + + let body = response.into_body().collect().await?.to_bytes(); + let body: serde_json::Value = serde_json::from_slice(&body)?; + + assert_eq!(body.as_array().expect("an array").len(), 1); + assert_eq!(body[0]["kind"], "alert"); + assert_eq!(body[0]["alert_status"], "invalid-parameters"); + assert!( + body[0]["alert_message"] + .as_str() + .expect("alert message") + .contains("dst_alt"), + "{:?}", + body[0] + ); Ok(()) } diff --git a/tools/tokengen/src/lib.rs b/tools/tokengen/src/lib.rs index 3fdb532ee..1532d6581 100644 --- a/tools/tokengen/src/lib.rs +++ b/tools/tokengen/src/lib.rs @@ -103,6 +103,8 @@ pub struct KdcClaims<'a> { pub krb_kdc: &'a str, #[serde(skip_serializing_if = "Option::is_none")] pub jet_gw_id: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub jet_cred_id: Option, pub exp: i64, pub nbf: i64, pub jti: Uuid, @@ -259,6 +261,7 @@ pub enum SubCommandArgs { Kdc { krb_realm: String, krb_kdc: String, + jet_cred_id: Option, }, Jrl { revoked_jti_list: Vec, @@ -452,13 +455,18 @@ pub fn generate_token( }; ("JREC", serde_json::to_value(claims)?) } - SubCommandArgs::Kdc { krb_realm, krb_kdc } => { + SubCommandArgs::Kdc { + krb_realm, + krb_kdc, + jet_cred_id, + } => { let claims = KdcClaims { exp, nbf, krb_realm: &krb_realm, krb_kdc: &krb_kdc, jet_gw_id, + jet_cred_id, jti, }; ("KDC", serde_json::to_value(claims)?) diff --git a/tools/tokengen/src/main.rs b/tools/tokengen/src/main.rs index c080b8ba1..2a3fbdaca 100644 --- a/tools/tokengen/src/main.rs +++ b/tools/tokengen/src/main.rs @@ -122,7 +122,15 @@ fn sign( jet_aid, jet_reuse, }, - SignSubCommand::Kdc { krb_realm, krb_kdc } => SubCommandArgs::Kdc { krb_realm, krb_kdc }, + SignSubCommand::Kdc { + krb_realm, + krb_kdc, + jet_cred_id, + } => SubCommandArgs::Kdc { + krb_realm, + krb_kdc, + jet_cred_id, + }, SignSubCommand::Jrl { jti } => SubCommandArgs::Jrl { revoked_jti_list: jti }, SignSubCommand::NetScan {} => SubCommandArgs::NetScan {}, }; @@ -258,6 +266,8 @@ enum SignSubCommand { krb_realm: String, #[clap(long)] krb_kdc: String, + #[clap(long)] + jet_cred_id: Option, }, Jrl { #[clap(long)] diff --git a/tools/tokengen/src/server/server_impl.rs b/tools/tokengen/src/server/server_impl.rs index 400ba8bf9..7913d0f5b 100644 --- a/tools/tokengen/src/server/server_impl.rs +++ b/tools/tokengen/src/server/server_impl.rs @@ -212,6 +212,7 @@ pub(crate) async fn kdc_handler( SubCommandArgs::Kdc { krb_realm: request.krb_realm, krb_kdc: request.krb_kdc, + jet_cred_id: request.jet_cred_id, }, ) .await @@ -341,6 +342,7 @@ pub(crate) struct KdcRequest { common: CommonRequest, krb_realm: String, krb_kdc: String, + jet_cred_id: Option, } #[derive(Deserialize)] diff --git a/utils/dotnet/Devolutions.Gateway.Utils/src/AssociationClaims.cs b/utils/dotnet/Devolutions.Gateway.Utils/src/AssociationClaims.cs index eca9a1f29..f07c6a4ae 100644 --- a/utils/dotnet/Devolutions.Gateway.Utils/src/AssociationClaims.cs +++ b/utils/dotnet/Devolutions.Gateway.Utils/src/AssociationClaims.cs @@ -50,4 +50,4 @@ public string GetContentType() { return "ASSOCIATION"; } -} \ No newline at end of file +} diff --git a/utils/dotnet/Devolutions.Gateway.Utils/src/KdcClaims.cs b/utils/dotnet/Devolutions.Gateway.Utils/src/KdcClaims.cs index d975d58a5..bb43f2ec9 100644 --- a/utils/dotnet/Devolutions.Gateway.Utils/src/KdcClaims.cs +++ b/utils/dotnet/Devolutions.Gateway.Utils/src/KdcClaims.cs @@ -10,6 +10,9 @@ public class KdcClaims : IGatewayClaims public string KrbRealm { get; set; } [JsonPropertyName("jet_gw_id")] public Guid ScopeGatewayId { get; set; } + [JsonPropertyName("jet_cred_id")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public Guid? JetCredId { get; set; } public KdcClaims(Guid scopeGatewayId, TargetAddr krbKdc, string krbRealm) { @@ -22,4 +25,4 @@ public string GetContentType() { return "KDC"; } -} \ No newline at end of file +} diff --git a/utils/dotnet/Devolutions.Gateway.Utils/src/ProvisionCredentialsRequest.cs b/utils/dotnet/Devolutions.Gateway.Utils/src/ProvisionCredentialsRequest.cs new file mode 100644 index 000000000..ccef5a0dd --- /dev/null +++ b/utils/dotnet/Devolutions.Gateway.Utils/src/ProvisionCredentialsRequest.cs @@ -0,0 +1,57 @@ +using System.Text.Json.Serialization; + +namespace Devolutions.Gateway.Utils; + +public class ProvisionCredentialsRequest +{ + [JsonPropertyName("id")] + public Guid Id { get; set; } + + [JsonPropertyName("kind")] + public string Kind => "provision-credentials"; + + [JsonPropertyName("token")] + public string Token { get; set; } + + [JsonPropertyName("proxy_credential")] + public CleartextCredential ProxyCredential { get; set; } + + [JsonPropertyName("target_credential")] + public CleartextCredential TargetCredential { get; set; } + + [JsonPropertyName("time_to_live")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public uint? TimeToLive { get; set; } + + public ProvisionCredentialsRequest( + Guid id, + string token, + CleartextCredential proxyCredential, + CleartextCredential targetCredential, + uint? timeToLive = null) + { + this.Id = id; + this.Token = token; + this.ProxyCredential = proxyCredential; + this.TargetCredential = targetCredential; + this.TimeToLive = timeToLive; + } +} + +public class CleartextCredential +{ + [JsonPropertyName("kind")] + public string Kind => "username-password"; + + [JsonPropertyName("username")] + public string Username { get; set; } + + [JsonPropertyName("password")] + public string Password { get; set; } + + public CleartextCredential(string username, string password) + { + this.Username = username; + this.Password = password; + } +} From 7d7ae77d6f2e5ebcee786bc558f01df67785f9a3 Mon Sep 17 00:00:00 2001 From: irving ou Date: Fri, 15 May 2026 19:01:12 -0400 Subject: [PATCH 2/3] refactor(dgw): keep credential store protocol-neutral --- devolutions-gateway/src/api/preflight.rs | 90 +-------- devolutions-gateway/src/api/rdp.rs | 4 - devolutions-gateway/src/credential/mod.rs | 147 ++++++--------- devolutions-gateway/src/credential/tests.rs | 131 ------------- .../src/credential_injection_kdc.rs | 177 +++++++++++++----- devolutions-gateway/src/generic_client.rs | 9 +- devolutions-gateway/src/listener.rs | 1 - devolutions-gateway/src/ngrok.rs | 1 - devolutions-gateway/src/rd_clean_path.rs | 11 +- devolutions-gateway/src/rdp_proxy.rs | 2 +- devolutions-gateway/src/token.rs | 6 +- 11 files changed, 212 insertions(+), 367 deletions(-) delete mode 100644 devolutions-gateway/src/credential/tests.rs diff --git a/devolutions-gateway/src/api/preflight.rs b/devolutions-gateway/src/api/preflight.rs index 924962535..c8309face 100644 --- a/devolutions-gateway/src/api/preflight.rs +++ b/devolutions-gateway/src/api/preflight.rs @@ -11,8 +11,7 @@ use uuid::Uuid; use crate::DgwState; use crate::config::Conf; -use crate::credential::CredentialStoreHandle; -use crate::credential_injection_kdc::CredentialInjectionKdcSessionStoreHandle; +use crate::credential::{CredentialStoreHandle, InsertError}; use crate::extract::PreflightScope; use crate::http::HttpError; use crate::session::SessionMessageSender; @@ -197,7 +196,6 @@ pub(super) async fn post_preflight( conf_handle, sessions, credential_store, - kerberos_session_store, .. }): State, _scope: PreflightScope, @@ -225,21 +223,12 @@ pub(super) async fn post_preflight( let conf = conf_handle.get_conf(); let sessions = sessions.clone(); let credential_store = credential_store.clone(); - let kerberos_session_store = kerberos_session_store.clone(); async move { let operation_id = operation.id; trace!(%operation.id, "Process preflight operation"); - if let Err(error) = handle_operation( - operation, - &outputs, - &conf, - &sessions, - &credential_store, - &kerberos_session_store, - ) - .await + if let Err(error) = handle_operation(operation, &outputs, &conf, &sessions, &credential_store).await { outputs.push(PreflightOutput { operation_id, @@ -268,7 +257,6 @@ async fn handle_operation( conf: &Conf, sessions: &SessionMessageSender, credential_store: &CredentialStoreHandle, - kerberos_session_store: &CredentialInjectionKdcSessionStoreHandle, ) -> Result<(), PreflightError> { match operation.kind.as_str() { OP_GET_VERSION => outputs.push(PreflightOutput { @@ -349,77 +337,17 @@ async fn handle_operation( }); } - // Token signature isn't validated at the preflight scope (DVLS signs and pushes; - // gateway only re-uses what DVLS already verified). We do parse JTI / dst_hst here - // because the credential store no longer touches JWT shape — see the contract on - // `CredentialStoreHandle::insert`. - let jti = crate::token::extract_jti(&token).map_err(|error| { - PreflightError::new(PreflightAlertStatus::InvalidParams, format!("invalid token: {error:#}")) - })?; - - // For `provision-credentials`: resolve `target_hostname` from `dst_hst` *before* - // calling either store, so an invalid target is reported as `invalid-parameters` - // at preflight rather than mid-CredSSP. Also derive per-session Kerberos material - // from the proxy username; both stores receive their entries paired and keyed by - // the same JTI. - let injection_payload = if let Some(mapping) = mapping { - const DEFAULT_DST_PORT: u16 = 3389; - let raw_dst_hst = crate::token::extract_dst_hst(&token) - .map_err(|error| { - PreflightError::new( - PreflightAlertStatus::InvalidParams, - format!("read dst_hst from association token: {error:#}"), - ) - })? - .ok_or_else(|| { - PreflightError::new( - PreflightAlertStatus::InvalidParams, - "association token has no dst_hst, required for credential injection", - ) - })?; - - let dst_alt = crate::token::extract_dst_alt(&token).map_err(|error| { - PreflightError::new( - PreflightAlertStatus::InvalidParams, - format!("read dst_alt from association token: {error:#}"), - ) - })?; - if !dst_alt.is_empty() { - return Err(PreflightError::new( - PreflightAlertStatus::InvalidParams, - "association token dst_alt is not supported for credential injection", - )); - } - - let target_hostname = crate::target_addr::TargetAddr::parse(&raw_dst_hst, DEFAULT_DST_PORT) - .map_err(|error| { - PreflightError::new( - PreflightAlertStatus::InvalidParams, - format!("parse dst_hst as target address: {error:#}"), - ) - })? - .host() - .to_owned(); - - let kdc_session = crate::credential_injection_kdc::derive_credential_injection_kdc_session( - mapping.proxy_username(), - jti, - ); - kerberos_session_store.insert(kdc_session, time_to_live); - - Some((mapping, target_hostname)) - } else { - None - }; - let previous_entry = credential_store - .insert(jti, token, injection_payload, time_to_live) + .insert(token, mapping, time_to_live) .inspect_err(|error| warn!(%operation.id, error = format!("{error:#}"), "Failed to insert credentials")) - .map_err(|_| { - PreflightError::new( + .map_err(|error| match error { + InsertError::InvalidToken(error) => { + PreflightError::new(PreflightAlertStatus::InvalidParams, format!("invalid token: {error:#}")) + } + InsertError::Internal(_) => PreflightError::new( PreflightAlertStatus::InternalServerError, "an internal error occurred".to_owned(), - ) + ), })?; if previous_entry.is_some() { diff --git a/devolutions-gateway/src/api/rdp.rs b/devolutions-gateway/src/api/rdp.rs index b684a28d5..6129a776c 100644 --- a/devolutions-gateway/src/api/rdp.rs +++ b/devolutions-gateway/src/api/rdp.rs @@ -26,7 +26,6 @@ pub async fn handler( recordings, shutdown_signal, credential_store, - kerberos_session_store, agent_tunnel_handle, .. }): State, @@ -48,7 +47,6 @@ pub async fn handler( recordings.active_recordings, source_addr, credential_store, - kerberos_session_store, agent_tunnel_handle, ) .instrument(span) @@ -69,7 +67,6 @@ async fn handle_socket( active_recordings: Arc, source_addr: SocketAddr, credential_store: crate::credential::CredentialStoreHandle, - kerberos_session_store: crate::credential_injection_kdc::CredentialInjectionKdcSessionStoreHandle, agent_tunnel_handle: Option>, ) { let (stream, close_handle) = crate::ws::handle( @@ -88,7 +85,6 @@ async fn handle_socket( subscriber_tx, &active_recordings, &credential_store, - &kerberos_session_store, agent_tunnel_handle, ) .await; diff --git a/devolutions-gateway/src/credential/mod.rs b/devolutions-gateway/src/credential/mod.rs index fea6829d6..166be9412 100644 --- a/devolutions-gateway/src/credential/mod.rs +++ b/devolutions-gateway/src/credential/mod.rs @@ -4,16 +4,40 @@ mod crypto; pub use crypto::EncryptedPassword; use std::collections::HashMap; +use std::fmt; use std::sync::Arc; +use anyhow::Context; use async_trait::async_trait; use devolutions_gateway_task::{ShutdownSignal, Task}; use parking_lot::Mutex; -use secrecy::{ExposeSecret as _, SecretString}; +use secrecy::ExposeSecret as _; use uuid::Uuid; use self::crypto::MASTER_KEY; +/// Error returned by [`CredentialStoreHandle::insert`]. +#[derive(Debug)] +pub enum InsertError { + /// The provided token is invalid (e.g., missing or malformed JTI). + /// + /// This is a client-side error: the caller supplied bad input. + InvalidToken(anyhow::Error), + /// An internal error occurred (e.g., encryption failure). + Internal(anyhow::Error), +} + +impl fmt::Display for InsertError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::InvalidToken(e) => e.fmt(f), + Self::Internal(e) => e.fmt(f), + } + } +} + +impl std::error::Error for InsertError {} + /// Credential at the application protocol level #[derive(Debug, Clone)] pub enum AppCredential { @@ -24,16 +48,10 @@ pub enum AppCredential { } impl AppCredential { - pub(crate) fn username(&self) -> &str { - match self { - AppCredential::UsernamePassword { username, password: _ } => username, - } - } - /// Decrypt the password using the global master key. /// /// Returns the username and a short-lived decrypted password that zeroizes on drop. - pub fn decrypt_password(&self) -> anyhow::Result<(String, SecretString)> { + pub fn decrypt_password(&self) -> anyhow::Result<(String, secrecy::SecretString)> { match self { AppCredential::UsernamePassword { username, password } => { let decrypted = MASTER_KEY.lock().decrypt(password)?; @@ -56,9 +74,12 @@ pub struct AppCredentialMapping { /// This type is never stored directly — hand it to [`CredentialStoreHandle::insert`]. #[derive(Debug, Deserialize)] #[serde(tag = "kind")] -pub(crate) enum CleartextAppCredential { +pub enum CleartextAppCredential { #[serde(rename = "username-password")] - UsernamePassword { username: String, password: SecretString }, + UsernamePassword { + username: String, + password: secrecy::SecretString, + }, } impl CleartextAppCredential { @@ -79,30 +100,20 @@ impl CleartextAppCredential { /// /// Passwords are encrypted on write. Hand this directly to [`CredentialStoreHandle::insert`]. #[derive(Debug, Deserialize)] -pub(crate) struct CleartextAppCredentialMapping { +pub struct CleartextAppCredentialMapping { #[serde(rename = "proxy_credential")] - pub(crate) proxy: CleartextAppCredential, + pub proxy: CleartextAppCredential, #[serde(rename = "target_credential")] - pub(crate) target: CleartextAppCredential, + pub target: CleartextAppCredential, } impl CleartextAppCredentialMapping { - pub(crate) fn encrypt(self) -> anyhow::Result { + fn encrypt(self) -> anyhow::Result { Ok(AppCredentialMapping { proxy: self.proxy.encrypt()?, target: self.target.encrypt()?, }) } - - /// Expose the proxy-side username without dropping ownership of the rest of the mapping. - /// - /// Used by the preflight layer to derive per-session Kerberos material before the mapping - /// is moved into the credential store. The password is not exposed. - pub(crate) fn proxy_username(&self) -> &str { - match &self.proxy { - CleartextAppCredential::UsernamePassword { username, password: _ } => username, - } - } } #[derive(Debug, Clone)] @@ -119,32 +130,21 @@ impl CredentialStoreHandle { Self(Arc::new(Mutex::new(CredentialStore::new()))) } - /// Insert a credential entry for the association token whose JTI is `jti`. - /// - /// The caller is responsible for extracting `jti` from a token whose signature has already - /// been validated upstream. `injection` carries the optional `provision-credentials` payload: - /// the cleartext mapping and the parsed RDP target hostname. The store has no other - /// dependency on the token's JWT shape. - pub(crate) fn insert( + pub fn insert( &self, - jti: Uuid, token: String, - injection: Option<(CleartextAppCredentialMapping, String)>, + mapping: Option, time_to_live: time::Duration, - ) -> anyhow::Result> { - let injection = injection - .map(|(mapping, target_hostname)| -> anyhow::Result { - Ok(InjectionState { - mapping: mapping.encrypt()?, - target_hostname, - }) - }) - .transpose()?; - Ok(self.0.lock().insert(jti, token, injection, time_to_live)) + ) -> Result, InsertError> { + let mapping = mapping + .map(CleartextAppCredentialMapping::encrypt) + .transpose() + .map_err(InsertError::Internal)?; + self.0.lock().insert(token, mapping, time_to_live) } - pub(crate) fn get(&self, jti: Uuid) -> Option { - self.0.lock().get(jti) + pub fn get(&self, token_id: Uuid) -> Option { + self.0.lock().get(token_id) } } @@ -154,31 +154,13 @@ struct CredentialStore { } #[derive(Debug)] -pub(crate) struct CredentialEntry { - /// The association token's JTI. Doubles as the credential-store key and as the value the - /// matching KDC token's `jet_cred_id` claim points back to. - pub(crate) jti: Uuid, - pub(crate) token: String, - pub(crate) expires_at: time::OffsetDateTime, - /// Credential-injection state for this entry, set when (and only when) the entry was - /// provisioned via `provision-credentials`. Plain `provision-token` entries leave this `None` - /// and are inert from the routing layer's perspective. Grouping the three correlated fields - /// into one option captures the "all set or all unset" invariant in the type system. - pub(crate) injection: Option, -} - -#[derive(Debug)] -pub(crate) struct InjectionState { - pub(crate) mapping: AppCredentialMapping, - /// Hostname of the target RDP server, parsed from the association token's `dst_hst` claim - /// at preflight. Fake-KDC validates client TGS-REQ sname against `TERMSRV/`; - /// without it the SPN check would silently fall back to Gateway's own hostname, so the - /// preflight handler rejects `provision-credentials` requests with missing/malformed `dst_hst` - /// or with alternate targets (`dst_alt`) until credential injection supports target failover. - pub(crate) target_hostname: String, +pub struct CredentialEntry { + pub token: String, + pub mapping: Option, + pub expires_at: time::OffsetDateTime, } -pub(crate) type ArcCredentialEntry = Arc; +pub type ArcCredentialEntry = Arc; impl CredentialStore { fn new() -> Self { @@ -189,29 +171,27 @@ impl CredentialStore { fn insert( &mut self, - jti: Uuid, token: String, - injection: Option, + mapping: Option, time_to_live: time::Duration, - ) -> Option { + ) -> Result, InsertError> { + let jti = crate::token::extract_jti(&token) + .context("failed to extract token ID") + .map_err(InsertError::InvalidToken)?; + let entry = CredentialEntry { - jti, token, + mapping, expires_at: time::OffsetDateTime::now_utc() + time_to_live, - injection, }; - self.entries.insert(jti, Arc::new(entry)) + let previous_entry = self.entries.insert(jti, Arc::new(entry)); + + Ok(previous_entry) } - fn get(&self, jti: Uuid) -> Option { - // Filter expired entries at lookup. The 15-minute cleanup task is best-effort; - // without this filter an expired entry remains usable until the next sweep, which - // makes `time_to_live` a soft hint rather than a hard limit. - self.entries - .get(&jti) - .filter(|entry| time::OffsetDateTime::now_utc() < entry.expires_at) - .map(Arc::clone) + fn get(&self, token_id: Uuid) -> Option { + self.entries.get(&token_id).map(Arc::clone) } } @@ -254,6 +234,3 @@ async fn cleanup_task(handle: CredentialStoreHandle, mut shutdown_signal: Shutdo debug!("Task terminated"); } - -#[cfg(test)] -mod tests; diff --git a/devolutions-gateway/src/credential/tests.rs b/devolutions-gateway/src/credential/tests.rs deleted file mode 100644 index f970db617..000000000 --- a/devolutions-gateway/src/credential/tests.rs +++ /dev/null @@ -1,131 +0,0 @@ -use super::*; - -fn cleartext_mapping(proxy_username: &str) -> CleartextAppCredentialMapping { - CleartextAppCredentialMapping { - proxy: CleartextAppCredential::UsernamePassword { - username: proxy_username.to_owned(), - password: SecretString::from("proxy-password"), - }, - target: CleartextAppCredential::UsernamePassword { - username: "target@example.invalid".to_owned(), - password: SecretString::from("target-password"), - }, - } -} - -#[test] -fn insert_indexes_by_jti() { - let store = CredentialStoreHandle::new(); - let jti = Uuid::new_v4(); - - let previous = store - .insert( - jti, - "raw-token".to_owned(), - Some((cleartext_mapping("proxy@example.invalid"), "target.example".to_owned())), - time::Duration::minutes(5), - ) - .expect("insert succeeds"); - - assert!(previous.is_none()); - let entry = store.get(jti).expect("entry is indexed by JTI"); - assert_eq!(entry.jti, jti); - assert_eq!(entry.token, "raw-token"); - assert_eq!( - entry - .injection - .as_ref() - .expect("injection state present") - .target_hostname, - "target.example" - ); -} - -#[test] -fn re_insert_under_same_jti_replaces_entry() { - let store = CredentialStoreHandle::new(); - let jti = Uuid::new_v4(); - - store - .insert( - jti, - "token-a".to_owned(), - Some((cleartext_mapping("proxy@example.invalid"), "host-a".to_owned())), - time::Duration::minutes(5), - ) - .expect("first insert succeeds"); - let previous = store - .insert( - jti, - "token-b".to_owned(), - Some((cleartext_mapping("proxy@example.invalid"), "host-b".to_owned())), - time::Duration::minutes(5), - ) - .expect("second insert succeeds"); - - let previous = previous.expect("replacement must report previous entry"); - assert_eq!(previous.jti, jti); - assert_eq!(previous.token, "token-a"); - - let current = store.get(jti).expect("replacement entry present"); - assert_eq!(current.token, "token-b"); -} - -#[test] -fn distinct_jtis_coexist() { - let store = CredentialStoreHandle::new(); - let first_jti = Uuid::new_v4(); - let second_jti = Uuid::new_v4(); - - store - .insert( - first_jti, - "token-1".to_owned(), - Some((cleartext_mapping("proxy@example.invalid"), "h1".to_owned())), - time::Duration::minutes(5), - ) - .expect("first insert succeeds"); - store - .insert( - second_jti, - "token-2".to_owned(), - Some((cleartext_mapping("proxy@example.invalid"), "h2".to_owned())), - time::Duration::minutes(5), - ) - .expect("second insert succeeds"); - - assert_eq!(store.get(first_jti).expect("first entry").jti, first_jti); - assert_eq!(store.get(second_jti).expect("second entry").jti, second_jti); -} - -#[test] -fn lookup_filters_expired_entries() { - let store = CredentialStoreHandle::new(); - let jti = Uuid::new_v4(); - - store - .insert( - jti, - "raw-token".to_owned(), - Some((cleartext_mapping("proxy@example.invalid"), "host".to_owned())), - // Negative TTL: entry is born already expired. Validates the lookup-time filter - // without depending on real elapsed time in tests. - time::Duration::seconds(-1), - ) - .expect("insert succeeds"); - - assert!(store.get(jti).is_none(), "expired entry must not be returned"); -} - -#[test] -fn provision_token_entry_has_no_injection_state() { - let store = CredentialStoreHandle::new(); - let jti = Uuid::new_v4(); - - store - .insert(jti, "raw-token".to_owned(), None, time::Duration::minutes(5)) - .expect("insert succeeds"); - - let entry = store.get(jti).expect("entry exists"); - assert!(entry.injection.is_none()); -} diff --git a/devolutions-gateway/src/credential_injection_kdc.rs b/devolutions-gateway/src/credential_injection_kdc.rs index aead87ade..be1f0782e 100644 --- a/devolutions-gateway/src/credential_injection_kdc.rs +++ b/devolutions-gateway/src/credential_injection_kdc.rs @@ -26,9 +26,8 @@ use thiserror::Error; use url::Url; use uuid::Uuid; -use crate::credential::{ - AppCredential, AppCredentialMapping, ArcCredentialEntry, CredentialStoreHandle, InjectionState, -}; +use crate::credential::{AppCredential, AppCredentialMapping, ArcCredentialEntry, CredentialStoreHandle}; +use crate::target_addr::TargetAddr; const IN_PROCESS_KDC_HOST: &str = "cred.invalid"; @@ -51,8 +50,12 @@ pub(crate) enum CredentialInjectionKdcResolveError { MissingCredential { jti: Uuid }, #[error("credential-injection state is not available for {jti}")] NonInjectionCredential { jti: Uuid }, - #[error("credential-injection Kerberos state is not available for {jti}")] - MissingKerberosSession { 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, @@ -129,29 +132,42 @@ impl fmt::Debug for CredentialInjectionKdc { } impl CredentialInjectionKdc { - pub(crate) fn from_parts( + pub(crate) fn from_entry(jti: Uuid, credential_entry: ArcCredentialEntry) -> anyhow::Result { + let target_hostname = target_hostname_from_token(&credential_entry.token)?; + let mapping = credential_entry + .mapping + .as_ref() + .context("credential entry has no credential-injection mapping")?; + let session = Arc::new(derive_credential_injection_kdc_session( + app_credential_username(&mapping.proxy), + jti, + )); + + Self::from_parts(jti, credential_entry, target_hostname, session) + } + + fn from_parts( + jti: Uuid, credential_entry: ArcCredentialEntry, + target_hostname: String, session: Arc, ) -> anyhow::Result { - let InjectionState { - mapping, - target_hostname, - } = credential_entry - .injection + let mapping = credential_entry + .mapping .as_ref() - .context("credential entry has no credential-injection state")?; + .context("credential entry has no credential-injection mapping")?; anyhow::ensure!( - credential_entry.jti == session.jti, + 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: credential_entry.jti, + jti, raw_token: credential_entry.token.clone(), credential_mapping: mapping.clone(), - target_hostname: target_hostname.clone(), + target_hostname, session, kdc_config, }) @@ -171,17 +187,19 @@ impl CredentialInjectionKdc { CredentialInjectionKdcResolveError::MissingCredential { jti } })?; - if credential_entry.injection.is_none() { + let Some(mapping) = credential_entry.mapping.as_ref() else { warn!(%jti, "KDC token references non-injection credential state"); return Err(CredentialInjectionKdcResolveError::NonInjectionCredential { jti }); - } + }; - let session = session_store.get(jti).ok_or_else(|| { - warn!(%jti, "KDC token references missing credential-injection Kerberos state"); - CredentialInjectionKdcResolveError::MissingKerberosSession { jti } - })?; + let target_hostname = target_hostname_from_token(&credential_entry.token) + .map_err(|source| CredentialInjectionKdcResolveError::InvalidAssociationToken { jti, source })?; + let proxy_username = app_credential_username(&mapping.proxy).to_owned(); + let session = session_store.get_or_insert_with(jti, credential_entry.expires_at, || { + derive_credential_injection_kdc_session(&proxy_username, jti) + }); - let kdc = Self::from_parts(credential_entry, session) + let kdc = Self::from_parts(jti, credential_entry, target_hostname, session) .map_err(|source| CredentialInjectionKdcResolveError::BuildKdcConfig { jti, source })?; Ok(Some(Box::new(kdc))) @@ -209,8 +227,8 @@ impl CredentialInjectionKdc { /// 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(self.target_credential().username()).context("invalid target credential username")?; + 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) @@ -321,6 +339,31 @@ impl CredentialInjectionKdc { } } +fn target_hostname_from_token(token: &str) -> anyhow::Result { + const DEFAULT_DST_PORT: u16 = 3389; + + let dst_alt = crate::token::extract_dst_alt(token).context("read dst_alt from association token")?; + anyhow::ensure!( + dst_alt.is_empty(), + "association token dst_alt is not supported for credential injection", + ); + + let raw_dst_hst = crate::token::extract_dst_hst(token) + .context("read dst_hst from association token")? + .context("association token has no dst_hst, required for credential injection")?; + + Ok(TargetAddr::parse(&raw_dst_hst, DEFAULT_DST_PORT) + .context("parse dst_hst as target address")? + .host() + .to_owned()) +} + +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 @@ -500,11 +543,11 @@ fn random_32_bytes() -> Vec { bytes } -/// Parallel store of [`CredentialInjectionKdcSession`] entries keyed by association-token JTI. +/// Lazy store of [`CredentialInjectionKdcSession`] entries keyed by association-token JTI. /// -/// Pairs 1:1 with [`crate::credential::CredentialStoreHandle`]: a credential entry provisioned -/// with `provision-credentials` has a matching entry here under the same JTI. The two stores -/// share neither lock nor map so that the credential store stays Kerberos-unaware. +/// Sessions are created on first KDC-proxy use from the credential entry and its original +/// association token. The stores share neither lock nor map so that the credential store stays +/// Kerberos-unaware. #[derive(Debug, Clone, Default)] pub struct CredentialInjectionKdcSessionStoreHandle(Arc>>); @@ -519,6 +562,7 @@ impl CredentialInjectionKdcSessionStoreHandle { Self(Arc::new(Mutex::new(HashMap::new()))) } + #[cfg(test)] pub(crate) fn insert(&self, session: CredentialInjectionKdcSession, time_to_live: time::Duration) { let jti = session.jti; let entry = Entry { @@ -528,6 +572,7 @@ impl CredentialInjectionKdcSessionStoreHandle { self.0.lock().insert(jti, entry); } + #[cfg(test)] pub(crate) fn get(&self, jti: Uuid) -> Option> { // Lookup-time TTL enforcement mirrors `CredentialStoreHandle::get`: the cleanup task is // best-effort, and we don't want to hand out Kerberos material whose paired credential @@ -538,6 +583,30 @@ impl CredentialInjectionKdcSessionStoreHandle { .filter(|entry| time::OffsetDateTime::now_utc() < entry.expires_at) .map(|entry| Arc::clone(&entry.session)) } + + pub(crate) fn get_or_insert_with( + &self, + jti: Uuid, + expires_at: time::OffsetDateTime, + make_session: impl FnOnce() -> CredentialInjectionKdcSession, + ) -> Arc { + let now = time::OffsetDateTime::now_utc(); + let mut entries = self.0.lock(); + + if let Some(entry) = entries.get(&jti).filter(|entry| now < entry.expires_at) { + return Arc::clone(&entry.session); + } + + let session = Arc::new(make_session()); + entries.insert( + jti, + Entry { + session: Arc::clone(&session), + expires_at, + }, + ); + session + } } pub struct CleanupTask { @@ -581,13 +650,12 @@ async fn cleanup_task(handle: CredentialInjectionKdcSessionStoreHandle, mut shut #[cfg(test)] mod tests { + use base64::Engine as _; use ironrdp_connector::sspi::network_client::NetworkProtocol; use secrecy::SecretString; use super::*; - use crate::credential::{ - AppCredentialMapping, CleartextAppCredential, CleartextAppCredentialMapping, CredentialEntry, - }; + use crate::credential::{CleartextAppCredential, CleartextAppCredentialMapping}; fn cleartext_mapping_with_target_username(target_username: &str) -> CleartextAppCredentialMapping { CleartextAppCredentialMapping { @@ -602,19 +670,32 @@ mod tests { } } + 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) -> ArcCredentialEntry { - let mapping: AppCredentialMapping = cleartext_mapping_with_target_username(target_username) - .encrypt() - .expect("encrypt mapping"); - Arc::new(CredentialEntry { - jti, - token: "raw-token".to_owned(), - expires_at: time::OffsetDateTime::now_utc() + time::Duration::minutes(5), - injection: Some(InjectionState { - mapping, - target_hostname: "target.example".to_owned(), - }), - }) + let store = CredentialStoreHandle::new(); + store + .insert( + 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) -> ArcCredentialEntry { @@ -624,13 +705,15 @@ mod tests { 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(entry, session).expect("valid credential-injection KDC") + 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(entry, session).expect("valid credential-injection KDC") + CredentialInjectionKdc::from_parts(jti, entry, "target.example".to_owned(), session) + .expect("valid credential-injection KDC") } fn network_request(url: &str) -> NetworkRequest { @@ -731,7 +814,7 @@ mod tests { session_jti, )); - let err = CredentialInjectionKdc::from_parts(entry, session) + 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!( @@ -758,7 +841,7 @@ mod tests { let jti = Uuid::new_v4(); credential_store - .insert(jti, "raw-token".to_owned(), None, time::Duration::minutes(5)) + .insert(association_token(jti), None, time::Duration::minutes(5)) .expect("provision-token entry inserts"); assert!( diff --git a/devolutions-gateway/src/generic_client.rs b/devolutions-gateway/src/generic_client.rs index 7966b946e..117eb41bf 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::CredentialStoreHandle; -use crate::credential_injection_kdc::{CredentialInjectionKdc, CredentialInjectionKdcSessionStoreHandle}; +use crate::credential_injection_kdc::CredentialInjectionKdc; use crate::proxy::Proxy; use crate::rdp_pcb::{extract_association_claims, read_pcb}; use crate::recording::ActiveRecordings; @@ -29,7 +29,6 @@ pub struct GenericClient { subscriber_tx: SubscriberSender, active_recordings: Arc, credential_store: CredentialStoreHandle, - kerberos_session_store: CredentialInjectionKdcSessionStoreHandle, #[builder(default)] agent_tunnel_handle: Option>, } @@ -54,7 +53,6 @@ where subscriber_tx, active_recordings, credential_store, - kerberos_session_store, agent_tunnel_handle, } = self; @@ -156,11 +154,10 @@ where // lookup by `claims.jti` is the primary path. if is_rdp && let Some(entry) = credential_store.get(claims.jti) - && entry.injection.is_some() - && let Some(kdc_session) = kerberos_session_store.get(claims.jti) + && entry.mapping.is_some() { anyhow::ensure!(token == entry.token, "token mismatch"); - let credential_injection_kdc = CredentialInjectionKdc::from_parts(entry, kdc_session)?; + let credential_injection_kdc = CredentialInjectionKdc::from_entry(claims.jti, entry)?; info!( jti = %credential_injection_kdc.jti(), diff --git a/devolutions-gateway/src/listener.rs b/devolutions-gateway/src/listener.rs index 5898b738a..0b7ce2740 100644 --- a/devolutions-gateway/src/listener.rs +++ b/devolutions-gateway/src/listener.rs @@ -159,7 +159,6 @@ async fn handle_tcp_peer(stream: TcpStream, state: DgwState, peer_addr: SocketAd .subscriber_tx(state.subscriber_tx) .active_recordings(state.recordings.active_recordings) .credential_store(state.credential_store) - .kerberos_session_store(state.kerberos_session_store) .agent_tunnel_handle(state.agent_tunnel_handle) .build() .serve() diff --git a/devolutions-gateway/src/ngrok.rs b/devolutions-gateway/src/ngrok.rs index 58115db89..71c0c005f 100644 --- a/devolutions-gateway/src/ngrok.rs +++ b/devolutions-gateway/src/ngrok.rs @@ -238,7 +238,6 @@ async fn run_tcp_tunnel(mut tunnel: ngrok::tunnel::TcpTunnel, state: DgwState) { .subscriber_tx(state.subscriber_tx) .active_recordings(state.recordings.active_recordings) .credential_store(state.credential_store) - .kerberos_session_store(state.kerberos_session_store) .agent_tunnel_handle(state.agent_tunnel_handle) .build() .serve() diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index 3f790974e..ce65b86f5 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -12,7 +12,7 @@ use tracing::field; use crate::config::Conf; use crate::credential::CredentialStoreHandle; -use crate::credential_injection_kdc::{CredentialInjectionKdc, CredentialInjectionKdcSessionStoreHandle}; +use crate::credential_injection_kdc::CredentialInjectionKdc; use crate::proxy::Proxy; use crate::recording::ActiveRecordings; use crate::session::{ConnectionModeDetails, DisconnectInterest, DisconnectedInfo, SessionInfo, SessionMessageSender}; @@ -522,7 +522,6 @@ pub async fn handle( subscriber_tx: SubscriberSender, active_recordings: &ActiveRecordings, credential_store: &CredentialStoreHandle, - kerberos_session_store: &CredentialInjectionKdcSessionStoreHandle, agent_tunnel_handle: Option>, ) -> anyhow::Result<()> { // Special handshake of our RDP extension @@ -541,13 +540,11 @@ 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, - // and the per-session Kerberos material lives in a parallel store under the same JTI. + // clean path procedure. The credential store is keyed on the association token's JTI. if let Some(credential_injection_kdc) = crate::token::extract_jti(token).ok().and_then(|jti| { let entry = credential_store.get(jti)?; - entry.injection.as_ref()?; - let kdc_session = kerberos_session_store.get(jti)?; - CredentialInjectionKdc::from_parts(entry, kdc_session).ok() + entry.mapping.as_ref()?; + CredentialInjectionKdc::from_entry(jti, entry).ok() }) { anyhow::ensure!(token == credential_injection_kdc.raw_token(), "token mismatch"); debug!( diff --git a/devolutions-gateway/src/rdp_proxy.rs b/devolutions-gateway/src/rdp_proxy.rs index 1ab4382f1..17a37e757 100644 --- a/devolutions-gateway/src/rdp_proxy.rs +++ b/devolutions-gateway/src/rdp_proxy.rs @@ -636,7 +636,7 @@ where /// Only handles real-network schemes (`tcp` / `udp`); credential-injection loopback requests /// are intercepted by [`CredentialInjectionKdc`] before reaching this function. /// -/// TODO(sspi-rs#NNN): when sspi-rs ships a pluggable KDC dispatcher API, the URL trick for +/// TODO(sspi-rs#664): when sspi-rs ships a pluggable KDC dispatcher API, the URL trick for /// credential injection goes away entirely and this helper can be inlined back into the /// CredSSP loops. async fn send_network_request(request: &NetworkRequest) -> anyhow::Result> { diff --git a/devolutions-gateway/src/token.rs b/devolutions-gateway/src/token.rs index 1e29b7471..60f1bc3ea 100644 --- a/devolutions-gateway/src/token.rs +++ b/devolutions-gateway/src/token.rs @@ -1236,9 +1236,9 @@ pub fn extract_session_id(token: &str) -> anyhow::Result { /// Extract the destination host claim (`dst_hst`) from an association token without verifying its /// signature. Returns `None` if the claim is missing. /// -/// Used by the credential store to remember the target server's hostname for a credential-injection -/// session, so that fake-KDC can validate the client's TGS-REQ sname against it (the SPN the client -/// actually requested is `TERMSRV/`, not Gateway's own hostname). +/// Used by the credential-injection KDC to validate the client's TGS-REQ sname against the target +/// server hostname (the SPN the client actually requested is `TERMSRV/`, not Gateway's own +/// hostname). pub fn extract_dst_hst(token: &str) -> anyhow::Result> { let payload = extract_payload(token)?; let Some(value) = payload.get("dst_hst") else { From e6b84895ba20140b01569c36cf18cc31a143764b Mon Sep 17 00:00:00 2001 From: irving ou Date: Fri, 15 May 2026 21:19:18 -0400 Subject: [PATCH 3/3] refactor(dgw): validate credential-injection token at preflight `provision-credentials` now validates the association token shape (JTI, dst_hst present, dst_alt empty) at preflight time and reserves a credential-injection context slot keyed on the token JTI. The Kerberos session and target hostname are derived lazily from the stored token on first CredSSP/KDC use, keeping `CredentialStore` strictly protocol-neutral while still failing fast on bad tokens. Renames `kerberos_session_store` to `credential_injection_context_store` to reflect that it now holds a per-session context (slot + lazy Kerberos session), not the session itself. --- devolutions-gateway/src/api/kdc_proxy.rs | 4 +- devolutions-gateway/src/api/preflight.rs | 40 ++- devolutions-gateway/src/api/rdp.rs | 4 + .../src/credential_injection_kdc.rs | 231 +++++++++++------- devolutions-gateway/src/generic_client.rs | 7 +- devolutions-gateway/src/lib.rs | 7 +- devolutions-gateway/src/listener.rs | 1 + devolutions-gateway/src/ngrok.rs | 1 + devolutions-gateway/src/rd_clean_path.rs | 14 +- devolutions-gateway/src/service.rs | 8 +- devolutions-gateway/src/token.rs | 33 +++ devolutions-gateway/tests/preflight.rs | 44 +++- 12 files changed, 282 insertions(+), 112 deletions(-) diff --git a/devolutions-gateway/src/api/kdc_proxy.rs b/devolutions-gateway/src/api/kdc_proxy.rs index eec557222..637ccaaa3 100644 --- a/devolutions-gateway/src/api/kdc_proxy.rs +++ b/devolutions-gateway/src/api/kdc_proxy.rs @@ -26,7 +26,7 @@ async fn kdc_proxy( State(DgwState { conf_handle, credential_store, - kerberos_session_store, + credential_injection_context_store, .. }): State, KdcToken(KdcTokenClaims { @@ -49,7 +49,7 @@ async fn kdc_proxy( enforce_credential_injection_enabled(jet_cred_id, conf.debug.enable_unstable)?; - match CredentialInjectionKdc::resolve(jet_cred_id, &credential_store, &kerberos_session_store) + match CredentialInjectionKdc::resolve(jet_cred_id, &credential_store, &credential_injection_context_store) .map_err(credential_injection_resolve_error)? { Some(kdc) => { diff --git a/devolutions-gateway/src/api/preflight.rs b/devolutions-gateway/src/api/preflight.rs index c8309face..bb0de6981 100644 --- a/devolutions-gateway/src/api/preflight.rs +++ b/devolutions-gateway/src/api/preflight.rs @@ -12,6 +12,7 @@ use uuid::Uuid; use crate::DgwState; use crate::config::Conf; use crate::credential::{CredentialStoreHandle, InsertError}; +use crate::credential_injection_kdc::CredentialInjectionKdcContextStoreHandle; use crate::extract::PreflightScope; use crate::http::HttpError; use crate::session::SessionMessageSender; @@ -196,6 +197,7 @@ pub(super) async fn post_preflight( conf_handle, sessions, credential_store, + credential_injection_context_store, .. }): State, _scope: PreflightScope, @@ -223,12 +225,21 @@ pub(super) async fn post_preflight( let conf = conf_handle.get_conf(); let sessions = sessions.clone(); let credential_store = credential_store.clone(); + let credential_injection_context_store = credential_injection_context_store.clone(); async move { let operation_id = operation.id; trace!(%operation.id, "Process preflight operation"); - if let Err(error) = handle_operation(operation, &outputs, &conf, &sessions, &credential_store).await + if let Err(error) = handle_operation( + operation, + &outputs, + &conf, + &sessions, + &credential_store, + &credential_injection_context_store, + ) + .await { outputs.push(PreflightOutput { operation_id, @@ -257,6 +268,7 @@ async fn handle_operation( conf: &Conf, sessions: &SessionMessageSender, credential_store: &CredentialStoreHandle, + credential_injection_context_store: &CredentialInjectionKdcContextStoreHandle, ) -> Result<(), PreflightError> { match operation.kind.as_str() { OP_GET_VERSION => outputs.push(PreflightOutput { @@ -310,6 +322,7 @@ async fn handle_operation( }); } OP_PROVISION_TOKEN | OP_PROVISION_CREDENTIALS => { + let is_provision_credentials = operation.kind.as_str() == OP_PROVISION_CREDENTIALS; let (token, time_to_live, mapping) = if operation.kind.as_str() == OP_PROVISION_TOKEN { let ProvisionTokenParams { token, time_to_live } = from_params(operation.params).map_err(PreflightError::invalid_params)?; @@ -337,6 +350,27 @@ async fn handle_operation( }); } + let credential_injection_jti = if is_provision_credentials { + Some( + crate::token::validate_credential_injection_association_token(&token) + .inspect_err(|error| { + warn!( + %operation.id, + error = format!("{error:#}"), + "Credential-injection token is not valid" + ) + }) + .map_err(|error| { + PreflightError::new( + PreflightAlertStatus::InvalidParams, + format!("invalid credential-injection token: {error:#}"), + ) + })?, + ) + } else { + None + }; + let previous_entry = credential_store .insert(token, mapping, time_to_live) .inspect_err(|error| warn!(%operation.id, error = format!("{error:#}"), "Failed to insert credentials")) @@ -350,6 +384,10 @@ async fn handle_operation( ), })?; + if let Some(jti) = credential_injection_jti { + credential_injection_context_store.register_provisioned_credentials(jti, time_to_live); + } + if previous_entry.is_some() { outputs.push(PreflightOutput { operation_id: operation.id, diff --git a/devolutions-gateway/src/api/rdp.rs b/devolutions-gateway/src/api/rdp.rs index 6129a776c..468d762ea 100644 --- a/devolutions-gateway/src/api/rdp.rs +++ b/devolutions-gateway/src/api/rdp.rs @@ -26,6 +26,7 @@ pub async fn handler( recordings, shutdown_signal, credential_store, + credential_injection_context_store, agent_tunnel_handle, .. }): State, @@ -47,6 +48,7 @@ pub async fn handler( recordings.active_recordings, source_addr, credential_store, + credential_injection_context_store, agent_tunnel_handle, ) .instrument(span) @@ -67,6 +69,7 @@ async fn handle_socket( active_recordings: Arc, source_addr: SocketAddr, credential_store: crate::credential::CredentialStoreHandle, + credential_injection_context_store: crate::credential_injection_kdc::CredentialInjectionKdcContextStoreHandle, agent_tunnel_handle: Option>, ) { let (stream, close_handle) = crate::ws::handle( @@ -85,6 +88,7 @@ async fn handle_socket( subscriber_tx, &active_recordings, &credential_store, + &credential_injection_context_store, agent_tunnel_handle, ) .await; diff --git a/devolutions-gateway/src/credential_injection_kdc.rs b/devolutions-gateway/src/credential_injection_kdc.rs index be1f0782e..9dfbf0634 100644 --- a/devolutions-gateway/src/credential_injection_kdc.rs +++ b/devolutions-gateway/src/credential_injection_kdc.rs @@ -27,7 +27,6 @@ use url::Url; use uuid::Uuid; use crate::credential::{AppCredential, AppCredentialMapping, ArcCredentialEntry, CredentialStoreHandle}; -use crate::target_addr::TargetAddr; const IN_PROCESS_KDC_HOST: &str = "cred.invalid"; @@ -50,6 +49,8 @@ pub(crate) enum CredentialInjectionKdcResolveError { MissingCredential { jti: Uuid }, #[error("credential-injection state is not available for {jti}")] NonInjectionCredential { jti: Uuid }, + #[error("credential-injection context is not available for {jti}")] + MissingContext { jti: Uuid }, #[error("association token for {jti} is not valid for credential injection")] InvalidAssociationToken { jti: Uuid, @@ -132,16 +133,21 @@ impl fmt::Debug for CredentialInjectionKdc { } impl CredentialInjectionKdc { - pub(crate) fn from_entry(jti: Uuid, credential_entry: ArcCredentialEntry) -> anyhow::Result { - let target_hostname = target_hostname_from_token(&credential_entry.token)?; + pub(crate) fn from_entry( + jti: Uuid, + credential_entry: ArcCredentialEntry, + context_store: &CredentialInjectionKdcContextStoreHandle, + ) -> anyhow::Result { let mapping = credential_entry .mapping .as_ref() .context("credential entry has no credential-injection mapping")?; - let session = Arc::new(derive_credential_injection_kdc_session( - app_credential_username(&mapping.proxy), - jti, - )); + let proxy_username = app_credential_username(&mapping.proxy).to_owned(); + let session = context_store + .get_or_insert_session(jti, || derive_credential_injection_kdc_session(&proxy_username, jti)) + .context("credential-injection context is not available")?; + let target_hostname = crate::token::extract_credential_injection_target_hostname(&credential_entry.token) + .context("extract credential-injection target hostname from association token")?; Self::from_parts(jti, credential_entry, target_hostname, session) } @@ -176,7 +182,7 @@ impl CredentialInjectionKdc { pub(crate) fn resolve( jet_cred_id: Option, credential_store: &CredentialStoreHandle, - session_store: &CredentialInjectionKdcSessionStoreHandle, + context_store: &CredentialInjectionKdcContextStoreHandle, ) -> Result { let Some(jti) = jet_cred_id else { return Ok(None); @@ -192,12 +198,22 @@ impl CredentialInjectionKdc { return Err(CredentialInjectionKdcResolveError::NonInjectionCredential { jti }); }; - let target_hostname = target_hostname_from_token(&credential_entry.token) - .map_err(|source| CredentialInjectionKdcResolveError::InvalidAssociationToken { jti, source })?; let proxy_username = app_credential_username(&mapping.proxy).to_owned(); - let session = session_store.get_or_insert_with(jti, credential_entry.expires_at, || { - derive_credential_injection_kdc_session(&proxy_username, jti) - }); + let session = context_store + .get_or_insert_session(jti, || derive_credential_injection_kdc_session(&proxy_username, jti)) + .ok_or_else(|| { + warn!(%jti, "KDC token references missing credential-injection context"); + CredentialInjectionKdcResolveError::MissingContext { jti } + })?; + let target_hostname = 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 kdc = Self::from_parts(jti, credential_entry, target_hostname, session) .map_err(|source| CredentialInjectionKdcResolveError::BuildKdcConfig { jti, source })?; @@ -339,25 +355,6 @@ impl CredentialInjectionKdc { } } -fn target_hostname_from_token(token: &str) -> anyhow::Result { - const DEFAULT_DST_PORT: u16 = 3389; - - let dst_alt = crate::token::extract_dst_alt(token).context("read dst_alt from association token")?; - anyhow::ensure!( - dst_alt.is_empty(), - "association token dst_alt is not supported for credential injection", - ); - - let raw_dst_hst = crate::token::extract_dst_hst(token) - .context("read dst_hst from association token")? - .context("association token has no dst_hst, required for credential injection")?; - - Ok(TargetAddr::parse(&raw_dst_hst, DEFAULT_DST_PORT) - .context("parse dst_hst as target address")? - .host() - .to_owned()) -} - fn app_credential_username(credential: &AppCredential) -> &str { match credential { AppCredential::UsernamePassword { username, password: _ } => username, @@ -409,9 +406,9 @@ fn realm_mismatch(expected: &str, actual: &str) -> Option { /// 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. -pub(crate) struct CredentialInjectionKdcSession { +struct CredentialInjectionKdcSession { jti: Uuid, - pub(crate) realm: String, + realm: String, kdc: CredentialInjectionKdcState, acceptor: CredentialInjectionAcceptorState, } @@ -460,10 +457,7 @@ impl fmt::Debug for CredentialInjectionAcceptorState { /// 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. -pub(crate) fn derive_credential_injection_kdc_session( - proxy_username: &str, - jti: Uuid, -) -> CredentialInjectionKdcSession { +fn derive_credential_injection_kdc_session(proxy_username: &str, jti: Uuid) -> CredentialInjectionKdcSession { let realm = proxy_username .split_once('@') .map(|(_, realm)| realm) @@ -533,7 +527,7 @@ fn kerberos_salt(realm: &str, principal: &str) -> String { format!("{}{local_name}", realm.to_ascii_uppercase()) } -pub(crate) fn synthetic_realm(jti: Uuid) -> String { +fn synthetic_realm(jti: Uuid) -> String { format!("CRED-{}.INVALID", jti.simple()).to_ascii_uppercase() } @@ -543,74 +537,59 @@ fn random_32_bytes() -> Vec { bytes } -/// Lazy store of [`CredentialInjectionKdcSession`] entries keyed by association-token JTI. +/// Store of credential-injection contexts keyed by association-token JTI. /// -/// Sessions are created on first KDC-proxy use from the credential entry and its original -/// association token. The stores share neither lock nor map so that the credential store stays -/// Kerberos-unaware. +/// Association tokens are validated at `provision-credentials` time, but target hostnames are +/// extracted lazily from the original token on first CredSSP/KDC use. +/// Kerberos sessions are also created lazily from the credential entry on first CredSSP/KDC use. +/// The store is separate from the credential store so that the credential store stays Kerberos-unaware. #[derive(Debug, Clone, Default)] -pub struct CredentialInjectionKdcSessionStoreHandle(Arc>>); +pub struct CredentialInjectionKdcContextStoreHandle(Arc>>); #[derive(Debug)] struct Entry { - session: Arc, + session: Option>, expires_at: time::OffsetDateTime, } -impl CredentialInjectionKdcSessionStoreHandle { +impl CredentialInjectionKdcContextStoreHandle { pub fn new() -> Self { Self(Arc::new(Mutex::new(HashMap::new()))) } - #[cfg(test)] - pub(crate) fn insert(&self, session: CredentialInjectionKdcSession, time_to_live: time::Duration) { - let jti = session.jti; + pub(crate) fn register_provisioned_credentials(&self, jti: Uuid, time_to_live: time::Duration) { let entry = Entry { - session: Arc::new(session), + session: None, expires_at: time::OffsetDateTime::now_utc() + time_to_live, }; self.0.lock().insert(jti, entry); } - #[cfg(test)] - pub(crate) fn get(&self, jti: Uuid) -> Option> { - // Lookup-time TTL enforcement mirrors `CredentialStoreHandle::get`: the cleanup task is - // best-effort, and we don't want to hand out Kerberos material whose paired credential - // entry has already expired. - self.0 - .lock() - .get(&jti) - .filter(|entry| time::OffsetDateTime::now_utc() < entry.expires_at) - .map(|entry| Arc::clone(&entry.session)) - } - - pub(crate) fn get_or_insert_with( + fn get_or_insert_session( &self, jti: Uuid, - expires_at: time::OffsetDateTime, make_session: impl FnOnce() -> CredentialInjectionKdcSession, - ) -> Arc { + ) -> Option> { let now = time::OffsetDateTime::now_utc(); let mut entries = self.0.lock(); - if let Some(entry) = entries.get(&jti).filter(|entry| now < entry.expires_at) { - return Arc::clone(&entry.session); - } + let entry = entries.get_mut(&jti).filter(|entry| now < entry.expires_at)?; - let session = Arc::new(make_session()); - entries.insert( - jti, - Entry { - session: Arc::clone(&session), - expires_at, - }, - ); - session + let session = match &entry.session { + Some(session) => Arc::clone(session), + None => { + let session = Arc::new(make_session()); + entry.session = Some(Arc::clone(&session)); + session + } + }; + + Some(session) } } pub struct CleanupTask { - pub handle: CredentialInjectionKdcSessionStoreHandle, + pub handle: CredentialInjectionKdcContextStoreHandle, } #[async_trait] @@ -626,7 +605,7 @@ impl Task for CleanupTask { } #[instrument(skip_all)] -async fn cleanup_task(handle: CredentialInjectionKdcSessionStoreHandle, mut shutdown_signal: ShutdownSignal) { +async fn cleanup_task(handle: CredentialInjectionKdcContextStoreHandle, mut shutdown_signal: ShutdownSignal) { use tokio::time::{Duration, sleep}; const TASK_INTERVAL: Duration = Duration::from_secs(60 * 15); // 15 minutes @@ -740,25 +719,37 @@ mod tests { #[test] fn store_lookup_filters_expired_entries() { - let store = CredentialInjectionKdcSessionStoreHandle::new(); + let store = CredentialInjectionKdcContextStoreHandle::new(); let jti = Uuid::new_v4(); - let session = derive_credential_injection_kdc_session("proxy@example.invalid", jti); // Negative TTL: entry is born already expired. - store.insert(session, time::Duration::seconds(-1)); + store.register_provisioned_credentials(jti, time::Duration::seconds(-1)); - assert!(store.get(jti).is_none(), "expired entry must not be returned"); + assert!( + store + .get_or_insert_session(jti, || derive_credential_injection_kdc_session( + "proxy@example.invalid", + jti + )) + .is_none(), + "expired entry must not be returned" + ); } #[test] fn store_returns_fresh_entry() { - let store = CredentialInjectionKdcSessionStoreHandle::new(); + let store = CredentialInjectionKdcContextStoreHandle::new(); let jti = Uuid::new_v4(); - let session = derive_credential_injection_kdc_session("proxy@example.invalid", jti); - store.insert(session, time::Duration::minutes(5)); + store.register_provisioned_credentials(jti, time::Duration::minutes(5)); + + let session = store + .get_or_insert_session(jti, || { + derive_credential_injection_kdc_session("proxy@example.invalid", jti) + }) + .expect("fresh entry returned"); - assert_eq!(store.get(jti).expect("fresh entry returned").realm, "example.invalid"); + assert_eq!(session.realm, "example.invalid"); } #[test] @@ -794,9 +785,9 @@ mod tests { #[test] fn resolve_with_no_jet_cred_id_forwards_to_real_kdc() { let credential_store = CredentialStoreHandle::new(); - let session_store = CredentialInjectionKdcSessionStoreHandle::new(); + let context_store = CredentialInjectionKdcContextStoreHandle::new(); - let dispatch = CredentialInjectionKdc::resolve(None, &credential_store, &session_store) + let dispatch = CredentialInjectionKdc::resolve(None, &credential_store, &context_store) .expect("plain KDC token should dispatch"); assert!(dispatch.is_none()); @@ -826,10 +817,10 @@ mod tests { #[test] fn resolve_with_missing_jet_cred_id_fails_closed() { let credential_store = CredentialStoreHandle::new(); - let session_store = CredentialInjectionKdcSessionStoreHandle::new(); + let context_store = CredentialInjectionKdcContextStoreHandle::new(); assert!( - CredentialInjectionKdc::resolve(Some(Uuid::new_v4()), &credential_store, &session_store).is_err(), + CredentialInjectionKdc::resolve(Some(Uuid::new_v4()), &credential_store, &context_store).is_err(), "KDC tokens with jet_cred_id must not fall back to real-KDC forwarding" ); } @@ -837,7 +828,7 @@ mod tests { #[test] fn resolve_with_non_injection_entry_fails_closed() { let credential_store = CredentialStoreHandle::new(); - let session_store = CredentialInjectionKdcSessionStoreHandle::new(); + let context_store = CredentialInjectionKdcContextStoreHandle::new(); let jti = Uuid::new_v4(); credential_store @@ -845,11 +836,56 @@ mod tests { .expect("provision-token entry inserts"); assert!( - CredentialInjectionKdc::resolve(Some(jti), &credential_store, &session_store).is_err(), + CredentialInjectionKdc::resolve(Some(jti), &credential_store, &context_store).is_err(), "KDC tokens with jet_cred_id must require provision-credentials state" ); } + #[test] + fn resolve_lazily_extracts_target_hostname_from_entry_token() { + let credential_store = CredentialStoreHandle::new(); + let context_store = CredentialInjectionKdcContextStoreHandle::new(); + let jti = Uuid::new_v4(); + + credential_store + .insert( + association_token(jti), + Some(cleartext_mapping_with_target_username("target")), + time::Duration::minutes(5), + ) + .expect("credential entry inserts"); + context_store.register_provisioned_credentials(jti, time::Duration::minutes(5)); + + let kdc = CredentialInjectionKdc::resolve(Some(jti), &credential_store, &context_store) + .expect("credential-injection KDC resolves") + .expect("credential-injection KDC is selected"); + + assert_eq!(kdc.target_hostname, "target.example"); + } + + #[test] + fn resolve_with_missing_context_fails_closed() { + let credential_store = CredentialStoreHandle::new(); + let context_store = CredentialInjectionKdcContextStoreHandle::new(); + let jti = Uuid::new_v4(); + + credential_store + .insert( + association_token(jti), + Some(cleartext_mapping_with_target_username("target")), + time::Duration::minutes(5), + ) + .expect("credential entry inserts"); + + assert!( + matches!( + CredentialInjectionKdc::resolve(Some(jti), &credential_store, &context_store), + Err(CredentialInjectionKdcResolveError::MissingContext { .. }) + ), + "KDC tokens with jet_cred_id must require provision-credentials context" + ); + } + #[test] fn intercept_ignores_non_loopback_host() { let jti = Uuid::new_v4(); @@ -915,4 +951,13 @@ mod tests { 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 117eb41bf..5ea8ee41d 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::CredentialStoreHandle; -use crate::credential_injection_kdc::CredentialInjectionKdc; +use crate::credential_injection_kdc::{CredentialInjectionKdc, CredentialInjectionKdcContextStoreHandle}; use crate::proxy::Proxy; use crate::rdp_pcb::{extract_association_claims, read_pcb}; use crate::recording::ActiveRecordings; @@ -29,6 +29,7 @@ pub struct GenericClient { subscriber_tx: SubscriberSender, active_recordings: Arc, credential_store: CredentialStoreHandle, + credential_injection_context_store: CredentialInjectionKdcContextStoreHandle, #[builder(default)] agent_tunnel_handle: Option>, } @@ -53,6 +54,7 @@ where subscriber_tx, active_recordings, credential_store, + credential_injection_context_store, agent_tunnel_handle, } = self; @@ -157,7 +159,8 @@ where && entry.mapping.is_some() { anyhow::ensure!(token == entry.token, "token mismatch"); - let credential_injection_kdc = CredentialInjectionKdc::from_entry(claims.jti, entry)?; + let credential_injection_kdc = + CredentialInjectionKdc::from_entry(claims.jti, entry, &credential_injection_context_store)?; info!( jti = %credential_injection_kdc.jti(), diff --git a/devolutions-gateway/src/lib.rs b/devolutions-gateway/src/lib.rs index 019252365..bebcd53a8 100644 --- a/devolutions-gateway/src/lib.rs +++ b/devolutions-gateway/src/lib.rs @@ -61,7 +61,7 @@ pub struct DgwState { pub recordings: recording::RecordingMessageSender, pub job_queue_handle: job_queue::JobQueueHandle, pub credential_store: credential::CredentialStoreHandle, - pub kerberos_session_store: credential_injection_kdc::CredentialInjectionKdcSessionStoreHandle, + pub credential_injection_context_store: credential_injection_kdc::CredentialInjectionKdcContextStoreHandle, pub monitoring_state: Arc, pub traffic_audit_handle: traffic_audit::TrafficAuditHandle, pub agent_tunnel_handle: Option>, @@ -90,7 +90,8 @@ impl DgwState { let (job_queue_handle, job_queue_rx) = job_queue::JobQueueHandle::new(); let (traffic_audit_handle, traffic_audit_rx) = traffic_audit::TrafficAuditHandle::new(); let credential_store = credential::CredentialStoreHandle::new(); - let kerberos_session_store = credential_injection_kdc::CredentialInjectionKdcSessionStoreHandle::new(); + let credential_injection_context_store = + credential_injection_kdc::CredentialInjectionKdcContextStoreHandle::new(); let monitoring_state = Arc::new(network_monitor::State::new(Arc::new(MockMonitorsCache))?); let state = Self { @@ -104,7 +105,7 @@ impl DgwState { job_queue_handle, traffic_audit_handle, credential_store, - kerberos_session_store, + credential_injection_context_store, monitoring_state, agent_tunnel_handle: None, }; diff --git a/devolutions-gateway/src/listener.rs b/devolutions-gateway/src/listener.rs index 0b7ce2740..288884ab4 100644 --- a/devolutions-gateway/src/listener.rs +++ b/devolutions-gateway/src/listener.rs @@ -159,6 +159,7 @@ async fn handle_tcp_peer(stream: TcpStream, state: DgwState, peer_addr: SocketAd .subscriber_tx(state.subscriber_tx) .active_recordings(state.recordings.active_recordings) .credential_store(state.credential_store) + .credential_injection_context_store(state.credential_injection_context_store) .agent_tunnel_handle(state.agent_tunnel_handle) .build() .serve() diff --git a/devolutions-gateway/src/ngrok.rs b/devolutions-gateway/src/ngrok.rs index 71c0c005f..2a1fd3167 100644 --- a/devolutions-gateway/src/ngrok.rs +++ b/devolutions-gateway/src/ngrok.rs @@ -238,6 +238,7 @@ async fn run_tcp_tunnel(mut tunnel: ngrok::tunnel::TcpTunnel, state: DgwState) { .subscriber_tx(state.subscriber_tx) .active_recordings(state.recordings.active_recordings) .credential_store(state.credential_store) + .credential_injection_context_store(state.credential_injection_context_store) .agent_tunnel_handle(state.agent_tunnel_handle) .build() .serve() diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index ce65b86f5..0ca951242 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -12,7 +12,7 @@ use tracing::field; use crate::config::Conf; use crate::credential::CredentialStoreHandle; -use crate::credential_injection_kdc::CredentialInjectionKdc; +use crate::credential_injection_kdc::{CredentialInjectionKdc, CredentialInjectionKdcContextStoreHandle}; use crate::proxy::Proxy; use crate::recording::ActiveRecordings; use crate::session::{ConnectionModeDetails, DisconnectInterest, DisconnectedInfo, SessionInfo, SessionMessageSender}; @@ -522,6 +522,7 @@ pub async fn handle( subscriber_tx: SubscriberSender, active_recordings: &ActiveRecordings, credential_store: &CredentialStoreHandle, + credential_injection_context_store: &CredentialInjectionKdcContextStoreHandle, agent_tunnel_handle: Option>, ) -> anyhow::Result<()> { // Special handshake of our RDP extension @@ -541,11 +542,12 @@ 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. - if let Some(credential_injection_kdc) = crate::token::extract_jti(token).ok().and_then(|jti| { - let entry = credential_store.get(jti)?; - entry.mapping.as_ref()?; - CredentialInjectionKdc::from_entry(jti, entry).ok() - }) { + if let Some(jti) = crate::token::extract_jti(token).ok() + && let Some(entry) = credential_store.get(jti) + && entry.mapping.is_some() + { + let credential_injection_kdc = + CredentialInjectionKdc::from_entry(jti, entry, credential_injection_context_store)?; anyhow::ensure!(token == credential_injection_kdc.raw_token(), "token mismatch"); debug!( jti = %credential_injection_kdc.jti(), diff --git a/devolutions-gateway/src/service.rs b/devolutions-gateway/src/service.rs index 47823b872..3cb1ee5c1 100644 --- a/devolutions-gateway/src/service.rs +++ b/devolutions-gateway/src/service.rs @@ -269,8 +269,8 @@ async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result { .context("failed to initialize traffic audit manager")?; let credential_store = CredentialStoreHandle::new(); - let kerberos_session_store = - devolutions_gateway::credential_injection_kdc::CredentialInjectionKdcSessionStoreHandle::new(); + let credential_injection_context_store = + devolutions_gateway::credential_injection_kdc::CredentialInjectionKdcContextStoreHandle::new(); let filesystem_monitor_config_cache = devolutions_gateway::api::monitoring::FilesystemConfigCache::new( config::get_data_dir().join("monitors_cache.json"), @@ -319,7 +319,7 @@ async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result { recordings: recording_manager_handle.clone(), job_queue_handle: job_queue_ctx.job_queue_handle.clone(), credential_store: credential_store.clone(), - kerberos_session_store: kerberos_session_store.clone(), + credential_injection_context_store: credential_injection_context_store.clone(), monitoring_state, traffic_audit_handle: traffic_audit_task.handle(), agent_tunnel_handle, @@ -359,7 +359,7 @@ async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result { }); tasks.register(devolutions_gateway::credential_injection_kdc::CleanupTask { - handle: kerberos_session_store, + handle: credential_injection_context_store, }); tasks.register(devolutions_log::LogDeleterTask::::new( diff --git a/devolutions-gateway/src/token.rs b/devolutions-gateway/src/token.rs index 60f1bc3ea..7d57cf0ff 100644 --- a/devolutions-gateway/src/token.rs +++ b/devolutions-gateway/src/token.rs @@ -26,6 +26,7 @@ pub const MAX_SUBKEY_TOKEN_VALIDITY_DURATION_SECS: i64 = 60 * 60 * 2; // 2 hours const LEEWAY_SECS: u16 = 60 * 5; // 5 minutes const RDP_MAX_REUSE_INTERVAL_SECS: i64 = 10; // 10 seconds const BRIDGE_TOKEN_MAX_TOKEN_VALIDITY_DURATION_SECS: i64 = 60 * 60 * 12; // 12 hours +const CREDENTIAL_INJECTION_DEFAULT_DST_PORT: u16 = 3389; /// This is the maximum number of reconnections allowed during the reconnection window. If the /// reconnection window (e.g.: 30 seconds) is over while the connection is still alive, the counter @@ -1263,6 +1264,38 @@ pub fn extract_dst_alt(token: &str) -> anyhow::Result> { .collect() } +/// Validate the association-token claims required by credential injection. +/// +/// This is intentionally a token-layer shape check only. +/// The credential-injection KDC still lazily extracts the target hostname from the original token +/// when it builds its per-session state. +pub fn validate_credential_injection_association_token(token: &str) -> anyhow::Result { + let jti = extract_jti(token).context("read jti from association token")?; + extract_credential_injection_target_hostname(token)?; + Ok(jti) +} + +/// Extract the target hostname used by credential injection from an association token. +/// +/// `dst_alt` is rejected for now because the Kerberos fake-KDC can validate only one target SPN for +/// the current credential-injection session. +pub fn extract_credential_injection_target_hostname(token: &str) -> anyhow::Result { + let dst_alt = extract_dst_alt(token).context("read dst_alt from association token")?; + anyhow::ensure!( + dst_alt.is_empty(), + "association token dst_alt is not supported for credential injection", + ); + + let raw_dst_hst = extract_dst_hst(token) + .context("read dst_hst from association token")? + .context("association token has no dst_hst, required for credential injection")?; + + Ok(TargetAddr::parse(&raw_dst_hst, CREDENTIAL_INJECTION_DEFAULT_DST_PORT) + .context("parse dst_hst as target address")? + .host() + .to_owned()) +} + #[deprecated = "make sure this is never used without a deliberate action"] pub mod unsafe_debug { // Any function in this module should only be used at development stage when deliberately diff --git a/devolutions-gateway/tests/preflight.rs b/devolutions-gateway/tests/preflight.rs index 1f78d3094..8246696ac 100644 --- a/devolutions-gateway/tests/preflight.rs +++ b/devolutions-gateway/tests/preflight.rs @@ -82,7 +82,7 @@ async fn test_provision_credentials_success() -> anyhow::Result<()> { let (app, _state, _handles) = make_router()?; // JWT payload includes `dst_hst` because credential injection requires a target hostname - // (fake-KDC validates TGS-REQ sname against `TERMSRV/`); insert rejects tokens without it. + // (fake-KDC validates TGS-REQ sname against `TERMSRV/`); preflight rejects tokens without it. let token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJqdGkiOiI1ZTNlODMzZi04NGM3LTQ1NDEtYjY3Ni1hY2MzMjk5ZTM5YjgiLCJkc3RfaHN0IjoidGFyZ2V0LmV4YW1wbGU6MzM4OSJ9.1qECGlrW7y9HWFArc6GPHLGTOY7PhAvzKJ5XMRBg4k4"; let op_id = Uuid::new_v4(); @@ -188,6 +188,48 @@ async fn test_provision_credentials_rejects_alternate_targets() -> anyhow::Resul Ok(()) } +#[tokio::test] +async fn test_provision_credentials_rejects_missing_target_hostname() -> anyhow::Result<()> { + let _guard = init_logger(); + + let (app, _state, _handles) = make_router()?; + + let token = unsigned_jws(json!({ + "jti": "5e3e833f-84c7-4541-b676-acc3299e39b8" + }))?; + + let op_id = Uuid::new_v4(); + + let op = json!([{ + "id": op_id, + "kind": "provision-credentials", + "token": token, + "proxy_credential": { "kind": "username-password", "username": "proxy_user", "password": "secret1" }, + "target_credential": { "kind": "username-password", "username": "target_user", "password": "secret2" }, + "time_to_live": 15 + }]); + + let response = app.oneshot(preflight_request(op)?).await?; + assert_eq!(response.status(), StatusCode::OK); + + let body = response.into_body().collect().await?.to_bytes(); + let body: serde_json::Value = serde_json::from_slice(&body)?; + + assert_eq!(body.as_array().expect("an array").len(), 1); + assert_eq!(body[0]["kind"], "alert"); + assert_eq!(body[0]["alert_status"], "invalid-parameters"); + assert!( + body[0]["alert_message"] + .as_str() + .expect("alert message") + .contains("dst_hst"), + "{:?}", + body[0] + ); + + Ok(()) +} + #[tokio::test] async fn test_provision_token_overwrite_alert() -> anyhow::Result<()> { let _guard = init_logger();