Support secure TCP rendezvous for logged-in clients - #689
Open
TylonHH wants to merge 1 commit into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe TCP rendezvous channel adds optional encrypted transport for recent logged-in clients through key exchange, encrypts outbound messages, decrypts inbound bytes, and preserves plaintext handling when encryption is unavailable. ChangesSecure TCP transport
Estimated code review effort: 4 (Complex) | ~40 minutes Sequence Diagram(s)sequenceDiagram
participant TCPClient
participant RendezvousServer
participant Encrypt
participant handle_tcp
RendezvousServer->>TCPClient: Send signed KeyExchange
TCPClient->>RendezvousServer: Send client KeyExchange
RendezvousServer->>Encrypt: Derive shared encryption state
TCPClient->>RendezvousServer: Send encrypted TCP bytes
RendezvousServer->>Encrypt: Decrypt inbound bytes
Encrypt->>handle_tcp: Pass decrypted request bytes
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
TylonHH
marked this pull request as ready for review
July 29, 2026 09:02
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.
Problem
Logged-in RustDesk clients (observed with 1.4.9) establish a TCP connection to the OSS rendezvous server, then wait for a secure-channel key exchange that stock
hbbsdoes not provide. The client eventually reports:This addresses the OSS-server side of #394.
Changes
KeyExchangeresponse and derive the shared session keyhbbrbehavior unchangedCompatibility
The handshake is opportunistic. A client that does not answer with
KeyExchangecontinues over the existing plaintext TCP path, so this should not require a coordinated client update.Validation
cargo check --bin hbbswith Rust 1.88 on Debian Bookwormgit diff --checkThe build completed successfully. It reported only warnings already present on the current
masterbranch.Live validation
The patched
hbbsimage was deployed in the affected OSShbbs/hbbrplus third-party API setup. The previously failing logged-in RustDesk 1.4.9 connection succeeded immediately without changing the client ID server, relay server, API server, or public key settings.Prior art
This implementation was adapted to current
masterfrom the proof of concept shared by @kamuzon in issue comment #394, specifically kamuzon/rustdesk-server@ec5956f. Credit also goes to @eltorio for the earlier secure-TCP investigation and patch discussed in that issue.Greptile Summary
This PR adds opportunistic Curve25519/secretbox encryption to the TCP rendezvous path in
hbbs, addressing theFailed to secure tcp: deadline has elapsederror that logged-in RustDesk 1.4.9 clients report against the OSS server. When a server identity key (sk) is configured, the server now sends a signed ephemeral box public key to every new TCP connection; clients that respond with aKeyExchangeget a fully encrypted session while older/plain clients fall through to the existing plaintext path.Sinkenum gains anOption<Encrypt>slot so the server can encrypt outbound frames once the session key is established; inbound decryption is handled by a separatereceive_encryptlocal that is set on the same loop iteration theKeyExchangeis processed.KeyExchangewith the wrong key count triggersbail!(hard close, logged only at DEBUG), while a message that parses but is not aKeyExchangesilently falls through to plaintext — both branches lack a WARN-level log entry, which will make production diagnostics harder.Confidence Score: 4/5
Safe to merge for the primary goal of unblocking logged-in 1.4.9 clients; all identified concerns are non-blocking observability and compatibility caveats rather than functional failures.
The key derivation, encryption/decryption wiring, and backward-compat fallthrough all appear correct. The three findings are about missing WARN-level logging on failed handshakes, asymmetric error handling between a structurally wrong KeyExchange (hard close) and an entirely non-KeyExchange first message (silent plaintext fallthrough), and the fact that the unconditional server-initiated KeyExchange offer could confuse older clients that don't expect a server-first frame — none of these represent data loss or a broken session under normal operation.
Files Needing Attention: src/rendezvous_server.rs — specifically the handshake error paths around lines 1248-1260 and the unconditional offer at lines 1229-1240.
Important Files Changed
Sequence Diagram
sequenceDiagram participant C as RustDesk Client (1.4.9+) participant S as hbbs Server C->>S: TCP connect Note over S: sk configured → gen ephemeral box keypair S->>C: "KeyExchange { keys: [sign(ephem_pk, server_sk)] }" Note over C: Verify signature with server identity key,<br/>generate secretbox key, encapsulate with server ephem_pk C->>S: "KeyExchange { keys: [client_pk, box(sym_key, client_sk, server_ephem_pk)] }" Note over S: Decrypt sym_key via Encrypt::decode,<br/>arm receive_encrypt + send_encrypt C->>S: "RegisterPeer (encrypted, nonce=1)" S->>C: "RegisterPeerResponse (encrypted, nonce=1)" C->>S: "PunchHoleRequest (encrypted, nonce=2)" Note over S: sink.take() → tcp_punch[addr] = Sink::TcpStream(s, Some(encrypt)) S->>C: PunchHole (encrypted via tcp_punch send_to_sink) Note over C,S: Old / plaintext client path participant O as Old Client O->>S: TCP connect S->>O: KeyExchange offer (ignored by old client) O->>S: RegisterPeer (plaintext) Note over S: handshake_secret taken but msg is not KeyExchange,<br/>receive_encrypt stays None, handle_tcp in plaintext S->>O: RegisterPeerResponse (plaintext)Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "support secure TCP rendezvous handshake" | Re-trigger Greptile
Summary by CodeRabbit
New Features
Compatibility