From 2f8b806cafe68d70d9d797c5aa1c1715267e7085 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sun, 5 Jul 2026 07:12:09 +0000 Subject: [PATCH 01/21] Add post-auth connection binder hook --- crates/ironrdp-server/src/builder.rs | 14 ++++++- crates/ironrdp-server/src/lib.rs | 6 +-- crates/ironrdp-server/src/server.rs | 57 +++++++++++++++++++++++++++- docs/wrdp/auth-delegation.md | 14 +++++++ 4 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 docs/wrdp/auth-delegation.md diff --git a/crates/ironrdp-server/src/builder.rs b/crates/ironrdp-server/src/builder.rs index 228507900..a0c8076fc 100644 --- a/crates/ironrdp-server/src/builder.rs +++ b/crates/ironrdp-server/src/builder.rs @@ -12,7 +12,9 @@ use super::display::{DesktopSize, RdpServerDisplay}; #[cfg(feature = "egfx")] use super::gfx::GfxServerFactory; use super::handler::{KeyboardEvent, MouseEvent, RdpServerInputHandler}; -use super::server::{ConnectionHandler, CredentialValidator, RdpServer, RdpServerOptions, RdpServerSecurity}; +use super::server::{ + ConnectionBinder, ConnectionHandler, CredentialValidator, RdpServer, RdpServerOptions, RdpServerSecurity, +}; use crate::{DisplayUpdate, RdpServerDisplayUpdates, SoundServerFactory}; pub struct WantsAddr {} @@ -39,6 +41,7 @@ pub struct BuilderDone { sound_factory: Option>, connection_handler: Option>, credential_validator: Option>, + connection_binder: Option>, #[cfg(feature = "egfx")] gfx_factory: Option>, display_suppressed: Option>, @@ -139,6 +142,7 @@ impl RdpServerBuilder { cliprdr_factory: None, connection_handler: None, credential_validator: None, + connection_binder: None, codecs: server_codecs_capabilities(&[]).expect("can't panic for &[]"), max_request_size: RdpServerOptions::DEFAULT_MAX_REQUEST_SIZE, #[cfg(feature = "egfx")] @@ -162,6 +166,7 @@ impl RdpServerBuilder { cliprdr_factory: None, connection_handler: None, credential_validator: None, + connection_binder: None, codecs: server_codecs_capabilities(&[]).expect("can't panic for &[]"), max_request_size: RdpServerOptions::DEFAULT_MAX_REQUEST_SIZE, #[cfg(feature = "egfx")] @@ -294,6 +299,12 @@ impl RdpServerBuilder { self } + /// Set a binder that replaces display/input handlers after credentials are accepted. + pub fn with_connection_binder(mut self, binder: Option>) -> Self { + self.state.connection_binder = binder; + self + } + /// Inject a shared NetworkAutoDetect RTT handle (milliseconds, `u32::MAX` /// until the first measurement). The server writes the latest measured RTT /// to the same instance the backend reads. When not called, the server @@ -344,6 +355,7 @@ impl RdpServerBuilder { ); server.set_credential_validator(self.state.credential_validator); server.set_auto_reconnect_cookie(self.state.auto_reconnect_cookie); + server.set_connection_binder(self.state.connection_binder); server } } diff --git a/crates/ironrdp-server/src/lib.rs b/crates/ironrdp-server/src/lib.rs index 112d4b235..7fc97fe92 100644 --- a/crates/ironrdp-server/src/lib.rs +++ b/crates/ironrdp-server/src/lib.rs @@ -34,9 +34,9 @@ pub use handler::{KeyboardEvent, MouseEvent, RdpServerInputHandler}; pub use helper::TlsIdentityCtx; pub use ironrdp_pdu::rdp::session_info::ServerAutoReconnect; pub use server::{ - AutoReconnectCookieHandle, ConnectionHandler, CredentialDecision, CredentialValidationError, CredentialValidator, - Credentials, ExactMatchCredentialValidator, PostConnectionAction, RdpServer, RdpServerOptions, RdpServerSecurity, - ServerEvent, ServerEventSender, TransportTls, + AutoReconnectCookieHandle, BoundConnection, ConnectionBinder, ConnectionHandler, CredentialDecision, + CredentialValidationError, CredentialValidator, Credentials, ExactMatchCredentialValidator, PostConnectionAction, + RdpServer, RdpServerOptions, RdpServerSecurity, ServerEvent, ServerEventSender, TransportTls, }; pub use sound::{RdpsndServerHandler, RdpsndServerMessage, SoundServerFactory}; diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index c79445875..df5e9ff51 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -192,6 +192,28 @@ pub trait CredentialValidator: Send + Sync { async fn validate(&self, credentials: &Credentials) -> Result; } +/// Display/input objects bound after a credential validator accepts a client. +/// +/// Servers with per-user desktop/session isolation can start with placeholder +/// display and input handlers, validate the client's credentials, and then +/// replace those placeholders with handlers attached to the authenticated +/// user's session before the RDP client loop starts. +pub struct BoundConnection { + pub display: Box, + pub input: Box, +} + +/// Async post-auth connection binder. +/// +/// This hook runs after [`CredentialValidator`] accepts credentials and before +/// static channels, display updates, or input dispatch begin. It lets a server +/// bind display/input resources to the authenticated identity without creating +/// per-user resources before authentication. +#[async_trait::async_trait] +pub trait ConnectionBinder: Send + Sync { + async fn bind_connection(&self, credentials: &Credentials) -> anyhow::Result; +} + /// A built-in [`CredentialValidator`] that accepts exactly one fixed set of credentials. /// /// This is the validation-policy equivalent of the acceptor's pre-loaded @@ -453,6 +475,7 @@ pub struct RdpServer { ev_receiver: Arc>>, creds: Option, credential_validator: Option>, + connection_binder: Option>, local_addr: Option, autodetect: Option, connection_handler: Option>, @@ -611,6 +634,7 @@ impl RdpServer { ev_receiver: Arc::new(Mutex::new(ev_receiver)), creds: None, credential_validator: None, + connection_binder: None, local_addr: None, autodetect: None, connection_handler, @@ -818,6 +842,15 @@ impl RdpServer { Ok(()) } + /// Set or clear a post-auth connection binder. + /// + /// When set, the binder is called after credentials have been validated. + /// The returned display/input handlers replace the server defaults for the + /// accepted connection. + pub fn set_connection_binder(&mut self, binder: Option>) { + self.connection_binder = binder; + } + pub fn event_sender(&self) -> &mpsc::UnboundedSender { &self.ev_sender } @@ -1625,11 +1658,14 @@ impl RdpServer { // async server layer, rather than in the sans-I/O acceptor, because real validators // (PAM/LDAP/DB) are I/O-bound. On rejection, deny with a ServerSetErrorInfoPdu before // closing, matching the acceptor's exact-match denial path. - if !is_auto_reconnect && let Some(validator) = self.credential_validator.clone() { + let authenticated_credentials = if is_auto_reconnect { + result.credentials.clone() + } else if let Some(validator) = self.credential_validator.clone() { if let Some(creds) = &result.credentials { match validator.validate(creds).await { Ok(CredentialDecision::Accept) => { debug!("Credential validation accepted"); + Some(creds.clone()) } Ok(CredentialDecision::Reject) => { warn!("Credential validation rejected"); @@ -1644,7 +1680,26 @@ impl RdpServer { } } else { debug!("Skipping credential validation (no credentials in AcceptorResult)"); + None } + } else { + result.credentials.clone() + }; + + if let Some(binder) = self.connection_binder.clone() { + let Some(credentials) = authenticated_credentials.as_ref() else { + warn!("Connection binder configured but no authenticated credentials are available"); + send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; + bail!("no authenticated credentials for connection binding"); + }; + + let bound = binder + .bind_connection(credentials) + .await + .context("connection binder failed")?; + *self.display.lock().await = bound.display; + *self.handler.lock().await = bound.input; + debug!("Connection binder installed display/input handlers"); } if !result.input_events.is_empty() { diff --git a/docs/wrdp/auth-delegation.md b/docs/wrdp/auth-delegation.md new file mode 100644 index 000000000..1f7f1cfb6 --- /dev/null +++ b/docs/wrdp/auth-delegation.md @@ -0,0 +1,14 @@ +# Post-auth connection binding for multi-user servers + +`wrdp` follows the same multi-user architecture model as `xrdp-sesman`: a +single public RDP listener authenticates the client first, then delegates the +connection to a per-user desktop/session stack. + +That model needs a server hook that runs after credentials have been accepted +but before display updates and input dispatch begin. The hook lets a server +start or locate the authenticated user's session and then replace placeholder +handlers with display/input handlers bound to that session. + +The `ConnectionBinder` API keeps protocol ownership inside IronRDP while +allowing downstream servers to keep user/session lifecycle code outside the RDP +state machine. From c76e44bc11dc2794b3a902a28747fc07aba268f2 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sun, 5 Jul 2026 07:16:53 +0000 Subject: [PATCH 02/21] Scope reactivation credential cache to connection --- crates/ironrdp-server/src/server.rs | 172 +++++++++++++++++---- docs/wrdp/reactivation-credential-cache.md | 10 ++ 2 files changed, 155 insertions(+), 27 deletions(-) create mode 100644 docs/wrdp/reactivation-credential-cache.md diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index df5e9ff51..40f783461 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -211,7 +211,7 @@ pub struct BoundConnection { /// per-user resources before authentication. #[async_trait::async_trait] pub trait ConnectionBinder: Send + Sync { - async fn bind_connection(&self, credentials: &Credentials) -> anyhow::Result; + async fn bind_connection(&self, credentials: &Credentials) -> Result; } /// A built-in [`CredentialValidator`] that accepts exactly one fixed set of credentials. @@ -1634,6 +1634,7 @@ impl RdpServer { reader: &mut Framed, writer: &mut Framed, result: AcceptorResult, + authenticated_credentials_cache: &mut Option, ) -> Result where R: FramedRead, @@ -1658,33 +1659,18 @@ impl RdpServer { // async server layer, rather than in the sans-I/O acceptor, because real validators // (PAM/LDAP/DB) are I/O-bound. On rejection, deny with a ServerSetErrorInfoPdu before // closing, matching the acceptor's exact-match denial path. - let authenticated_credentials = if is_auto_reconnect { - result.credentials.clone() - } else if let Some(validator) = self.credential_validator.clone() { - if let Some(creds) = &result.credentials { - match validator.validate(creds).await { - Ok(CredentialDecision::Accept) => { - debug!("Credential validation accepted"); - Some(creds.clone()) - } - Ok(CredentialDecision::Reject) => { - warn!("Credential validation rejected"); - send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; - bail!("credential validation rejected"); - } - Err(e) => { - error!(error = %e, "Credential validator backend error"); - send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; - bail!("credential validation backend error"); - } - } - } else { - debug!("Skipping credential validation (no credentials in AcceptorResult)"); - None - } + let credential_validator = if is_auto_reconnect { + None } else { - result.credentials.clone() + self.credential_validator.clone() }; + let authenticated_credentials = resolve_authenticated_credentials( + credential_validator, + result.credentials.as_ref(), + result.reactivation, + authenticated_credentials_cache, + ) + .await?; if let Some(binder) = self.connection_binder.clone() { let Some(credentials) = authenticated_credentials.as_ref() else { @@ -2061,6 +2047,8 @@ impl RdpServer { where S: AsyncRead + AsyncWrite + Sync + Send + Unpin, { + let mut authenticated_credentials_cache = None; + loop { let (new_framed, result) = ironrdp_acceptor::accept_finalize(framed, &mut acceptor) .await @@ -2068,7 +2056,10 @@ impl RdpServer { let (mut reader, mut writer) = split_tokio_framed(new_framed); - match self.client_accepted(&mut reader, &mut writer, result).await? { + match self + .client_accepted(&mut reader, &mut writer, result, &mut authenticated_credentials_cache) + .await? + { RunState::Continue => { unreachable!(); } @@ -2099,6 +2090,51 @@ impl RdpServer { } } +async fn resolve_authenticated_credentials( + credential_validator: Option>, + result_credentials: Option<&Credentials>, + reactivation: bool, + authenticated_credentials_cache: &mut Option, +) -> Result> { + if let Some(validator) = credential_validator { + if let Some(creds) = result_credentials { + match validator.validate(creds).await { + Ok(CredentialDecision::Accept) => { + debug!("Credential validation accepted"); + *authenticated_credentials_cache = Some(creds.clone()); + Ok(Some(creds.clone())) + } + Ok(CredentialDecision::Reject) => { + warn!("Credential validation rejected"); + bail!("credential validation rejected"); + } + Err(e) => { + error!(error = %e, "Credential validator backend error"); + bail!("credential validation backend error"); + } + } + } else if reactivation { + let credentials = authenticated_credentials_cache.clone(); + if credentials.is_some() { + debug!("Reusing cached authenticated credentials for reactivation"); + } else { + debug!("Skipping credential validation for reactivation without cached credentials"); + } + Ok(credentials) + } else { + debug!("Skipping credential validation (no credentials in AcceptorResult)"); + Ok(None) + } + } else if let Some(creds) = result_credentials { + *authenticated_credentials_cache = Some(creds.clone()); + Ok(Some(creds.clone())) + } else if reactivation { + Ok(authenticated_credentials_cache.clone()) + } else { + Ok(None) + } +} + /// Encode a server-initiated Auto-Detect Request PDU for the MCS message channel. /// /// The request is framed by a Basic Security Header (SEC_AUTODETECT_REQ) per @@ -2236,3 +2272,85 @@ impl<'a, W: FramedWrite> SharedWriter<'a, W> { } } } + + +#[cfg(test)] +mod wrdp_reactivation_tests { + use super::*; + + struct AllowUserValidator(&'static str); + + #[async_trait::async_trait] + impl CredentialValidator for AllowUserValidator { + async fn validate(&self, credentials: &Credentials) -> Result { + if credentials.username == self.0 { + Ok(CredentialDecision::Accept) + } else { + Ok(CredentialDecision::Reject) + } + } + } + + fn creds(username: &str) -> Credentials { + Credentials { + username: username.to_owned(), + password: "secret".to_owned(), + domain: None, + } + } + + #[tokio::test] + async fn reactivation_without_credentials_reuses_same_connection_validated_identity() { + let validator = Arc::new(AllowUserValidator("alice")); + let mut per_connection_cache = None; + + let first = resolve_authenticated_credentials( + Some(validator.clone()), + Some(&creds("alice")), + false, + &mut per_connection_cache, + ) + .await + .expect("initial validation should succeed") + .expect("initial validation should produce credentials"); + assert_eq!(first.username, "alice"); + + let reactivated = resolve_authenticated_credentials( + Some(validator), + None, + true, + &mut per_connection_cache, + ) + .await + .expect("reactivation should reuse same-connection cache") + .expect("reactivation should have cached credentials"); + assert_eq!(reactivated.username, "alice"); + } + + #[tokio::test] + async fn reactivation_without_credentials_cannot_use_previous_tcp_connection_cache() { + let validator = Arc::new(AllowUserValidator("alice")); + let mut first_connection_cache = None; + resolve_authenticated_credentials( + Some(validator.clone()), + Some(&creds("alice")), + false, + &mut first_connection_cache, + ) + .await + .expect("initial validation should succeed"); + assert!(first_connection_cache.is_some()); + + let mut second_connection_cache = None; + let reactivated = resolve_authenticated_credentials( + Some(validator), + None, + true, + &mut second_connection_cache, + ) + .await + .expect("missing same-connection cache is not a backend error"); + assert!(reactivated.is_none()); + assert!(second_connection_cache.is_none()); + } +} diff --git a/docs/wrdp/reactivation-credential-cache.md b/docs/wrdp/reactivation-credential-cache.md new file mode 100644 index 000000000..2949076ac --- /dev/null +++ b/docs/wrdp/reactivation-credential-cache.md @@ -0,0 +1,10 @@ +# Reactivation credential cache scope + +During Deactivation-Reactivation some clients do not send a second credentials +PDU. A server that binds display/input handlers after authentication still needs +the identity accepted earlier on the same TCP connection. + +The cache introduced here is deliberately scoped to `accept_finalize()`, i.e. to +one TCP connection. It allows same-connection reactivation to reuse the validated +identity but prevents a new TCP connection from inheriting credentials accepted +on a previous connection. From 932a07528e95f44082c76632135c640e559f97a3 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sat, 4 Jul 2026 19:34:26 +0000 Subject: [PATCH 03/21] Expose CredSSP delegated credentials to servers --- crates/ironrdp-acceptor/src/connection.rs | 18 +++++++++++++++--- crates/ironrdp-acceptor/src/credssp.rs | 19 +++++++++++-------- crates/ironrdp-acceptor/src/lib.rs | 5 ++++- docs/wrdp/credssp-delegated-credentials.md | 14 ++++++++++++++ 4 files changed, 44 insertions(+), 12 deletions(-) create mode 100644 docs/wrdp/credssp-delegated-credentials.md diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index bcbed1517..541eb69ea 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -1,6 +1,7 @@ use core::any::TypeId; use core::mem; +use ironrdp_connector::sspi::AuthIdentity; use ironrdp_connector::{ ConnectorError, ConnectorErrorExt as _, ConnectorResult, DesktopSize, Sequence, State, Written, encode_x224_packet, general_err, reason_err, @@ -106,11 +107,11 @@ pub struct AcceptorResult { /// implement UDP multitransport can use it to decide whether to send a /// Server Initiate Multitransport Request. pub multitransport_flags: gcc::MultiTransportFlags, - /// Credentials received from the client during SecureSettingsExchange. + /// Credentials received from the client. /// /// Present for TLS-mode connections where the client sends credentials - /// in the ClientInfoPdu. `None` for CredSSP/Hybrid connections (where - /// authentication happens during the CredSSP exchange instead). + /// in the ClientInfoPdu, and for CredSSP/Hybrid connections once the + /// delegated TSPasswordCreds have been decrypted by CredSSP. /// /// Servers that need to validate credentials (e.g., via PAM or LDAP) /// can use this field for post-handshake validation. @@ -295,6 +296,17 @@ impl Acceptor { matches!(self.state, AcceptorState::Credssp { .. }) } + /// Store credentials delegated by CredSSP/NLA so server code can use the + /// same post-handshake validation and binding path as TLS ClientInfo + /// credentials. + pub(crate) fn set_received_credssp_credentials(&mut self, identity: AuthIdentity) { + self.received_credentials = Some(Credentials { + username: identity.username.account_name().to_owned(), + password: identity.password.as_ref().clone(), + domain: identity.username.domain_name().map(str::to_owned), + }); + } + /// # Panics /// /// Panics if state is not [AcceptorState::Credssp]. diff --git a/crates/ironrdp-acceptor/src/credssp.rs b/crates/ironrdp-acceptor/src/credssp.rs index e665724db..1be36eb09 100644 --- a/crates/ironrdp-acceptor/src/credssp.rs +++ b/crates/ironrdp-acceptor/src/credssp.rs @@ -153,18 +153,19 @@ impl<'a> CredsspSequence<'a> { &mut self, result: Result, output: &mut WriteBuf, - ) -> ConnectorResult { - let (ts_request, next_state) = match result { - Ok(ServerState::ReplyNeeded(ts_request)) => (Some(ts_request), CredsspState::Ongoing), - Ok(ServerState::Finished(_id)) => (None, CredsspState::Finished), + ) -> ConnectorResult<(Written, Option)> { + let (ts_request, next_state, credentials) = match result { + Ok(ServerState::ReplyNeeded(ts_request)) => (Some(ts_request), CredsspState::Ongoing, None), + Ok(ServerState::Finished(id)) => (None, CredsspState::Finished, Some(id)), Err(err) => ( err.ts_request.map(|ts_request| *ts_request), CredsspState::ServerError(err.error), + None, ), }; self.state = next_state; - if let Some(ts_request) = ts_request { + let written = if let Some(ts_request) = ts_request { debug!(?ts_request, "Send"); let length = usize::from(ts_request.buffer_len()); let unfilled_buffer = output.unfilled_to(length); @@ -175,9 +176,11 @@ impl<'a> CredsspSequence<'a> { output.advance(length); - Ok(Written::from_size(length)?) + Written::from_size(length)? } else { - Ok(Written::Nothing) - } + Written::Nothing + }; + + Ok((written, credentials)) } } diff --git a/crates/ironrdp-acceptor/src/lib.rs b/crates/ironrdp-acceptor/src/lib.rs index a8a709687..ba864fa6e 100644 --- a/crates/ironrdp-acceptor/src/lib.rs +++ b/crates/ironrdp-acceptor/src/lib.rs @@ -208,7 +208,10 @@ where }; // drop generator buf.clear(); - let written = sequence.handle_process_result(result, buf)?; + let (written, delegated_credentials) = sequence.handle_process_result(result, buf)?; + if let Some(credentials) = delegated_credentials { + acceptor.set_received_credssp_credentials(credentials); + } if let Some(response_len) = written.size() { let response = &buf[..response_len]; diff --git a/docs/wrdp/credssp-delegated-credentials.md b/docs/wrdp/credssp-delegated-credentials.md new file mode 100644 index 000000000..032020be6 --- /dev/null +++ b/docs/wrdp/credssp-delegated-credentials.md @@ -0,0 +1,14 @@ +# CredSSP delegated credentials handoff + +`ironrdp-acceptor` already exposes credentials sent later in the TLS +SecureSettingsExchange path. CredSSP/Hybrid authentication completes earlier, so +servers that delegate final account checks after the protocol handshake also need +access to the decrypted `TSPasswordCreds` produced by the CredSSP server state +machine. + +This change carries the delegated identity from the CredSSP sequence into +`AcceptorResult::credentials`, matching the existing ClientInfoPdu handoff shape. + +This is useful for servers that follow the xrdp-sesman multi-user architecture +model: the RDP protocol stack authenticates the transport, then the embedding +server delegates account authorization and session launch to a separate service. From 3cda7869d200c09bdfc31889561d21b40276060d Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sun, 5 Jul 2026 09:26:54 +0000 Subject: [PATCH 04/21] Address post-auth binder review feedback --- crates/ironrdp-server/src/server.rs | 41 +++++++++++++---------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 40f783461..19137f251 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -844,9 +844,9 @@ impl RdpServer { /// Set or clear a post-auth connection binder. /// - /// When set, the binder is called after credentials have been validated. - /// The returned display/input handlers replace the server defaults for the - /// accepted connection. + /// When set, the binder is called only when authenticated credentials are + /// available. The returned display/input handlers replace the server + /// defaults for the accepted connection. pub fn set_connection_binder(&mut self, binder: Option>) { self.connection_binder = binder; } @@ -1664,13 +1664,20 @@ impl RdpServer { } else { self.credential_validator.clone() }; - let authenticated_credentials = resolve_authenticated_credentials( + let authenticated_credentials = match resolve_authenticated_credentials( credential_validator, result.credentials.as_ref(), result.reactivation, authenticated_credentials_cache, ) - .await?; + .await + { + Ok(credentials) => credentials, + Err(e) => { + send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; + return Err(e); + } + }; if let Some(binder) = self.connection_binder.clone() { let Some(credentials) = authenticated_credentials.as_ref() else { @@ -2315,15 +2322,10 @@ mod wrdp_reactivation_tests { .expect("initial validation should produce credentials"); assert_eq!(first.username, "alice"); - let reactivated = resolve_authenticated_credentials( - Some(validator), - None, - true, - &mut per_connection_cache, - ) - .await - .expect("reactivation should reuse same-connection cache") - .expect("reactivation should have cached credentials"); + let reactivated = resolve_authenticated_credentials(Some(validator), None, true, &mut per_connection_cache) + .await + .expect("reactivation should reuse same-connection cache") + .expect("reactivation should have cached credentials"); assert_eq!(reactivated.username, "alice"); } @@ -2342,14 +2344,9 @@ mod wrdp_reactivation_tests { assert!(first_connection_cache.is_some()); let mut second_connection_cache = None; - let reactivated = resolve_authenticated_credentials( - Some(validator), - None, - true, - &mut second_connection_cache, - ) - .await - .expect("missing same-connection cache is not a backend error"); + let reactivated = resolve_authenticated_credentials(Some(validator), None, true, &mut second_connection_cache) + .await + .expect("missing same-connection cache is not a backend error"); assert!(reactivated.is_none()); assert!(second_connection_cache.is_none()); } From 27058f7ba90631c6caa7f7ada42abc7452ec46a0 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sun, 5 Jul 2026 09:30:35 +0000 Subject: [PATCH 05/21] Avoid rebinding during RDP reactivation --- crates/ironrdp-server/src/server.rs | 32 ++++++++++++++++------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 19137f251..09356af7a 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -205,7 +205,7 @@ pub struct BoundConnection { /// Async post-auth connection binder. /// -/// This hook runs after [`CredentialValidator`] accepts credentials and before +/// This hook runs once authenticated credentials are available and before /// static channels, display updates, or input dispatch begin. It lets a server /// bind display/input resources to the authenticated identity without creating /// per-user resources before authentication. @@ -1679,20 +1679,24 @@ impl RdpServer { } }; - if let Some(binder) = self.connection_binder.clone() { - let Some(credentials) = authenticated_credentials.as_ref() else { - warn!("Connection binder configured but no authenticated credentials are available"); - send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; - bail!("no authenticated credentials for connection binding"); - }; + if !result.reactivation { + if let Some(binder) = self.connection_binder.clone() { + let Some(credentials) = authenticated_credentials.as_ref() else { + warn!("Connection binder configured but no authenticated credentials are available"); + send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; + bail!("no authenticated credentials for connection binding"); + }; - let bound = binder - .bind_connection(credentials) - .await - .context("connection binder failed")?; - *self.display.lock().await = bound.display; - *self.handler.lock().await = bound.input; - debug!("Connection binder installed display/input handlers"); + let bound = binder + .bind_connection(credentials) + .await + .context("connection binder failed")?; + *self.display.lock().await = bound.display; + *self.handler.lock().await = bound.input; + debug!("Connection binder installed display/input handlers"); + } + } else if self.connection_binder.is_some() { + debug!("Skipping connection binder during reactivation"); } if !result.input_events.is_empty() { From 054a8220a2ec7a4fce69fbbcfc49d525dd43a505 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Mon, 6 Jul 2026 17:40:24 +0000 Subject: [PATCH 06/21] Keep bound server handlers connection-local --- crates/ironrdp-server/src/server.rs | 159 ++++++++++++++++++++++++++-- 1 file changed, 148 insertions(+), 11 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 09356af7a..cfe117ed7 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -3,7 +3,7 @@ use core::net::SocketAddr; use core::sync::atomic::{AtomicBool, AtomicU32, Ordering}; use core::time::Duration; use std::rc::Rc; -use std::sync::Arc; +use std::sync::{Arc, Mutex as StdMutex}; use anyhow::{Context as _, Result, bail}; use ironrdp_acceptor::{Acceptor, AcceptorResult, BeginResult, DesktopSize}; @@ -37,12 +37,12 @@ use tracing::{debug, error, trace, warn}; use crate::autodetect::{AutoDetectManager, RttSnapshot}; use crate::clipboard::CliprdrServerFactory; -use crate::display::{DisplayUpdate, RdpServerDisplay}; +use crate::display::{DisplayUpdate, RdpServerDisplay, RdpServerDisplayUpdates}; use crate::echo::{EchoDvcBridge, EchoServerHandle, EchoServerMessage, build_echo_request}; use crate::encoder::{UpdateEncoder, UpdateEncoderCodecs}; #[cfg(feature = "egfx")] use crate::gfx::{EgfxServerMessage, GfxServerFactory}; -use crate::handler::RdpServerInputHandler; +use crate::handler::{KeyboardEvent, MouseEvent, RdpServerInputHandler}; use crate::{SoundServerFactory, builder, capabilities}; /// TCP listen backlog size for the RDP server socket. @@ -214,6 +214,111 @@ pub trait ConnectionBinder: Send + Sync { async fn bind_connection(&self, credentials: &Credentials) -> Result; } +struct BoundDisplaySlot { + default: Box, + bound: Arc>>>, +} + +impl BoundDisplaySlot { + fn new(default: Box, bound: Arc>>>) -> Self { + Self { default, bound } + } +} + +#[async_trait::async_trait] +impl RdpServerDisplay for BoundDisplaySlot { + async fn size(&mut self) -> DesktopSize { + let bound_display = { + let mut bound = self.bound.lock().expect("bound display lock poisoned"); + bound.take() + }; + + if let Some(mut display) = bound_display { + let size = display.size().await; + *self.bound.lock().expect("bound display lock poisoned") = Some(display); + size + } else { + self.default.size().await + } + } + + async fn request_initial_size(&mut self, client_size: DesktopSize) -> DesktopSize { + let bound_display = { + let mut bound = self.bound.lock().expect("bound display lock poisoned"); + bound.take() + }; + + if let Some(mut display) = bound_display { + let size = display.request_initial_size(client_size).await; + *self.bound.lock().expect("bound display lock poisoned") = Some(display); + size + } else { + self.default.request_initial_size(client_size).await + } + } + + async fn updates(&mut self) -> Result> { + let bound_display = { + let mut bound = self.bound.lock().expect("bound display lock poisoned"); + bound.take() + }; + + if let Some(mut display) = bound_display { + let updates = display.updates().await; + *self.bound.lock().expect("bound display lock poisoned") = Some(display); + updates + } else { + self.default.updates().await + } + } + + fn request_layout(&mut self, layout: DisplayControlMonitorLayout) { + let mut bound = self.bound.lock().expect("bound display lock poisoned"); + if let Some(display) = bound.as_mut() { + display.request_layout(layout); + } else { + drop(bound); + self.default.request_layout(layout); + } + } +} + +struct BoundInputSlot { + default: Box, + bound: Arc>>>, +} + +impl BoundInputSlot { + fn new( + default: Box, + bound: Arc>>>, + ) -> Self { + Self { default, bound } + } +} + +impl RdpServerInputHandler for BoundInputSlot { + fn keyboard(&mut self, event: KeyboardEvent) { + let mut bound = self.bound.lock().expect("bound input lock poisoned"); + if let Some(handler) = bound.as_mut() { + handler.keyboard(event); + } else { + drop(bound); + self.default.keyboard(event); + } + } + + fn mouse(&mut self, event: MouseEvent) { + let mut bound = self.bound.lock().expect("bound input lock poisoned"); + if let Some(handler) = bound.as_mut() { + handler.mouse(event); + } else { + drop(bound); + self.default.mouse(event); + } + } +} + /// A built-in [`CredentialValidator`] that accepts exactly one fixed set of credentials. /// /// This is the validation-policy equivalent of the acceptor's pre-loaded @@ -463,6 +568,8 @@ pub struct RdpServer { // FIXME: replace with a channel and poll/process the handler? handler: Arc>>, display: Arc>>, + bound_handler: Arc>>>, + bound_display: Arc>>>, static_channels: StaticChannelSet, sound_factory: Option>, cliprdr_factory: Option>, @@ -618,10 +725,21 @@ impl RdpServer { if let Some(gfx) = gfx_factory.as_mut() { gfx.set_sender(ev_sender.clone()); } + let bound_handler = Arc::new(StdMutex::new(None)); + let bound_display = Arc::new(StdMutex::new(None)); + Self { opts, - handler: Arc::new(Mutex::new(handler)), - display: Arc::new(Mutex::new(display)), + handler: Arc::new(Mutex::new(Box::new(BoundInputSlot::new( + handler, + Arc::clone(&bound_handler), + )))), + display: Arc::new(Mutex::new(Box::new(BoundDisplaySlot::new( + display, + Arc::clone(&bound_display), + )))), + bound_handler, + bound_display, static_channels: StaticChannelSet::new(), sound_factory, cliprdr_factory, @@ -851,6 +969,16 @@ impl RdpServer { self.connection_binder = binder; } + async fn install_bound_connection(&mut self, bound: BoundConnection) { + *self.bound_display.lock().expect("bound display lock poisoned") = Some(bound.display); + *self.bound_handler.lock().expect("bound input lock poisoned") = Some(bound.input); + } + + async fn clear_bound_connection(&mut self) { + self.bound_display.lock().expect("bound display lock poisoned").take(); + self.bound_handler.lock().expect("bound input lock poisoned").take(); + } + pub fn event_sender(&self) -> &mpsc::UnboundedSender { &self.ev_sender } @@ -1246,6 +1374,7 @@ impl RdpServer { error!(?error, "Connection error"); } + self.clear_bound_connection().await; self.static_channels = StaticChannelSet::new(); if let Some(ref mut handler) = self.connection_handler { @@ -1681,18 +1810,26 @@ impl RdpServer { if !result.reactivation { if let Some(binder) = self.connection_binder.clone() { + if self.credential_validator.is_none() && !matches!(self.opts.security, RdpServerSecurity::Hybrid(_)) { + warn!("Connection binder requires authenticated credentials from a validator or CredSSP/Hybrid"); + send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; + bail!("connection binder requires authenticated credentials"); + } + let Some(credentials) = authenticated_credentials.as_ref() else { warn!("Connection binder configured but no authenticated credentials are available"); send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; bail!("no authenticated credentials for connection binding"); }; - let bound = binder - .bind_connection(credentials) - .await - .context("connection binder failed")?; - *self.display.lock().await = bound.display; - *self.handler.lock().await = bound.input; + let bound = match binder.bind_connection(credentials).await { + Ok(bound) => bound, + Err(e) => { + send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; + return Err(e).context("connection binder failed"); + } + }; + self.install_bound_connection(bound).await; debug!("Connection binder installed display/input handlers"); } } else if self.connection_binder.is_some() { From cb3b4e08d2269a03ea628a6c024167004ad094ff Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Mon, 6 Jul 2026 17:44:18 +0000 Subject: [PATCH 07/21] Clarify credential checks during reactivation --- crates/ironrdp-server/src/server.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index cfe117ed7..903e764aa 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -1787,7 +1787,9 @@ impl RdpServer { // Validate credentials if a validator is configured. The validator runs here, in the // async server layer, rather than in the sans-I/O acceptor, because real validators // (PAM/LDAP/DB) are I/O-bound. On rejection, deny with a ServerSetErrorInfoPdu before - // closing, matching the acceptor's exact-match denial path. + // closing, matching the acceptor's exact-match denial path. Reactivation still resolves + // credentials so clients that resend them are re-validated before channel state is reused. + // A verified auto-reconnect cookie bypasses the configured credential validator. let credential_validator = if is_auto_reconnect { None } else { From 638043e202370361943f3ed238e6e1a274a4bcc6 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Mon, 6 Jul 2026 17:48:18 +0000 Subject: [PATCH 08/21] Update server credential hook docs --- crates/ironrdp-server/src/builder.rs | 12 +++++------- crates/ironrdp-server/src/server.rs | 15 +++++++-------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/crates/ironrdp-server/src/builder.rs b/crates/ironrdp-server/src/builder.rs index a0c8076fc..3f62e1708 100644 --- a/crates/ironrdp-server/src/builder.rs +++ b/crates/ironrdp-server/src/builder.rs @@ -280,20 +280,18 @@ impl RdpServerBuilder { self } - /// Set a credential validator for TLS-mode connections. + /// Set a credential validator for accepted client credentials. /// - /// When set, credentials received from the client during - /// `SecureSettingsExchange` (`ClientInfoPdu`) are passed to this - /// validator before the session is established. Rejection or a backend + /// When set, credentials surfaced by the acceptor are passed to this + /// validator before the session is established. This includes + /// `SecureSettingsExchange` (`ClientInfoPdu`) credentials and, when + /// available, CredSSP/Hybrid delegated credentials. Rejection or a backend /// error closes the connection. Pass `None` (the default) to skip /// validation entirely. /// /// A valid Server Auto-Reconnect Cookie bypasses this validator. Applications /// that must validate every connection should leave automatic reconnection /// disabled. - /// - /// Not used for CredSSP/Hybrid connections (those use pre-loaded - /// credentials for NTLM challenge-response). pub fn with_credential_validator(mut self, validator: Option>) -> Self { self.state.credential_validator = validator; self diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 903e764aa..2a87f3666 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -192,10 +192,10 @@ pub trait CredentialValidator: Send + Sync { async fn validate(&self, credentials: &Credentials) -> Result; } -/// Display/input objects bound after a credential validator accepts a client. +/// Display/input objects bound after the server authenticates a client. /// /// Servers with per-user desktop/session isolation can start with placeholder -/// display and input handlers, validate the client's credentials, and then +/// display and input handlers, authenticate the client's credentials, and then /// replace those placeholders with handlers attached to the authenticated /// user's session before the RDP client loop starts. pub struct BoundConnection { @@ -773,11 +773,12 @@ impl RdpServer { builder::RdpServerBuilder::new() } - /// Set or clear the credential validator for TLS-mode connections. + /// Set or clear the credential validator for accepted client credentials. /// - /// When set, credentials received from the client during - /// `SecureSettingsExchange` are validated through this callback before - /// the session is established. If the validator returns + /// When set, credentials surfaced by the acceptor are validated through + /// this callback before the session is established. This includes + /// `SecureSettingsExchange` (`ClientInfoPdu`) credentials and, when + /// available, CredSSP/Hybrid delegated credentials. If the validator returns /// [`CredentialDecision::Reject`] (or a [`CredentialValidationError`]), /// the connection is rejected. Passing `None` clears any previously /// configured validator. @@ -790,8 +791,6 @@ impl RdpServer { /// the builder's `with_credential_validator` method /// ([`RdpServer::builder`]); this setter exists for dynamic /// post-construction reconfiguration. - /// - /// Not used for CredSSP/Hybrid connections (those use pre-loaded credentials). pub fn set_credential_validator(&mut self, validator: Option>) { self.credential_validator = validator; } From 4e7ad62350961b8f3eb8960dd136421dd4c06026 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Mon, 6 Jul 2026 17:54:29 +0000 Subject: [PATCH 09/21] Avoid caching authenticated credentials --- crates/ironrdp-server/src/server.rs | 87 +++++++++-------------------- 1 file changed, 26 insertions(+), 61 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 2a87f3666..4686f7a97 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -1762,7 +1762,6 @@ impl RdpServer { reader: &mut Framed, writer: &mut Framed, result: AcceptorResult, - authenticated_credentials_cache: &mut Option, ) -> Result where R: FramedRead, @@ -1786,8 +1785,8 @@ impl RdpServer { // Validate credentials if a validator is configured. The validator runs here, in the // async server layer, rather than in the sans-I/O acceptor, because real validators // (PAM/LDAP/DB) are I/O-bound. On rejection, deny with a ServerSetErrorInfoPdu before - // closing, matching the acceptor's exact-match denial path. Reactivation still resolves - // credentials so clients that resend them are re-validated before channel state is reused. + // closing, matching the acceptor's exact-match denial path. Reactivation still validates + // credentials again if the client resends them before channel state is reused. // A verified auto-reconnect cookie bypasses the configured credential validator. let credential_validator = if is_auto_reconnect { None @@ -1798,7 +1797,6 @@ impl RdpServer { credential_validator, result.credentials.as_ref(), result.reactivation, - authenticated_credentials_cache, ) .await { @@ -2196,8 +2194,6 @@ impl RdpServer { where S: AsyncRead + AsyncWrite + Sync + Send + Unpin, { - let mut authenticated_credentials_cache = None; - loop { let (new_framed, result) = ironrdp_acceptor::accept_finalize(framed, &mut acceptor) .await @@ -2205,10 +2201,7 @@ impl RdpServer { let (mut reader, mut writer) = split_tokio_framed(new_framed); - match self - .client_accepted(&mut reader, &mut writer, result, &mut authenticated_credentials_cache) - .await? - { + match self.client_accepted(&mut reader, &mut writer, result).await? { RunState::Continue => { unreachable!(); } @@ -2243,15 +2236,13 @@ async fn resolve_authenticated_credentials( credential_validator: Option>, result_credentials: Option<&Credentials>, reactivation: bool, - authenticated_credentials_cache: &mut Option, -) -> Result> { - if let Some(validator) = credential_validator { - if let Some(creds) = result_credentials { +) -> Result> { + if let Some(creds) = result_credentials { + if let Some(validator) = credential_validator { match validator.validate(creds).await { Ok(CredentialDecision::Accept) => { debug!("Credential validation accepted"); - *authenticated_credentials_cache = Some(creds.clone()); - Ok(Some(creds.clone())) + Ok(Some(creds)) } Ok(CredentialDecision::Reject) => { warn!("Credential validation rejected"); @@ -2262,24 +2253,14 @@ async fn resolve_authenticated_credentials( bail!("credential validation backend error"); } } - } else if reactivation { - let credentials = authenticated_credentials_cache.clone(); - if credentials.is_some() { - debug!("Reusing cached authenticated credentials for reactivation"); - } else { - debug!("Skipping credential validation for reactivation without cached credentials"); - } - Ok(credentials) } else { - debug!("Skipping credential validation (no credentials in AcceptorResult)"); - Ok(None) + Ok(Some(creds)) } - } else if let Some(creds) = result_credentials { - *authenticated_credentials_cache = Some(creds.clone()); - Ok(Some(creds.clone())) } else if reactivation { - Ok(authenticated_credentials_cache.clone()) + debug!("Skipping credential validation for reactivation without credentials"); + Ok(None) } else { + debug!("Skipping credential validation (no credentials in AcceptorResult)"); Ok(None) } } @@ -2449,47 +2430,31 @@ mod wrdp_reactivation_tests { } #[tokio::test] - async fn reactivation_without_credentials_reuses_same_connection_validated_identity() { + async fn reactivation_without_credentials_does_not_retain_validated_identity() { let validator = Arc::new(AllowUserValidator("alice")); - let mut per_connection_cache = None; + let initial_credentials = creds("alice"); - let first = resolve_authenticated_credentials( - Some(validator.clone()), - Some(&creds("alice")), - false, - &mut per_connection_cache, - ) - .await - .expect("initial validation should succeed") - .expect("initial validation should produce credentials"); + let first = resolve_authenticated_credentials(Some(validator.clone()), Some(&initial_credentials), false) + .await + .expect("initial validation should succeed") + .expect("initial validation should produce credentials"); assert_eq!(first.username, "alice"); - let reactivated = resolve_authenticated_credentials(Some(validator), None, true, &mut per_connection_cache) + let reactivated = resolve_authenticated_credentials(Some(validator), None, true) .await - .expect("reactivation should reuse same-connection cache") - .expect("reactivation should have cached credentials"); - assert_eq!(reactivated.username, "alice"); + .expect("missing reactivation credentials is not a backend error"); + assert!(reactivated.is_none()); } #[tokio::test] - async fn reactivation_without_credentials_cannot_use_previous_tcp_connection_cache() { + async fn reactivation_with_credentials_revalidates_resent_identity() { let validator = Arc::new(AllowUserValidator("alice")); - let mut first_connection_cache = None; - resolve_authenticated_credentials( - Some(validator.clone()), - Some(&creds("alice")), - false, - &mut first_connection_cache, - ) - .await - .expect("initial validation should succeed"); - assert!(first_connection_cache.is_some()); + let reactivation_credentials = creds("alice"); - let mut second_connection_cache = None; - let reactivated = resolve_authenticated_credentials(Some(validator), None, true, &mut second_connection_cache) + let reactivated = resolve_authenticated_credentials(Some(validator), Some(&reactivation_credentials), true) .await - .expect("missing same-connection cache is not a backend error"); - assert!(reactivated.is_none()); - assert!(second_connection_cache.is_none()); + .expect("resent reactivation credentials should be validated") + .expect("resent reactivation credentials should remain available"); + assert_eq!(reactivated.username, "alice"); } } From fc1288da46bf3fd6406f114c13336f05b86aa471 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Wed, 8 Jul 2026 07:04:38 +0000 Subject: [PATCH 10/21] Document bound handler scoping --- crates/ironrdp-server/src/server.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 4686f7a97..b67d27bef 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -568,6 +568,9 @@ pub struct RdpServer { // FIXME: replace with a channel and poll/process the handler? handler: Arc>>, display: Arc>>, + // ConnectionBinder installs per-user handlers into these slots. The + // default handler/display above stay stable for the server lifetime, while + // the slots are cleared after each connection to avoid cross-user reuse. bound_handler: Arc>>>, bound_display: Arc>>>, static_channels: StaticChannelSet, @@ -1821,6 +1824,8 @@ impl RdpServer { bail!("no authenticated credentials for connection binding"); }; + // Bound handlers are connection-local: install them into the dispatch slots + // for this client only, then clear the slots when the connection ends. let bound = match binder.bind_connection(credentials).await { Ok(bound) => bound, Err(e) => { @@ -2431,10 +2436,10 @@ mod wrdp_reactivation_tests { #[tokio::test] async fn reactivation_without_credentials_does_not_retain_validated_identity() { - let validator = Arc::new(AllowUserValidator("alice")); + let validator: Arc = Arc::new(AllowUserValidator("alice")); let initial_credentials = creds("alice"); - let first = resolve_authenticated_credentials(Some(validator.clone()), Some(&initial_credentials), false) + let first = resolve_authenticated_credentials(Some(Arc::clone(&validator)), Some(&initial_credentials), false) .await .expect("initial validation should succeed") .expect("initial validation should produce credentials"); From 901aa70413f4857f511b53b2feef0e6f6209b58f Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Wed, 8 Jul 2026 07:36:55 +0000 Subject: [PATCH 11/21] Track accepted credential origin --- crates/ironrdp-acceptor/src/connection.rs | 19 +++- crates/ironrdp-acceptor/src/lib.rs | 2 +- crates/ironrdp-server/src/builder.rs | 10 +-- crates/ironrdp-server/src/lib.rs | 1 + crates/ironrdp-server/src/server.rs | 86 +++++++++++++------ .../tests/server/credential_validator.rs | 42 +++++++-- 6 files changed, 121 insertions(+), 39 deletions(-) diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index 541eb69ea..6b99252b3 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -40,6 +40,7 @@ pub struct Acceptor { saved_for_reactivation: AcceptorState, pub(crate) creds: Option, received_credentials: Option, + received_credentials_origin: Option, received_auto_reconnect: Option, reactivation: bool, honor_client_desktop_size: Option, @@ -77,6 +78,14 @@ fn set_bitmap_desktop_size(capabilities: &mut [CapabilitySet], size: DesktopSize } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CredentialOrigin { + /// Received in the ClientInfoPdu (MS-RDPBCGR 2.2.1.11); not authenticated by the handshake. + ClientInfo, + /// Delegated TSPasswordCreds decrypted by CredSSP (MS-CSSP); authenticated by the exchange. + CredSspDelegated, +} + #[derive(Debug)] pub struct AcceptorResult { pub static_channels: StaticChannelSet, @@ -114,8 +123,11 @@ pub struct AcceptorResult { /// delegated TSPasswordCreds have been decrypted by CredSSP. /// /// Servers that need to validate credentials (e.g., via PAM or LDAP) - /// can use this field for post-handshake validation. + /// can use this field for post-handshake validation. Check + /// [`Self::credentials_origin`] to distinguish unauthenticated ClientInfo + /// credentials from CredSSP-delegated credentials authenticated by the exchange. pub credentials: Option, + pub credentials_origin: Option, /// Client Auto-Reconnect Packet received in the Client Info PDU. /// /// This is present when the client resumes a session using an @@ -146,6 +158,7 @@ impl Acceptor { saved_for_reactivation: Default::default(), creds, received_credentials: None, + received_credentials_origin: None, received_auto_reconnect: None, reactivation: false, honor_client_desktop_size: None, @@ -232,6 +245,7 @@ impl Acceptor { saved_for_reactivation, creds: consumed.creds, received_credentials: consumed.received_credentials, + received_credentials_origin: consumed.received_credentials_origin, received_auto_reconnect: consumed.received_auto_reconnect, reactivation: true, honor_client_desktop_size: consumed.honor_client_desktop_size, @@ -305,6 +319,7 @@ impl Acceptor { password: identity.password.as_ref().clone(), domain: identity.username.domain_name().map(str::to_owned), }); + self.received_credentials_origin = Some(CredentialOrigin::CredSspDelegated); } /// # Panics @@ -334,6 +349,7 @@ impl Acceptor { multitransport_flags: self.multitransport_flags, reactivation: self.reactivation, credentials: self.received_credentials.take(), + credentials_origin: self.received_credentials_origin.take(), auto_reconnect: self.received_auto_reconnect.take(), }), previous_state => { @@ -812,6 +828,7 @@ impl Sequence for Acceptor { // Store credentials for later retrieval via AcceptorResult. self.received_credentials = Some(creds); + self.received_credentials_origin = Some(CredentialOrigin::ClientInfo); } ( diff --git a/crates/ironrdp-acceptor/src/lib.rs b/crates/ironrdp-acceptor/src/lib.rs index ba864fa6e..771226a3c 100644 --- a/crates/ironrdp-acceptor/src/lib.rs +++ b/crates/ironrdp-acceptor/src/lib.rs @@ -18,7 +18,7 @@ pub use ironrdp_connector::DesktopSize; use ironrdp_pdu::nego; pub use self::channel_connection::{ChannelConnectionSequence, ChannelConnectionState}; -pub use self::connection::{Acceptor, AcceptorResult, AcceptorState}; +pub use self::connection::{Acceptor, AcceptorResult, AcceptorState, CredentialOrigin}; pub use self::finalization::{FinalizationSequence, FinalizationState}; use crate::credssp::resolve_generator; diff --git a/crates/ironrdp-server/src/builder.rs b/crates/ironrdp-server/src/builder.rs index 3f62e1708..0190b4e0a 100644 --- a/crates/ironrdp-server/src/builder.rs +++ b/crates/ironrdp-server/src/builder.rs @@ -283,11 +283,11 @@ impl RdpServerBuilder { /// Set a credential validator for accepted client credentials. /// /// When set, credentials surfaced by the acceptor are passed to this - /// validator before the session is established. This includes - /// `SecureSettingsExchange` (`ClientInfoPdu`) credentials and, when - /// available, CredSSP/Hybrid delegated credentials. Rejection or a backend - /// error closes the connection. Pass `None` (the default) to skip - /// validation entirely. + /// validator before the session is established, together with their + /// origin. This includes `SecureSettingsExchange` (`ClientInfoPdu`) + /// credentials and, when available, CredSSP/Hybrid delegated credentials. + /// Rejection or a backend error closes the connection. Pass `None` (the + /// default) to skip validation entirely. /// /// A valid Server Auto-Reconnect Cookie bypasses this validator. Applications /// that must validate every connection should leave automatic reconnection diff --git a/crates/ironrdp-server/src/lib.rs b/crates/ironrdp-server/src/lib.rs index 7fc97fe92..ccd22a81d 100644 --- a/crates/ironrdp-server/src/lib.rs +++ b/crates/ironrdp-server/src/lib.rs @@ -32,6 +32,7 @@ pub use gfx::{EgfxServerMessage, GfxDvcBridge, GfxServerFactory, GfxServerHandle pub use handler::{KeyboardEvent, MouseEvent, RdpServerInputHandler}; #[cfg(feature = "helper")] pub use helper::TlsIdentityCtx; +pub use ironrdp_acceptor::CredentialOrigin; pub use ironrdp_pdu::rdp::session_info::ServerAutoReconnect; pub use server::{ AutoReconnectCookieHandle, BoundConnection, ConnectionBinder, ConnectionHandler, CredentialDecision, diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index b67d27bef..c5a69751d 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -6,7 +6,7 @@ use std::rc::Rc; use std::sync::{Arc, Mutex as StdMutex}; use anyhow::{Context as _, Result, bail}; -use ironrdp_acceptor::{Acceptor, AcceptorResult, BeginResult, DesktopSize}; +use ironrdp_acceptor::{Acceptor, AcceptorResult, BeginResult, CredentialOrigin, DesktopSize}; use ironrdp_async::Framed; use ironrdp_cliprdr::CliprdrServer; use ironrdp_cliprdr::backend::ClipboardMessage; @@ -142,20 +142,21 @@ impl core::error::Error for CredentialValidationError { } } -/// Server-side credential validator for TLS-mode connections. +/// Server-side credential validator for accepted client credentials. /// -/// Called during connection setup when the server receives client credentials -/// via `ClientInfoPdu`. Not used for CredSSP/Hybrid connections (those use -/// pre-loaded credentials for NTLM challenge-response). +/// Called during connection setup when the acceptor surfaces credentials from +/// either `ClientInfoPdu` or CredSSP delegated TSPasswordCreds. Use the +/// [`CredentialOrigin`] argument to distinguish unauthenticated ClientInfo +/// credentials from CredSSP-delegated credentials authenticated by the exchange. /// -/// Implement this trait to validate credentials against external systems +/// Implement this trait to validate or authorize credentials against external systems /// (PAM, LDAP, database, etc.). For blocking backends, wrap the call in /// `tokio::task::spawn_blocking` to avoid stalling the async runtime. /// /// # Example /// /// ```ignore -/// use ironrdp_server::{CredentialDecision, CredentialValidationError, CredentialValidator, Credentials}; +/// use ironrdp_server::{CredentialDecision, CredentialOrigin, CredentialValidationError, CredentialValidator, Credentials}; /// /// struct StaticValidator { /// expected_user: String, @@ -178,7 +179,7 @@ impl core::error::Error for CredentialValidationError { /// ``` #[async_trait::async_trait] pub trait CredentialValidator: Send + Sync { - /// Validate credentials received from the client. + /// Validate or authorize credentials received from the client. /// /// Return `Ok(CredentialDecision::Accept)` to permit the connection, /// `Ok(CredentialDecision::Reject)` to refuse it. Return @@ -189,7 +190,11 @@ pub trait CredentialValidator: Send + Sync { /// database driver) should offload the work, for example with /// `tokio::task::spawn_blocking`, so the returned future does not stall the /// caller's executor. Native-async backends can simply `.await`. - async fn validate(&self, credentials: &Credentials) -> Result; + async fn validate( + &self, + credentials: &Credentials, + origin: CredentialOrigin, + ) -> Result; } /// Display/input objects bound after the server authenticates a client. @@ -216,6 +221,9 @@ pub trait ConnectionBinder: Send + Sync { struct BoundDisplaySlot { default: Box, + // Async display methods temporarily take the bound display out of this + // slot before awaiting. That relies on the outer tokio::Mutex around + // RdpServer::display to serialize all display callers. bound: Arc>>>, } @@ -285,6 +293,8 @@ impl RdpServerDisplay for BoundDisplaySlot { struct BoundInputSlot { default: Box, + // Kept parallel to BoundDisplaySlot. The outer tokio::Mutex around + // RdpServer::handler serializes access before input dispatch reaches this slot. bound: Arc>>>, } @@ -337,7 +347,11 @@ impl ExactMatchCredentialValidator { #[async_trait::async_trait] impl CredentialValidator for ExactMatchCredentialValidator { - async fn validate(&self, credentials: &Credentials) -> Result { + async fn validate( + &self, + credentials: &Credentials, + _origin: CredentialOrigin, + ) -> Result { if credentials == &self.expected { Ok(CredentialDecision::Accept) } else { @@ -570,7 +584,8 @@ pub struct RdpServer { display: Arc>>, // ConnectionBinder installs per-user handlers into these slots. The // default handler/display above stay stable for the server lifetime, while - // the slots are cleared after each connection to avoid cross-user reuse. + // the slots are cleared at connection entry and after each connection to + // avoid cross-user reuse. bound_handler: Arc>>>, bound_display: Arc>>>, static_channels: StaticChannelSet, @@ -1771,6 +1786,7 @@ impl RdpServer { W: FramedWrite, { debug!("Client accepted"); + self.clear_bound_connection().await; let is_auto_reconnect = if let Some(reconnect) = result.auto_reconnect.as_ref() { if !self.verify_auto_reconnect_cookie(reconnect) { @@ -1799,6 +1815,7 @@ impl RdpServer { let authenticated_credentials = match resolve_authenticated_credentials( credential_validator, result.credentials.as_ref(), + result.credentials_origin, result.reactivation, ) .await @@ -1812,8 +1829,10 @@ impl RdpServer { if !result.reactivation { if let Some(binder) = self.connection_binder.clone() { - if self.credential_validator.is_none() && !matches!(self.opts.security, RdpServerSecurity::Hybrid(_)) { - warn!("Connection binder requires authenticated credentials from a validator or CredSSP/Hybrid"); + if self.credential_validator.is_none() + && result.credentials_origin != Some(CredentialOrigin::CredSspDelegated) + { + warn!("Connection binder requires authenticated credentials from a validator or CredSSP"); send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; bail!("connection binder requires authenticated credentials"); } @@ -2240,11 +2259,16 @@ impl RdpServer { async fn resolve_authenticated_credentials( credential_validator: Option>, result_credentials: Option<&Credentials>, + credentials_origin: Option, reactivation: bool, ) -> Result> { if let Some(creds) = result_credentials { + let Some(origin) = credentials_origin else { + bail!("credentials provided without an origin"); + }; + if let Some(validator) = credential_validator { - match validator.validate(creds).await { + match validator.validate(creds, origin).await { Ok(CredentialDecision::Accept) => { debug!("Credential validation accepted"); Ok(Some(creds)) @@ -2417,7 +2441,11 @@ mod wrdp_reactivation_tests { #[async_trait::async_trait] impl CredentialValidator for AllowUserValidator { - async fn validate(&self, credentials: &Credentials) -> Result { + async fn validate( + &self, + credentials: &Credentials, + _origin: CredentialOrigin, + ) -> Result { if credentials.username == self.0 { Ok(CredentialDecision::Accept) } else { @@ -2439,13 +2467,18 @@ mod wrdp_reactivation_tests { let validator: Arc = Arc::new(AllowUserValidator("alice")); let initial_credentials = creds("alice"); - let first = resolve_authenticated_credentials(Some(Arc::clone(&validator)), Some(&initial_credentials), false) - .await - .expect("initial validation should succeed") - .expect("initial validation should produce credentials"); + let first = resolve_authenticated_credentials( + Some(Arc::clone(&validator)), + Some(&initial_credentials), + Some(CredentialOrigin::ClientInfo), + false, + ) + .await + .expect("initial validation should succeed") + .expect("initial validation should produce credentials"); assert_eq!(first.username, "alice"); - let reactivated = resolve_authenticated_credentials(Some(validator), None, true) + let reactivated = resolve_authenticated_credentials(Some(validator), None, None, true) .await .expect("missing reactivation credentials is not a backend error"); assert!(reactivated.is_none()); @@ -2456,10 +2489,15 @@ mod wrdp_reactivation_tests { let validator = Arc::new(AllowUserValidator("alice")); let reactivation_credentials = creds("alice"); - let reactivated = resolve_authenticated_credentials(Some(validator), Some(&reactivation_credentials), true) - .await - .expect("resent reactivation credentials should be validated") - .expect("resent reactivation credentials should remain available"); + let reactivated = resolve_authenticated_credentials( + Some(validator), + Some(&reactivation_credentials), + Some(CredentialOrigin::ClientInfo), + true, + ) + .await + .expect("resent reactivation credentials should be validated") + .expect("resent reactivation credentials should remain available"); assert_eq!(reactivated.username, "alice"); } } diff --git a/crates/ironrdp-testsuite-core/tests/server/credential_validator.rs b/crates/ironrdp-testsuite-core/tests/server/credential_validator.rs index 4b4dd280a..e62840fc0 100644 --- a/crates/ironrdp-testsuite-core/tests/server/credential_validator.rs +++ b/crates/ironrdp-testsuite-core/tests/server/credential_validator.rs @@ -2,7 +2,9 @@ use core::fmt; use std::sync::Arc; use async_trait::async_trait; -use ironrdp_server::{CredentialDecision, CredentialValidationError, CredentialValidator, Credentials}; +use ironrdp_server::{ + CredentialDecision, CredentialOrigin, CredentialValidationError, CredentialValidator, Credentials, +}; fn fixed_creds() -> Credentials { Credentials { @@ -15,7 +17,11 @@ fn fixed_creds() -> Credentials { struct AlwaysAccept; #[async_trait] impl CredentialValidator for AlwaysAccept { - async fn validate(&self, _: &Credentials) -> Result { + async fn validate( + &self, + _: &Credentials, + _: CredentialOrigin, + ) -> Result { Ok(CredentialDecision::Accept) } } @@ -23,7 +29,11 @@ impl CredentialValidator for AlwaysAccept { struct AlwaysReject; #[async_trait] impl CredentialValidator for AlwaysReject { - async fn validate(&self, _: &Credentials) -> Result { + async fn validate( + &self, + _: &Credentials, + _: CredentialOrigin, + ) -> Result { Ok(CredentialDecision::Reject) } } @@ -40,7 +50,11 @@ impl core::error::Error for BackendDown {} struct AlwaysBackendError; #[async_trait] impl CredentialValidator for AlwaysBackendError { - async fn validate(&self, _: &Credentials) -> Result { + async fn validate( + &self, + _: &Credentials, + _: CredentialOrigin, + ) -> Result { Err(CredentialValidationError::new(BackendDown)) } } @@ -48,19 +62,28 @@ impl CredentialValidator for AlwaysBackendError { #[tokio::test] async fn validator_accept_returns_accept() { let v = AlwaysAccept; - assert_eq!(v.validate(&fixed_creds()).await.unwrap(), CredentialDecision::Accept); + assert_eq!( + v.validate(&fixed_creds(), CredentialOrigin::ClientInfo).await.unwrap(), + CredentialDecision::Accept + ); } #[tokio::test] async fn validator_reject_returns_reject() { let v = AlwaysReject; - assert_eq!(v.validate(&fixed_creds()).await.unwrap(), CredentialDecision::Reject); + assert_eq!( + v.validate(&fixed_creds(), CredentialOrigin::ClientInfo).await.unwrap(), + CredentialDecision::Reject + ); } #[tokio::test] async fn validator_backend_error_propagates_source() { let v = AlwaysBackendError; - let err = v.validate(&fixed_creds()).await.expect_err("expected backend error"); + let err = v + .validate(&fixed_creds(), CredentialOrigin::ClientInfo) + .await + .expect_err("expected backend error"); assert_eq!(err.to_string(), "credential validator backend failure"); let inner = core::error::Error::source(&err).expect("source must be Some"); assert_eq!(inner.to_string(), "ldap server unreachable"); @@ -70,5 +93,8 @@ async fn validator_backend_error_propagates_source() { async fn validator_can_be_held_behind_arc_dyn() { // Exercises the Send + Sync + 'static bounds the trait promises through Arc. let v: Arc = Arc::new(AlwaysAccept); - assert_eq!(v.validate(&fixed_creds()).await.unwrap(), CredentialDecision::Accept); + assert_eq!( + v.validate(&fixed_creds(), CredentialOrigin::ClientInfo).await.unwrap(), + CredentialDecision::Accept + ); } From 40ab94de4bfc7ad75b55747ad3a0336d3f260524 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sat, 11 Jul 2026 07:58:15 +0000 Subject: [PATCH 12/21] Prioritize graphics dynamic channel creation --- crates/ironrdp-server/src/server.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index c5a69751d..ee4f98352 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -1094,18 +1094,12 @@ impl RdpServer { acceptor.attach_static_channel(RdpsndServer::new(backend)); } - let dcs_backend = DisplayControlBackend::new(Arc::clone(&self.display)); - let dvc = dvc::DrdynvcServer::new() - .with_dynamic_channel(AInputHandler { - handler: Arc::clone(&self.handler), - }) - .with_dynamic_channel(DisplayControlServer::new(Box::new(dcs_backend))); - - let dvc = { - let echo_handle = self.echo_handle.clone(); - dvc.with_dynamic_channel(EchoDvcBridge::new(echo_handle)) - }; - + // Register the graphics channel first. Microsoft mobile clients can + // stop processing a burst of server-created DVCs after encountering an + // optional channel they do not implement. Keeping rdpgfx at channel ID + // zero ensures its create request and capability exchange cannot be + // starved by Advanced Input, DisplayControl, or ECHO negotiation. + let dvc = dvc::DrdynvcServer::new(); #[cfg(feature = "egfx")] let dvc = { let mut dvc = dvc; @@ -1122,6 +1116,15 @@ impl RdpServer { dvc }; + let dcs_backend = DisplayControlBackend::new(Arc::clone(&self.display)); + let dvc = dvc + .with_dynamic_channel(AInputHandler { + handler: Arc::clone(&self.handler), + }) + .with_dynamic_channel(DisplayControlServer::new(Box::new(dcs_backend))); + let echo_handle = self.echo_handle.clone(); + let dvc = dvc.with_dynamic_channel(EchoDvcBridge::new(echo_handle)); + acceptor.attach_static_channel(dvc); } From a4e8a9bdb030150df2e18e05153f83aaacce709d Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sat, 11 Jul 2026 08:08:18 +0000 Subject: [PATCH 13/21] Preserve bound handlers during reactivation --- crates/ironrdp-server/src/server.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index ee4f98352..07f97f94b 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -1789,7 +1789,6 @@ impl RdpServer { W: FramedWrite, { debug!("Client accepted"); - self.clear_bound_connection().await; let is_auto_reconnect = if let Some(reconnect) = result.auto_reconnect.as_ref() { if !self.verify_auto_reconnect_cookie(reconnect) { @@ -2221,6 +2220,11 @@ impl RdpServer { where S: AsyncRead + AsyncWrite + Sync + Send + Unpin, { + // Clear per-user resources once for this TCP connection. Do not clear + // inside the loop: reactivation re-enters client_accepted without + // rebinding, and must keep the existing display/input handlers. + self.clear_bound_connection().await; + loop { let (new_framed, result) = ironrdp_acceptor::accept_finalize(framed, &mut acceptor) .await From 4e2c3dacfa729357a02c6cda4468bffdccc24472 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sat, 11 Jul 2026 08:09:42 +0000 Subject: [PATCH 14/21] Pair received credentials with origin --- crates/ironrdp-acceptor/src/connection.rs | 43 +++++++++-------- crates/ironrdp-acceptor/src/lib.rs | 2 +- crates/ironrdp-server/src/lib.rs | 2 +- crates/ironrdp-server/src/server.rs | 58 ++++++++++------------- 4 files changed, 51 insertions(+), 54 deletions(-) diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index 6b99252b3..93200ab1b 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -39,8 +39,7 @@ pub struct Acceptor { static_channels: StaticChannelSet, saved_for_reactivation: AcceptorState, pub(crate) creds: Option, - received_credentials: Option, - received_credentials_origin: Option, + received_credentials: Option, received_auto_reconnect: Option, reactivation: bool, honor_client_desktop_size: Option, @@ -86,6 +85,12 @@ pub enum CredentialOrigin { CredSspDelegated, } +#[derive(Debug)] +pub struct ReceivedCredentials { + pub credentials: Credentials, + pub origin: CredentialOrigin, +} + #[derive(Debug)] pub struct AcceptorResult { pub static_channels: StaticChannelSet, @@ -116,18 +121,17 @@ pub struct AcceptorResult { /// implement UDP multitransport can use it to decide whether to send a /// Server Initiate Multitransport Request. pub multitransport_flags: gcc::MultiTransportFlags, - /// Credentials received from the client. + /// Credentials received from the client together with their origin. /// /// Present for TLS-mode connections where the client sends credentials /// in the ClientInfoPdu, and for CredSSP/Hybrid connections once the /// delegated TSPasswordCreds have been decrypted by CredSSP. /// /// Servers that need to validate credentials (e.g., via PAM or LDAP) - /// can use this field for post-handshake validation. Check - /// [`Self::credentials_origin`] to distinguish unauthenticated ClientInfo - /// credentials from CredSSP-delegated credentials authenticated by the exchange. - pub credentials: Option, - pub credentials_origin: Option, + /// can use this field for post-handshake validation. The origin distinguishes + /// unauthenticated ClientInfo credentials from CredSSP-delegated credentials + /// authenticated by the exchange. + pub received_credentials: Option, /// Client Auto-Reconnect Packet received in the Client Info PDU. /// /// This is present when the client resumes a session using an @@ -158,7 +162,6 @@ impl Acceptor { saved_for_reactivation: Default::default(), creds, received_credentials: None, - received_credentials_origin: None, received_auto_reconnect: None, reactivation: false, honor_client_desktop_size: None, @@ -245,7 +248,6 @@ impl Acceptor { saved_for_reactivation, creds: consumed.creds, received_credentials: consumed.received_credentials, - received_credentials_origin: consumed.received_credentials_origin, received_auto_reconnect: consumed.received_auto_reconnect, reactivation: true, honor_client_desktop_size: consumed.honor_client_desktop_size, @@ -314,12 +316,14 @@ impl Acceptor { /// same post-handshake validation and binding path as TLS ClientInfo /// credentials. pub(crate) fn set_received_credssp_credentials(&mut self, identity: AuthIdentity) { - self.received_credentials = Some(Credentials { - username: identity.username.account_name().to_owned(), - password: identity.password.as_ref().clone(), - domain: identity.username.domain_name().map(str::to_owned), + self.received_credentials = Some(ReceivedCredentials { + credentials: Credentials { + username: identity.username.account_name().to_owned(), + password: identity.password.as_ref().clone(), + domain: identity.username.domain_name().map(str::to_owned), + }, + origin: CredentialOrigin::CredSspDelegated, }); - self.received_credentials_origin = Some(CredentialOrigin::CredSspDelegated); } /// # Panics @@ -348,8 +352,7 @@ impl Acceptor { keyboard_layout: self.keyboard_layout, multitransport_flags: self.multitransport_flags, reactivation: self.reactivation, - credentials: self.received_credentials.take(), - credentials_origin: self.received_credentials_origin.take(), + received_credentials: self.received_credentials.take(), auto_reconnect: self.received_auto_reconnect.take(), }), previous_state => { @@ -827,8 +830,10 @@ impl Sequence for Acceptor { } // Store credentials for later retrieval via AcceptorResult. - self.received_credentials = Some(creds); - self.received_credentials_origin = Some(CredentialOrigin::ClientInfo); + self.received_credentials = Some(ReceivedCredentials { + credentials: creds, + origin: CredentialOrigin::ClientInfo, + }); } ( diff --git a/crates/ironrdp-acceptor/src/lib.rs b/crates/ironrdp-acceptor/src/lib.rs index 771226a3c..776443202 100644 --- a/crates/ironrdp-acceptor/src/lib.rs +++ b/crates/ironrdp-acceptor/src/lib.rs @@ -18,7 +18,7 @@ pub use ironrdp_connector::DesktopSize; use ironrdp_pdu::nego; pub use self::channel_connection::{ChannelConnectionSequence, ChannelConnectionState}; -pub use self::connection::{Acceptor, AcceptorResult, AcceptorState, CredentialOrigin}; +pub use self::connection::{Acceptor, AcceptorResult, AcceptorState, CredentialOrigin, ReceivedCredentials}; pub use self::finalization::{FinalizationSequence, FinalizationState}; use crate::credssp::resolve_generator; diff --git a/crates/ironrdp-server/src/lib.rs b/crates/ironrdp-server/src/lib.rs index ccd22a81d..b739d2584 100644 --- a/crates/ironrdp-server/src/lib.rs +++ b/crates/ironrdp-server/src/lib.rs @@ -32,7 +32,7 @@ pub use gfx::{EgfxServerMessage, GfxDvcBridge, GfxServerFactory, GfxServerHandle pub use handler::{KeyboardEvent, MouseEvent, RdpServerInputHandler}; #[cfg(feature = "helper")] pub use helper::TlsIdentityCtx; -pub use ironrdp_acceptor::CredentialOrigin; +pub use ironrdp_acceptor::{CredentialOrigin, ReceivedCredentials}; pub use ironrdp_pdu::rdp::session_info::ServerAutoReconnect; pub use server::{ AutoReconnectCookieHandle, BoundConnection, ConnectionBinder, ConnectionHandler, CredentialDecision, diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 07f97f94b..b09eefc68 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -6,7 +6,7 @@ use std::rc::Rc; use std::sync::{Arc, Mutex as StdMutex}; use anyhow::{Context as _, Result, bail}; -use ironrdp_acceptor::{Acceptor, AcceptorResult, BeginResult, CredentialOrigin, DesktopSize}; +use ironrdp_acceptor::{Acceptor, AcceptorResult, BeginResult, CredentialOrigin, DesktopSize, ReceivedCredentials}; use ironrdp_async::Framed; use ironrdp_cliprdr::CliprdrServer; use ironrdp_cliprdr::backend::ClipboardMessage; @@ -1816,8 +1816,7 @@ impl RdpServer { }; let authenticated_credentials = match resolve_authenticated_credentials( credential_validator, - result.credentials.as_ref(), - result.credentials_origin, + result.received_credentials.as_ref(), result.reactivation, ) .await @@ -1832,7 +1831,8 @@ impl RdpServer { if !result.reactivation { if let Some(binder) = self.connection_binder.clone() { if self.credential_validator.is_none() - && result.credentials_origin != Some(CredentialOrigin::CredSspDelegated) + && result.received_credentials.as_ref().map(|received| received.origin) + != Some(CredentialOrigin::CredSspDelegated) { warn!("Connection binder requires authenticated credentials from a validator or CredSSP"); send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; @@ -2265,17 +2265,13 @@ impl RdpServer { async fn resolve_authenticated_credentials( credential_validator: Option>, - result_credentials: Option<&Credentials>, - credentials_origin: Option, + received_credentials: Option<&ReceivedCredentials>, reactivation: bool, ) -> Result> { - if let Some(creds) = result_credentials { - let Some(origin) = credentials_origin else { - bail!("credentials provided without an origin"); - }; - + if let Some(received) = received_credentials { + let creds = &received.credentials; if let Some(validator) = credential_validator { - match validator.validate(creds, origin).await { + match validator.validate(creds, received.origin).await { Ok(CredentialDecision::Accept) => { debug!("Credential validation accepted"); Ok(Some(creds)) @@ -2472,20 +2468,18 @@ mod wrdp_reactivation_tests { #[tokio::test] async fn reactivation_without_credentials_does_not_retain_validated_identity() { let validator: Arc = Arc::new(AllowUserValidator("alice")); - let initial_credentials = creds("alice"); + let initial_credentials = ReceivedCredentials { + credentials: creds("alice"), + origin: CredentialOrigin::ClientInfo, + }; - let first = resolve_authenticated_credentials( - Some(Arc::clone(&validator)), - Some(&initial_credentials), - Some(CredentialOrigin::ClientInfo), - false, - ) - .await - .expect("initial validation should succeed") - .expect("initial validation should produce credentials"); + let first = resolve_authenticated_credentials(Some(Arc::clone(&validator)), Some(&initial_credentials), false) + .await + .expect("initial validation should succeed") + .expect("initial validation should produce credentials"); assert_eq!(first.username, "alice"); - let reactivated = resolve_authenticated_credentials(Some(validator), None, None, true) + let reactivated = resolve_authenticated_credentials(Some(validator), None, true) .await .expect("missing reactivation credentials is not a backend error"); assert!(reactivated.is_none()); @@ -2494,17 +2488,15 @@ mod wrdp_reactivation_tests { #[tokio::test] async fn reactivation_with_credentials_revalidates_resent_identity() { let validator = Arc::new(AllowUserValidator("alice")); - let reactivation_credentials = creds("alice"); + let reactivation_credentials = ReceivedCredentials { + credentials: creds("alice"), + origin: CredentialOrigin::ClientInfo, + }; - let reactivated = resolve_authenticated_credentials( - Some(validator), - Some(&reactivation_credentials), - Some(CredentialOrigin::ClientInfo), - true, - ) - .await - .expect("resent reactivation credentials should be validated") - .expect("resent reactivation credentials should remain available"); + let reactivated = resolve_authenticated_credentials(Some(validator), Some(&reactivation_credentials), true) + .await + .expect("resent reactivation credentials should be validated") + .expect("resent reactivation credentials should remain available"); assert_eq!(reactivated.username, "alice"); } } From c1ff6d05c6fbaa3280112a6ecd1652dffbd2af84 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sat, 11 Jul 2026 08:10:29 +0000 Subject: [PATCH 15/21] Move credential handoff docs into rustdoc --- crates/ironrdp-acceptor/src/connection.rs | 33 ++++++++++++++++------ crates/ironrdp-server/src/server.rs | 21 ++++++++++---- docs/wrdp/auth-delegation.md | 14 --------- docs/wrdp/credssp-delegated-credentials.md | 14 --------- docs/wrdp/reactivation-credential-cache.md | 10 ------- 5 files changed, 39 insertions(+), 53 deletions(-) delete mode 100644 docs/wrdp/auth-delegation.md delete mode 100644 docs/wrdp/credssp-delegated-credentials.md delete mode 100644 docs/wrdp/reactivation-credential-cache.md diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index 93200ab1b..81d205371 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -77,14 +77,29 @@ fn set_bitmap_desktop_size(capabilities: &mut [CapabilitySet], size: DesktopSize } } +/// Protocol source and handshake-authentication status of received credentials. +/// +/// Servers must not infer this from their configured security mode: the origin +/// records what the acceptor actually received during negotiation. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum CredentialOrigin { - /// Received in the ClientInfoPdu (MS-RDPBCGR 2.2.1.11); not authenticated by the handshake. + /// Received in the ClientInfoPdu (MS-RDPBCGR 2.2.1.11). + /// + /// These credentials are client-supplied and have not been authenticated by + /// the protocol handshake. A server should validate them before using them + /// to select identity-bound resources. ClientInfo, - /// Delegated TSPasswordCreds decrypted by CredSSP (MS-CSSP); authenticated by the exchange. + /// Delegated TSPasswordCreds decrypted by CredSSP (MS-CSSP). + /// + /// CredSSP authenticated the principal during the exchange. A server may + /// still run authorization policy before starting or selecting a session. CredSspDelegated, } +/// Credentials received by the acceptor together with their protocol origin. +/// +/// Keeping both values in one type makes it impossible to expose credentials +/// without the provenance required to interpret their authentication status. #[derive(Debug)] pub struct ReceivedCredentials { pub credentials: Credentials, @@ -123,14 +138,14 @@ pub struct AcceptorResult { pub multitransport_flags: gcc::MultiTransportFlags, /// Credentials received from the client together with their origin. /// - /// Present for TLS-mode connections where the client sends credentials - /// in the ClientInfoPdu, and for CredSSP/Hybrid connections once the - /// delegated TSPasswordCreds have been decrypted by CredSSP. + /// For TLS/Standard connections, this contains credentials sent later in + /// the ClientInfoPdu and marks them as unauthenticated by the handshake. + /// For CredSSP/Hybrid connections, it contains the delegated TSPasswordCreds + /// decrypted by the CredSSP state machine and marks them as authenticated by + /// that exchange. /// - /// Servers that need to validate credentials (e.g., via PAM or LDAP) - /// can use this field for post-handshake validation. The origin distinguishes - /// unauthenticated ClientInfo credentials from CredSSP-delegated credentials - /// authenticated by the exchange. + /// Embedding servers can use the value for post-handshake validation or + /// authorization and for selecting per-user session resources. pub received_credentials: Option, /// Client Auto-Reconnect Packet received in the Client Info PDU. /// diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index b09eefc68..7994caf20 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -150,7 +150,9 @@ impl core::error::Error for CredentialValidationError { /// credentials from CredSSP-delegated credentials authenticated by the exchange. /// /// Implement this trait to validate or authorize credentials against external systems -/// (PAM, LDAP, database, etc.). For blocking backends, wrap the call in +/// (PAM, LDAP, database, etc.). ClientInfo credentials require authentication; +/// CredSSP-delegated credentials may still require server-specific authorization. +/// For blocking backends, wrap the call in /// `tokio::task::spawn_blocking` to avoid stalling the async runtime. /// /// # Example @@ -168,6 +170,7 @@ impl core::error::Error for CredentialValidationError { /// async fn validate( /// &self, /// creds: &Credentials, +/// _origin: CredentialOrigin, /// ) -> Result { /// if creds.username == self.expected_user && creds.password == self.expected_password { /// Ok(CredentialDecision::Accept) @@ -208,12 +211,18 @@ pub struct BoundConnection { pub input: Box, } -/// Async post-auth connection binder. +/// Async post-auth connection binder for identity-bound display and input resources. /// -/// This hook runs once authenticated credentials are available and before -/// static channels, display updates, or input dispatch begin. It lets a server -/// bind display/input resources to the authenticated identity without creating -/// per-user resources before authentication. +/// A multi-user server can keep protocol ownership inside IronRDP while keeping +/// account authorization and session lifecycle outside the RDP state machine. +/// It may start with placeholder handlers, authenticate or authorize the received +/// credentials, then use this hook to start or locate the user's session and +/// install the returned handlers before static channels, display updates, or +/// input dispatch begin. +/// +/// The binder runs once for the initial acceptance of a TCP connection. During +/// Deactivation-Reactivation (for example, a client resize), the existing bound +/// handlers remain installed and the binder is not called again. #[async_trait::async_trait] pub trait ConnectionBinder: Send + Sync { async fn bind_connection(&self, credentials: &Credentials) -> Result; diff --git a/docs/wrdp/auth-delegation.md b/docs/wrdp/auth-delegation.md deleted file mode 100644 index 1f7f1cfb6..000000000 --- a/docs/wrdp/auth-delegation.md +++ /dev/null @@ -1,14 +0,0 @@ -# Post-auth connection binding for multi-user servers - -`wrdp` follows the same multi-user architecture model as `xrdp-sesman`: a -single public RDP listener authenticates the client first, then delegates the -connection to a per-user desktop/session stack. - -That model needs a server hook that runs after credentials have been accepted -but before display updates and input dispatch begin. The hook lets a server -start or locate the authenticated user's session and then replace placeholder -handlers with display/input handlers bound to that session. - -The `ConnectionBinder` API keeps protocol ownership inside IronRDP while -allowing downstream servers to keep user/session lifecycle code outside the RDP -state machine. diff --git a/docs/wrdp/credssp-delegated-credentials.md b/docs/wrdp/credssp-delegated-credentials.md deleted file mode 100644 index 032020be6..000000000 --- a/docs/wrdp/credssp-delegated-credentials.md +++ /dev/null @@ -1,14 +0,0 @@ -# CredSSP delegated credentials handoff - -`ironrdp-acceptor` already exposes credentials sent later in the TLS -SecureSettingsExchange path. CredSSP/Hybrid authentication completes earlier, so -servers that delegate final account checks after the protocol handshake also need -access to the decrypted `TSPasswordCreds` produced by the CredSSP server state -machine. - -This change carries the delegated identity from the CredSSP sequence into -`AcceptorResult::credentials`, matching the existing ClientInfoPdu handoff shape. - -This is useful for servers that follow the xrdp-sesman multi-user architecture -model: the RDP protocol stack authenticates the transport, then the embedding -server delegates account authorization and session launch to a separate service. diff --git a/docs/wrdp/reactivation-credential-cache.md b/docs/wrdp/reactivation-credential-cache.md deleted file mode 100644 index 2949076ac..000000000 --- a/docs/wrdp/reactivation-credential-cache.md +++ /dev/null @@ -1,10 +0,0 @@ -# Reactivation credential cache scope - -During Deactivation-Reactivation some clients do not send a second credentials -PDU. A server that binds display/input handlers after authentication still needs -the identity accepted earlier on the same TCP connection. - -The cache introduced here is deliberately scoped to `accept_finalize()`, i.e. to -one TCP connection. It allows same-connection reactivation to reuse the validated -identity but prevents a new TCP connection from inheriting credentials accepted -on a previous connection. From 01a917c1c20f2a89ce6388bd9eaba91185eaaf11 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Tue, 14 Jul 2026 06:35:19 +0000 Subject: [PATCH 16/21] Preserve credential validator backend errors --- crates/ironrdp-server/src/server.rs | 36 +++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 7994caf20..5f6de2be7 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -2291,7 +2291,7 @@ async fn resolve_authenticated_credentials( } Err(e) => { error!(error = %e, "Credential validator backend error"); - bail!("credential validation backend error"); + Err(e.into()) } } } else { @@ -2444,7 +2444,6 @@ impl<'a, W: FramedWrite> SharedWriter<'a, W> { } } - #[cfg(test)] mod wrdp_reactivation_tests { use super::*; @@ -2466,6 +2465,21 @@ mod wrdp_reactivation_tests { } } + struct FailingValidator; + + #[async_trait::async_trait] + impl CredentialValidator for FailingValidator { + async fn validate( + &self, + _credentials: &Credentials, + _origin: CredentialOrigin, + ) -> Result { + Err(CredentialValidationError::new(std::io::Error::other( + "backend unavailable", + ))) + } + } + fn creds(username: &str) -> Credentials { Credentials { username: username.to_owned(), @@ -2508,4 +2522,22 @@ mod wrdp_reactivation_tests { .expect("resent reactivation credentials should remain available"); assert_eq!(reactivated.username, "alice"); } + + #[tokio::test] + async fn credential_validator_backend_error_is_preserved() { + let validator: Arc = Arc::new(FailingValidator); + let received_credentials = ReceivedCredentials { + credentials: creds("alice"), + origin: CredentialOrigin::ClientInfo, + }; + + let error = resolve_authenticated_credentials(Some(validator), Some(&received_credentials), false) + .await + .expect_err("backend failure should be returned"); + let validation_error = error + .downcast_ref::() + .expect("credential validation error should remain downcastable"); + let source = core::error::Error::source(validation_error).expect("backend source should be preserved"); + assert_eq!(source.to_string(), "backend unavailable"); + } } From 2ecda48cd55902b899692e0fa2140b0536cd9f4b Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Mon, 3 Aug 2026 11:43:11 +0000 Subject: [PATCH 17/21] Validate credentials before session activation --- Cargo.lock | 1 + crates/ironrdp-acceptor/Cargo.toml | 1 + crates/ironrdp-acceptor/src/connection.rs | 51 ++++-- crates/ironrdp-acceptor/src/lib.rs | 97 ++++++++++- crates/ironrdp-server/src/server.rs | 194 +++++++++++++++------- 5 files changed, 262 insertions(+), 82 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 02947818a..959656c4d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2413,6 +2413,7 @@ dependencies = [ name = "ironrdp-acceptor" version = "0.10.0" dependencies = [ + "async-trait", "ironrdp-async", "ironrdp-connector", "ironrdp-core 0.2.1", diff --git a/crates/ironrdp-acceptor/Cargo.toml b/crates/ironrdp-acceptor/Cargo.toml index 71968d35a..7339a60d3 100644 --- a/crates/ironrdp-acceptor/Cargo.toml +++ b/crates/ironrdp-acceptor/Cargo.toml @@ -22,6 +22,7 @@ ironrdp-pdu = { path = "../ironrdp-pdu", version = "0.9" } # public ironrdp-svc = { path = "../ironrdp-svc", version = "0.8" } # public ironrdp-connector = { path = "../ironrdp-connector", version = "0.10" } # public ironrdp-async = { path = "../ironrdp-async", version = "0.10" } # public +async-trait = "0.1" tracing = { version = "0.1", features = ["log"] } [lints] diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index 81d205371..6e4dab1ff 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -40,6 +40,7 @@ pub struct Acceptor { saved_for_reactivation: AcceptorState, pub(crate) creds: Option, received_credentials: Option, + credentials_handled: bool, received_auto_reconnect: Option, reactivation: bool, honor_client_desktop_size: Option, @@ -100,7 +101,7 @@ pub enum CredentialOrigin { /// /// Keeping both values in one type makes it impossible to expose credentials /// without the provenance required to interpret their authentication status. -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct ReceivedCredentials { pub credentials: Credentials, pub origin: CredentialOrigin, @@ -136,17 +137,15 @@ pub struct AcceptorResult { /// implement UDP multitransport can use it to decide whether to send a /// Server Initiate Multitransport Request. pub multitransport_flags: gcc::MultiTransportFlags, - /// Credentials received from the client together with their origin. + /// Credentials received from the client during SecureSettingsExchange. /// - /// For TLS/Standard connections, this contains credentials sent later in - /// the ClientInfoPdu and marks them as unauthenticated by the handshake. - /// For CredSSP/Hybrid connections, it contains the delegated TSPasswordCreds - /// decrypted by the CredSSP state machine and marks them as authenticated by - /// that exchange. + /// Present for TLS-mode connections where the client sends credentials + /// in the ClientInfoPdu. For CredSSP/Hybrid connections, credentials are + /// handled before capability exchange and are not exposed here. /// - /// Embedding servers can use the value for post-handshake validation or - /// authorization and for selecting per-user session resources. - pub received_credentials: Option, + /// Servers that need to validate credentials (e.g., via PAM or LDAP) + /// should use a [`CredentialsHandler`](crate::CredentialsHandler). + pub credentials: Option, /// Client Auto-Reconnect Packet received in the Client Info PDU. /// /// This is present when the client resumes a session using an @@ -177,6 +176,7 @@ impl Acceptor { saved_for_reactivation: Default::default(), creds, received_credentials: None, + credentials_handled: false, received_auto_reconnect: None, reactivation: false, honor_client_desktop_size: None, @@ -263,6 +263,7 @@ impl Acceptor { saved_for_reactivation, creds: consumed.creds, received_credentials: consumed.received_credentials, + credentials_handled: consumed.credentials_handled, received_auto_reconnect: consumed.received_auto_reconnect, reactivation: true, honor_client_desktop_size: consumed.honor_client_desktop_size, @@ -327,6 +328,32 @@ impl Acceptor { matches!(self.state, AcceptorState::Credssp { .. }) } + pub fn desktop_size(&self) -> DesktopSize { + self.desktop_size + } + + pub fn is_reactivation(&self) -> bool { + self.reactivation + } + + pub fn is_ready_for_capability_exchange(&self) -> bool { + matches!(self.state, AcceptorState::CapabilitiesSendServer { .. }) + } + + /// Returns credentials received during the current handshake, if any. + pub fn received_credentials(&self) -> Option<&ReceivedCredentials> { + self.received_credentials.as_ref() + } + + /// Takes credentials received during the current handshake, if any. + pub fn credentials_need_handling(&self) -> bool { + self.received_credentials.is_some() && !self.credentials_handled + } + + pub fn mark_credentials_handled(&mut self) { + self.credentials_handled = true; + } + /// Store credentials delegated by CredSSP/NLA so server code can use the /// same post-handshake validation and binding path as TLS ClientInfo /// credentials. @@ -339,6 +366,7 @@ impl Acceptor { }, origin: CredentialOrigin::CredSspDelegated, }); + self.credentials_handled = false; } /// # Panics @@ -367,7 +395,7 @@ impl Acceptor { keyboard_layout: self.keyboard_layout, multitransport_flags: self.multitransport_flags, reactivation: self.reactivation, - received_credentials: self.received_credentials.take(), + credentials: self.received_credentials.take().map(|received| received.credentials), auto_reconnect: self.received_auto_reconnect.take(), }), previous_state => { @@ -849,6 +877,7 @@ impl Sequence for Acceptor { credentials: creds, origin: CredentialOrigin::ClientInfo, }); + self.credentials_handled = false; } ( diff --git a/crates/ironrdp-acceptor/src/lib.rs b/crates/ironrdp-acceptor/src/lib.rs index 776443202..01da41035 100644 --- a/crates/ironrdp-acceptor/src/lib.rs +++ b/crates/ironrdp-acceptor/src/lib.rs @@ -4,7 +4,7 @@ use ironrdp_async::{Framed, FramedRead, FramedWrite, NetworkClient, StreamWrapper, single_sequence_step}; use ironrdp_connector::sspi::credssp::EarlyUserAuthResult; use ironrdp_connector::sspi::{AuthIdentity, KerberosServerConfig, Username}; -use ironrdp_connector::{ConnectorResult, ServerName, custom_err, general_err}; +use ironrdp_connector::{ServerName, custom_err, general_err}; use ironrdp_core::WriteBuf; use tracing::{debug, instrument, trace}; @@ -14,7 +14,7 @@ pub mod credssp; mod finalization; mod util; -pub use ironrdp_connector::DesktopSize; +pub use ironrdp_connector::{ConnectorError, ConnectorErrorExt, ConnectorResult, DesktopSize}; use ironrdp_pdu::nego; pub use self::channel_connection::{ChannelConnectionSequence, ChannelConnectionState}; @@ -30,6 +30,25 @@ where Continue(Framed), } +#[async_trait::async_trait(?Send)] +pub trait CredentialsHandler { + async fn handle_credentials(&mut self, credentials: Option) -> ConnectorResult<()>; + + async fn prepare_capability_exchange(&mut self, desktop_size: DesktopSize) -> ConnectorResult<()> { + let _ = desktop_size; + Ok(()) + } +} + +struct NoopCredentialsHandler; + +#[async_trait::async_trait(?Send)] +impl CredentialsHandler for NoopCredentialsHandler { + async fn handle_credentials(&mut self, _credentials: Option) -> ConnectorResult<()> { + Ok(()) + } +} + pub async fn accept_begin(mut framed: Framed, acceptor: &mut Acceptor) -> ConnectorResult> where S: FramedRead + FramedWrite + StreamWrapper, @@ -63,17 +82,42 @@ where S: FramedRead + FramedWrite, N: NetworkClient, { - let mut buf = WriteBuf::new(); + accept_credssp_with( + framed, + acceptor, + network_client, + client_computer_name, + public_key, + kerberos_config, + &mut NoopCredentialsHandler, + ) + .await +} +/// Runs CredSSP and invokes `credentials_handler` before HYBRID_EX reports success. +pub async fn accept_credssp_with( + framed: &mut Framed, + acceptor: &mut Acceptor, + network_client: &mut N, + client_computer_name: ServerName, + public_key: Vec, + kerberos_config: Option, + credentials_handler: &mut H, +) -> ConnectorResult<()> +where + S: FramedRead + FramedWrite, + N: NetworkClient, + H: CredentialsHandler, +{ if acceptor.should_perform_credssp() { perform_credssp_step( framed, acceptor, network_client, - &mut buf, client_computer_name, public_key, kerberos_config, + credentials_handler, ) .await } else { @@ -82,11 +126,24 @@ where } pub async fn accept_finalize( + framed: Framed, + acceptor: &mut Acceptor, +) -> ConnectorResult<(Framed, AcceptorResult)> +where + S: FramedRead + FramedWrite, +{ + accept_finalize_with(framed, acceptor, &mut NoopCredentialsHandler).await +} + +/// Finalizes the RDP handshake and invokes `credentials_handler` before capability exchange. +pub async fn accept_finalize_with( mut framed: Framed, acceptor: &mut Acceptor, + credentials_handler: &mut H, ) -> ConnectorResult<(Framed, AcceptorResult)> where S: FramedRead + FramedWrite, + H: CredentialsHandler, { let mut buf = WriteBuf::new(); @@ -95,24 +152,37 @@ where return Ok((framed, result)); } single_sequence_step(&mut framed, acceptor, &mut buf).await?; + if acceptor.credentials_need_handling() { + credentials_handler + .handle_credentials(acceptor.received_credentials().cloned()) + .await?; + acceptor.mark_credentials_handled(); + } + if !acceptor.is_reactivation() && acceptor.is_ready_for_capability_exchange() { + credentials_handler + .prepare_capability_exchange(acceptor.desktop_size()) + .await?; + } } } #[instrument(level = "trace", skip_all, ret)] -async fn perform_credssp_step( +async fn perform_credssp_step( framed: &mut Framed, acceptor: &mut Acceptor, network_client: &mut N, - buf: &mut WriteBuf, client_computer_name: ServerName, public_key: Vec, kerberos_config: Option, + credentials_handler: &mut H, ) -> ConnectorResult<()> where S: FramedRead + FramedWrite, N: NetworkClient, + H: CredentialsHandler, { assert!(acceptor.should_perform_credssp()); + let mut buf = WriteBuf::new(); let AcceptorState::Credssp { protocol, .. } = acceptor.state else { unreachable!() }; @@ -121,13 +191,24 @@ where framed, acceptor, network_client, - buf, + &mut buf, client_computer_name, public_key, kerberos_config, ) .await; + let result = match result { + Ok(()) => { + credentials_handler + .handle_credentials(acceptor.received_credentials().cloned()) + .await?; + acceptor.mark_credentials_handled(); + Ok(()) + } + Err(error) => Err(error), + }; + if protocol.intersects(nego::SecurityProtocol::HYBRID_EX) { trace!(?result, "HYBRID_EX"); @@ -139,7 +220,7 @@ where buf.clear(); result - .to_buffer(&mut *buf) + .to_buffer(&mut buf) .map_err(|e| ironrdp_connector::custom_err!("to_buffer", e))?; let response = &buf[..result.buffer_len()]; framed diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 5f6de2be7..b2607fa06 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -6,7 +6,10 @@ use std::rc::Rc; use std::sync::{Arc, Mutex as StdMutex}; use anyhow::{Context as _, Result, bail}; -use ironrdp_acceptor::{Acceptor, AcceptorResult, BeginResult, CredentialOrigin, DesktopSize, ReceivedCredentials}; +use ironrdp_acceptor::{ + Acceptor, AcceptorResult, BeginResult, ConnectorError, ConnectorErrorExt as _, CredentialOrigin, + CredentialsHandler, DesktopSize, ReceivedCredentials, +}; use ironrdp_async::Framed; use ironrdp_cliprdr::CliprdrServer; use ironrdp_cliprdr::backend::ClipboardMessage; @@ -610,6 +613,8 @@ pub struct RdpServer { creds: Option, credential_validator: Option>, connection_binder: Option>, + pending_authenticated_credentials: Option, + pending_bound_connection: Option, local_addr: Option, autodetect: Option, connection_handler: Option>, @@ -780,6 +785,8 @@ impl RdpServer { creds: None, credential_validator: None, connection_binder: None, + pending_authenticated_credentials: None, + pending_bound_connection: None, local_addr: None, autodetect: None, connection_handler, @@ -1277,6 +1284,9 @@ impl RdpServer { }, BeginResult::Continue(framed) => { + self.clear_bound_connection().await; + self.pending_authenticated_credentials = None; + self.pending_bound_connection = None; self.accept_finalize(framed, acceptor).await?; } }; @@ -1299,6 +1309,9 @@ impl RdpServer { S: AsyncRead + AsyncWrite + Sync + Send + Unpin, { acceptor.mark_security_upgrade_as_done(); + self.clear_bound_connection().await; + self.pending_authenticated_credentials = None; + self.pending_bound_connection = None; if let RdpServerSecurity::Hybrid((_, pub_key)) = &self.opts.security { // Generic streams don't expose peer address. Use a neutral @@ -1306,13 +1319,14 @@ impl RdpServer { // uses this value in practice. let client_name = "rdp-client".to_owned(); - ironrdp_acceptor::accept_credssp( + ironrdp_acceptor::accept_credssp_with( &mut framed, &mut acceptor, &mut ironrdp_tokio::reqwest::ReqwestNetworkClient::new(), client_name.into(), pub_key.clone(), None, + self, ) .await?; } @@ -1787,6 +1801,44 @@ impl RdpServer { state } + async fn prepare_authenticated_connection( + &mut self, + received_credentials: Option, + ) -> core::result::Result<(), ConnectorError> { + let authenticated_credentials = + resolve_authenticated_credentials(self.credential_validator.clone(), received_credentials.as_ref(), false) + .await + .map_err(|error| ConnectorError::reason("credential validation failed", format!("{error:#}")))?; + + if self.credential_validator.is_some() && authenticated_credentials.is_none() { + return Err(ConnectorError::general("no credentials available for validation")); + } + + if let Some(binder) = self.connection_binder.clone() { + if self.credential_validator.is_none() + && received_credentials.as_ref().map(|received| received.origin) + != Some(CredentialOrigin::CredSspDelegated) + { + return Err(ConnectorError::general( + "connection binder requires authenticated credentials from a validator or CredSSP", + )); + } + + let credentials = authenticated_credentials + .as_ref() + .ok_or_else(|| ConnectorError::general("no authenticated credentials for connection binding"))?; + let bound = binder + .bind_connection(credentials) + .await + .map_err(|error| ConnectorError::reason("connection binder failed", format!("{error:#}")))?; + + self.pending_bound_connection = Some(bound); + } + + self.pending_authenticated_credentials = authenticated_credentials.cloned(); + Ok(()) + } + async fn client_accepted( &mut self, reader: &mut Framed, @@ -1812,62 +1864,15 @@ impl RdpServer { false }; - // Validate credentials if a validator is configured. The validator runs here, in the - // async server layer, rather than in the sans-I/O acceptor, because real validators - // (PAM/LDAP/DB) are I/O-bound. On rejection, deny with a ServerSetErrorInfoPdu before - // closing, matching the acceptor's exact-match denial path. Reactivation still validates - // credentials again if the client resends them before channel state is reused. - // A verified auto-reconnect cookie bypasses the configured credential validator. - let credential_validator = if is_auto_reconnect { - None - } else { - self.credential_validator.clone() - }; - let authenticated_credentials = match resolve_authenticated_credentials( - credential_validator, - result.received_credentials.as_ref(), - result.reactivation, - ) - .await + if result.reactivation { + debug!("Reactivation reuses the authenticated connection binding"); + } else if !is_auto_reconnect + && (self.credential_validator.is_some() || self.connection_binder.is_some()) + && self.pending_authenticated_credentials.is_none() { - Ok(credentials) => credentials, - Err(e) => { - send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; - return Err(e); - } - }; - - if !result.reactivation { - if let Some(binder) = self.connection_binder.clone() { - if self.credential_validator.is_none() - && result.received_credentials.as_ref().map(|received| received.origin) - != Some(CredentialOrigin::CredSspDelegated) - { - warn!("Connection binder requires authenticated credentials from a validator or CredSSP"); - send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; - bail!("connection binder requires authenticated credentials"); - } - - let Some(credentials) = authenticated_credentials.as_ref() else { - warn!("Connection binder configured but no authenticated credentials are available"); - send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; - bail!("no authenticated credentials for connection binding"); - }; - - // Bound handlers are connection-local: install them into the dispatch slots - // for this client only, then clear the slots when the connection ends. - let bound = match binder.bind_connection(credentials).await { - Ok(bound) => bound, - Err(e) => { - send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; - return Err(e).context("connection binder failed"); - } - }; - self.install_bound_connection(bound).await; - debug!("Connection binder installed display/input handlers"); - } - } else if self.connection_binder.is_some() { - debug!("Skipping connection binder during reactivation"); + warn!("Initial acceptance reached activation without prepared credentials"); + send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; + bail!("credentials were not prepared before activation"); } if !result.input_events.is_empty() { @@ -2229,13 +2234,8 @@ impl RdpServer { where S: AsyncRead + AsyncWrite + Sync + Send + Unpin, { - // Clear per-user resources once for this TCP connection. Do not clear - // inside the loop: reactivation re-enters client_accepted without - // rebinding, and must keep the existing display/input handlers. - self.clear_bound_connection().await; - loop { - let (new_framed, result) = ironrdp_acceptor::accept_finalize(framed, &mut acceptor) + let (new_framed, result) = ironrdp_acceptor::accept_finalize_with(framed, &mut acceptor, self) .await .context("failed to accept client during finalize")?; @@ -2272,6 +2272,20 @@ impl RdpServer { } } +fn validate_bound_display_size( + negotiated_size: DesktopSize, + bound_size: DesktopSize, +) -> core::result::Result<(), ConnectorError> { + if bound_size == negotiated_size { + Ok(()) + } else { + Err(ConnectorError::reason( + "bound display dimensions differ from negotiated dimensions", + format!("negotiated {negotiated_size:?}, bound {bound_size:?}"), + )) + } +} + async fn resolve_authenticated_credentials( credential_validator: Option>, received_credentials: Option<&ReceivedCredentials>, @@ -2444,6 +2458,45 @@ impl<'a, W: FramedWrite> SharedWriter<'a, W> { } } +#[async_trait::async_trait(?Send)] +impl CredentialsHandler for RdpServer { + async fn handle_credentials( + &mut self, + credentials: Option, + ) -> core::result::Result<(), ConnectorError> { + self.prepare_authenticated_connection(credentials).await + } + + async fn prepare_capability_exchange( + &mut self, + desktop_size: DesktopSize, + ) -> core::result::Result<(), ConnectorError> { + if (self.credential_validator.is_some() || self.connection_binder.is_some()) + && self.pending_authenticated_credentials.is_none() + { + return Err(ConnectorError::general( + "credentials were not prepared before capability exchange", + )); + } + + let Some(bound) = self.pending_bound_connection.take() else { + return Ok(()); + }; + + let mut bound_display = bound.display; + let bound_size = bound_display.size().await; + validate_bound_display_size(desktop_size, bound_size)?; + + self.install_bound_connection(BoundConnection { + display: bound_display, + input: bound.input, + }) + .await; + debug!(?bound_size, "Connection binder installed display/input handlers"); + Ok(()) + } +} + #[cfg(test)] mod wrdp_reactivation_tests { use super::*; @@ -2523,6 +2576,21 @@ mod wrdp_reactivation_tests { assert_eq!(reactivated.username, "alice"); } + #[test] + fn bound_display_size_must_match_negotiated_size() { + let negotiated = DesktopSize { + width: 1280, + height: 720, + }; + assert!(validate_bound_display_size(negotiated, negotiated).is_ok()); + + let incompatible = DesktopSize { + width: 1920, + height: 1080, + }; + assert!(validate_bound_display_size(negotiated, incompatible).is_err()); + } + #[tokio::test] async fn credential_validator_backend_error_is_preserved() { let validator: Arc = Arc::new(FailingValidator); From 68da924bbab6a4939982c31b28c7f9696411ee06 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Mon, 3 Aug 2026 18:51:01 +0100 Subject: [PATCH 18/21] Pass negotiated desktop size to connection binder --- crates/ironrdp-server/src/server.rs | 50 ++++++++++++++++------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 585398029..2f5b85fb1 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -234,12 +234,14 @@ pub struct BoundConnection { /// install the returned handlers before static channels, display updates, or /// input dispatch begin. /// -/// The binder runs once for the initial acceptance of a TCP connection. During -/// Deactivation-Reactivation (for example, a client resize), the existing bound -/// handlers remain installed and the binder is not called again. +/// The binder runs once for the initial acceptance of a TCP connection, after +/// the operational desktop size has been negotiated. `desktop_size` is the +/// exact size the returned display must expose. During Deactivation-Reactivation +/// (for example, a later client resize), the existing bound handlers remain +/// installed and the binder is not called again. #[async_trait::async_trait] pub trait ConnectionBinder: Send + Sync { - async fn bind_connection(&self, credentials: &Credentials) -> Result; + async fn bind_connection(&self, credentials: &Credentials, desktop_size: DesktopSize) -> Result; } struct BoundDisplaySlot { @@ -1826,25 +1828,13 @@ impl RdpServer { return Err(ConnectorError::general("no credentials available for validation")); } - if let Some(binder) = self.connection_binder.clone() { - if self.credential_validator.is_none() - && received_credentials.as_ref().map(|received| received.origin) - != Some(CredentialOrigin::CredSspDelegated) - { - return Err(ConnectorError::general( - "connection binder requires authenticated credentials from a validator or CredSSP", - )); - } - - let credentials = authenticated_credentials - .as_ref() - .ok_or_else(|| ConnectorError::general("no authenticated credentials for connection binding"))?; - let bound = binder - .bind_connection(credentials) - .await - .map_err(|error| ConnectorError::reason("connection binder failed", format!("{error:#}")))?; - - self.pending_bound_connection = Some(bound); + if self.connection_binder.is_some() + && self.credential_validator.is_none() + && received_credentials.as_ref().map(|received| received.origin) != Some(CredentialOrigin::CredSspDelegated) + { + return Err(ConnectorError::general( + "connection binder requires authenticated credentials from a validator or CredSSP", + )); } self.pending_authenticated_credentials = authenticated_credentials.cloned(); @@ -2491,6 +2481,20 @@ impl CredentialsHandler for RdpServer { )); } + if self.pending_bound_connection.is_none() + && let Some(binder) = self.connection_binder.clone() + { + let credentials = self + .pending_authenticated_credentials + .as_ref() + .ok_or_else(|| ConnectorError::general("no authenticated credentials for connection binding"))?; + let bound = binder + .bind_connection(credentials, desktop_size) + .await + .map_err(|error| ConnectorError::reason("connection binder failed", format!("{error:#}")))?; + self.pending_bound_connection = Some(bound); + } + let Some(bound) = self.pending_bound_connection.take() else { return Ok(()); }; From 11144ab8b8fc7eef092f3ca0f18c7f67659e3763 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Wed, 5 Aug 2026 15:49:02 +0100 Subject: [PATCH 19/21] Make fork CI and fuzz workflows manual-only --- .github/workflows/ci.yml | 6 +----- .github/workflows/fuzz.yml | 3 +-- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3d41d43b3..0a8d6b09d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,11 +1,7 @@ name: CI +# Fork policy: ordinary CI is opt-in to avoid duplicating upstream runs. on: - push: - branches: - - master - pull_request: - types: [opened, synchronize, reopened] workflow_dispatch: env: diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index 73dba8a41..7dcb01b49 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -1,9 +1,8 @@ name: Fuzz +# Fork policy: fuzzing is opt-in; upstream owns scheduled fuzz runs. on: workflow_dispatch: - schedule: - - cron: '12 3 * * 0' # At 03:12 AM UTC on Sunday. env: CARGO_INCREMENTAL: 0 From 62f6d9b2c295ccd8d50a8b65436093a24f8da6f4 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Wed, 5 Aug 2026 15:49:52 +0100 Subject: [PATCH 20/21] Make fork RDP automation manual-only --- .github/workflows/agentic-rdp.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/agentic-rdp.yml b/.github/workflows/agentic-rdp.yml index 80c9a4956..ba2a6de16 100644 --- a/.github/workflows/agentic-rdp.yml +++ b/.github/workflows/agentic-rdp.yml @@ -1,7 +1,7 @@ name: Agentic RDP +# Fork policy: expensive RDP automation runs only when explicitly requested. on: - push: workflow_dispatch: inputs: desktop_size: From 7f9c57071bdda4b29040426f9e1912dfd0da6772 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Wed, 5 Aug 2026 20:41:45 +0100 Subject: [PATCH 21/21] Make Advanced Input own mouse delivery --- crates/ironrdp-server/src/server.rs | 36 +++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 2f5b85fb1..a81869041 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -473,6 +473,7 @@ impl RdpServerSecurity { struct AInputHandler { handler: Arc>>, + active: Arc, } impl_as_any!(AInputHandler); @@ -485,12 +486,15 @@ impl dvc::DvcProcessor for AInputHandler { fn start(&mut self, _channel_id: u32) -> PduResult> { use ironrdp_ainput::{ServerPdu, VersionPdu}; + self.active.store(true, Ordering::Release); let pdu = ServerPdu::Version(VersionPdu::default()); Ok(vec![Box::new(pdu)]) } - fn close(&mut self, _channel_id: u32) {} + fn close(&mut self, _channel_id: u32) { + self.active.store(false, Ordering::Release); + } fn process(&mut self, _channel_id: u32, payload: &[u8]) -> PduResult> { use ironrdp_ainput::ClientPdu; @@ -607,6 +611,9 @@ pub struct RdpServer { // FIXME: replace with a channel and poll/process the handler? handler: Arc>>, display: Arc>>, + /// Advanced Input owns mouse delivery while its DVC is active. Core mouse + /// events from the overlapping FastPath path are then suppressed. + advanced_input_active: Arc, // ConnectionBinder installs per-user handlers into these slots. The // default handler/display above stay stable for the server lifetime, while // the slots are cleared at connection entry and after each connection to @@ -783,6 +790,7 @@ impl RdpServer { display, Arc::clone(&bound_display), )))), + advanced_input_active: Arc::new(AtomicBool::new(false)), bound_handler, bound_display, static_channels: StaticChannelSet::new(), @@ -1023,6 +1031,7 @@ impl RdpServer { async fn clear_bound_connection(&mut self) { self.bound_display.lock().expect("bound display lock poisoned").take(); self.bound_handler.lock().expect("bound input lock poisoned").take(); + self.advanced_input_active.store(false, Ordering::Release); } pub fn event_sender(&self) -> &mpsc::UnboundedSender { @@ -1149,6 +1158,7 @@ impl RdpServer { let dvc = dvc .with_dynamic_channel(AInputHandler { handler: Arc::clone(&self.handler), + active: Arc::clone(&self.advanced_input_active), }) .with_dynamic_channel(DisplayControlServer::new(Box::new(dcs_backend))); let echo_handle = self.echo_handle.clone(); @@ -2056,15 +2066,21 @@ impl RdpServer { } FastPathInputEvent::MouseEvent(mouse) => { - handler.mouse(mouse.into()); + if !self.advanced_input_active.load(Ordering::Acquire) { + handler.mouse(mouse.into()); + } } FastPathInputEvent::MouseEventEx(mouse) => { - handler.mouse(mouse.into()); + if !self.advanced_input_active.load(Ordering::Acquire) { + handler.mouse(mouse.into()); + } } FastPathInputEvent::MouseEventRel(mouse) => { - handler.mouse(mouse.into()); + if !self.advanced_input_active.load(Ordering::Acquire) { + handler.mouse(mouse.into()); + } } FastPathInputEvent::QoeEvent(quality) => { @@ -2216,15 +2232,21 @@ impl RdpServer { } ironrdp_pdu::input::InputEvent::Mouse(mouse) => { - handler.mouse(mouse.into()); + if !self.advanced_input_active.load(Ordering::Acquire) { + handler.mouse(mouse.into()); + } } ironrdp_pdu::input::InputEvent::MouseX(mouse) => { - handler.mouse(mouse.into()); + if !self.advanced_input_active.load(Ordering::Acquire) { + handler.mouse(mouse.into()); + } } ironrdp_pdu::input::InputEvent::MouseRel(mouse) => { - handler.mouse(mouse.into()); + if !self.advanced_input_active.load(Ordering::Acquire) { + handler.mouse(mouse.into()); + } } ironrdp_pdu::input::InputEvent::Unused(_) => {}