feat(pam-rdp): record RDP sessions#218
Open
bernie-g wants to merge 9 commits intofix/cli-rdp-musl-staticfrom
Open
feat(pam-rdp): record RDP sessions#218bernie-g wants to merge 9 commits intofix/cli-rdp-musl-staticfrom
bernie-g wants to merge 9 commits intofix/cli-rdp-musl-staticfrom
Conversation
Tap each PDU in the post-CredSSP byte bridge and stream structured events (target_frame / keyboard / unicode / mouse) through the existing session logger so they land in the encrypted chunk pipeline. Capture switches the post-CredSSP path from copy_bidirectional to a PDU-framed bridge: read_pdu yields TPKT/FastPath frames pure-framing, no RDP state machine, the bytes are forwarded unchanged, and the tap emits SessionEvent variants on an mpsc channel. This preserves the no-MCS/capability/share-state-drift property of the byte-pump it replaces. The FFI gains rdp_bridge_poll_event for Go to drain those events with a timeout. TargetFrame payloads are handed across as libc::malloc'd buffers; the Go side defers C.free after copying. Go-side, RDPProxy.HandleConnection spawns a drain goroutine that JSON- encodes each event and calls SessionLogger.LogTerminalEvent with ChannelType=rdp. The chunk uploader is protocol-agnostic, so RDP sessions now flow into pam_session_event_chunks like SSH/HTTP do. session.LogTerminalEvent skips masking for the rdp channel because the data field carries a base64-JSON envelope; SSH-shaped masking regexes would corrupt valid recordings.
Three fixes that together make RDP recording playback render correctly: - Filter Order, BitmapCodecs, and INFO_COMPRESSION on the wire so the server only emits Bitmap update PDUs IronRDP-session can decompress. Implemented as byte surgery on Confirm Active and Client Info PDUs; IronRDP's typed decode->encode loses unrelated fields. New cap_filter module + walk_caps + 14 unit tests pin the byte-preservation contract. - Override ev.ElapsedNs with time.Since(SessionStartedAt) in the Go drain so reconnects within the same PAM session don't restart the bridge's local clock from zero. SessionUploader exposes GetSessionStartedAt (reconstructed from the persisted lastEndElapsedMs). - Stamp chunk endElapsedMs from the last entry's elapsedTime instead of time.Since(state.startedAt) at flush moment, so the playback total doesn't reach past the last actual frame. readFromOffset returns the trailing entry's elapsed time; falls back to wallclock for non-terminal sessions whose entries lack the field. Comment cleanup pass across the touched RDP files.
|
💬 Discussion in Slack: #pr-review-cli-218-feat-pam-rdp-record-rdp-sessions Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
…isement Adds two MITM bridge fixes so both Windows App/mstsc and FreeRDP work through the gateway: - Connector now advertises HYBRID_EX|HYBRID|SSL (matching native clients) instead of IronRDP's hardcoded HYBRID|HYBRID_EX. Native clients validate the MCS Connect Response echo of clientRequestedProtocols against what they sent on their own X.224 step and disconnect on mismatch. Done via a small connector_x224_with_protocol helper that replaces ironrdp_tokio::connect_begin (which exposes no knob for the protocol set). - filter_client_mcs_connect_initial now mutates CS_CORE.serverSelectedProtocol to HYBRID_EX before forwarding (FreeRDP echoes the wrong value, which target Windows servers reject) in addition to clearing CS_NET channels to stop the target from opening virtual channels the bridge can't service. Bridge errors and panics also surface to the gateway stderr via eprintln so silent Rust failures aren't lost.
Generated .rdp file now sets `authentication level:i:0`. mstsc validates the server's TLS cert by default and rejects the bridge's self-signed cert with "unexpected server authentication certificate", terminating the connection before the X.224 handshake. FreeRDP and Windows App don't enforce the same check, so this only manifests for mstsc users. Verified through mstsc on a Windows EC2 connecting via gateway+relay.
875ac29 to
3f00ca5
Compare
PR #191's release pipeline flipped the linux builds from CGO_ENABLED=0 to CGO_ENABLED=1 to link the Rust IronRDP bridge. With CGO on, the Go linker hands off to gcc, which dynamically links against the build host's glibc. v0.43.80 ended up with a GLIBC_2.39 floor from the ubuntu-24.04 GitHub runner, breaking ~80% of customer environments (Ubuntu 22.04, RHEL 8/9, Amazon Linux, Alpine, distroless/static). Switch the linux RDP builds to musl-static so the binary is fully self-contained again, matching pre-PAM portability: - build-rdp-bridge.yml: linux Rust matrix swapped from *-linux-gnu* to *-linux-musl* (windows-gnu kept). - goreleaser.yaml: each linux-*-rdp build entry uses CC=<triple>-unknown-linux-musl-gcc, points CGO_LDFLAGS at the musl target dir, adds -extldflags '-static' to ldflags, and adds osusergo,netgo to build tags to keep Go's pure-Go user/DNS resolvers (matching pre-RDP behaviour and sidestepping musl's NSS-less getaddrinfo). - release_build_infisical_cli.yml: install musl cross-toolchains from cross-tools/musl-cross GitHub releases (CDN-backed, replaces the unreliable musl.cc single-host mirror); pinned to release 20260430. curl retries kept for any network blips. - README.md (rust bridge): updated example triples. Adds a release-time gate: every linux RDP binary in dist/ must be 'statically linked', and the amd64 binary must --version cleanly across a matrix of older / minimal distros (Ubuntu 20.04+, RHEL 8+, Amazon Linux 2+, Alpine, distroless/static). A regression of the v0.43.80 shape now blocks publish. The Alpine Docker images and the .apk package are fixed for free since copying a musl-static binary into Alpine works cleanly. No Go or Rust source code changed beyond restoring the RDP feature.
…at/pam-rdp-recording # Conflicts: # packages/pam/handlers/rdp/bridge_cgo_unix.go # packages/pam/handlers/rdp/proxy.go # packages/pam/pam-proxy.go
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 795ae0530b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- Bridge tap channel switched from unbounded to bounded(1024) with try_send; drops on full instead of risking gateway OOM under heavy graphics. - bridge_pdus uses tokio::select! instead of try_join! so a normal client disconnect doesn't hang on the t2c branch waiting for a quiet target. - HandleConnection no longer cancels the drain on normal session end; the drain runs to PollEnded so the recording tail is preserved. Cancellation paths still cancel explicitly. - SessionUploader.RegisterSession preserves the existing in-memory anchor when called multiple times for the same session (RDP reconnects), so elapsedNs stays monotonic across reconnects within a single PAM session. - uploadSessionFile bulk-upload fallback handles ResourceTypeWindows the same way as SSH (TerminalEvent records); previously fell through to the database-row decoder which silently zero-filled input/output.
5b44f1a to
760fef6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description 📣
Implements end-to-end RDP session recording: a Rust IronRDP-based MITM bridge taps each PDU on the gateway, the Go side streams events into the chunked-recording uploader, and byte-level capability filters force the server into a codec set the WASM replay decoder can decompress. Event timestamps are anchored to the PAM session start so reconnects within a single session play back as one continuous timeline.
Type ✨
Tests 🛠️
```sh
cd packages/pam/handlers/rdp/native && cargo test --lib
go vet -tags rdp ./packages/pam/...
```
Manually verified: Windows Server 2022 RDP playback (single connection, reconnect within session, multi-reconnect), playback total matches last frame, non-RDP session types (SSH/DB/K8s) unaffected.