Repository navigation
Credential hand-over to sub-servers (Wrdp/canonical stack) - #1409
Rui Carmo (rcarmo) wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enables IronRDP-based servers to own the RDP handshake and credential collection while handing off the authenticated session to a separate per-user process (the "wrdp"/xrdp-sesman multi-user model). It threads CredSSP-delegated credentials into AcceptorResult::credentials, adds a same-TCP-connection credential cache so reactivation can reuse a validated identity, and introduces a ConnectionBinder hook that swaps in per-user display/input handlers after credentials are available.
Changes:
- Capture CredSSP/Hybrid delegated
TSPasswordCredsfrom the CredSSP server state machine and expose them viaAcceptorResult::credentials(previously TLSClientInfoPduonly). - Add
ConnectionBinder/BoundConnectionserver API (builder + setter + re-exports) that replaces display/input handlers post-auth. - Centralize credential resolution in
resolve_authenticated_credentials, backed by anaccept_finalize()-scoped cache for reactivation, plus unit tests anddocs/wrdp/notes.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
crates/ironrdp-acceptor/src/credssp.rs |
handle_process_result now returns the delegated AuthIdentity alongside Written. |
crates/ironrdp-acceptor/src/lib.rs |
Stores CredSSP-delegated credentials into the acceptor during the CredSSP loop. |
crates/ironrdp-acceptor/src/connection.rs |
Adds set_received_credssp_credentials and updates AcceptorResult::credentials docs to cover CredSSP. |
crates/ironrdp-server/src/server.rs |
Adds ConnectionBinder/BoundConnection, binder invocation in client_accepted, reactivation cache + resolve_authenticated_credentials, and tests. |
crates/ironrdp-server/src/builder.rs |
Wires connection_binder through the builder and build(). |
crates/ironrdp-server/src/lib.rs |
Re-exports BoundConnection/ConnectionBinder. |
docs/wrdp/*.md |
New design notes for auth delegation, CredSSP handoff, and reactivation cache scope. |
💡 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: client_accepted now catches credential-resolution failures, sends ServerSetErrorInfoPdu(ServerDeniedConnection) via send_access_denied(...), then returns the original error.
|
|
||
| /// Set or clear a post-auth connection binder. | ||
| /// | ||
| /// When set, the binder is called after credentials have been validated. |
There was a problem hiding this comment.
Mitigated on the canonical rcarmo/IronRDP:wrdp branch in b51bd0a68fb26bfc179571f59ea137eccc850fce and retained at 3b05f6fc70d91c2529df8454ce73a292c05195e6: the setter docs now say the binder runs when authenticated credentials are available, not strictly only after a configured validator accepts them.
|
Closing in favor of #1413, which my agent put together based on Copilot feedback |
…servers (#1413) ## Summary This replaces and consolidates #1409, #1410, #1411, and #1412 into a single branch from `rcarmo:wrdp`. The split PR heads use `wrdp/*` branch names, but the fork now uses the canonical `wrdp` branch. Because Git cannot keep both `refs/heads/wrdp` and `refs/heads/wrdp/*` at the same time, this PR carries the updated commits and review fixes together. ## Why `wrdp` follows the `xrdp-sesman` multi-user architecture model, so it needs a way to hand authenticated credentials from the protocol handshake into the per-user session binder. ## Changes - Adds an async post-auth connection binder hook in `ironrdp-server`. - Scopes reactivation credential reuse to the current TCP connection. - Exposes CredSSP delegated credentials to server consumers via the same `Credentials` shape used by ClientInfo. - Restores the client-facing denial PDU on credential reject/backend-error paths before returning the error. - Clarifies binder docs so they refer to authenticated credentials being available, not only to configured validator acceptance. - Skips binder re-execution during deactivation/reactivation so resize/reactivation does not restart per-user binding or replace handlers mid-session. ## Copilot review follow-up This includes fixes for the Copilot comments from the superseded PRs: - #1409: denial PDU regression and binder doc wording. - #1411: binder should not run again during reactivation. - #1412: denial PDU regression and reactivation rebinding. - #1410 had no inline Copilot review comments. ## Validation - `cargo check -p ironrdp-server -p ironrdp-acceptor` - `git diff --check` --------- Co-authored-by: Rui Carmo <rcarmo@users.noreply.github.com> Co-authored-by: Rui Carmo <rcarmo@rcarmo.com> Co-authored-by: Rui Carmo <rui.carmo@gmail.com> Co-authored-by: Marc-André Moreau <mamoreau@devolutions.net> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Hello!
I am creating a “wrdp” server that follows the xrdp-sesman model, and broke off this set of commits (along with a little note you can discard) that allows IronRDP to effectively own credential handling while still enabling session hand-off to a process that will then handle the actual user session setup - without that process needing to reinvent the wheel.