Skip to content

fix(rdpeudp): harden TLS sideband setup - #1811

Open
Marc-André Moreau (mamoreau-devolutions) wants to merge 2 commits into
masterfrom
copilot/rdpeudp-tls-hardening
Open

fix(rdpeudp): harden TLS sideband setup#1811
Marc-André Moreau (mamoreau-devolutions) wants to merge 2 commits into
masterfrom
copilot/rdpeudp-tls-hardening

Conversation

@mamoreau-devolutions

Copy link
Copy Markdown
Contributor

Extract the reusable rustls verifier/client-config builder out of ironrdp-tls into a new rustls_verifier module (gated behind a rustls-verifier feature) so the RDPEUDP2 reliable-UDP TLS sideband can share the same certificate validation policy and callback semantics as the primary TCP transport, without pulling in a specific TLS stream backend.

ironrdp-rdpeudp-tokio now depends on ironrdp-tls (rustls-verifier only) and gains a UdpTlsConfig type covering certificate validation mode, callback, and endpoint. The synchronous validation callback runs on a blocking-pool thread with a nested current-thread runtime so it cannot starve the current-thread RDPEUDP driver. TLS handshake and RDPEMT tunnel establishment are now wrapped in bounded timeouts (tls_timeout, tunnel_timeout) with dedicated error kinds.

The Driver gains a Drop impl that closes SharedIo and wakes parked readers when the stream was not already released, so aborting or dropping a connection attempt cannot strand a blocking-pool thread or leave detached TLS work parked forever.

MultitransportBootstrap::connect now takes the TLS config directly instead of an optional raw verifier, and its docs clarify that S_OK is only sent once Soft-Sync has been negotiated.

This is an independent split extracted from draft #1777, targeting master directly (no stacking). It is the second of the split PRs from that source.

Scope

Included:

  • crates/ironrdp-tls/{Cargo.toml,src/lib.rs,src/rustls.rs} + new src/rustls_verifier.rs
  • crates/ironrdp-rdpeudp-tokio/{Cargo.toml,README.md,src/lib.rs,src/multitransport.rs,src/tls.rs,src/transport.rs,src/error.rs,src/driver.rs}
  • crates/ironrdp-testsuite-extra/tests/rdpeudp_tokio.rs (RDPEUDP-specific test updates only)
  • Cargo.lock regenerated to add the single new ironrdp-rdpeudp-tokio -> ironrdp-tls dependency edge

Deliberately excluded (client/connector-dependent, out of scope for this slice):

  • crates/ironrdp-testsuite-extra/Cargo.toml — the source PR's "udp" feature addition applies to the ironrdp-client dev-dependency, which doesn't exist without the client-side UDP wiring landed in a separate PR
  • crates/ironrdp-testsuite-extra/tests/client/config.rs — the reliable_udp_is_opt_in test depends on ironrdp-connector::ConfigBuilder::with_udp_transport, out of scope for this slice

Validation

  • cargo build -p ironrdp-tls -p ironrdp-rdpeudp-tokio
  • cargo check --workspace --all-targets (confirms no other crate references the removed APIs)
  • cargo test -p ironrdp-testsuite-extra --test integration_tests_extra rdpeudp_tokio:: — 12/12 passed
  • cargo test -p ironrdp-rdpeudp-tokio --features rustls-ring — 51/51 passed
  • cargo check -p ironrdp-tls across rustls-verifier, stub, native-tls, rustls-no-provider feature combos
  • cargo xtask check fmt -v, cargo xtask check lints -v, cargo xtask check locks -v, cargo xtask check typos -v — all passing

Extract the reusable rustls verifier/client-config builder out of
ironrdp-tls into a new rustls_verifier module (gated behind a
rustls-verifier feature) so the RDPEUDP2 reliable-UDP TLS sideband can
share the same certificate validation policy and callback semantics as
the primary TCP transport, without pulling in a specific TLS stream
backend.

ironrdp-rdpeudp-tokio now depends on ironrdp-tls (rustls-verifier only)
and gains a UdpTlsConfig type covering certificate validation mode,
callback, and endpoint. The synchronous validation callback runs on a
blocking-pool thread with a nested current-thread runtime so it cannot
starve the current-thread RDPEUDP driver. TLS handshake and RDPEMT
tunnel establishment are now wrapped in bounded timeouts
(tls_timeout, tunnel_timeout) with dedicated error kinds.

The Driver gains a Drop impl that closes SharedIo and wakes parked
readers when the stream was not already released, so aborting or
dropping a connection attempt cannot strand a blocking-pool thread or
leave detached TLS work parked forever.

MultitransportBootstrap::connect now takes the TLS config directly
instead of an optional raw verifier, and its docs clarify that S_OK is
only sent once Soft-Sync has been negotiated.
Copilot AI balanced review requested due to automatic review settings August 27, 2026 15:44
@github-actions github-actions Bot added maintainer-required Maintainer review or intervention is required risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny size/XL Size: up to 1299 counted lines and 49 files; exceeds L in either measure labels Aug 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Hardens RDPEUDP2 TLS sideband setup by sharing certificate validation with the primary transport, bounding connection phases, and improving cancellation cleanup.

Changes:

  • Extracts a reusable rustls verifier/configuration feature.
  • Adds UDP TLS configuration, callback isolation, and handshake timeouts.
  • Ensures dropped drivers wake blocked I/O and expands integration coverage.

Dedicated prose reviewers were skipped due to the five-skill limit; documentation was reviewed directly.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Cargo.lock Records the new TLS dependency edge.
crates/ironrdp-tls/Cargo.toml Adds the verifier-only feature.
crates/ironrdp-tls/src/lib.rs Exports the shared rustls builder.
crates/ironrdp-tls/src/rustls.rs Reuses shared TLS configuration.
crates/ironrdp-tls/src/rustls_verifier.rs Implements certificate verifier construction.
crates/ironrdp-rdpeudp-tokio/Cargo.toml Adds verifier-only TLS dependency.
crates/ironrdp-rdpeudp-tokio/README.md Documents supported transport and strict validation.
crates/ironrdp-rdpeudp-tokio/src/driver.rs Closes shared I/O when dropped.
crates/ironrdp-rdpeudp-tokio/src/error.rs Adds TLS and tunnel timeout errors.
crates/ironrdp-rdpeudp-tokio/src/lib.rs Exports UdpTlsConfig.
crates/ironrdp-rdpeudp-tokio/src/multitransport.rs Forwards sideband TLS policy.
crates/ironrdp-rdpeudp-tokio/src/tls.rs Runs callback handshakes off-runtime.
crates/ironrdp-rdpeudp-tokio/src/transport.rs Applies TLS and tunnel timeouts.
crates/ironrdp-testsuite-extra/tests/rdpeudp_tokio.rs Tests callbacks, current-thread operation, and cleanup.

Comment thread crates/ironrdp-rdpeudp-tokio/src/transport.rs Outdated
Comment thread crates/ironrdp-tls/src/rustls_verifier.rs
Extend the default TLS sideband budget beyond the 120-second
interactive certificate callback window and retain time for handshake
overhead.

Document that the reusable rustls config builder requires an installed
or unambiguous process-level crypto provider.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer-required Maintainer review or intervention is required risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny size/XL Size: up to 1299 counted lines and 49 files; exceeds L in either measure

Development

Successfully merging this pull request may close these issues.

2 participants