mirror of
https://github.com/UNITRONIX/BetterDesk.git
synced 2026-09-10 01:27:11 +00:00
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.
This commit is contained in:
@@ -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).
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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{
|
||||
|
||||
Reference in New Issue
Block a user