From 3c933af785fc9c0817c4ef628cdfcb76cef84100 Mon Sep 17 00:00:00 2001 From: UNITRONIX <36471318+UNITRONIX@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:22:45 +0200 Subject: [PATCH] fix(signal): enhance single-IP fallback authorization logic Refine the authorization process for peers sharing the same public IP. Implement a safe fallback for stock RustDesk clients that use a new TCP port, ensuring that only one live peer at the IP is authorized. This addresses issues with identity inheritance and improves connection reliability. Update related tests to reflect the new logic. --- CHANGELOG.md | 1 + betterdesk-server/signal/handler.go | 26 ++++++++++++++++++++---- betterdesk-server/signal/handler_test.go | 20 +++++++++--------- 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56be3d03..95ca99ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - **RdClient desktop — native Cliprdr + folder file transfer stack (Refs #350):** recovered onto current `dev` from divergent history — Tauri `desktop_*` / `desktop_clipboard_*` IPC, `cliprdr.js`, desktop DnD, streamed folder upload/download. Requires rebuilt `rdclient-desktop` **and** panel update. ### Fixed +- **Outbound “ID does not exist” after 3.5.16 (#302 residual):** stock RustDesk PunchHole/RequestRelay on a new TCP port (no login token, no shared RegisterPk session) was rejected as `initiator_not_registered` because 3.5.16 removed the safe `FindAllByIP` fallback restored in 3.5.15. Auth again authorizes when exactly one live peer shares the public IP; multiple live peers at that IP still refuse with `initiator_ambiguous_same_nat` (no identity inheritance). Ships via panel update (Go signal restart). Verify: stock client connect no longer shows “ID does not exist” when the initiator is the sole live peer at its public IP. - **Relay `Unauthorized relay UUID` after P2P fallback (#356):** when hole punch timed out and the target sent `RelayResponse`, signal forwarded the UUID without minting a relay ticket, so hbbr rejected both peers (`Reset by the peer(0)`). `handleRelayResponseForward` now authorizes the initiator/target pair before advertising the UUID (same ticket path as `RequestRelay`). Ships via panel update (Go signal/relay restart). Verify: connection that needs relay after P2P timeout succeeds; no `[relay] Unauthorized relay UUID` for that session. - **RdClient desktop — Copy-Paste / File Transfer creates 0KB empty remote files (#350):** Tauri IPC returns file chunks as base64 strings; JS treated them as `Uint8Array` constructors (`new Uint8Array(base64String)`), which always yields length 0, so Cliprdr and the File Transfer modal wrote empty remote files. `coerceBinaryPayload` now decodes base64 before upload/Cliprdr paths. Ships via panel update (`local-files.js` / `filetransfer.js` / `cliprdr.js` / `compress.js` / `protocol.js`). - **RdClient desktop — Cliprdr paste still 0KB after base64 coerce (#350):** outbound FILEGROUPDESCRIPTOR advertised `FD_FILESIZE` plus Windows `FD_CREATETIME` (0x08 mistyped as “unix mode”). Remote CliprdrStream trusted a bad/zero stream length and returned EOF without `FILECONTENTS_RANGE`. Descriptors now match RustDesk (`FD_ATTRIBUTES | FD_WRITESTIME | FD_PROGRESSUI`, size via `FILECONTENTS_SIZE` probe); FileContents rejects empty RANGE ACKs and serializes responses. Requires rebuilt `rdclient-desktop` **and** panel update (`cliprdr.js`). Verify: paste/upload a non-empty local file and confirm remote size matches. diff --git a/betterdesk-server/signal/handler.go b/betterdesk-server/signal/handler.go index 3c1afae7..71052514 100644 --- a/betterdesk-server/signal/handler.go +++ b/betterdesk-server/signal/handler.go @@ -1909,6 +1909,9 @@ func (s *Server) authorizeRelayTicket(relayUUID, initiatorID, targetID string) b // 2. Valid BetterDesk client login token on the punch/relay message (#327) // 3. Panel signal-proxy CIDR (Web Remote) // 4. Live peer with exact ip:port match (FindByAddr) +// 5. Exactly one live peer at the same public IP (safe FindByIP fallback for +// stock clients that PunchHole on a new TCP port). Multiple live peers at +// that IP → initiator_ambiguous_same_nat (no identity inheritance, #302) // // Managed and locked modes additionally require an approved DB peer row (pending // enrollment alone is not enough). Panel proxy initiators skip peer-map / DB @@ -1948,10 +1951,25 @@ func (s *Server) requireAuthorizedInitiator(raddr *net.UDPAddr, targetID, token return s.finalizeAuthorizedInitiator(initiator.ID, raddr, targetID, initiator.Banned) } - // Never inherit an identity solely from a public IP address. NAT addresses - // are shared and attacker-controlled source ports are trivial to create. - s.logUnauthorizedInitiator(raddr, "", targetID, "initiator_not_registered") - return "", false + // 5. Safe IP-only fallback: stock RustDesk opens PunchHole on a new TCP + // port after RegisterPk/UDP heartbeat, so FindByAddr misses. Authorize only + // when exactly one live peer shares this public IP. + var live []*peer.Entry + for _, e := range s.peers.FindAllByIP(raddr.IP) { + if e != nil && !e.IsExpired(config.RegTimeout) { + live = append(live, e) + } + } + switch len(live) { + case 0: + s.logUnauthorizedInitiator(raddr, "", targetID, "initiator_not_registered") + return "", false + case 1: + return s.finalizeAuthorizedInitiator(live[0].ID, raddr, targetID, live[0].Banned) + default: + s.logUnauthorizedInitiator(raddr, "", targetID, "initiator_ambiguous_same_nat") + return "", false + } } // bindTCPSessionPeer records the peer ID on an open tcpPunchConn so a later diff --git a/betterdesk-server/signal/handler_test.go b/betterdesk-server/signal/handler_test.go index 0a25fafe..41170d4e 100644 --- a/betterdesk-server/signal/handler_test.go +++ b/betterdesk-server/signal/handler_test.go @@ -1246,16 +1246,16 @@ func TestExactAddrInitiatorAuthorized(t *testing.T) { t.Fatalf("exact addr auth = (%q, %v), want EXACTINIT1", id, ok) } - // A different port at the same public IP is not an authenticated identity. + // Sole live peer at this IP: stock clients PunchHole on a new TCP port. id, ok = srv.requireAuthorizedInitiator(udpAddr("198.51.100.81", 51001), "TGTEXACT1", "") - if ok || id != "" { - t.Fatalf("IP-only fallback auth = (%q, %v), want rejection", id, ok) + if !ok || id != "EXACTINIT1" { + t.Fatalf("single-IP fallback auth = (%q, %v), want EXACTINIT1", id, ok) } } -func TestSingleIPFallbackRejectsDifferentPort(t *testing.T) { - // A stock client must use the same registered endpoint, a bound TCP - // session, or an opaque client token; a shared NAT address is insufficient. +func TestSingleIPFallbackAuthorizesDifferentPort(t *testing.T) { + // Stock RustDesk PunchHole uses a new TCP port; with exactly one live peer + // at the public IP, authorize via FindAllByIP (3.5.15 / regression after 3.5.16). srv, database := newTestSignalServer(t, config.EnrollmentModeOpen) if err := database.UpsertPeer(&db.Peer{ID: "SOLEINIT1", Status: "ONLINE", IP: "78.31.94.73"}); err != nil { t.Fatalf("UpsertPeer: %v", err) @@ -1264,13 +1264,13 @@ func TestSingleIPFallbackRejectsDifferentPort(t *testing.T) { putOnlinePeer(srv, "TGTSINGLE1", "203.0.113.90", 52000, peer.ConnTCP) id, ok := srv.requireAuthorizedInitiator(udpAddr("78.31.94.73", 55041), "TGTSINGLE1", "") - if ok || id != "" { - t.Fatalf("IP-only fallback = (%q, %v), want rejection", id, ok) + if !ok || id != "SOLEINIT1" { + t.Fatalf("single-IP fallback = (%q, %v), want SOLEINIT1", id, ok) } resp := srv.handlePunchHoleRequestTCP(&pb.PunchHoleRequest{Id: "TGTSINGLE1"}, udpAddr("78.31.94.73", 55041)) - if phr := resp.GetPunchHoleResponse(); phr == nil || phr.Failure != pb.PunchHoleResponse_ID_NOT_EXIST { - t.Fatalf("IP-only PunchHole must be unauthorized, got %+v", resp) + if phr := resp.GetPunchHoleResponse(); phr != nil && phr.Failure == pb.PunchHoleResponse_ID_NOT_EXIST { + t.Fatal("single live peer PunchHole must not be refused as unauthorized") } }