From bbdde6a33226e8f59d4e45ec1dd9016a4fd12efe Mon Sep 17 00:00:00 2001 From: UNITRONIX <36471318+UNITRONIX@users.noreply.github.com> Date: Fri, 7 Aug 2026 06:49:31 +0200 Subject: [PATCH] fix(signal): mint relay ticket on P2P RelayResponse forward (#356) P2P fallback advertised UUIDs without AuthorizeRelayPair, so hbbr rejected peers as unauthorized. Authorize before forward; keep Claim hardening intact. --- CHANGELOG.md | 1 + betterdesk-server/relay/authorization.go | 6 ++ betterdesk-server/signal/handler.go | 41 +++++++++--- betterdesk-server/signal/handler_test.go | 81 ++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad0f48e9..56be3d03 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 +- **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. - **Enrollment QA follow-up (#351):** Enrollment Requests **All** filter aggregates pending + approved + rejected Go history. Reject & Ban → **Allow re-enroll** / Unban hard-deletes the enrollment audit peer so managed mode re-queues instead of leaving a zombie or bypassing approval. Orphan legacy `rejected_device_*` locks appear under Rejected. Devices `?search=` is applied on load (View device). Pending metadata can be enriched when HTTP enrollment supplies hostname/platform/version after a signal queue. Copy Device ID on the registrations table. Ships via panel update (Go API/signal restart). diff --git a/betterdesk-server/relay/authorization.go b/betterdesk-server/relay/authorization.go index ed541bff..1596d41f 100644 --- a/betterdesk-server/relay/authorization.go +++ b/betterdesk-server/relay/authorization.go @@ -48,6 +48,12 @@ func AuthorizeRelayPair(uuid, initiatorID, targetID string) bool { return defaultAuthorizationRegistry.Authorize(uuid, initiatorID, targetID) } +// ClaimRelayPair reserves one of the two connections for a signal-authorized +// UUID on the shared all-in-one registry (same store Claim uses in hbbr). +func ClaimRelayPair(uuid string) bool { + return defaultAuthorizationRegistry.Claim(uuid) +} + // RevokeRelayPairsForPeer removes unpaired tickets involving a banned peer. func RevokeRelayPairsForPeer(peerID string) { defaultAuthorizationRegistry.RevokeForPeer(peerID) diff --git a/betterdesk-server/signal/handler.go b/betterdesk-server/signal/handler.go index 271f63f1..3c1afae7 100644 --- a/betterdesk-server/signal/handler.go +++ b/betterdesk-server/signal/handler.go @@ -1591,17 +1591,38 @@ func (s *Server) handleRelayResponseForward(msg *pb.RendezvousMessage, senderAdd } } + // Resolve the initiator so we can mint a relay ticket before advertising the + // UUID. Without this, P2P→relay fallback forwards a RelayResponse that the + // hardened relay rejects as "Unauthorized relay UUID" (#356). + initiatorID := s.peerIDForAddr(initiatorAddr) + if initiatorID == "" && initiatorAddr != nil { + if n := s.peers.CountByIP(initiatorAddr.IP); n > 1 { + log.Printf("[signal] RelayResponse forward: ambiguous initiator IP lookup for %s (%d peers)", initiatorAddr.IP, n) + } else if entry := s.peers.FindByIP(initiatorAddr.IP); entry != nil { + initiatorID = entry.ID + log.Printf("[signal] RelayResponse forward: resolved initiator %s to peer %s via IP lookup", initiatorAddr, initiatorID) + } + } + if targetID == "" || initiatorID == "" { + log.Printf("[signal] RelayResponse forward: refusing uuid=%q — unresolved pair (initiator=%q target=%q sender=%s)", + rr.Uuid, initiatorID, targetID, senderAddr) + return + } + if !s.authorizeRelayTicket(rr.Uuid, initiatorID, targetID) { + log.Printf("[signal] RelayResponse forward: relay ticket rejected (uuid=%q initiator=%q target=%q) — not forwarding", + rr.Uuid, initiatorID, targetID) + return + } + var signedPk []byte - if targetID != "" { - if target := s.peers.Get(targetID); target != nil && len(target.PK) > 0 { - // Sign the PK with server's Ed25519 key (enables client E2E verification) - signed, err := s.kp.SignIdPk(targetID, target.PK) - if err != nil { - log.Printf("[signal] Failed to sign PK for %s in RelayResponse: %v", targetID, err) - } else { - signedPk = signed - log.Printf("[signal] Signed PK for %s in RelayResponse: %d bytes", targetID, len(signedPk)) - } + if target := s.peers.Get(targetID); target != nil && len(target.PK) > 0 { + // Sign the PK with server's Ed25519 key (enables client E2E verification) + signed, err := s.kp.SignIdPk(targetID, target.PK) + if err != nil { + log.Printf("[signal] Failed to sign PK for %s in RelayResponse: %v", targetID, err) + } else { + signedPk = signed + log.Printf("[signal] Signed PK for %s in RelayResponse: %d bytes", targetID, len(signedPk)) } } diff --git a/betterdesk-server/signal/handler_test.go b/betterdesk-server/signal/handler_test.go index 4618e4ba..0a25fafe 100644 --- a/betterdesk-server/signal/handler_test.go +++ b/betterdesk-server/signal/handler_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/unitronix/betterdesk-server/config" + cryptopkg "github.com/unitronix/betterdesk-server/crypto" "github.com/unitronix/betterdesk-server/db" "github.com/unitronix/betterdesk-server/events" "github.com/unitronix/betterdesk-server/peer" @@ -1373,6 +1374,86 @@ func TestClientTokenAuthorizesViewerOnlyPunch(t *testing.T) { } } +func TestHandleRelayResponseForwardAuthorizesRelayTicket(t *testing.T) { + // #356: P2P→RelayResponse forward must mint a relay ticket before the + // initiator/target reach hbbr, otherwise Claim rejects with + // "Unauthorized relay UUID" and clients see Reset by the peer. + srv, _ := newTestSignalServer(t, config.EnrollmentModeOpen) + initiatorAddr := udpAddr("198.51.100.56", 58616) + targetAddr := udpAddr("203.0.113.56", 59057) + putOnlinePeer(srv, "INIT356", initiatorAddr.IP.String(), initiatorAddr.Port, peer.ConnTCP) + putOnlinePeer(srv, "TGT356", targetAddr.IP.String(), targetAddr.Port, peer.ConnTCP) + + // Keep a TCP punch sink so forward returns before the nil udpConn path. + client, server := net.Pipe() + t.Cleanup(func() { client.Close(); server.Close() }) + go func() { + buf := make([]byte, 64*1024) + for { + if _, err := client.Read(buf); err != nil { + return + } + } + }() + srv.tcpPunchConns.Store(normalizeAddrKey(initiatorAddr.String()), &tcpPunchConn{ + conn: server, + createdAt: time.Now(), + peerID: "INIT356", + }) + + const relayUUID = "356-p2p-relay-fallback-uuid" + msg := &pb.RendezvousMessage{ + Union: &pb.RendezvousMessage_RelayResponse{ + RelayResponse: &pb.RelayResponse{ + SocketAddr: cryptopkg.EncodeAddr(initiatorAddr), + Uuid: relayUUID, + Union: &pb.RelayResponse_Id{Id: "TGT356"}, + }, + }, + } + srv.handleRelayResponseForward(msg, targetAddr) + + if relay.AuthorizeRelayPair(relayUUID, "OTHER356", "TGT356") { + t.Fatal("existing ticket must not rebind to a different initiator") + } + if !relay.AuthorizeRelayPair(relayUUID, "INIT356", "TGT356") { + t.Fatal("same initiator/target pair must remain authorized after forward") + } + if !relay.ClaimRelayPair(relayUUID) { + t.Fatal("first Claim after RelayResponse forward must succeed") + } + if !relay.ClaimRelayPair(relayUUID) { + t.Fatal("second Claim after RelayResponse forward must succeed") + } + if relay.ClaimRelayPair(relayUUID) { + t.Fatal("third Claim must fail after the pair is consumed") + } +} + +func TestHandleRelayResponseForwardRefusesUnresolvedPair(t *testing.T) { + srv, _ := newTestSignalServer(t, config.EnrollmentModeOpen) + initiatorAddr := udpAddr("198.51.100.57", 58617) + targetAddr := udpAddr("203.0.113.57", 59058) + // Target is known; initiator is not in the peer map / TCP session → refuse. + putOnlinePeer(srv, "TGT356B", targetAddr.IP.String(), targetAddr.Port, peer.ConnTCP) + + const relayUUID = "356-unresolved-initiator-uuid" + msg := &pb.RendezvousMessage{ + Union: &pb.RendezvousMessage_RelayResponse{ + RelayResponse: &pb.RelayResponse{ + SocketAddr: cryptopkg.EncodeAddr(initiatorAddr), + Uuid: relayUUID, + Union: &pb.RelayResponse_Id{Id: "TGT356B"}, + }, + }, + } + srv.handleRelayResponseForward(msg, targetAddr) + + if relay.ClaimRelayPair(relayUUID) { + t.Fatal("unresolved initiator must not mint a claimable relay ticket") + } +} + func TestBannedTokenInitiatorRevokesSessionAndRelayTickets(t *testing.T) { srv, database := newTestSignalServer(t, config.EnrollmentModeOpen) if err := database.UpsertPeer(&db.Peer{