Wrdp/reactivation credential cache#1412
Conversation
There was a problem hiding this comment.
Pull request overview
This PR is the final piece of the #1409/#1410/#1411 "wrdp" credential-handover series. It adds a per-connection credential cache so that deactivation-reactivation flows (where some clients omit a second credentials PDU) can reuse the identity validated earlier on the same TCP connection, and it introduces a post-auth ConnectionBinder hook that lets multi-user servers swap in per-session display/input handlers after credentials are accepted.
Changes:
- Add
resolve_authenticated_credentials, backed by a cache scoped toaccept_finalize()(one TCP connection), to reuse validated credentials across same-connection reactivations while preventing cross-connection inheritance. - Add a
ConnectionBindertrait /BoundConnectiontype plusRdpServer::set_connection_binderand builderwith_connection_binder, running the binder inclient_acceptedafter credential resolution. - Add two docs describing cache scope and post-auth binding, and unit tests for the cache behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/ironrdp-server/src/server.rs | Adds ConnectionBinder/BoundConnection, the resolve_authenticated_credentials cache helper, binder execution in client_accepted, and unit tests. |
| crates/ironrdp-server/src/builder.rs | Threads connection_binder through the builder state and applies it in build(). |
| crates/ironrdp-server/src/lib.rs | Exports BoundConnection and ConnectionBinder. |
| docs/wrdp/reactivation-credential-cache.md | Documents the per-connection cache scope rationale. |
| docs/wrdp/auth-delegation.md | Documents the post-auth connection binding model. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let authenticated_credentials = resolve_authenticated_credentials( | ||
| self.credential_validator.clone(), | ||
| result.credentials.as_ref(), | ||
| result.reactivation, | ||
| authenticated_credentials_cache, | ||
| ) | ||
| .await?; |
There was a problem hiding this comment.
Fixed on the canonical rcarmo/IronRDP:wrdp branch in b51bd0a68fb26bfc179571f59ea137eccc850fce and retained at 3b05f6fc70d91c2529df8454ce73a292c05195e6: credential rejection/backend-error paths now send the denial PDU before returning the error.
| if let Some(binder) = self.connection_binder.clone() { | ||
| let Some(credentials) = authenticated_credentials.as_ref() else { | ||
| warn!("Connection binder configured but no authenticated credentials are available"); | ||
| send_access_denied(result.io_channel_id, result.user_channel_id, writer).await?; | ||
| bail!("no authenticated credentials for connection binding"); | ||
| }; | ||
|
|
||
| let bound = binder | ||
| .bind_connection(credentials) | ||
| .await | ||
| .context("connection binder failed")?; | ||
| *self.display.lock().await = bound.display; | ||
| *self.handler.lock().await = bound.input; | ||
| debug!("Connection binder installed display/input handlers"); | ||
| } |
There was a problem hiding this comment.
Fixed on the canonical rcarmo/IronRDP:wrdp branch in 3b05f6fc70d91c2529df8454ce73a292c05195e6: the binder block is gated on !result.reactivation, avoiding repeated per-user session lookup/setup and handler replacement during resize/reactivation.
|
Closing in favor of #1413, which my agent put together based on Copilot feedback |
This is the final piece in the #1409 #1410 #1411 series - it is effectively an in-session cache to cope with Windows App’s foibles when negotiating credentials, and only holds on to the credentials during session setup.