From 146e3374c45306b80c95b8b8cddfd9d0dff8fb77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 20 Oct 2026 16:03:12 -0400 Subject: [PATCH 1/7] fix(rdpdr): complete smartcard calls in the noop backend `NoopRdpdrBackend::handle_scard_call` returned no messages, so every MS-RDPESC IOCTL sent to an announced smartcard (EstablishContext, ListReaders, GetStatusChange, ...) was never completed and server-side WinSCard callers hung. The .NET FFI (`with_static_channel_rdpdr`) and `ironrdp-client` announce a smartcard on top of this backend. The noop backend now answers each call with the IOCTL-specific return structure carrying SCARD_E_NO_SERVICE and IoStatus STATUS_SUCCESS, as MS-RDPESC 3.1.4 requires for smart card errors, or STATUS_BUFFER_TOO_SMALL when the server's output buffer cannot hold it. The new `ScardCall::error_return` helper builds the matching return structure for any call. Co-Authored-By: Claude Opus 5.5 --- crates/ironrdp-rdpdr/src/backend/noop.rs | 22 +++++-- crates/ironrdp-rdpdr/src/pdu/esc/mod.rs | 61 +++++++++++++++++++ .../ironrdp-testsuite-core/tests/rdpdr/mod.rs | 56 +++++++++++++++++ 3 files changed, 134 insertions(+), 5 deletions(-) diff --git a/crates/ironrdp-rdpdr/src/backend/noop.rs b/crates/ironrdp-rdpdr/src/backend/noop.rs index 9bb56d3e7b..308175dfc8 100644 --- a/crates/ironrdp-rdpdr/src/backend/noop.rs +++ b/crates/ironrdp-rdpdr/src/backend/noop.rs @@ -5,10 +5,10 @@ use ironrdp_svc::SvcMessage; use super::RdpdrBackend; use crate::pdu::RdpdrPdu; use crate::pdu::efs::{ - DeviceControlRequest, DeviceIoRequest, DeviceIoResponse, DeviceWriteResponse, NtStatus, + DeviceControlRequest, DeviceControlResponse, DeviceIoRequest, DeviceIoResponse, DeviceWriteResponse, NtStatus, ServerDeviceAnnounceResponse, }; -use crate::pdu::esc::{ScardCall, ScardIoCtlCode}; +use crate::pdu::esc::{ReturnCode, ScardCall, ScardIoCtlCode}; #[derive(Debug)] pub struct NoopRdpdrBackend; @@ -19,12 +19,24 @@ impl RdpdrBackend for NoopRdpdrBackend { fn handle_server_device_announce_response(&mut self, _pdu: ServerDeviceAnnounceResponse) -> PduResult<()> { Ok(()) } + /// Completes every smart card call with `SCARD_E_NO_SERVICE`. + /// + /// The server waits for a completion of each IOCTL, so leaving one unanswered would hang + /// the server-side WinSCard caller. fn handle_scard_call( &mut self, - _req: DeviceControlRequest, - _call: ScardCall, + req: DeviceControlRequest, + call: ScardCall, ) -> PduResult> { - Ok(Vec::new()) + let output = call.error_return(req.io_control_code, ReturnCode::NoService); + let fits = u32::try_from(output.size()).is_ok_and(|size| size <= req.output_buffer_length); + let response = if fits { + DeviceControlResponse::new(req, NtStatus::SUCCESS, Some(output)) + } else { + // The server retries with a larger output buffer (MS-RDPESC 3.2.5.2). + DeviceControlResponse::new(req, NtStatus::BUFFER_TOO_SMALL, None) + }; + Ok(vec![SvcMessage::from(RdpdrPdu::DeviceControlResponse(response))]) } fn handle_drive_io_request(&mut self, _req: crate::pdu::efs::ServerDriveIoRequest) -> PduResult> { Err(pdu_other_err!( diff --git a/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs b/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs index 8ded7a9ae9..e0e4645061 100644 --- a/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs +++ b/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs @@ -167,6 +167,67 @@ impl ScardCall { } } } + + /// Builds the IOCTL-specific `*_Return` structure for a call that failed with `return_code`. + /// + /// Per [\[MS-RDPESC\]] section 3.1.4, smart card errors are carried in the `ReturnCode` of the + /// structure the server decodes for this IOCTL, while `IoStatus` stays `STATUS_SUCCESS`. All + /// other output fields are left empty. + /// + /// [\[MS-RDPESC\]]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpesc/0428ca28-b4dc-46a3-97c3-01887fa44a90 + pub fn error_return(&self, io_ctl_code: ScardIoCtlCode, return_code: ReturnCode) -> Box { + let undefined = CardProtocol::SCARD_PROTOCOL_UNDEFINED; + let charset = match io_ctl_code { + ScardIoCtlCode::ListReadersA | ScardIoCtlCode::ListReaderGroupsA | ScardIoCtlCode::StatusA => { + CharacterSet::Ansi + } + _ => CharacterSet::Unicode, + }; + + match self { + ScardCall::EstablishContextCall(_) => { + Box::new(EstablishContextReturn::new(return_code, ScardContext::from_native(0))) + } + ScardCall::ListReaderGroupsCall(_) | ScardCall::ListReadersCall(_) => { + Box::new(ListReadersReturn::probe(return_code, 0, charset)) + } + // LocateCards_Return has the same layout as GetStatusChange_Return. + ScardCall::GetStatusChangeCall(_) | ScardCall::LocateCardsCall(_) | ScardCall::LocateCardsByAtrCall(_) => { + Box::new(GetStatusChangeReturn::new(return_code, Vec::new())) + } + ScardCall::ConnectCall(_) => Box::new(ConnectReturn::new( + return_code, + ScardHandle::from_native(ScardContext::from_native(0), 0), + undefined, + )), + ScardCall::ReconnectCall(_) => Box::new(ReconnectReturn::new(return_code, undefined)), + ScardCall::TransmitCall(_) => Box::new(TransmitReturn::new(return_code, None, Vec::new())), + ScardCall::StatusCall(_) => Box::new(StatusReturn::names_probe( + return_code, + 0, + CardState::Unknown, + undefined, + [0; 32], + 0, + charset, + )), + ScardCall::StateCall(_) => Box::new(StateReturn::atr_probe(return_code, CardState::Unknown, undefined, 0)), + ScardCall::ControlCall(_) => Box::new(ControlReturn::new(return_code, Vec::new())), + ScardCall::GetAttribCall(_) => Box::new(GetAttribReturn::new(return_code, Vec::new())), + ScardCall::GetTransmitCountCall(_) => Box::new(GetTransmitCountReturn::new(return_code, 0)), + ScardCall::GetDeviceTypeIdCall(_) => Box::new(GetDeviceTypeIdReturn::new(return_code, 0)), + ScardCall::ReadCacheCall(_) => Box::new(ReadCacheReturn::new(return_code, Vec::new())), + ScardCall::GetReaderIconCall(_) => Box::new(GetReaderIconReturn::new(return_code, Vec::new())), + ScardCall::AccessStartedEventCall(_) + | ScardCall::HCardAndDispositionCall(_) + | ScardCall::SetAttribCall(_) + | ScardCall::ContextCall(_) + | ScardCall::WriteCacheCall(_) + | ScardCall::ContextAndStringCall(_) + | ScardCall::ContextAndTwoStringCall(_) + | ScardCall::Unsupported => Box::new(LongReturn::new(return_code)), + } + } } /// MS-RDPESC `cbContext` / `cbHandle` maximum (`range(0,16)`). diff --git a/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs b/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs index 74573573c9..0285164488 100644 --- a/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs +++ b/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs @@ -534,6 +534,62 @@ fn smartcard_device_announce_remains_pre_logon() { assert_eq!(announced_devices(&responses[0]), vec![(42, DeviceType::Print)]); } +fn encoded_smartcard_establish_context_request(output_buffer_length: u32) -> Vec { + let mut encoded = encode_vec(&RdpdrPdu::DeviceIoRequest(DeviceIoRequest { + device_id: 1, + file_id: 0, + completion_id: 0x200, + major_function: MajorFunction::DeviceControl, + minor_function: MinorFunction::from(0), + })) + .unwrap(); + // EstablishContext_Call { dwScope: SCARD_SCOPE_USER } in an MS-RPCE type serialization stream. + let input = [ + [0x01, 0x10, 0x08, 0x00, 0xCC, 0xCC, 0xCC, 0xCC], // Common type header + [0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], // Private header, 8-byte object buffer + [0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], // dwScope + alignment padding + ] + .concat(); + encoded.extend_from_slice(&output_buffer_length.to_le_bytes()); + encoded.extend_from_slice(&u32::try_from(input.len()).unwrap().to_le_bytes()); + encoded.extend_from_slice(&0x0009_0014u32.to_le_bytes()); // SCARD_IOCTL_ESTABLISHCONTEXT + encoded.extend_from_slice(&[0; 20]); // Padding + encoded.extend_from_slice(&input); + encoded +} + +#[test] +fn noop_backend_completes_smartcard_calls_with_no_service() { + const SCARD_E_NO_SERVICE: u32 = 0x8010_001D; + + let mut rdpdr = Rdpdr::new(Box::new(NoopRdpdrBackend), "IronRDP".to_owned()).with_smartcard(1); + + let responses = rdpdr + .process(&encoded_smartcard_establish_context_request(2048)) + .unwrap(); + assert_eq!(responses.len(), 1); + + let encoded = responses[0].encode_unframed_pdu().unwrap(); + assert_eq!(&encoded[..4], &[0x72, 0x44, 0x43, 0x49]); // RDPDR + DEVICE_IOCOMPLETION + assert_eq!(read_u32(&encoded[8..]), 0x200); // CompletionId + assert_eq!(ntstatus_at(&encoded, 12), NtStatus::SUCCESS); + // OutputBuffer holds EstablishContext_Return after the 16-byte MS-RPCE headers. + assert_eq!(read_u32_as_usize(&encoded[16..]), encoded.len() - 20); + assert_eq!(read_u32(&encoded[36..]), SCARD_E_NO_SERVICE); +} + +#[test] +fn noop_backend_reports_small_smartcard_output_buffer() { + let mut rdpdr = Rdpdr::new(Box::new(NoopRdpdrBackend), "IronRDP".to_owned()).with_smartcard(1); + + let responses = rdpdr.process(&encoded_smartcard_establish_context_request(0)).unwrap(); + assert_eq!(responses.len(), 1); + + let encoded = responses[0].encode_unframed_pdu().unwrap(); + assert_eq!(ntstatus_at(&encoded, 12), NtStatus::BUFFER_TOO_SMALL); + assert_eq!(read_u32(&encoded[16..]), 0); // OutputBufferLength +} + #[test] fn rdp51_client_id_confirm_announces_all_devices_pre_logon() { let mut rdpdr = Rdpdr::new(Box::new(NoopRdpdrBackend), "IronRDP".to_owned()) From fc9301a675e61a7cdfbe560a8cd8a060a196a0e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 20 Oct 2026 16:37:38 -0400 Subject: [PATCH 2/7] feat(ffi): redirect smartcards through WinSCard on Windows `ClientConnector.WithStaticChannelRdpdr` now backs the announced smartcard with the native WinSCard passthrough from `ironrdp-rdpdr-native` on Windows, the same backend the viewer and ActiveX use. Other platforms keep the noop backend, which answers every call with SCARD_E_NO_SERVICE. The native backend completes blocking calls (GetStatusChange, Connect, Transmit, ...) on worker threads. `ActiveStage.Process` now forwards those completions after each frame, and the new `ActiveStage.PollRdpdrDeferred` lets hosts flush them while the server is idle. The Avalonia example polls every 50 ms from its read loop, like `ironrdp-client` does. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 1 + ffi/Cargo.toml | 3 ++ .../MainWindow.axaml.cs | 29 ++++++++++++++++- .../Generated/ActiveStage.cs | 30 ++++++++++++++++++ .../Generated/ClientConnector.cs | 7 +++++ .../Generated/RawActiveStage.cs | 11 +++++++ .../Generated/RawClientConnector.cs | 7 +++++ ffi/src/connector/mod.rs | 24 +++++++++++--- ffi/src/session/mod.rs | 31 ++++++++++++++++++- 9 files changed, 136 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3947a6b4a9..ddaadd9d56 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1702,6 +1702,7 @@ dependencies = [ "ironrdp-core 0.2.1", "ironrdp-dvc-pipe-proxy", "ironrdp-rdcleanpath", + "ironrdp-rdpdr-native", "ironrdp-vmconnect", "sspi", "thiserror 2.0.21", diff --git a/ffi/Cargo.toml b/ffi/Cargo.toml index ce53aa5a74..1e21affda1 100644 --- a/ffi/Cargo.toml +++ b/ffi/Cargo.toml @@ -26,6 +26,9 @@ tracing = { version = "0.1", features = ["log"] } tracing-subscriber = { version = "0.3", features = ["env-filter"] } anyhow = "1.0" +[target.'cfg(windows)'.dependencies] +ironrdp-rdpdr-native.path = "../crates/ironrdp-rdpdr-native" + [target.'cfg(windows)'.build-dependencies] embed-resource = "3.0" diff --git a/ffi/dotnet/Devolutions.IronRdp.AvaloniaExample/MainWindow.axaml.cs b/ffi/dotnet/Devolutions.IronRdp.AvaloniaExample/MainWindow.axaml.cs index 5f1d065c24..44808f3af5 100644 --- a/ffi/dotnet/Devolutions.IronRdp.AvaloniaExample/MainWindow.axaml.cs +++ b/ffi/dotnet/Devolutions.IronRdp.AvaloniaExample/MainWindow.axaml.cs @@ -20,6 +20,8 @@ namespace Devolutions.IronRdp.AvaloniaExample; public partial class MainWindow : Window { + static readonly TimeSpan RdpdrDeferredPollInterval = TimeSpan.FromMilliseconds(50); + readonly InputDatabase? _inputDatabase = InputDatabase.New(); ActiveStage? _activeStage; DecodedImage? _decodedImage; @@ -304,13 +306,33 @@ private void ReadPduAndProcessActiveStage() try { var keepLooping = true; + Task<(Action, byte[])>? readPduTask = null; while (keepLooping) { - var (action, payload) = await _framed!.ReadPdu(); + // Start the next read only once the previous frame's outputs are handled: + // reactivation reads from the same stream. + readPduTask ??= _framed!.ReadPdu(); + + // Wake up periodically to send RDPDR completions (e.g. smartcard calls) that + // finished while the server sent nothing. + if (readPduTask != await Task.WhenAny(readPduTask, Task.Delay(RdpdrDeferredPollInterval))) + { + var deferred = WithStageLock(() => _activeStage!.PollRdpdrDeferred()); + keepLooping = await HandleActiveStageOutput(deferred); + continue; + } + + var (action, payload) = await readPduTask; + readPduTask = null; var outputIterator = WithStageLock(() => _activeStage!.Process(_decodedImage!, action, payload)); keepLooping = await HandleActiveStageOutput(outputIterator); } + // The session can end while a read is still pending: report its failure, if any. + readPduTask?.ContinueWith( + t => Trace.TraceError("Error reading PDU: " + t.Exception!.GetBaseException().Message), + TaskContinuationOptions.OnlyOnFaulted); + Trace.TraceInformation("ReadPduAndProcessActiveStage loop ended"); } catch (Exception e) @@ -492,6 +514,11 @@ private void OnKeyUp(object? sender, KeyEventArgs? e) private void SendFastpath(FastPathInputEventIterator fastpath) { + if (_activeStage == null || _decodedImage == null) + { + return; + } + var output = WithStageLock(() => _activeStage!.ProcessFastpathInput(_decodedImage!, fastpath)); var _ = HandleActiveStageOutput(output); } diff --git a/ffi/dotnet/Devolutions.IronRdp/Generated/ActiveStage.cs b/ffi/dotnet/Devolutions.IronRdp/Generated/ActiveStage.cs index d221197b81..741aa0431a 100644 --- a/ffi/dotnet/Devolutions.IronRdp/Generated/ActiveStage.cs +++ b/ffi/dotnet/Devolutions.IronRdp/Generated/ActiveStage.cs @@ -123,6 +123,36 @@ public ActiveStageOutputIterator Process(DecodedImage image, Action action, byte } } + /// + /// Collects RDPDR completions that finished asynchronously (e.g. smartcard waits). + /// + /// + /// [`Self::process`] already collects them after each frame. Also call this on a short + /// timer (e.g. every 50 ms) so that they reach the server while it sends no frames. Do + /// not call it concurrently with other methods of this `ActiveStage`. + /// + /// + /// + /// A ActiveStageOutputIterator allocated on Rust side. + /// + public ActiveStageOutputIterator PollRdpdrDeferred() + { + unsafe + { + if (_inner == null) + { + throw new ObjectDisposedException("ActiveStage"); + } + Raw.SessionFfiResultBoxActiveStageOutputIteratorBoxIronRdpError result = Raw.ActiveStage.PollRdpdrDeferred(_inner); + if (!result.isOk) + { + throw new IronRdpException(new IronRdpError(result.Err)); + } + Raw.ActiveStageOutputIterator* retVal = result.Ok; + return new ActiveStageOutputIterator(retVal); + } + } + /// /// /// A ActiveStageOutputIterator allocated on Rust side. diff --git a/ffi/dotnet/Devolutions.IronRdp/Generated/ClientConnector.cs b/ffi/dotnet/Devolutions.IronRdp/Generated/ClientConnector.cs index d2dcabe00f..03cd1646d7 100644 --- a/ffi/dotnet/Devolutions.IronRdp/Generated/ClientConnector.cs +++ b/ffi/dotnet/Devolutions.IronRdp/Generated/ClientConnector.cs @@ -89,6 +89,13 @@ public void WithStaticChannelRdpSnd() /// /// Must use /// + /// + /// Announces a smartcard device. On Windows, calls are passed through to the local + /// WinSCard service; elsewhere every call fails with `SCARD_E_NO_SERVICE`. + ///
+ /// Some WinSCard calls complete asynchronously: call `ActiveStage::poll_rdpdr_deferred` + /// periodically to send their results to the server. + /// /// public void WithStaticChannelRdpdr(string computerName, uint smartCardDeviceId) { diff --git a/ffi/dotnet/Devolutions.IronRdp/Generated/RawActiveStage.cs b/ffi/dotnet/Devolutions.IronRdp/Generated/RawActiveStage.cs index a74555bea9..4f62354d61 100644 --- a/ffi/dotnet/Devolutions.IronRdp/Generated/RawActiveStage.cs +++ b/ffi/dotnet/Devolutions.IronRdp/Generated/RawActiveStage.cs @@ -33,6 +33,17 @@ public partial struct ActiveStage [DllImport(NativeLib, CallingConvention = CallingConvention.Cdecl, EntryPoint = "ActiveStage_process", ExactSpelling = true)] public static unsafe extern SessionFfiResultBoxActiveStageOutputIteratorBoxIronRdpError Process(ActiveStage* self, DecodedImage* image, Action* action, byte* payload, nuint payloadSz); + /// + /// Collects RDPDR completions that finished asynchronously (e.g. smartcard waits). + /// + /// + /// [`Self::process`] already collects them after each frame. Also call this on a short + /// timer (e.g. every 50 ms) so that they reach the server while it sends no frames. Do + /// not call it concurrently with other methods of this `ActiveStage`. + /// + [DllImport(NativeLib, CallingConvention = CallingConvention.Cdecl, EntryPoint = "ActiveStage_poll_rdpdr_deferred", ExactSpelling = true)] + public static unsafe extern SessionFfiResultBoxActiveStageOutputIteratorBoxIronRdpError PollRdpdrDeferred(ActiveStage* self); + [DllImport(NativeLib, CallingConvention = CallingConvention.Cdecl, EntryPoint = "ActiveStage_process_fastpath_input", ExactSpelling = true)] public static unsafe extern SessionFfiResultBoxActiveStageOutputIteratorBoxIronRdpError ProcessFastpathInput(ActiveStage* self, DecodedImage* image, FastPathInputEventIterator* fastpathInput); diff --git a/ffi/dotnet/Devolutions.IronRdp/Generated/RawClientConnector.cs b/ffi/dotnet/Devolutions.IronRdp/Generated/RawClientConnector.cs index 8de238c443..1df8001111 100644 --- a/ffi/dotnet/Devolutions.IronRdp/Generated/RawClientConnector.cs +++ b/ffi/dotnet/Devolutions.IronRdp/Generated/RawClientConnector.cs @@ -28,6 +28,13 @@ public partial struct ClientConnector /// /// Must use /// + /// + /// Announces a smartcard device. On Windows, calls are passed through to the local + /// WinSCard service; elsewhere every call fails with `SCARD_E_NO_SERVICE`. + ///
+ /// Some WinSCard calls complete asynchronously: call `ActiveStage::poll_rdpdr_deferred` + /// periodically to send their results to the server. + /// [DllImport(NativeLib, CallingConvention = CallingConvention.Cdecl, EntryPoint = "ClientConnector_with_static_channel_rdpdr", ExactSpelling = true)] public static unsafe extern ConnectorFfiResultVoidBoxIronRdpError WithStaticChannelRdpdr(ClientConnector* self, byte* computerName, nuint computerNameSz, uint smartCardDeviceId); diff --git a/ffi/src/connector/mod.rs b/ffi/src/connector/mod.rs index bfcdf67083..0c0c03af53 100644 --- a/ffi/src/connector/mod.rs +++ b/ffi/src/connector/mod.rs @@ -55,6 +55,12 @@ pub mod ffi { // FIXME: We need to create opaque for ironrdp::rdpdr::Rdpdr /// Must use + /// + /// Announces a smartcard device. On Windows, calls are passed through to the local + /// WinSCard service; elsewhere every call fails with `SCARD_E_NO_SERVICE`. + /// + /// Some WinSCard calls complete asynchronously: call `ActiveStage::poll_rdpdr_deferred` + /// periodically to send their results to the server. pub fn with_static_channel_rdpdr( &mut self, computer_name: &str, @@ -63,12 +69,20 @@ pub mod ffi { let Some(connector) = self.0.take() else { return Err(ValueConsumedError::for_item("connector").into()); }; - self.0 = Some( - connector.with_static_channel( - ironrdp::rdpdr::Rdpdr::new(Box::new(ironrdp::rdpdr::NoopRdpdrBackend {}), computer_name.to_owned()) - .with_smartcard(smart_card_device_id), - ), + + #[cfg(windows)] + let backend: Box = Box::new( + ironrdp_rdpdr_native::WindowsRdpdrBackendFactory::from_drives(Vec::new()) + .map_err(|_| "invalid RDPDR backend configuration")? + .with_smartcard(true) + .build(), ); + #[cfg(not(windows))] + let backend: Box = Box::new(ironrdp::rdpdr::NoopRdpdrBackend); + + self.0 = Some(connector.with_static_channel( + ironrdp::rdpdr::Rdpdr::new(backend, computer_name.to_owned()).with_smartcard(smart_card_device_id), + )); Ok(()) } diff --git a/ffi/src/session/mod.rs b/ffi/src/session/mod.rs index 59a904d1e4..0c0697019b 100644 --- a/ffi/src/session/mod.rs +++ b/ffi/src/session/mod.rs @@ -131,12 +131,23 @@ pub mod ffi { ) -> Result, Box> { // The .NET host calls this right after reading each frame, so the call time stands in // for the arrival time. - let outputs = self + let mut outputs = self .0 .process(&mut image.0, action.0, payload, Some(crate::utils::monotonic_now()))?; + outputs.extend(self.poll_rdpdr_deferred_output()?); Ok(Box::new(ActiveStageOutputIterator(outputs.into()))) } + /// Collects RDPDR completions that finished asynchronously (e.g. smartcard waits). + /// + /// [`Self::process`] already collects them after each frame. Also call this on a short + /// timer (e.g. every 50 ms) so that they reach the server while it sends no frames. Do + /// not call it concurrently with other methods of this `ActiveStage`. + pub fn poll_rdpdr_deferred(&mut self) -> Result, Box> { + let outputs = self.poll_rdpdr_deferred_output()?; + Ok(Box::new(ActiveStageOutputIterator(outputs.into_iter().collect()))) + } + pub fn process_fastpath_input( &mut self, image: &mut DecodedImage, @@ -302,6 +313,24 @@ pub mod ffi { pub fn set_enable_server_pointer(&mut self, enable_server_pointer: bool) { self.0.set_enable_server_pointer(enable_server_pointer); } + + fn poll_rdpdr_deferred_output( + &mut self, + ) -> Result, Box> { + let Some(rdpdr) = self.0.get_svc_processor_mut::() else { + return Ok(None); + }; + + let messages = rdpdr.poll_deferred_messages()?; + if messages.is_empty() { + return Ok(None); + } + + let frame = self + .0 + .process_svc_messages_by_name(&ironrdp::rdpdr::Rdpdr::NAME, messages)?; + Ok(Some(ironrdp::session::ActiveStageOutput::ResponseFrame(frame))) + } } pub enum ActiveStageOutputType { From c4974e08f5244deefba94f4c80278c8976162a5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 20 Oct 2026 20:02:44 -0400 Subject: [PATCH 3/7] fix(rdpdr): zero smartcard error returns and share completion helpers Error returns built for failed smartcard calls carried a nonzero REDIR_SCARDCONTEXT/REDIR_SCARDHANDLE, while MS-RDPESC 2.2.3 requires all fields besides ReturnCode to be zero. Add `ScardContext::empty` and `ScardHandle::empty`, and use them in the shared error path and in the native backend's EstablishContext/Connect failures. An IOCTL with no known return structure is now completed with STATUS_UNSUCCESSFUL and no output (MS-RDPESC 3.1.4, FreeRDP) instead of a Long_Return the server cannot decode. ReleaseTartedEvent keeps its Long_Return. `ScardCall::error_response` replaces `error_return` and builds the whole completion, and `esc::complete_call` holds the STATUS_BUFFER_TOO_SMALL retry rule. The native backend now uses both instead of its private copies. Co-Authored-By: Claude Opus 5.5 --- .../src/windows/scard/mod.rs | 70 +++++------------ crates/ironrdp-rdpdr/src/backend/noop.rs | 11 +-- crates/ironrdp-rdpdr/src/pdu/esc/mod.rs | 77 +++++++++++++++---- .../ironrdp-testsuite-core/tests/rdpdr/mod.rs | 26 ++++++- 4 files changed, 108 insertions(+), 76 deletions(-) diff --git a/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs b/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs index 54e6cfca31..cd0ee2e719 100644 --- a/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs +++ b/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs @@ -11,9 +11,9 @@ use ironrdp_pdu::utils::{CharacterSet, encoded_multistring_len}; use ironrdp_rdpdr::pdu::RdpdrPdu; use ironrdp_rdpdr::pdu::efs::{DeviceControlRequest, DeviceControlResponse, NtStatus}; use ironrdp_rdpdr::pdu::esc::{ - CardProtocol, CardState, CardStateFlags, ConnectCall, ConnectReturn, ContextAndStringCall, ContextAndTwoStringCall, - ContextCall, ControlCall, ControlReturn, EstablishContextCall, EstablishContextReturn, GetAttribCall, - GetAttribReturn, GetDeviceTypeIdCall, GetDeviceTypeIdReturn, GetReaderIconCall, GetReaderIconReturn, + self, CardProtocol, CardState, CardStateFlags, ConnectCall, ConnectReturn, ContextAndStringCall, + ContextAndTwoStringCall, ContextCall, ControlCall, ControlReturn, EstablishContextCall, EstablishContextReturn, + GetAttribCall, GetAttribReturn, GetDeviceTypeIdCall, GetDeviceTypeIdReturn, GetReaderIconCall, GetReaderIconReturn, GetStatusChangeCall, GetStatusChangeReturn, GetTransmitCountCall, GetTransmitCountReturn, HCardAndDispositionCall, ListReaderGroupsCall, ListReadersCall, ListReadersReturn, LocateCardsAtrMask, LocateCardsByAtrCall, LocateCardsCall, LongReturn, ReadCacheCall, ReadCacheReturn, ReaderState, ReaderStateCommonCall, ReconnectCall, @@ -245,7 +245,9 @@ impl ScardSession { ScardCall::GetReaderIconCall(c) => Ok(vec![self.get_reader_icon(req, c)]), ScardCall::ContextAndStringCall(c) => Ok(vec![self.context_and_string(req, ioctl, c)]), ScardCall::ContextAndTwoStringCall(c) => Ok(vec![self.context_and_two_string(req, ioctl, c)]), - other => Ok(vec![complete_rpce(req, unsupported(ioctl, other))]), + ScardCall::Unsupported => Ok(vec![SvcMessage::from(RdpdrPdu::DeviceControlResponse( + ScardCall::Unsupported.error_response(req, ReturnCode::UnsupportedFeature), + ))]), } } @@ -1312,20 +1314,19 @@ fn buffer_too_small(req: DeviceControlRequest) -> SvcMessage { } fn complete_rpce(req: DeviceControlRequest, output: Box) -> SvcMessage { - let (status, buf) = if output.size() <= req.output_buffer_length as usize { - (NtStatus::SUCCESS, Some(output)) - } else { - (NtStatus::BUFFER_TOO_SMALL, None) - }; - SvcMessage::from(RdpdrPdu::DeviceControlResponse(DeviceControlResponse::new( - req, status, buf, - ))) + SvcMessage::from(RdpdrPdu::DeviceControlResponse(esc::complete_call(req, output))) } fn complete_long(req: DeviceControlRequest, code: ReturnCode) -> SvcMessage { complete_rpce(req, Box::new(LongReturn::new(code))) } fn complete_establish(req: DeviceControlRequest, code: ReturnCode, ctx: ScardContext) -> SvcMessage { + // MS-RDPESC 2.2.3: all fields besides ReturnCode are zero on failure. + let ctx = if code == ReturnCode::Success { + ctx + } else { + ScardContext::empty() + }; complete_rpce(req, Box::new(EstablishContextReturn::new(code, ctx))) } fn complete_gsc( @@ -1341,6 +1342,12 @@ fn complete_connect( handle: ScardHandle, proto: CardProtocol, ) -> SvcMessage { + // MS-RDPESC 2.2.3: all fields besides ReturnCode are zero on failure. + let handle = if code == ReturnCode::Success { + handle + } else { + ScardHandle::empty() + }; complete_rpce(req, Box::new(ConnectReturn::new(code, handle, proto))) } fn complete_reconnect(req: DeviceControlRequest, code: ReturnCode, proto: CardProtocol) -> SvcMessage { @@ -1719,45 +1726,6 @@ fn reader_states_from_native(states: Vec) -> Vec Box { - let code = ReturnCode::UnsupportedFeature; - let undef = CardProtocol::SCARD_PROTOCOL_UNDEFINED; - match call { - ScardCall::LocateCardsCall(_) | ScardCall::LocateCardsByAtrCall(_) => { - Box::new(GetStatusChangeReturn::new(code, Vec::new())) - } - ScardCall::ControlCall(_) => Box::new(ControlReturn::new(code, Vec::new())), - ScardCall::GetAttribCall(_) => Box::new(GetAttribReturn::new(code, Vec::new())), - ScardCall::GetTransmitCountCall(_) => Box::new(GetTransmitCountReturn::new(code, 0)), - ScardCall::GetDeviceTypeIdCall(_) => Box::new(GetDeviceTypeIdReturn::new(code, 0)), - ScardCall::ReadCacheCall(_) => Box::new(ReadCacheReturn::new(code, Vec::new())), - ScardCall::GetReaderIconCall(_) => Box::new(GetReaderIconReturn::new(code, Vec::new())), - ScardCall::EstablishContextCall(_) => Box::new(EstablishContextReturn::new(code, ScardContext::from_native(0))), - ScardCall::ListReaderGroupsCall(_) | ScardCall::ListReadersCall(_) => { - Box::new(ListReadersReturn::probe(code, 0, list_cs(ioctl))) - } - ScardCall::GetStatusChangeCall(_) => Box::new(GetStatusChangeReturn::new(code, Vec::new())), - ScardCall::ConnectCall(_) => Box::new(ConnectReturn::new( - code, - ScardHandle::from_native(ScardContext::from_native(0), 0), - undef, - )), - ScardCall::ReconnectCall(_) => Box::new(ReconnectReturn::new(code, undef)), - ScardCall::TransmitCall(_) => Box::new(TransmitReturn::new(code, None, Vec::new())), - ScardCall::StatusCall(_) => Box::new(StatusReturn::names_probe( - code, - 0, - CardState::Unknown, - undef, - [0u8; 32], - 0, - status_cs(ioctl), - )), - ScardCall::StateCall(_) => Box::new(StateReturn::atr_probe(code, CardState::Unknown, undef, 0)), - _ => Box::new(LongReturn::new(code)), - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/ironrdp-rdpdr/src/backend/noop.rs b/crates/ironrdp-rdpdr/src/backend/noop.rs index 308175dfc8..77eeac34ac 100644 --- a/crates/ironrdp-rdpdr/src/backend/noop.rs +++ b/crates/ironrdp-rdpdr/src/backend/noop.rs @@ -5,7 +5,7 @@ use ironrdp_svc::SvcMessage; use super::RdpdrBackend; use crate::pdu::RdpdrPdu; use crate::pdu::efs::{ - DeviceControlRequest, DeviceControlResponse, DeviceIoRequest, DeviceIoResponse, DeviceWriteResponse, NtStatus, + DeviceControlRequest, DeviceIoRequest, DeviceIoResponse, DeviceWriteResponse, NtStatus, ServerDeviceAnnounceResponse, }; use crate::pdu::esc::{ReturnCode, ScardCall, ScardIoCtlCode}; @@ -28,14 +28,7 @@ impl RdpdrBackend for NoopRdpdrBackend { req: DeviceControlRequest, call: ScardCall, ) -> PduResult> { - let output = call.error_return(req.io_control_code, ReturnCode::NoService); - let fits = u32::try_from(output.size()).is_ok_and(|size| size <= req.output_buffer_length); - let response = if fits { - DeviceControlResponse::new(req, NtStatus::SUCCESS, Some(output)) - } else { - // The server retries with a larger output buffer (MS-RDPESC 3.2.5.2). - DeviceControlResponse::new(req, NtStatus::BUFFER_TOO_SMALL, None) - }; + let response = call.error_response(req, ReturnCode::NoService); Ok(vec![SvcMessage::from(RdpdrPdu::DeviceControlResponse(response))]) } fn handle_drive_io_request(&mut self, _req: crate::pdu::efs::ServerDriveIoRequest) -> PduResult> { diff --git a/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs b/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs index e0e4645061..95704ff19c 100644 --- a/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs +++ b/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs @@ -15,7 +15,7 @@ use ironrdp_pdu::utils::{ }; use tracing::{error, warn}; -use super::efs::IoCtlCode; +use super::efs::{DeviceControlRequest, DeviceControlResponse, IoCtlCode, NtStatus}; use crate::pdu::esc::ndr::{Decode as _, Encode as _}; /// [2.2.2] TS Server-Generated Structures @@ -168,14 +168,27 @@ impl ScardCall { } } - /// Builds the IOCTL-specific `*_Return` structure for a call that failed with `return_code`. + /// Builds the completion for this call failing with `return_code`. /// /// Per [\[MS-RDPESC\]] section 3.1.4, smart card errors are carried in the `ReturnCode` of the - /// structure the server decodes for this IOCTL, while `IoStatus` stays `STATUS_SUCCESS`. All - /// other output fields are left empty. + /// structure the server decodes for this IOCTL, while `IoStatus` stays `STATUS_SUCCESS`; per + /// section 2.2.3, all other fields of that structure are zero. An IOCTL this crate does not + /// decode has no known return structure, so it is completed with `STATUS_UNSUCCESSFUL` and no + /// output instead. /// /// [\[MS-RDPESC\]]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpesc/0428ca28-b4dc-46a3-97c3-01887fa44a90 - pub fn error_return(&self, io_ctl_code: ScardIoCtlCode, return_code: ReturnCode) -> Box { + pub fn error_response( + &self, + req: DeviceControlRequest, + return_code: ReturnCode, + ) -> DeviceControlResponse { + match self.error_return(req.io_control_code, return_code) { + Some(output) => complete_call(req, output), + None => DeviceControlResponse::new(req, NtStatus::UNSUCCESSFUL, None), + } + } + + fn error_return(&self, io_ctl_code: ScardIoCtlCode, return_code: ReturnCode) -> Option> { let undefined = CardProtocol::SCARD_PROTOCOL_UNDEFINED; let charset = match io_ctl_code { ScardIoCtlCode::ListReadersA | ScardIoCtlCode::ListReaderGroupsA | ScardIoCtlCode::StatusA => { @@ -184,9 +197,9 @@ impl ScardCall { _ => CharacterSet::Unicode, }; - match self { + let output: Box = match self { ScardCall::EstablishContextCall(_) => { - Box::new(EstablishContextReturn::new(return_code, ScardContext::from_native(0))) + Box::new(EstablishContextReturn::new(return_code, ScardContext::empty())) } ScardCall::ListReaderGroupsCall(_) | ScardCall::ListReadersCall(_) => { Box::new(ListReadersReturn::probe(return_code, 0, charset)) @@ -195,11 +208,7 @@ impl ScardCall { ScardCall::GetStatusChangeCall(_) | ScardCall::LocateCardsCall(_) | ScardCall::LocateCardsByAtrCall(_) => { Box::new(GetStatusChangeReturn::new(return_code, Vec::new())) } - ScardCall::ConnectCall(_) => Box::new(ConnectReturn::new( - return_code, - ScardHandle::from_native(ScardContext::from_native(0), 0), - undefined, - )), + ScardCall::ConnectCall(_) => Box::new(ConnectReturn::new(return_code, ScardHandle::empty(), undefined)), ScardCall::ReconnectCall(_) => Box::new(ReconnectReturn::new(return_code, undefined)), ScardCall::TransmitCall(_) => Box::new(TransmitReturn::new(return_code, None, Vec::new())), ScardCall::StatusCall(_) => Box::new(StatusReturn::names_probe( @@ -224,9 +233,33 @@ impl ScardCall { | ScardCall::ContextCall(_) | ScardCall::WriteCacheCall(_) | ScardCall::ContextAndStringCall(_) - | ScardCall::ContextAndTwoStringCall(_) - | ScardCall::Unsupported => Box::new(LongReturn::new(return_code)), - } + | ScardCall::ContextAndTwoStringCall(_) => Box::new(LongReturn::new(return_code)), + // Decoded as unsupported, but MS-RDPESC lists it with a Long_Return. + ScardCall::Unsupported if io_ctl_code == ScardIoCtlCode::ReleaseTartedEvent => { + Box::new(LongReturn::new(return_code)) + } + ScardCall::Unsupported => return None, + }; + Some(output) + } +} + +/// Completes a smart card IOCTL with its encoded `*_Return` structure. +/// +/// When the structure does not fit the server's output buffer, the IOCTL is completed with +/// `STATUS_BUFFER_TOO_SMALL` and no output instead, so that the server retries with a larger +/// buffer ([\[MS-RDPESC\]] section 3.2.5.2). +/// +/// [\[MS-RDPESC\]]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpesc/0428ca28-b4dc-46a3-97c3-01887fa44a90 +pub fn complete_call( + req: DeviceControlRequest, + output: Box, +) -> DeviceControlResponse { + let fits = u32::try_from(output.size()).is_ok_and(|size| size <= req.output_buffer_length); + if fits { + DeviceControlResponse::new(req, NtStatus::SUCCESS, Some(output)) + } else { + DeviceControlResponse::new(req, NtStatus::BUFFER_TOO_SMALL, None) } } @@ -266,6 +299,11 @@ impl ScardContext { Self { len: 4, bytes } } + /// Creates an empty context (`cbContext` = 0, NULL `pbContext`). + pub fn empty() -> Self { + Self { len: 0, bytes: [0; 16] } + } + /// Creates a context from opaque wire bytes (`0..=16`). pub fn from_opaque(opaque: &[u8]) -> DecodeResult { let len = scard_wire_len(u32::try_from(opaque.len()).unwrap_or(u32::MAX))?; @@ -1542,6 +1580,15 @@ impl ScardHandle { Self { context, len: 4, bytes } } + /// Creates an empty handle in an empty context (`cbHandle` = 0, NULL `pbHandle`). + pub fn empty() -> Self { + Self { + context: ScardContext::empty(), + len: 0, + bytes: [0; 16], + } + } + /// Creates a handle from opaque wire bytes (`0..=16`). pub fn from_opaque(context: ScardContext, opaque: &[u8]) -> DecodeResult { let len = scard_wire_len(u32::try_from(opaque.len()).unwrap_or(u32::MAX))?; diff --git a/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs b/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs index 0285164488..c181f994e2 100644 --- a/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs +++ b/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs @@ -23,7 +23,7 @@ use ironrdp_rdpdr::pdu::efs::{ ServerDriveQueryVolumeInformationRequest, ServerDriveSetInformationRequest, ServerDriveSetSecurityRequest, SharedAccess, VERSION_MINOR_RDP51, VersionAndIdPdu, VersionAndIdPduKind, }; -use ironrdp_rdpdr::pdu::esc::{ScardCall, ScardIoCtlCode}; +use ironrdp_rdpdr::pdu::esc::{ReturnCode, ScardCall, ScardIoCtlCode}; use ironrdp_rdpdr::server::{NoopRdpdrServerBackend, RdpdrServer, RdpdrServerBackend}; use ironrdp_rdpdr::{NoopRdpdrBackend, Rdpdr, RdpdrBackend}; @@ -576,6 +576,30 @@ fn noop_backend_completes_smartcard_calls_with_no_service() { // OutputBuffer holds EstablishContext_Return after the 16-byte MS-RPCE headers. assert_eq!(read_u32_as_usize(&encoded[16..]), encoded.len() - 20); assert_eq!(read_u32(&encoded[36..]), SCARD_E_NO_SERVICE); + // MS-RDPESC 2.2.3: the context is zero (cbContext 0, NULL pbContext) on failure. + assert_eq!(&encoded[40..48], &[0; 8]); +} + +#[test] +fn smartcard_error_response_rejects_calls_without_return_structure() { + let req = DeviceControlRequest { + header: DeviceIoRequest { + device_id: 1, + file_id: 0, + completion_id: 0x300, + major_function: MajorFunction::DeviceControl, + minor_function: MinorFunction::from(0), + }, + output_buffer_length: 2048, + input_buffer_length: 0, + io_control_code: ScardIoCtlCode::GetStatusChangeW, + }; + + let response = ScardCall::Unsupported.error_response(req, ReturnCode::NoService); + + let encoded = encode_vec(&RdpdrPdu::DeviceControlResponse(response)).unwrap(); + assert_eq!(ntstatus_at(&encoded, 12), NtStatus::UNSUCCESSFUL); + assert_eq!(read_u32(&encoded[16..]), 0); // OutputBufferLength } #[test] From 161c6f855c4d9c7a939dfabaab90f7c20299bf05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 20 Oct 2026 20:48:09 -0400 Subject: [PATCH 4/7] fix(rdpdr): pass zeroed fields at smartcard failure sites The native backend's Connect failure after a WinSCard error still sent the negotiated protocol, which MS-RDPESC 2.2.3 requires to be zero. Replace the success guards in `complete_establish`/`complete_connect` with `establish_failed`/`connect_failed`, which every failure site now calls instead of building placeholder handles. Also: - ffi: keep the factory error as the cause instead of replacing it. - Avalonia example: report a read that fails after the session ended instead of leaving its exception unobserved. - testsuite: build smartcard requests with the shared device-control framing helper. Co-Authored-By: Claude Opus 5.5 --- .../src/windows/scard/mod.rs | 49 +++++-------------- crates/ironrdp-rdpdr/src/pdu/esc/mod.rs | 1 + .../ironrdp-testsuite-core/tests/rdpdr/mod.rs | 37 ++++++++------ ffi/src/connector/mod.rs | 6 ++- 4 files changed, 42 insertions(+), 51 deletions(-) diff --git a/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs b/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs index cd0ee2e719..74acb53be2 100644 --- a/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs +++ b/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs @@ -142,18 +142,13 @@ impl ScardSession { active_protocol, } => { if code != ReturnCode::Success { - return complete_connect(req, code, ScardHandle::from_native(context, 0), active_protocol); + return connect_failed(req, code); } let ctx = context.native(); if !self.contexts.contains_key(&ctx) { // SAFETY: orphan card after context vanished. let _ = unsafe { SCardDisconnect(native_handle, 0) }; - return complete_connect( - req, - ReturnCode::InvalidHandle, - ScardHandle::from_native(context, 0), - CardProtocol::SCARD_PROTOCOL_UNDEFINED, - ); + return connect_failed(req, ReturnCode::InvalidHandle); } self.cards.insert(native_handle, ctx); complete_connect( @@ -328,7 +323,7 @@ impl ScardSession { // SAFETY: WinSCard context allocation. let code = map_status(unsafe { SCardEstablishContext(scope, None, None, &mut native) }); if code != ReturnCode::Success { - return complete_establish(req, code, ScardContext::from_native(0)); + return establish_failed(req, code); } self.contexts.insert(native, ()); complete_establish(req, ReturnCode::Success, ScardContext::from_native(native)) @@ -423,14 +418,7 @@ impl ScardSession { } let native_ctx = match self.ctx(call.common.context) { Ok(n) => n, - Err(c) => { - return Ok(vec![complete_connect( - req, - c, - ScardHandle::from_native(call.common.context, 0), - CardProtocol::SCARD_PROTOCOL_UNDEFINED, - )]); - } + Err(c) => return Ok(vec![connect_failed(req, c)]), }; let context = call.common.context; self.defer( @@ -460,14 +448,7 @@ impl ScardSession { active_protocol: CardProtocol::from_bits_retain(active), } }, - move |req, c| { - complete_connect( - req, - c, - ScardHandle::from_native(context, 0), - CardProtocol::SCARD_PROTOCOL_UNDEFINED, - ) - }, + connect_failed, ) } @@ -1321,14 +1302,12 @@ fn complete_long(req: DeviceControlRequest, code: ReturnCode) -> complete_rpce(req, Box::new(LongReturn::new(code))) } fn complete_establish(req: DeviceControlRequest, code: ReturnCode, ctx: ScardContext) -> SvcMessage { - // MS-RDPESC 2.2.3: all fields besides ReturnCode are zero on failure. - let ctx = if code == ReturnCode::Success { - ctx - } else { - ScardContext::empty() - }; complete_rpce(req, Box::new(EstablishContextReturn::new(code, ctx))) } +// MS-RDPESC 2.2.3: all fields besides ReturnCode are zero on failure. +fn establish_failed(req: DeviceControlRequest, code: ReturnCode) -> SvcMessage { + complete_establish(req, code, ScardContext::empty()) +} fn complete_gsc( req: DeviceControlRequest, code: ReturnCode, @@ -1342,14 +1321,12 @@ fn complete_connect( handle: ScardHandle, proto: CardProtocol, ) -> SvcMessage { - // MS-RDPESC 2.2.3: all fields besides ReturnCode are zero on failure. - let handle = if code == ReturnCode::Success { - handle - } else { - ScardHandle::empty() - }; complete_rpce(req, Box::new(ConnectReturn::new(code, handle, proto))) } +// MS-RDPESC 2.2.3: all fields besides ReturnCode are zero on failure. +fn connect_failed(req: DeviceControlRequest, code: ReturnCode) -> SvcMessage { + complete_connect(req, code, ScardHandle::empty(), CardProtocol::SCARD_PROTOCOL_UNDEFINED) +} fn complete_reconnect(req: DeviceControlRequest, code: ReturnCode, proto: CardProtocol) -> SvcMessage { complete_rpce(req, Box::new(ReconnectReturn::new(code, proto))) } diff --git a/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs b/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs index 95704ff19c..414d348741 100644 --- a/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs +++ b/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs @@ -238,6 +238,7 @@ impl ScardCall { ScardCall::Unsupported if io_ctl_code == ScardIoCtlCode::ReleaseTartedEvent => { Box::new(LongReturn::new(return_code)) } + // Unreachable today: every other `ScardIoCtlCode` decodes to a known call. ScardCall::Unsupported => return None, }; Some(output) diff --git a/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs b/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs index c181f994e2..7038fbaab9 100644 --- a/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs +++ b/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs @@ -159,13 +159,26 @@ fn encoded_printer_device_control_request() -> Vec { } fn encoded_printer_device_control_request_with_input(input: &[u8]) -> Vec { - let mut encoded = encoded_printer_device_io_request(MajorFunction::DeviceControl); - encoded.extend_from_slice(&0u32.to_le_bytes()); // OutputBufferLength - encoded.extend_from_slice(&u32::try_from(input.len()).unwrap().to_le_bytes()); - encoded.extend_from_slice(&0u32.to_le_bytes()); // IoControlCode - encoded.extend_from_slice(&[0; 20]); // Padding - encoded.extend_from_slice(input); - encoded + encoded_device_control_request( + encoded_printer_device_io_request(MajorFunction::DeviceControl), + 0, + 0, + input, + ) +} + +fn encoded_device_control_request( + mut encoded_header: Vec, + output_buffer_length: u32, + io_control_code: u32, + input: &[u8], +) -> Vec { + encoded_header.extend_from_slice(&output_buffer_length.to_le_bytes()); + encoded_header.extend_from_slice(&u32::try_from(input.len()).unwrap().to_le_bytes()); + encoded_header.extend_from_slice(&io_control_code.to_le_bytes()); + encoded_header.extend_from_slice(&[0; 20]); // Padding + encoded_header.extend_from_slice(input); + encoded_header } fn encoded_printer_create_request(path_length: u32) -> Vec { @@ -535,7 +548,7 @@ fn smartcard_device_announce_remains_pre_logon() { } fn encoded_smartcard_establish_context_request(output_buffer_length: u32) -> Vec { - let mut encoded = encode_vec(&RdpdrPdu::DeviceIoRequest(DeviceIoRequest { + let header = encode_vec(&RdpdrPdu::DeviceIoRequest(DeviceIoRequest { device_id: 1, file_id: 0, completion_id: 0x200, @@ -550,12 +563,8 @@ fn encoded_smartcard_establish_context_request(output_buffer_length: u32) -> Vec [0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], // dwScope + alignment padding ] .concat(); - encoded.extend_from_slice(&output_buffer_length.to_le_bytes()); - encoded.extend_from_slice(&u32::try_from(input.len()).unwrap().to_le_bytes()); - encoded.extend_from_slice(&0x0009_0014u32.to_le_bytes()); // SCARD_IOCTL_ESTABLISHCONTEXT - encoded.extend_from_slice(&[0; 20]); // Padding - encoded.extend_from_slice(&input); - encoded + // SCARD_IOCTL_ESTABLISHCONTEXT + encoded_device_control_request(header, output_buffer_length, 0x0009_0014, &input) } #[test] diff --git a/ffi/src/connector/mod.rs b/ffi/src/connector/mod.rs index 0c0c03af53..528dedc717 100644 --- a/ffi/src/connector/mod.rs +++ b/ffi/src/connector/mod.rs @@ -73,7 +73,11 @@ pub mod ffi { #[cfg(windows)] let backend: Box = Box::new( ironrdp_rdpdr_native::WindowsRdpdrBackendFactory::from_drives(Vec::new()) - .map_err(|_| "invalid RDPDR backend configuration")? + .map_err(|error| { + crate::error::GenericError( + anyhow::Error::new(error).context("invalid RDPDR backend configuration"), + ) + })? .with_smartcard(true) .build(), ); From 4ecf802c9fded7d9b4067be0e43702fdbacecd22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 20 Oct 2026 21:45:41 -0400 Subject: [PATCH 5/7] fix(rdpdr): send NULL buffers in smartcard error returns MS-RDPESC 2.2.3 zeroes every field besides ReturnCode on failure, but several error returns still carried non-NULL NDR pointers: - The shared error path built Transmit, Control, GetAttrib and ReadCache returns with an empty buffer, which encodes a referent ID and a conformant count. It now uses the existing NULL-buffer probe constructors. - GetStatusChange_Return/LocateCards_Return and GetReaderIcon_Return always wrote a pointer. Like FreeRDP, they now write a zero count and a NULL pointer when empty, and drop their payload on failure (reader states are kept on SCARD_E_TIMEOUT, as FreeRDP does). - The native backend sent pioRecvPci on a failed Transmit, and could pass WinSCard out-parameters through on failed Reconnect, GetTransmitCount and GetDeviceTypeId. Also: - ffi: `ActiveStage.Process` no longer flushes deferred RDPDR frames after DeactivateAll (stale channel IDs) or Terminate. - .NET: `Framed.Write` used a thread-affine Mutex across an await, which throws when the continuation resumes on another thread; it now uses SemaphoreSlim. - Avalonia example: guard pointer press/release before the session is active. Co-Authored-By: Claude Opus 5.5 --- .../src/windows/scard/mod.rs | 11 ++ crates/ironrdp-rdpdr/src/pdu/esc/mod.rs | 73 ++++++++-- .../ironrdp-testsuite-core/tests/rdpdr/mod.rs | 127 ++++++++++++++++++ .../MainWindow.axaml.cs | 10 ++ ffi/src/session/mod.rs | 13 +- 5 files changed, 219 insertions(+), 15 deletions(-) diff --git a/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs b/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs index 74acb53be2..74f98df5a6 100644 --- a/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs +++ b/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs @@ -1327,7 +1327,14 @@ fn complete_connect( fn connect_failed(req: DeviceControlRequest, code: ReturnCode) -> SvcMessage { complete_connect(req, code, ScardHandle::empty(), CardProtocol::SCARD_PROTOCOL_UNDEFINED) } +// The helpers below take raw WinSCard results: MS-RDPESC 2.2.3 zeroes every field besides +// ReturnCode on failure, whatever WinSCard left in its out-parameters. fn complete_reconnect(req: DeviceControlRequest, code: ReturnCode, proto: CardProtocol) -> SvcMessage { + let proto = if code == ReturnCode::Success { + proto + } else { + CardProtocol::SCARD_PROTOCOL_UNDEFINED + }; complete_rpce(req, Box::new(ReconnectReturn::new(code, proto))) } @@ -1358,10 +1365,12 @@ fn attrib_ret( } fn complete_tx_count(req: DeviceControlRequest, code: ReturnCode, count: u32) -> SvcMessage { + let count = if code == ReturnCode::Success { count } else { 0 }; complete_rpce(req, Box::new(GetTransmitCountReturn::new(code, count))) } fn complete_device_type(req: DeviceControlRequest, code: ReturnCode, id: u32) -> SvcMessage { + let id = if code == ReturnCode::Success { id } else { 0 }; complete_rpce(req, Box::new(GetDeviceTypeIdReturn::new(code, id))) } @@ -1411,6 +1420,8 @@ fn xmit_ret( data: Option>, recv_len: u32, ) -> SvcMessage { + // A failed call returns no pioRecvPci; recv_len stays as the SCARD_E_INSUFFICIENT_BUFFER size hint. + let pci = pci.filter(|_| code == ReturnCode::Success); let pdu = match data { Some(buf) => TransmitReturn::new(code, pci, buf), None => TransmitReturn::recv_probe(code, pci, recv_len), diff --git a/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs b/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs index 414d348741..ff5549aa3d 100644 --- a/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs +++ b/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs @@ -210,7 +210,7 @@ impl ScardCall { } ScardCall::ConnectCall(_) => Box::new(ConnectReturn::new(return_code, ScardHandle::empty(), undefined)), ScardCall::ReconnectCall(_) => Box::new(ReconnectReturn::new(return_code, undefined)), - ScardCall::TransmitCall(_) => Box::new(TransmitReturn::new(return_code, None, Vec::new())), + ScardCall::TransmitCall(_) => Box::new(TransmitReturn::recv_probe(return_code, None, 0)), ScardCall::StatusCall(_) => Box::new(StatusReturn::names_probe( return_code, 0, @@ -221,11 +221,11 @@ impl ScardCall { charset, )), ScardCall::StateCall(_) => Box::new(StateReturn::atr_probe(return_code, CardState::Unknown, undefined, 0)), - ScardCall::ControlCall(_) => Box::new(ControlReturn::new(return_code, Vec::new())), - ScardCall::GetAttribCall(_) => Box::new(GetAttribReturn::new(return_code, Vec::new())), + ScardCall::ControlCall(_) => Box::new(ControlReturn::out_probe(return_code, 0)), + ScardCall::GetAttribCall(_) => Box::new(GetAttribReturn::attr_probe(return_code, 0)), ScardCall::GetTransmitCountCall(_) => Box::new(GetTransmitCountReturn::new(return_code, 0)), ScardCall::GetDeviceTypeIdCall(_) => Box::new(GetDeviceTypeIdReturn::new(return_code, 0)), - ScardCall::ReadCacheCall(_) => Box::new(ReadCacheReturn::new(return_code, Vec::new())), + ScardCall::ReadCacheCall(_) => Box::new(ReadCacheReturn::data_probe(return_code, 0)), ScardCall::GetReaderIconCall(_) => Box::new(GetReaderIconReturn::new(return_code, Vec::new())), ScardCall::AccessStartedEventCall(_) | ScardCall::HCardAndDispositionCall(_) @@ -1452,18 +1452,36 @@ impl GetStatusChangeReturn { reader_states, }) } + + /// Reader states put on the wire. + /// + /// MS-RDPESC 2.2.3 zeroes every field besides `ReturnCode` on failure. Like FreeRDP, the + /// states are kept on `SCARD_E_TIMEOUT`, which still reports the current reader states. + fn wire_reader_states(&self) -> &[ReaderStateCommonCall] { + if matches!(self.return_code, ReturnCode::Success | ReturnCode::Timeout) { + &self.reader_states + } else { + &[] + } + } } impl rpce::HeaderlessEncode for GetStatusChangeReturn { fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> { ensure_size!(in: dst, size: self.size()); dst.write_u32(self.return_code.into()); - let reader_states_len = - cast_length!("GetStatusChangeReturn", "reader_states", self.reader_states.len(), in: dst)?; + let reader_states = self.wire_reader_states(); + let reader_states_len = cast_length!("GetStatusChangeReturn", "reader_states", reader_states.len(), in: dst)?; + if reader_states.is_empty() { + // cReaders = 0 and a NULL rgReaderStates pointer. + dst.write_u32(0); + dst.write_u32(0); + return Ok(()); + } let mut index = 0; ndr::encode_ptr(Some(reader_states_len), &mut index, dst)?; dst.write_u32(reader_states_len); - for reader_state in &self.reader_states { + for reader_state in reader_states { reader_state.encode(dst)?; } Ok(()) @@ -1474,10 +1492,15 @@ impl rpce::HeaderlessEncode for GetStatusChangeReturn { } fn size(&self) -> usize { + let reader_states = self.wire_reader_states(); self.return_code.size() // dst.write_u32(self.return_code.into()); + ndr::ptr_size(true) // ndr::encode_ptr(Some(reader_states_len), &mut index, dst)?; - + 4 // dst.write_u32(reader_states_len); - + self.reader_states.iter().map(|_s| ReaderStateCommonCall::size()).sum::() + + if reader_states.is_empty() { + 0 + } else { + 4 // dst.write_u32(reader_states_len); + + reader_states.len() * ReaderStateCommonCall::size() + } } } @@ -2627,25 +2650,42 @@ impl GetReaderIconReturn { pub fn new(return_code: ReturnCode, data: Vec) -> rpce::Pdu { rpce::Pdu(Self { return_code, data }) } + + /// Icon bytes put on the wire: MS-RDPESC 2.2.3 zeroes every field besides `ReturnCode` on + /// failure. + fn wire_data(&self) -> &[u8] { + if self.return_code == ReturnCode::Success { + &self.data + } else { + &[] + } + } } impl rpce::HeaderlessEncode for GetReaderIconReturn { fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> { ensure_size!(in: dst, size: self.size()); dst.write_u32(self.return_code.into()); + let data = self.wire_data(); // MS-RDPESC: cbDataLen range(0,4194304) - if self.data.len() > Self::MAX_DATA_LEN { + if data.len() > Self::MAX_DATA_LEN { return Err(invalid_field_err!( "encode", "GetReaderIconReturn cbDataLen out of range", in: dst )); } - let data_len: u32 = cast_length!("GetReaderIconReturn", "data_len", self.data.len(), in: dst)?; + let data_len: u32 = cast_length!("GetReaderIconReturn", "data_len", data.len(), in: dst)?; + if data.is_empty() { + // cbDataLen = 0 and a NULL pbData pointer. + dst.write_u32(0); + dst.write_u32(0); + return Ok(()); + } let mut index = 0; ndr::encode_ptr(Some(data_len), &mut index, dst)?; dst.write_u32(data_len); - dst.write_slice(&self.data); + dst.write_slice(data); Ok(()) } @@ -2654,10 +2694,15 @@ impl rpce::HeaderlessEncode for GetReaderIconReturn { } fn size(&self) -> usize { + let data = self.wire_data(); size_of::() // dst.write_u32(self.return_code.into()); + ndr::ptr_size(true) // ndr::encode_ptr(Some(data_len), &mut index, dst)?; - + size_of::() // dst.write_u32(data_len); - + self.data.len() // dst.write_slice(&self.data); + + if data.is_empty() { + 0 + } else { + size_of::() // dst.write_u32(data_len); + + data.len() // dst.write_slice(data); + } } } diff --git a/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs b/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs index 7038fbaab9..5bdcff574c 100644 --- a/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs +++ b/crates/ironrdp-testsuite-core/tests/rdpdr/mod.rs @@ -611,6 +611,133 @@ fn smartcard_error_response_rejects_calls_without_return_structure() { assert_eq!(read_u32(&encoded[16..]), 0); // OutputBufferLength } +#[test] +fn smartcard_error_responses_zero_every_field_but_return_code() { + use ironrdp_rdpdr::pdu::esc::{ + CardProtocol, ConnectCall, ConnectCommon, ControlCall, GetAttribCall, GetReaderIconCall, GetStatusChangeCall, + ReadCacheCall, ReadCacheCommon, ReconnectCall, SCardIORequest, ScardContext, ScardHandle, TransmitCall, + }; + + const SCARD_E_NO_SERVICE: u32 = 0x8010_001D; + + let handle = ScardHandle::new(ScardContext::new(7), 9); + let calls = [ + ( + ScardIoCtlCode::ConnectW, + ScardCall::ConnectCall(ConnectCall { + reader: "Reader".to_owned(), + common: ConnectCommon { + context: ScardContext::new(7), + share_mode: 2, + preferred_protocols: CardProtocol::SCARD_PROTOCOL_T1, + }, + }), + ), + ( + ScardIoCtlCode::Reconnect, + ScardCall::ReconnectCall(ReconnectCall { + handle, + share_mode: 2, + preferred_protocols: CardProtocol::SCARD_PROTOCOL_T1, + initialization: 0, + }), + ), + ( + ScardIoCtlCode::Transmit, + ScardCall::TransmitCall(TransmitCall { + handle, + send_pci: SCardIORequest { + protocol: CardProtocol::SCARD_PROTOCOL_T1, + extra_bytes_length: 0, + extra_bytes: Vec::new(), + }, + send_length: 1, + send_buffer: vec![0], + recv_pci: None, + recv_buffer_is_null: false, + recv_length: 258, + }), + ), + ( + ScardIoCtlCode::Control, + ScardCall::ControlCall(ControlCall { + handle, + control_code: 1, + in_buffer: Vec::new(), + out_buffer_is_null: false, + out_buffer_size: 64, + }), + ), + ( + ScardIoCtlCode::GetAttrib, + ScardCall::GetAttribCall(GetAttribCall { + handle, + attr_id: 1, + attr_is_null: false, + attr_length: 64, + }), + ), + ( + ScardIoCtlCode::ReadCacheW, + ScardCall::ReadCacheCall(ReadCacheCall { + lookup_name: "Cache".to_owned(), + common: ReadCacheCommon { + context: ScardContext::new(7), + card_uuid: vec![0; 16], + freshness_counter: 0, + data_is_null: false, + data_len: 64, + }, + }), + ), + ( + ScardIoCtlCode::GetStatusChangeW, + ScardCall::GetStatusChangeCall(GetStatusChangeCall { + context: ScardContext::new(7), + timeout: 0, + states_ptr_length: 0, + states_ptr: 0, + states_length: 0, + states: Vec::new(), + }), + ), + ( + ScardIoCtlCode::GetReaderIcon, + ScardCall::GetReaderIconCall(GetReaderIconCall { + context: ScardContext::new(7), + reader_name: "Reader".to_owned(), + }), + ), + ]; + + for (io_control_code, call) in calls { + let req = DeviceControlRequest { + header: DeviceIoRequest { + device_id: 1, + file_id: 0, + completion_id: 0x400, + major_function: MajorFunction::DeviceControl, + minor_function: MinorFunction::from(0), + }, + output_buffer_length: 2048, + input_buffer_length: 0, + io_control_code, + }; + + let response = call.error_response(req, ReturnCode::NoService); + + let encoded = encode_vec(&RdpdrPdu::DeviceControlResponse(response)).unwrap(); + assert_eq!(ntstatus_at(&encoded, 12), NtStatus::SUCCESS, "{io_control_code:?}"); + // The return structure starts after the 16-byte MS-RPCE headers. + assert_eq!(read_u32(&encoded[36..]), SCARD_E_NO_SERVICE, "{io_control_code:?}"); + assert!( + encoded[40..].iter().all(|&byte| byte == 0), + "{io_control_code:?}: nonzero field in {:02x?}", + &encoded[40..] + ); + } +} + #[test] fn noop_backend_reports_small_smartcard_output_buffer() { let mut rdpdr = Rdpdr::new(Box::new(NoopRdpdrBackend), "IronRDP".to_owned()).with_smartcard(1); diff --git a/ffi/dotnet/Devolutions.IronRdp.AvaloniaExample/MainWindow.axaml.cs b/ffi/dotnet/Devolutions.IronRdp.AvaloniaExample/MainWindow.axaml.cs index 44808f3af5..f93a2c09a7 100644 --- a/ffi/dotnet/Devolutions.IronRdp.AvaloniaExample/MainWindow.axaml.cs +++ b/ffi/dotnet/Devolutions.IronRdp.AvaloniaExample/MainWindow.axaml.cs @@ -407,6 +407,11 @@ private static Config BuildConfig(string username, string password, string? doma private void OnPointerPressed(object sender, PointerPressedEventArgs e) { + if (_activeStage == null || _decodedImage == null) + { + return; + } + PointerUpdateKind mouseButton = e.GetCurrentPoint((Visual?)sender).Properties.PointerUpdateKind; MouseButtonType buttonType = mouseButton switch @@ -447,6 +452,11 @@ private void OnPointerMoved(object sender, PointerEventArgs e) private void OnPointerReleased(object sender, PointerReleasedEventArgs e) { + if (_activeStage == null || _decodedImage == null) + { + return; + } + PointerUpdateKind mouseButton = e.GetCurrentPoint((Visual?)sender).Properties.PointerUpdateKind; MouseButtonType buttonType = mouseButton switch diff --git a/ffi/src/session/mod.rs b/ffi/src/session/mod.rs index 0c0697019b..d50f1516b6 100644 --- a/ffi/src/session/mod.rs +++ b/ffi/src/session/mod.rs @@ -134,7 +134,18 @@ pub mod ffi { let mut outputs = self .0 .process(&mut image.0, action.0, payload, Some(crate::utils::monotonic_now()))?; - outputs.extend(self.poll_rdpdr_deferred_output()?); + // After DeactivateAll the frame would carry pre-reactivation channel IDs; after + // Terminate there is no session left. The next poll flushes them instead. + let session_continues = !outputs.iter().any(|output| { + matches!( + output, + ironrdp::session::ActiveStageOutput::DeactivateAll + | ironrdp::session::ActiveStageOutput::Terminate(_) + ) + }); + if session_continues { + outputs.extend(self.poll_rdpdr_deferred_output()?); + } Ok(Box::new(ActiveStageOutputIterator(outputs.into()))) } From 547ab8d8c57ab99a0f68f9d0de71ec6de4ba1dce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 20 Oct 2026 21:55:39 -0400 Subject: [PATCH 6/7] refactor(ffi): align the Avalonia example with #2161 and #2162 Drop the `Framed.Write` lock fix, which #2161 makes with a test, and take the Avalonia example's ActiveStage lock as #2162 writes it (`_stageLock`, `WithStageLock`, `SendFastpath`). On top of #2162 this PR only adds the RDPDR poll in the read loop, the read-ordering fix and a guard in `SendFastpath`. Co-Authored-By: Claude Opus 5.5 --- .../MainWindow.axaml.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/ffi/dotnet/Devolutions.IronRdp.AvaloniaExample/MainWindow.axaml.cs b/ffi/dotnet/Devolutions.IronRdp.AvaloniaExample/MainWindow.axaml.cs index f93a2c09a7..44808f3af5 100644 --- a/ffi/dotnet/Devolutions.IronRdp.AvaloniaExample/MainWindow.axaml.cs +++ b/ffi/dotnet/Devolutions.IronRdp.AvaloniaExample/MainWindow.axaml.cs @@ -407,11 +407,6 @@ private static Config BuildConfig(string username, string password, string? doma private void OnPointerPressed(object sender, PointerPressedEventArgs e) { - if (_activeStage == null || _decodedImage == null) - { - return; - } - PointerUpdateKind mouseButton = e.GetCurrentPoint((Visual?)sender).Properties.PointerUpdateKind; MouseButtonType buttonType = mouseButton switch @@ -452,11 +447,6 @@ private void OnPointerMoved(object sender, PointerEventArgs e) private void OnPointerReleased(object sender, PointerReleasedEventArgs e) { - if (_activeStage == null || _decodedImage == null) - { - return; - } - PointerUpdateKind mouseButton = e.GetCurrentPoint((Visual?)sender).Properties.PointerUpdateKind; MouseButtonType buttonType = mouseButton switch From 8786bd9a5be416ef4ee4a09ce8cc07ad0f3717d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 20 Oct 2026 22:37:31 -0400 Subject: [PATCH 7/7] refactor(rdpdr): simplify smartcard error helpers Fold the private `error_return` into `ScardCall::error_response`, inline the single-use `establish_failed`, and have the FFI's deferred RDPDR poll helper return a `Vec` instead of an `Option` used as a collection. No behavior change. Co-Authored-By: Claude Opus 5.5 --- .../src/windows/scard/mod.rs | 7 +-- crates/ironrdp-rdpdr/src/pdu/esc/mod.rs | 51 ++++++++++--------- ffi/src/session/mod.rs | 10 ++-- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs b/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs index 74f98df5a6..16a687bbf0 100644 --- a/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs +++ b/crates/ironrdp-rdpdr-native/src/windows/scard/mod.rs @@ -323,7 +323,8 @@ impl ScardSession { // SAFETY: WinSCard context allocation. let code = map_status(unsafe { SCardEstablishContext(scope, None, None, &mut native) }); if code != ReturnCode::Success { - return establish_failed(req, code); + // MS-RDPESC 2.2.3: all fields besides ReturnCode are zero on failure. + return complete_establish(req, code, ScardContext::empty()); } self.contexts.insert(native, ()); complete_establish(req, ReturnCode::Success, ScardContext::from_native(native)) @@ -1304,10 +1305,6 @@ fn complete_long(req: DeviceControlRequest, code: ReturnCode) -> fn complete_establish(req: DeviceControlRequest, code: ReturnCode, ctx: ScardContext) -> SvcMessage { complete_rpce(req, Box::new(EstablishContextReturn::new(code, ctx))) } -// MS-RDPESC 2.2.3: all fields besides ReturnCode are zero on failure. -fn establish_failed(req: DeviceControlRequest, code: ReturnCode) -> SvcMessage { - complete_establish(req, code, ScardContext::empty()) -} fn complete_gsc( req: DeviceControlRequest, code: ReturnCode, diff --git a/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs b/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs index ff5549aa3d..66f73df6ba 100644 --- a/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs +++ b/crates/ironrdp-rdpdr/src/pdu/esc/mod.rs @@ -182,26 +182,22 @@ impl ScardCall { req: DeviceControlRequest, return_code: ReturnCode, ) -> DeviceControlResponse { - match self.error_return(req.io_control_code, return_code) { - Some(output) => complete_call(req, output), - None => DeviceControlResponse::new(req, NtStatus::UNSUCCESSFUL, None), - } - } - - fn error_return(&self, io_ctl_code: ScardIoCtlCode, return_code: ReturnCode) -> Option> { + let io_ctl_code = req.io_control_code; let undefined = CardProtocol::SCARD_PROTOCOL_UNDEFINED; - let charset = match io_ctl_code { - ScardIoCtlCode::ListReadersA | ScardIoCtlCode::ListReaderGroupsA | ScardIoCtlCode::StatusA => { - CharacterSet::Ansi - } - _ => CharacterSet::Unicode, - }; let output: Box = match self { ScardCall::EstablishContextCall(_) => { Box::new(EstablishContextReturn::new(return_code, ScardContext::empty())) } ScardCall::ListReaderGroupsCall(_) | ScardCall::ListReadersCall(_) => { + let charset = if matches!( + io_ctl_code, + ScardIoCtlCode::ListReadersA | ScardIoCtlCode::ListReaderGroupsA + ) { + CharacterSet::Ansi + } else { + CharacterSet::Unicode + }; Box::new(ListReadersReturn::probe(return_code, 0, charset)) } // LocateCards_Return has the same layout as GetStatusChange_Return. @@ -211,15 +207,22 @@ impl ScardCall { ScardCall::ConnectCall(_) => Box::new(ConnectReturn::new(return_code, ScardHandle::empty(), undefined)), ScardCall::ReconnectCall(_) => Box::new(ReconnectReturn::new(return_code, undefined)), ScardCall::TransmitCall(_) => Box::new(TransmitReturn::recv_probe(return_code, None, 0)), - ScardCall::StatusCall(_) => Box::new(StatusReturn::names_probe( - return_code, - 0, - CardState::Unknown, - undefined, - [0; 32], - 0, - charset, - )), + ScardCall::StatusCall(_) => { + let charset = if io_ctl_code == ScardIoCtlCode::StatusA { + CharacterSet::Ansi + } else { + CharacterSet::Unicode + }; + Box::new(StatusReturn::names_probe( + return_code, + 0, + CardState::Unknown, + undefined, + [0; 32], + 0, + charset, + )) + } ScardCall::StateCall(_) => Box::new(StateReturn::atr_probe(return_code, CardState::Unknown, undefined, 0)), ScardCall::ControlCall(_) => Box::new(ControlReturn::out_probe(return_code, 0)), ScardCall::GetAttribCall(_) => Box::new(GetAttribReturn::attr_probe(return_code, 0)), @@ -239,9 +242,9 @@ impl ScardCall { Box::new(LongReturn::new(return_code)) } // Unreachable today: every other `ScardIoCtlCode` decodes to a known call. - ScardCall::Unsupported => return None, + ScardCall::Unsupported => return DeviceControlResponse::new(req, NtStatus::UNSUCCESSFUL, None), }; - Some(output) + complete_call(req, output) } } diff --git a/ffi/src/session/mod.rs b/ffi/src/session/mod.rs index d50f1516b6..89ccdf035c 100644 --- a/ffi/src/session/mod.rs +++ b/ffi/src/session/mod.rs @@ -156,7 +156,7 @@ pub mod ffi { /// not call it concurrently with other methods of this `ActiveStage`. pub fn poll_rdpdr_deferred(&mut self) -> Result, Box> { let outputs = self.poll_rdpdr_deferred_output()?; - Ok(Box::new(ActiveStageOutputIterator(outputs.into_iter().collect()))) + Ok(Box::new(ActiveStageOutputIterator(outputs.into()))) } pub fn process_fastpath_input( @@ -327,20 +327,20 @@ pub mod ffi { fn poll_rdpdr_deferred_output( &mut self, - ) -> Result, Box> { + ) -> Result, Box> { let Some(rdpdr) = self.0.get_svc_processor_mut::() else { - return Ok(None); + return Ok(Vec::new()); }; let messages = rdpdr.poll_deferred_messages()?; if messages.is_empty() { - return Ok(None); + return Ok(Vec::new()); } let frame = self .0 .process_svc_messages_by_name(&ironrdp::rdpdr::Rdpdr::NAME, messages)?; - Ok(Some(ironrdp::session::ActiveStageOutput::ResponseFrame(frame))) + Ok(vec![ironrdp::session::ActiveStageOutput::ResponseFrame(frame)]) } }