From 1634a4e782833d3be3dfecae58f0f1c2f9a1440f Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sun, 5 Jul 2026 07:12:09 +0000 Subject: [PATCH 01/33] Add post-auth connection binder hook --- crates/ironrdp-server/src/builder.rs | 14 ++++++- crates/ironrdp-server/src/lib.rs | 4 +- crates/ironrdp-server/src/server.rs | 57 +++++++++++++++++++++++++++- docs/wrdp/auth-delegation.md | 14 +++++++ 4 files changed, 84 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 d1d3e93f6e..145c3898dd 100644 --- a/crates/ironrdp-server/src/builder.rs +++ b/crates/ironrdp-server/src/builder.rs @@ -13,8 +13,8 @@ use super::display::{DesktopSize, RdpServerDisplay}; use super::gfx::GfxServerFactory; use super::handler::{KeyboardEvent, MouseEvent, RdpServerInputHandler}; use super::server::{ - ConnectionHandler, ConnectionPolicy, CredentialValidator, RdpServer, RdpServerOptions, RdpServerSecurity, - StaticChannelFactory, + ConnectionBinder, ConnectionHandler, ConnectionPolicy, CredentialValidator, RdpServer, RdpServerOptions, + RdpServerSecurity, StaticChannelFactory, }; use crate::error::ServerResult; #[cfg(feature = "usb")] @@ -52,6 +52,7 @@ pub struct BuilderDone { rdpeai_factory: Option>, connection_handler: Option>, credential_validator: Option>, + connection_binder: Option>, #[cfg(feature = "egfx")] gfx_factory: Option>, #[cfg(feature = "usb")] @@ -166,6 +167,7 @@ impl RdpServerBuilder { rdpeai_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")] @@ -203,6 +205,7 @@ impl RdpServerBuilder { rdpeai_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")] @@ -392,6 +395,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 @@ -551,6 +560,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 b1f9554080..73561a260f 100644 --- a/crates/ironrdp-server/src/lib.rs +++ b/crates/ironrdp-server/src/lib.rs @@ -53,8 +53,8 @@ pub use rdpei::{ TouchEventPdu, TouchFrame, }; pub use server::{ - AutoReconnectCookieHandle, ConnectionHandler, ConnectionInfo, ConnectionPolicy, CredentialDecision, - CredentialValidationError, CredentialValidator, Credentials, ErrorInfoDisconnectHandle, + AutoReconnectCookieHandle, BoundConnection, ConnectionBinder, ConnectionHandler, ConnectionInfo, ConnectionPolicy, + CredentialDecision, CredentialValidationError, CredentialValidator, Credentials, ErrorInfoDisconnectHandle, ExactMatchCredentialValidator, PostConnectionAction, RdpServer, RdpServerOptions, RdpServerSecurity, ServerEvent, ServerEventSender, StaticChannelFactory, TransportTls, pick_remotefx_entropy_coder, }; diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 37af847670..06525884a9 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -295,6 +295,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 @@ -752,6 +774,7 @@ pub struct RdpServer { ev_receiver: Arc>>, creds: Option, credential_validator: Option>, + connection_binder: Option>, local_addr: Option, /// The local address the current client reached. See /// [`Self::set_connection_local_addr`]. @@ -1632,6 +1655,7 @@ impl RdpServer { ev_receiver: Arc::new(Mutex::new(ev_receiver)), creds: None, credential_validator: None, + connection_binder: None, local_addr: None, connection_local_addr: None, autodetect_enabled: false, @@ -1905,6 +1929,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 } @@ -4212,11 +4245,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"); @@ -4233,7 +4269,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.reactivation { diff --git a/docs/wrdp/auth-delegation.md b/docs/wrdp/auth-delegation.md new file mode 100644 index 0000000000..1f7f1cfb6b --- /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 5bafaad3b796efa2524036729850e78a316b2611 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sun, 5 Jul 2026 07:16:53 +0000 Subject: [PATCH 02/33] Scope reactivation credential cache to connection --- crates/ironrdp-server/src/server.rs | 169 +++++++++++++++++---- docs/wrdp/reactivation-credential-cache.md | 10 ++ 2 files changed, 151 insertions(+), 28 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 06525884a9..495f0b8e7b 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -314,7 +314,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. @@ -4215,6 +4215,7 @@ impl RdpServer { result: AcceptorResult, udp_transport: Rc>>, pending_udp_accept: Option>>, + authenticated_credentials_cache: &mut Option, ) -> ServerResult where R: FramedRead, @@ -4245,35 +4246,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, supports_err_info, writer) - .await?; - return Err(ServerError::reason("credential validation", "rejected by validator")); - } - Err(e) => { - error!(error = %e, "Credential validator backend error"); - send_access_denied(result.io_channel_id, result.user_channel_id, supports_err_info, writer) - .await?; - return Err(ServerError::custom("credential validation", e)); - } - } - } 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 { @@ -4872,6 +4856,7 @@ impl RdpServer { // synchronously as part of finalize, so establishing it must never // block the RDP handshake finalize itself is driving. let mut pending_udp_accept: Option>> = None; + let mut authenticated_credentials_cache = None; loop { // Bounded: see `FINALIZE_TIMEOUT`. The bound belongs on THIS call @@ -4929,6 +4914,7 @@ impl RdpServer { result, Rc::clone(&udp_transport), pending_udp_accept.take(), + &mut authenticated_credentials_cache, ) .await? { @@ -4981,6 +4967,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 @@ -6024,3 +6055,85 @@ mod cliprdr_error_tests { assert!(writer.0.is_empty(), "a refused message has nothing to put on the wire"); } } + + +#[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 0000000000..2949076acb --- /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 8b022ab71da5db397c9eb32bd93edd58f8a9c2fc Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sat, 4 Jul 2026 19:34:26 +0000 Subject: [PATCH 03/33] 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 c342625d0b..e6bbfafa1f 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, MonotonicInstant, Sequence, State, Written, encode_x224_packet, general_err, reason_err, @@ -195,11 +196,11 @@ pub struct AcceptorResult { /// reported success. `None` when no request was sent, or a matching /// response never arrived. pub multitransport_response_success: Option, - /// 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. @@ -542,6 +543,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 e665724dbf..1be36eb09d 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 8f186029b8..cfa8227a62 100644 --- a/crates/ironrdp-acceptor/src/lib.rs +++ b/crates/ironrdp-acceptor/src/lib.rs @@ -264,7 +264,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 0000000000..032020be6c --- /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 be4c612a00ad3affe2061d75c089718685149070 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sun, 5 Jul 2026 09:26:54 +0000 Subject: [PATCH 04/33] 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 495f0b8e7b..5e0541dc4a 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -1931,9 +1931,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; } @@ -4251,13 +4251,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 { @@ -6098,15 +6105,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"); } @@ -6125,14 +6127,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 aee82d3d9e28216dfebded197fb15bdf97a784ac Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sun, 5 Jul 2026 09:30:35 +0000 Subject: [PATCH 05/33] 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 5e0541dc4a..93c9df9011 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -308,7 +308,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. @@ -4266,20 +4266,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.reactivation { From 61e6f8ac47f79e1c3769e91ebb878b5c260537ea Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Mon, 6 Jul 2026 17:40:24 +0000 Subject: [PATCH 06/33] Keep bound server handlers connection-local --- crates/ironrdp-server/src/server.rs | 159 +++++++++++++++++++++++++--- 1 file changed, 147 insertions(+), 12 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 93c9df9011..2c87b342c7 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -7,8 +7,7 @@ use core::time::Duration; use std::collections::HashMap; use std::collections::VecDeque; use std::rc::Rc; -use std::sync::Arc; -use std::sync::LazyLock; +use std::sync::{Arc, LazyLock, Mutex as StdMutex}; use std::time::Instant; use ironrdp_acceptor::{Acceptor, AcceptorResult, BeginResult, DesktopSize}; @@ -54,13 +53,13 @@ use tracing::{debug, error, info, trace, warn}; use crate::autodetect::{AutoDetectManager, AutoDetectOutcome}; 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}; use crate::error::{ServerError, ServerErrorExt as _, ServerErrorKind, ServerResult}; #[cfg(feature = "egfx")] use crate::gfx::{EgfxServerMessage, GfxServerFactory}; -use crate::handler::RdpServerInputHandler; +use crate::handler::{KeyboardEvent, MouseEvent, RdpServerInputHandler}; use crate::heartbeat::HeartbeatConfig; use crate::multitransport; use crate::rdpeai::RdpeaiServerFactory; @@ -317,6 +316,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 @@ -758,6 +862,8 @@ pub struct RdpServer { handler: Arc>>, display: Arc>>, static_channel_factories: Vec>, + bound_handler: Arc>>>, + bound_display: Arc>>>, sound_factory: Option>, cliprdr_factory: Option>, rdpei_factory: Option>, @@ -1633,11 +1739,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_channel_factories, sound_factory, cliprdr_factory, @@ -1938,6 +2054,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 } @@ -2637,6 +2763,7 @@ impl RdpServer { } continue; } + self.clear_bound_connection().await; let started = tokio::time::Instant::now(); @@ -4268,18 +4395,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 8f091bdc13f77b710cc91a6b29a45beb198c581b Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Mon, 6 Jul 2026 17:44:18 +0000 Subject: [PATCH 07/33] 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 2c87b342c7..652879cb01 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -4372,7 +4372,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 3d2834c95b7b70253b3e6e0b2f62b659a3f1f916 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Mon, 6 Jul 2026 17:48:18 +0000 Subject: [PATCH 08/33] 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 145c3898dd..40b273fb38 100644 --- a/crates/ironrdp-server/src/builder.rs +++ b/crates/ironrdp-server/src/builder.rs @@ -376,20 +376,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 652879cb01..69c170d4c7 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -294,10 +294,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 { @@ -1807,11 +1807,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. @@ -1824,8 +1825,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 7074e63b8b561ea96100e2fffe41c2167415ba40 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Mon, 6 Jul 2026 17:54:29 +0000 Subject: [PATCH 09/33] Avoid caching authenticated credentials --- crates/ironrdp-server/src/server.rs | 82 +++++++++-------------------- 1 file changed, 25 insertions(+), 57 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 69c170d4c7..0b3b081ab0 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -4341,7 +4341,6 @@ impl RdpServer { result: AcceptorResult, udp_transport: Rc>>, pending_udp_accept: Option>>, - authenticated_credentials_cache: &mut Option, ) -> ServerResult where R: FramedRead, @@ -4371,8 +4370,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 @@ -4383,7 +4382,6 @@ impl RdpServer { credential_validator, result.credentials.as_ref(), result.reactivation, - authenticated_credentials_cache, ) .await { @@ -5003,7 +5001,6 @@ impl RdpServer { // synchronously as part of finalize, so establishing it must never // block the RDP handshake finalize itself is driving. let mut pending_udp_accept: Option>> = None; - let mut authenticated_credentials_cache = None; loop { // Bounded: see `FINALIZE_TIMEOUT`. The bound belongs on THIS call @@ -5061,7 +5058,6 @@ impl RdpServer { result, Rc::clone(&udp_transport), pending_udp_accept.take(), - &mut authenticated_credentials_cache, ) .await? { @@ -5118,15 +5114,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"); @@ -5137,24 +5131,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) } } @@ -6230,47 +6214,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 9ae3bc21e9890133105deb00bccaa557fee2e1a4 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Wed, 8 Jul 2026 07:04:38 +0000 Subject: [PATCH 10/33] 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 0b3b081ab0..843d867bee 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -862,6 +862,9 @@ pub struct RdpServer { handler: Arc>>, display: Arc>>, static_channel_factories: Vec>, + // 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>>>, sound_factory: Option>, @@ -4406,6 +4409,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) => { @@ -6215,10 +6220,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 c4d47b85687ef3cc2908bf031a76f0a6e43b0b7e Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Wed, 8 Jul 2026 07:36:55 +0000 Subject: [PATCH 11/33] 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 | 2 +- crates/ironrdp-server/src/server.rs | 87 ++++++++++++++----- .../tests/server/credential_validator.rs | 42 +++++++-- 6 files changed, 122 insertions(+), 40 deletions(-) diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index e6bbfafa1f..77433b6971 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -53,6 +53,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, @@ -142,6 +143,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, @@ -203,8 +212,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 @@ -238,6 +250,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, @@ -473,6 +486,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, @@ -552,6 +566,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 @@ -589,6 +604,7 @@ impl Acceptor { client_early_capability_flags: self.early_capability_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 => { @@ -1116,6 +1132,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 cfa8227a62..20ec4e98c4 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, MultitransportSecurityRng}; +pub use self::connection::{Acceptor, AcceptorResult, AcceptorState, CredentialOrigin, MultitransportSecurityRng}; 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 40b273fb38..ec4371cc5b 100644 --- a/crates/ironrdp-server/src/builder.rs +++ b/crates/ironrdp-server/src/builder.rs @@ -379,11 +379,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 73561a260f..bc2b2b40cd 100644 --- a/crates/ironrdp-server/src/lib.rs +++ b/crates/ironrdp-server/src/lib.rs @@ -39,7 +39,7 @@ pub use gfx::{EgfxServerMessage, GfxDvcBridge, GfxServerFactory, GfxServerHandle pub use handler::{KeyboardEvent, MouseButton, MouseEvent, RdpServerInputHandler}; #[cfg(feature = "helper")] pub use helper::TlsIdentityCtx; -pub use ironrdp_acceptor::Acceptor; +pub use ironrdp_acceptor::{Acceptor, CredentialOrigin}; pub use ironrdp_pdu::rdp::server_error_info::ErrorInfo; pub use ironrdp_pdu::rdp::session_info::ServerAutoReconnect; #[cfg(feature = "usb")] diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 843d867bee..c98ee5332c 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -10,7 +10,8 @@ use std::rc::Rc; use std::sync::{Arc, LazyLock, Mutex as StdMutex}; use std::time::Instant; -use ironrdp_acceptor::{Acceptor, AcceptorResult, BeginResult, DesktopSize}; +use anyhow::{Context as _, Result, bail}; +use ironrdp_acceptor::{Acceptor, AcceptorResult, BeginResult, CredentialOrigin, DesktopSize}; use ironrdp_async::Framed; use ironrdp_cliprdr::CliprdrServer; use ironrdp_cliprdr::backend::ClipboardMessage; @@ -244,20 +245,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, @@ -280,7 +282,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 @@ -291,7 +293,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. @@ -318,6 +324,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>>>, } @@ -387,6 +396,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>>>, } @@ -439,7 +450,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 { @@ -864,7 +879,8 @@ pub struct RdpServer { static_channel_factories: Vec>, // 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>>>, sound_factory: Option>, @@ -4350,6 +4366,7 @@ impl RdpServer { W: FramedWrite, { debug!("Client accepted"); + self.clear_bound_connection().await; // MS-RDPBCGR 3.3.5.7.1: a Set Error Info PDU MUST NOT be sent to a client that did not // set `SUPPORT_ERR_INFO_PDU`; such a client is just disconnected. @@ -4384,6 +4401,7 @@ impl RdpServer { let authenticated_credentials = match resolve_authenticated_credentials( credential_validator, result.credentials.as_ref(), + result.credentials_origin, result.reactivation, ) .await @@ -4397,8 +4415,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"); } @@ -5118,11 +5138,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)) @@ -6201,7 +6226,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 { @@ -6223,13 +6252,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()); @@ -6240,10 +6274,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 4b4dd280a3..e62840fc0a 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 f96d40868a3c4959fb570f48412d05c13f22d969 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sat, 11 Jul 2026 08:08:18 +0000 Subject: [PATCH 12/33] Preserve bound handlers during reactivation --- crates/ironrdp-server/src/server.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index c98ee5332c..c3d83a48c1 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -4366,7 +4366,6 @@ impl RdpServer { W: FramedWrite, { debug!("Client accepted"); - self.clear_bound_connection().await; // MS-RDPBCGR 3.3.5.7.1: a Set Error Info PDU MUST NOT be sent to a client that did not // set `SUPPORT_ERR_INFO_PDU`; such a client is just disconnected. @@ -5026,6 +5025,10 @@ impl RdpServer { // synchronously as part of finalize, so establishing it must never // block the RDP handshake finalize itself is driving. let mut pending_udp_accept: Option>> = None; + // 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 { // Bounded: see `FINALIZE_TIMEOUT`. The bound belongs on THIS call From 0613f4ebc0106d0ac356ccc12ed8f329c4c95cad Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sat, 11 Jul 2026 08:09:42 +0000 Subject: [PATCH 13/33] Pair received credentials with origin --- crates/ironrdp-acceptor/src/connection.rs | 43 +++++++++-------- crates/ironrdp-acceptor/src/lib.rs | 4 +- crates/ironrdp-server/src/lib.rs | 2 +- crates/ironrdp-server/src/server.rs | 58 ++++++++++------------- 4 files changed, 53 insertions(+), 54 deletions(-) diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index 77433b6971..1bbcf21df5 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -52,8 +52,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, @@ -151,6 +150,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, @@ -205,18 +210,17 @@ pub struct AcceptorResult { /// reported success. `None` when no request was sent, or a matching /// response never arrived. pub multitransport_response_success: Option, - /// 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 @@ -250,7 +254,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, @@ -486,7 +489,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, @@ -561,12 +563,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 @@ -603,8 +607,7 @@ impl Acceptor { multitransport_response_success: self.received_multitransport_response, client_early_capability_flags: self.early_capability_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 => { @@ -1131,8 +1134,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 20ec4e98c4..438892b9bc 100644 --- a/crates/ironrdp-acceptor/src/lib.rs +++ b/crates/ironrdp-acceptor/src/lib.rs @@ -18,7 +18,9 @@ pub use ironrdp_connector::DesktopSize; use ironrdp_pdu::nego; pub use self::channel_connection::{ChannelConnectionSequence, ChannelConnectionState}; -pub use self::connection::{Acceptor, AcceptorResult, AcceptorState, CredentialOrigin, MultitransportSecurityRng}; +pub use self::connection::{ + Acceptor, AcceptorResult, AcceptorState, CredentialOrigin, MultitransportSecurityRng, 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 bc2b2b40cd..9e9b6ea426 100644 --- a/crates/ironrdp-server/src/lib.rs +++ b/crates/ironrdp-server/src/lib.rs @@ -39,7 +39,7 @@ pub use gfx::{EgfxServerMessage, GfxDvcBridge, GfxServerFactory, GfxServerHandle pub use handler::{KeyboardEvent, MouseButton, MouseEvent, RdpServerInputHandler}; #[cfg(feature = "helper")] pub use helper::TlsIdentityCtx; -pub use ironrdp_acceptor::{Acceptor, CredentialOrigin}; +pub use ironrdp_acceptor::{Acceptor, CredentialOrigin, ReceivedCredentials}; pub use ironrdp_pdu::rdp::server_error_info::ErrorInfo; pub use ironrdp_pdu::rdp::session_info::ServerAutoReconnect; #[cfg(feature = "usb")] diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index c3d83a48c1..2e05721db7 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -11,7 +11,7 @@ use std::sync::{Arc, LazyLock, Mutex as StdMutex}; use std::time::Instant; 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; @@ -4399,8 +4399,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 @@ -4415,7 +4414,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?; @@ -5140,17 +5140,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)) @@ -6253,20 +6249,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()); @@ -6275,17 +6269,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 cc260ea346e81a0b9c6409434cb2010300f04e5e Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Sat, 11 Jul 2026 08:10:29 +0000 Subject: [PATCH 14/33] 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 1bbcf21df5..12964d7f3f 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -142,14 +142,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, @@ -212,14 +227,14 @@ pub struct AcceptorResult { pub multitransport_response_success: Option, /// 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 2e05721db7..ab05242562 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -253,7 +253,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 @@ -271,6 +273,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) @@ -311,12 +314,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 1f7f1cfb6b..0000000000 --- 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 032020be6c..0000000000 --- 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 2949076acb..0000000000 --- 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 db9e3c09bb4604881c8a528b21fd1669b3f86509 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Tue, 14 Jul 2026 06:35:19 +0000 Subject: [PATCH 15/33] 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 ab05242562..02f0c19004 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -5166,7 +5166,7 @@ async fn resolve_authenticated_credentials( } Err(e) => { error!(error = %e, "Credential validator backend error"); - bail!("credential validation backend error"); + Err(e.into()) } } } else { @@ -6225,7 +6225,6 @@ mod cliprdr_error_tests { } } - #[cfg(test)] mod wrdp_reactivation_tests { use super::*; @@ -6247,6 +6246,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(), @@ -6289,4 +6303,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 c2d292abeadbe73e6beed8ecf208a3eabbafe8e2 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Mon, 3 Aug 2026 11:43:11 +0000 Subject: [PATCH 16/33] 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 | 121 +++++++++++++-- crates/ironrdp-server/src/server.rs | 181 +++++++++++++++------- 5 files changed, 275 insertions(+), 80 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9dfa30a6a2..bcd384dddc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2602,6 +2602,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 6f08e9661e..3dafd1c8f4 100644 --- a/crates/ironrdp-acceptor/Cargo.toml +++ b/crates/ironrdp-acceptor/Cargo.toml @@ -23,6 +23,7 @@ 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 rand = "0.9" +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 12964d7f3f..baafecd814 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -53,6 +53,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, @@ -165,7 +166,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, @@ -225,17 +226,15 @@ pub struct AcceptorResult { /// reported success. `None` when no request was sent, or a matching /// response never arrived. pub multitransport_response_success: Option, - /// 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 @@ -269,6 +268,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, @@ -504,6 +504,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, @@ -574,6 +575,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. @@ -586,6 +613,7 @@ impl Acceptor { }, origin: CredentialOrigin::CredSspDelegated, }); + self.credentials_handled = false; } /// # Panics @@ -622,7 +650,7 @@ impl Acceptor { multitransport_response_success: self.received_multitransport_response, client_early_capability_flags: self.early_capability_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 => { @@ -1153,6 +1181,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 438892b9bc..12854168c5 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}; @@ -32,6 +32,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, @@ -65,17 +84,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 { @@ -90,7 +134,20 @@ pub async fn accept_finalize( where S: FramedRead + FramedWrite, { - accept_finalize_with_multitransport(framed, acceptor, |_, _| async {}).await + 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( + framed: Framed, + acceptor: &mut Acceptor, + credentials_handler: &mut H, +) -> ConnectorResult<(Framed, AcceptorResult)> +where + S: FramedRead + FramedWrite, + H: CredentialsHandler, +{ + accept_finalize_with_handlers(framed, acceptor, credentials_handler, |_, _| async {}).await } /// Completes the connection sequence, notifying `multitransport_handler` each @@ -116,13 +173,30 @@ where /// after `multitransport_request()` returned `Some`, which the two methods' /// own contract does not allow. pub async fn accept_finalize_with_multitransport( - mut framed: Framed, + framed: Framed, acceptor: &mut Acceptor, - mut multitransport_handler: H, + multitransport_handler: H, ) -> ConnectorResult<(Framed, AcceptorResult)> where S: FramedRead + FramedWrite, H: AsyncFnMut(ironrdp_pdu::rdp::multitransport::MultitransportRequestPdu, bool), +{ + accept_finalize_with_handlers(framed, acceptor, &mut NoopCredentialsHandler, multitransport_handler).await +} + +/// Finalizes the handshake with both connection-setup and multitransport handlers. +/// +/// The handlers follow the timing described by [`accept_finalize_with`] and [`accept_finalize_with_multitransport`]. +pub async fn accept_finalize_with_handlers( + mut framed: Framed, + acceptor: &mut Acceptor, + credentials_handler: &mut H, + mut multitransport_handler: M, +) -> ConnectorResult<(Framed, AcceptorResult)> +where + S: FramedRead + FramedWrite, + H: CredentialsHandler, + M: AsyncFnMut(ironrdp_pdu::rdp::multitransport::MultitransportRequestPdu, bool), { let mut buf = WriteBuf::new(); // `multitransport_request()` borrows rather than consumes (the field it @@ -144,7 +218,17 @@ where } 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?; + } if !notified && let Some(request) = acceptor.multitransport_request() { let request = request.clone(); let soft_sync = acceptor @@ -157,20 +241,22 @@ where } #[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!() }; @@ -179,13 +265,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"); @@ -197,7 +294,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 02f0c19004..05e278fa12 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -11,7 +11,10 @@ use std::sync::{Arc, LazyLock, Mutex as StdMutex}; use std::time::Instant; 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; @@ -909,6 +912,8 @@ pub struct RdpServer { creds: Option, credential_validator: Option>, connection_binder: Option>, + pending_authenticated_credentials: Option, + pending_bound_connection: Option, local_addr: Option, /// The local address the current client reached. See /// [`Self::set_connection_local_addr`]. @@ -1800,6 +1805,8 @@ impl RdpServer { creds: None, credential_validator: None, connection_binder: None, + pending_authenticated_credentials: None, + pending_bound_connection: None, local_addr: None, connection_local_addr: None, autodetect_enabled: false, @@ -4361,6 +4368,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, conn: &mut ConnectionState, @@ -4395,62 +4440,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.reactivation { @@ -5054,9 +5052,10 @@ impl RdpServer { // called, see `PendingConnection::new`), so the handler below // simply never runs and this is exactly `accept_finalize`'s own // behavior. - let finalize = ironrdp_acceptor::accept_finalize_with_multitransport( + let finalize = ironrdp_acceptor::accept_finalize_with_handlers( framed, &mut acceptor, + self, async |request, _soft_sync| { // Defensive only: reached only if the acceptor sent a // request despite one of these being unset, which @@ -5147,6 +5146,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>, @@ -6225,6 +6238,45 @@ mod cliprdr_error_tests { } } +#[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::*; @@ -6304,6 +6356,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 66808d26e8b35a75a0b1f7acbc41d3188f3f8bc4 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Mon, 3 Aug 2026 18:51:01 +0100 Subject: [PATCH 17/33] 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 05e278fa12..c1e3d53211 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -326,12 +326,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 { @@ -4381,25 +4383,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(); @@ -6259,6 +6249,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 3abac00007c60e4f396a90ad8cd21073dffc0070 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Wed, 5 Aug 2026 15:49:02 +0100 Subject: [PATCH 18/33] 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 2518c873d5..854adbddbb 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 73dba8a416..7dcb01b495 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 f6c972c2434b50f18fd1366924b7f54b4bd5f2ea Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Wed, 5 Aug 2026 15:49:52 +0100 Subject: [PATCH 19/33] Make fork RDP automation manual-only --- .github/workflows/agentic-rdp.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/agentic-rdp.yml b/.github/workflows/agentic-rdp.yml index 82fc43d005..ba2a6de169 100644 --- a/.github/workflows/agentic-rdp.yml +++ b/.github/workflows/agentic-rdp.yml @@ -1,5 +1,6 @@ name: Agentic RDP +# Fork policy: expensive RDP automation runs only when explicitly requested. on: workflow_dispatch: inputs: From 109461c3718057789276151253c0f79906ff5566 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Wed, 5 Aug 2026 20:41:45 +0100 Subject: [PATCH 20/33] 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 c1e3d53211..7eff42b293 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -732,6 +732,7 @@ impl RdpServerSecurity { struct AInputHandler { handler: Arc>>, + active: Arc, } impl_as_any!(AInputHandler); @@ -744,12 +745,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; @@ -891,6 +895,9 @@ pub struct RdpServer { handler: Arc>>, display: Arc>>, static_channel_factories: Vec>, + /// 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 @@ -1787,6 +1794,7 @@ impl RdpServer { display, Arc::clone(&bound_display), )))), + advanced_input_active: Arc::new(AtomicBool::new(false)), bound_handler, bound_display, static_channel_factories, @@ -2098,6 +2106,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 { @@ -2252,6 +2261,7 @@ impl RdpServer { let dvc = dvc::DrdynvcServer::new() .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))); @@ -4727,15 +4737,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) => { @@ -4973,15 +4989,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(_) => {} From 3c9728b96d3d214a99c94262a3b8a09ea4bd4caa Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Fri, 21 Aug 2026 17:19:36 +0100 Subject: [PATCH 21/33] Preserve HYBRID_EX credential denial response --- crates/ironrdp-acceptor/src/lib.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/ironrdp-acceptor/src/lib.rs b/crates/ironrdp-acceptor/src/lib.rs index 12854168c5..398ea9ee38 100644 --- a/crates/ironrdp-acceptor/src/lib.rs +++ b/crates/ironrdp-acceptor/src/lib.rs @@ -273,13 +273,16 @@ where .await; let result = match result { - Ok(()) => { - credentials_handler - .handle_credentials(acceptor.received_credentials().cloned()) - .await?; - acceptor.mark_credentials_handled(); - Ok(()) - } + Ok(()) => match credentials_handler + .handle_credentials(acceptor.received_credentials().cloned()) + .await + { + Ok(()) => { + acceptor.mark_credentials_handled(); + Ok(()) + } + Err(error) => Err(error), + }, Err(error) => Err(error), }; From fe2b70e0cf5f4f88907d5422b7af98029dc17648 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Fri, 21 Aug 2026 17:22:06 +0100 Subject: [PATCH 22/33] Send ERRINFO when credential hooks reject --- crates/ironrdp-acceptor/src/connection.rs | 19 ++++++++------- crates/ironrdp-acceptor/src/lib.rs | 28 +++++++++++++++++++---- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index baafecd814..2d3dc35c54 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -601,6 +601,16 @@ impl Acceptor { self.credentials_handled = true; } + /// Encode the protocol-defined access-denied response for a rejected + /// credential or capability-exchange hook. + pub fn encode_access_denied(&self, output: &mut WriteBuf) -> ConnectorResult { + let info = ServerSetErrorInfoPdu(ErrorInfo::ProtocolIndependentCode( + ProtocolIndependentCode::ServerDeniedConnection, + )); + debug!(message = ?info, "Send"); + util::encode_send_data_indication(self.user_channel_id, self.io_channel_id, &info, output) + } + /// Store credentials delegated by CredSSP/NLA so server code can use the /// same post-handshake validation and binding path as TLS ClientInfo /// credentials. @@ -1164,14 +1174,7 @@ impl Sequence for Acceptor { if expected != &creds { // FIXME: How authorization should be denied with standard RDP security? // Since standard RDP security is not a priority, we just send a ServerDeniedConnection ServerSetErrorInfo PDU. - let info = ServerSetErrorInfoPdu(ErrorInfo::ProtocolIndependentCode( - ProtocolIndependentCode::ServerDeniedConnection, - )); - - debug!(message = ?info, "Send"); - - util::encode_send_data_indication(self.user_channel_id, self.io_channel_id, &info, output)?; - + self.encode_access_denied(output)?; return Err(ConnectorError::general("invalid credentials")); } } diff --git a/crates/ironrdp-acceptor/src/lib.rs b/crates/ironrdp-acceptor/src/lib.rs index 398ea9ee38..be075f66e4 100644 --- a/crates/ironrdp-acceptor/src/lib.rs +++ b/crates/ironrdp-acceptor/src/lib.rs @@ -219,15 +219,33 @@ where single_sequence_step(&mut framed, acceptor, &mut buf).await?; if acceptor.credentials_need_handling() { - credentials_handler + if let Err(error) = credentials_handler .handle_credentials(acceptor.received_credentials().cloned()) - .await?; + .await + { + buf.clear(); + let written = acceptor.encode_access_denied(&mut buf)?; + framed + .write_all(&buf[..written]) + .await + .map_err(|e| ironrdp_connector::custom_err!("write access denied", e))?; + return Err(error); + } acceptor.mark_credentials_handled(); } - if !acceptor.is_reactivation() && acceptor.is_ready_for_capability_exchange() { - credentials_handler + if !acceptor.is_reactivation() + && acceptor.is_ready_for_capability_exchange() + && let Err(error) = credentials_handler .prepare_capability_exchange(acceptor.desktop_size()) - .await?; + .await + { + buf.clear(); + let written = acceptor.encode_access_denied(&mut buf)?; + framed + .write_all(&buf[..written]) + .await + .map_err(|e| ironrdp_connector::custom_err!("write access denied", e))?; + return Err(error); } if !notified && let Some(request) = acceptor.multitransport_request() { let request = request.clone(); From 0e13fcc03caca7e603d7cf3cf22119de07789d64 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Fri, 21 Aug 2026 17:24:12 +0100 Subject: [PATCH 23/33] Restore bound displays when async calls cancel --- crates/ironrdp-server/src/server.rs | 173 +++++++++++++++++++++++----- 1 file changed, 143 insertions(+), 30 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 7eff42b293..71f8d11813 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -338,12 +338,41 @@ 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. + // Async display methods lease the bound display from this slot. The lease + // restores it on cancellation unless a newer connection binding replaced it. bound: Arc>>>, } +struct BoundDisplayLease { + slot: Arc>>>, + display: Option>, +} + +impl BoundDisplayLease { + fn take(slot: &Arc>>>) -> Option { + let display = slot.lock().expect("bound display lock poisoned").take()?; + Some(Self { + slot: Arc::clone(slot), + display: Some(display), + }) + } + + fn display_mut(&mut self) -> &mut dyn RdpServerDisplay { + self.display + .as_deref_mut() + .expect("bound display lease always owns a display") + } +} + +impl Drop for BoundDisplayLease { + fn drop(&mut self) { + let mut slot = self.slot.lock().expect("bound display lock poisoned"); + if slot.is_none() { + *slot = self.display.take(); + } + } +} + impl BoundDisplaySlot { fn new(default: Box, bound: Arc>>>) -> Self { Self { default, bound } @@ -353,45 +382,24 @@ impl BoundDisplaySlot { #[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 + if let Some(mut lease) = BoundDisplayLease::take(&self.bound) { + lease.display_mut().size().await } 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 + if let Some(mut lease) = BoundDisplayLease::take(&self.bound) { + lease.display_mut().request_initial_size(client_size).await } 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 + if let Some(mut lease) = BoundDisplayLease::take(&self.bound) { + lease.display_mut().updates().await } else { self.default.updates().await } @@ -6415,3 +6423,108 @@ mod wrdp_reactivation_tests { assert_eq!(source.to_string(), "backend unavailable"); } } + +#[cfg(test)] +mod tests { + use core::future::pending; + + use ironrdp_core::impl_as_any; + use ironrdp_pdu::gcc::ChannelName; + use ironrdp_svc::{SvcMessage, SvcServerProcessor}; + + use super::*; + + /// A channel backend that owns a resource, released on drop the way + /// `RdpsndServer` stops its handler. + #[derive(Debug)] + struct ResourceChannel(Arc); + + impl Drop for ResourceChannel { + fn drop(&mut self) { + self.0.store(true, Ordering::Relaxed); + } + } + + impl_as_any!(ResourceChannel); + + impl SvcProcessor for ResourceChannel { + fn channel_name(&self) -> ChannelName { + ChannelName::from_static(b"testchan") + } + + fn process(&mut self, _payload: &[u8]) -> PduResult> { + Ok(Vec::new()) + } + } + + impl SvcServerProcessor for ResourceChannel {} + + struct PendingDisplay; + + #[async_trait::async_trait] + impl RdpServerDisplay for PendingDisplay { + async fn size(&mut self) -> DesktopSize { + pending().await + } + + async fn updates(&mut self) -> Result> { + pending().await + } + } + + struct FixedDisplay(DesktopSize); + + #[async_trait::async_trait] + impl RdpServerDisplay for FixedDisplay { + async fn size(&mut self) -> DesktopSize { + self.0 + } + + async fn updates(&mut self) -> Result> { + pending().await + } + } + + #[tokio::test] + async fn cancelling_bound_display_call_restores_authenticated_display() { + let expected = DesktopSize { + width: 1280, + height: 720, + }; + let bound = Arc::new(StdMutex::new(Some( + Box::new(PendingDisplay) as Box + ))); + let mut slot = BoundDisplaySlot::new(Box::new(FixedDisplay(expected)), Arc::clone(&bound)); + + assert!( + tokio::time::timeout(core::time::Duration::ZERO, slot.size()) + .await + .is_err() + ); + assert!(bound.lock().expect("bound display lock poisoned").is_some()); + } + + #[tokio::test] + async fn run_connection_releases_the_static_channels() { + let mut server = RdpServer::builder() + .with_addr(([127, 0, 0, 1], 0)) + .with_no_security() + .with_no_input() + .with_no_display() + .build(); + + let released = Arc::new(AtomicBool::new(false)); + server.static_channels.insert(ResourceChannel(Arc::clone(&released))); + + // A stream that is already at EOF: the connection ends early, which + // is the path an embedder's accept loop sees when a client vanishes. + let (client, server_side) = tokio::io::duplex(64); + drop(client); + let _ = server.run_connection(server_side).await; + + assert!( + released.load(Ordering::Relaxed), + "the channel backends of a finished connection must be released, not held until the next client" + ); + } +} From ce6186123510aa16de3629bcddd96766efcab223 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Fri, 21 Aug 2026 17:26:07 +0100 Subject: [PATCH 24/33] Remove dead credential reactivation state --- crates/ironrdp-server/src/server.rs | 88 ++++------------------------- 1 file changed, 12 insertions(+), 76 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 71f8d11813..247894ce15 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -930,7 +930,6 @@ pub struct RdpServer { credential_validator: Option>, connection_binder: Option>, pending_authenticated_credentials: Option, - pending_bound_connection: Option, local_addr: Option, /// The local address the current client reached. See /// [`Self::set_connection_local_addr`]. @@ -1824,7 +1823,6 @@ impl RdpServer { credential_validator: None, connection_binder: None, pending_authenticated_credentials: None, - pending_bound_connection: None, local_addr: None, connection_local_addr: None, autodetect_enabled: false, @@ -4393,7 +4391,7 @@ impl RdpServer { received_credentials: Option, ) -> core::result::Result<(), ConnectorError> { let authenticated_credentials = - resolve_authenticated_credentials(self.credential_validator.clone(), received_credentials.as_ref(), false) + resolve_authenticated_credentials(self.credential_validator.clone(), received_credentials.as_ref()) .await .map_err(|error| ConnectorError::reason("credential validation failed", format!("{error:#}")))?; @@ -5183,7 +5181,6 @@ fn validate_bound_display_size( async fn resolve_authenticated_credentials( credential_validator: Option>, received_credentials: Option<&ReceivedCredentials>, - reactivation: bool, ) -> Result> { if let Some(received) = received_credentials { let creds = &received.credentials; @@ -5205,9 +5202,6 @@ async fn resolve_authenticated_credentials( } else { Ok(Some(creds)) } - } else if reactivation { - debug!("Skipping credential validation for reactivation without credentials"); - Ok(None) } else { debug!("Skipping credential validation (no credentials in AcceptorResult)"); Ok(None) @@ -6279,23 +6273,17 @@ 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 { + let Some(binder) = self.connection_binder.clone() else { return Ok(()); }; + 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:#}")))?; let mut bound_display = bound.display; let bound_size = bound_display.size().await; @@ -6312,26 +6300,9 @@ impl CredentialsHandler for RdpServer { } #[cfg(test)] -mod wrdp_reactivation_tests { +mod credential_binding_tests { use super::*; - struct AllowUserValidator(&'static str); - - #[async_trait::async_trait] - impl CredentialValidator for AllowUserValidator { - async fn validate( - &self, - credentials: &Credentials, - _origin: CredentialOrigin, - ) -> Result { - if credentials.username == self.0 { - Ok(CredentialDecision::Accept) - } else { - Ok(CredentialDecision::Reject) - } - } - } - struct FailingValidator; #[async_trait::async_trait] @@ -6355,41 +6326,6 @@ 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 = ReceivedCredentials { - credentials: creds("alice"), - origin: CredentialOrigin::ClientInfo, - }; - - 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, true) - .await - .expect("missing reactivation credentials is not a backend error"); - assert!(reactivated.is_none()); - } - - #[tokio::test] - async fn reactivation_with_credentials_revalidates_resent_identity() { - let validator = Arc::new(AllowUserValidator("alice")); - let reactivation_credentials = ReceivedCredentials { - credentials: creds("alice"), - origin: CredentialOrigin::ClientInfo, - }; - - 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"); - } - #[test] fn bound_display_size_must_match_negotiated_size() { let negotiated = DesktopSize { @@ -6413,7 +6349,7 @@ mod wrdp_reactivation_tests { origin: CredentialOrigin::ClientInfo, }; - let error = resolve_authenticated_credentials(Some(validator), Some(&received_credentials), false) + let error = resolve_authenticated_credentials(Some(validator), Some(&received_credentials)) .await .expect_err("backend failure should be returned"); let validation_error = error From df3cd29c0180e61f7465ed4696ab483253cc2cf4 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Fri, 21 Aug 2026 17:27:10 +0100 Subject: [PATCH 25/33] Make bound connection slot updates synchronous --- crates/ironrdp-server/src/server.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 247894ce15..8f1d86eafe 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -2104,12 +2104,12 @@ impl RdpServer { self.connection_binder = binder; } - async fn install_bound_connection(&mut self, bound: BoundConnection) { + 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) { + 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); @@ -2815,7 +2815,7 @@ impl RdpServer { } continue; } - self.clear_bound_connection().await; + self.clear_bound_connection(); let started = tokio::time::Instant::now(); @@ -6292,8 +6292,7 @@ impl CredentialsHandler for RdpServer { self.install_bound_connection(BoundConnection { display: bound_display, input: bound.input, - }) - .await; + }); debug!(?bound_size, "Connection binder installed display/input handlers"); Ok(()) } From 8b9ae8af9a63ec285e7c67307e0f7bd388ace716 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Fri, 21 Aug 2026 17:28:30 +0100 Subject: [PATCH 26/33] Preserve credential validator API compatibility --- crates/ironrdp-server/src/server.rs | 35 +++++----- .../tests/server/credential_validator.rs | 65 +++++++++++-------- 2 files changed, 57 insertions(+), 43 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 8f1d86eafe..ead7bbe6c6 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -252,8 +252,8 @@ impl core::error::Error for CredentialValidationError { /// /// 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. +/// [`CredentialValidator::validate_received`] hook to distinguish unauthenticated +/// ClientInfo credentials from CredSSP-delegated credentials authenticated by the exchange. /// /// Implement this trait to validate or authorize credentials against external systems /// (PAM, LDAP, database, etc.). ClientInfo credentials require authentication; @@ -264,7 +264,7 @@ impl core::error::Error for CredentialValidationError { /// # Example /// /// ```ignore -/// use ironrdp_server::{CredentialDecision, CredentialOrigin, CredentialValidationError, CredentialValidator, Credentials}; +/// use ironrdp_server::{CredentialDecision, CredentialValidationError, CredentialValidator, Credentials}; /// /// struct StaticValidator { /// expected_user: String, @@ -276,7 +276,6 @@ 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) @@ -299,11 +298,21 @@ 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( + async fn validate(&self, credentials: &Credentials) -> Result; + + /// Validate credentials with their transport/authentication provenance. + /// + /// Existing validators remain source-compatible through the default + /// implementation. Validators that need different ClientInfo and CredSSP + /// policies can override this method. + async fn validate_received( &self, credentials: &Credentials, origin: CredentialOrigin, - ) -> Result; + ) -> Result { + let _ = origin; + self.validate(credentials).await + } } /// Display/input objects bound after the server authenticates a client. @@ -472,11 +481,7 @@ impl ExactMatchCredentialValidator { #[async_trait::async_trait] impl CredentialValidator for ExactMatchCredentialValidator { - async fn validate( - &self, - credentials: &Credentials, - _origin: CredentialOrigin, - ) -> Result { + async fn validate(&self, credentials: &Credentials) -> Result { if credentials == &self.expected { Ok(CredentialDecision::Accept) } else { @@ -5185,7 +5190,7 @@ async fn resolve_authenticated_credentials( if let Some(received) = received_credentials { let creds = &received.credentials; if let Some(validator) = credential_validator { - match validator.validate(creds, received.origin).await { + match validator.validate_received(creds, received.origin).await { Ok(CredentialDecision::Accept) => { debug!("Credential validation accepted"); Ok(Some(creds)) @@ -6306,11 +6311,7 @@ mod credential_binding_tests { #[async_trait::async_trait] impl CredentialValidator for FailingValidator { - async fn validate( - &self, - _credentials: &Credentials, - _origin: CredentialOrigin, - ) -> Result { + async fn validate(&self, _credentials: &Credentials) -> Result { Err(CredentialValidationError::new(std::io::Error::other( "backend unavailable", ))) diff --git a/crates/ironrdp-testsuite-core/tests/server/credential_validator.rs b/crates/ironrdp-testsuite-core/tests/server/credential_validator.rs index e62840fc0a..c6c1226a9d 100644 --- a/crates/ironrdp-testsuite-core/tests/server/credential_validator.rs +++ b/crates/ironrdp-testsuite-core/tests/server/credential_validator.rs @@ -17,11 +17,7 @@ fn fixed_creds() -> Credentials { struct AlwaysAccept; #[async_trait] impl CredentialValidator for AlwaysAccept { - async fn validate( - &self, - _: &Credentials, - _: CredentialOrigin, - ) -> Result { + async fn validate(&self, _: &Credentials) -> Result { Ok(CredentialDecision::Accept) } } @@ -29,11 +25,7 @@ impl CredentialValidator for AlwaysAccept { struct AlwaysReject; #[async_trait] impl CredentialValidator for AlwaysReject { - async fn validate( - &self, - _: &Credentials, - _: CredentialOrigin, - ) -> Result { + async fn validate(&self, _: &Credentials) -> Result { Ok(CredentialDecision::Reject) } } @@ -50,40 +42,47 @@ impl core::error::Error for BackendDown {} struct AlwaysBackendError; #[async_trait] impl CredentialValidator for AlwaysBackendError { - async fn validate( + async fn validate(&self, _: &Credentials) -> Result { + Err(CredentialValidationError::new(BackendDown)) + } +} + +struct CredSspOnly; +#[async_trait] +impl CredentialValidator for CredSspOnly { + async fn validate(&self, _: &Credentials) -> Result { + Ok(CredentialDecision::Reject) + } + + async fn validate_received( &self, _: &Credentials, - _: CredentialOrigin, + origin: CredentialOrigin, ) -> Result { - Err(CredentialValidationError::new(BackendDown)) + Ok(if origin == CredentialOrigin::CredSspDelegated { + CredentialDecision::Accept + } else { + CredentialDecision::Reject + }) } } #[tokio::test] async fn validator_accept_returns_accept() { let v = AlwaysAccept; - assert_eq!( - v.validate(&fixed_creds(), CredentialOrigin::ClientInfo).await.unwrap(), - CredentialDecision::Accept - ); + assert_eq!(v.validate(&fixed_creds()).await.unwrap(), CredentialDecision::Accept); } #[tokio::test] async fn validator_reject_returns_reject() { let v = AlwaysReject; - assert_eq!( - v.validate(&fixed_creds(), CredentialOrigin::ClientInfo).await.unwrap(), - CredentialDecision::Reject - ); + assert_eq!(v.validate(&fixed_creds()).await.unwrap(), CredentialDecision::Reject); } #[tokio::test] async fn validator_backend_error_propagates_source() { let v = AlwaysBackendError; - let err = v - .validate(&fixed_creds(), CredentialOrigin::ClientInfo) - .await - .expect_err("expected backend error"); + let err = v.validate(&fixed_creds()).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"); @@ -93,8 +92,22 @@ 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); +} + +#[tokio::test] +async fn validator_can_override_origin_aware_policy() { + let v = CredSspOnly; + assert_eq!( + v.validate_received(&fixed_creds(), CredentialOrigin::ClientInfo) + .await + .unwrap(), + CredentialDecision::Reject + ); assert_eq!( - v.validate(&fixed_creds(), CredentialOrigin::ClientInfo).await.unwrap(), + v.validate_received(&fixed_creds(), CredentialOrigin::CredSspDelegated) + .await + .unwrap(), CredentialDecision::Accept ); } From ac2c41234f31dfea8c38b84ec0eef5fc6b7e835b Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Fri, 21 Aug 2026 17:30:13 +0100 Subject: [PATCH 27/33] Keep delegated credentials out of acceptor results --- crates/ironrdp-acceptor/src/connection.rs | 53 ++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index 2d3dc35c54..cfdb9e2db7 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -601,6 +601,13 @@ impl Acceptor { self.credentials_handled = true; } + fn credentials_for_result(&mut self) -> Option { + self.received_credentials + .take() + .filter(|received| received.origin == CredentialOrigin::ClientInfo) + .map(|received| received.credentials) + } + /// Encode the protocol-defined access-denied response for a rejected /// credential or capability-exchange hook. pub fn encode_access_denied(&self, output: &mut WriteBuf) -> ConnectorResult { @@ -660,7 +667,7 @@ impl Acceptor { multitransport_response_success: self.received_multitransport_response, client_early_capability_flags: self.early_capability_flags, reactivation: self.reactivation, - credentials: self.received_credentials.take().map(|received| received.credentials), + credentials: self.credentials_for_result(), auto_reconnect: self.received_auto_reconnect.take(), }), previous_state => { @@ -1481,6 +1488,50 @@ impl Sequence for Acceptor { } } +#[cfg(test)] +mod tests { + use super::*; + + fn credentials(username: &str) -> Credentials { + Credentials { + username: username.to_owned(), + password: "secret".to_owned(), + domain: None, + } + } + + #[test] + fn acceptor_result_exposes_only_client_info_credentials() { + let mut acceptor = Acceptor::new( + SecurityProtocol::SSL, + DesktopSize { + width: 1024, + height: 768, + }, + Vec::new(), + None, + ); + + acceptor.received_credentials = Some(ReceivedCredentials { + credentials: credentials("nla-user"), + origin: CredentialOrigin::CredSspDelegated, + }); + assert!(acceptor.credentials_for_result().is_none()); + + acceptor.received_credentials = Some(ReceivedCredentials { + credentials: credentials("tls-user"), + origin: CredentialOrigin::ClientInfo, + }); + assert_eq!( + acceptor + .credentials_for_result() + .expect("ClientInfo credentials should remain available") + .username, + "tls-user" + ); + } +} + fn create_gcc_blocks( io_channel: u16, channel_ids: Vec, From 0086d11d931381e102ad98c23cffb3c16e3e9a00 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Fri, 21 Aug 2026 17:30:25 +0100 Subject: [PATCH 28/33] Restore upstream CI and fuzz triggers --- .github/workflows/agentic-rdp.yml | 1 - .github/workflows/ci.yml | 6 +++++- .github/workflows/fuzz.yml | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/agentic-rdp.yml b/.github/workflows/agentic-rdp.yml index ba2a6de169..82fc43d005 100644 --- a/.github/workflows/agentic-rdp.yml +++ b/.github/workflows/agentic-rdp.yml @@ -1,6 +1,5 @@ name: Agentic RDP -# Fork policy: expensive RDP automation runs only when explicitly requested. on: workflow_dispatch: inputs: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 854adbddbb..2518c873d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,7 +1,11 @@ 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 7dcb01b495..73dba8a416 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -1,8 +1,9 @@ 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 0c314272c43bd798bae31098a3203ad7a58ad716 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Fri, 21 Aug 2026 17:32:54 +0100 Subject: [PATCH 29/33] Activate Advanced Input ownership on first event --- crates/ironrdp-server/src/server.rs | 41 +++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index ead7bbe6c6..bf3a29dfc4 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -758,7 +758,6 @@ 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)]) @@ -773,6 +772,7 @@ impl dvc::DvcProcessor for AInputHandler { match decode(payload).map_err(|e| decode_err!(e))? { ClientPdu::Mouse(pdu) => { + self.active.store(true, Ordering::Release); let handler = Arc::clone(&self.handler); task::spawn_blocking(move || { handler.blocking_lock().mouse(pdu.into()); @@ -6364,7 +6364,8 @@ mod credential_binding_tests { mod tests { use core::future::pending; - use ironrdp_core::impl_as_any; + use ironrdp_core::{Encode as _, WriteCursor, impl_as_any}; + use ironrdp_dvc::DvcProcessor as _; use ironrdp_pdu::gcc::ChannelName; use ironrdp_svc::{SvcMessage, SvcServerProcessor}; @@ -6395,6 +6396,42 @@ mod tests { impl SvcServerProcessor for ResourceChannel {} + struct NoopInput; + + impl RdpServerInputHandler for NoopInput { + fn keyboard(&mut self, _event: KeyboardEvent) {} + fn mouse(&mut self, _event: MouseEvent) {} + } + + #[tokio::test] + async fn advanced_input_owns_mouse_only_after_first_pdu() { + use ironrdp_ainput::{ClientPdu, MouseEventFlags, MousePdu}; + + let active = Arc::new(AtomicBool::new(false)); + let mut handler = AInputHandler { + handler: Arc::new(Mutex::new(Box::new(NoopInput))), + active: Arc::clone(&active), + }; + + handler.start(1).expect("AInput start failed"); + assert!(!active.load(Ordering::Acquire)); + + let pdu = ClientPdu::Mouse(MousePdu { + time: 0, + flags: MouseEventFlags::MOVE, + x: 1, + y: 1, + }); + let mut payload = vec![0; pdu.size()]; + pdu.encode(&mut WriteCursor::new(&mut payload)) + .expect("AInput encoding failed"); + handler.process(1, &payload).expect("AInput processing failed"); + assert!(active.load(Ordering::Acquire)); + + handler.close(1); + assert!(!active.load(Ordering::Acquire)); + } + struct PendingDisplay; #[async_trait::async_trait] From 4019fc7848579d107c8b164c68446836c863a765 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Fri, 21 Aug 2026 17:36:02 +0100 Subject: [PATCH 30/33] Defer validation for auto-reconnect attempts --- crates/ironrdp-acceptor/src/connection.rs | 34 ++++++++++++++++++++++- crates/ironrdp-acceptor/src/lib.rs | 1 + crates/ironrdp-server/src/server.rs | 6 ++++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index cfdb9e2db7..1d5587a89f 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -583,6 +583,14 @@ impl Acceptor { self.reactivation } + /// Whether the client supplied a well-formed Client Auto-Reconnect Packet. + /// + /// The server still has to verify its security verifier before accepting + /// the connection. + pub fn is_auto_reconnect_attempt(&self) -> bool { + self.received_auto_reconnect.is_some() + } + pub fn is_ready_for_capability_exchange(&self) -> bool { matches!(self.state, AcceptorState::CapabilitiesSendServer { .. }) } @@ -594,7 +602,7 @@ impl Acceptor { /// Takes credentials received during the current handshake, if any. pub fn credentials_need_handling(&self) -> bool { - self.received_credentials.is_some() && !self.credentials_handled + !self.is_auto_reconnect_attempt() && self.received_credentials.is_some() && !self.credentials_handled } pub fn mark_credentials_handled(&mut self) { @@ -1530,6 +1538,30 @@ mod tests { "tls-user" ); } + + #[test] + fn auto_reconnect_defers_client_info_credentials() { + let mut acceptor = Acceptor::new( + SecurityProtocol::SSL, + DesktopSize { + width: 1024, + height: 768, + }, + Vec::new(), + None, + ); + acceptor.received_credentials = Some(ReceivedCredentials { + credentials: credentials("reconnect-user"), + origin: CredentialOrigin::ClientInfo, + }); + acceptor.received_auto_reconnect = Some(ClientAutoReconnect { + logon_id: 7, + security_verifier: [0x5A; 16], + }); + + assert!(acceptor.is_auto_reconnect_attempt()); + assert!(!acceptor.credentials_need_handling()); + } } fn create_gcc_blocks( diff --git a/crates/ironrdp-acceptor/src/lib.rs b/crates/ironrdp-acceptor/src/lib.rs index be075f66e4..cf768c47a1 100644 --- a/crates/ironrdp-acceptor/src/lib.rs +++ b/crates/ironrdp-acceptor/src/lib.rs @@ -234,6 +234,7 @@ where acceptor.mark_credentials_handled(); } if !acceptor.is_reactivation() + && !acceptor.is_auto_reconnect_attempt() && acceptor.is_ready_for_capability_exchange() && let Err(error) = credentials_handler .prepare_capability_exchange(acceptor.desktop_size()) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index bf3a29dfc4..a8debd2e8e 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -4445,6 +4445,12 @@ impl RdpServer { return Err(ServerError::reason("auto-reconnect validation", "cookie rejected")); } + if self.connection_binder.is_some() { + warn!("Auto-reconnect cannot restore a credential-bound connection safely"); + send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; + bail!("auto-reconnect is unsupported with a connection binder"); + } + debug!("Auto-reconnect cookie validation accepted"); true } else { From b736ce139bc40cd5412ca8d0e423d36c3be4fd73 Mon Sep 17 00:00:00 2001 From: Rui Carmo Date: Fri, 21 Aug 2026 17:51:05 +0100 Subject: [PATCH 31/33] Name credential hooks for connection setup --- crates/ironrdp-acceptor/src/connection.rs | 16 +++++++-------- crates/ironrdp-acceptor/src/lib.rs | 24 +++++++++++++---------- crates/ironrdp-server/src/server.rs | 6 +++--- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index 1d5587a89f..8ee9a25887 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -233,7 +233,7 @@ pub struct AcceptorResult { /// handled before capability exchange and are not exposed here. /// /// Servers that need to validate credentials (e.g., via PAM or LDAP) - /// should use a [`CredentialsHandler`](crate::CredentialsHandler). + /// should use a [`ConnectionSetupHandler`](crate::ConnectionSetupHandler). pub credentials: Option, /// Client Auto-Reconnect Packet received in the Client Info PDU. /// @@ -575,11 +575,11 @@ impl Acceptor { matches!(self.state, AcceptorState::Credssp { .. }) } - pub fn desktop_size(&self) -> DesktopSize { + pub(crate) fn desktop_size(&self) -> DesktopSize { self.desktop_size } - pub fn is_reactivation(&self) -> bool { + pub(crate) fn is_reactivation(&self) -> bool { self.reactivation } @@ -587,25 +587,25 @@ impl Acceptor { /// /// The server still has to verify its security verifier before accepting /// the connection. - pub fn is_auto_reconnect_attempt(&self) -> bool { + pub(crate) fn is_auto_reconnect_attempt(&self) -> bool { self.received_auto_reconnect.is_some() } - pub fn is_ready_for_capability_exchange(&self) -> bool { + pub(crate) 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> { + pub(crate) 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 { + pub(crate) fn credentials_need_handling(&self) -> bool { !self.is_auto_reconnect_attempt() && self.received_credentials.is_some() && !self.credentials_handled } - pub fn mark_credentials_handled(&mut self) { + pub(crate) fn mark_credentials_handled(&mut self) { self.credentials_handled = true; } diff --git a/crates/ironrdp-acceptor/src/lib.rs b/crates/ironrdp-acceptor/src/lib.rs index cf768c47a1..d4d91f02a1 100644 --- a/crates/ironrdp-acceptor/src/lib.rs +++ b/crates/ironrdp-acceptor/src/lib.rs @@ -32,8 +32,12 @@ where Continue(Framed), } +/// Async hooks that complete authenticated connection setup at protocol-safe points. +/// +/// Credential handling runs before HYBRID_EX reports success. Capability setup +/// runs after the operational desktop size is known but before capability exchange. #[async_trait::async_trait(?Send)] -pub trait CredentialsHandler { +pub trait ConnectionSetupHandler { async fn handle_credentials(&mut self, credentials: Option) -> ConnectorResult<()>; async fn prepare_capability_exchange(&mut self, desktop_size: DesktopSize) -> ConnectorResult<()> { @@ -42,10 +46,10 @@ pub trait CredentialsHandler { } } -struct NoopCredentialsHandler; +struct NoopConnectionSetupHandler; #[async_trait::async_trait(?Send)] -impl CredentialsHandler for NoopCredentialsHandler { +impl ConnectionSetupHandler for NoopConnectionSetupHandler { async fn handle_credentials(&mut self, _credentials: Option) -> ConnectorResult<()> { Ok(()) } @@ -91,12 +95,12 @@ where client_computer_name, public_key, kerberos_config, - &mut NoopCredentialsHandler, + &mut NoopConnectionSetupHandler, ) .await } -/// Runs CredSSP and invokes `credentials_handler` before HYBRID_EX reports success. +/// Runs CredSSP and invokes `connection_setup_handler` before HYBRID_EX reports success. pub async fn accept_credssp_with( framed: &mut Framed, acceptor: &mut Acceptor, @@ -109,7 +113,7 @@ pub async fn accept_credssp_with( where S: FramedRead + FramedWrite, N: NetworkClient, - H: CredentialsHandler, + H: ConnectionSetupHandler, { if acceptor.should_perform_credssp() { perform_credssp_step( @@ -134,10 +138,10 @@ pub async fn accept_finalize( where S: FramedRead + FramedWrite, { - accept_finalize_with(framed, acceptor, &mut NoopCredentialsHandler).await + accept_finalize_with(framed, acceptor, &mut NoopConnectionSetupHandler).await } -/// Finalizes the RDP handshake and invokes `credentials_handler` before capability exchange. +/// Finalizes the RDP handshake and invokes `connection_setup_handler` before capability exchange. pub async fn accept_finalize_with( framed: Framed, acceptor: &mut Acceptor, @@ -145,7 +149,7 @@ pub async fn accept_finalize_with( ) -> ConnectorResult<(Framed, AcceptorResult)> where S: FramedRead + FramedWrite, - H: CredentialsHandler, + H: ConnectionSetupHandler, { accept_finalize_with_handlers(framed, acceptor, credentials_handler, |_, _| async {}).await } @@ -272,7 +276,7 @@ async fn perform_credssp_step( where S: FramedRead + FramedWrite, N: NetworkClient, - H: CredentialsHandler, + H: ConnectionSetupHandler, { assert!(acceptor.should_perform_credssp()); let mut buf = WriteBuf::new(); diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index a8debd2e8e..061fb8cb51 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -12,8 +12,8 @@ use std::time::Instant; use anyhow::{Context as _, Result, bail}; use ironrdp_acceptor::{ - Acceptor, AcceptorResult, BeginResult, ConnectorError, ConnectorErrorExt as _, CredentialOrigin, - CredentialsHandler, DesktopSize, ReceivedCredentials, + Acceptor, AcceptorResult, BeginResult, ConnectionSetupHandler, ConnectorError, ConnectorErrorExt as _, + CredentialOrigin, DesktopSize, ReceivedCredentials, }; use ironrdp_async::Framed; use ironrdp_cliprdr::CliprdrServer; @@ -6264,7 +6264,7 @@ mod cliprdr_error_tests { } #[async_trait::async_trait(?Send)] -impl CredentialsHandler for RdpServer { +impl ConnectionSetupHandler for RdpServer { async fn handle_credentials( &mut self, credentials: Option, From 0d8af13545a30e6a0d1d35c68404a7b8ccfd311c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Thu, 8 Oct 2026 15:11:56 -0400 Subject: [PATCH 32/33] fix(server): preserve authorization semantics Encode capability-gated denials as complete Set Error Info PDUs using the insufficient-privileges code, including server denial paths. Defer plain HYBRID authorization until channel joins and ClientInfo. Keep HYBRID_EX early authorization and preserve complete CredSSP UPNs. Add handshake regressions for denial framing, protocol timing, and UPN and down-level identity handling. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- crates/ironrdp-acceptor/src/connection.rs | 52 +- crates/ironrdp-acceptor/src/lib.rs | 49 +- crates/ironrdp-server/src/server.rs | 519 ++++++++++++------ .../tests/server/acceptor.rs | 392 ++++++++++++- 4 files changed, 815 insertions(+), 197 deletions(-) diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index 8ee9a25887..e584758a7e 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -1,7 +1,7 @@ use core::any::TypeId; use core::mem; -use ironrdp_connector::sspi::AuthIdentity; +use ironrdp_connector::sspi::{AuthIdentity, UsernameParts}; use ironrdp_connector::{ ConnectorError, ConnectorErrorExt as _, ConnectorResult, DesktopSize, MonotonicInstant, Sequence, State, Written, encode_x224_packet, general_err, reason_err, @@ -168,6 +168,8 @@ pub enum CredentialOrigin { /// without the provenance required to interpret their authentication status. #[derive(Clone, Debug)] pub struct ReceivedCredentials { + /// CredSSP UPNs remain intact in `username`, with no `domain`. + /// Down-level names separate the account name and optional NetBIOS domain. pub credentials: Credentials, pub origin: CredentialOrigin, } @@ -600,9 +602,12 @@ impl Acceptor { self.received_credentials.as_ref() } - /// Takes credentials received during the current handshake, if any. + /// Whether credentials need handling after channel joins and ClientInfo. pub(crate) fn credentials_need_handling(&self) -> bool { - !self.is_auto_reconnect_attempt() && self.received_credentials.is_some() && !self.credentials_handled + matches!(self.state, AcceptorState::LicensingExchange { .. }) + && !self.is_auto_reconnect_attempt() + && self.received_credentials.is_some() + && !self.credentials_handled } pub(crate) fn mark_credentials_handled(&mut self) { @@ -616,25 +621,44 @@ impl Acceptor { .map(|received| received.credentials) } - /// Encode the protocol-defined access-denied response for a rejected - /// credential or capability-exchange hook. + /// Encodes a Set Error Info PDU for a user authorization failure ([MS-RDPBCGR] section 2.2.5.1.1). + /// + /// Returns zero without writing if the client did not advertise error-info support. + /// + /// [MS-RDPBCGR]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/ pub fn encode_access_denied(&self, output: &mut WriteBuf) -> ConnectorResult { - let info = ServerSetErrorInfoPdu(ErrorInfo::ProtocolIndependentCode( - ProtocolIndependentCode::ServerDeniedConnection, - )); + if !self + .early_capability_flags + .contains(gcc::ClientEarlyCapabilityFlags::SUPPORT_ERR_INFO_PDU) + { + debug!("Omitting access-denied PDU because the client did not advertise error-info support"); + return Ok(0); + } + + let info = wrap_share_data( + rdp::headers::ShareDataPdu::ServerSetErrorInfo(ServerSetErrorInfoPdu(ErrorInfo::ProtocolIndependentCode( + ProtocolIndependentCode::ServerInsufficientPrivileges, + ))), + 0, + ); debug!(message = ?info, "Send"); util::encode_send_data_indication(self.user_channel_id, self.io_channel_id, &info, output) } - /// Store credentials delegated by CredSSP/NLA so server code can use the - /// same post-handshake validation and binding path as TLS ClientInfo - /// credentials. + /// Stores CredSSP/NLA credentials for the same validation and binding path as TLS ClientInfo credentials. pub(crate) fn set_received_credssp_credentials(&mut self, identity: AuthIdentity) { + let (username, domain) = match identity.username.parts() { + UsernameParts::UserPrincipalName(parts) => (parts.upn().to_owned(), None), + UsernameParts::DownLevelLogonName(parts) => ( + parts.account_name().to_owned(), + parts.netbios_domain().map(str::to_owned), + ), + }; self.received_credentials = Some(ReceivedCredentials { credentials: Credentials { - username: identity.username.account_name().to_owned(), + username, password: identity.password.as_ref().clone(), - domain: identity.username.domain_name().map(str::to_owned), + domain, }, origin: CredentialOrigin::CredSspDelegated, }); @@ -1187,8 +1211,6 @@ impl Sequence for Acceptor { if let Some(expected) = &self.creds { if expected != &creds { - // FIXME: How authorization should be denied with standard RDP security? - // Since standard RDP security is not a priority, we just send a ServerDeniedConnection ServerSetErrorInfo PDU. self.encode_access_denied(output)?; return Err(ConnectorError::general("invalid credentials")); } diff --git a/crates/ironrdp-acceptor/src/lib.rs b/crates/ironrdp-acceptor/src/lib.rs index d4d91f02a1..04ef26af5b 100644 --- a/crates/ironrdp-acceptor/src/lib.rs +++ b/crates/ironrdp-acceptor/src/lib.rs @@ -34,8 +34,9 @@ where /// Async hooks that complete authenticated connection setup at protocol-safe points. /// -/// Credential handling runs before HYBRID_EX reports success. Capability setup -/// runs after the operational desktop size is known but before capability exchange. +/// Credential handling runs before HYBRID_EX reports success. +/// For TLS and plain HYBRID, it runs after channel joins and ClientInfo. +/// Capability setup runs after the operational desktop size is known but before capability exchange. #[async_trait::async_trait(?Send)] pub trait ConnectionSetupHandler { async fn handle_credentials(&mut self, credentials: Option) -> ConnectorResult<()>; @@ -100,7 +101,9 @@ where .await } -/// Runs CredSSP and invokes `connection_setup_handler` before HYBRID_EX reports success. +/// Runs CredSSP and invokes `credentials_handler` before HYBRID_EX reports success. +/// +/// Plain HYBRID defers credential handling to [`accept_finalize_with`] after channel joins. pub async fn accept_credssp_with( framed: &mut Framed, acceptor: &mut Acceptor, @@ -141,7 +144,7 @@ where accept_finalize_with(framed, acceptor, &mut NoopConnectionSetupHandler).await } -/// Finalizes the RDP handshake and invokes `connection_setup_handler` before capability exchange. +/// Finalizes the RDP handshake and invokes `credentials_handler` before capability exchange. pub async fn accept_finalize_with( framed: Framed, acceptor: &mut Acceptor, @@ -185,12 +188,22 @@ where S: FramedRead + FramedWrite, H: AsyncFnMut(ironrdp_pdu::rdp::multitransport::MultitransportRequestPdu, bool), { - accept_finalize_with_handlers(framed, acceptor, &mut NoopCredentialsHandler, multitransport_handler).await + accept_finalize_with_handlers( + framed, + acceptor, + &mut NoopConnectionSetupHandler, + multitransport_handler, + ) + .await } /// Finalizes the handshake with both connection-setup and multitransport handlers. /// /// The handlers follow the timing described by [`accept_finalize_with`] and [`accept_finalize_with_multitransport`]. +/// +/// # Panics +/// +/// Panics if [`Acceptor::multitransport_soft_sync_negotiated`] returns `None` for an existing multitransport request. pub async fn accept_finalize_with_handlers( mut framed: Framed, acceptor: &mut Acceptor, @@ -199,7 +212,7 @@ pub async fn accept_finalize_with_handlers( ) -> ConnectorResult<(Framed, AcceptorResult)> where S: FramedRead + FramedWrite, - H: CredentialsHandler, + H: ConnectionSetupHandler, M: AsyncFnMut(ironrdp_pdu::rdp::multitransport::MultitransportRequestPdu, bool), { let mut buf = WriteBuf::new(); @@ -229,10 +242,12 @@ where { buf.clear(); let written = acceptor.encode_access_denied(&mut buf)?; - framed - .write_all(&buf[..written]) - .await - .map_err(|e| ironrdp_connector::custom_err!("write access denied", e))?; + if written != 0 { + framed + .write_all(&buf[..written]) + .await + .map_err(|e| ironrdp_connector::custom_err!("write access denied", e))?; + } return Err(error); } acceptor.mark_credentials_handled(); @@ -246,10 +261,12 @@ where { buf.clear(); let written = acceptor.encode_access_denied(&mut buf)?; - framed - .write_all(&buf[..written]) - .await - .map_err(|e| ironrdp_connector::custom_err!("write access denied", e))?; + if written != 0 { + framed + .write_all(&buf[..written]) + .await + .map_err(|e| ironrdp_connector::custom_err!("write access denied", e))?; + } return Err(error); } if !notified && let Some(request) = acceptor.multitransport_request() { @@ -296,7 +313,7 @@ where .await; let result = match result { - Ok(()) => match credentials_handler + Ok(()) if protocol.intersects(nego::SecurityProtocol::HYBRID_EX) => match credentials_handler .handle_credentials(acceptor.received_credentials().cloned()) .await { @@ -306,7 +323,7 @@ where } Err(error) => Err(error), }, - Err(error) => Err(error), + result => result, }; if protocol.intersects(nego::SecurityProtocol::HYBRID_EX) { diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 061fb8cb51..b822eada64 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -10,7 +10,6 @@ use std::rc::Rc; use std::sync::{Arc, LazyLock, Mutex as StdMutex}; use std::time::Instant; -use anyhow::{Context as _, Result, bail}; use ironrdp_acceptor::{ Acceptor, AcceptorResult, BeginResult, ConnectionSetupHandler, ConnectorError, ConnectorErrorExt as _, CredentialOrigin, DesktopSize, ReceivedCredentials, @@ -18,7 +17,7 @@ use ironrdp_acceptor::{ use ironrdp_async::Framed; use ironrdp_cliprdr::CliprdrServer; use ironrdp_cliprdr::backend::ClipboardMessage; -use ironrdp_core::{decode, encode_vec, impl_as_any}; +use ironrdp_core::{WriteBuf, decode, encode_vec, impl_as_any}; use ironrdp_displaycontrol::pdu::{DisplayControlCapabilities, DisplayControlMonitorLayout}; use ironrdp_displaycontrol::server::{DisplayControlHandler, DisplayControlServer}; use ironrdp_dvc as dvc; @@ -342,27 +341,47 @@ pub struct BoundConnection { /// installed and the binder is not called again. #[async_trait::async_trait] pub trait ConnectionBinder: Send + Sync { - async fn bind_connection(&self, credentials: &Credentials, desktop_size: DesktopSize) -> Result; + async fn bind_connection( + &self, + credentials: &Credentials, + desktop_size: DesktopSize, + ) -> ServerResult; } struct BoundDisplaySlot { default: Box, // Async display methods lease the bound display from this slot. The lease // restores it on cancellation unless a newer connection binding replaced it. - bound: Arc>>>, + bound: Arc>, +} + +#[derive(Default)] +struct BoundDisplayState { + display: Option>, + generation: Arc<()>, +} + +impl BoundDisplayState { + fn clear(&mut self) { + self.display.take(); + self.generation = Arc::new(()); + } } struct BoundDisplayLease { - slot: Arc>>>, + slot: Arc>, display: Option>, + generation: Arc<()>, } impl BoundDisplayLease { - fn take(slot: &Arc>>>) -> Option { - let display = slot.lock().expect("bound display lock poisoned").take()?; + fn take(slot: &Arc>) -> Option { + let mut state = slot.lock().expect("bound display lock poisoned"); + let display = state.display.take()?; Some(Self { slot: Arc::clone(slot), display: Some(display), + generation: Arc::clone(&state.generation), }) } @@ -376,14 +395,14 @@ impl BoundDisplayLease { impl Drop for BoundDisplayLease { fn drop(&mut self) { let mut slot = self.slot.lock().expect("bound display lock poisoned"); - if slot.is_none() { - *slot = self.display.take(); + if Arc::ptr_eq(&slot.generation, &self.generation) && slot.display.is_none() { + slot.display = self.display.take(); } } } impl BoundDisplaySlot { - fn new(default: Box, bound: Arc>>>) -> Self { + fn new(default: Box, bound: Arc>) -> Self { Self { default, bound } } } @@ -406,7 +425,15 @@ impl RdpServerDisplay for BoundDisplaySlot { } } - async fn updates(&mut self) -> Result> { + async fn monitor_count(&mut self) -> u32 { + if let Some(mut lease) = BoundDisplayLease::take(&self.bound) { + lease.display_mut().monitor_count().await + } else { + self.default.monitor_count().await + } + } + + async fn updates(&mut self) -> ServerResult> { if let Some(mut lease) = BoundDisplayLease::take(&self.bound) { lease.display_mut().updates().await } else { @@ -416,7 +443,7 @@ impl RdpServerDisplay for BoundDisplaySlot { 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() { + if let Some(display) = bound.display.as_mut() { display.request_layout(layout); } else { drop(bound); @@ -916,7 +943,7 @@ pub struct RdpServer { // the slots are cleared at connection entry and after each connection to // avoid cross-user reuse. bound_handler: Arc>>>, - bound_display: Arc>>>, + bound_display: Arc>, sound_factory: Option>, cliprdr_factory: Option>, rdpei_factory: Option>, @@ -934,7 +961,7 @@ pub struct RdpServer { creds: Option, credential_validator: Option>, connection_binder: Option>, - pending_authenticated_credentials: Option, + pending_authenticated_credentials: Arc>>, local_addr: Option, /// The local address the current client reached. See /// [`Self::set_connection_local_addr`]. @@ -1247,6 +1274,53 @@ enum NegotiatedTransport { struct PendingConnection { security: RdpServerSecurity, acceptor: Acceptor, + credentials_handler: ServerCredentialsHandler, +} + +#[derive(Default)] +struct ServerCredentialsHandler { + validator: Option>, + require_authenticated: bool, + authenticated_credentials: Option, +} + +#[async_trait::async_trait(?Send)] +impl ConnectionSetupHandler for ServerCredentialsHandler { + async fn handle_credentials(&mut self, credentials: Option) -> Result<(), ConnectorError> { + let authenticated = resolve_authenticated_credentials(self.validator.clone(), credentials.as_ref()) + .await + .map_err(|error| ConnectorError::custom("credential validation failed", error))?; + if self.validator.is_some() && authenticated.is_none() { + return Err(ConnectorError::general("no credentials available for validation")); + } + if self.require_authenticated + && self.validator.is_none() + && 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.authenticated_credentials = authenticated.cloned(); + Ok(()) + } +} + +struct BoundConnectionGuard { + display: Arc>, + input: Arc>>>, + credentials: Arc>>, +} + +impl Drop for BoundConnectionGuard { + fn drop(&mut self) { + self.display.lock().expect("bound display lock poisoned").clear(); + self.input.lock().expect("bound input lock poisoned").take(); + self.credentials + .lock() + .expect("authenticated credentials lock poisoned") + .take(); + } } impl PendingConnection { @@ -1257,6 +1331,7 @@ impl PendingConnection { creds: Option, honor_client_desktop_size: Option, udp_bind_addr: Option, + credentials_handler: ServerCredentialsHandler, ) -> Self { let mut acceptor = Acceptor::new(security.flag(), desktop_size, capabilities, creds); acceptor.set_honor_client_desktop_size(honor_client_desktop_size); @@ -1274,7 +1349,11 @@ impl PendingConnection { | ironrdp_pdu::gcc::MultiTransportFlags::SOFT_SYNC_TCP_TO_UDP, )); } - Self { security, acceptor } + Self { + security, + acceptor, + credentials_handler, + } } /// Mutable access to the acceptor for the one thing that must happen @@ -1305,7 +1384,11 @@ impl PendingConnection { where S: AsyncRead + AsyncWrite + Send + Sync + Unpin, { - let PendingConnection { security, mut acceptor } = self; + let PendingConnection { + security, + mut acceptor, + mut credentials_handler, + } = self; let security = &security; let framed = TokioFramed::new(stream); @@ -1336,11 +1419,12 @@ impl PendingConnection { } }; let mut framed = TokioFramed::new(accept); - complete_security_upgrade(security, &mut framed, &mut acceptor).await?; + complete_security_upgrade(security, &mut framed, &mut acceptor, &mut credentials_handler).await?; Ok(Some(NegotiatedConnection { transport: NegotiatedTransport::Tls(Box::new(framed)), acceptor, local_addr: None, + authenticated_credentials: credentials_handler.authenticated_credentials, })) } // The stream is already past TLS (terminated at a lower @@ -1348,11 +1432,12 @@ impl PendingConnection { // tls_acceptor.accept on it. TransportTls::AlreadyDone => { let mut framed = TokioFramed::new(stream); - complete_security_upgrade(security, &mut framed, &mut acceptor).await?; + complete_security_upgrade(security, &mut framed, &mut acceptor, &mut credentials_handler).await?; Ok(Some(NegotiatedConnection { transport: NegotiatedTransport::Offloaded(framed), acceptor, local_addr: None, + authenticated_credentials: credentials_handler.authenticated_credentials, })) } }, @@ -1361,6 +1446,7 @@ impl PendingConnection { transport: NegotiatedTransport::Continued(framed), acceptor, local_addr: None, + authenticated_credentials: credentials_handler.authenticated_credentials, })), } } @@ -1376,6 +1462,7 @@ struct NegotiatedConnection { /// The local address the client reached, when the stream is a socket /// the server accepted itself; see [`RdpServer::set_connection_local_addr`]. local_addr: Option, + authenticated_credentials: Option, } /// State that belongs to a single client connection. @@ -1483,6 +1570,7 @@ async fn complete_security_upgrade( security: &RdpServerSecurity, framed: &mut TokioFramed, acceptor: &mut Acceptor, + credentials_handler: &mut ServerCredentialsHandler, ) -> ServerResult<()> where S: AsyncRead + AsyncWrite + Send + Sync + Unpin, @@ -1495,13 +1583,14 @@ where // uses this value in practice. let client_name = "rdp-client".to_owned(); - ironrdp_acceptor::accept_credssp( + ironrdp_acceptor::accept_credssp_with( framed, acceptor, &mut ironrdp_tokio::reqwest::ReqwestNetworkClient::new(), client_name.into(), pub_key.clone(), None, + credentials_handler, ) .await .map_err_kind("accept_credssp", ServerErrorKind::Connector)?; @@ -1647,6 +1736,8 @@ struct NegotiationContext { opts: RdpServerOptions, creds: Option, display: Arc>>, + credential_validator: Option>, + requires_authenticated_credentials: bool, } /// A candidate that has negotiated AND authenticated, and so has earned the @@ -1681,6 +1772,11 @@ async fn negotiate_candidate( ctx.creds.clone(), ctx.opts.honor_client_desktop_size, ctx.opts.udp_bind_addr, + ServerCredentialsHandler { + validator: ctx.credential_validator.clone(), + require_authenticated: ctx.requires_authenticated_credentials, + ..ServerCredentialsHandler::default() + }, ); // NOTE: deliberately NO channel attachment here. Building the cliprdr / @@ -1794,7 +1890,7 @@ impl RdpServer { gfx.set_sender(ev_sender.clone()); } let bound_handler = Arc::new(StdMutex::new(None)); - let bound_display = Arc::new(StdMutex::new(None)); + let bound_display = Arc::new(StdMutex::new(BoundDisplayState::default())); Self { opts, @@ -1827,7 +1923,7 @@ impl RdpServer { creds: None, credential_validator: None, connection_binder: None, - pending_authenticated_credentials: None, + pending_authenticated_credentials: Arc::new(StdMutex::new(None)), local_addr: None, connection_local_addr: None, autodetect_enabled: false, @@ -2110,13 +2206,19 @@ impl RdpServer { } fn install_bound_connection(&mut self, bound: BoundConnection) { - *self.bound_display.lock().expect("bound display lock poisoned") = Some(bound.display); + let mut display = self.bound_display.lock().expect("bound display lock poisoned"); + display.clear(); + display.display = Some(bound.display); *self.bound_handler.lock().expect("bound input lock poisoned") = Some(bound.input); } fn clear_bound_connection(&mut self) { - self.bound_display.lock().expect("bound display lock poisoned").take(); + self.bound_display.lock().expect("bound display lock poisoned").clear(); self.bound_handler.lock().expect("bound input lock poisoned").take(); + self.pending_authenticated_credentials + .lock() + .expect("authenticated credentials lock poisoned") + .take(); self.advanced_input_active.store(false, Ordering::Release); } @@ -2394,6 +2496,8 @@ impl RdpServer { opts: self.opts.clone(), creds: self.creds.clone(), display: Arc::clone(&self.display), + credential_validator: self.credential_validator.clone(), + requires_authenticated_credentials: self.connection_binder.is_some(), } } @@ -2543,6 +2647,7 @@ impl RdpServer { where S: AsyncRead + AsyncWrite + Send + Sync + Unpin, { + self.clear_bound_connection(); // Per-connection state must start fresh: if the previous client // disconnected while it had sent `SuppressOutput { None }` (e.g., // closed the mstsc window while minimized so the matching resume @@ -2569,6 +2674,11 @@ impl RdpServer { self.creds.clone(), self.opts.honor_client_desktop_size, self.opts.udp_bind_addr, + ServerCredentialsHandler { + validator: self.credential_validator.clone(), + require_authenticated: self.connection_binder.is_some(), + ..ServerCredentialsHandler::default() + }, ); self.attach_channels(pending.acceptor_mut(), monitor_count); @@ -2597,7 +2707,18 @@ impl RdpServer { transport, acceptor, local_addr, + authenticated_credentials, } = negotiated; + self.clear_bound_connection(); + let _binding = BoundConnectionGuard { + display: Arc::clone(&self.bound_display), + input: Arc::clone(&self.bound_handler), + credentials: Arc::clone(&self.pending_authenticated_credentials), + }; + *self + .pending_authenticated_credentials + .lock() + .expect("authenticated credentials lock poisoned") = authenticated_credentials; if local_addr.is_some() { self.connection_local_addr = local_addr; } @@ -4394,26 +4515,17 @@ impl RdpServer { 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()) - .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 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(); + ) -> Result<(), ConnectorError> { + let mut handler = ServerCredentialsHandler { + validator: self.credential_validator.clone(), + require_authenticated: self.connection_binder.is_some(), + ..ServerCredentialsHandler::default() + }; + handler.handle_credentials(received_credentials).await?; + *self + .pending_authenticated_credentials + .lock() + .expect("authenticated credentials lock poisoned") = handler.authenticated_credentials; Ok(()) } @@ -4422,7 +4534,7 @@ impl RdpServer { conn: &mut ConnectionState, reader: &mut Framed, writer: &mut Framed, - result: AcceptorResult, + (result, acceptor): (AcceptorResult, &Acceptor), udp_transport: Rc>>, pending_udp_accept: Option>>, ) -> ServerResult @@ -4432,23 +4544,20 @@ impl RdpServer { { debug!("Client accepted"); - // MS-RDPBCGR 3.3.5.7.1: a Set Error Info PDU MUST NOT be sent to a client that did not - // set `SUPPORT_ERR_INFO_PDU`; such a client is just disconnected. - let supports_err_info = result - .client_early_capability_flags - .contains(ironrdp_pdu::gcc::ClientEarlyCapabilityFlags::SUPPORT_ERR_INFO_PDU); - let is_auto_reconnect = if let Some(reconnect) = result.auto_reconnect.as_ref() { if !self.verify_auto_reconnect_cookie(reconnect) { warn!("Auto-reconnect cookie validation rejected"); - send_access_denied(result.io_channel_id, result.user_channel_id, supports_err_info, writer).await?; + send_access_denied(acceptor, writer).await?; return Err(ServerError::reason("auto-reconnect validation", "cookie rejected")); } if self.connection_binder.is_some() { warn!("Auto-reconnect cannot restore a credential-bound connection safely"); - send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; - bail!("auto-reconnect is unsupported with a connection binder"); + send_access_denied(acceptor, writer).await?; + return Err(ServerError::reason( + "auto-reconnect validation", + "unsupported with a connection binder", + )); } debug!("Auto-reconnect cookie validation accepted"); @@ -4461,11 +4570,18 @@ impl RdpServer { 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() + && self + .pending_authenticated_credentials + .lock() + .expect("authenticated credentials lock poisoned") + .is_none() { 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"); + send_access_denied(acceptor, writer).await?; + return Err(ServerError::reason( + "credential validation", + "credentials were not prepared before activation", + )); } if !result.reactivation { @@ -5061,11 +5177,6 @@ impl RdpServer { // synchronously as part of finalize, so establishing it must never // block the RDP handshake finalize itself is driving. let mut pending_udp_accept: Option>> = None; - // 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 { // Bounded: see `FINALIZE_TIMEOUT`. The bound belongs on THIS call // and not on `accept_finalize` itself or its callers — the loop @@ -5120,7 +5231,7 @@ impl RdpServer { conn, &mut reader, &mut writer, - result, + (result, &acceptor), Rc::clone(&udp_transport), pending_udp_accept.take(), ) @@ -5175,10 +5286,7 @@ impl RdpServer { } } -fn validate_bound_display_size( - negotiated_size: DesktopSize, - bound_size: DesktopSize, -) -> core::result::Result<(), ConnectorError> { +fn validate_bound_display_size(negotiated_size: DesktopSize, bound_size: DesktopSize) -> Result<(), ConnectorError> { if bound_size == negotiated_size { Ok(()) } else { @@ -5192,7 +5300,7 @@ fn validate_bound_display_size( async fn resolve_authenticated_credentials( credential_validator: Option>, received_credentials: Option<&ReceivedCredentials>, -) -> Result> { +) -> ServerResult> { if let Some(received) = received_credentials { let creds = &received.credentials; if let Some(validator) = credential_validator { @@ -5203,11 +5311,11 @@ async fn resolve_authenticated_credentials( } Ok(CredentialDecision::Reject) => { warn!("Credential validation rejected"); - bail!("credential validation rejected"); + Err(ServerError::reason("credential validation", "rejected by validator")) } Err(e) => { error!(error = %e, "Credential validator backend error"); - Err(e.into()) + Err(ServerError::custom("credential validation", e)) } } } else { @@ -5344,36 +5452,17 @@ fn with_connection_handler( handler.map(|h| f(h.borrow_mut().as_mut())) } -/// Send a `ServerSetErrorInfoPdu(ServerDeniedConnection)` to the client, then return. -/// -/// Used to deny a connection after credential validation rejects it, mirroring the -/// acceptor's exact-match denial so both paths refuse the same spec-defined way. -/// -/// Sends nothing when the client did not opt in via `SUPPORT_ERR_INFO_PDU` -/// (MS-RDPBCGR 3.3.5.7.1); the caller still closes the connection. -async fn send_access_denied( - io_channel_id: u16, - user_channel_id: u16, - supports_err_info: bool, - writer: &mut impl FramedWrite, -) -> ServerResult<()> { - if !supports_err_info { - return Ok(()); - } - let info = ServerSetErrorInfoPdu(ErrorInfo::ProtocolIndependentCode( - ProtocolIndependentCode::ServerDeniedConnection, - )); - let user_data = encode_vec(&info).map_err(ServerError::encode)?.into(); - let pdu = SendDataIndication { - initiator_id: user_channel_id, - channel_id: io_channel_id, - user_data, - }; - let msg = encode_vec(&X224(pdu)).map_err(ServerError::encode)?; - writer - .write_all(&msg) - .await - .map_err(|e| ServerError::io("write access_denied", e))?; +async fn send_access_denied(acceptor: &Acceptor, writer: &mut impl FramedWrite) -> ServerResult<()> { + let mut buf = WriteBuf::new(); + let written = acceptor + .encode_access_denied(&mut buf) + .map_err_kind("encode access denied", ServerErrorKind::Connector)?; + if written != 0 { + writer + .write_all(&buf[..written]) + .await + .map_err(|e| ServerError::io("write access denied", e))?; + } Ok(()) } @@ -5458,6 +5547,52 @@ impl<'a, W: FramedWrite> SharedWriter<'a, W> { } } +#[async_trait::async_trait(?Send)] +impl ConnectionSetupHandler for RdpServer { + async fn handle_credentials(&mut self, credentials: Option) -> Result<(), ConnectorError> { + self.prepare_authenticated_connection(credentials).await + } + + async fn prepare_capability_exchange(&mut self, desktop_size: DesktopSize) -> Result<(), ConnectorError> { + if (self.credential_validator.is_some() || self.connection_binder.is_some()) + && self + .pending_authenticated_credentials + .lock() + .expect("authenticated credentials lock poisoned") + .is_none() + { + return Err(ConnectorError::general( + "credentials were not prepared before capability exchange", + )); + } + + let Some(binder) = self.connection_binder.clone() else { + return Ok(()); + }; + let credentials = self + .pending_authenticated_credentials + .lock() + .expect("authenticated credentials lock poisoned") + .clone() + .ok_or_else(|| ConnectorError::general("no authenticated credentials for connection binding"))?; + let bound = binder + .bind_connection(&credentials, desktop_size) + .await + .map_err(|error| ConnectorError::custom("connection binder failed", error))?; + + 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, + }); + debug!(?bound_size, "Connection binder installed display/input handlers"); + Ok(()) + } +} + #[cfg(test)] mod preempt_tests { use core::net::Ipv4Addr; @@ -5496,6 +5631,8 @@ mod preempt_tests { }, creds: None, display: Arc::new(Mutex::new(Box::new(NoDisplay))), + credential_validator: None, + requires_authenticated_credentials: false, } } @@ -6263,52 +6400,6 @@ mod cliprdr_error_tests { } } -#[async_trait::async_trait(?Send)] -impl ConnectionSetupHandler 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(binder) = self.connection_binder.clone() else { - return Ok(()); - }; - 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:#}")))?; - - 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, - }); - debug!(?bound_size, "Connection binder installed display/input handlers"); - Ok(()) - } -} - #[cfg(test)] mod credential_binding_tests { use super::*; @@ -6358,12 +6449,57 @@ mod credential_binding_tests { let error = resolve_authenticated_credentials(Some(validator), Some(&received_credentials)) .await .expect_err("backend failure should be returned"); - let validation_error = error + let validation_error = core::error::Error::source(&error) + .expect("validator source should be preserved") .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"); } + + #[tokio::test] + async fn candidate_credential_validation_does_not_change_the_active_identity() { + let mut server = RdpServer::builder() + .with_addr(([127, 0, 0, 1], 0)) + .with_no_security() + .with_no_input() + .with_no_display() + .build(); + server.set_credential_validator(Some(Arc::new(ExactMatchCredentialValidator::new(creds("alice"))))); + *server.pending_authenticated_credentials.lock().unwrap() = Some(creds("bob")); + let context = server.negotiation_context(); + let mut accepted = ServerCredentialsHandler { + validator: context.credential_validator.clone(), + ..ServerCredentialsHandler::default() + }; + accepted + .handle_credentials(Some(ReceivedCredentials { + credentials: creds("alice"), + origin: CredentialOrigin::CredSspDelegated, + })) + .await + .unwrap(); + assert_eq!(accepted.authenticated_credentials, Some(creds("alice"))); + + let mut rejected = ServerCredentialsHandler { + validator: context.credential_validator, + ..ServerCredentialsHandler::default() + }; + assert!( + rejected + .handle_credentials(Some(ReceivedCredentials { + credentials: creds("mallory"), + origin: CredentialOrigin::CredSspDelegated, + })) + .await + .is_err() + ); + assert!(rejected.authenticated_credentials.is_none()); + assert_eq!( + *server.pending_authenticated_credentials.lock().unwrap(), + Some(creds("bob")) + ); + } } #[cfg(test)] @@ -6392,7 +6528,7 @@ mod tests { impl SvcProcessor for ResourceChannel { fn channel_name(&self) -> ChannelName { - ChannelName::from_static(b"testchan") + ChannelName::from_static(b"testchn\0") } fn process(&mut self, _payload: &[u8]) -> PduResult> { @@ -6446,7 +6582,7 @@ mod tests { pending().await } - async fn updates(&mut self) -> Result> { + async fn updates(&mut self) -> ServerResult> { pending().await } } @@ -6459,9 +6595,13 @@ mod tests { self.0 } - async fn updates(&mut self) -> Result> { + async fn updates(&mut self) -> ServerResult> { pending().await } + + async fn monitor_count(&mut self) -> u32 { + 2 + } } #[tokio::test] @@ -6470,31 +6610,88 @@ mod tests { width: 1280, height: 720, }; - let bound = Arc::new(StdMutex::new(Some( - Box::new(PendingDisplay) as Box - ))); + let bound = Arc::new(StdMutex::new(BoundDisplayState { + display: Some(Box::new(PendingDisplay)), + ..BoundDisplayState::default() + })); let mut slot = BoundDisplaySlot::new(Box::new(FixedDisplay(expected)), Arc::clone(&bound)); - assert!( - tokio::time::timeout(core::time::Duration::ZERO, slot.size()) - .await - .is_err() - ); - assert!(bound.lock().expect("bound display lock poisoned").is_some()); + assert!(tokio::time::timeout(Duration::ZERO, slot.size()).await.is_err()); + assert!(bound.lock().expect("bound display lock poisoned").display.is_some()); + } + + #[tokio::test] + async fn bound_display_forwards_monitor_count() { + let bound = Arc::new(StdMutex::new(BoundDisplayState { + display: Some(Box::new(FixedDisplay(DesktopSize { + width: 1280, + height: 720, + }))), + ..BoundDisplayState::default() + })); + let mut slot = BoundDisplaySlot::new(Box::new(PendingDisplay), Arc::clone(&bound)); + assert_eq!(slot.monitor_count().await, 2); + bound.lock().unwrap().clear(); + assert_eq!(slot.monitor_count().await, 1); + } + + #[tokio::test] + async fn cancelling_connection_drops_bound_resources_and_credentials() { + let display = Arc::new(StdMutex::new(BoundDisplayState { + display: Some(Box::new(PendingDisplay)), + ..BoundDisplayState::default() + })); + let input: Arc>>> = + Arc::new(StdMutex::new(Some(Box::new(NoopInput)))); + let credentials = Arc::new(StdMutex::new(Some(Credentials { + username: "alice".to_owned(), + password: "secret".to_owned(), + domain: None, + }))); + let connection = async { + let _binding = BoundConnectionGuard { + display: Arc::clone(&display), + input: Arc::clone(&input), + credentials: Arc::clone(&credentials), + }; + pending::<()>().await; + }; + assert!(tokio::time::timeout(Duration::ZERO, connection).await.is_err()); + assert!(display.lock().unwrap().display.is_none()); + assert!(input.lock().unwrap().is_none()); + assert!(credentials.lock().unwrap().is_none()); + } + + #[test] + fn a_cleared_connection_cannot_restore_an_old_display_lease() { + let display = Arc::new(StdMutex::new(BoundDisplayState { + display: Some(Box::new(PendingDisplay)), + ..BoundDisplayState::default() + })); + let lease = BoundDisplayLease::take(&display).unwrap(); + display.lock().unwrap().clear(); + drop(lease); + assert!(display.lock().unwrap().display.is_none()); } #[tokio::test] async fn run_connection_releases_the_static_channels() { + struct ResourceFactory(Arc); + impl StaticChannelFactory for ResourceFactory { + fn attach(&self, acceptor: &mut Acceptor) { + acceptor.attach_static_channel(ResourceChannel(Arc::clone(&self.0))); + } + } + + let released = Arc::new(AtomicBool::new(false)); let mut server = RdpServer::builder() .with_addr(([127, 0, 0, 1], 0)) .with_no_security() .with_no_input() .with_no_display() + .with_static_channel_factory(Box::new(ResourceFactory(Arc::clone(&released)))) .build(); - let released = Arc::new(AtomicBool::new(false)); - server.static_channels.insert(ResourceChannel(Arc::clone(&released))); - // A stream that is already at EOF: the connection ends early, which // is the path an embedder's accept loop sees when a client vanishes. let (client, server_side) = tokio::io::duplex(64); diff --git a/crates/ironrdp-testsuite-core/tests/server/acceptor.rs b/crates/ironrdp-testsuite-core/tests/server/acceptor.rs index e41c342fe8..800b1e6a8e 100644 --- a/crates/ironrdp-testsuite-core/tests/server/acceptor.rs +++ b/crates/ironrdp-testsuite-core/tests/server/acceptor.rs @@ -1,19 +1,39 @@ +use core::cell::RefCell; +use core::future::{Ready, ready}; use std::borrow::Cow; +use std::collections::VecDeque; +use std::io; +use std::rc::Rc; -use ironrdp_acceptor::{Acceptor, MultitransportSecurityRng}; +use ironrdp_acceptor::{ + Acceptor, ConnectionSetupHandler, MultitransportSecurityRng, ReceivedCredentials, accept_credssp_with, + accept_finalize_with, +}; +use ironrdp_async::bytes::BytesMut; +use ironrdp_async::{Framed, FramedRead, FramedWrite, NetworkClient, StreamWrapper}; +use ironrdp_connector::credssp::CredsspSequence; +use ironrdp_connector::sspi::credssp::TsRequest; +use ironrdp_connector::sspi::generator::{GeneratorState, NetworkRequest}; +use ironrdp_connector::{ConnectorError, ConnectorErrorExt as _, ConnectorResult}; use ironrdp_connector::{DesktopSize, Sequence as _, Written, encode_x224_packet}; use ironrdp_core::{WriteBuf, decode, encode_vec}; -use ironrdp_pdu::gcc::{ClientMessageChannelData, MultiTransportChannelData, MultiTransportFlags}; +use ironrdp_pdu::gcc::{ + ClientCoreOptionalData, ClientEarlyCapabilityFlags, ClientMessageChannelData, MultiTransportChannelData, + MultiTransportFlags, +}; use ironrdp_pdu::mcs::{self, ConnectInitial}; use ironrdp_pdu::nego::{self, SecurityProtocol}; -use ironrdp_pdu::rdp::client_info::CompressionType; +use ironrdp_pdu::rdp::ClientInfoPdu; +use ironrdp_pdu::rdp::client_info::{CompressionType, Credentials}; use ironrdp_pdu::rdp::finalization_messages::{ControlAction, ControlPdu, SynchronizePdu}; use ironrdp_pdu::rdp::headers::{ - BasicSecurityHeaderFlags, CompressionFlags, ShareControlHeader, ShareControlPdu, ShareDataHeader, ShareDataPdu, - StreamPriority, + BasicSecurityHeader, BasicSecurityHeaderFlags, CompressionFlags, ShareControlHeader, ShareControlPdu, + ShareDataHeader, ShareDataPdu, StreamPriority, }; use ironrdp_pdu::rdp::multitransport::{MultitransportRequestPdu, MultitransportResponsePdu, RequestedProtocol}; +use ironrdp_pdu::rdp::server_error_info::{ErrorInfo, ProtocolIndependentCode, ServerSetErrorInfoPdu}; use ironrdp_pdu::x224::{X224, X224Data}; +use ironrdp_testsuite_core::client_info::CLIENT_INFO_UNICODE; use ironrdp_testsuite_core::gcc::CLIENT_GCC_WITHOUT_OPTIONAL_FIELDS; use ironrdp_testsuite_core::rdp::{CLIENT_DEMAND_ACTIVE_PDU_BUFFER, CLIENT_INFO_PDU_BUFFER}; @@ -720,3 +740,365 @@ fn multitransport_not_offered_when_client_does_not_reciprocate() { assert!(matches!(written, Written::Nothing)); assert!(acceptor.multitransport_request().is_none()); } + +#[derive(Default)] +struct HandshakeStream { + incoming: VecDeque>, + outgoing: Rc>>>, + credssp_client: Option, +} + +impl StreamWrapper for HandshakeStream { + type InnerStream = Self; + + fn from_inner(stream: Self) -> Self { + stream + } + + fn into_inner(self) -> Self { + self + } + + fn get_inner(&self) -> &Self { + self + } + + fn get_inner_mut(&mut self) -> &mut Self { + self + } +} + +impl FramedRead for HandshakeStream { + type ReadFut<'read> = Ready>; + + fn read<'a>(&'a mut self, buf: &'a mut BytesMut) -> Self::ReadFut<'a> { + let chunk = self.incoming.pop_front().unwrap_or_default(); + buf.extend_from_slice(&chunk); + ready(Ok(chunk.len())) + } +} + +impl FramedWrite for HandshakeStream { + type WriteAllFut<'write> = Ready>; + + fn write_all<'a>(&'a mut self, buf: &'a [u8]) -> Self::WriteAllFut<'a> { + self.outgoing.borrow_mut().push(buf.to_vec()); + if let Some(client) = self.credssp_client.as_mut() + && buf.len() != 4 + { + let request = client.decode_server_message(buf).unwrap().unwrap(); + self.incoming.push_back(client_credssp_message(client, request)); + } + ready(Ok(())) + } +} + +fn client_credssp_message(client: &mut CredsspSequence, request: TsRequest) -> Vec { + let result = { + let mut generator = client.process_ts_request(request); + let GeneratorState::Completed(result) = generator.start() else { + panic!("NTLM should not make network requests"); + }; + result.unwrap() + }; + let mut buf = WriteBuf::new(); + client.handle_process_result(result, &mut buf).unwrap(); + buf.filled().to_vec() +} + +struct NoNetwork; + +impl NetworkClient for NoNetwork { + async fn send(&mut self, _: &NetworkRequest) -> ConnectorResult> { + Err(ConnectorError::general("unexpected network request during NTLM")) + } +} + +#[derive(Default)] +struct SetupHandler { + received: Vec, + reject_credentials: bool, + reject_capabilities: bool, + capability_calls: usize, +} + +#[async_trait::async_trait(?Send)] +impl ConnectionSetupHandler for SetupHandler { + async fn handle_credentials(&mut self, credentials: Option) -> ConnectorResult<()> { + self.received + .push(credentials.expect("handshake should supply credentials")); + if self.reject_credentials { + Err(ConnectorError::general("test credential rejection")) + } else { + Ok(()) + } + } + + async fn prepare_capability_exchange(&mut self, _: DesktopSize) -> ConnectorResult<()> { + self.capability_calls += 1; + if self.reject_capabilities { + Err(ConnectorError::general("test capability rejection")) + } else { + Ok(()) + } + } +} + +fn negotiated_acceptor(protocol: SecurityProtocol) -> Acceptor { + let mut acceptor = Acceptor::new( + protocol, + DesktopSize { + width: 1024, + height: 768, + }, + Vec::new(), + protocol + .intersects(SecurityProtocol::HYBRID | SecurityProtocol::HYBRID_EX) + .then(|| Credentials { + username: "alice".to_owned(), + password: "secret".to_owned(), + domain: None, + }), + ); + acceptor + .step(&encode_connection_request(protocol), None, &mut WriteBuf::new()) + .unwrap(); + acceptor.step(&[], None, &mut WriteBuf::new()).unwrap(); + acceptor.mark_security_upgrade_as_done(); + acceptor +} + +fn post_credssp_messages(error_info: Option) -> VecDeque> { + let mut gcc = CLIENT_GCC_WITHOUT_OPTIONAL_FIELDS.clone(); + gcc.network = None; + gcc.core.optional_data = ClientCoreOptionalData { + post_beta2_color_depth: gcc.core.optional_data.post_beta2_color_depth, + client_product_id: gcc.core.optional_data.client_product_id, + serial_number: gcc.core.optional_data.serial_number, + high_color_depth: gcc.core.optional_data.high_color_depth, + supported_color_depths: gcc.core.optional_data.supported_color_depths, + early_capability_flags: error_info, + ..ClientCoreOptionalData::default() + }; + let mut initial = WriteBuf::new(); + encode_x224_packet(&ConnectInitial::with_gcc_blocks(gcc).unwrap(), &mut initial).unwrap(); + + let info = ClientInfoPdu { + security_header: BasicSecurityHeader { + flags: BasicSecurityHeaderFlags::INFO_PKT, + }, + client_info: CLIENT_INFO_UNICODE.clone(), + }; + [ + initial.filled().to_vec(), + encode_vec(&X224(mcs::ErectDomainPdu { + sub_height: 0, + sub_interval: 0, + })) + .unwrap(), + encode_vec(&X224(mcs::AttachUserRequest)).unwrap(), + encode_vec(&X224(mcs::ChannelJoinRequest { + initiator_id: 1002, + channel_id: 1002, + })) + .unwrap(), + encode_vec(&X224(mcs::ChannelJoinRequest { + initiator_id: 1002, + channel_id: 1003, + })) + .unwrap(), + encode_vec(&X224(mcs::SendDataRequest { + initiator_id: 1002, + channel_id: 1003, + user_data: encode_vec(&info).unwrap().into(), + })) + .unwrap(), + ] + .into() +} + +fn assert_access_denied(bytes: &[u8]) { + let data = decode::>>(bytes).unwrap().0; + assert_eq!(data.initiator_id, 1002); + assert_eq!(data.channel_id, 1003); + assert_eq!(data.user_data.len(), 22); + let header = decode::(data.user_data.as_ref()).unwrap(); + assert_eq!(header.pdu_source, 0); + let ShareControlPdu::Data(data) = header.share_control_pdu else { + panic!("expected a Share Data PDU"); + }; + assert!(data.compression_flags.is_empty()); + assert_eq!( + data.share_data_pdu, + ShareDataPdu::ServerSetErrorInfo(ServerSetErrorInfoPdu(ErrorInfo::ProtocolIndependentCode( + ProtocolIndependentCode::ServerInsufficientPrivileges, + ))) + ); +} + +#[rstest::rstest] +#[case(None)] +#[case(Some(ClientEarlyCapabilityFlags::empty()))] +#[case(Some(ClientEarlyCapabilityFlags::SUPPORT_ERR_INFO_PDU))] +#[tokio::test] +async fn tls_hook_rejection_respects_error_info_support( + #[case] error_info: Option, + #[values(false, true)] reject_credentials: bool, +) { + let mut acceptor = negotiated_acceptor(SecurityProtocol::SSL); + let mut handler = SetupHandler { + reject_credentials, + reject_capabilities: !reject_credentials, + ..SetupHandler::default() + }; + let framed = Framed::::new(HandshakeStream { + incoming: post_credssp_messages(error_info), + ..HandshakeStream::default() + }); + let outgoing = Rc::clone(&framed.get_inner().0.outgoing); + let result = accept_finalize_with(framed, &mut acceptor, &mut handler).await; + let error = result.err().expect("hook rejection should fail the handshake"); + assert!(error.to_string().contains(if reject_credentials { + "test credential rejection" + } else { + "test capability rejection" + })); + assert_eq!(handler.received.len(), 1); + assert_eq!( + handler.received[0].origin, + ironrdp_acceptor::CredentialOrigin::ClientInfo + ); + assert_eq!(handler.capability_calls, usize::from(!reject_credentials)); + + let outgoing = outgoing.borrow(); + let supports_error_info = + error_info.is_some_and(|flags| flags.contains(ClientEarlyCapabilityFlags::SUPPORT_ERR_INFO_PDU)); + assert_eq!( + outgoing.len(), + 4 + usize::from(!reject_credentials) + usize::from(supports_error_info) + ); + assert_eq!( + decode::>(&outgoing[3]) + .unwrap() + .0 + .channel_id, + 1003 + ); + let mut denied = WriteBuf::new(); + let written = acceptor.encode_access_denied(&mut denied).unwrap(); + if supports_error_info { + assert_eq!(written, denied.filled_len()); + assert_access_denied(denied.filled()); + assert_eq!(outgoing.last().unwrap(), denied.filled()); + } else { + assert_eq!(written, 0); + assert!(denied.filled().is_empty()); + } +} + +#[rstest::rstest] +#[case(SecurityProtocol::HYBRID, None)] +#[case(SecurityProtocol::HYBRID, Some(ClientEarlyCapabilityFlags::SUPPORT_ERR_INFO_PDU))] +#[case(SecurityProtocol::HYBRID_EX, None)] +#[tokio::test] +async fn credssp_authorization_uses_selected_protocol( + #[case] protocol: SecurityProtocol, + #[case] error_info: Option, + #[values("alice@corp.example.com", "CORP\\alice", "alice")] username: &str, + #[values(false, true)] reject_credentials: bool, +) { + let mut acceptor = negotiated_acceptor(protocol); + let public_key = vec![0x42; 32]; + let (mut client, request) = CredsspSequence::init( + ironrdp_connector::Credentials::UsernamePassword { + username: username.to_owned(), + password: "secret".to_owned(), + }, + None, + protocol, + "server.example.com".to_owned().into(), + public_key.clone(), + None, + ) + .unwrap(); + let first_message = client_credssp_message(&mut client, request); + let mut framed = Framed::::new(HandshakeStream { + incoming: [first_message].into(), + credssp_client: Some(client), + ..HandshakeStream::default() + }); + let outgoing = Rc::clone(&framed.get_inner().0.outgoing); + let mut handler = SetupHandler { + reject_credentials, + reject_capabilities: !reject_credentials, + ..SetupHandler::default() + }; + let result = accept_credssp_with( + &mut framed, + &mut acceptor, + &mut NoNetwork, + "client.example.com".to_owned().into(), + public_key, + None, + &mut handler, + ) + .await; + + if protocol == SecurityProtocol::HYBRID_EX && reject_credentials { + assert!(result.unwrap_err().to_string().contains("test credential rejection")); + assert_eq!(outgoing.borrow().last().unwrap(), &5u32.to_le_bytes()); + } else { + result.unwrap(); + if protocol == SecurityProtocol::HYBRID { + assert!(handler.received.is_empty(), "plain HYBRID must defer authorization"); + } else { + assert_eq!(handler.received.len(), 1); + assert_eq!(outgoing.borrow().last().unwrap(), &0u32.to_le_bytes()); + } + let credssp_write_count = outgoing.borrow().len(); + let (stream, _) = framed.get_inner_mut(); + stream.credssp_client = None; + stream.incoming = post_credssp_messages(error_info); + let error = accept_finalize_with(framed, &mut acceptor, &mut handler) + .await + .err() + .expect("hook rejection should fail the handshake"); + assert!(error.to_string().contains(if reject_credentials { + "test credential rejection" + } else { + "test capability rejection" + })); + let outgoing = outgoing.borrow(); + let handshake_writes = &outgoing[credssp_write_count..]; + let supports_error_info = + error_info.is_some_and(|flags| flags.contains(ClientEarlyCapabilityFlags::SUPPORT_ERR_INFO_PDU)); + assert_eq!( + handshake_writes.len(), + 4 + usize::from(!reject_credentials) + usize::from(supports_error_info) + ); + assert_eq!( + decode::>(&handshake_writes[3]) + .unwrap() + .0 + .channel_id, + 1003 + ); + if supports_error_info { + assert_access_denied(handshake_writes.last().unwrap()); + } + } + + assert_eq!(handler.received.len(), 1); + assert_eq!( + handler.received[0].origin, + ironrdp_acceptor::CredentialOrigin::CredSspDelegated + ); + let (expected_username, expected_domain) = match username { + "CORP\\alice" => ("alice", Some("CORP")), + _ => (username, None), + }; + assert_eq!(handler.received[0].credentials.username, expected_username); + assert_eq!(handler.received[0].credentials.domain.as_deref(), expected_domain); + assert_eq!(handler.received[0].credentials.password, "secret"); + assert_eq!(handler.capability_calls, usize::from(!reject_credentials)); +} From 36b30add4195a02653691844b39f35d2153e6751 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Thu, 8 Oct 2026 16:29:43 -0400 Subject: [PATCH 33/33] fix(server): keep the accept-loop future compact Box connection futures under Queue and Reject, matching Preempt, so the accept loop stays below the 16 KiB large-future lint threshold when workspace features are unified. This adds one allocation per served connection without changing connection handling or cancellation. Add a future-size regression and correct test-module placement and qualification errors exposed by the full workspace lint command. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- crates/ironrdp-acceptor/src/connection.rs | 72 +++++++++---------- crates/ironrdp-server/src/server.rs | 12 ++-- .../tests/server/connection_policy.rs | 8 +++ 3 files changed, 51 insertions(+), 41 deletions(-) diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index e584758a7e..bcac64496d 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -1518,6 +1518,42 @@ impl Sequence for Acceptor { } } +fn create_gcc_blocks( + io_channel: u16, + channel_ids: Vec, + requested: SecurityProtocol, + skip_channel_join: bool, + message_channel_id: Option, + offer_multitransport: Option, +) -> gcc::ServerGccBlocks { + gcc::ServerGccBlocks { + core: gcc::ServerCoreData { + version: gcc::RdpVersion::V5_PLUS, + optional_data: gcc::ServerCoreOptionalData { + client_requested_protocols: Some(requested), + early_capability_flags: skip_channel_join + .then_some(gcc::ServerEarlyCapabilityFlags::SKIP_CHANNELJOIN_SUPPORTED), + }, + }, + security: gcc::ServerSecurityData::no_security(), + network: gcc::ServerNetworkData { + channel_ids, + io_channel, + }, + message_channel: message_channel_id.map(|id| gcc::ServerMessageChannelData { + mcs_message_channel_id: id, + }), + // Only meaningful alongside a message channel: the request and any + // response it draws both travel there (MS-RDPBCGR 2.2.15.1, 2.2.15.2). + // The caller has already filtered the offer to None when + // the client did not populate its own MultiTransportChannelData + // block, per 2.2.1.4's requirement that this block be omitted then. + multi_transport_channel: message_channel_id + .and(offer_multitransport) + .map(|flags| gcc::MultiTransportChannelData { flags }), + } +} + #[cfg(test)] mod tests { use super::*; @@ -1585,39 +1621,3 @@ mod tests { assert!(!acceptor.credentials_need_handling()); } } - -fn create_gcc_blocks( - io_channel: u16, - channel_ids: Vec, - requested: SecurityProtocol, - skip_channel_join: bool, - message_channel_id: Option, - offer_multitransport: Option, -) -> gcc::ServerGccBlocks { - gcc::ServerGccBlocks { - core: gcc::ServerCoreData { - version: gcc::RdpVersion::V5_PLUS, - optional_data: gcc::ServerCoreOptionalData { - client_requested_protocols: Some(requested), - early_capability_flags: skip_channel_join - .then_some(gcc::ServerEarlyCapabilityFlags::SKIP_CHANNELJOIN_SUPPORTED), - }, - }, - security: gcc::ServerSecurityData::no_security(), - network: gcc::ServerNetworkData { - channel_ids, - io_channel, - }, - message_channel: message_channel_id.map(|id| gcc::ServerMessageChannelData { - mcs_message_channel_id: id, - }), - // Only meaningful alongside a message channel: the request and any - // response it draws both travel there (MS-RDPBCGR 2.2.15.1, 2.2.15.2). - // The caller has already filtered the offer to None when - // the client did not populate its own MultiTransportChannelData - // block, per 2.2.1.4's requirement that this block be omitted then. - multi_transport_channel: message_channel_id - .and(offer_multitransport) - .map(|flags| gcc::MultiTransportChannelData { flags }), - } -} diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index b822eada64..c1dadc23ed 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -3135,7 +3135,7 @@ impl RdpServer { // taken by the outer loop, not rejected here. let result = match entry { Entry::Fresh(stream, _) => { - let mut conn = core::pin::pin!(self.run_connection_inner(stream, TransportTls::Managed)); + let mut conn = Box::pin(self.run_connection_inner(stream, TransportTls::Managed)); loop { tokio::select! { biased; @@ -3151,7 +3151,7 @@ impl RdpServer { } } // Unreachable: Reject never produces a Negotiated candidate. - Entry::Negotiated(candidate, _) => self.serve_negotiated(candidate).await, + Entry::Negotiated(candidate, _) => Box::pin(self.serve_negotiated(candidate)).await, }; (result, None) } @@ -3159,10 +3159,12 @@ impl RdpServer { let result = match entry { // Same anyhow-vs-ServerResult reasoning as the preemption // branch above. - Entry::Fresh(stream, _) => self.run_connection_inner(stream, TransportTls::Managed).await, + Entry::Fresh(stream, _) => { + Box::pin(self.run_connection_inner(stream, TransportTls::Managed)).await + } // Unreachable in practice: `pending` is only ever populated // by the preemption branch above. - Entry::Negotiated(candidate, _) => self.serve_negotiated(candidate).await, + Entry::Negotiated(candidate, _) => Box::pin(self.serve_negotiated(candidate)).await, }; (result, None) } @@ -5612,7 +5614,7 @@ mod preempt_tests { height: 768, } } - async fn updates(&mut self) -> ServerResult> { + async fn updates(&mut self) -> ServerResult> { unreachable!("negotiation never asks for updates") } } diff --git a/crates/ironrdp-testsuite-core/tests/server/connection_policy.rs b/crates/ironrdp-testsuite-core/tests/server/connection_policy.rs index 32c1ebbbc6..673e268c33 100644 --- a/crates/ironrdp-testsuite-core/tests/server/connection_policy.rs +++ b/crates/ironrdp-testsuite-core/tests/server/connection_policy.rs @@ -45,6 +45,14 @@ fn build(policy: ConnectionPolicy) -> RdpServer { .build() } +#[test] +fn accept_loop_future_stays_below_clippy_large_future_limit() { + let mut server = build(ConnectionPolicy::Queue); + let future = server.run(); + let size = size_of_val(&future); + assert!(size < 16 * 1024, "accept-loop future is {size} bytes"); +} + /// With `Reject`, a second connection arriving while a session runs is closed /// immediately: the read returns EOF rather than hanging. #[tokio::test]