The controller sends every candidate twice, because the server's hop to a
peer registered over UDP can lose one. The ICE agent that dedups repeats
sits downstream of the answerer's queue, so the answerer paid for both
copies: a slot, a JSON parse, and the ICE agent's lock, once per repeat.
Remember a digest of what was queued and skip the repeat. Recorded only
once queued, so a candidate a full queue refused stays repairable by the
re-send.
The queue's depth is unchanged. A real peer gathers well under it - four
STUN servers, link-local IPv6 filtered, one component - and the drain
empties it as candidates trickle in, so what this removes is the redundant
work, not an overflow.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns
punch_udp sent a zero-length datagram and called the hole open on whatever
arrived next. The rendezvous NAT test's own leftover replies satisfy that
immediately - connect() does not flush the receive queue - so the retry loop
never ran and success meant nothing. The dead socket then cost KCP its full
timeout to rediscover, which is how a failed punch came to take 18 seconds.
Probes now carry a magic and a 64-bit transaction id, and both ends answer
each other's probes, so returning is a fact: a reply echoing our own id is the
one thing that proves the pair carries traffic both ways. With failure now
distinguishable from 'not yet', the window drops from 20s to 3s.
Two asymmetries fall out of that:
Only the connector stops on its own acknowledgement, because only it has
something to send next. An acknowledgement proves our probe came back, not
that the peer's probe was answered - and after punch_udp returns nothing
answers probes any more, since KCP's io loop drops anything shorter than its
header. A listener that stopped there would go mute while a peer whose own
probe or answer was lost - the normal state of a hole still opening - kept
probing an endpoint that works, until it timed out.
So the listener stops on the peer's first real packet instead, and hands that
packet to KcpStream::accept as its init_packet: its arrival proves the pair as
well as an acknowledgement would, and KCP never retransmits its SYN.
`get_local_endpoint_trickle` became `local_endpoint() -> &str`, which
cannot fail, so both call sites lose an unreachable error arm — the
mediator's closed a pc against a failure that no longer exists.
`punch_type` named one transport, and picked it off `allow_tcp_punch`.
A round carries several at once — a NAT port and a v6 address and an
offer — and since the TCP punch became a switch it can carry none, so
one name had to misreport both: the logs of the round that broke WebRTC
read "#1 UDP punch attempt" while the request also carried the v6
address and the offer that was actually failing, and a round with
nothing to punch with was labelled "WebRTC". List them instead —
"UDP+IPv6+WebRTC" — and call the empty round "Relay", which is what it
can still end as and what `typ` prints for it.
The offer is moved into the request rather than cloned into it; that
was its last use.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019UzcMTdYTEv2QbMHcTSUy3
386 added comment lines down to 287 across client, mediator, kcp_stream
and common. Same rule as hbb_common 3d64e43: out go past-bug narration,
rejected alternatives, measurements and restatements of the code; the
non-derivable why stays.
is_direct_transport goes with them. Judging the race by a transport
label was replaced by the resolved direct flag, leaving it used only by
its own test — and, having been inserted between the doc comment and
race_transports_prefer_webrtc, it had also taken that function's
contract with it. Removing it reattaches the doc where it belongs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
Third review round. Two of these are regressions from the previous one.
- The RelayResponse race predicate was `is_direct_transport(result.2)`,
which answers true for the label "WebRTC" - but WebRTC is only a
direct path when ICE nominated a non-TURN pair. A TURN-relayed WebRTC
result therefore committed instantly and cancelled the IPv6 attempt
racing beside it, which is the same inversion the previous fix removed
in the other direction. (That fix was also argued from a wrong premise:
the site does carry an IPv6 future, pushed ~50 lines earlier than the
relay one.) Each future now resolves whether its path is direct and
the predicate reads that bool, matching the outer race, and the
downstream recomputation goes away.
- policy_relay still folded in Config::is_proxy(), and that is what gets
persisted into the peer's config as force-always-relay - so one
session through a proxy pinned the peer to relay forever and disabled
WebRTC for it, exactly the latch the previous round fixed for
WebSocket. Split out peer_relay: the saved option or an explicit
request for THIS peer, and the only part written back.
- The controlled side buffered remote ICE candidates in an unbounded
channel while the controller caps the same buffer at 64, and draining
one costs a JSON parse plus the ICE agent's lock. Whoever can reach a
session's route could grow it without limit inside the long-lived
service process. Bounded, with the overflow logged through the
existing throttle.
- That route was also removed by key alone when an answerer finished, so
a punch retry that built a fresh answerer under the same fingerprint
had its live sender deleted by the previous one's cleanup - after
which it received no candidates at all. Evict only our own sender, the
way the session cache already guards the analogous case.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
Review of #15684 and hbb_common#579. Each of these left the code reading
correct while the feature did not function.
- The RelayResponse race classified P2P with `result.2 == "IPv6"`, but
that site's futures are only ever the relay ("Relay"/"WebSocket") and
the WebRTC branch's own "WebRTC" — so the predicate was constantly
false. When the relay landed first the result was still right (the
webrtc arm's `others_fut.is_none()` fallback), but when WebRTC
connected FIRST it was parked as if it were a relay and the relay was
committed on arrival, discarding a live direct connection. That is the
LAN case: the better the network, the worse the outcome. Classify by
what the label means, via is_direct_transport, and test both orderings
— only the relay-first one was covered.
- handle_peer_info wrote "force-always-relay=Y" into the peer's saved
config whenever force_relay was set, which now includes the WebSocket
transport. One ws session therefore turned the peer into a permanent
relay-by-policy peer, and relay-by-policy means Relay-only ICE, so
WebRTC could never go direct to it again — the flagship path worked
exactly once. Persist policy_relay, which is the user's choice; the
transport is a property of this client, not of the peer.
- The answerer gated on this machine's enable-webrtc option, but that is
LocalConfig: the UI process writes it and never syncs it over IPC,
while handle_punch_hole runs in the server process, which on Windows
resolves LocalConfig under a different profile and reads the
private-server default of "N". The gate refused to answer in exactly
the self-hosted deployments the transport exists for. Drop it: the
answerer follows the request, like the udp/ipv6 legs, and the option
still gates the feature where it can — an offer only exists because
some controller had it enabled.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
OPTION_ENABLE_WEBRTC (hbb_common 48c2d4d) follows the udp/ipv6 punch
options end to end: default on against the public server, off against
private ones, same settings UI placement on desktop and mobile, and the
same bool2option local-option handling. Gates:
- controller: should_create_webrtc_offerer checks it first — no pc, no
STUN/TURN gathering, no offer in the request;
- controlled: unlike the udp/ipv6 legs, which deliberately follow the
request, answering builds a pc that gathers ICE from this host, so
the answerer honors this machine's own switch too.
Translations for "Enable WebRTC P2P connection" added to all 50 lang
files next to the IPv6 entry (IPv6 and WebRTC are invariant terms in
the same grammatical slot in every one of them).
Also stop probing v6 reachability (test_ipv6) under any forced relay:
the v6 punch socket is never bound there, so the probe was wasted work
on every ws/proxy/relay connection.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
Companion to hbb_common 68d2729: the full-ICE declaration now lives as
an `ice_policy: "all"` key inside the webrtc:// envelope, so the request
assembly no longer sets webrtc_all_ice and the controlled side asks the
envelope (endpoint_declares_all_ice) instead of a PunchHole field. The
rendezvous server carries the offer opaquely — no forwarding to keep in
sync. Skew behavior is unchanged: an unmarked or unparseable envelope
reads as the old Relay-only semantics.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
WebSocket support folds into force_relay because a ws tunnel kills
classic TCP/UDP punching — but that conflated transport necessity with
relay policy, and the WebRTC decisions keyed off the merged flag: a ws
client built no offerer at all without TURN, and only a Relay-only-ICE
one with it. ws deployments could never reach a direct WebRTC
connection, which is exactly the path they are supposed to live on.
Split the flag. LoginConfigHandler now tracks policy_relay (the
force-always-relay option, an explicit relay request — /r ids and
retry-via-relay included — and proxy) separately; force_relay stays
policy_relay || use_ws() and keeps governing the classic paths, so
non-ws behavior is unchanged everywhere:
- the offerer's existence and ICE policy follow policy_relay: under
pure ws the offer gathers every candidate type and may go direct;
under relay-by-policy it stays Relay-only ICE, TURN-gated, exactly
as before;
- the RelayResponse race applies the prefer-P2P window under ws (a
direct ICE path is worth delaying an already-ready relay for) while
policy relay keeps first-success semantics;
- the request carries webrtc_all_ice (hbb_common 64b54ab) so the
controlled side knows the offer is full-ICE: it answers with full ICE
and no TURN requirement, while offers without the bit keep today's
relay-only answer path on every version-skew combination.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
Debug output goes to the log file, so a site that fires per received message
or per retry lets someone else decide how much a machine writes to disk. The
WebRTC work added the first such sites.
- KCP io loop: absorbing ICMP errors as packet loss made a broken socket write
~100 lines a second for the 60s until the pong timeout reaps it. Log by run
instead: one line when a run starts, one per ~5s while it persists so a stuck
socket stays visible, and one on recovery with the total.
- punch_udp: the recv error retries every 10ms for up to MAX_TIME, so one line
per occurrence wrote thousands per punch. Log the first, report the count in
the timeout message.
- ICE candidate paths (client, mediator): the peer sets the candidate rate and
the rendezvous route carrying them needs no prior punch, so throttle to one
line a minute each with the suppressed count.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
- the WebRTC offer now rides any punch request; only an offer-less request
may close and reuse the rendezvous socket for TCP punching
(request_allows_tcp_punch replaces the udp_port-based invariant), with a
separate offer-less request racing as the TCP fallback
- WebSocket mode no longer disables WebRTC — ws only tunnels the
signaling/relay legs while ICE stays the only P2P path there; SOCKS proxy
still disables it (ICE would bypass the proxy and leak the real IP)
- controlled side: WebRTC-only punch replies and trickled ICE candidates go
over dedicated TCP connections to the rendezvous server instead of the UDP
mediator channel, for ws/TCP-only hbbs deployments; drop the now-redundant
rz_sender plumbing and the 400ms candidate re-send on that leg
- guard is_udp handling against responses to requests that advertised no
udp_port; skip the IPv6 socket bind under force-relay
- test_udp_uat: drop the STUN port race — the punch port must come from the
rendezvous server's TestNatResponse observing this socket's mapping, a
STUN probe from another socket can advertise an unreachable port
- bump hbb_common (webrtc 0.13 MSRV pin rationale + upgrade checklist docs)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- prefer-P2P racing (race_transports_prefer_webrtc) across punch and RelayResponse; ICE bridge with 400ms candidate resend
- controlled-side answerer and ICE routing; sign local DTLS fingerprint into SignedId, controller verifies the binding fail-closed
- fix pc leaks: close_webrtc() on insecure-decline paths (io_loop, port_forward); compute direct before disarming the offerer guard
- point hbb_common to the WebRTC data-plane commit 9f5a296
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Support controller user attribution in audit logs
This PR supports associating audit logs with the controller user.
## Implementation:
- Add `ControlledContext { conn_audit_token }` to `PunchHole`, `RequestRelay`, and `FetchLocalAddr`.
- The server sends a controller-user identity snapshot to the controlled client through rendezvous messages.
- The controlled client sends the token back to the server when posting the `on_open` conn audit or IP whitelist alarm audit.
- This lets the server attach the controller user to audit logs.
## How the controlled client helps identify the controller user:
- Conn audit: sends the token to the server in `on_open`; the server creates the audit log and caches the user snapshot.
- File audit: sends `id` and `conn_id`; the server uses them to find the cached user snapshot.
- Alarm audit: IP whitelist sends the token directly; other alarm logs send `id` and `conn_id`, and the server uses them to find the cached user
snapshot.
## Compatibility:
- Supported only for logs created with a new server and a new controlled client.
- Does not require upgrading the controller client.
## Test
- [x] New/old clients connected to new/old servers, and conn/file/alarm audit logs worked normally.
- [x] New client connected to new server generated searchable conn/file/alarm audit logs.
- [x] Punch hole, local addr, and relay paths worked with audit logs and control role on new/old servers.
- [x] Direct IP connections produced audit logs, but do not support user audit.
Signed-off-by: 21pages <sunboeasy@gmail.com>
* rename conn_audit_token to conn_audit_ref
Signed-off-by: 21pages <sunboeasy@gmail.com>
---------
Signed-off-by: 21pages <sunboeasy@gmail.com>
* --deploy, reuse the device token
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix review
* no id validation in deploy, so to keep the same behavior in udp register
pk
* Fix collapsed toolbar drag preview sizing
* Revert "Fix collapsed toolbar drag preview sizing"
This reverts commit 66e39abb74.
* remove too many logs
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix(macos): sync config to root when root config is empty
Signed-off-by: 21pages <sunboeasy@gmail.com>
* fix(server): gate startup on initial config sync; document CheckIfResendPk limitation
- wait up to 3s for initial root->local config sync before starting server services
- continue startup when timeout is hit, while keeping sync/watch running in background
- avoid blocking non-server process startup
- clarify that CheckIfResendPk only re-registers PK for current ID and does not solve multi-ID when root uses a non-default mac-generated ID
Signed-off-by: 21pages <sunboeasy@gmail.com>
---------
Signed-off-by: 21pages <sunboeasy@gmail.com>
1. Enable the RustDesk client to use WebSocket for either controlling or being controlled.
2. Fix TCP sending `register_pk` frequently
Note:
1. Because hbb_common directly uses `use_ws` to read config directly, rustdesk also directly reads config
Signed-off-by: 21pages <sunboeasy@gmail.com>
