Skip to content

chore(deps): bump IronRDP pin a5d1c682 -> e258f6a0 - #194

Draft
clintcan wants to merge 22 commits into
mainfrom
chore/pin-bump-e258f6a0
Draft

clintcan wants to merge 22 commits into
mainfrom
chore/pin-bump-e258f6a0

Conversation

@clintcan

@clintcan clintcan commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Moves the IronRDP pin from a5d1c682 to e258f6a0 (391 upstream commits; no crate versions changed, only the git revs).

Draft: the code is done and passes locally on macOS. Still to do: live client tests, the soak, then v0.10.0.

What changed

  • The vendored forks are now upstream e258f6a0 verbatim, plus macrdp's own files, plus small hook edits marked (macrdp ...). Each fork's CLAUDE.md lists what is left.
    • ironrdp-dvc: one addition (request_soft_sync).
    • ironrdp-acceptor: the client fingerprint, and the requested protocol for the multitransport offer.
    • ironrdp-rdpdr: the smart-card (server-direction) side only. The drive PDUs are upstream now.
    • ironrdp-server: the old divergence log is frozen in DIVERGENCE-HISTORY.md, and the multitransport code moved to src/mt/.
  • Retired now that upstream has them: SuppressOutput, NSCodec, honor-client-size, the keyboard-layout cell, the auto-reconnect cookie, mouse position before button (#1769), and auth-gated preemption. macrdp now sets these through the builder, including ConnectionPolicy::Preempt. Upstream's default is Queue, which would silently bring back the second-client hang.
  • Renamed to sit next to upstream's own versions: vendor/ironrdp-rdpeudp is now vendor/macrdp-rdpeudp, and the drive types are now RdpdrDrive*. Upstream's RDPDR, mic, USB and UDP paths are left unset, so they stay inactive.
  • macrdp src/:
    • src/conn_hooks.rs composes the connection handlers. It replaces lock_activity's wrapper, which would have swallowed upstream's new on_connection_info.
    • Mouse buttons now carry their position.
    • The display traits return ServerResult.
    • x509-cert declares its std feature, which it had only been getting indirectly.

Behaviour changes to live-test

  • Auto-reconnect cookie: upstream HMAC-verifies a returning cookie, so a client reconnecting after a macrdp process restart is expected to be refused. The cookie is also rotated on every reactivation, which includes blank-recovery heals.
  • SuppressOutput: upstream advertises support for it, so FreeRDP and Thincast now get the minimize mute.
  • UDP offer: now goes only to clients that open the MCS message channel.
  • USB: upstream decodes URBDRC completions more strictly.

Verified locally (macOS)

  • Formatting (stable and nightly), clippy -D warnings, cargo check --all-targets, and a release build.
  • 266 tests, also on 1.95.0 with -D warnings.
  • The audit-log FreeRDP integration test, on a fresh release build.
  • cargo-deny for the root crate and the standalone crate.
  • Every vendored crate builds standalone, including the server with --all-features. macrdp-rdpeudp passes 53 tests and its fuzz harness builds.

Not verifiable here: the Linux build. That's what this PR's CI is for.

Remaining before merge

Live tests with mstsc, FreeRDP, Windows App and iOS (list in TODO.md), then a 48–72 h soak including a headless mode, then v0.10.0. This also supersedes #182.

3-way merge conflict counts per fork (139 hunks), the build cascade (dvc fork
must go; acceptor/rdpdr API changes; 37 vendored-server errors before src/),
the multitransport module and ironrdp-rdpeudp name clashes, and the order of
work.
391 upstream commits; no crate version changed, so only the git revs move.
The branch does not build until the vendored forks are rebased onto the new
rev (dvc, acceptor, rdpdr, server — see the dry-run map in TODO.md).
The fork's Soft-Sync PDU codec (divergence 1) is retired: upstream #1584 has
its own (SoftSyncTunnelType, SoftSyncRequestPdu::new, tunnels_to_switch), with
the same flag rule. src/ is now upstream e258f6a0 verbatim plus one pure
addition, DrdynvcServer::request_soft_sync(tunnel_type, channel_ids):

- upstream's state machine validates the client's response against a request
  (an unsolicited/unrequested-tunnel response is an error that ends the
  session), but its only request API is request_reliable_udp, which is
  reliable-only, needs a channel, and registers tunnel routing that makes
  process() reject the channel's data over TCP;
- request_soft_sync records the request (so the response validates) for the
  lossy tunnel or with no channels, and leaves per-channel routing to the
  caller, keeping macrdp's TCP fallback for a wedged tunnel working.

Five tests in the module (run on a scratch copy with test = true: 12 pass);
mutation-checked — registering routing fails the key test. Standalone build,
clippy -D warnings and stable fmt clean; the diff against upstream src/ has
no removed lines. Callers are ported with the server-fork rebase; the branch
still does not build until then.
src/ is upstream e258f6a0 verbatim except connection.rs (+59/-9), which
keeps two divergences:

- (4) client fingerprint (client_name / client_version / client_build on
  AcceptorResult), unchanged in behaviour.
- (5) new: set_multitransport_requested_protocol(RequestedProtocol).
  Upstream's own offer (#1951) only requests UdpFecR; lossy audio needs
  UdpFecL. Default stays UdpFecR, so upstream behaviour is unchanged when
  it isn't called.

Retires our multitransport offer machinery (MultitransportOffer,
multitransport_offered, the finalization channel skip) in favour of
upstream's offer, security-RNG hook and late-response tolerance.

Tested against IronRDP's testsuite-core (17 acceptor tests incl. 4 new,
1731 total) with both divergences mutation-checked; see CLAUDE.md.

The branch still doesn't build: the rdpdr and server forks are next.
Upstream made the drive (EFS) PDU layer two-way (#1779) and fixed
variable-length smart-card contexts/handles (#1654), so most of the fork
retires. src/ is now upstream verbatim plus two new files, wired in by
four lines across three upstream files:

- pdu/server_direction.rs: a public Encode/SvcEncode for
  ServerDriveIoRequest (upstream's equivalent is private) and
  ScardControlRequest, now built on upstream's DeviceControlRequest::encode.
- pdu/esc/server_direction.rs: the smart-card server direction upstream
  still lacks (encode *Call, decode *Return), adapted to upstream's new
  Option-based return shapes. Keeps the live-Windows rule that an empty
  pbExtraBytes is a NULL referent, which upstream's SCardIORequest encoder
  does not do.
- device_type() made pub.

52 tests pass (upstream's 38 plus 14 of ours), clippy clean; seven
mutations of the live-Windows rules are each caught. Our server code
needs 10 mechanical fixes, listed in CLAUDE.md for the server step.

Also: Cargo.lock picks up rand (acceptor, previous step) and getrandom
(rdpdr), both already locked; root Cargo.toml comments for the acceptor
and rdpdr forks were stale and now match what each fork carries.

The branch still doesn't build: the server fork is next.
Upstream e258f6a0 now ships much of what the fork carried, so the fork
was rebuilt as upstream verbatim plus macrdp's own files and marked hook
edits, rather than a 51-conflict merge.

Retired (macrdp's src/ switches to upstream's API in the next step):
SuppressOutput (5), NSCodec (6, now the `nscodec` feature), honor client
desktop size (9), keyboard-layout cell (10), auto-reconnect cookie (13),
mouse position before button (21), auth-gated preemption (22/23, now
ConnectionPolicy::Preempt), and the undocumented ClipboardFileCopy event.

Kept, re-applied onto upstream's code: the audio task and lag model
(2/3/8), batch priority (4), our RDPDR/USB/camera/microphone channels
(11/16/19/25, alongside upstream's inert equivalents), UDP multitransport
(12, all server-side logic in mt/session.rs, ported to the rebased
acceptor's offer API and dvc's request_soft_sync), the EGFX decline flag
(14), accept-time RTT (15, now also for preemption winners),
on_authenticated (18), the fingerprint log (20), and input reset (24).

Name clashes with upstream resolved by renaming ours: the drive channel's
types and builder method, the USB builder method, the src/multitransport
module (now src/mt), and our RDPEUDP crate (now macrdp-rdpeudp, so
upstream's ironrdp-rdpeudp can be pinned alongside it).

Behaviour changes from adopting upstream, on the live-test list:
returning auto-reconnect cookies are now HMAC-verified (a reconnect after
a process restart is denied), and spec-following clients now receive
SuppressOutput support and so get minimize handling.

The old divergence log is kept as DIVERGENCE-HISTORY.md; CLAUDE.md is
rewritten for the new shape. The server crate builds under every feature
combination, fmt and clippy clean apart from two upstream lints. macrdp's
own src/ is the next step, so the branch still doesn't build.
The branch builds again: 266 tests pass, fmt (stable and nightly) and
clippy -D warnings are clean, and cargo-deny passes on the new
dependencies.

- Builder instead of runtime setters for what upstream now owns: the
  display-suppressed flag, honor-client-desktop-size (with the
  --max-client-size ceiling folded into one Option), the auto-reconnect
  cookie, and ConnectionPolicy::Preempt. Preempt must be explicit:
  upstream's default (Queue) silently brings back the second-client
  hang. conn_test's servers mirror these settings.
- Connection hooks are composed, not wrapped. lock_activity's
  ActivityHandler forwarded each hook by hand, so upstream's new
  on_connection_info would have been silently swallowed whenever
  --lock-on-disconnect was on. ConnectionHooks (src/conn_hooks.rs) runs
  the auth guard, the activity record, and the new keyboard-layout hook
  in order, stopping at the first rejection.
- Mouse buttons carry their position (upstream #1769): move first, then
  click, for every button. Horizontal wheel is newly reported and
  mapped to horizontal scrolling.
- The display traits return upstream's error type; the capture loop
  stays on anyhow and converts at the boundary (no change to its body).
- Smaller moves: drive types renamed (RdpdrDrive*), the USB builder
  method, ClipboardFileCopy to SendInitiateFileCopy, the smart card
  handle rebuilt through from_opaque, and the Soft-Sync wire-byte tests
  ported to upstream's PDU types with the expected bytes unchanged.
- x509-cert declares its std feature: macrdp used Time::to_system_time
  but only got the feature by unifying with the old server's x509-cert.
- macrdp-rdpeudp: the standalone pin was still a5d1c682 and its fuzz
  harness still asked for ironrdp-core 0.1 at 879ffed (broken since the
  last bump, unnoticed because CI doesn't build it). Both now e258f6a0.

Not verified locally: the Linux build (cross-compiling the C deps
fails here); CI covers it. Live testing is still to come.
- vendor/ironrdp-rdpeudp is now vendor/macrdp-rdpeudp, and the server's
  multitransport code lives in src/mt/.
- APIs that moved upstream: the keyboard layout now arrives through
  on_connection_info, honor-size, the auto-reconnect cookie and
  SuppressOutput are set through the builder, and Mac-to-Windows file
  copy goes through ClipboardMessage::SendInitiateFileCopy.
- Two behaviour changes are flagged in the notes they touch: upstream
  HMAC-verifies the auto-reconnect cookie, so a reconnect after a
  process restart is denied it; and upstream advertises SuppressOutput,
  so FreeRDP and Thincast now get the minimize mute.
- TODO records progress and the live-test list.
The four step-4 audits' recommendations are all in the code. Three
behaviour caveats from them weren't recorded anywhere:
- The ARC cookie is now rotated on every Deactivation-Reactivation,
  including blank-recovery heals.
- The multitransport offer now requires the client's MCS message
  channel.
- Upstream decodes URBDRC completions more strictly (as isoch).
capture.rs and conn_test.rs still named the retired runtime setter
RdpServer::set_honor_client_desktop_size. Comment-only.
The Mac mini live tests so far: the passes, the two fixes that landed on
main (#195, #196), and what's still to do. Also a Deferred entry for the
intermittent client-to-mini clipboard gap: it happens on v0.9.6 too, so
it isn't a bump regression. It includes the test gotchas behind
yesterday's false reading.
…of upstream #2108)

RGBAPointer and LargePointer went out with an empty AND mask. MS-RDPBCGR
2.2.9.1.1.4.4 requires one, and Windows App drops a 32 bpp pointer
without it: on macOS it stays on the default arrow (verified by upstream
#2108's author on macrdp), and it's the leading suspect for the iOS
client showing no cursor at all (#211). mstsc and FreeRDP accept the
empty mask.

This is upstream's fix byte for byte (Devolutions/IronRDP#2108, by
@antonmos, merged 2026-10-19, after the e258f6a0 pin): a new
encoder/pointer_mask.rs builds the mask from the alpha channel, padded to
2 bytes per scan line. Recorded as divergence (27); drop it at the next
pin bump.

Its three unit tests live in pointer_mask.rs, but this lib is
test = false, so they were run in a scratch crate: all pass.
The iOS Windows App advertises 8 EGFX capability sets and this build
parses 6 (#211). The dropped two were invisible, so there was no way to
tell whether one of them carried AVC support. The capabilities log line
now lists each unparsed set's version, its flags word and length, and
whether the version is unknown or failed to decode. Only the flags word
is printed, so a long body can't flood the log.

Tested: a unit test covers an unknown "Azure" version, a known version
with a truncated body, and a 4096-byte body; reading the flags
big-endian or dropping the length fails it.
…ck) onto e258f6a0

#222 landed on main against the a5d1c682 fork; this carries it onto the
re-vendored server. client_loop reads the socket in its own read_pdus arm:
fast-path input goes straight to the input handler, X224 PDUs go through a
bounded queue to dispatch_pdu, which still takes the lock (upstream's
per-PDU lock-wait logging stays there, where the lock still is). Frames
still queued when a reactivation ends the loop go into
MacrdpServerState::read_ahead and replay ahead of the reactivation's
backlog; they are dropped if the next handshake is not a reactivation.

Divergence 27 (TCP_NODELAY) is not ported: e258f6a0 sets it at both
accept sites that serve a connection.

The conn_test from #222 needed two changes for the new pin: the display
trait now returns ServerResult, and the flood frames must differ, because
upstream's bitmap encoder skips an unchanged frame (so the pipe never
filled and the blocking write never happened). The test fails without
this server change and passes with it.
…e AND mask to 29

Main added (26) input off the server lock, (27) TCP_NODELAY and (28)
zstd-safe 8.2 while this branch was open. (26) and (28) are kept, (27)
is retired because e258f6a0 sets TCP_NODELAY itself, and the branch's
AND-mask backport moves from (27) to (29) so each number means one
thing. Main's entries for 26-28 are appended to DIVERGENCE-HISTORY.md,
whose numbers this log shares. Also lists clippy's PendingIo warning,
which is upstream's, under known leftovers.
…uto-reconnect cookie

Upstream e258f6a0 verifies a returning auto-reconnect cookie and denies the
connection when it doesn't verify. A cookie issued before a server restart
never verifies, so restarting macrdp under a connected FreeRDP made the client
retry the same cookie about three times a second. Each try ended within a
second, so auth_guard counted it as a failed login and locked out the client's
address (30 s, then 59 s). Each try also evicted the live session first,
because preemption runs before the cookie check. Found live on the mini.

MS-RDPBCGR 3.3.5.3.11: "If logon with the cookie fails, the credentials
supplied in the Client Info PDU SHOULD be used". Divergence (30) does that.
It grants nothing extra: CredSSP or the acceptor's credential check has
already authenticated the client, and the credential validator still runs.

Test: conn_test::a_stale_auto_reconnect_cookie_falls_back_to_a_normal_logon
checks that the server is still serving a second after the client connects
with a stale cookie (the client can't tell on its own: it counts as connected
before the server checks). It fails with "cookie rejected" when the denial is
restored. Live-verified on the mini: one reconnect, normal logon, no lockout.
Records the 2026-10-28 FreeRDP pass on the mini (H.264, AAC, minimize, drive
read/write, clipboard, takeover, no-AVC decline) and the auto-reconnect cookie
regression it found, now fixed as divergence (30). Replaces the stale open-PR
snapshot, queues committing the vendor lockfiles with --locked after the bump,
and updates the CLAUDE.md status line.
The bump's ConnectionHandler::on_disconnected takes a ServerError, not an anyhow::Error.
@clintcan
clintcan force-pushed the chore/pin-bump-e258f6a0 branch from f6f6ad6 to d82fbb7 Compare October 28, 2026 22:11
…hat emit audit events

#233's handler test emits audit events on another thread while the
audit-capture tests run. tracing caches a callsite's enabled state
process-wide, so a concurrent registration could leave the capture's
success event disabled (about 1 run in 40 locally, and on Linux CI).
0 failures in 300 runs with the lock.
Same lockfile bump as #243 on main, so the pin bump's cargo-deny scan
stays green until the next rebase picks it up from main.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant