From adb79db8ace082beb2b86424b6171f1fa8bd9c85 Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Fri, 6 Mar 2026 23:23:15 -0600 Subject: [PATCH 1/5] feat(server): add CredentialValidator trait for server-side auth Add a CredentialValidator trait that allows servers to validate client credentials during connection setup. Called after the acceptor phase extracts credentials from ClientInfoPdu. This enables PAM, LDAP, and other validate-on-receipt auth flows for TLS-mode connections. Not used for CredSSP/Hybrid connections which handle authentication through NTLM challenge-response. - Add CredentialValidator trait with validate() method - Add set_credential_validator() to RdpServer - Validate credentials in client_accepted() before session start --- crates/ironrdp-server/src/server.rs | 49 +++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 23149c8b61..85a87c3a5f 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -89,6 +89,20 @@ pub trait ConnectionHandler: Send { } } +/// Server-side credential validator for TLS-mode connections. +/// +/// 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). +/// +/// Implement this trait to validate credentials against external systems +/// (PAM, LDAP, database, etc.). +pub trait CredentialValidator: Send + Sync { + /// Validate credentials received from the client. + /// Return `Ok(true)` to accept, `Ok(false)` to reject. + fn validate(&self, credentials: &Credentials) -> Result; +} + #[derive(Clone)] pub struct RdpServerOptions { pub addr: SocketAddr, @@ -290,6 +304,7 @@ pub struct RdpServer { ev_sender: mpsc::UnboundedSender, ev_receiver: Arc>>, creds: Option, + credential_validator: Option>, local_addr: Option, autodetect: Option, connection_handler: Option>, @@ -384,6 +399,7 @@ impl RdpServer { ev_sender, ev_receiver: Arc::new(Mutex::new(ev_receiver)), creds: None, + credential_validator: None, local_addr: None, autodetect: None, connection_handler, @@ -395,6 +411,17 @@ impl RdpServer { builder::RdpServerBuilder::new() } + /// Set a credential validator for TLS-mode connections. + /// + /// When set, credentials received from the client during `SecureSettingsExchange` + /// are validated through this callback before the session is established. + /// If validation fails, the connection is rejected. + /// + /// Not used for CredSSP/Hybrid connections (those use pre-loaded credentials). + pub fn set_credential_validator(&mut self, validator: Arc) { + self.credential_validator = Some(validator); + } + pub fn event_sender(&self) -> &mpsc::UnboundedSender { &self.ev_sender } @@ -1026,6 +1053,28 @@ impl RdpServer { { debug!("Client accepted"); + // Validate credentials if a validator is configured + if let Some(validator) = &self.credential_validator { + if let Some(creds) = &result.credentials { + match validator.validate(creds) { + Ok(true) => { + debug!("Credential validation succeeded for user: {}", creds.username); + } + Ok(false) => { + warn!("Credential validation failed for user: {}", creds.username); + bail!("credential validation failed"); + } + Err(e) => { + error!("Credential validator error: {e:#}"); + bail!("credential validation error: {e}"); + } + } + } else { + warn!("Credential validator configured but no credentials received from client"); + bail!("no credentials received for validation"); + } + } + if !result.input_events.is_empty() { debug!("Handling input event backlog from acceptor sequence"); self.handle_input_backlog( From c62cf606b7c20c685c2e052d76b84861fc04cdb9 Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Wed, 18 Mar 2026 00:00:47 -0500 Subject: [PATCH 2/5] fix(server): address Copilot review on CredentialValidator - Skip validation when credentials are None (reactivation, CredSSP) instead of bailing, consistent with #1150/#1155 design - Remove username from log messages to avoid leaking sensitive data - Keep validator error details in structured tracing field only - Add spawn_blocking guidance to trait doc for blocking backends --- crates/ironrdp-server/src/server.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 85a87c3a5f..ac567c0869 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -96,7 +96,8 @@ pub trait ConnectionHandler: Send { /// pre-loaded credentials for NTLM challenge-response). /// /// Implement this trait to validate credentials against external systems -/// (PAM, LDAP, database, etc.). +/// (PAM, LDAP, database, etc.). For blocking backends, wrap the call in +/// `tokio::task::spawn_blocking` to avoid stalling the async runtime. pub trait CredentialValidator: Send + Sync { /// Validate credentials received from the client. /// Return `Ok(true)` to accept, `Ok(false)` to reject. @@ -1058,20 +1059,19 @@ impl RdpServer { if let Some(creds) = &result.credentials { match validator.validate(creds) { Ok(true) => { - debug!("Credential validation succeeded for user: {}", creds.username); + debug!("Credential validation succeeded"); } Ok(false) => { - warn!("Credential validation failed for user: {}", creds.username); + warn!("Credential validation failed"); bail!("credential validation failed"); } Err(e) => { - error!("Credential validator error: {e:#}"); - bail!("credential validation error: {e}"); + error!(error = %e, "Credential validator error"); + bail!("credential validation error"); } } } else { - warn!("Credential validator configured but no credentials received from client"); - bail!("no credentials received for validation"); + debug!("Skipping credential validation (no credentials in AcceptorResult)"); } } From fca935390eff66d58ced7156c37b4543f25c79e6 Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Sat, 16 May 2026 11:28:56 -0500 Subject: [PATCH 3/5] fix(server): re-export CredentialValidator from crate root #1270 (wildcard cleanup) replaced `pub use server::*` with an explicit re-export list. This PR predates that change, so without an update the trait would be defined in server.rs but unreachable from downstream consumers via `ironrdp_server::CredentialValidator`. Adds CredentialValidator to the explicit list in lib.rs. --- crates/ironrdp-server/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ironrdp-server/src/lib.rs b/crates/ironrdp-server/src/lib.rs index 0a4da486ad..390016c4a2 100644 --- a/crates/ironrdp-server/src/lib.rs +++ b/crates/ironrdp-server/src/lib.rs @@ -33,8 +33,8 @@ pub use handler::{KeyboardEvent, MouseEvent, RdpServerInputHandler}; #[cfg(feature = "helper")] pub use helper::TlsIdentityCtx; pub use server::{ - ConnectionHandler, Credentials, PostConnectionAction, RdpServer, RdpServerOptions, RdpServerSecurity, ServerEvent, - ServerEventSender, + ConnectionHandler, CredentialValidator, Credentials, PostConnectionAction, RdpServer, RdpServerOptions, + RdpServerSecurity, ServerEvent, ServerEventSender, }; pub use sound::{RdpsndServerHandler, RdpsndServerMessage, SoundServerFactory}; From d0576e3f5746d4b9c98b4300ad1fc898967d6f5a Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Wed, 20 May 2026 11:53:08 -0500 Subject: [PATCH 4/5] refactor(server)!: typed CredentialValidator API + builder method Reshape the public surface introduced earlier in this PR before it lands, to avoid a future breaking touch when ironrdp-server's anyhow-removal pass arrives. Mirrors the post-#1264 hand-rolled error pattern (Display + core::error::Error impls, no thiserror). Trait signature was `Result` (i.e. anyhow::Result through the file-level `use anyhow::Result`). Two issues: - New public surface depending on anyhow at a moment when the workspace is moving the other way (#1277 just removed anyhow from rdpsnd-native). - `Ok(true)` vs `Ok(false)` is stringly typed at the call site: a bare bool with no semantic clue. New shape: - `CredentialDecision::{Accept, Reject}` for the binary outcome. - `CredentialValidationError` wrapping any `core::error::Error + Send + Sync` for the case where the validator backend itself failed (LDAP unreachable, PAM transport error, etc.). Manual Display + Error impls; source chains through. - `validate(&self, &Credentials) -> Result`. The accept/reject/backend-error trichotomy is now explicit at every call site. Rejection is no longer an error; backend failure is. Log + bail strings updated to match the trichotomy. Also addresses the API consistency note: every other configurable on `RdpServerBuilder` goes through `with_*` on the builder. Added `with_credential_validator(Option>)` following the same shape as `with_connection_handler`. The post-construction `set_credential_validator` setter is kept (now takes `Option` to match the field and the builder's setter shape) for dynamic reconfiguration after `build()`. Tests for the trait, the `CredentialDecision` enum, the error wrapper's source chaining, and the `Send + Sync + 'static` bounds through `Arc` live in `crates/ironrdp-testsuite-core/tests/server/credential_validator.rs`, matching the canonical IronRDP split: public-API tests in `ironrdp-testsuite-core`, inline `#[cfg(test)]` only for crate-internal behavior (per the `autodetect.rs` precedent which has both: inline tests on internal state machine, public-API tests in `testsuite-core/tests/server/autodetect.rs`). Verification: `cargo xtask check fmt/lints/tests/typos/locks` all pass; new tests pass in the testsuite-core harness (4 added). Refs #1154, #1150, #1155. --- crates/ironrdp-server/src/builder.rs | 26 +++- crates/ironrdp-server/src/lib.rs | 4 +- crates/ironrdp-server/src/server.rs | 119 +++++++++++++++--- .../tests/server/credential_validator.rs | 70 +++++++++++ .../tests/server/mod.rs | 1 + 5 files changed, 200 insertions(+), 20 deletions(-) create mode 100644 crates/ironrdp-testsuite-core/tests/server/credential_validator.rs diff --git a/crates/ironrdp-server/src/builder.rs b/crates/ironrdp-server/src/builder.rs index cc5d8e8b4f..fb959830ce 100644 --- a/crates/ironrdp-server/src/builder.rs +++ b/crates/ironrdp-server/src/builder.rs @@ -11,7 +11,7 @@ use super::display::{DesktopSize, RdpServerDisplay}; #[cfg(feature = "egfx")] use super::gfx::GfxServerFactory; use super::handler::{KeyboardEvent, MouseEvent, RdpServerInputHandler}; -use super::server::{ConnectionHandler, RdpServer, RdpServerOptions, RdpServerSecurity}; +use super::server::{ConnectionHandler, CredentialValidator, RdpServer, RdpServerOptions, RdpServerSecurity}; use crate::{DisplayUpdate, RdpServerDisplayUpdates, SoundServerFactory}; pub struct WantsAddr {} @@ -37,6 +37,7 @@ pub struct BuilderDone { cliprdr_factory: Option>, sound_factory: Option>, connection_handler: Option>, + credential_validator: Option>, #[cfg(feature = "egfx")] gfx_factory: Option>, display_suppressed: Option>, @@ -133,6 +134,7 @@ impl RdpServerBuilder { sound_factory: None, cliprdr_factory: None, connection_handler: None, + credential_validator: None, codecs: server_codecs_capabilities(&[]).expect("can't panic for &[]"), max_request_size: RdpServerOptions::DEFAULT_MAX_REQUEST_SIZE, #[cfg(feature = "egfx")] @@ -152,6 +154,7 @@ impl RdpServerBuilder { sound_factory: None, cliprdr_factory: None, connection_handler: None, + credential_validator: None, codecs: server_codecs_capabilities(&[]).expect("can't panic for &[]"), max_request_size: RdpServerOptions::DEFAULT_MAX_REQUEST_SIZE, #[cfg(feature = "egfx")] @@ -223,8 +226,23 @@ impl RdpServerBuilder { self } + /// Set a credential validator for TLS-mode connections. + /// + /// When set, credentials received from the client during + /// `SecureSettingsExchange` (`ClientInfoPdu`) are passed to this + /// validator before the session is established. Rejection or a backend + /// error closes the connection. Pass `None` (the default) to skip + /// validation entirely. + /// + /// 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 + } + pub fn build(self) -> RdpServer { - RdpServer::new( + let mut server = RdpServer::new( RdpServerOptions { addr: self.state.addr, security: self.state.security, @@ -239,7 +257,9 @@ impl RdpServerBuilder { #[cfg(feature = "egfx")] self.state.gfx_factory, self.state.display_suppressed, - ) + ); + server.set_credential_validator(self.state.credential_validator); + server } } diff --git a/crates/ironrdp-server/src/lib.rs b/crates/ironrdp-server/src/lib.rs index 390016c4a2..d66c756dae 100644 --- a/crates/ironrdp-server/src/lib.rs +++ b/crates/ironrdp-server/src/lib.rs @@ -33,8 +33,8 @@ pub use handler::{KeyboardEvent, MouseEvent, RdpServerInputHandler}; #[cfg(feature = "helper")] pub use helper::TlsIdentityCtx; pub use server::{ - ConnectionHandler, CredentialValidator, Credentials, PostConnectionAction, RdpServer, RdpServerOptions, - RdpServerSecurity, ServerEvent, ServerEventSender, + ConnectionHandler, CredentialDecision, CredentialValidationError, CredentialValidator, Credentials, + PostConnectionAction, RdpServer, RdpServerOptions, RdpServerSecurity, ServerEvent, ServerEventSender, }; pub use sound::{RdpsndServerHandler, RdpsndServerMessage, SoundServerFactory}; diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index ac567c0869..6d9da6b582 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -1,3 +1,4 @@ +use core::fmt; use core::net::SocketAddr; use core::sync::atomic::{AtomicBool, Ordering}; use core::time::Duration; @@ -89,6 +90,55 @@ pub trait ConnectionHandler: Send { } } +/// Outcome of a successful [`CredentialValidator::validate`] call. +/// +/// A rejection from a working validator is not an error: the validator did +/// its job and decided the credentials do not authenticate. Backend failures +/// (LDAP unreachable, PAM transport broken, database connection lost) are +/// reported via [`CredentialValidationError`] instead. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CredentialDecision { + /// Credentials accepted; the connection proceeds. + Accept, + /// Credentials rejected; the connection is closed. + Reject, +} + +/// Error returned by a [`CredentialValidator`] when the validator backend +/// itself fails (rather than the credentials being invalid). +/// +/// Wraps any [`core::error::Error`] from the backend (LDAP/PAM/DB/etc.) so +/// the trait does not require a particular error library in implementors or +/// consumers. +#[derive(Debug)] +pub struct CredentialValidationError { + source: Box, +} + +impl CredentialValidationError { + /// Wrap a backend error as a credential-validation failure. + pub fn new(source: E) -> Self + where + E: core::error::Error + Send + Sync + 'static, + { + Self { + source: Box::new(source), + } + } +} + +impl fmt::Display for CredentialValidationError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("credential validator backend failure") + } +} + +impl core::error::Error for CredentialValidationError { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + Some(&*self.source) + } +} + /// Server-side credential validator for TLS-mode connections. /// /// Called during connection setup when the server receives client credentials @@ -98,10 +148,41 @@ pub trait ConnectionHandler: Send { /// Implement this trait to validate 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 std::sync::Arc; +/// use ironrdp_server::{ +/// CredentialDecision, CredentialValidationError, CredentialValidator, Credentials, +/// }; +/// +/// struct StaticValidator { +/// expected_user: String, +/// expected_password: String, +/// } +/// +/// impl CredentialValidator for StaticValidator { +/// fn validate( +/// &self, +/// creds: &Credentials, +/// ) -> Result { +/// if creds.username == self.expected_user && creds.password == self.expected_password { +/// Ok(CredentialDecision::Accept) +/// } else { +/// Ok(CredentialDecision::Reject) +/// } +/// } +/// } +/// ``` pub trait CredentialValidator: Send + Sync { /// Validate credentials received from the client. - /// Return `Ok(true)` to accept, `Ok(false)` to reject. - fn validate(&self, credentials: &Credentials) -> Result; + /// + /// Return `Ok(CredentialDecision::Accept)` to permit the connection, + /// `Ok(CredentialDecision::Reject)` to refuse it. Return + /// `Err(CredentialValidationError::new(_))` only when the validator + /// itself could not produce a decision (backend system error). + fn validate(&self, credentials: &Credentials) -> Result; } #[derive(Clone)] @@ -412,15 +493,23 @@ impl RdpServer { builder::RdpServerBuilder::new() } - /// Set a credential validator for TLS-mode connections. + /// Set or clear the credential validator for TLS-mode connections. + /// + /// When set, credentials received from the client during + /// `SecureSettingsExchange` are validated through this callback before + /// the session is established. If the validator returns + /// [`CredentialDecision::Reject`] (or a [`CredentialValidationError`]), + /// the connection is rejected. Passing `None` clears any previously + /// configured validator. /// - /// When set, credentials received from the client during `SecureSettingsExchange` - /// are validated through this callback before the session is established. - /// If validation fails, the connection is rejected. + /// Most callers should configure the validator at construction time via + /// 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: Arc) { - self.credential_validator = Some(validator); + pub fn set_credential_validator(&mut self, validator: Option>) { + self.credential_validator = validator; } pub fn event_sender(&self) -> &mpsc::UnboundedSender { @@ -1058,16 +1147,16 @@ impl RdpServer { if let Some(validator) = &self.credential_validator { if let Some(creds) = &result.credentials { match validator.validate(creds) { - Ok(true) => { - debug!("Credential validation succeeded"); + Ok(CredentialDecision::Accept) => { + debug!("Credential validation accepted"); } - Ok(false) => { - warn!("Credential validation failed"); - bail!("credential validation failed"); + Ok(CredentialDecision::Reject) => { + warn!("Credential validation rejected"); + bail!("credential validation rejected"); } Err(e) => { - error!(error = %e, "Credential validator error"); - bail!("credential validation error"); + error!(error = %e, "Credential validator backend error"); + bail!("credential validation backend error"); } } } else { diff --git a/crates/ironrdp-testsuite-core/tests/server/credential_validator.rs b/crates/ironrdp-testsuite-core/tests/server/credential_validator.rs new file mode 100644 index 0000000000..fc26a41c3b --- /dev/null +++ b/crates/ironrdp-testsuite-core/tests/server/credential_validator.rs @@ -0,0 +1,70 @@ +use core::fmt; +use std::sync::Arc; + +use ironrdp_server::{CredentialDecision, CredentialValidationError, CredentialValidator, Credentials}; + +fn fixed_creds() -> Credentials { + Credentials { + username: "alice".to_owned(), + password: "hunter2".to_owned(), + domain: None, + } +} + +struct AlwaysAccept; +impl CredentialValidator for AlwaysAccept { + fn validate(&self, _: &Credentials) -> Result { + Ok(CredentialDecision::Accept) + } +} + +struct AlwaysReject; +impl CredentialValidator for AlwaysReject { + fn validate(&self, _: &Credentials) -> Result { + Ok(CredentialDecision::Reject) + } +} + +#[derive(Debug)] +struct BackendDown; +impl fmt::Display for BackendDown { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("ldap server unreachable") + } +} +impl core::error::Error for BackendDown {} + +struct AlwaysBackendError; +impl CredentialValidator for AlwaysBackendError { + fn validate(&self, _: &Credentials) -> Result { + Err(CredentialValidationError::new(BackendDown)) + } +} + +#[test] +fn validator_accept_returns_accept() { + let v = AlwaysAccept; + assert_eq!(v.validate(&fixed_creds()).unwrap(), CredentialDecision::Accept); +} + +#[test] +fn validator_reject_returns_reject() { + let v = AlwaysReject; + assert_eq!(v.validate(&fixed_creds()).unwrap(), CredentialDecision::Reject); +} + +#[test] +fn validator_backend_error_propagates_source() { + let v = AlwaysBackendError; + let err = v.validate(&fixed_creds()).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"); +} + +#[test] +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()).unwrap(), CredentialDecision::Accept); +} diff --git a/crates/ironrdp-testsuite-core/tests/server/mod.rs b/crates/ironrdp-testsuite-core/tests/server/mod.rs index 6717853733..7706b1c82b 100644 --- a/crates/ironrdp-testsuite-core/tests/server/mod.rs +++ b/crates/ironrdp-testsuite-core/tests/server/mod.rs @@ -1,3 +1,4 @@ mod acceptor; mod autodetect; +mod credential_validator; mod fast_path; From f1d006a77eb6bee676b524ddebf09fedf2616b4a Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Mon, 1 Jun 2026 19:49:52 -0500 Subject: [PATCH 5/5] feat(server)!: make CredentialValidator async and deny rejected connections Make CredentialValidator::validate async (via async-trait, matching the existing RdpServerDisplay traits, so no new dependency on the server crate). Validators backed by PAM, LDAP, or a database can offload blocking work with spawn_blocking instead of stalling the server runtime; native-async backends just await. On rejection or backend error, send a ServerSetErrorInfoPdu with ServerDeniedConnection before closing, matching the acceptor's exact-match denial so both paths refuse the same spec-defined way rather than dropping the connection silently. Add a built-in ExactMatchCredentialValidator so the single-known-account case stays a one-liner. --- Cargo.lock | 2 + crates/ironrdp-server/src/lib.rs | 3 +- crates/ironrdp-server/src/server.rs | 78 ++++++++++++++++--- crates/ironrdp-testsuite-core/Cargo.toml | 2 + .../tests/server/credential_validator.rs | 34 ++++---- 5 files changed, 94 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9794ca196c..08a9a285c6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2905,6 +2905,7 @@ version = "0.0.0" dependencies = [ "anyhow", "array-concat", + "async-trait", "expect-test", "hex", "ironrdp-acceptor", @@ -2936,6 +2937,7 @@ dependencies = [ "pretty_assertions", "proptest", "rstest", + "tokio", "visibility", ] diff --git a/crates/ironrdp-server/src/lib.rs b/crates/ironrdp-server/src/lib.rs index d66c756dae..4ae6b71679 100644 --- a/crates/ironrdp-server/src/lib.rs +++ b/crates/ironrdp-server/src/lib.rs @@ -34,7 +34,8 @@ pub use handler::{KeyboardEvent, MouseEvent, RdpServerInputHandler}; pub use helper::TlsIdentityCtx; pub use server::{ ConnectionHandler, CredentialDecision, CredentialValidationError, CredentialValidator, Credentials, - PostConnectionAction, RdpServer, RdpServerOptions, RdpServerSecurity, ServerEvent, ServerEventSender, + ExactMatchCredentialValidator, PostConnectionAction, RdpServer, RdpServerOptions, RdpServerSecurity, ServerEvent, + ServerEventSender, }; pub use sound::{RdpsndServerHandler, RdpsndServerMessage, SoundServerFactory}; diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 6d9da6b582..f471a2a3e6 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -20,6 +20,7 @@ use ironrdp_pdu::mcs::{SendDataIndication, SendDataRequest}; use ironrdp_pdu::rdp::capability_sets::{BitmapCodecs, CapabilitySet, CmdFlags, CodecProperty, GeneralExtraFlags}; pub use ironrdp_pdu::rdp::client_info::Credentials; use ironrdp_pdu::rdp::headers::{ServerDeactivateAll, ShareControlPdu}; +use ironrdp_pdu::rdp::server_error_info::{ErrorInfo, ProtocolIndependentCode, ServerSetErrorInfoPdu}; use ironrdp_pdu::x224::X224; use ironrdp_pdu::{Action, PduResult, decode_err, mcs, nego, rdp}; use ironrdp_rdpsnd as rdpsnd; @@ -152,18 +153,16 @@ impl core::error::Error for CredentialValidationError { /// # Example /// /// ```ignore -/// use std::sync::Arc; -/// use ironrdp_server::{ -/// CredentialDecision, CredentialValidationError, CredentialValidator, Credentials, -/// }; +/// use ironrdp_server::{CredentialDecision, CredentialValidationError, CredentialValidator, Credentials}; /// /// struct StaticValidator { /// expected_user: String, /// expected_password: String, /// } /// +/// #[async_trait::async_trait] /// impl CredentialValidator for StaticValidator { -/// fn validate( +/// async fn validate( /// &self, /// creds: &Credentials, /// ) -> Result { @@ -175,6 +174,7 @@ impl core::error::Error for CredentialValidationError { /// } /// } /// ``` +#[async_trait::async_trait] pub trait CredentialValidator: Send + Sync { /// Validate credentials received from the client. /// @@ -182,7 +182,39 @@ pub trait CredentialValidator: Send + Sync { /// `Ok(CredentialDecision::Reject)` to refuse it. Return /// `Err(CredentialValidationError::new(_))` only when the validator /// itself could not produce a decision (backend system error). - fn validate(&self, credentials: &Credentials) -> Result; + /// + /// Implementors backed by blocking systems (PAM, libldap, a synchronous + /// 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; +} + +/// A built-in [`CredentialValidator`] that accepts exactly one fixed set of credentials. +/// +/// This is the validation-policy equivalent of the acceptor's pre-loaded +/// exact-match: it keeps the common "one known account" case a one-liner while +/// going through the same hook as PAM, LDAP, or database-backed validators. +pub struct ExactMatchCredentialValidator { + expected: Credentials, +} + +impl ExactMatchCredentialValidator { + /// Build a validator that accepts only `expected` and rejects everything else. + pub fn new(expected: Credentials) -> Self { + Self { expected } + } +} + +#[async_trait::async_trait] +impl CredentialValidator for ExactMatchCredentialValidator { + async fn validate(&self, credentials: &Credentials) -> Result { + if credentials == &self.expected { + Ok(CredentialDecision::Accept) + } else { + Ok(CredentialDecision::Reject) + } + } } #[derive(Clone)] @@ -1143,19 +1175,24 @@ impl RdpServer { { debug!("Client accepted"); - // Validate credentials if a validator is configured - if let Some(validator) = &self.credential_validator { + // 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. + if let Some(validator) = self.credential_validator.clone() { if let Some(creds) = &result.credentials { - match validator.validate(creds) { + match validator.validate(creds).await { Ok(CredentialDecision::Accept) => { debug!("Credential validation accepted"); } Ok(CredentialDecision::Reject) => { warn!("Credential validation rejected"); + send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; bail!("credential validation rejected"); } Err(e) => { error!(error = %e, "Credential validator backend error"); + send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; bail!("credential validation backend error"); } } @@ -1576,6 +1613,29 @@ async fn deactivate_all( Ok(()) } +/// 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. +async fn send_access_denied( + io_channel_id: u16, + user_channel_id: u16, + writer: &mut impl FramedWrite, +) -> Result<(), anyhow::Error> { + let info = ServerSetErrorInfoPdu(ErrorInfo::ProtocolIndependentCode( + ProtocolIndependentCode::ServerDeniedConnection, + )); + let user_data = encode_vec(&info)?.into(); + let pdu = SendDataIndication { + initiator_id: user_channel_id, + channel_id: io_channel_id, + user_data, + }; + let msg = encode_vec(&X224(pdu))?; + writer.write_all(&msg).await?; + Ok(()) +} + struct SharedWriter<'w, W: FramedWrite> { writer: Rc>, } diff --git a/crates/ironrdp-testsuite-core/Cargo.toml b/crates/ironrdp-testsuite-core/Cargo.toml index 9c6961c96b..68cbd45fd8 100644 --- a/crates/ironrdp-testsuite-core/Cargo.toml +++ b/crates/ironrdp-testsuite-core/Cargo.toml @@ -34,6 +34,7 @@ visibility = { version = "0.1", optional = true } [dev-dependencies] anyhow = "1" +async-trait = "0.1" expect-test.workspace = true hex = "0.4" ironrdp-cliprdr-format.path = "../ironrdp-cliprdr-format" @@ -60,6 +61,7 @@ png = "0.18" pretty_assertions = "1.4" proptest.workspace = true rstest.workspace = true +tokio = { version = "1", features = ["macros", "rt"] } [lints] workspace = true diff --git a/crates/ironrdp-testsuite-core/tests/server/credential_validator.rs b/crates/ironrdp-testsuite-core/tests/server/credential_validator.rs index fc26a41c3b..4b4dd280a3 100644 --- a/crates/ironrdp-testsuite-core/tests/server/credential_validator.rs +++ b/crates/ironrdp-testsuite-core/tests/server/credential_validator.rs @@ -1,6 +1,7 @@ use core::fmt; use std::sync::Arc; +use async_trait::async_trait; use ironrdp_server::{CredentialDecision, CredentialValidationError, CredentialValidator, Credentials}; fn fixed_creds() -> Credentials { @@ -12,15 +13,17 @@ fn fixed_creds() -> Credentials { } struct AlwaysAccept; +#[async_trait] impl CredentialValidator for AlwaysAccept { - fn validate(&self, _: &Credentials) -> Result { + async fn validate(&self, _: &Credentials) -> Result { Ok(CredentialDecision::Accept) } } struct AlwaysReject; +#[async_trait] impl CredentialValidator for AlwaysReject { - fn validate(&self, _: &Credentials) -> Result { + async fn validate(&self, _: &Credentials) -> Result { Ok(CredentialDecision::Reject) } } @@ -35,36 +38,37 @@ impl fmt::Display for BackendDown { impl core::error::Error for BackendDown {} struct AlwaysBackendError; +#[async_trait] impl CredentialValidator for AlwaysBackendError { - fn validate(&self, _: &Credentials) -> Result { + async fn validate(&self, _: &Credentials) -> Result { Err(CredentialValidationError::new(BackendDown)) } } -#[test] -fn validator_accept_returns_accept() { +#[tokio::test] +async fn validator_accept_returns_accept() { let v = AlwaysAccept; - assert_eq!(v.validate(&fixed_creds()).unwrap(), CredentialDecision::Accept); + assert_eq!(v.validate(&fixed_creds()).await.unwrap(), CredentialDecision::Accept); } -#[test] -fn validator_reject_returns_reject() { +#[tokio::test] +async fn validator_reject_returns_reject() { let v = AlwaysReject; - assert_eq!(v.validate(&fixed_creds()).unwrap(), CredentialDecision::Reject); + assert_eq!(v.validate(&fixed_creds()).await.unwrap(), CredentialDecision::Reject); } -#[test] -fn validator_backend_error_propagates_source() { +#[tokio::test] +async fn validator_backend_error_propagates_source() { let v = AlwaysBackendError; - let err = v.validate(&fixed_creds()).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"); } -#[test] -fn validator_can_be_held_behind_arc_dyn() { +#[tokio::test] +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()).unwrap(), CredentialDecision::Accept); + assert_eq!(v.validate(&fixed_creds()).await.unwrap(), CredentialDecision::Accept); }